execution/commitment: drop dead accessors and a vestigial parameter - #23185
Merged
Conversation
- cell.GetAccountAddr / cell.GetStorageAddr: no reference anywhere, including tests. - CompactKey: no reference anywhere, including tests. - collectDeleteUpdate's evictCache parameter: #21380 removed the hph.cache.EvictBranch call it gated when the trie stopped owning a cache, leaving the parameter unread and its docstring describing eviction that no longer happens. All three call sites passed true. EvictBranch no longer exists in the tree, so nothing moved rather than being lost.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR cleans up the execution/commitment package by removing unused accessors/helpers and simplifying an internal API after the cache-ownership changes introduced in the parallel-path audit.
Changes:
- Removed dead
cellaccessors (GetAccountAddr,GetStorageAddr) from the trie reader. - Removed the unreferenced
CompactKeyhelper (and its now-unusederrorsimport). - Dropped the vestigial
evictCacheparameter fromHexPatriciaHashed.collectDeleteUpdateand updated all call sites accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| execution/commitment/trie_reader.go | Removes unused cell address accessor methods. |
| execution/commitment/streaming_deep_fold.go | Updates collectDeleteUpdate call to match the simplified signature. |
| execution/commitment/keys_nibbles.go | Removes unreferenced CompactKey helper and cleans up imports. |
| execution/commitment/hex_patricia_hashed.go | Simplifies collectDeleteUpdate signature and updates internal callers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
AskAlexSharov
approved these changes
Aug 11, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Aug 11, 2026
# Conflicts: # execution/commitment/trie_reader.go
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 12, 2026
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 unreferenced things in the commitment package, found while auditing the parallel path.
Changes
cell.GetAccountAddr/cell.GetStorageAddr— no reference anywhere, including tests.CompactKey— no reference anywhere, including tests.collectDeleteUpdate'sevictCacheparameter — State Cache Consolidation (PR #1 of the perf stack) #21380 removed thehph.cache.EvictBranchcall it gated when the trie stopped owning a cache, leaving the parameter unread and its docstring describing eviction that no longer happens. All three call sites passedtrue.EvictBranchno longer exists in the tree, so the invalidation moved rather than being lost.