Skip to content

feat(l1): implement prestateTracer for debug_traceTransaction and debug_traceBlockByNumber#6501

Open
avilagaston9 wants to merge 5 commits intofeat/l1/ws-subscriptionsfrom
feat/l1/prestate-tracer
Open

feat(l1): implement prestateTracer for debug_traceTransaction and debug_traceBlockByNumber#6501
avilagaston9 wants to merge 5 commits intofeat/l1/ws-subscriptionsfrom
feat/l1/prestate-tracer

Conversation

@avilagaston9
Copy link
Copy Markdown
Contributor

@avilagaston9 avilagaston9 commented Apr 17, 2026

Depends on: #6496 (eth_subscribe/eth_unsubscribe WS support) — must be merged first.

Motivation

The prestateTracer is a standard Ethereum debug tracer that captures pre-execution account state (balance, nonce, code, storage) and optionally computes a post-execution diff. It's needed by external tools like the Credible Layer sidecar, which uses debug_traceBlockByNumber with prestateTracer in diff mode to build its local state database.

Description

Implement the prestateTracer (with optional diffMode) across the tracing stack:

  • Prestate types in ethrex-common with serde formatting to match Geth's output format.
  • trace_tx_prestate in ethrex-vm snapshots the GeneralizedDatabase cache before executing a transaction, then compares against the post-execution cache to identify touched accounts. For already-cached accounts whose new storage slots were loaded during the tx, it merges values from initial_accounts_state to capture original slot values.
  • trace_transaction_prestate / trace_block_prestate in ethrex-blockchain rebuild parent state and orchestrate per-tx tracing with timeouts.
  • PrestateTracer variant wired into the RPC debug_traceTransaction and debug_traceBlockByNumber handlers with PrestateTracerConfig { diffMode }.

Also extracts the duplicated TestDatabase into a shared test/tests/levm/test_db.rs module reused by both l2_hook_tests and the new prestate_tracer_tests.

How to Test

# Trace a transaction with prestateTracer
curl -X POST http://localhost:8545 -H "Content-Type: application/json" \
  -d '{"jsonrpc":"2.0","method":"debug_traceTransaction","params":["<TX_HASH>",{"tracer":"prestateTracer"}],"id":1}'

# Trace with diff mode
curl -X POST http://localhost:8545 -H "Content-Type: application/json" \
  -d '{"jsonrpc":"2.0","method":"debug_traceTransaction","params":["<TX_HASH>",{"tracer":"prestateTracer","tracerConfig":{"diffMode":true}}],"id":1}'

# Trace a block
curl -X POST http://localhost:8545 -H "Content-Type: application/json" \
  -d '{"jsonrpc":"2.0","method":"debug_traceBlockByNumber","params":["latest",{"tracer":"prestateTracer","tracerConfig":{"diffMode":true}}],"id":1}'

…ockByNumber.

This adds the prestateTracer (with optional diff mode) to the existing debug
tracing infrastructure. The tracer captures pre-execution account state
(balance, nonce, code, storage) and optionally computes a post-execution diff.

Key changes:
- PrestateTrace / PrePostState types in ethrex-common with JSON serialization
- PrestateTracerObserver in ethrex-vm that records accessed accounts and storage
- trace_transaction_prestate / trace_block_prestate in ethrex-blockchain
- PrestateTracer variant wired into the RPC debug_trace* handlers
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 17, 2026

Lines of code report

Total lines added: 288
Total lines removed: 0
Total lines changed: 288

Detailed view
+-------------------------------------------+-------+------+
| File                                      | Lines | Diff |
+-------------------------------------------+-------+------+
| ethrex/crates/blockchain/tracing.rs       | 187   | +57  |
+-------------------------------------------+-------+------+
| ethrex/crates/common/tracing.rs           | 76    | +29  |
+-------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/tracing.rs   | 228   | +52  |
+-------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/tracing.rs | 193   | +128 |
+-------------------------------------------+-------+------+
| ethrex/crates/vm/tracing.rs               | 65    | +22  |
+-------------------------------------------+-------+------+

…n, and serde round-trip.

