feat(l1): add debug_traceCall and debug_traceBlockByHash#6931
Conversation
Lines of code reportTotal lines added: Detailed view |
🤖 Kimi Code ReviewReview of PR #6931 (adding Issues Found1. Integer truncation risk ( let tx_index = self.trace_config.tx_index.map(|i| i as usize);Converting let tx_index = match self.trace_config.tx_index {
Some(i) => Some(i.try_into().map_err(|_| RpcErr::BadParams("tx_index out of range".to_owned()))?),
None => None,
};2. Magic number for disabled gas limit ( env.block_gas_limit = i64::MAX as u64; // disable block gas limitPrefer env.block_gas_limit = u64::MAX;Security & Correctness
Code Quality
Recommendations
Overall: Approved with minor fixes. The implementation correctly follows geth's Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code Review
Aside from those points, the refactor is straightforward and the VM-level tests are useful. I’d add RPC coverage for Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryThis PR adds geth-style
Confidence Score: 4/5The new RPC path can return incorrect traces or reject compatible requests.
crates/networking/rpc/tracing.rs
|
| Filename | Overview |
|---|---|
| crates/networking/rpc/tracing.rs | Adds TraceCallRequest, config parsing, block resolution, tracer dispatch, and response shaping; the RPC edge cases need fixes. |
| crates/blockchain/tracing.rs | Adds blockchain trace-call methods that rebuild parent state, rerun the target block, and invoke VM tracing. |
| crates/vm/backends/levm/tracing.rs | Refactors shared tracer execution and adds LEVM call-style tracing through generic transaction environment setup. |
| crates/vm/tracing.rs | Adds straightforward EVM trace-call wrappers for the LEVM backend. |
| crates/networking/rpc/rpc.rs | Registers debug_traceCall in the debug namespace. |
| test/tests/levm/trace_call_tests.rs | Adds VM-level tests for call tracer sender behavior, opcode tracing, and prestate diff tracing. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant RPC as TraceCallRequest
participant BC as Blockchain
participant EVM as Evm/LEVM
Client->>RPC: debug_traceCall(callObject, block, traceConfig)
RPC->>RPC: parse block, tracer config, txIndex
RPC->>BC: "trace_call_* with GenericTransaction"
BC->>BC: rebuild_parent_state(parent_hash, reexec)
BC->>EVM: rerun_block(block, txIndex)
EVM->>EVM: prepare_call_env(GenericTransaction)
EVM->>EVM: execute synthetic call with tracer
EVM-->>RPC: trace result
RPC-->>Client: call frame / prestate / structLogs
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant RPC as TraceCallRequest
participant BC as Blockchain
participant EVM as Evm/LEVM
Client->>RPC: debug_traceCall(callObject, block, traceConfig)
RPC->>RPC: parse block, tracer config, txIndex
RPC->>BC: "trace_call_* with GenericTransaction"
BC->>BC: rebuild_parent_state(parent_hash, reexec)
BC->>EVM: rerun_block(block, txIndex)
EVM->>EVM: prepare_call_env(GenericTransaction)
EVM->>EVM: execute synthetic call with tracer
EVM-->>RPC: trace result
RPC-->>Client: call frame / prestate / structLogs
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
crates/networking/rpc/tracing.rs:431
**Out-Of-Range Index Builds Impossible State**
When `txIndex` is greater than the selected block's transaction count, `rerun_block` never reaches the stop index, so it executes every transaction but skips withdrawals because the stop index is still `Some`. The trace then runs on a post-transaction, pre-withdrawal state that never existed on chain, so callers can receive incorrect trace output instead of a bounds error.
### Issue 2 of 3
crates/networking/rpc/tracing.rs:397-405
**Null Optional Params Are Rejected**
When a geth-compatible caller sends `debug_traceCall(callObject, null, {"tracer":"callTracer"})`, `params.get(1)` is present and `BlockIdentifierOrHash::parse` receives JSON `null` instead of defaulting to `latest`. The request returns a bad-params error, and a null third parameter fails the same way, even though these are optional RPC arguments.
### Issue 3 of 3
crates/networking/rpc/tracing.rs:404-405
**Default Tracer Returns Wrong Shape**
When `traceConfig` is omitted, `TraceCallConfig::default()` uses the shared `CallTracer` default, but geth's `debug_traceCall` default is the opcode struct logger. A caller that sends only `debug_traceCall(callObject)` receives a call-frame object instead of the expected `structLogs` response, which breaks clients that parse the geth default shape.
Reviews (1): Last reviewed commit: "feat(l1): add debug_traceCall RPC method" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| // `None` traces on top of the full block (geth's default); `Some(i)` runs the | ||
| // call against the state just before the block's transaction `i`. | ||
| let tx_index = self.trace_config.tx_index.map(|i| i as usize); |
There was a problem hiding this comment.
When txIndex is greater than the selected block's transaction count, rerun_block never reaches the stop index, so it executes every transaction but skips withdrawals because the stop index is still Some. The trace then runs on a post-transaction, pre-withdrawal state that never existed on chain, so callers can receive incorrect trace output instead of a bounds error.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 431
Comment:
**Out-Of-Range Index Builds Impossible State**
When `txIndex` is greater than the selected block's transaction count, `rerun_block` never reaches the stop index, so it executes every transaction but skips withdrawals because the stop index is still `Some`. The trace then runs on a post-transaction, pre-withdrawal state that never existed on chain, so callers can receive incorrect trace output instead of a bounds error.
How can I resolve this? If you propose a fix, please make it concise.| let block = match params.get(1) { | ||
| Some(value) => BlockIdentifierOrHash::parse(value.clone(), 1)?, | ||
| None => BlockIdentifierOrHash::Identifier(BlockIdentifier::default()), | ||
| }; | ||
|
|
||
| let trace_config = match params.get(2) { | ||
| Some(value) => serde_json::from_value(value.clone())?, | ||
| None => TraceCallConfig::default(), | ||
| }; |
There was a problem hiding this comment.
When a geth-compatible caller sends debug_traceCall(callObject, null, {"tracer":"callTracer"}), params.get(1) is present and BlockIdentifierOrHash::parse receives JSON null instead of defaulting to latest. The request returns a bad-params error, and a null third parameter fails the same way, even though these are optional RPC arguments.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 397-405
Comment:
**Null Optional Params Are Rejected**
When a geth-compatible caller sends `debug_traceCall(callObject, null, {"tracer":"callTracer"})`, `params.get(1)` is present and `BlockIdentifierOrHash::parse` receives JSON `null` instead of defaulting to `latest`. The request returns a bad-params error, and a null third parameter fails the same way, even though these are optional RPC arguments.
How can I resolve this? If you propose a fix, please make it concise.| None => TraceCallConfig::default(), | ||
| }; |
There was a problem hiding this comment.
When traceConfig is omitted, TraceCallConfig::default() uses the shared CallTracer default, but geth's debug_traceCall default is the opcode struct logger. A caller that sends only debug_traceCall(callObject) receives a call-frame object instead of the expected structLogs response, which breaks clients that parse the geth default shape.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 404-405
Comment:
**Default Tracer Returns Wrong Shape**
When `traceConfig` is omitted, `TraceCallConfig::default()` uses the shared `CallTracer` default, but geth's `debug_traceCall` default is the opcode struct logger. A caller that sends only `debug_traceCall(callObject)` receives a call-frame object instead of the expected `structLogs` response, which breaks clients that parse the geth default shape.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
🤖 Claude Code ReviewNow I have all angles covered. Let me synthesize the final review from the surviving findings. Review:
|
Read the block's committed post-state directly (like eth_call) when no txIndex is given, instead of rebuilding parent state and re-running every transaction in the block. Falls back to the rebuild+rerun path for a specific txIndex or when the block's state isn't stored.
Mirror debug_traceBlockByNumber for a block hash. Extract the shared tracer dispatch into a trace_block helper used by both handlers.
Replace the per-transaction spawn_blocking + Arc<Mutex> loop with a single blocking pass over the block, and compute the block-invariant EVM config once instead of per transaction. Removes N-1 thread handoffs and N config recomputes per block trace. The trace timeout now bounds the whole block trace rather than each transaction.
edg-l
changed the title
- callTracer top-frame gasUsed now reports post-refund gas (matches receipt) - debug_traceCall fills nonce from account state when omitted - decode Error(string) revertReason; map revert error to "execution reverted" - omit empty fields (to/value/output/error/revertReason/calls) like geth - drop `to` on failed CREATE; omit value for STATICCALL - propagate parent-failure when clearing reverted logs - emit block-absolute log index for withLog traces
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
| None => { | ||
| let nonce = context | ||
| .storage | ||
| .get_nonce_by_account_address(block_number, self.transaction.from) |
There was a problem hiding this comment.
Considering the post-state will lead the transaction to fail due to a nonce mismatch when tx_index is set (ie it isn't added on top of the block).
This isn't a problem in geth because they skip the nonce check.
| #[serde(flatten)] | ||
| base: TraceConfig, | ||
| #[serde(default)] | ||
| tx_index: Option<u64>, |
There was a problem hiding this comment.
The doc above says stateOverrides/blockOverrides "are not supported", but with #[serde(flatten)] on base and no deny_unknown_fields on TraceCallConfig, a client that sends either field gets its trace run without the overrides — silently, with no error. Geth's TraceCallConfig includes both, so this is a fairly high-probability compat mismatch (a client relying on geth semantics gets a trace against unexpected state). Two loud-fail options: #[serde(deny_unknown_fields)] on the whole struct, or explicit state_overrides: Option<Value> / block_overrides: Option<Value> fields that reject with BadParams("stateOverrides not supported") when Some. The explicit form has the advantage of surviving geth adding other new fields in the future without breaking us.
- skip nonce check in debug_traceCall (geth parity): a call traced on top
of a mid-block state (txIndex) no longer fails on a nonce mismatch
- treat JSON null block/traceConfig params as omitted (accept
traceCall(call, null, {...}))
- reject stateOverrides/blockOverrides explicitly instead of silently
ignoring them
- split two glued docstrings in levm tracing
- VM-level: trace_call skips the sender nonce check (mismatched nonce) - RPC parse: null block/traceConfig params accepted, txIndex parses, stateOverrides/blockOverrides rejected - accept geth-style hex-string txIndex (hexutil.Uint) in addition to a JSON number - expose the rpc tracing module so its handlers can be unit-tested
…-alt # Conflicts: # crates/networking/rpc/lib.rs # crates/networking/rpc/rpc.rs
3 participants
Adds two geth-style debug tracing endpoints and tidies up the block-trace path while doing so.
This started as
debug_traceCallbut grew to coverdebug_traceBlockByHashtoo, since Dora calls that on every block on the devnet.What's new
debug_traceCall(callObject, blockNrOrHash, traceConfig)callTracer,prestateTracer,opcodeTracer.latest.txIndexconfig field (trace the call against the state just before txi). Out-of-rangetxIndexis rejected to match geth instead of silently tracing against a bogus state.stateOverrides/blockOverridesare not supported (documented in the config struct).eth_callmachinery: the call is an unsignedGenericTransaction, so the sender comes from thefromfield with no signature recovery, and fees/base-fee are relaxed the same waysimulate_tx_from_genericdoes.debug_traceBlockByHash(blockHash, traceConfig)debug_traceBlockByNumber; this mirrors it for a block hash. The two now share a singletrace_blockdispatch helper.Performance
Dora hits these on every block, so two things got optimized:
debug_traceCallno longer re-executes the block. For the default case (notxIndex) it reads the block's committed post-state directly, likeeth_call, instead of rebuilding the parent state and replaying every transaction. Falls back to rebuild + rerun for a specifictxIndexor when the block's state isn't stored (archive/pruned gap). A defaulttraceCallon an N-transaction block drops from ~N+1 EVM executions to 1.debug_traceBlock*traces the whole block in one pass. Replaced the per-transactionspawn_blocking+Arc<Mutex>loop with a single blocking pass, and the block-invariantEVMConfigis computed once instead of per transaction. Removes N-1 thread handoffs and N config recomputes per block trace. Side effect: the trace timeout now bounds the whole block trace rather than each transaction, which is the more sensible budget for this endpoint anyway.Implementation notes
Layered the same way as the existing tracers:
crates/vm/backends/levm/tracing.rs: refactored the threetrace_tx_*fns to sharerun_*cores, addedtrace_call_*(generic-tx) andtrace_block_*(batch) variants on top.crates/vm/tracing.rs:Evmwrappers.crates/blockchain/tracing.rs:Blockchain::trace_call_*and the rewritten one-passtrace_block_*.crates/networking/rpc/tracing.rs:TraceCallRequest,TraceBlockByHashRequest, sharedtrace_blockhelper.crates/networking/rpc/rpc.rs: routes both new methods.Tests
Three VM-level tests in
test/tests/levm/trace_call_tests.rscovering the call / opcode / prestate-diff paths, pinning the from-field sender behaviour. The existing opcode and prestate tracer tests still pass after the core refactor.Possible follow-up (not in this PR)
On Amsterdam the block carries a BAL (EIP-7928), which records per-transaction state changes. That makes it possible to reconstruct each transaction's pre-state from the parent state plus the BAL without replaying predecessors, which would let
debug_traceBlock*trace transactions in parallel instead of sequentially. Out of scope here (correctness risk, needs the per-block BAL available at trace time), but worth a separate issue.callTracer /
debug_traceCallgeth-compatibility fixesFollow-up on a glamsterdam-devnet-6 report: execution was consensus-correct, but the callTracer reporting layer diverged from geth's de-facto output. Fixed by comparing against go-ethereum (
eth/tracers/native/call.go):gasUsed(correctness bug): the top-level frame now reports post-refund gas — matching the receipt and geth'scallstack[0].GasUsed = receipt.GasUsed— instead of the pre-refund / EIP-7778 block-accounting value. Previously overstated gas on refund txs (e.g. by the full EIP-3529 refund).debug_traceCallnonce: fills the nonce from account state when omitted (matching geth'sToMessageand our owneth_estimateGas) instead of rejecting the call on a nonce mismatch against a default of 0.revertReason: decodes the ABIError(string)payload (geth'sabi.UnpackRevert) instead of raw UTF-8, which producednullfor standardrevert("...")."execution reverted"; static halts map to geth's wording (out of gas,write protection,contract address collision,max code size exceeded, …).to/value/output/error/revertReason/callslike geth'somitempty; droptoon a failed CREATE; omitvaluefor STATICCALL.clearFailedLogs), and emit a block-absolute logindex(geth'slog.Index) forwithLogtraces.Scope / compatibility: these changes touch only the callTracer (
CallTraceFrame/CallLog/LevmCallTracer). The opcodeTracer (EIP-3155Eip3155Step, consumed by the goevmlabstatetestsubcommand from #6663 / #6595), the prestateTracer, andVMError'sDisplay(which feeds the execution-spec-tests / hive error mapping) are all left untouched — verified against branch history, so goevmlab output is byte-for-byte unchanged.Adds 9 VM-level tests in
test/tests/levm/trace_call_tests.rscovering post-refund gas, revert-reason decode, geth error mapping, omit-empty shape, and block-absolute log index.