feat(l1): implement debug_dumpBlock endpoint#6701
Conversation
|
Lines of code reportTotal lines added: Detailed view |
azteca1998
changed the title
🤖 Claude Code ReviewHere is my review of PR #6701: PR #6701 —
|
🤖 Kimi Code ReviewThe implementation is solid and well-documented. A few specific observations: Security/DoS Concerns
Code Quality
Testing
Nits
The RPC routing and module registration are correct. The pagination logic using hashed addresses is sound for trie iteration. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Codex Code Review
I did not review any EVM/gas-accounting path here; this PR is RPC-only. I also could not run the targeted test slice because Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Greptile SummaryImplements the
Confidence Score: 4/5The core logic is sound and the pagination cursor works correctly; the two open items are a balance serialization format that silently diverges from geth without being documented, and no server-side cap when callers override maxResults explicitly. The implementation is structurally correct — block resolution, trie iteration, spawn_blocking usage, and pagination cursor semantics are all handled properly. The balance field serializes as hex rather than the decimal string geth produces, and that divergence is missing from the documented list, which could quietly break any consumer relying on geth-format output. The uncapped maxResults is a minor resource concern for large state roots but is isolated to this debug endpoint. crates/networking/rpc/debug/dump_block.rs warrants a second look for the balance serialization format and the maxResults bound.
|
| Filename | Overview |
|---|---|
| crates/networking/rpc/debug/dump_block.rs | Core implementation of debug_dumpBlock; pagination logic is correct, but balance serializes as hex (undocumented geth divergence) and user-supplied maxResults has no upper-bound cap. |
| crates/networking/rpc/debug/mod.rs | Trivial one-line addition exposing the new dump_block module; no issues. |
| crates/networking/rpc/rpc.rs | Registers debug_dumpBlock route in the dispatch table; correctly placed and no issues. |
| test/tests/rpc/debug_dump_block_tests.rs | Good integration test coverage for basic state dump, block-not-found error, latest tag, and pagination; tests expect hex-encoded balance which is consistent with ethrex's U256 serde but differs from geth. |
| test/tests/rpc/helpers.rs | New shared RPC test helpers for building in-memory stores and executing blocks; well-structured with appropriate dead-code allowance. |
| test/tests/rpc/mod.rs | Module registration for new test files; no issues. |
Sequence Diagram
sequenceDiagram
participant Client
participant RPC as map_debug_requests
participant Handler as DumpBlockRequest
participant Storage as Store
Client->>RPC: debug_dumpBlock(block, config?)
RPC->>Handler: parse params
Handler->>Storage: resolve_block_number(block)
Storage-->>Handler: "Option<block_number>"
Handler->>Storage: get_block_header(block_number)
Storage-->>Handler: "Option<BlockHeader>"
Handler->>Handler: spawn_blocking
Handler->>Storage: iter_accounts_from(state_root, start)
loop up to maxResults accounts
Storage-->>Handler: (hashed_addr, AccountState)
end
Handler-->>Handler: "(accounts BTreeMap, next Option<H256>)"
Handler-->>RPC: "DumpResult { root, accounts, next? }"
RPC-->>Client: JSON response
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/debug/dump_block.rs:63-70
**Undocumented balance format divergence from geth**
`balance: U256` with `ethereum_types`'s default serde implementation serializes as a hex string (`"0x0de0b6b3a7640000"`), but geth's `debug_dumpBlock` returns `balance` as a decimal string (`"1000000000000000000"`). The doc comment above lists three explicit divergences from geth but omits this one. Any tool parsing the balance field expecting geth's decimal format will silently misread the value. Either serialize balance as decimal to match geth, or add it to the documented divergences list.
### Issue 2 of 2
crates/networking/rpc/debug/dump_block.rs:108-120
**`maxResults` has no upper bound when explicitly provided**
The `DEFAULT_MAX_RESULTS = 100_000` cap only applies when `maxResults` is omitted from the config object. When a caller explicitly provides `{"maxResults": 18446744073709551615}`, the whole trie is iterated without any limit. On a large mainnet state (~200 M+ accounts) this could exhaust server memory or hold the blocking thread for minutes. Consider clamping the user-supplied value to a maximum (e.g., `max_results.min(MAX_ALLOWED_RESULTS)`) so the per-request bound holds regardless of what the caller sends.
Reviews (1): Last reviewed commit: "fix(rpc): make debug_dumpBlock non-block..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
|
|
||
| #[derive(Serialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| struct DumpAccount { | ||
| balance: U256, | ||
| nonce: u64, | ||
| root: H256, |
There was a problem hiding this comment.
balance: U256 with ethereum_types's default serde implementation serializes as a hex string ("0x0de0b6b3a7640000"), but geth's debug_dumpBlock returns balance as a decimal string ("1000000000000000000"). The doc comment above lists three explicit divergences from geth but omits this one. Any tool parsing the balance field expecting geth's decimal format will silently misread the value. Either serialize balance as decimal to match geth, or add it to the documented divergences list.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/debug/dump_block.rs
Line: 63-70
Comment:
**Undocumented balance format divergence from geth**
`balance: U256` with `ethereum_types`'s default serde implementation serializes as a hex string (`"0x0de0b6b3a7640000"`), but geth's `debug_dumpBlock` returns `balance` as a decimal string (`"1000000000000000000"`). The doc comment above lists three explicit divergences from geth but omits this one. Any tool parsing the balance field expecting geth's decimal format will silently misread the value. Either serialize balance as decimal to match geth, or add it to the documented divergences list.
How can I resolve this? If you propose a fix, please make it concise.| let storage = context.storage.clone(); | ||
|
|
||
| // The trie iterator opens long-lived locked DB transactions and walks | ||
| // the state synchronously — must run off the async runtime. | ||
| let (accounts, next) = tokio::task::spawn_blocking(move || { | ||
| let iter = storage.iter_accounts_from(state_root, start)?; | ||
| let mut accounts: BTreeMap<H256, DumpAccount> = BTreeMap::new(); | ||
| let mut next = None; | ||
| for (hashed_addr, account_state) in iter { | ||
| if accounts.len() >= max_results { | ||
| next = Some(hashed_addr); | ||
| break; | ||
| } |
There was a problem hiding this comment.
maxResults has no upper bound when explicitly provided
The DEFAULT_MAX_RESULTS = 100_000 cap only applies when maxResults is omitted from the config object. When a caller explicitly provides {"maxResults": 18446744073709551615}, the whole trie is iterated without any limit. On a large mainnet state (~200 M+ accounts) this could exhaust server memory or hold the blocking thread for minutes. Consider clamping the user-supplied value to a maximum (e.g., max_results.min(MAX_ALLOWED_RESULTS)) so the per-request bound holds regardless of what the caller sends.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/networking/rpc/debug/dump_block.rs
Line: 108-120
Comment:
**`maxResults` has no upper bound when explicitly provided**
The `DEFAULT_MAX_RESULTS = 100_000` cap only applies when `maxResults` is omitted from the config object. When a caller explicitly provides `{"maxResults": 18446744073709551615}`, the whole trie is iterated without any limit. On a large mainnet state (~200 M+ accounts) this could exhaust server memory or hold the blocking thread for minutes. Consider clamping the user-supplied value to a maximum (e.g., `max_results.min(MAX_ALLOWED_RESULTS)`) so the per-request bound holds regardless of what the caller sends.
How can I resolve this? If you propose a fix, please make it concise.The previous implementation was structurally wrong for two of the three
flat-frame variants:
- `create` actions emitted `input` and a result of `{gasUsed, output}`.
geth's flatCallTracer (and Parity's spec) require `init` in the action
and `{address, code, gasUsed}` in the result so callers can recover the
deployed contract's address and runtime bytecode.
- `suicide` (SELFDESTRUCT) frames emitted the call-shape action and a
bogus `{gasUsed, output}` result. The Parity shape is
`{address, balance, refundAddress}` with no result at all.
Split `FlatCallAction` / `FlatCallResult` into per-variant enums (untagged
so the outer `type` field continues to discriminate), map the LEVM call
tracer's SELFDESTRUCT frame fields onto the suicide shape (`from` →
destructed contract, `to` → refund target, `value` → balance), and emit
`init`/`address`/`code` for create frames. Use `&'static str` for the
static label fields to avoid per-frame allocations.
Unit tests now assert against the serialized JSON projection — the wire
shape clients actually receive — and cover create/create2, selfdestruct,
and failed-frame `result` omission. Integration tests gain a CREATE
end-to-end (deploys a tiny init code and verifies the action/result
shape), a `debug_traceBlockByNumber` test, and a missing-tx failure case
that Codex's review flagged as a gap.
Test setup boilerplate moves out of the misnamed `debug_trace_tests.rs`
into a shared `rpc::helpers` module so #6694 and #6701 can rebase to
re-use it instead of duplicating ~140 lines per feature. The file is
renamed to `debug_flat_call_tracer_tests.rs`.
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.
Three real issues with the original handler: - Wrong return values. The handler emitted `Address::from(hash)`, which takes the low 20 bytes of the 32-byte hashed-address key. That value isn't the original address — without a preimage store (ethrex has none) you can't recover it. Switch the response type to `Vec<H256>` of hashed addresses and document the divergence from geth on the type. The new `*_includes_sender` integration test asserts the sender's `keccak256(address)` appears in the result; it would have failed against the original truncation. - Blocking I/O on the async runtime. `Store::iter_accounts` opens four long-lived locked DB transactions and walks the trie synchronously (same reason snap-sync's validators use `spawn_blocking`). The handler now runs the diff on a blocking thread. - Full state materialised twice. The original collected every account from both tries into `HashSet<(H256, Vec<u8>)>` keyed on a stringified debug-format value — O(state) memory and millions of `format!()` allocations on mainnet. Replace with a streaming merge of the two iterators (both yield in hashed-key order from the trie traversal), comparing `AccountState` by value. O(1) extra memory. Test setup boilerplate moves out of the misnamed `debug_trace_tests.rs` into the shared `rpc::helpers` module introduced in #6701; the file is renamed to `debug_get_modified_accounts_tests.rs`. Integration coverage now spans: sender-appears-in-diff, by-number/by-hash equivalence, empty-range, start-after-end (both variants), and unknown-block.
Tests as shipped exercised one path: callTracer, "latest" block, against a genesis-only state. That left the three tracer variants asymmetric on coverage and didn't validate any of the items on the PR's own test plan. Added integration tests covering: prestateTracer + opcodeTracer paths, block-by-number, block-by-hash (EIP-1898 form), default-to-latest when the block parameter is omitted, unknown-block error, and empty-params error. The post-block tests pin `nonce` in the call object because ethrex's LEVM enforces nonce equality even on simulated calls — a pre-existing constraint shared with `eth_call`, documented inline so the next person doesn't repeat the diagnosis. Documented two ergonomic gaps on `TraceCallRequest` itself: - `stateOverrides` / `blockOverrides` from the third parameter are silently ignored. Real geth-compat support requires plumbing overrides down to the VM, so this is called out as a known divergence rather than fixed in-flight. - Pruned nodes that don't have the target block's state will surface the storage layer's "state root missing" error as a generic Internal. Test setup boilerplate moves out of the misnamed `debug_trace_tests.rs` into the shared `rpc::helpers` module introduced in #6701; the file is renamed to `debug_trace_call_tests.rs`. Also polished the handler's default-block fallback (one allocation instead of an `Option::clone`) and switched the not-found error from "Block not Found" to "Block not found" to match the casing every other endpoint uses.
…the test plan The handler iterated the state trie synchronously inside an async RPC method via `iter_accounts_from`, which opens four long-lived locked DB transactions and walks the trie node by node (same mechanics that forced `debug_dumpBlock` onto `spawn_blocking`). Switch to a blocking thread. Widen the first parameter from a raw `H256` block hash to `BlockIdentifierOrHash`, so clients can pass a block number, a tag (`latest`, etc.), an EIP-1898 object, or a hash — matching geth's documented `blockNrOrHash` signature for `debug_accountRange`. Document the implementation gaps on the type rather than papering over them: `txIndex` is parsed (and accepts geth's `-1` sentinel via `i64`) but ignored — returning the state immediately after the Nth transaction needs mid-block trie-root reconstruction. `code` / `storage` are not emitted; `nocode` / `nostorage` / `incompletes` (geth params 5–7) are therefore not accepted. `AccountEntry.key` is always `null` because ethrex has no preimage store. The PR shipped with zero integration coverage of its own test plan. Test setup boilerplate moves into the shared `rpc::helpers` module from #6701, the misnamed `debug_trace_tests.rs` becomes `debug_account_range_tests.rs`, and the new tests check all four items: returns account entries with the sender's hashed address visible, paginates correctly (max=1 page then continue from `next`), reports all-zero `next` on completion, and errors on both unknown block hash and unknown block number. Also added parse tests for the new block-number and tag inputs.
…fy content
Same shape of issues as the sibling debug_accountRange PR:
- The trie iteration ran inside `async fn handle`, blocking the tokio
worker on `iter_storage_from`'s long-lived locked DB transactions.
Wrapped in `tokio::task::spawn_blocking`.
- The first parameter was a raw `H256` block hash. Widened to
`BlockIdentifierOrHash` so clients can pass a block number, tag, hash,
or EIP-1898 object — matching geth's documented `blockNrOrHash`.
- `txIndex` was `usize`, refusing geth's `-1` sentinel at parse time;
switched to `i64`. The limitation (txIndex is currently ignored — we
always return state at the end of the block) is documented on the
type rather than hidden in a one-line TODO.
The shipped integration test queried storage on the *sender* (an EOA
with no storage trie), so it only exercised the empty-result path. None
of the four items on the PR's own test plan had real coverage.
Replaced it with a deploy-based fixture: the new helper
`setup_block_with_storage_contract` deploys a tiny contract whose
constructor sets three storage slots (slot 0 = 0x11, 1 = 0x22, 2 = 0x33)
via SSTORE, computing the deployed address via `calculate_create_address`.
Tests now verify: the three slots come back with the right values keyed
on their `keccak256(slot_index)` hashes, pagination with `max=1` walks
all three slots via `nextKey`, an unknown address returns
`{storage:{}, nextKey:null}` (matches geth), an EOA's address behaves
identically (no error), block-by-number resolves the same state, and an
unknown block hash returns "Block not found".
Test setup boilerplate moves into the shared `rpc::helpers` module from
#6701 (with the new `create_deploy_tx` / `setup_block_with_storage_contract`
helpers); the misnamed `debug_trace_tests.rs` is renamed to
`debug_storage_range_at_tests.rs`.
…oots
The shipped integration test only asserted that the response is "an
array of hex strings" — a check any half-broken implementation would
pass. The four items on the PR's own test plan all lacked real coverage.
Three meaningful tests now lock in the algorithm's correctness:
- single-tx: the only intermediate root must equal `block.header.state_root`.
For an empty-withdrawal block, state_after_last_tx == block.state_root,
so this is the strongest single-block correctness check available
without re-running execution in the test harness.
- multi-tx (3 transfers from the same sender via a new
`setup_multi_transfer_block` helper): three roots are returned,
consecutive ones differ (each tx mutates the sender's nonce/balance),
and the final root equals `block.state_root`.
- unknown-block: returns "Block not found".
A fourth test covers the config-object form (`{timeout, reexec}`).
Test setup boilerplate moves into the shared `rpc::helpers` module from
#6701 (with the new `setup_multi_transfer_block` helper); the misnamed
`debug_trace_tests.rs` is renamed to `debug_intermediate_roots_tests.rs`.
Implementation review note: the handler currently rebuilds the full
state trie from `parent_state_root` on every iteration (open trie +
apply cumulative updates), which is O(N²) in the tx count. Correct and
fine for typical 200-tx mainnet blocks but worth optimising via
incremental updates if this endpoint ever sees serious use. The
quadratic cost was not changed here — the change scope is correctness
coverage.
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`.
Tests as shipped exercised one path: callTracer, "latest" block, against a genesis-only state. That left the three tracer variants asymmetric on coverage and didn't validate any of the items on the PR's own test plan. Added integration tests covering: prestateTracer + opcodeTracer paths, block-by-number, block-by-hash (EIP-1898 form), default-to-latest when the block parameter is omitted, unknown-block error, and empty-params error. The post-block tests pin `nonce` in the call object because ethrex's LEVM enforces nonce equality even on simulated calls — a pre-existing constraint shared with `eth_call`, documented inline so the next person doesn't repeat the diagnosis. Documented two ergonomic gaps on `TraceCallRequest` itself: - `stateOverrides` / `blockOverrides` from the third parameter are silently ignored. Real geth-compat support requires plumbing overrides down to the VM, so this is called out as a known divergence rather than fixed in-flight. - Pruned nodes that don't have the target block's state will surface the storage layer's "state root missing" error as a generic Internal. Test setup boilerplate moves out of the misnamed `debug_trace_tests.rs` into the shared `rpc::helpers` module introduced in #6701; the file is renamed to `debug_trace_call_tests.rs`. Also polished the handler's default-block fallback (one allocation instead of an `Option::clone`) and switched the not-found error from "Block not Found" to "Block not found" to match the casing every other endpoint uses.
Three real issues with the original handler: - Wrong return values. The handler emitted `Address::from(hash)`, which takes the low 20 bytes of the 32-byte hashed-address key. That value isn't the original address — without a preimage store (ethrex has none) you can't recover it. Switch the response type to `Vec<H256>` of hashed addresses and document the divergence from geth on the type. The new `*_includes_sender` integration test asserts the sender's `keccak256(address)` appears in the result; it would have failed against the original truncation. - Blocking I/O on the async runtime. `Store::iter_accounts` opens four long-lived locked DB transactions and walks the trie synchronously (same reason snap-sync's validators use `spawn_blocking`). The handler now runs the diff on a blocking thread. - Full state materialised twice. The original collected every account from both tries into `HashSet<(H256, Vec<u8>)>` keyed on a stringified debug-format value — O(state) memory and millions of `format!()` allocations on mainnet. Replace with a streaming merge of the two iterators (both yield in hashed-key order from the trie traversal), comparing `AccountState` by value. O(1) extra memory. Test setup boilerplate moves out of the misnamed `debug_trace_tests.rs` into the shared `rpc::helpers` module introduced in #6701; the file is renamed to `debug_get_modified_accounts_tests.rs`. Integration coverage now spans: sender-appears-in-diff, by-number/by-hash equivalence, empty-range, start-after-end (both variants), and unknown-block.
Add debug_dumpBlock which dumps the entire state at a given block, returning the state root and all accounts with their balance, nonce, storage root, and code hash.
Execute a block, then query debug_dumpBlock by number. Assert the response has root and accounts fields with non-empty state.
The handler iterated the entire state trie synchronously inside an async
RPC method via `iter_accounts`, which opens four long-lived locked DB
transactions (see `BackendTrieDBLocked`) and walks the trie node by
node. On any non-trivial state this stalls the tokio worker and pins DB
locks for the duration. Snap-sync already wraps the same call in
`spawn_blocking` for exactly this reason.
Move the traversal onto a blocking thread and add geth-style pagination:
`debug_dumpBlock` now accepts an optional `{start, maxResults}` config
object and emits a `next` cursor when the result is truncated. The
default `maxResults` is 100_000 — a divergence from geth's unbounded
default, documented on the type, that protects the RPC server from
mainnet-scale dumps. Also documents that `code`/`storage` are not
emitted and that `iter_accounts` silently truncates on decode errors.
Test setup helpers move out of the single test file into a shared
`rpc::helpers` module so #6694 and any future endpoint can reuse them
instead of duplicating ~140 lines per feature. The misnamed
`debug_trace_tests.rs` becomes `debug_dump_block_tests.rs` and gains
coverage for: block-not-found, `latest` tag resolution, sender appears
in the dump with expected nonce/balance, and end-to-end pagination
across two pages via `start`/`maxResults`/`next`.
azteca1998
force-pushed
the
feat/debug-dump-block
branch
from
e959e66 to
8130d7c
Compare
ElFantasma
left a comment
ElFantasma
left a comment
There was a problem hiding this comment.
Caller-supplied maxResults bypasses the DoS protection the default is there to provide. The doc says DEFAULT_MAX_RESULTS exists "to protect the RPC server" against unbounded dumps — but self.config.max_results.unwrap_or(DEFAULT_MAX_RESULTS) lets a caller pass {"maxResults": 18446744073709551615} and dump the entire state into one un-paginated BTreeMap in memory, defeating that protection. If the protection is real, the value should be clamped to a hard ceiling rather than only used as a default — see inline. (Mitigating context: debug_* is typically gated behind an access-controlled namespace, so the exposure is limited to operators who deliberately enabled the debug RPC. Still worth a ceiling, since the stated intent is server protection.)
Also, PR should point out that it closes the sub-issue instead of the tracking issue
|
|
||
| let state_root = header.state_root; | ||
| let start = self.config.start.unwrap_or_else(H256::zero); | ||
| let max_results = self.config.max_results.unwrap_or(DEFAULT_MAX_RESULTS); |
There was a problem hiding this comment.
DEFAULT_MAX_RESULTS only acts as a default, not a ceiling — a caller passing {"maxResults": <huge>} overrides it entirely and dumps the full state into one in-memory BTreeMap, which is exactly the unbounded-response DoS the constant's doc says it's protecting against:
Picked to bound response size on large states ... ethrex diverges here to protect the RPC server.
If the protection is meant to hold regardless of caller input, clamp to a hard ceiling:
/// Absolute cap; callers cannot exceed this even via `maxResults`.
const MAX_RESULTS_CEILING: usize = 100_000;
...
let max_results = self
.config
.max_results
.unwrap_or(DEFAULT_MAX_RESULTS)
.min(MAX_RESULTS_CEILING);Then large states are walked via the next cursor across multiple calls instead of one unbounded response. (If unbounded single-shot dumps are intentionally allowed for trusted debug operators, a one-line doc note saying so would resolve the apparent contradiction with the "protect the RPC server" rationale.)
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.
The previous implementation was structurally wrong for two of the three
flat-frame variants:
- `create` actions emitted `input` and a result of `{gasUsed, output}`.
geth's flatCallTracer (and Parity's spec) require `init` in the action
and `{address, code, gasUsed}` in the result so callers can recover the
deployed contract's address and runtime bytecode.
- `suicide` (SELFDESTRUCT) frames emitted the call-shape action and a
bogus `{gasUsed, output}` result. The Parity shape is
`{address, balance, refundAddress}` with no result at all.
Split `FlatCallAction` / `FlatCallResult` into per-variant enums (untagged
so the outer `type` field continues to discriminate), map the LEVM call
tracer's SELFDESTRUCT frame fields onto the suicide shape (`from` →
destructed contract, `to` → refund target, `value` → balance), and emit
`init`/`address`/`code` for create frames. Use `&'static str` for the
static label fields to avoid per-frame allocations.
Unit tests now assert against the serialized JSON projection — the wire
shape clients actually receive — and cover create/create2, selfdestruct,
and failed-frame `result` omission. Integration tests gain a CREATE
end-to-end (deploys a tiny init code and verifies the action/result
shape), a `debug_traceBlockByNumber` test, and a missing-tx failure case
that Codex's review flagged as a gap.
Test setup boilerplate moves out of the misnamed `debug_trace_tests.rs`
into a shared `rpc::helpers` module so #6694 and #6701 can rebase to
re-use it instead of duplicating ~140 lines per feature. The file is
renamed to `debug_flat_call_tracer_tests.rs`.
…ug_dumpBlock - Clamp user-supplied maxResults to DEFAULT_MAX_RESULTS so callers cannot bypass the response-size protection by passing a huge value. - Document the balance hex-vs-decimal format divergence from geth.
2 participants
Summary
debug_dumpBlockRPC endpoint that dumps the entire state at a given blockiter_accountsto iterate the state trie at the block's state rootCloses part of #6572
Closes #6650