feat(l1): implement muxTracer for debug_traceTransaction#6706
feat(l1): implement muxTracer for debug_traceTransaction#6706azteca1998 wants to merge 5 commits into
Conversation
Add support for the `muxTracer` tracer type which runs multiple
sub-tracers on the same transaction and returns a combined result
map of `tracerName -> result`.
Supported sub-tracers: callTracer, prestateTracer, opcodeTracer,
4byteTracer, noopTracer.
Example config:
{"tracer": "muxTracer", "tracerConfig": {
"callTracer": {"onlyTopCall": true},
"prestateTracer": {"diffMode": false}
}}
Note: block-level muxTracer is not yet supported and returns an error.
|
Lines of code reportTotal lines added: Detailed view |
Tests for parse, config validation, tracer type deserialization, and collect_four_byte_selectors helper function.
Trace a real transfer with muxTracer running callTracer and prestateTracer simultaneously. Assert both sub-results are present.
azteca1998
changed the title
This PR duplicated the same `collect_four_byte_selectors` body that
ships in the standalone 4byteTracer PR — including its three correctness
bugs against geth's reference (eth/tracers/native/4byte.go):
- size was `len(input)` instead of `len(input) - 4`,
- the top-level transaction call was counted (geth skips depth 0),
- every CallType was counted (geth only counts CALL + DELEGATECALL,
skipping STATICCALL, CALLCODE, CREATE*, SELFDESTRUCT).
Fix all three here and add precompile-address skipping to match what
geth's `isPrecompiled` filter does on mainnet. Pre-existing unit tests
that asserted the buggy output are rewritten to lock in the corrected
semantics: top-frame skip, short-input skip, arg-size key, CALL/DELEGATECALL
filter, precompile skip, duplicate counting.
Two ergonomic fixes on the muxTracer handler itself:
- `debug_traceBlockByNumber` with muxTracer was surfacing as
`RpcErr::Internal`; switched to `BadParams` since refusing the request
is a user-input issue, not a server fault. The error message now also
points the caller at the per-transaction form.
- Documented all three known divergences from geth on the `MuxTracer`
variant: per-sub-tracer re-execution cost, no block-level support,
and the closed set of routable sub-tracers (no flatCallTracer yet).
Integration coverage now actually proves the multiplexer is faithful:
three cross-validation tests assert that the mux output's `callTracer`,
`prestateTracer`, and combined-multi-tracer slots equal what each
sub-tracer returns when invoked standalone. Plus tests for the noop
shape (`{}`), unknown sub-tracer, missing `tracerConfig`, and the
block-level `BadParams` surface.
Test setup boilerplate moves into the shared `rpc::helpers` module from
#6701; the misnamed `debug_trace_tests.rs` is renamed to
`debug_mux_tracer_tests.rs`.
🤖 Kimi Code ReviewOverall Assessment: Solid implementation with good test coverage and clear documentation. The multiplexer correctly re-runs transactions per sub-tracer (documented limitation) and the new Issues Found: 1. Timeout Multiplication (Performance/DoS)
2. Inconsistent Error Construction
3. Potential JSON Clone Overhead (Minor)
4. Missing Precompile Context (Accepted Limitation)
Positive Notes:
Security Check: No memory safety issues found. The implementation uses safe Rust patterns throughout with proper error propagation. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Claude Code ReviewNow I have all the context needed for a complete review. PR Review:
|
🤖 Codex Code Review
I did not run the test suite here: Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryThis PR adds
Confidence Score: 4/5The change is additive and isolated to the tracing layer; existing tracer paths are untouched and the new muxTracer path is well-tested for the main sub-tracers. The core mux dispatch logic is straightforward and the integration tests cover the primary sub-tracers and error cases well. The main open questions are around the 4byteTracer: the comment attributes STATICCALL filtering to geth's own behavior, but geth's native tracer does not filter by opcode, so the implementation diverges silently for transactions with STATICCALL internal calls. No integration test covers the 4byteTracer sub-tracer end-to-end within muxTracer, leaving that path untested. crates/networking/rpc/tracing.rs — specifically the 4byteTracer helpers and the opcode-filtering comment; test/tests/rpc/debug_mux_tracer_tests.rs for the missing 4byteTracer integration test.
|
| Filename | Overview |
|---|---|
| crates/networking/rpc/tracing.rs | Adds MuxTracer variant to TracerType, implements sequential sub-tracer dispatch in run_tx_sub_tracer, and introduces 4byteTracer helpers; main concerns are a misdocumented geth divergence for STATICCALL filtering and the uncapped per-sub-tracer timeout. |
| test/tests/rpc/debug_mux_tracer_tests.rs | New integration tests covering muxTracer happy paths (callTracer, prestateTracer, noopTracer sub-tracers), error paths (unknown sub-tracer, missing config, block-level not supported); 4byteTracer sub-tracer is not exercised end-to-end. |
| test/tests/rpc/helpers.rs | New shared test helpers module extracted for reuse across RPC integration tests; provides store setup, block building, and rpc_call/rpc_call_expect_err utilities. |
| test/tests/rpc/mod.rs | Registers the two new test modules (debug_mux_tracer_tests, helpers) in the rpc test mod. |
Sequence Diagram
sequenceDiagram
participant Client
participant RPC as TraceTransactionRequest::handle
participant Sub as run_tx_sub_tracer
participant BC as blockchain
Client->>RPC: debug_traceTransaction(txHash, muxTracer config)
RPC->>RPC: parse tracerConfig → HashMap
loop for each (tracer_name, sub_config)
RPC->>Sub: run_tx_sub_tracer(...)
alt callTracer / prestateTracer / opcodeTracer / 4byteTracer / noopTracer
Sub->>BC: "trace_transaction_*()"
BC-->>Sub: result
Sub-->>RPC: Value
else unknown
Sub-->>RPC: BadParams error
end
RPC->>RPC: results.insert(name, value)
end
RPC-->>Client: "{tracerName: result, ...}"
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
crates/networking/rpc/tracing.rs:491-499
**Inaccurate attribution of geth's 4byteTracer opcode filtering**
The comment states "geth's tracer filters on the opcode and skips STATICCALL, CALLCODE, CREATE*, and SELFDESTRUCT," but geth's native Go implementation (`eth/tracers/native/4byte.go`) does not filter by opcode in `CaptureEnter` — it captures any call type with at least 4 bytes of input. This means STATICCALL and CALLCODE selectors are counted by geth but not by this implementation. The filtering is a real divergence, but it's being incorrectly attributed to geth's own behavior. Any contract that uses `STATICCALL` with a function selector (e.g., view-function dispatch via delegate or proxy patterns) would produce a different selector map here versus geth.
### Issue 2 of 4
crates/networking/rpc/tracing.rs:472-475
**`4byteTracer` ignores `sub_config` without validation**
The `"4byteTracer"` arm silently ignores `sub_config` — any keys the caller passes are discarded without warning. This is fine given geth's 4byteTracer accepts no configuration, but a stale or mistyped config field will never surface as an error. At minimum, asserting `sub_config` is null or an empty object makes the contract explicit and prevents silent misconfiguration.
```suggestion
"4byteTracer" => {
// 4byteTracer accepts no configuration; any supplied config is ignored.
let call_trace = context
.blockchain
.trace_transaction_calls(tx_hash, reexec, timeout, false, false)
```
### Issue 3 of 4
crates/networking/rpc/tracing.rs:228-251
**Each sub-tracer receives the full timeout, not a proportional share**
`timeout` is passed unchanged to every `run_tx_sub_tracer` call, so with `N` sub-tracers and the default 5 s timeout the wall-clock budget is up to `N × 5 s`. A user who sets `"timeout": "2s"` expecting a 2-second cap for the whole mux request will be surprised when the call takes up to 10 s with 5 sub-tracers. The `MuxTracer` variant comment already documents the N× cost divergence, but the call site itself has no note. Consider at least noting this in a call-site comment so future maintainers don't accidentally "fix" the pass-through.
### Issue 4 of 4
test/tests/rpc/debug_mux_tracer_tests.rs:1-10
**No integration test for the `4byteTracer` sub-tracer within muxTracer**
The integration test suite validates `callTracer`, `prestateTracer`, and `noopTracer` as mux sub-tracers, but not `4byteTracer`. Given that `4byteTracer` has its own new implementation (`collect_four_byte_selectors`, `collect_four_byte_recursive`, `is_precompile_address`) rather than delegating to an existing path, an end-to-end test covering at least the happy path (e.g., a simple ETH transfer returns `{}` since there are no internal calls) would close the coverage gap.
Reviews (1): Last reviewed commit: "fix(rpc): correct 4byteTracer inside mux..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// Collects 4-byte function selectors and calldata sizes from a call trace | ||
| /// tree, matching geth's built-in `4byteTracer` | ||
| /// (https://github.com/ethereum/go-ethereum/blob/master/eth/tracers/native/4byte.go): | ||
| /// | ||
| /// - The top-level transaction call is **not** counted; only nested calls are. | ||
| /// - Only `CALL` and `DELEGATECALL` are counted — geth's tracer filters on the | ||
| /// opcode and skips STATICCALL, CALLCODE, CREATE*, and SELFDESTRUCT. | ||
| /// - Invocations targeting precompile addresses are skipped. | ||
| /// - The reported size is `len(calldata) - 4` (the argument-bytes length). |
There was a problem hiding this comment.
The comment states "geth's tracer filters on the opcode and skips STATICCALL, CALLCODE, CREATE*, and SELFDESTRUCT," but geth's native Go implementation (eth/tracers/native/4byte.go) does not filter by opcode in CaptureEnter — it captures any call type with at least 4 bytes of input. This means STATICCALL and CALLCODE selectors are counted by geth but not by this implementation. The filtering is a real divergence, but it's being incorrectly attributed to geth's own behavior. Any contract that uses STATICCALL with a function selector (e.g., view-function dispatch via delegate or proxy patterns) would produce a different selector map here versus geth.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 491-499
Comment:
**Inaccurate attribution of geth's 4byteTracer opcode filtering**
The comment states "geth's tracer filters on the opcode and skips STATICCALL, CALLCODE, CREATE*, and SELFDESTRUCT," but geth's native Go implementation (`eth/tracers/native/4byte.go`) does not filter by opcode in `CaptureEnter` — it captures any call type with at least 4 bytes of input. This means STATICCALL and CALLCODE selectors are counted by geth but not by this implementation. The filtering is a real divergence, but it's being incorrectly attributed to geth's own behavior. Any contract that uses `STATICCALL` with a function selector (e.g., view-function dispatch via delegate or proxy patterns) would produce a different selector map here versus geth.
How can I resolve this? If you propose a fix, please make it concise.| "4byteTracer" => { | ||
| let call_trace = context | ||
| .blockchain | ||
| .trace_transaction_calls(tx_hash, reexec, timeout, false, false) |
There was a problem hiding this comment.
4byteTracer ignores sub_config without validation
The "4byteTracer" arm silently ignores sub_config — any keys the caller passes are discarded without warning. This is fine given geth's 4byteTracer accepts no configuration, but a stale or mistyped config field will never surface as an error. At minimum, asserting sub_config is null or an empty object makes the contract explicit and prevents silent misconfiguration.
| "4byteTracer" => { | |
| let call_trace = context | |
| .blockchain | |
| .trace_transaction_calls(tx_hash, reexec, timeout, false, false) | |
| "4byteTracer" => { | |
| // 4byteTracer accepts no configuration; any supplied config is ignored. | |
| let call_trace = context | |
| .blockchain | |
| .trace_transaction_calls(tx_hash, reexec, timeout, false, false) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 472-475
Comment:
**`4byteTracer` ignores `sub_config` without validation**
The `"4byteTracer"` arm silently ignores `sub_config` — any keys the caller passes are discarded without warning. This is fine given geth's 4byteTracer accepts no configuration, but a stale or mistyped config field will never surface as an error. At minimum, asserting `sub_config` is null or an empty object makes the contract explicit and prevents silent misconfiguration.
```suggestion
"4byteTracer" => {
// 4byteTracer accepts no configuration; any supplied config is ignored.
let call_trace = context
.blockchain
.trace_transaction_calls(tx_hash, reexec, timeout, false, false)
```
How can I resolve this? If you propose a fix, please make it concise.| TracerType::MuxTracer => { | ||
| let mux_config: HashMap<String, Value> = | ||
| if let Some(value) = &self.trace_config.tracer_config { | ||
| serde_json::from_value(value.clone())? | ||
| } else { | ||
| return Err(RpcErr::BadParams( | ||
| "muxTracer requires tracerConfig".to_owned(), | ||
| )); | ||
| }; | ||
| let mut results = serde_json::Map::new(); | ||
| for (tracer_name, sub_config) in &mux_config { | ||
| let result = run_tx_sub_tracer( | ||
| tracer_name, | ||
| sub_config, | ||
| self.tx_hash, | ||
| reexec, | ||
| timeout, | ||
| &context, | ||
| ) | ||
| .await?; | ||
| results.insert(tracer_name.clone(), result); | ||
| } | ||
| Ok(Value::Object(results)) | ||
| } |
There was a problem hiding this comment.
timeout is passed unchanged to every run_tx_sub_tracer call, so with N sub-tracers and the default 5 s timeout the wall-clock budget is up to N × 5 s. A user who sets "timeout": "2s" expecting a 2-second cap for the whole mux request will be surprised when the call takes up to 10 s with 5 sub-tracers. The MuxTracer variant comment already documents the N× cost divergence, but the call site itself has no note. Consider at least noting this in a call-site comment so future maintainers don't accidentally "fix" the pass-through.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 228-251
Comment:
**Each sub-tracer receives the full timeout, not a proportional share**
`timeout` is passed unchanged to every `run_tx_sub_tracer` call, so with `N` sub-tracers and the default 5 s timeout the wall-clock budget is up to `N × 5 s`. A user who sets `"timeout": "2s"` expecting a 2-second cap for the whole mux request will be surprised when the call takes up to 10 s with 5 sub-tracers. The `MuxTracer` variant comment already documents the N× cost divergence, but the call site itself has no note. Consider at least noting this in a call-site comment so future maintainers don't accidentally "fix" the pass-through.
How can I resolve this? If you propose a fix, please make it concise.| use serde_json::{Value, json}; | ||
|
|
||
| use super::helpers::{rpc_call, rpc_call_expect_err, setup_single_transfer_block}; | ||
|
|
||
| async fn trace_with( | ||
| store: ðrex_storage::Store, | ||
| tx: ethrex_common::H256, | ||
| config: Value, | ||
| ) -> Value { | ||
| rpc_call( |
There was a problem hiding this comment.
4byteTracer sub-tracer within muxTracer
The integration test suite validates callTracer, prestateTracer, and noopTracer as mux sub-tracers, but not 4byteTracer. Given that 4byteTracer has its own new implementation (collect_four_byte_selectors, collect_four_byte_recursive, is_precompile_address) rather than delegating to an existing path, an end-to-end test covering at least the happy path (e.g., a simple ETH transfer returns {} since there are no internal calls) would close the coverage gap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: test/tests/rpc/debug_mux_tracer_tests.rs
Line: 1-10
Comment:
**No integration test for the `4byteTracer` sub-tracer within muxTracer**
The integration test suite validates `callTracer`, `prestateTracer`, and `noopTracer` as mux sub-tracers, but not `4byteTracer`. Given that `4byteTracer` has its own new implementation (`collect_four_byte_selectors`, `collect_four_byte_recursive`, `is_precompile_address`) rather than delegating to an existing path, an end-to-end test covering at least the happy path (e.g., a simple ETH transfer returns `{}` since there are no internal calls) would close the coverage gap.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Summary
muxTracertracer type fordebug_traceTransactiontracerName -> resultcallTracer,prestateTracer,opcodeTracer,4byteTracer,noopTracerExample
{ "tracer": "muxTracer", "tracerConfig": { "callTracer": {"onlyTopCall": true}, "prestateTracer": {"diffMode": false} } }Closes part of #6572