Skip to content

feat(l1): implement debug_traceBlock RPC endpoint#6696

Open
azteca1998 wants to merge 4 commits into
mainfrom
feat/debug-trace-block-rlp
Open

feat(l1): implement debug_traceBlock RPC endpoint#6696
azteca1998 wants to merge 4 commits into
mainfrom
feat/debug-trace-block-rlp

Conversation

@azteca1998
Copy link
Copy Markdown
Contributor

Summary

  • Adds debug_traceBlock which accepts a hex-encoded RLP block, decodes it, and traces all transactions
  • Extracts shared block tracing logic into a trace_block helper used by both traceBlockByNumber and traceBlock
  • Supports all existing tracers: callTracer, prestateTracer, opcodeTracer
  • Matches geth's debug_traceBlock

Closes part of #6572

Test plan

  • debug_traceBlock with valid RLP block returns traces
  • Invalid RLP returns proper error
  • Non-hex-prefixed input returns proper error

Add debug_traceBlock which accepts a hex-encoded RLP block, decodes it,
and traces all transactions. Extracts shared block tracing logic into a
`trace_block` helper to avoid duplication with traceBlockByNumber.

Part of #6572
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 21, 2026

⚠️ 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 github-actions Bot added the L1 Ethereum client label May 21, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 21, 2026

Lines of code report

Total lines added: 102
Total lines removed: 0
Total lines changed: 102

Detailed view
+-----------------------------------------+-------+------+
| File                                    | Lines | Diff |
+-----------------------------------------+-------+------+
| ethrex/crates/networking/rpc/rpc.rs     | 1302  | +1   |
+-----------------------------------------+-------+------+
| ethrex/crates/networking/rpc/tracing.rs | 416   | +101 |
+-----------------------------------------+-------+------+

Build a real block with a signed transfer, RLP-encode it, and
pass it to the debug_traceBlock RPC endpoint. Assert the response
contains the expected {txHash, result} structure with a CALL trace.
@azteca1998 azteca1998 linked an issue May 21, 2026 that may be closed by this pull request
@azteca1998 azteca1998 marked this pull request as ready for review May 21, 2026 15:43
@azteca1998 azteca1998 requested a review from a team as a code owner May 21, 2026 15:43
@ethrex-project-sync ethrex-project-sync Bot moved this to In Review in ethrex_l1 May 21, 2026
@github-actions
Copy link
Copy Markdown

🤖 Kimi Code Review

Overall Assessment: Good implementation of debug_traceBlock with proper code reuse via the extracted trace_block helper. The refactoring to share logic between debug_traceBlockByNumber and debug_traceBlock is clean. However, there are security and performance concerns regarding input validation.

Critical Issues

1. DoS Vulnerability: No Size Limit on RLP Input

File: crates/networking/rpc/tracing.rs
Lines: 277-282

The method accepts arbitrary hex-encoded RLP data without size validation. A malicious client could send a multi-gigabyte payload causing OOM.

// Current code - vulnerable
let hex_str: String = serde_json::from_value(params[0].clone())?;
// ... decode without size check
let block = Block::decode(&rlp_bytes)?;

Recommendation: Add a size limit check before decoding (e.g., 10MB max for raw block data).

2. Unnecessary Clone in Parameter Parsing

File: crates/networking/rpc/tracing.rs
Line: 277

let hex_str: String = serde_json::from_value(params[0].clone())?;

Recommendation: Use params[0].as_str() to avoid cloning the Value if it's already a string:

let hex_str = params[0].as_str()
    .ok_or(RpcErr::BadParams("Expected string".to_owned()))?;

Minor Issues

3. Expensive Block Clone in Handler

File: crates/networking/rpc/tracing.rs
Line: 285

trace_block(self.block.clone(), &self.trace_config, &context).await

Since trace_block takes Block by value (consuming it), and handle() receives &self, cloning is required. However, if the underlying blockchain methods (trace_block_calls, etc.) can accept &Block, consider changing trace_block signature to take reference to avoid the clone for large blocks.

4. Test Dependency on L2 Crate

File: test/tests/rpc/debug_trace_tests.rs
Line: 13

use ethrex_l2_rpc::signer::{LocalSigner, Signable, Signer};

This creates a dependency from general RPC tests to L2-specific code. If LocalSigner is used for L1 testing, consider moving it to a common test utilities crate or ensure this test is behind an l2 feature flag.

5. Variable Shadowing

File: crates/networking/rpc/tracing.rs
Lines: 277-278

hex_str is first String, then &str. Rename the second binding to avoid confusion:

let hex_data = hex_str.strip_prefix("0x").ok_or(...)?;

Positive Feedback

  • Correct EIP compliance: The RLP decoding and tracer configuration handling matches the expected behavior for debug_traceBlock.
  • Good test coverage: Unit tests for parsing edge cases (missing 0x, invalid hex, etc.) and integration tests for full flow.
  • Code deduplication: Extracting trace_block to share logic between debug_traceBlockByNumber and debug_traceBlock is idiomatic and maintainable.
  • Proper error propagation: Uses RpcErr::BadParams for client errors and RpcErr::Internal for execution failures.

Suggested Patch for Issue 1 (Size Limit)

const MAX_TRACE_BLOCK_SIZE: usize = 10 * 1024 * 1024; // 10MB

fn parse(params: &Option<Vec<serde_json::Value>>) -> Result<Self, RpcErr> {
    // ... existing validation ...
    let hex_str = params[0].as_str()
        .ok_or(RpcErr::BadParams("Expected string".to_owned()))?;
    let hex_str = hex_str.strip_prefix("0x").ok_or(...)?;
    
    if hex_str.len() > MAX_TRACE_BLOCK_SIZE * 2 {
        return Err(RpcErr::BadParams("Block data too large".to_owned()));
    }
    // ... rest of parsing ...
}

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

@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review

Here is my review of PR #6696:


PR Review: feat(l1): implement debug_traceBlock RPC endpoint

Overview

This PR adds debug_traceBlock to the debug RPC namespace. It accepts a 0x-prefixed hex-encoded RLP block, decodes it, and traces all its transactions. The main implementation work is a clean refactor: shared tracing logic is extracted from TraceBlockByNumberRequest::handle into a standalone trace_block helper that both TraceBlockByNumberRequest and the new TraceBlockRequest delegate to. The approach is correct and the structure matches geth's behavior.


Correctness

Block not validated against chain state

TraceBlockRequest::handle passes the decoded block straight into trace_block, which calls trace_block_calls/trace_block_prestate/trace_block_opcodes. If the block's parent hash doesn't exist in the local chain, the tracing layer will likely surface an internal error rather than a descriptive RpcErr. This is an edge case but will produce a confusing message for callers. A pre-check or a cleaner error mapping would help.

params.len() > 2 guard — correctly rejects requests with 3+ parameters.


Security / DoS Risk

Missing size cap on input (tracing.rs, parse method)

let hex_str: String = serde_json::from_value(params[0].clone())?;
let hex_str = hex_str.strip_prefix("0x")...;
let rlp_bytes = hex::decode(hex_str)...;
let block = Block::decode(&rlp_bytes)...;

There is no upper bound on hex_str length or rlp_bytes.len() before decoding. A malicious client could send a multi-megabyte hex string and force the server to:

  1. Allocate the raw byte vector, then
  2. Attempt a full Block::decode, which may walk deeply nested RLP structures.

Ethereum's maximum block size is bounded in consensus, but the RPC layer has no such guarantee. Adding a guard like:

if rlp_bytes.len() > MAX_BLOCK_SIZE_BYTES {
    return Err(RpcErr::BadParams("Block data exceeds maximum size".to_owned()));
}

before Block::decode would prevent this class of attack. Other decode-from-input paths in the codebase are worth checking for the same pattern.


Performance

Unnecessary Block clone in handle (tracing.rs, TraceBlockRequest::handle)

async fn handle(&self, context: RpcApiContext) -> Result<Value, RpcErr> {
    trace_block(self.block.clone(), &self.trace_config, &context).await
}