Zero-pad storage values to 32 bytes (format!("0x{:064x}")) to match geth's
prestateTracer output format. Only compute post_map when diff_mode is true to
avoid unnecessary work in the common non-diff path. Add #[serde(default)] to
PrestateAccountState.storage so deserialization handles absent storage field,
fixing a round-trip inconsistency with skip_serializing_if.
… add PrestateResult enum.

When a transaction accesses a new storage slot on an account already cached
from a previous transaction in the same block, build_account_state_map was
using only pre_snapshot storage (which lacked slots first loaded during the
current tx). Now merges original values from initial_accounts_state for
newly-loaded slots so the pre-state output includes every accessed slot.

Replace the (PrestateTrace, Option<PrePostState>) return type with a
PrestateResult enum (Prestate | Diff) across the tracing stack for
a self-documenting API that avoids an unused empty map allocation.

Add regression tests for both non-diff and diff modes that verify
newly-accessed storage slots appear with their original pre-tx values.
…e tests to test crate.

Replace String fields in PrestateAccountState with U256 (balance), Bytes (code),
HashMap<H256, H256> (storage), and Address map keys — matching how the rest of
the codebase represents these values. Serde handles hex formatting at the
serialization boundary instead of manual format!() calls in business logic.

Split build_account_state_map (which used a use_pre: bool flag) into
find_touched_accounts, build_pre_state_map, and build_post_state_map so each
function has a single responsibility. De-duplicate the pre_map computation that
was repeated in both branches of trace_tx_prestate.

Extract the duplicated TestDatabase into a shared test/tests/levm/test_db.rs
module, update l2_hook_tests to import it, and move the prestate tracer
regression tests to test/tests/levm/prestate_tracer_tests.rs. Remove serde
round-trip tests from crates/common/tracing.rs. Drop hex and once_cell
dependencies from ethrex-vm.
…once=0

in serialization, and gate dead code behind l2 feature.

For already-cached accounts, build_pre_state_map and build_post_state_map
now filter storage to only slots actually touched by the current transaction
(newly loaded or value changed), instead of including all cached slots from
previous transactions. Added skip_serializing_if for nonce=0 to match Geth's
output format. Gated read_env_file_by_config behind cfg(feature = "l2") to
fix a dead-code warning. Added tests for the storage filtering fix and for
CREATE accounts appearing correctly in diff mode.
@avilagaston9 avilagaston9 marked this pull request as ready for review April 17, 2026 21:48
@avilagaston9 avilagaston9 requested a review from a team as a code owner April 17, 2026 21:48
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Apr 17, 2026
@github-actions
Copy link
Copy Markdown

🤖 Codex Code Review

  1. High: crates/vm/backends/levm/tracing.rs:139 drops read-only accesses when the account/slot was already cached before the tx. find_touched_accounts() only keeps accounts whose info or storage changed between pre_snapshot and post_cache. That means a later tx in a block that only re-reads a slot loaded by an earlier tx will omit that contract entirely from non-diff prestateTracer output. The result becomes cache-dependent and will diverge from geth-style prestate semantics for touched-but-unchanged accounts. The new tests only cover newly loaded slots and writes, not this repeated read case.

  2. Medium: crates/vm/backends/levm/tracing.rs:167 can serialize touched contracts with code: 0x even when they have non-empty code. build_account_output() reads code only from db.codes and falls back to Bytes::new() on cache miss, but load_account() only loads AccountState, not code (crates/vm/levm/src/db/gen_db.rs:159). Balance-only touches such as a SELFDESTRUCT beneficiary (crates/vm/levm/src/opcode_handlers/system.rs:598, crates/vm/levm/src/db/gen_db.rs:600) never populate db.codes, so the tracer can emit an incorrect empty-code snapshot. This should fetch via db.get_code(...) instead of silently defaulting.

I couldn’t run the targeted tests in this environment: the pinned toolchain wants to write under /home/runner/.rustup and the fallback +stable path still needed network access for crates.io. Static review only.


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

@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review

Now I have a complete picture of the PR. Let me write up the review.


PR #6501prestateTracer for debug_traceTransaction / debug_traceBlockByNumber

Overview

