Skip to content

Detect Geth genesis format independent of property order - #12784

Open
flcl42 wants to merge 5 commits into
NethermindEth:masterfrom
flcl42:geth-genesis-format-detection
Open

Detect Geth genesis format independent of property order#12784
flcl42 wants to merge 5 commits into
NethermindEth:masterfrom
flcl42:geth-genesis-format-detection

Conversation

@flcl42

@flcl42 flcl42 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Changes

  • Make Geth genesis auto-detection order-independent by checking for a top-level config property while preserving Parity-style files that also contain config alongside engine, params, genesis, or accounts.
  • Scan with a pooled incremental JSON reader, skip nested values, and avoid materializing a JSON document.
  • Preserve Parity fallback and support non-seekable input streams.

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

Parameterized regression coverage uses one test source for Geth config in its normal position, after nonce, and after a value larger than the initial detection buffer; separate coverage protects Parity chainspecs with a top-level config. Existing Merge/TTD tests also pass.

Documentation

Requires documentation update

  • Yes
  • No

Requires explanation in Release Notes

  • Yes
  • No

@flcl42

flcl42 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for this change — the order-independent detection and the pooled incremental reader look solid. I reviewed the implementation and stress-tested the streaming/resume logic (BytesConsumed shifting, buffer growth, TrySkip after a split value) across many buffer sizes; it behaves correctly, and the new tests cover the interesting cases (config-first, config-after-nonce, config-after-a-value-larger-than-the-buffer).

Two observations, both non-blocking:

1. The non-seekable path now fully buffers the stream. Load does streamData.CopyTo(bufferedStream) for any non-seekable input. The previous PrefixedStream existed specifically to avoid materializing a non-seekable stream (it only buffered a 256-byte header and replayed it). This path is actually exercised in production: ZstdChainSpecLoader passes a ZstdSharp.DecompressionStream to this loader, and that stream reports CanSeek == false (verified against ZstdSharp.Port 0.8.8). So every .zst chainspec now has its entire decompressed contents copied into a MemoryStream. Not a correctness issue, but it regresses the exact memory behavior PrefixedStream was introduced for, and the PR's "avoid materializing a JSON document" note only holds for seekable inputs.

2. Geth detection now scans the whole top level. Because the code can only decide "Geth" after reaching the root EndObject (to rule out engine/params/genesis/accounts), Geth files get a full skip-scan during detection and then a full re-parse by GethGenesisLoader. Previously detection was O(first 256 bytes). This is inherent to order-independent detection and the skip makes it cheap (no allocations), but for a genesis with a large alloc it means two full passes. Reasonable tradeoff, just calling it out.

Nothing that should block merge from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant