Skip to content

feat(l1): implement noopTracer for debug trace endpoints#6694

Draft
azteca1998 wants to merge 5 commits into
mainfrom
feat/noop-tracer
Draft

feat(l1): implement noopTracer for debug trace endpoints#6694
azteca1998 wants to merge 5 commits into
mainfrom
feat/noop-tracer

Conversation

@azteca1998
Copy link
Copy Markdown
Contributor

Summary

  • Adds noopTracer variant to the tracer type enum
  • Returns {} for debug_traceTransaction, and {txHash, result: {}} per tx for debug_traceBlockByNumber
  • Useful for benchmarking raw execution overhead without tracing cost
  • Matches geth's noopTracer

Closes part of #6572

Test plan

  • debug_traceTransaction with {"tracer": "noopTracer"} returns {}
  • debug_traceBlockByNumber with {"tracer": "noopTracer"} returns array of {txHash, result: {}}

Add noopTracer variant that returns an empty JSON object {} per
transaction. Useful for benchmarking execution overhead without
tracing cost. Supported in both debug_traceTransaction and
debug_traceBlockByNumber.

Part of #6572
@azteca1998 azteca1998 requested a review from a team as a code owner May 21, 2026 10:45
@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

🤖 Kimi Code Review

The PR adds a NoopTracer variant for benchmarking execution overhead without tracing costs. The implementation is correct and follows existing patterns, with minor optimization opportunities.