This PR implements the standard Geth prestateTracer across the full stack: common types, LEVM tracing logic, blockchain orchestration, and RPC wiring. It also extracts a shared TestDatabase from l2_hook_tests. The implementation is well-structured and includes targeted regression tests for the specific state-tracking edge cases.


Correctness

Core tracing logic (crates/vm/backends/levm/tracing.rs)

The find_touched_accounts + build_pre_state_map + build_post_state_map pipeline is logically sound. The three-source-of-truth model (pre_snapshot, post_cache, initial_accounts_state) is correctly reasoned about, and the regression test prestate_trace_includes_newly_accessed_storage_slots validates the key edge case.

Silent failure when code hash is absent from db.codes (tracing.rs:372–379):

db.codes
    .get(&account.info.code_hash)
    .map(|c| c.bytecode.clone())
    .unwrap_or_default()

If the code isn't in db.codes (e.g., because the code cache wasn't populated), code silently becomes Bytes::default(). The caller gets an empty code field with no indication of failure. At minimum this warrants a debug assertion or a trace-level log, since a missing code hash for a non-empty-code account is unexpected and would silently corrupt the output.

Unhandled case: account in post_cache but missing from both pre_snapshot and initial_accounts_state (tracing.rs:354–360):

let Some(initial) = db.initial_accounts_state.get(addr) else {
    continue;
};

The comment says accounts first loaded during this tx come from initial_accounts_state, which requires that LEVM always populates initial_accounts_state when lazy-loading from DB. If any code path creates or touches an account without going through the DB load path (e.g., some precompile interactions or coinbase credit), such accounts would be silently dropped from the trace. This should be verified against LEVM's account loading contract, and a comment should assert the invariant being relied on.

find_touched_accounts doesn't detect accounts removed from post_cache — unlikely in the current LEVM (accounts are never evicted from current_accounts_state), but worth a comment noting this assumption.

Reverted transactions: vm.execute()? propagates errors but not EVM-level reverts (REVERT opcode). On revert, state changes roll back but gas fees (balance deductions) do not. The pre-state map will therefore show the coinbase and sender with fee changes even on a reverted tx. This is consistent with Geth behavior, but there is no test validating it.

Self-destruct: No test covers the SELFDESTRUCT opcode. In Geth's prestate tracer, the self-destructed account appears in both pre and post maps (pre with its original state, post with zeroed state). Whether LEVM's current_accounts_state properly reflects a self-destructed account (zeroed balance, incremented beneficiary balance) needs a test.


Serialization / Geth Compatibility

PrestateAccountState.balance (common/tracing.rs:113): balance has no skip_serializing_if, so it is always emitted. This matches Geth (Geth uses omitempty on *big.Int which is never nil). Correct.

balance hex format: U256 from ethereum_types serializes as "0x..." by default, matching Geth's format. Good.

PrestateResult does not implement Serialize: The caller must match and serialize manually in two separate RPC handlers. Adding impl Serialize for PrestateResult with a delegating match would eliminate duplication and make future callers safer, but this is minor.


Performance

CacheDB clone per traced tx (levm/tracing.rs:298):

let pre_snapshot: CacheDB = db.current_accounts_state.clone();

For a large block with many accounts/storage slots, this clone runs for every transaction. It's unavoidable for correctness (we need a snapshot), but worth noting in code comments so reviewers understand why a cheaper approach isn't used.

Unnecessary Arc<Mutex<>> in trace_block_prestate (blockchain/tracing.rs:71–72):

let vm = Arc::new(Mutex::new(vm));
let block = Arc::new(block);

The loop is sequential — each iteration awaits before the next begins — so the Mutex is never contested. The comment explains the motivation is spawn_blocking's 'static requirement; it's worth saying that explicitly: // spawn_blocking requires 'static: Arc<Mutex<>> is used to satisfy the bound even though access is sequential.


