feat(l1): implement flatCallTracer

[azteca1998]

Summary

• Add flatCallTracer tracer type for debug_traceTransaction and block tracing endpoints
• Returns a flat array of call frames following the Parity/OpenEthereum trace format
• Each frame includes traceAddress (position in call tree as array of indices) and subtraces (direct child count)
• Uses the action/result sub-object structure: action.callType, action.from, action.to, etc.
• Supports frame types: "call", "create", and "suicide"
• Implemented by post-processing the existing call tracer output (depth-first recursive flattening)

Closes part of #6572

Closes #6647

[github-actions]

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

Known Issues

Tests intentionally excluded from CI. Source of truth for the Known
Issues section the L1 workflow appends to each ef-tests job summary
and posts as a sticky PR comment.

EF Tests — Stateless coverage narrowed to EIP-8025 optional-proofs

make -C tooling/ef_tests/blockchain test calls test-stateless-zkevm
instead of test-stateless. The zkevm@v0.3.3 fixtures are filled against
bal@v5.6.1, out of sync with current bal spec; the broad target trips ~549
fixtures. Re-broaden once the zkevm bundle is regenerated.

Why and resolution path

PR #6527 broadened
test-stateless to extract the entire for_amsterdam/ tree from the
zkevm bundle and run all of it under --features stateless; combined with
this branch's bal-devnet-7 semantics that scope produces ~549
GasUsedMismatch / ReceiptsRootMismatch /
BlockAccessListHashMismatch failures.

test-stateless-zkevm filters cargo to the eip8025_optional_proofs
suite, which still validates the stateless harness without the bal-version
mismatch.

Re-broaden by switching test: back to test-stateless in
tooling/ef_tests/blockchain/Makefile once the zkevm bundle is regenerated
against the current bal spec.

[github-actions]

Lines of code report

Total lines added: 350
Total lines removed: 0
Total lines changed: 350

Detailed view

+-----------------------------------------+-------+------+
| File                                    | Lines | Diff |
+-----------------------------------------+-------+------+
| ethrex/crates/networking/rpc/tracing.rs | 665   | +350 |
+-----------------------------------------+-------+------+

[github-actions]

🤖 Kimi Code Review

The PR implements flatCallTracer support for the RPC debug API, converting nested call traces into the Parity/OpenEthereum flat format with geth extensions. The implementation is solid and well-tested.

Minor suggestions:

crates/networking/rpc/tracing.rs

1. Line 441: The result variable shadows the function parameter result in flatten_recursive. While valid Rust, consider renaming the local variable (e.g., call_result) to avoid confusion:

   let call_result = if frame.error.is_some() || matches!(frame.call_type, CallType::SELFDESTRUCT) {
       None
   } else {
       // ...
   };

2. Lines 458-462: When pushing the frame, you're cloning trace_address to a Vec even though you only need it for this push. This is fine given the 1024 depth limit, but you could avoid the clone by passing ownership in the recursion and using child_address directly. Current approach is acceptable for clarity.

3. Lines 380-396: In TraceBlockByNumberRequest, the error message "Empty call trace" could be more specific to indicate which transaction hash failed, aiding debugging:

   .ok_or_else(|| RpcErr::Internal(format!("Empty call trace for tx {}", hash)))?

test/tests/rpc/helpers.rs

4. Line 26: ethrex_l2_rpc::signer import creates a test dependency on L2 code. If the project supports no-L2 builds, this might break. Consider using ethrex_common::signer or a generic signing utility if available. If L2 is always included, disregard.

5. Line 67: Store::new("store.db", EngineType::InMemory) - the path argument is ignored for in-memory stores but could confuse readers. Consider ":memory:" or a comment noting this.

test/tests/rpc/debug_flat_call_tracer_tests.rs

6. Line 94: The assertion assert!(f["result"].get("address").is_none()) in the value transfer test assumes the result object exists. If trace_tx_flat_call_tracer_value_transfer ever returns a frame without a result (e.g., on failure), this would panic. Consider checking existence first:

   assert!(f.get("result").map(|r| r.get("address").is_none()).unwrap_or(true));

Positive notes:

• Correct SELFDESTRUCT mapping to Parity's suicide format (address/balance/refundAddress) at lines 497-501
• Proper CREATE/CREATE2 discrimination using creation_method field
• Good test coverage including nested trace addresses and call type variants
• Appropriate use of #[serde(untagged)] to match geth's flat output format
• Memory-safe recursion bounded by EVM call depth limit (comment at line 477)

The implementation correctly follows the geth flatCallTracer specification and appears ready for merge.

---

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

[github-actions]

🤖 Codex Code Review

No material findings from the diff review.

The flatCallTracer mapping in crates/networking/rpc/tracing.rs looks internally consistent with the existing CallTraceFrame model, and the added unit/integration coverage exercises the main wire-shape cases: plain call, nested calls, CREATE, CREATE2, SELFDESTRUCT, and failed calls.