trace_block consumes the block by value. Since &self borrows TraceBlockRequest, a clone is forced here. For large blocks this is non-trivial. Options:

  • Change trace_block to accept &Block (if the callee doesn't need ownership).
  • Wrap block in Arc<Block> so the clone is pointer-sized.
  • Switch handle to consume self — though that depends on whether the RpcHandler trait allows it.

The clone in TraceBlockByNumberRequest::handle was also there previously (the block is fetched and immediately passed to trace_block), so this is not a regression, but the new code repeats the pattern.


Code Quality

TraceBlockRequest missing field visibility docs / #[derive] alignment

TraceBlockByNumberRequest and TraceTransactionRequest have #[derive(Deserialize)] (or similar) and doc comments. TraceBlockRequest has neither. It is fine given that parsing is custom, but consistency with surrounding types would be cleaner.

Test helper rpc_call doesn't check for RPC-level errors (debug_trace_tests.rs)

async fn rpc_call(...) -> Value {
    ...
    map_http_requests(&request, context).await.expect("RPC call should succeed")
}

map_http_requests returns Ok(Value) even when the response carries a JSON-RPC error object ({"error": {...}}). The returned Value is never inspected for "error" before the test proceeds. In trace_block_rlp the first assertion is result.as_array().expect(...), which would panic with a non-actionable message if the RPC returned an error. Adding an explicit error check in rpc_call (or at call sites) would make test failures easier to diagnose.

build_and_execute_block assertion (debug_trace_tests.rs)

assert_eq!(block.body.transactions.len(), transactions.len());

This is a good sanity check. One edge case not covered by any test: an empty block (0 transactions). debug_traceBlock on an empty block should return []. Worth a quick smoke test given trace_block would fall through all three match arms and iterate over an empty collection.


Minor / Nits

  • The comment on trace_block's doc line says "used by debug_traceBlockByHash" as well, but that endpoint hasn't been updated to use the shared helper. Either update it (free DRY win) or remove it from the doc comment.
  • params[0].clone() clones the serde_json::Value before serde_json::from_value. from_value takes ownership, so the clone is unnecessary if params[0] is accessed by index after consuming — but since it isn't reused, .clone() can be removed in favour of params.into_iter().next().unwrap() or direct indexing with .take(). Minor.

Summary

The refactor is clean and the implementation is correct for the happy path. The two things worth addressing before merge:

  1. Input size limit in parse — this is the only meaningful security gap.
  2. Block clone in handle — performance concern worth resolving cleanly.

Everything else is polish. Good test coverage for the parse-level error cases; the integration test is solid.


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

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 21, 2026

Greptile Summary

This PR implements the debug_traceBlock RPC endpoint, which accepts a hex-encoded RLP block, decodes it, and traces all its transactions. It also refactors the duplicated block-tracing logic from TraceBlockByNumberRequest into a shared trace_block() async helper now used by both handlers.

  • New TraceBlockRequest handler: parses a 0x-prefixed hex RLP payload, decodes it into a Block, deserializes an optional TraceConfig, and delegates to the shared trace_block() helper — supporting all three existing tracers (callTracer, prestateTracer, opcodeTracer).
  • trace_block() extraction: eliminates ~100 lines of duplicated match logic; TraceBlockByNumberRequest is cleanly updated to call the shared helper.
  • Tests: unit tests for parse-layer error cases (missing prefix, invalid hex, wrong param count) live in tracing.rs; an integration test in debug_trace_tests.rs exercises the full RPC stack with a real in-memory blockchain.

Confidence Score: 4/5

Safe to merge; the refactoring is clean and the new endpoint behaves correctly, with one small hardening gap worth addressing before the endpoint is exposed to untrusted traffic.

The implementation is straightforward and well-tested. The only concern is that TraceBlockRequest::parse performs hex::decode and Block::decode on an unbounded caller-supplied string — every other debug parse path deals with fixed-size hashes or block numbers, making this the sole entry point where a single request can force large allocations. Adding a size cap before decode would close that gap.

crates/networking/rpc/tracing.rs — specifically the TraceBlockRequest::parse method around the hex/RLP decode step

Important Files Changed

Filename Overview
crates/networking/rpc/tracing.rs Adds TraceBlockRequest (RLP-input handler) and extracts shared trace_block() helper; no size guard on the hex/RLP input
crates/networking/rpc/rpc.rs Adds debug_traceBlock route to the dispatch table; trivial one-liner change, correct placement
test/tests/rpc/debug_trace_tests.rs Integration test for debug_traceBlock happy path; unit-level error cases are covered in tracing.rs
test/tests/rpc/mod.rs Module declaration for new debug_trace_tests; no logic changes

Sequence Diagram

sequenceDiagram
    participant Client
    participant RPC as rpc.rs (map_debug_requests)
    participant Handler as TraceBlockRequest
    participant Helper as trace_block()
    participant BC as Blockchain

    Client->>RPC: debug_traceBlock(hexRlpBlock, traceConfig?)
    RPC->>Handler: parse(params)
    Handler->>Handler: strip 0x prefix
    Handler->>Handler: hex::decode → rlp_bytes
    Handler->>Handler: Block::decode(rlp_bytes)
    Handler->>Handler: deserialize TraceConfig
    Handler-->>RPC: "TraceBlockRequest{block, trace_config}"
    RPC->>Handler: handle(context)
    Handler->>Helper: trace_block(block.clone(), trace_config, context)
    alt callTracer
        Helper->>BC: trace_block_calls(block, reexec, timeout, ...)
        BC-->>Helper: "Vec<(H256, Vec<CallTraceFrame>)>"
        Helper-->>RPC: "JSON array of {txHash, result}"
    else prestateTracer
        Helper->>BC: trace_block_prestate(block, reexec, timeout, ...)
        BC-->>Helper: "Vec<(H256, PrestateResult)>"
        Helper-->>RPC: "JSON array of {txHash, result}"
    else opcodeTracer
        Helper->>BC: trace_block_opcodes(block, reexec, timeout, cfg)
        BC-->>Helper: "Vec<(H256, StructLoggerResult)>"
        Helper-->>RPC: "JSON array of {txHash, result}"
    end
    RPC-->>Client: JSON response
Loading
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:274-279
No size limit is enforced on the hex-encoded RLP input before calling `hex::decode` and `Block::decode`. A caller can submit an arbitrarily large payload, forcing the server to allocate and parse several hundred MB of data per request. All other RPC parse paths deal with small structured values (hashes, block numbers), so this is the only path with an unbounded allocation. Even a rough cap (e.g. 4–8 MB of raw bytes) would eliminate the trivial exhaustion case.

```suggestion
        let hex_str: String = serde_json::from_value(params[0].clone())?;
        let hex_str = hex_str.strip_prefix("0x").ok_or(RpcErr::BadParams(
            "Block data must be 0x-prefixed".to_owned(),
        ))?;
        const MAX_RLP_HEX_LEN: usize = 8 * 1024 * 1024 * 2; // 8 MB of raw bytes → 16 MB hex chars
        if hex_str.len() > MAX_RLP_HEX_LEN {
            return Err(RpcErr::BadParams("Block RLP exceeds maximum allowed size".to_owned()));
        }
        let rlp_bytes =
            hex::decode(hex_str).map_err(|e| RpcErr::BadParams(format!("Invalid hex: {e}")))?;
```

Reviews (1): Last reviewed commit: "style: run cargo fmt on test and source ..." | Re-trigger Greptile

Comment on lines +274 to +279
let hex_str: String = serde_json::from_value(params[0].clone())?;
let hex_str = hex_str.strip_prefix("0x").ok_or(RpcErr::BadParams(
"Block data must be 0x-prefixed".to_owned(),
))?;
let rlp_bytes =
hex::decode(hex_str).map_err(|e| RpcErr::BadParams(format!("Invalid hex: {e}")))?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No size limit is enforced on the hex-encoded RLP input before calling hex::decode and Block::decode. A caller can submit an arbitrarily large payload, forcing the server to allocate and parse several hundred MB of data per request. All other RPC parse paths deal with small structured values (hashes, block numbers), so this is the only path with an unbounded allocation. Even a rough cap (e.g. 4–8 MB of raw bytes) would eliminate the trivial exhaustion case.

Suggested change
let hex_str: String = serde_json::from_value(params[0].clone())?;
let hex_str = hex_str.strip_prefix("0x").ok_or(RpcErr::BadParams(
"Block data must be 0x-prefixed".to_owned(),
))?;
let rlp_bytes =
hex::decode(hex_str).map_err(|e| RpcErr::BadParams(format!("Invalid hex: {e}")))?;
let hex_str: String = serde_json::from_value(params[0].clone())?;
let hex_str = hex_str.strip_prefix("0x").ok_or(RpcErr::BadParams(
"Block data must be 0x-prefixed".to_owned(),
))?;
const MAX_RLP_HEX_LEN: usize = 8 * 1024 * 1024 * 2; // 8 MB of raw bytes → 16 MB hex chars
if hex_str.len() > MAX_RLP_HEX_LEN {
return Err(RpcErr::BadParams("Block RLP exceeds maximum allowed size".to_owned()));
}
let rlp_bytes =
hex::decode(hex_str).map_err(|e| RpcErr::BadParams(format!("Invalid hex: {e}")))?;
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 274-279

Comment:
No size limit is enforced on the hex-encoded RLP input before calling `hex::decode` and `Block::decode`. A caller can submit an arbitrarily large payload, forcing the server to allocate and parse several hundred MB of data per request. All other RPC parse paths deal with small structured values (hashes, block numbers), so this is the only path with an unbounded allocation. Even a rough cap (e.g. 4–8 MB of raw bytes) would eliminate the trivial exhaustion case.

```suggestion
        let hex_str: String = serde_json::from_value(params[0].clone())?;
        let hex_str = hex_str.strip_prefix("0x").ok_or(RpcErr::BadParams(
            "Block data must be 0x-prefixed".to_owned(),
        ))?;
        const MAX_RLP_HEX_LEN: usize = 8 * 1024 * 1024 * 2; // 8 MB of raw bytes → 16 MB hex chars
        if hex_str.len() > MAX_RLP_HEX_LEN {
            return Err(RpcErr::BadParams("Block RLP exceeds maximum allowed size".to_owned()));
        }
        let rlp_bytes =
            hex::decode(hex_str).map_err(|e| RpcErr::BadParams(format!("Invalid hex: {e}")))?;
```

How can I resolve this? If you propose a fix, please make it concise.

@github-actions
Copy link
Copy Markdown

🤖 Codex Code Review

Findings

  1. High: debug_traceBlock now accepts arbitrary RLP and sends it straight into the tracing path without any block validation, so it can return traces for blocks that ethrex itself would reject. The new handler decodes the raw block at tracing.rs and immediately traces it at tracing.rs. That path rebuilds parent state and replays txs via blockchain/tracing.rs and vm/backends/levm/tracing.rs, but it never runs the normal pre/post-execution checks that imported blocks get in blockchain.rs. A caller can therefore tweak header fields or roots and still get a “successful” trace for an invalid block. I’d either validate raw blocks before tracing or return an explicit invalid-block error.

  2. Medium: TraceBlockRequest::handle clones the full decoded Block before tracing at tracing.rs. For raw-block tracing this is an avoidable O(block size) copy, and with the RPC body limit set to 256 MB it can noticeably increase peak memory and CPU for a single debug request. Using Arc<Block> in the request or deferring decode until handle would avoid the extra copy.

Aside from those points, the shared trace_block refactor is clean and the added parse tests are useful. I couldn’t run cargo test -p ethrex-rpc debug_trace_tests here because rustup failed to create temp files under /home/runner/.rustup/tmp in this environment.


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

Implement debug_traceBlockByHash and debug_traceBlock (raw RLP)

1 participant