rpc/jsonrpc: keep system address in executionWitness when a tx opcode touches it#21565
Merged
Conversation
… touches it debug_executionWitness dropped the system address (0xff..fe) unless it had a state change, on the rationale that it is only ever the msg.sender of begin/ end-block system calls. But a user transaction can access it via an account- accessing opcode (BALANCE/EXTCODE*/CALL/STATICCALL); per EIP-7928 that is an account-only entry that must appear in the witness. Such a read often hits the state-object cache (the account was loaded during the system call), so the recorder never sees it. Detect the access via the per-transaction access set (ibs.AccessedAddresses()) and keep the system address when a user tx touched it.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a gap in debug_executionWitness where the system address (0xff…fe) could be dropped from the witness even when a user transaction accessed it via an account-accessing opcode (e.g. BALANCE, EXTCODE*, CALL), by leveraging the per-transaction address access set to detect such touches even when the state-object cache prevents the RecordingState reader from observing the read.
Changes:
- Add
RecordingState.systemAddrTouchedInTxto remember if any user tx accessed the system address via an opcode. - After each
ApplyMessage, inspectibs.AccessedAddresses()and mark the system address as touched when present. - Adjust
collectAccessedStateto retain the system address even without state change whensystemAddrTouchedInTxis set.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+613
to
+618
| // A user tx that accesses the system address via an opcode keeps it in the | ||
| // witness; the per-tx access set captures this even on state-cache hits. | ||
| if acc := ibs.AccessedAddresses(); acc != nil { | ||
| if _, ok := acc[params.SystemAddress]; ok { | ||
| recordingState.MarkSystemAddrTouchedInTx() | ||
| } |
yperbasis
approved these changes
Jun 2, 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.
debug_executionWitnessdropped the system address (0xff..fe) from the witness unless it had a state change — the rationale being it's only ever themsg.senderof begin/end-block system calls. But a user transaction can access it via an account-accessing opcode (BALANCE/EXTCODESIZE/EXTCODEHASH/EXTCODECOPY/CALL/STATICCALL); per EIP-7928 that's an account-only entry the witness must carry. Such a read usually hits the state-object cache (the account was loaded during the system call), so theRecordingStatereader never sees it.Fix: detect the access via the per-transaction access set (
ibs.AccessedAddresses()) and keep the system address when a user tx touched it.Verified against the EEST zkevm witness suite:
bal_account_touch_system_address(all 6 opcode variants: balance/call/extcodesize/extcodehash/extcodecopy/staticcall) goes green, fullfor_amsterdamcorpus shows no regressions. The zkevm suite lives on #21487 until merged, so its CI coverage of this fixture arrives once that lands.make lintclean.