Residual risk: this was static review only. I could not run the test suite in this environment because cargo/rustup failed creating temp files on the read-only filesystem, so runtime validation is still missing.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

[github-actions]

🤖 Claude Code Review

Now I have a complete picture of the PR. Here is my review:

---

PR #6705 — feat(l1): implement flatCallTracer

Overview

This PR implements the flatCallTracer for debug_traceTransaction and debug_traceBlockByNumber. It post-processes the existing nested CallTraceFrame tree into a flat array using pre-order DFS, attaching traceAddress (path to each frame in the call tree) and subtraces (direct child count), following the Parity/OpenEthereum wire shape plus geth's creationMethod extension on create frames.

---

Correctness

The core algorithm is correct:

• Pre-order DFS produces the right ordering (root before children, sibling order preserved).
• traceAddress accumulation is correct — each child receives a copy of the parent's address with its sibling index appended.
• subtraces uses frame.calls.len(), counting only direct children, which matches the geth/Parity spec.
• The three frame types (call, create, suicide) are correctly dispatched based on CallType.
• FlatCallResult is correctly omitted for error frames and for SELFDESTRUCT frames.
• CREATE vs CREATE2 distinction via creationMethod matches geth's call_flat.go.
• The #[serde(untagged)] on FlatCallAction/FlatCallResult correctly emits the inner fields without a discriminator key.
• Recursion depth is bounded by the EVM's 1024 call stack limit — safe.

---

Issues

1. ok_or vs ok_or_else inconsistency

TraceTransactionRequest (line ~231):

.ok_or(RpcErr::Internal("Empty call trace".to_string()))?;

TraceBlockByNumberRequest (line ~388):

.ok_or_else(|| RpcErr::Internal("Empty call trace".to_string()))?;

The eager ok_or evaluates its argument unconditionally; ok_or_else is the idiomatic lazy form. String::to_string() is cheap so this is not a performance concern, but the inconsistency is noisy. Prefer ok_or_else in both sites.

2. Redundant Vec allocation per child in flatten_recursive (lines 578–580)

for (i, sub_call) in frame.calls.iter().enumerate() {
    let mut child_address = trace_address.to_vec();  // new alloc every iteration
    child_address.push(i);
    flatten_recursive(sub_call, &child_address, result);
}

A push/pop approach reuses the same buffer across siblings:

let mut child_address = trace_address.to_vec();
for (i, sub_call) in frame.calls.iter().enumerate() {
    child_address.push(i);
    flatten_recursive(sub_call, &child_address, result);
    child_address.pop();
}

At the 1024-depth limit this is not a correctness issue, but the current code allocates O(siblings × depth) Vecs while the push/pop form allocates one per depth level.

3. Implicit assumption: frame.output = deployed bytecode for CREATE

In FlatCallResult::Create, code: frame.output.clone() assumes the call tracer sets frame.output to the runtime bytecode after deployment. This assumption is correct for the existing call tracer, but it is not documented and a future refactor could silently break it. A one-line comment at the site would make the invariant explicit.

4. Opaque boolean arguments in trace_transaction_calls

.trace_transaction_calls(
    self.tx_hash,
    reexec,
    timeout,
    false, // need all subcalls
    false,
)

The comment correctly explains the first false, but the second false (presumably with_log) is unexplained. A comment for both, or named constants, would help.

---

Minor / Nits

• frame_type: &'static str is fine; the strings are literals. The alternative would be a FrameType enum with a as_str() method, but it's unnecessary complexity for this use case.
• FlatCallAction and FlatCallResult are Serialize-only with no Deserialize — correct, these are output types.
• No debug_traceBlockByHash endpoint exists in the routing table (rpc.rs:1073), so the PR correctly covers all live endpoints.

---

Test Coverage

Coverage is solid:

• TracerType deserialization round-trips.
• Unit tests exercise each frame type (CALL, CREATE, CREATE2, SELFDESTRUCT, error) plus multi-level nesting.
• Integration tests cover value-transfer, contract deployment, unknown-hash error, and block-level tracing.

Gap: No test covers a partially-failing call tree — where a parent frame succeeds but a subcall inside it fails. The code handles this correctly (the subcall will have error set and no result), but a test would prevent regressions.

---

Summary

The implementation is correct and well-structured. The main actionable items are: the ok_or inconsistency (easy fix), the child-address allocation pattern (minor optimization), a comment for the second boolean argument, and an additional test for partial subcall failures.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[greptile-apps]

Greptile Summary

This PR adds flatCallTracer support to the debug_traceTransaction and debug_traceBlockByNumber endpoints by post-processing the existing callTracer output into a depth-first flat array with Parity/OpenEthereum-style action/result sub-objects, traceAddress, and subtraces fields.

• Adds the FlatCallTracer variant to TracerType, wires it into both RPC handlers, and implements flatten_call_trace/flatten_recursive with correct pre-order DFS traversal and per-variant serialization for call, create, and suicide frame types.
• Adds comprehensive unit tests for the flattening logic and integration tests for value-transfer, contract-deploy, unknown-hash-error, and block-level tracing scenarios.