crates/networking/rpc/tracing.rs

  1. Iterator efficiency (minor) - Lines 366-377: The block tracing implementation allocates an intermediate Vec<H256> unnecessarily. You can fuse the operations to avoid the temporary allocation:

    let block_trace: BlockTrace<Value> = block
        .body
        .transactions
        .iter()
        .map(|tx| (tx.hash(), serde_json::json!({})).into())
        .collect();
  2. Documentation consistency - Line 62-65: The comment correctly documents geth compatibility. Consider adding a note that this tracer ignores all tracerConfig options (if that's the case), similar to how other tracers might document their configuration behavior.

  3. Type safety - Line 372: The .into() conversion assumes the target type implements From<(H256, Value)>. Verify this matches the BlockTrace definition used by other tracers (appears consistent with the pattern).

Security & Correctness

  • No vulnerabilities detected. The tracer returns constant empty JSON objects with no side effects on EVM execution or state.
  • Gas accounting and consensus logic are unaffected (this is RPC-layer only).

Verdict
Approach is sound. Consider applying the iterator fusion suggestion in Point 1 for slightly better memory efficiency with large blocks, though this is non-blocking given the tracer's benchmarking purpose.


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

@azteca1998 azteca1998 marked this pull request as draft May 21, 2026 10:46
@ethrex-project-sync ethrex-project-sync Bot moved this to In Progress in ethrex_l1 May 21, 2026
@github-actions
Copy link
Copy Markdown

🤖 Codex Code Review

  1. noopTracer currently changes the semantics of both RPCs by skipping replay entirely instead of replaying with tracing disabled. In crates/networking/rpc/tracing.rs:216 and crates/networking/rpc/tracing.rs:363, the new branch returns {} / [{txHash,result:{}}] without calling into blockchain at all. That means:

    • debug_traceTransaction now succeeds for any existing-looking request shape even if the tx is missing, the parent state cannot be rebuilt within reexec, sender recovery fails, or execution itself would error.
    • debug_traceBlockByNumber ignores replay/execution failures and timeout, so it reports success for blocks that the normal tracers cannot actually replay.
    • The stated benchmarking use case is not met, because this measures “skip all execution” overhead, not “execute without trace collection”.

    Suggested fix: keep the RPC output empty, but add a real no-op replay path in blockchain/vm that still rebuilds state and executes the tx/block, then discard the collected result.

I didn’t find other issues in the small diff, but this one is significant enough that I would not merge as-is. There’s also no coverage for the new tracer path, so once the replay semantics are fixed, add RPC tests for success and failure cases.


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

@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review

Here is the review:


PR Review: feat(l1): implement noopTracer for debug trace endpoints

Summary

Clean, focused implementation matching geth's noopTracer semantics. No security issues, no correctness bugs, and error handling is consistent with the existing tracer branches. Two minor points below.


Findings

1. Unnecessary intermediate Vec<H256> allocation (lines 366–375)

The tx_hashes intermediate collection is created only to be immediately consumed. The two iterator chains can be collapsed into one:

// current
let tx_hashes: Vec<H256> = block
    .body
    .transactions
    .iter()
    .map(|tx| tx.hash())
    .collect();
let block_trace: BlockTrace<Value> = tx_hashes
    .into_iter()
    .map(|hash| (hash, serde_json::json!({})).into())
    .collect();

// simpler
let block_trace: BlockTrace<Value> = block
    .body
    .transactions
    .iter()
    .map(|tx| (tx.hash(), serde_json::json!({})).into())
    .collect();

Since this tracer is specifically described as useful for benchmarking raw execution overhead, eliminating the extra heap allocation is a small but meaningful consistency win.


2. Dead reexec / timeout computation for NoopTracer (lines 257–258)

let reexec = self.trace_config.reexec.unwrap_or(DEFAULT_REEXEC);
let timeout = self.trace_config.timeout.unwrap_or(DEFAULT_TIMEOUT);

These are computed unconditionally but are never read in the NoopTracer arm. This is harmless today (both are just integer unwraps), but it's a slight inconsistency since NoopTracer skips execution entirely. A broader refactor could move these declarations inside the arms that actually use them — though that's a pre-existing pattern affecting all non-noop branches, so it's out of scope here.


What's correct

  • #[serde(rename_all = "camelCase")] correctly maps NoopTracer"noopTracer" on the wire.
  • Returning serde_json::json!({}) for debug_traceTransaction matches geth's noopTracer output.
  • Wrapping results in BlockTrace<Value> with (hash, {}).into() for debug_traceBlockByNumber correctly produces the [{txHash, result: {}}] shape geth returns.
  • No execution or state access is performed for NoopTracer — the block is fetched only to enumerate transaction hashes, which is the minimum required for the response shape.
  • The #[allow(clippy::enum_variant_names)] suppression correctly covers the new variant without requiring a separate allow at the variant level.

Overall: Approve pending the intermediate allocation cleanup in point 1. Point 2 is a nit, not a blocker.


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

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 21, 2026

Lines of code report

Total lines added: 86
Total lines removed: 0
Total lines changed: 86

Detailed view
+-----------------------------------------+-------+------+
| File                                    | Lines | Diff |
+-----------------------------------------+-------+------+
| ethrex/crates/networking/rpc/tracing.rs | 401   | +86  |
+-----------------------------------------+-------+------+

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 21, 2026

Greptile Summary

This PR adds a NoopTracer variant to the TracerType enum, wiring it into both debug_traceTransaction and debug_traceBlockByNumber handlers to return empty JSON objects ({}) without re-executing any transactions.

  • debug_traceTransaction with noopTracer returns {} unconditionally, skipping both transaction lookup and EVM execution — any hash, including fabricated ones, receives a 200 response.
  • debug_traceBlockByNumber with noopTracer maps each transaction's hash to {txHash, result: {}} without re-executing any transactions; block existence is validated, but execution is bypassed entirely.

Confidence Score: 3/5

The debug_traceTransaction arm of noopTracer returns {} for any hash — real, fake, or zeroed — because no transaction lookup is performed before responding.

The NoopTracer path in TraceTransactionRequest skips the transaction existence check that every other tracer performs, so a caller can pass a fabricated hash and receive a successful {} response.

crates/networking/rpc/tracing.rs — specifically the NoopTracer arm inside TraceTransactionRequest::handle

Important Files Changed

Filename Overview
crates/networking/rpc/tracing.rs Adds NoopTracer variant that returns empty JSON objects without re-executing transactions; the traceTransaction arm lacks any tx-existence check, and the overall behaviour diverges from geth's noopTracer which still runs the EVM.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[RPC Request] --> B{Endpoint}
    B --> C[debug_traceTransaction]
    B --> D[debug_traceBlockByNumber]
    C --> E{TracerType}
    E -->|callTracer / prestateTracer / opcodeTracer| F[Validate tx hash, re-execute via EVM, return trace]
    E -->|noopTracer| G[Return empty object - NO tx validation, NO execution]
    D --> H[Resolve and fetch block - existence validated]
    H --> I{TracerType}
    I -->|callTracer / prestateTracer / opcodeTracer| J[Re-execute all txs via EVM, return per-tx trace]
    I -->|noopTracer| K[Map tx hashes to empty objects - NO execution]
Loading
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:216
**Missing transaction existence validation**

For every other tracer, the handler calls a blockchain method that validates the transaction hash is real and returns an error if it is not found. The `NoopTracer` arm skips execution entirely, so calling `debug_traceTransaction` with a non-existent or fabricated hash returns `{}` with status 200 instead of an appropriate error. Clients that probe for transaction existence via this endpoint will silently receive a success response and may incorrectly assume the transaction was traced.

### Issue 2 of 2
crates/networking/rpc/tracing.rs:363-377
**Behavioral divergence from geth's noopTracer**

The PR description states this matches geth's `noopTracer`, but geth still re-executes transactions through the EVM with all hook methods implemented as no-ops — the empty result is a consequence of no tracing being recorded, not of skipping execution. This implementation skips execution entirely, so it cannot be used for its stated purpose of "benchmarking raw execution overhead without tracing cost".

Reviews (1): Last reviewed commit: "feat(l1): implement noopTracer for debug..." | Re-trigger Greptile

Comment thread crates/networking/rpc/tracing.rs Outdated
emit,
})?)
}
TracerType::NoopTracer => Ok(serde_json::json!({})),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Missing transaction existence validation

