execution/commitment/trie, execution/state: clean up dead trie/HashBuilder code#21284
Merged
Conversation
…ld and GenStructStepOld Two small, self-contained dead-code removals: - execution/state/triedb_state.go: the TrieDbState.hashBuilder field was declared and initialised in four constructors (NewTrieDbState / Copy / WithNewBuffer / WithLastBuffer) but never read anywhere — pure dead state. Drop the field and the four trie.NewHashBuilder(false) call sites. - execution/commitment/trie/gen_struct_step.go: GenStructStepOld is superseded by GenStructStepEx (the implementation that the public GenStructStep wraps) and has no callers in-tree. Drop it. The legacy HashCollector / StorageHashCollector function types existed only to type GenStructStepOld's signature and are now also orphan; drop both. HashCollector2 / StorageHashCollector2 remain. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…2 to drop the `2` suffix Now that the legacy HashCollector/StorageHashCollector types are gone, the `2` suffix on the surviving types is meaningless. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The GenStructStep wrapper always called GenStructStepEx with retainProof=nil, cutoff=false, and no other caller used the extended form. Inlining the body with those constants folded in lets the retainIfProving closure and the `proving` branches drop out, and the `cutoff` parameter — which was never even referenced in the body — disappears too. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs dead-code cleanup and API simplification in the execution trie/state layers by removing unused fields and pruning legacy/unused struct-generation helpers in execution/commitment/trie.
Changes:
- Remove the unused
TrieDbState.hashBuilderfield and its constructor initializations. - Delete the legacy
GenStructStepOldpath and renameHashCollector2/StorageHashCollector2toHashCollector/StorageHashCollector. - Inline
GenStructStepExintoGenStructStep, dropping the extra proof/cutoff parameters and related branches.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| execution/state/triedb_state.go | Drops dead hashBuilder state from TrieDbState and removes unused initializations. |
| execution/commitment/trie/gen_struct_step.go | Removes legacy APIs/types and simplifies GenStructStep by inlining the extended variant and pruning proof/cutoff logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ashBuilder After inlining GenStructStepEx into GenStructStep, nothing calls setProofElement on the structInfoReceiver interface anymore, so the HashBuilder.proofElement field and all `if hb.proofElement != nil` branches in leaf / accountLeaf / leafHashWithKeyVal / extensionHash / branchHash are unreachable. Drop the interface method, the field, the setter, and the dead branches. The proofElement type and ProofRetainer machinery in retain_list.go stay (they are still exercised by their own tests directly). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Giulio2002
approved these changes
May 20, 2026
Giulio2002
left a comment
Contributor
There was a problem hiding this comment.
LGTM — straightforward dead-code cleanup and internal rename/inlining with no behavior change.
yperbasis
added a commit
that referenced
this pull request
May 20, 2026
Two real conflicts, both fallout from prior moksha cleanups landing on main via separate spinoff PRs while moksha's local versions of those cleanups were still in flight: 1) execution/commitment/trie/gen_struct_step.go — main inlined GenStructStepEx into GenStructStep and dropped GenStructStepOld / the orphan HashCollector type aliases via #21284 (the spinoff originally cut from this branch). Moksha still had the pre-inlining layout. Took main's file as the base and re-applied moksha's "drop incarnation from structInfoReceiver" change: accountLeaf / accountLeafHash signatures lose `incarnation uint64`, GenStructStepAccountData loses its Incarnation field, the call sites in the inlined GenStructStep drop the argument. 2) execution/state/triedb_state.go — main dropped the dead TrieDbState.hashBuilder field via #21284. Moksha had concurrently dropped the dead incarnationMap field. Both removals are correct on the merged branch; resolution removes both struct fields and their four corresponding constructor inits (NewTrieDbState, Copy, WithNewBuffer, WithLastBuffer). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
Five self-contained cleanups in
execution/commitment/trieandexecution/state(−222 / +9):execution/state/triedb_state.go— theTrieDbState.hashBuilderfield was declared and initialised in four constructors (NewTrieDbState,Copy,WithNewBuffer,WithLastBuffer) but never read anywhere. Pure dead state — drop the field and the fourtrie.NewHashBuilder(false)call sites.execution/commitment/trie/gen_struct_step.go—GenStructStepOldis superseded byGenStructStepEx(the implementation that the publicGenStructStepwraps) and has no callers in-tree. Drop it. The legacyHashCollector/StorageHashCollectorfunction types existed only to typeGenStructStepOld's signature and become orphan onceGenStructStepOldgoes; drop both.HashCollector2→HashCollector,StorageHashCollector2→StorageHashCollector. With the legacy types gone, the2suffix on the survivors is meaningless.GenStructStepExintoGenStructStep. The wrapper always called the extended form withretainProof=nil, cutoff=false, and no other caller used those parameters —cutoffwas never even referenced in the body. Folding the constants in lets theretainIfProvingclosure and theprovingbranches drop out, and the two extra parameters disappear with them.setProofElementintegration inHashBuilder. Once the proof hook is gone fromGenStructStep, nothing callse.setProofElement(...)anymore, soHashBuilder.proofElementcan never be non-nil. Remove the field, the setter, the interface method, and the now-unreachableif hb.proofElement != nilbranches inleaf/accountLeaf/leafHashWithKeyVal/extensionHash/branchHash. TheproofElementtype andProofRetainermachinery inretain_list.gostay (they are still exercised by their own tests directly).Test plan
make erigon integration— cleango build ./...— cleanmake lint— 0 issuesgo test -short -failfast ./execution/state/... ./execution/commitment/...— all green🤖 Generated with Claude Code