Conversation
ApplyTransactionWithEVM decided inline which handler a transaction goes through, so any caller that has to replay a transaction had to re-derive that decision on its own — and every copy drifts from block processing the moment the routing changes. Pull the decision into routeTransaction, the per-transaction finalisation into finaliseTxState, and the sender nonce handling of ApplySignTransaction into applySignTransactionNonce, so a replay can follow exactly the same path. The log both non-EVM handlers record moves into addNonEVMTxLog for the same reason: it is not a receipt artefact, StateDB.AddLog advances the block wide log count that every later log takes its index from, so a copy that skipped it would make the following transactions report indexes the chain never had. Also add core.ApplyTransactionForReplay, the entry point a replay should use instead of core.ApplyMessage: it shares the routing above, records the same log through the same helper, and skips the receipt and its bloom, which a replay does not need. It refuses an EVM that carries a tracer, as it never fires the OnTxStart/OnTxEnd hooks, and it takes the state to replay on from evm.StateDB, so the finalisation, the nonce handling and the TRC21 fee handling cannot land on a state other than the one the execution wrote to. Both refusals are sentinel errors, so a caller can tell them apart from a transaction failure. The replay paths converted in the commits that follow go through it. No behaviour change: the conditions, the finalisation, the nonce handling and the log are the ones ApplyTransactionWithEVM already used. finaliseTxState takes the state the EVM executes against rather than the plain *state.StateDB, because a hooked state reports the balance burnt by self-destructed accounts to the tracer on Finalise (core/state/statedb_hooked.go); IntermediateRoot forwards unchanged either way. TestApplyTransactionForReplayKeepsTheNonEVMTxLog drives the replay and block processing of a transaction to a system address and compares the logs they record.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Sep 19, 2026
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai and
wanwiset25
and
a balanced review from Copilot
September 19, 2026 10:20
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Automated coverage is missing for the documented post-receiver-fork nonce regression.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Fixes debug_intermediateRoots by replaying transactions through block-processing-compatible routing.
Changes:
- Preserves roots for native XDCX/sign transactions.
- Replays fee ownership, nonce handling, and historical balance behavior correctly.
- Adds regression tests comparing replayed roots and native transaction effects.
| File | Description |
|---|---|
eth/tracers/api.go |
Uses the shared replay transaction path. |
eth/tracers/api_intermediate_roots_test.go |
Tests roots against block processing. |
core/state_processor.go |
Adds shared routing and replay helpers. |
core/state_processor_test.go |
Tests native replay logs and nonce behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
debug_intermediateRoots rebuilt the pre-state of a block with core.ApplyMessage, which knows nothing about the routing block processing applies, and skipped every transaction sent to the XDCX system addresses outright. While the receiver fork is active those transactions are handled by ApplyEmptyTransaction, which neither checks nor increments the sender nonce and leaves the state untouched, whereas the replay executed them as ordinary EVM transactions and bumped the nonce; the block fee was credited to the zero address instead of the coinbase owner, and the historical balance bypass was not applied either. It therefore returned one root fewer than the block has transactions, and the roots it did return were not the ones block processing produces. Replay through core.ApplyTransactionForReplay, the entry point that shares the routing with ApplyTransactionWithEVM but skips the receipt, its logs and the bloom, and stop skipping those transactions: every transaction then gets an intermediate root, and a nonce-less one leaves the state untouched, so its root is the previous one. Add TestIntermediateRootsMatchesBlockProcessing, which compares the roots against a replay through core.ApplyTransactionWithEVM, pins the last one to the block root after the block finalisation and fails when the coinbase owner fee is dropped; the base has no intermediate-roots test, so this one is also the first to pin the replay instead of only counting its roots. No corresponding fix exists upstream: geth has no non-EVM transaction concept.
gzliudan
force-pushed
the
fix-trace-replay-intermediate-roots
branch
from
September 19, 2026 10:32
7f8d548 to
bdda0d9
Compare
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.
Proposed changes
debug_intermediateRootsrebuilds the pre-state of a block by re-executing the transactions before each one. It did that withcore.ApplyMessage, which knows nothing about the routing block processing applies, and it also dropped every transaction sent to the XDCX system addresses0x91/0x92/0x93/0x94withtx.IsSkipNonceTransaction(), a condition that does not consult the fork at all.Inside the XDCX receiver fork window (mainnet
TIPXDCXBlock38,383,838…TIPXDCXReceiverDisableBlock80,370,900) those transactions are handled byApplyEmptyTransaction: no EVM execution, no nonce check, no nonce increment and no state change, so the skip keeps the state right but leaves one root missing. Outside the window block processing does execute them and does bump the nonce the following transaction relies on, so dropping them loses exactly that increment and the replay hits the error on the next transaction. The replay also credited the block fee to the zero address instead of the coinbase owner and skipped the historical balance bypass, so even the roots it did return were not the ones block processing produces.The same unconditional skip in
traceBlockand in the state feeder of the JS tracer path — the one that makesdebug_traceBlock*fail withnonce too highoutside the window and leave anullhole inside it — is a separate change, submitted as #2581.Symptoms
debug_intermediateRootswhenever the receiver fork is active (mainnet, one root fewer per skip-nonce transaction).IntermediateRootshits the replay error caused by the missing nonce increment and silently returns the roots collected so far (mainnet block 37,849,457,0x2418971, 204 transactions, returns 94 roots).debug_intermediateRootsroot diverges from the block state root whenever the block charges a non-zero fee, because the replay credited it to the zero address instead of the coinbase owner (mainnet block 107,084,658,0x661FB72).Fix
IntermediateRootsreplays throughcore.ApplyTransactionForReplay, the entry point that shares the routing withApplyTransactionWithEVMbut skips the receipt, its logs and the bloom, and it stops skipping the transactions sent to the system addresses. Every transaction then gets an intermediate root, a nonce-less one leaves the state untouched so its root is the previous one, the coinbase owner is credited the way block processing credits it, and the historical balance bypass applies. An EVM this replay cannot use is reported as it is instead of returning a short root list.Upstream
No upstream fix to port: geth has no non-EVM transaction concept, so neither the trailing
ApplyMessagenor the unconditional skip have a geth counterpart.Tests
eth/tracers/api_test.go—TestIntermediateRootsMatchesBlockProcessing: compares the roots against a replay throughcore.ApplyTransactionWithEVM, pins the last root to the block root after block finalisation and fails when the coinbase owner fee is dropped; the base has no intermediate-roots test, so this one is also the first to pin the replay instead of only counting its roots. The case drives a fee paying transaction, a transaction to an XDCX system address and a follower of the same sender reusing its nonce.End-to-end verification
The branch binary was run against archive nodes of both live networks, on the same data directory and the same node as the baseline binary (
dev-upgrade@cdce8fc5c), and the tracing RPCs were compared on the same blocks.Mainnet block 37,849,457 (
0x2418971, 204 transactions, outside the receiver fork window, contains sign transactions):debug_intermediateRootsstateRootMainnet block 39,083,312 (
0x2545d30, 14 transactions, inside the window; tx 0 goes to0x92, tx 5 to0x90from the same sender with the same nonce) and Apothem block 48,667,667 (0x2e69c13, 8 transactions; tx 0 to0x92, tx 5 to0x89from the same sender with the same nonce):debug_intermediateRootsstateRootMainnet block 107,084,658 (
0x661FB72, 6 transactions, after the receiver-disable fork, no system-address transaction,baseFeePerGas12.5 Gwei): onlydebug_intermediateRootsdiffers — the baseline last root is not the blockstateRoot, this branch's is.Manual test plan: start a node on that data directory with
--rpcapi debugand calldebug_intermediateRootson0x2418971,0x2545d30and0x2e69c13; the baseline binary truncates or diverges on every one of them and this branch passes them.Regression: on both networks this branch imported testnet and mainnet segments normally, with no bad block, no panic and no error attributable to the change.
Types of changes
Impacted Components
debug_intermediateRootsJSON-RPC method)Checklist
Relation to other work
This is the second of three PRs that split a single debug tracing series. It depends on the replay entry point #2589 adds (
core.ApplyTransactionForReplay, together with the shared routing it follows); with that PR merged, this branch rebases trivially.The third (#2591) fixes the traces and intermediate roots of the block that activates TIPSigning, which did not remove the legacy block signers account block processing removes before the first transaction.
To keep reviewability, this PR keeps its test in a file of its own (
eth/tracers/api_intermediate_roots_test.go) instead of appending to the sharedeth/tracers/api_test.go, so it does not overlap the other two PRs textually.#2581 fixes the unconditional skip in
traceBlockand in the state feeder of the JS tracer path — thenonce too highand thenullhole indebug_traceBlock*. It carries its own copy of theskipNonceForkCases/newSkipNonceBackendfixtures, which are introduced by the first PR of this series, because its block-level tests build on them.