Minor Issues

  • Error message capitalisation inconsistency (blockchain/tracing.rs:35,39): "Transaction not Found" vs "Block not Found" — capitalisation after "not" varies and differs from the convention used elsewhere in the file ("Block not found" at rpc line 192).

  • rerun_block call comment (blockchain/tracing.rs:68): // Run system calls but stop before tx 0 — the intent of stopping at Some(0) is clear, but it may be worth noting that trace_tx_prestate will subsequently execute each tx (advancing the state), making this the correct setup point.

  • EMPTY_KECCACK_HASH typo (levm/tracing.rs:268): KECCACK should be KECCAK. This appears to be a pre-existing constant name in the codebase, but if there's any opportunity to fix it this would be the place.

  • build_account_output is a free function but only called from build_pre_state_map and build_post_state_map inside the same file. This is fine but noting for clarity.


Test Coverage

The three unit tests cover the regression scenario well. Missing coverage:

  • ETH transfer (pure balance change with no code execution)
  • Reverted transaction (EVM REVERT) — pre should include sender/coinbase fee changes
  • Self-destruct — zeroed account in post, beneficiary delta
  • Empty block (no transactions) in trace_block_prestate
  • RPC-level integration test comparing serialized JSON output against a known-good Geth trace snapshot

Summary

The implementation is architecturally sound and the regression tests for the newly-accessed-slot edge case are thorough and well-documented. The main actionable items are: (1) add a visible failure or assertion in build_account_output when db.codes misses a non-empty code hash; (2) add a comment asserting the initial_accounts_state population invariant in find_touched_accounts; (3) add tests for reverted txs and self-destruct. The Arc<Mutex<>> comment should mention the spawn_blocking 'static constraint for clarity.


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

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 17, 2026

Greptile Summary

This PR implements the prestateTracer (with optional diffMode) for debug_traceTransaction and debug_traceBlockByNumber, targeting the Credible Layer sidecar's state-reconstruction use case. The orchestration layer (RPC → blockchain → vm) is cleanly structured and mirrors the existing callTracer pattern; the key engineering work is in find_touched_accounts / build_pre_state_map, which correctly handles the tricky case of multi-tx blocks where a newly-accessed storage slot of an already-cached account must be sourced from initial_accounts_state.

There are three Geth-compatibility gaps worth tracking for future work: read-only accessed accounts are excluded from the non-diff prestate, self-destructed accounts are invisible to find_touched_accounts (which only iterates post_cache), and diff-mode post-state can include storage slots that were SLOADed but not SSTOREd (appearing as a spurious change).

Confidence Score: 5/5

Safe to merge — no P0/P1 issues; all findings are Geth-compatibility gaps that don't affect the primary diff-mode use case.

All three findings are P2: divergences from Geth's exact prestateTracer semantics (read-only account exclusion, self-destruct invisibility, SLOAD-only false-positive in post-state). The stated primary consumer (Credible Layer in diff mode) is covered by the existing tests. The orchestration and type infrastructure are correct and mirror established patterns in the codebase.

crates/vm/backends/levm/tracing.rs — find_touched_accounts and build_post_state_map have the Geth-compatibility gaps noted above.

Important Files Changed

Filename Overview
crates/vm/backends/levm/tracing.rs Core prestate tracing logic; correctly handles the newly-loaded-slot merge case (the main regression target), but diverges from Geth: read-only accounts are excluded, self-destructed accounts may be missed, and diff-mode post-state can contain false-positive storage entries for SLOAD-only accounts.
crates/common/tracing.rs Adds PrestateAccountState, PrestateTrace, PrestateResult, and PrePostState types with appropriate serde attributes; nonce/code/storage omit-when-empty logic is correct; balance always serializes (minor Geth format divergence).
crates/blockchain/tracing.rs Adds trace_transaction_prestate and trace_block_prestate; sequencing (rerun_block then trace_tx_prestate) is correct; Arc/Mutex pattern mirrors existing trace_block_calls correctly.
crates/networking/rpc/tracing.rs Wires PrestateTracer variant into debug_traceTransaction and debug_traceBlockByNumber; serde(rename_all = "camelCase") on TracerType correctly maps PrestateTracer to the "prestateTracer" JSON string.
crates/vm/tracing.rs Thin wrapper that delegates trace_tx_prestate to LEVM; straightforward and mirrors the existing trace_tx_calls pattern.
test/tests/levm/prestate_tracer_tests.rs Four targeted tests covering the key regression (newly-accessed storage in subsequent txs), exclusion of prior-tx slots, diff mode, and CREATE-child tracking; good coverage of the main cases.
test/tests/levm/test_db.rs Shared TestDatabase extracted cleanly from l2_hook_tests; no issues.
test/tests/levm/l2_hook_tests.rs Refactored to use shared TestDatabase; unused imports removed, logic unchanged.
test/tests/l2/utils.rs read_env_file_by_config gated behind #[cfg(feature = "l2")] and File/BufRead imports moved inside; reasonable change to prevent unused-import warnings on non-l2 builds.
test/tests/levm/mod.rs Adds test_db and prestate_tracer_tests modules; no issues.