Confidence Score: 4/5

Safe to merge; the change is additive and isolated to a new tracer variant with no impact on existing tracers.

The flattening logic, trace-address computation, and per-variant serialization are all well-tested and correct. The only notable gap is that value is always serialized for STATICCALL and DELEGATECALL frames, whereas geth's reference implementation omits this field for those call types — a DELEGATECALL frame preserving a non-zero parent msg.value could emit a misleading non-zero value in the output.

crates/networking/rpc/tracing.rs — specifically the FlatCallAction::Call construction for STATICCALL and DELEGATECALL variants.

Important Files Changed

Filename	Overview
crates/networking/rpc/tracing.rs	Adds FlatCallTracer variant to TracerType and all handler/serialization logic for flatCallTracer; minor deviation from geth reference on value field for STATICCALL/DELEGATECALL.
test/tests/rpc/debug_flat_call_tracer_tests.rs	New integration tests covering value-transfer tx, contract deployment tx, unknown-hash error path, and block-level tracing for flatCallTracer.
test/tests/rpc/helpers.rs	New shared test helpers (store setup, block building, RPC dispatch) used by all rpc integration tests; private key is test-only and intentionally hardcoded.
test/tests/rpc/mod.rs	Registers the two new modules (debug_flat_call_tracer_tests, helpers) in the rpc test module.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["RPC: debug_traceTransaction\n/ debug_traceBlockByNumber"] --> B{TracerType?}
    B -->|flatCallTracer| C["trace_transaction_calls()\n/ trace_block_calls()"]
    C --> D["CallTrace: Vec<CallTraceFrame>\n(nested tree)"]
    D --> E["flatten_call_trace(root)"]
    E --> F["flatten_recursive(frame, trace_address, result)"]
    F --> G{frame.call_type}
    G -->|CALL/CALLCODE\nSTATICCALL/DELEGATECALL| H["FlatCallAction::Call\n{callType, from, to, gas, input, value}"]
    G -->|CREATE/CREATE2| I["FlatCallAction::Create\n{creationMethod, from, gas, init, value}"]
    G -->|SELFDESTRUCT| J["FlatCallAction::Suicide\n{address, balance, refundAddress}"]
    F --> K{frame.error?}
    K -->|Some| L["result = None"]
    K -->|None + Suicide| L
    K -->|None + Call| M["FlatCallResult::Call\n{gasUsed, output}"]
    K -->|None + Create| N["FlatCallResult::Create\n{address, code, gasUsed}"]
    F --> O["Push FlatCallFrame\n{action, error, result,\nsubtraces=calls.len(),\ntraceAddress}"]
    O --> P["Recurse for each child\nwith traceAddress ++ [i]"]
    P --> Q["Vec<FlatCallFrame>\n(flat array, DFS order)"]

Loading

Comments Outside Diff (1)

1. crates/networking/rpc/tracing.rs, line 188-204 (link)

   value always emitted for STATICCALL and DELEGATECALL

   Geth's reference flatCallTracer (call_flat.go) sets Value to nil for STATICCALL and DELEGATECALL frames, which causes the field to be omitted from JSON output (json:"value,omitempty"). The current implementation always populates value: frame.value. For DELEGATECALL specifically, the underlying call tracer may capture the parent's msg.value context into frame.value (so it can be non-zero), which would be serialized as a non-zero ether amount even though DELEGATECALL never transfers ether. Tools doing strict geth compatibility checks would see unexpected value fields on these frame types.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: crates/networking/rpc/tracing.rs
   Line: 188-204
   
   Comment:
   **`value` always emitted for STATICCALL and DELEGATECALL**
   
   Geth's reference `flatCallTracer` (`call_flat.go`) sets `Value` to `nil` for STATICCALL and DELEGATECALL frames, which causes the field to be omitted from JSON output (`json:"value,omitempty"`). The current implementation always populates `value: frame.value`. For DELEGATECALL specifically, the underlying call tracer may capture the parent's `msg.value` context into `frame.value` (so it can be non-zero), which would be serialized as a non-zero ether amount even though DELEGATECALL never transfers ether. Tools doing strict geth compatibility checks would see unexpected `value` fields on these frame types.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/networking/rpc/tracing.rs:188-204
**`value` always emitted for STATICCALL and DELEGATECALL**

Geth's reference `flatCallTracer` (`call_flat.go`) sets `Value` to `nil` for STATICCALL and DELEGATECALL frames, which causes the field to be omitted from JSON output (`json:"value,omitempty"`). The current implementation always populates `value: frame.value`. For DELEGATECALL specifically, the underlying call tracer may capture the parent's `msg.value` context into `frame.value` (so it can be non-zero), which would be serialized as a non-zero ether amount even though DELEGATECALL never transfers ether. Tools doing strict geth compatibility checks would see unexpected `value` fields on these frame types.

_{Reviews (1): Last reviewed commit: "fix(rpc): correct flatCallTracer shape f..." | Re-trigger Greptile}