You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
configproperty while preserving Parity-style files that also containconfigalongsideengine,params,genesis, oraccounts.Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
Parameterized regression coverage uses one test source for Geth
configin its normal position, afternonce, and after a value larger than the initial detection buffer; separate coverage protects Parity chainspecs with a top-levelconfig. Existing Merge/TTD tests also pass.Documentation
Requires documentation update
Requires explanation in Release Notes