rpc: fix debug_executionWitness header fields and system address inclusion (EIP-7928)#21371
Open
lupin012 wants to merge 21 commits into
Open
rpc: fix debug_executionWitness header fields and system address inclusion (EIP-7928)#21371lupin012 wants to merge 21 commits into
lupin012 wants to merge 21 commits into
Conversation
…ess in witness - marshalWitnessHeader: add baseFeePerGas and blockAccessListHash to nullFields (aligned with Geth PR #34972 which renamed balHash → blockAccessListHash); remove stale balHash rename logic that was shadowing the correct field name - RecordingState: add ReallyChangedAccounts to track accounts with actual nonce/balance/codeHash changes, distinct from ModifiedAccounts which is also populated by no-op TouchAccount calls - collectAccessedState: per EIP-7928, exclude system address (0xff...fe) from witness when it has no real state changes and no storage access; TouchAccount on every block's system call was causing a sibling MPT node to appear in the merged witness Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Restores the STEP 1 collapse-sibling detection that was accidentally removed in the previous commit. Without it, trie nodes collapsed during commitment traversal are missing from the witness and stateless re-execution produces a wrong state root (TestExecutionWitness/multiple_blocks fails at block 13). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Log accessed accounts (with reallyChanged/hasStorage flags), sibling collapse paths, witness stats and final node count at Info level. Useful for diagnosing extra MPT nodes vs Geth on mainnet CI runs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tness Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nessTrie Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Revert temporary debug commits: - Remove Info logs added to debug_executionWitness witness generation - Remove primary-vs-total node count log - Re-enable debug_executionWitness/test_01-10 in CI (were disabled for single-test debugging) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to align Erigon’s debug_executionWitness output with Geth by reducing unnecessary witness trie expansions caused by the system address (0xff..fe) being included via no-op touches (notably post-Cancun / EIP-4788-related paths).
Changes:
- Add
ReallyChangedAccountstracking inRecordingStateto distinguish “touched/modified” from “actually changed” account updates. - Filter the system address out of the accessed address set when it did not experience “real” changes.
- Update
marshalWitnessHeadernull-field handling and adjust CI rpc-tests version pin.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
rpc/jsonrpc/debug_execution_witness.go |
Adds “really changed” tracking and filters SystemAddress from witness inputs; changes header marshalling behavior. |
.github/workflows/scripts/rpc_version.env |
Updates the rpc-tests version/branch used by QA workflows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lupin012
changed the title
The !deleted guard added in 36dcd84 prevented filtering the system address (0xff...fe) from the witness when Erigon marks it as deleted. In EIP-4788 Cancun blocks the EVM cleanup deletes this zero-state account, causing it to stay in the witness and adding spurious nodes (tests 141-144). Deletion of a non-existent account is a no-op; it is not meaningful state change and must not block the sysAddr filter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…eleted flag Replace the simple !deleted guard with a pre/post existence comparison. The system address (0xff...fe) is filtered from the witness unless its account presence in the trie actually changed: non-existent→existent or existent→non-existent. A spurious DeletedAccounts entry caused by EVM cleanup of a zero-state account (e.g. EIP-4788 Cancun blocks) is no longer mistaken for a real deletion. This also correctly handles AuRa/Gnosis chains where TouchAccount creates a real empty system account. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
yperbasis
approved these changes
May 26, 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.
With this fix, the responses from Geth and Erigon are fully aligned for all blocks on the Hive chain (from 8 to 44). For blocks 1 to 7, Geth crashes because it doesn't support witness calculation pre-Byzantium
Mainnet little improve but it is not enough
The fix resolves the system-address witness pollution introduced by EIP-4788/EIP-2935
(post-Cancun blocks). The system address (
0xff...fe) was being included in the witnesson every block via a no-op
TouchAccountcall, causing its MPT sibling to be expandedunnecessarily. The
ReallyChangedAccountsfilter removes it when no actual state changeoccurred.
Tests 06 and 10 show a −4 reduction for the same reason as test_05 (EIP-4788 active),
but remain non-zero because of additional extra nodes whose root cause is not yet identified.
Tests 01–04, 08–09 (pre-Cancun or non-EIP-4788 blocks) are unaffected by this fix.
debug_executionWitness — Reth vs Erigon/Geth Differences (Hive Chain Blocks 1–44)
1. State — Extra
0x80Node (All Tests)0x80) in the state array and This list is sorted in different way. This represents the empty storage trie root for accounts that do not have any storage.2. Headers[0] — Serialization Format
3. Keys — Accessed Addresses
keysfield with the list of addresses accessed during block execution.nullfor this field.4. Codes — Bytecode Inclusion
GetCodeorGetCodeSize. Consequently, some tests return 0 codes in Erigon versus 2–5 codes in Reth.