Sequence Diagram

sequenceDiagram
    participant Client
    participant RPC as RPC Handler
    participant BC as Blockchain
    participant EVM as Evm
    participant LEVM as LEVM

    Client->>RPC: debug_traceTransaction { tracer: prestateTracer, diffMode }
    RPC->>BC: trace_transaction_prestate(tx_hash, reexec, timeout, diff_mode)
    BC->>BC: rebuild_parent_state(parent_hash, reexec)
    BC->>EVM: rerun_block(block, Some(tx_index))
    note over EVM: executes txs 0..tx_index-1 to set up state
    BC->>EVM: trace_tx_prestate(block, tx_index, diff_mode)
    EVM->>LEVM: trace_tx_prestate(db, header, tx, diff_mode)
    note over LEVM: snapshot pre_snapshot = db.current_accounts_state
    LEVM->>LEVM: VM::new + execute() updates db in place
    LEVM->>LEVM: find_touched_accounts(pre_snapshot, post_cache, db)
    LEVM->>LEVM: build_pre_state_map(...)
    alt diff_mode
        LEVM->>LEVM: build_post_state_map(...)
        LEVM-->>EVM: PrestateResult::Diff(PrePostState)
    else non-diff
        LEVM-->>EVM: PrestateResult::Prestate(PrestateTrace)
    end
    EVM-->>BC: PrestateResult
    BC-->>RPC: PrestateResult
    RPC-->>Client: serialized trace or diff
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: crates/vm/backends/levm/tracing.rs
Line: 139-163

Comment:
**Non-diff mode omits read-only accessed accounts**

`find_touched_accounts` skips any account where `pre == post` (line 142–144), so accounts that were only READ (not modified) during execution are silently dropped. Geth's `prestateTracer` in non-diff mode captures the pre-state of every account touched by execution — including call targets that returned read-only results (e.g. price oracles, ERC-20 `balanceOf`). Callers comparing output with Geth for non-diff mode will see incomplete results.

The same `continue` makes diff mode exclude read-only accounts too, which is actually the correct Geth behaviour for `post` — but it also means their pre-state is absent, which could be relevant for tools that need context about accessed-but-unmodified accounts.

For the stated primary use case (diff mode for Credible Layer) this doesn't block anything, but it's worth noting the divergence from the Geth spec.

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

---

This is a comment left during a code review.
Path: crates/vm/backends/levm/tracing.rs
Line: 139-163

Comment:
**Self-destructed accounts will be missing from prestate output**

`find_touched_accounts` only iterates over `post_cache` (`current_accounts_state` after execution). If LEVM removes or zeroes-out a self-destructed account from `current_accounts_state`, that account won't be in the iteration set and its pre-state (which existed before the tx) will silently be absent from both diff and non-diff output. Geth's prestateTracer includes self-destructed accounts in `pre` with their original state.

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

---

This is a comment left during a code review.
Path: crates/vm/backends/levm/tracing.rs
Line: 237-261

Comment:
**Diff-mode post-state may include unchanged storage for newly-accessed accounts**

For an account first accessed in this tx (absent from `pre_snapshot`), `build_post_state_map` skips the `pre_cached` retain block (line 249). That means every storage slot loaded during execution ends up in `post`, even slots that were only SLOADed (not SSTOREd). The resulting diff would show `pre: {}` (empty — from `initial_accounts_state` with empty storage) and `post: {slotK: original_value}`, suggesting the storage changed when it was merely read.

