feat(l1): implement 4byteTracer#6704
Conversation
|
Lines of code reportTotal lines added: Detailed view |
azteca1998
changed the title
Three correctness bugs versus geth's built-in 4byteTracer (eth/tracers/native/4byte.go), each of which would cause clients matching against geth output to mismatch every entry: - The reported size was `len(calldata)` instead of `len(calldata) - 4`. Geth's key shape is `"0xSELECTOR-N"` where N is the argument-bytes length excluding the selector itself. - The top-level transaction call was being counted. Geth skips depth 0 and only records nested invocations. - Every CallType was counted, including STATICCALL, CALLCODE, CREATE, CREATE2, and SELFDESTRUCT. Geth filters on the opcode and only counts CALL and DELEGATECALL — for create variants and selfdestruct the input isn't a function selector, and the geth tracer doesn't surface STATICCALL or CALLCODE either. Also skip precompile-targeted invocations (addresses 0x01..=0x11 plus P256VERIFY at 0x100) — matching geth's per-fork `isPrecompiled` check fork-agnostically, since any precompile address is a precompile once its fork activates. Unit tests now lock in each rule: top-frame skip, short-input skip, call-type filter, precompile skip, arg-size key, duplicate counting. Integration tests gain block-level coverage and a missing-tx failure case. Variant doc clarifies the size semantics and skipped categories. Test setup boilerplate moves out of the misnamed `debug_trace_tests.rs` into a shared `rpc::helpers` module so #6694, #6701, #6705, #6704 can all rebase to drop their duplicate copies; the file is renamed to `debug_four_byte_tracer_tests.rs` to fit its contents.
🤖 Kimi Code ReviewThe PR implements geth's Issues Found1. Potential incorrect parameter for subcall tracing ( The If Suggestion: Verify the semantics of this parameter. If it controls subcall inclusion, change to .trace_transaction_calls(
self.tx_hash,
reexec,
timeout,
true, // need all subcalls to collect nested selectors
false,
)The same applies to the block tracing variant at line 392. Minor Observations2. Precompile range check ( The 3. Error handling on empty traces ( The code returns Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code ReviewFindings
Notes
Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
🤖 Claude Code ReviewNow I have enough context for a thorough review. Code Review:
|
Greptile SummaryThis PR implements the
Confidence Score: 3/5Safe to merge for the wired-up plumbing; the main open question is whether the STATICCALL/CALLCODE filtering matches geth's reference implementation. The selector-collection logic is well unit-tested and the RPC plumbing is straightforward. However, the claim that geth's 4byteTracer also skips STATICCALL and CALLCODE frames deserves verification against the linked geth source — if geth's CaptureEnter records all call types (filtering only on input length and precompile status), then view-function calls via STATICCALL would be silently dropped and the tracer would return a different map than geth for any transaction that makes read-only contract calls. crates/networking/rpc/tracing.rs — specifically the call-type filter in collect_four_byte_recursive and the is_precompile_address boundary at 0x11.
|
| Filename | Overview |
|---|---|
| crates/networking/rpc/tracing.rs | Adds FourByteTracer to TraceTransactionRequest and TraceBlockByNumberRequest; implements collect_four_byte_selectors/collect_four_byte_recursive helpers plus is_precompile_address; potential behavioral divergence from geth regarding STATICCALL/CALLCODE recording |
| test/tests/rpc/debug_four_byte_tracer_tests.rs | New integration tests for 4byteTracer; only covers the empty-map case via ETH transfers, not actual selector recording |
| test/tests/rpc/helpers.rs | New shared test utilities: in-memory store setup, block building, RPC dispatch helpers; clean and well-structured |
| test/tests/rpc/mod.rs | Trivial module registration for new test files |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[RPC debug_traceTransaction or debug_traceBlockByNumber] --> B{TracerType}
B -->|FourByteTracer| C[trace_transaction_calls with only_top_call=false]
C --> D[Take first CallTraceFrame - top-level tx call]
D --> E[collect_four_byte_selectors - skip top frame itself]
E --> F[iterate top_frame.calls at depth 1+]
F --> G[collect_four_byte_recursive per sub-call]
G --> H{CallType is CALL or DELEGATECALL?}
H -->|No| J[skip recording - still recurse into children]
H -->|Yes| I{input length at least 4 and not precompile?}
I -->|No| J
I -->|Yes| K[build key 0xSELECTOR-argsize and increment count]
K --> L[recurse into frame.calls]
J --> L
L -->|more sub-calls| G
L -->|no more| M[return HashMap String to u64]
M --> N[serialize to JSON object]
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
crates/networking/rpc/tracing.rs:430-442
**STATICCALL/CALLCODE filtering may diverge from geth**
The implementation skips `STATICCALL` and `CALLCODE` frames entirely when recording selectors. Geth's native `4byteTracer` uses the `CaptureEnter` hook, which is called for *all* call types (CALL, DELEGATECALL, STATICCALL, CALLCODE, CREATE, CREATE2). It only gates on `len(input) < 4` and `isPrecompiled(to)` — there is no op-type filter. If that is still the case in the current geth source, every STATICCALL with ≥4 bytes of calldata to a non-precompile would be recorded by geth but silently skipped here, producing a different selector map for any transaction that makes view-function calls (which commonly use STATICCALL).
### Issue 2 of 2
test/tests/rpc/debug_four_byte_tracer_tests.rs:1-71
**Integration tests only exercise the empty-map code path**
All three integration tests are built on `setup_single_transfer_block`, which creates a plain ETH transfer. Because the tracer skips the top-level call and the transfer has no nested calls, the selector map is always empty. This means the RPC serialization path for a non-empty map (the case that actually exercises `collect_four_byte_selectors` through the full dispatch chain) has no integration-level coverage. A test with a transaction that invokes a deployed contract — so at least one nested CALL/DELEGATECALL with ≥4 bytes of calldata appears in the result — would catch any wire-format or plumbing regression that the unit tests in `tracing.rs` cannot.
Reviews (1): Last reviewed commit: "fix(rpc): align 4byteTracer with geth's ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| && frame.input.len() >= 4 | ||
| && !is_precompile_address(&frame.to) | ||
| { | ||
| let selector = hex::encode(&frame.input[..4]); | ||
| let arg_size = frame.input.len() - 4; | ||
| let key = format!("0x{selector}-{arg_size}"); | ||
| *selectors.entry(key).or_insert(0) += 1; | ||
| } | ||
| for sub_call in &frame.calls { | ||
| collect_four_byte_recursive(sub_call, selectors); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
The implementation skips STATICCALL and CALLCODE frames entirely when recording selectors. Geth's native 4byteTracer uses the CaptureEnter hook, which is called for all call types (CALL, DELEGATECALL, STATICCALL, CALLCODE, CREATE, CREATE2). It only gates on len(input) < 4 and isPrecompiled(to) — there is no op-type filter. If that is still the case in the current geth source, every STATICCALL with ≥4 bytes of calldata to a non-precompile would be recorded by geth but silently skipped here, producing a different selector map for any transaction that makes view-function calls (which commonly use STATICCALL).
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 430-442
Comment:
**STATICCALL/CALLCODE filtering may diverge from geth**
The implementation skips `STATICCALL` and `CALLCODE` frames entirely when recording selectors. Geth's native `4byteTracer` uses the `CaptureEnter` hook, which is called for *all* call types (CALL, DELEGATECALL, STATICCALL, CALLCODE, CREATE, CREATE2). It only gates on `len(input) < 4` and `isPrecompiled(to)` — there is no op-type filter. If that is still the case in the current geth source, every STATICCALL with ≥4 bytes of calldata to a non-precompile would be recorded by geth but silently skipped here, producing a different selector map for any transaction that makes view-function calls (which commonly use STATICCALL).
How can I resolve this? If you propose a fix, please make it concise.| use ethrex_common::H256; | ||
| use serde_json::{Value, json}; | ||
|
|
||
| use super::helpers::{rpc_call, rpc_call_expect_err, setup_single_transfer_block}; | ||
|
|
||
| #[tokio::test] | ||
| async fn trace_tx_four_byte_tracer_value_transfer_is_empty() { | ||
| let env = setup_single_transfer_block().await; | ||
|
|
||
| let result = rpc_call( | ||
| &env.store, | ||
| "debug_traceTransaction", | ||
| vec![ | ||
| json!(format!("{:#x}", env.tx_hash)), | ||
| json!({"tracer": "4byteTracer"}), | ||
| ], | ||
| ) | ||
| .await; | ||
|
|
||
| // A simple ETH transfer has no nested calls and the tracer skips the | ||
| // top-level call, so the result must be the empty map. | ||
| let obj = result.as_object().expect("response should be an object"); | ||
| assert!(obj.is_empty(), "simple transfer has no 4-byte selectors"); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn trace_tx_four_byte_tracer_unknown_hash_errors() { | ||
| let env = setup_single_transfer_block().await; | ||
|
|
||
| let err = rpc_call_expect_err( | ||
| &env.store, | ||
| "debug_traceTransaction", | ||
| vec![ | ||
| json!(format!("{:#x}", H256::from_low_u64_be(0xdeadbeef))), | ||
| json!({"tracer": "4byteTracer"}), | ||
| ], | ||
| ) | ||
| .await; | ||
| let msg = format!("{err:?}"); | ||
| assert!( | ||
| msg.contains("Transaction not Found"), | ||
| "expected tx-not-found error, got: {msg}" | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn trace_block_four_byte_tracer() { | ||
| let env = setup_single_transfer_block().await; | ||
|
|
||
| let result: Value = rpc_call( | ||
| &env.store, | ||
| "debug_traceBlockByNumber", | ||
| vec![ | ||
| json!(format!("{:#x}", env.block.header.number)), | ||
| json!({"tracer": "4byteTracer"}), | ||
| ], | ||
| ) | ||
| .await; | ||
|
|
||
| let arr = result.as_array().expect("response should be an array"); | ||
| assert_eq!(arr.len(), 1, "one tx in block"); | ||
| let entry = arr[0].as_object().expect("entry should be an object"); | ||
| assert_eq!( | ||
| entry["txHash"].as_str().unwrap().to_lowercase(), | ||
| format!("{:#x}", env.tx_hash).to_lowercase() | ||
| ); | ||
| let selectors = entry["result"] | ||
| .as_object() | ||
| .expect("result should be a selector map"); | ||
| assert!(selectors.is_empty(), "value transfer yields no selectors"); | ||
| } |
There was a problem hiding this comment.
All three integration tests are built on setup_single_transfer_block, which creates a plain ETH transfer. Because the tracer skips the top-level call and the transfer has no nested calls, the selector map is always empty. This means the RPC serialization path for a non-empty map (the case that actually exercises collect_four_byte_selectors through the full dispatch chain) has no integration-level coverage. A test with a transaction that invokes a deployed contract — so at least one nested CALL/DELEGATECALL with ≥4 bytes of calldata appears in the result — would catch any wire-format or plumbing regression that the unit tests in tracing.rs cannot.
Prompt To Fix With AI
This is a comment left during a code review.
Path: test/tests/rpc/debug_four_byte_tracer_tests.rs
Line: 1-71
Comment:
**Integration tests only exercise the empty-map code path**
All three integration tests are built on `setup_single_transfer_block`, which creates a plain ETH transfer. Because the tracer skips the top-level call and the transfer has no nested calls, the selector map is always empty. This means the RPC serialization path for a non-empty map (the case that actually exercises `collect_four_byte_selectors` through the full dispatch chain) has no integration-level coverage. A test with a transaction that invokes a deployed contract — so at least one nested CALL/DELEGATECALL with ≥4 bytes of calldata appears in the result — would catch any wire-format or plumbing regression that the unit tests in `tracing.rs` cannot.
How can I resolve this? If you propose a fix, please make it concise.… tracing Add support for the `4byteTracer` tracer type which records all function selectors (first 4 bytes of calldata) and calldata sizes for every call in a transaction. Returns a map of "0xSELECTOR-SIZE" -> count. Implemented by post-processing the existing call tracer output, walking the call trace tree to extract selector information from each frame.
Trace a real transfer with 4byteTracer and assert the response is an empty selector map (no calldata in a plain transfer).
Three correctness bugs versus geth's built-in 4byteTracer (eth/tracers/native/4byte.go), each of which would cause clients matching against geth output to mismatch every entry: - The reported size was `len(calldata)` instead of `len(calldata) - 4`. Geth's key shape is `"0xSELECTOR-N"` where N is the argument-bytes length excluding the selector itself. - The top-level transaction call was being counted. Geth skips depth 0 and only records nested invocations. - Every CallType was counted, including STATICCALL, CALLCODE, CREATE, CREATE2, and SELFDESTRUCT. Geth filters on the opcode and only counts CALL and DELEGATECALL — for create variants and selfdestruct the input isn't a function selector, and the geth tracer doesn't surface STATICCALL or CALLCODE either. Also skip precompile-targeted invocations (addresses 0x01..=0x11 plus P256VERIFY at 0x100) — matching geth's per-fork `isPrecompiled` check fork-agnostically, since any precompile address is a precompile once its fork activates. Unit tests now lock in each rule: top-frame skip, short-input skip, call-type filter, precompile skip, arg-size key, duplicate counting. Integration tests gain block-level coverage and a missing-tx failure case. Variant doc clarifies the size semantics and skipped categories. Test setup boilerplate moves out of the misnamed `debug_trace_tests.rs` into a shared `rpc::helpers` module so #6694, #6701, #6705, #6704 can all rebase to drop their duplicate copies; the file is renamed to `debug_four_byte_tracer_tests.rs` to fit its contents.
azteca1998
force-pushed
the
feat/four-byte-tracer
branch
from
64a9cbd to
0963449
Compare
Geth's 4byteTracer (CaptureEnter) counts all call types with >= 4 bytes of input data. Only CREATE/CREATE2/SELFDESTRUCT are skipped (their input is init-code, not ABI-encoded calls).
1 participant
Summary
4byteTracertracer type fordebug_traceTransactionand block tracing endpoints"0xSELECTOR-SIZE" → count(e.g.{"0xa9059cbb-68": 1})Closes part of #6572
Closes #6644