For every other tracer, the handler calls a blockchain method that validates the transaction hash is real and returns an error if it is not found. The NoopTracer arm skips execution entirely, so calling debug_traceTransaction with a non-existent or fabricated hash returns {} with status 200 instead of an appropriate error. Clients that probe for transaction existence via this endpoint will silently receive a success response and may incorrectly assume the transaction was traced.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 216

Comment:
**Missing transaction existence validation**

For every other tracer, the handler calls a blockchain method that validates the transaction hash is real and returns an error if it is not found. The `NoopTracer` arm skips execution entirely, so calling `debug_traceTransaction` with a non-existent or fabricated hash returns `{}` with status 200 instead of an appropriate error. Clients that probe for transaction existence via this endpoint will silently receive a success response and may incorrectly assume the transaction was traced.

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

Comment on lines +363 to +377
TracerType::NoopTracer => {
// Return one empty object per transaction in the block, matching
// geth's behaviour where noopTracer produces {} for each tx.
let tx_hashes: Vec<H256> = block
.body
.transactions
.iter()
.map(|tx| tx.hash())
.collect();
let block_trace: BlockTrace<Value> = tx_hashes
.into_iter()
.map(|hash| (hash, serde_json::json!({})).into())
.collect();
Ok(serde_json::to_value(block_trace)?)
}
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 Behavioral divergence from geth's noopTracer

The PR description states this matches geth's noopTracer, but geth still re-executes transactions through the EVM with all hook methods implemented as no-ops — the empty result is a consequence of no tracing being recorded, not of skipping execution. This implementation skips execution entirely, so it cannot be used for its stated purpose of "benchmarking raw execution overhead without tracing cost".

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/tracing.rs
Line: 363-377

Comment:
**Behavioral divergence from geth's noopTracer**

The PR description states this matches geth's `noopTracer`, but geth still re-executes transactions through the EVM with all hook methods implemented as no-ops — the empty result is a consequence of no tracing being recorded, not of skipping execution. This implementation skips execution entirely, so it cannot be used for its stated purpose of "benchmarking raw execution overhead without tracing cost".

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

Trace a real executed transaction with noopTracer and assert
the response is an empty JSON object.
noopTracer was returning {} without validating that the transaction
exists or re-executing it. This diverged from geth, which still runs
the EVM with no-op hooks. Now we call trace_transaction_calls /
trace_block_calls and discard the results, matching geth semantics
and properly erroring on invalid tx hashes.
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 Progress

Development

Successfully merging this pull request may close these issues.

1 participant