execution/commitment: fix parallel/streaming deep-fold correctness bugs#22184
Merged
Conversation
- updateCell sheds a reused cell's stale storage plain key when stamping an account key, so an injected deep-fold storage root is no longer shadowed by a stale storage slot in computeCellHash. - aggregateMountedStorageRoot computes the storage root for a single-surviving- child collapse directly at the account boundary (extension/leaf) instead of folding to the trie root and prepending the account prefix (panic / wrong hash). - seedRootBase gives every mount base a real wall row so foldMounted returns one uniform convention; the stitchSplitCells extension strip is removed and the foldMounted no-wall fallback becomes a hard error.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes three correctness issues in the experimental parallel/streaming commitment “deep fold” paths (--experimental.parallel-commitment / --experimental.streaming-commitment). The fixes target scenarios that could previously yield an incorrect state root, corrupt persisted branch data across blocks, or panic during storage-root folding, while keeping the default (sequential) commitment path unchanged.
Changes:
- Fix
updateCellreuse semantics by clearing stale storage identity when stamping an account key, preventing stale-slot hashing from overriding an injected deep-fold storage root. - Correct single-surviving-child storage-root aggregation by constructing the extension/leaf root directly (avoids account-prefix contamination that could overflow extensions or return the wrong hash).
- Make mounted folding consistently stop at the mount boundary by seeding an explicit “row-0 wall”, removing the now-unnecessary mount-nibble stripping in split stitching and turning the prior no-wall fallback into a hard error; add regression tests covering the failure modes.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| execution/commitment/streaming_deep_fold.go | Builds correct storage roots for single-child collapses without prepending the account prefix; avoids panic/wrong-root cases. |
| execution/commitment/streaming_commitment.go | Seeds the base so mounted folds return cells excluding the mount nibble; removes stitching-time extension stripping. |
| execution/commitment/parallel_mount.go | Adds seedRootBase and uses it for parallel mounted processing to ensure foldMounted stops at the mount wall. |
| execution/commitment/nibble_addr_test.go | Adds a helper to brute-force addresses sharing an arbitrary hashed-nibble prefix for deterministic regression scenarios. |
| execution/commitment/hex_patricia_hashed.go | Clears stale storage fields when converting a reused cell to an account cell; makes folding past an unseeded mount wall a hard error. |
| execution/commitment/deepfold_regression_test.go | Adds focused regression tests for the three reported correctness issues (wrong root, persisted-branch corruption, panic). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
AskAlexSharov
approved these changes
Jul 3, 2026
…etAccountStorageRoot, not updateCell updateCell runs on every account update, so clearing a cell's stale storage identity there dropped a singleton account's storage on an account-only re-touch — a wrong state root on the default commitment path. Move the reset into setAccountStorageRoot, whose only caller is the deep fold, so the injected-storage-root fix stays confined to the experimental parallel/streaming path.
…ccount-only re-touch A persistent-trie test (not cross-engine, which is blind to a bug shared by all engines) that a singleton account re-touched account-only keeps its storage slot — guards the default path against the deep-fold storage reset leaking back into updateCell.
mh0lt
added a commit
that referenced
this pull request
Jul 3, 2026
Bring the cache stack current with main (parallel/streaming commitment correctness fixes #22184/#22113, nibblized-keccak cache #22185, erigondb.toml commitment referencing #21452, trie io.Writer trace #21859, etc.). One conflict in commitment_context.go's trieContext: keep both this branch's probeSd/probeTx (adaptive trunk-pin probe) and main's traceW (io.Writer trace).
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Three correctness bugs in the parallel/streaming commitment deep fold (gated behind
--experimental.parallel-commitment/--experimental.streaming-commitment, default off). On affected inputs each yields a wrong state root, a corrupted persisted branch, or a panic. All reproduced red-first and verified against the sequential trie (root + stored-branch parity).Changes
updateCellclears a reused cell's stale storage plain key when stamping an account key, so an injected deep-fold storage root is not shadowed by the stale slot incomputeCellHash.aggregateMountedStorageRootbuilds the storage root for a single-surviving-child collapse directly at the account boundary. The oldbase.fold()prepended the 64-nibble account prefix, overflowingcell.extension(panic) or returning the child hash instead of the extension hash (all-zero / wrong root).seedRootBaseseeds a row-0 wall sofoldMountedalways excludes the mount nibble; thestitchSplitCellsstrip is removed and the no-wall fallback becomes a hard error. Fixes an over-strip of extension-topped cells that corrupted the persisted branch and diverged the next block.No change to the default single-trie commitment path.