In Geth's diff mode, post-state only contains slots whose values actually changed. Consumers of the diff (e.g. the Credible Layer sidecar) would need to handle these false positives.

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

---

This is a comment left during a code review.
Path: crates/common/tracing.rs
Line: 74-86

Comment:
**`balance` always serialized even for zero-balance accounts**

Geth omits `balance` when it is zero (the field simply isn't emitted). The struct currently has no `skip_serializing_if` guard on `balance`, so zero-balance accounts will always include `"balance":"0x0"` in the output. Low impact, but diverges slightly from Geth's wire format. Consider:

```suggestion
    #[serde(skip_serializing_if = "U256::is_zero")]
    pub balance: U256,
```

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

Reviews (1): Last reviewed commit: "Fix prestateTracer to exclude storage sl..." | Re-trigger Greptile

Comment on lines +139 to +163
for (addr, post_account) in post_cache {
let pre_account = match pre_snapshot.get(addr) {
Some(pre) => {
if pre.info == post_account.info && pre.storage == post_account.storage {
continue;
}
pre
}
None => {
// Account was first loaded during this tx.
// Pre-state comes from initial_accounts_state (the pristine DB-loaded value).
let Some(initial) = db.initial_accounts_state.get(addr) else {
continue;
};
if initial.info == post_account.info && initial.storage == post_account.storage {
continue;
}
initial
}
};

touched.push((*addr, pre_account, post_account));
}

touched
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 Non-diff mode omits read-only accessed accounts

find_touched_accounts skips any account where pre == post (line 142–144), so accounts that were only READ (not modified) during execution are silently dropped. Geth's prestateTracer in non-diff mode captures the pre-state of every account touched by execution — including call targets that returned read-only results (e.g. price oracles, ERC-20 balanceOf). Callers comparing output with Geth for non-diff mode will see incomplete results.

The same continue makes diff mode exclude read-only accounts too, which is actually the correct Geth behaviour for post — but it also means their pre-state is absent, which could be relevant for tools that need context about accessed-but-unmodified accounts.

For the stated primary use case (diff mode for Credible Layer) this doesn't block anything, but it's worth noting the divergence from the Geth spec.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/vm/backends/levm/tracing.rs
Line: 139-163

Comment:
**Non-diff mode omits read-only accessed accounts**

`find_touched_accounts` skips any account where `pre == post` (line 142–144), so accounts that were only READ (not modified) during execution are silently dropped. Geth's `prestateTracer` in non-diff mode captures the pre-state of every account touched by execution — including call targets that returned read-only results (e.g. price oracles, ERC-20 `balanceOf`). Callers comparing output with Geth for non-diff mode will see incomplete results.

The same `continue` makes diff mode exclude read-only accounts too, which is actually the correct Geth behaviour for `post` — but it also means their pre-state is absent, which could be relevant for tools that need context about accessed-but-unmodified accounts.

For the stated primary use case (diff mode for Credible Layer) this doesn't block anything, but it's worth noting the divergence from the Geth spec.

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

Comment on lines +139 to +163
for (addr, post_account) in post_cache {
let pre_account = match pre_snapshot.get(addr) {
Some(pre) => {
if pre.info == post_account.info && pre.storage == post_account.storage {
continue;
}
pre
}
None => {
// Account was first loaded during this tx.
// Pre-state comes from initial_accounts_state (the pristine DB-loaded value).
let Some(initial) = db.initial_accounts_state.get(addr) else {
continue;
};
if initial.info == post_account.info && initial.storage == post_account.storage {
continue;
}
initial
}
};

touched.push((*addr, pre_account, post_account));
}

touched
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 Self-destructed accounts will be missing from prestate output

find_touched_accounts only iterates over post_cache (current_accounts_state after execution). If LEVM removes or zeroes-out a self-destructed account from current_accounts_state, that account won't be in the iteration set and its pre-state (which existed before the tx) will silently be absent from both diff and non-diff output. Geth's prestateTracer includes self-destructed accounts in pre with their original state.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/vm/backends/levm/tracing.rs
Line: 139-163

Comment:
**Self-destructed accounts will be missing from prestate output**

`find_touched_accounts` only iterates over `post_cache` (`current_accounts_state` after execution). If LEVM removes or zeroes-out a self-destructed account from `current_accounts_state`, that account won't be in the iteration set and its pre-state (which existed before the tx) will silently be absent from both diff and non-diff output. Geth's prestateTracer includes self-destructed accounts in `pre` with their original state.

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

Comment on lines +237 to +261
/// Build the post-tx state map for all accounts touched by a transaction.
fn build_post_state_map(
pre_snapshot: &CacheDB,
post_cache: &CacheDB,
db: &GeneralizedDatabase,
) -> PrestateTrace {
let mut result = PrestateTrace::new();

for (addr, _, post_account) in find_touched_accounts(pre_snapshot, post_cache, db) {
let mut state = build_account_output(post_account, db);

// For already-cached accounts, filter to only slots touched by this tx.
if let Some(pre_cached) = pre_snapshot.get(&addr) {
state.storage.retain(|k, _| {
if !pre_cached.storage.contains_key(k) {
return true;
}
pre_cached.storage.get(k) != post_account.storage.get(k)
});
}

result.insert(addr, state);
}

result
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 Diff-mode post-state may include unchanged storage for newly-accessed accounts

For an account first accessed in this tx (absent from pre_snapshot), build_post_state_map skips the pre_cached retain block (line 249). That means every storage slot loaded during execution ends up in post, even slots that were only SLOADed (not SSTOREd). The resulting diff would show pre: {} (empty — from initial_accounts_state with empty storage) and post: {slotK: original_value}, suggesting the storage changed when it was merely read.

In Geth's diff mode, post-state only contains slots whose values actually changed. Consumers of the diff (e.g. the Credible Layer sidecar) would need to handle these false positives.

Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/vm/backends/levm/tracing.rs
Line: 237-261

Comment:
**Diff-mode post-state may include unchanged storage for newly-accessed accounts**

For an account first accessed in this tx (absent from `pre_snapshot`), `build_post_state_map` skips the `pre_cached` retain block (line 249). That means every storage slot loaded during execution ends up in `post`, even slots that were only SLOADed (not SSTOREd). The resulting diff would show `pre: {}` (empty — from `initial_accounts_state` with empty storage) and `post: {slotK: original_value}`, suggesting the storage changed when it was merely read.

In Geth's diff mode, post-state only contains slots whose values actually changed. Consumers of the diff (e.g. the Credible Layer sidecar) would need to handle these false positives.

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

Comment thread crates/common/tracing.rs
Comment on lines +74 to +86
pub struct PrestateAccountState {
pub balance: U256,
#[serde(default, skip_serializing_if = "is_zero_nonce")]
pub nonce: u64,
#[serde(
default,
skip_serializing_if = "Bytes::is_empty",
with = "crate::serde_utils::bytes"
)]
pub code: Bytes,
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
pub storage: HashMap<H256, H256>,
}
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 balance always serialized even for zero-balance accounts

Geth omits balance when it is zero (the field simply isn't emitted). The struct currently has no skip_serializing_if guard on balance, so zero-balance accounts will always include "balance":"0x0" in the output. Low impact, but diverges slightly from Geth's wire format. Consider:

Suggested change
pub struct PrestateAccountState {
pub balance: U256,
#[serde(default, skip_serializing_if = "is_zero_nonce")]
pub nonce: u64,
#[serde(
default,
skip_serializing_if = "Bytes::is_empty",
with = "crate::serde_utils::bytes"
)]
pub code: Bytes,
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
pub storage: HashMap<H256, H256>,
}
#[serde(skip_serializing_if = "U256::is_zero")]
pub balance: U256,
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/common/tracing.rs
Line: 74-86

Comment:
**`balance` always serialized even for zero-balance accounts**

Geth omits `balance` when it is zero (the field simply isn't emitted). The struct currently has no `skip_serializing_if` guard on `balance`, so zero-balance accounts will always include `"balance":"0x0"` in the output. Low impact, but diverges slightly from Geth's wire format. Consider:

```suggestion
    #[serde(skip_serializing_if = "U256::is_zero")]
    pub balance: U256,
```

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

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.

1 participant