refactor(l1): remove keys dependency from ExecutionWitness

[avilagaston9]

Motivation

The keys field in ExecutionWitness / RpcExecutionWitness is being phased out of the RPC spec. Our execution_witness_from_rpc_chain_config function (used by ethrex-replay) depended on keys to rebuild storage_trie_roots, which meant it would silently produce empty storage tries when keys was missing or empty.

Follows up on #6338.

Description

• Remove keys from ExecutionWitness. Keep it in RpcExecutionWitness with #[serde(default)] for interoperability, but always emit it empty.
• Change storage_trie_roots key from Address to H256 (the keccak256 hash of the address). This aligns with how the state trie stores account paths.
• Replace the keys-based loop in execution_witness_from_rpc_chain_config with a recursive state trie walker that discovers accounts and their storage roots directly from the trie leaves.
• Update GuestProgramState fields (storage_tries, verified_storage_roots, account_hashes_by_address) to use H256 keys consistently.

How to Test

• cargo check --all-targets
• cargo clippy --all-targets -- -D warnings
• Existing execution witness tests should pass unchanged

End-to-end test with ethrex-replay

1. Start an L1 dev node (from the ethrex repo, on this branch):

   make -C crates/l2 init-l1

2. Send a transaction to produce a non-empty block:

   PRIVATE_KEY=0x850643a0224065ecce3882673c21f56bcf6eef86274cc21cadff15930b59fc8c
   rex send 0x0000000000000000000000000000000000000001 \
     --value 1000000000000000000 \
     -k $PRIVATE_KEY \
     --rpc-url http://localhost:8545

   Note the block number from the receipt.

3. Build ethrex-replay (from PR #68):

   cd ~/Documents/ethrex-replay
   git checkout fix/remove-keys-dependency
   cargo build --release

4. Run replay — no-zkvm path (uses Blockchain::add_block directly):

   RUST_LOG=info ./target/release/ethrex-replay block <BLOCK_NUMBER> \
     --rpc-url http://localhost:8545 \
     --no-zkvm

5. Run replay — exec mode (builds ExecutionWitness → GuestProgramState, runs guest program natively):

   RUST_LOG=info ./target/release/ethrex-replay block <BLOCK_NUMBER> \
     --rpc-url http://localhost:8545

Both should exit 0 with a successful replay.

[github-actions]

🤖 Kimi Code Review

Review Summary

This PR removes the keys field from ExecutionWitness and changes storage trie indexing from Address to H256 (hashed addresses). The changes appear to be a cleanup/refactoring effort with some potential issues.

Issues Found

1. Critical Bug in collect_accounts_from_node (line 107-137)

The function assumes all leaf nodes in the state trie represent accounts, but this is incorrect. The state trie uses hashed addresses as keys, and leaf nodes can contain arbitrary data. The function should validate that the leaf actually contains an encoded AccountState before processing it.

Fix: Add proper validation:

Node::Leaf(leaf) => {
    let full_path = path.concat(&leaf.partial);
    let path_bytes = full_path.to_bytes();
    if path_bytes.len() == 32 {
        let hashed_address = H256::from_slice(&path_bytes);
        // Validate this is actually an account before decoding
        if let Ok(account_state) = AccountState::decode(&leaf.value) {
            accounts.push((hashed_address, account_state.storage_root));
        }
    }
}

2. Inconsistent Hashing Usage (multiple locations)

The PR introduces H256 for hashed addresses but still uses .as_bytes().to_vec() in several places:

• Line 1571: hashed_address.as_bytes().to_vec()
• Line 1800: hashed_address.as_bytes().to_vec()

Fix: Since H256 implements AsRef<[u8]>, you can use .as_ref() instead of .as_bytes().to_vec() to avoid unnecessary allocation.

3. Potential Performance Issue in collect_accounts_from_node

The function creates a new Vec for each leaf node's path bytes, which could be expensive for large tries. Consider using a more efficient approach.

4. Missing Error Handling in collect_accounts_from_node

The function silently ignores decode errors for AccountState. While this might be intentional, it could mask important issues.

5. Type Safety Issue

The change from Address to H256 for storage trie keys is good for consistency, but the hash_address function (line 545) could be more type-safe:

fn hash_address(address: &Address) -> H256 {
    H256(keccak_hash(address.to_fixed_bytes()))
}

This is correct, but consider making it a method on Address for better ergonomics.

Positive Changes

1. Removal of redundant keys field: This eliminates duplicate data and reduces witness size
2. Consistent use of hashed addresses: Using H256 throughout instead of mixing Address and Vec<u8> improves type safety
3. Better separation of concerns: The RPC witness construction now walks the trie instead of relying on the keys field

Minor Suggestions

1. Documentation: Update comments to reflect that storage trie roots are now keyed by hashed addresses, not plain addresses
2. Variable naming: Consider renaming hashed_address_h256 to just hashed_address since the type makes the distinction clear

---

Automated review by Kimi (Moonshot AI) · custom prompt

[github-actions]

🤖 Codex Code Review

1. Medium: malformed account leaves are silently ignored instead of rejected
   At execution_witness.rs:127, AccountState::decode(&leaf.value) is wrapped in if let Ok(...), so decode failures are dropped.
   This is a consensus-adjacent witness path; accepting malformed leaves can mask corrupted/malicious RPC witness data and produce incomplete storage_trie_roots without an explicit error.
   Suggested change: make collect_accounts_from_node return Result<_, GuestProgramStateError> and propagate decode failures.

2. Low: invalid leaf-path lengths are also silently skipped
   At execution_witness.rs:125, non-32-byte reconstructed keys are ignored. For state trie account leaves, this should be structurally invalid and likely deserves an error, not silent drop.
   Suggested change: treat unexpected path length as malformed witness input and return an error.

No obvious EVM opcode/gas-accounting regressions were introduced in this diff, and the shift to hashed-address keyed storage tries is internally consistent across producer/consumer paths.
Testing gap: I didn’t find coverage for the new trie-walk reconstruction path (collect_accounts_from_node), so adding targeted tests for malformed leaf/value handling would reduce risk.

---

Automated review by OpenAI Codex · custom prompt

[github-actions]

🤖 Claude Code Review

Now I have a thorough understanding of the code. Let me produce the review.

---

Code Review: PR #6356 — Remove keys from ExecutionWitness, rekey storage_trie_roots by H256

Summary

The overall direction is correct and well-motivated: removing the deprecated keys field, using hashed addresses consistently, and walking the embedded state trie to discover accounts rather than relying on a separate key list. Most of the changes are clean. However there is one critical bug that would silently break stateless execution.

---

Critical Bug — collect_accounts_from_node never collects any accounts

File: crates/networking/rpc/debug/execution_witness.rs, line 71

collect_accounts_from_node(state_trie_root, Nibbles::from_bytes(&[]), &mut accounts);

Nibbles::from_bytes(&[]) is the wrong initializer for an empty traversal path. Looking at the implementation:

pub fn from_bytes(bytes: &[u8]) -> Self {
    Self::from_raw(bytes, true)   // is_leaf = true
}
pub fn from_raw(bytes: &[u8], is_leaf: bool) -> Self {
    // ... expands bytes to nibbles ...
    if is_leaf { data.push(16); }  // appends the leaf terminator
    ...
}

Nibbles::from_bytes(&[]) produces data = [16] — a single-element vector containing the leaf terminator 16, not an empty nibble sequence.

Effect on path reconstruction:

As the walker descends through branch nodes (path.append_new(i as u8)) and extensions (path.concat(&ext.prefix)), the rogue leading 16 is carried along. When it reaches a leaf with partial = [n₁, …, n_k, 16] (remaining nibbles + leaf flag):

full_path.data = [16, <accumulated branch/ext nibbles>, n₁, …, n_k, 16]

That's 66 elements total (1 spurious + 64 real + 1 leaf flag). After to_bytes() trims the trailing leaf flag, 65 nibbles remain. chunks(2) yields 32 full pairs + 1 partial = 33 bytes.

The check path_bytes.len() == 32 therefore always fails. accounts is always empty, storage_trie_roots is always empty, and stateless execution via execution_witness_from_rpc_chain_config silently produces a witness with no storage tries.

Fix: Replace with the derived Default impl (which produces data = []):

collect_accounts_from_node(state_trie_root, Nibbles::default(), &mut accounts);

This is an important catch because the PR description lists "Existing execution witness tests should pass unchanged" — but if no test exercises execution_witness_from_rpc_chain_config on a witness with non-empty storage, this bug goes undetected.

---

Minor Issues

Naming: hashed_address_h256 (blockchain.rs, lines ~1567 and ~1796)

The PR description itself notes this should be simplified. Since the H256 return type of hash_address makes the distinction obvious, hashed_address is cleaner:

// current
let hashed_address_h256 = H256::from_slice(&hashed_address);

// suggested
let hashed_address = H256::from_slice(&hashed_address_bytes);
// or collapse into one line using hash_address directly as H256

RpcExecutionWitness.keys always serializes as an empty array

keys: Vec::new(),

keys has #[serde(default)] for deserialization but no #[serde(skip_serializing_if = "Vec::is_empty")]. Every serialized RpcExecutionWitness will include "keys": []. If the spec is phasing this field out, consider #[serde(skip_serializing_if = "Vec::is_empty")] on the field, or leave as-is if matching the exact wire format expected by consumers.

Silent account decode failure in collect_accounts_from_node (line 127)

if let Ok(account_state) = AccountState::decode(&leaf.value) {
    accounts.push(...)
}

Decoding failures are silently swallowed. This is defensively reasonable (a malformed witness shouldn't crash the RPC), but it means a corrupted witness silently produces missing storage tries. A tracing::warn! or a returned error would aid debugging.

Recursive traversal depth

collect_accounts_from_node is recursive. In practice the Ethereum state trie is bounded to 64 nibble levels (32-byte keccak keys), so stack overflow is not a real concern. Worth noting for completeness.

---

Positive Changes

• Removing the keys construction loops in blockchain.rs (two identical blocks) is cleaner and correct. touched_account_storage_slots.keys() is the right source of truth.
• Changing hash_address to return H256 instead of Vec<u8> is a good type-safety improvement. The *self.account_hashes_by_address.entry(...).or_insert_with(...) pattern (dereferencing to get a Copy H256) is idiomatic.
• Keeping RpcExecutionWitness.keys with #[serde(default)] for backward-compatible deserialization is the right approach for a field being phased out of the spec.
• The GuestProgramState field type changes (storage_tries, verified_storage_roots keyed by H256) are consistent with how the state trie actually stores account paths and avoid a double-hash at lookup time.

---

Verdict

Block on the Nibbles::from_bytes(&[]) bug — it makes the entire execution_witness_from_rpc_chain_config path (used by ethrex-replay) silently non-functional for any witness containing accounts with non-empty storage. The one-word fix is Nibbles::default(). Everything else can be addressed as follow-up or is subjective.

---

Automated review by Claude (Anthropic) · custom prompt

[github-actions]

🤖 Kimi Code Review

Review Summary

The PR removes the keys field from ExecutionWitness and changes storage trie indexing from Address to H256 (hashed addresses). These are good changes that reduce redundancy and improve consistency with Ethereum's state trie structure.

Issues Found

1. Potential Bug in collect_accounts_from_node (crates/networking/rpc/debug/execution_witness.rs:106-138)

The function assumes all leaf nodes in the state trie contain account data, but this isn't guaranteed. Missing validation could lead to:

• Invalid RLP decoding when AccountState::decode(&leaf.value) fails on non-account leaf nodes
• Silent skipping of valid accounts if decoding fails for legitimate reasons

Fix: Add proper error handling or validation:

Node::Leaf(leaf) => {
    let full_path = path.concat(&leaf.partial);
    let path_bytes = full_path.to_bytes();
    if path_bytes.len() == 32 {
        let hashed_address = H256::from_slice(&path_bytes);
        // Only attempt decode if value looks like an account
        if !leaf.value.is_empty() {
            match AccountState::decode(&leaf.value) {
                Ok(account_state) => accounts.push((hashed_address, account_state.storage_root)),
                Err(e) => debug!("Skipping non-account leaf at {}: {}", hashed_address, e),
            }
        }
    }
}

2. Inconsistent Hashing in hash_address (crates/common/types/block_execution_witness.rs:534-536)

The function returns H256 but the name suggests it returns bytes. Consider renaming to hash_address_to_h256 or similar for clarity.

3. Redundant Conversion in get_account_storage_trie (crates/common/types/block_execution_witness.rs:497-540)

The function computes hashed_address twice - once inline and once via get_account_state. This could be optimized by passing the hashed address from the caller.

4. Missing Error Handling in execution_witness_from_rpc_chain_config (crates/networking/rpc/debug/execution_witness.rs:64-91)

The function doesn't handle the case where collect_accounts_from_node might find no accounts, which could be valid for empty blocks but might indicate an issue with the witness data.

Positive Observations

1. Good removal of redundant keys field - The flattened keys list was duplicative since addresses can be derived from storage trie keys
2. Proper use of hashed addresses - Aligns with Ethereum's state trie design where addresses are keccak256 hashed before use as trie keys
3. Maintains backward compatibility - The RpcExecutionWitness still has a keys field marked with #[serde(default)] to handle older RPC responses
4. Clean separation of concerns - The trie walking logic is properly extracted into a separate function

The changes appear safe and improve the codebase's alignment with Ethereum specifications.

---

Automated review by Kimi (Moonshot AI) · custom prompt

[github-actions]

Lines of code report

Total lines added: 59
Total lines removed: 22
Total lines changed: 81

Detailed view

+---------------------------------------------------------+-------+------+
| File                                                    | Lines | Diff |
+---------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/blockchain.rs                  | 2189  | -22  |
+---------------------------------------------------------+-------+------+
| ethrex/crates/common/types/block_execution_witness.rs   | 414   | +8   |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/debug/execution_witness.rs | 235   | +51  |
+---------------------------------------------------------+-------+------+

[github-actions]

🤖 Codex Code Review

1. High - incorrect trie path seed breaks account discovery from RPC witness
   In crates/networking/rpc/debug/execution_witness.rs:71, collect_accounts_from_node is started with Nibbles::from_bytes(&[]).
   from_bytes appends a leaf terminator nibble (16) by design (see crates/common/trie/nibbles.rs:455). For trie traversal this is wrong; the seed path should be empty/non-leaf.
   Impact: discovered account paths are malformed, so path_bytes.len() == 32 in the leaf case can fail systematically, resulting in missing storage_trie_roots and downstream storage-trie validation failures during replay.
   Suggested fix: initialize traversal with Nibbles::default() or Nibbles::from_raw(&[], false).

2. Medium - malformed account leaf values are silently ignored
   In crates/networking/rpc/debug/execution_witness.rs:127, AccountState::decode(&leaf.value) failures are dropped (if let Ok(...)).
   For witness reconstruction, this should likely be a hard error, otherwise corrupted witness nodes can be silently skipped and produce partial storage_trie_roots, making failures harder to diagnose and potentially masking invalid inputs.
   Suggested fix: make collect_accounts_from_node return Result<..., GuestProgramStateError> and propagate decode errors.

Overall, the refactor direction (moving away from RPC keys and keying storage tries by hashed address) looks good and aligns with trie semantics.
I couldn’t run cargo check in this environment due rustup temp-file permission errors (/home/runner/.rustup/tmp/...).

---

Automated review by OpenAI Codex · custom prompt

[github-actions]

🤖 Claude Code Review

Here is the review:

---

Code Review: PR #6356 — refactor(l1): remove keys dependency from ExecutionWitness

Summary

The motivation is sound and the key type change from Address to H256 for storage_trie_roots is the correct alignment with how the state trie stores paths. However, there is a critical correctness bug in the new trie-walking code that will silently produce empty results, reintroducing the exact failure mode the PR is trying to fix.

---

Critical Bug — collect_accounts_from_node always produces empty results

File: crates/networking/rpc/debug/execution_witness.rs, call site around line 67:

collect_accounts_from_node(state_trie_root, Nibbles::from_bytes(&[]), &mut accounts);

Nibbles::from_bytes always appends the leaf flag (16) to the nibble data, even for empty input. Looking at the implementation in crates/common/trie/nibbles.rs:

pub fn from_bytes(bytes: &[u8]) -> Self {
    Self::from_raw(bytes, true)   // is_leaf = true → pushes 16
}

So Nibbles::from_bytes(&[]) produces Nibbles { data: [16] }, not an empty path.

This spurious 16 is prepended to every reconstructed path during traversal. At the leaf node:

path.concat(&leaf.partial)

produces [16, ...branch/ext nibbles..., ...leaf nibbles..., 16] — 66 elements total for a 32-byte key. to_bytes() trims only the last 16, yielding 65 nibbles → 33 bytes.

The guard:

if path_bytes.len() == 32 {

therefore never passes, accounts is always empty, storage_trie_roots is always empty, and execution_witness_from_rpc_chain_config silently returns a witness with no storage tries — exactly the silent failure mode the PR was written to prevent.

Fix: Use Nibbles::from_hex(vec![]) or Nibbles::from_raw(&[], false) (both create an empty, non-leaf Nibbles) for the initial path:

collect_accounts_from_node(state_trie_root, Nibbles::from_hex(vec![]), &mut accounts);

---

Minor Issues

Silent skip for hash-only node references

In collect_accounts_from_node, only NodeRef::Node variants are followed:

if let NodeRef::Node(child_node, _) = child {

NodeRef::Hash children are silently skipped (no error, no warning). For the embedded execution witness, all trie nodes should be in memory, so this is likely safe in practice. A comment explaining this assumption would help future readers, and consider adding a debug_assert! or log warning if a NodeRef::Hash is encountered mid-traversal.

Extension node field naming

In the new walker:

Node::Extension(ext) => {
    if let NodeRef::Node(child_node, _) = &ext.child {
        collect_accounts_from_node(child_node, path.concat(&ext.prefix), accounts);
    }
}

ext.prefix is correct per the ExtensionNode struct. No issue here, just confirming it is prefix not partial.

Duplicate build_execution_witness logic in blockchain.rs

The PR correctly updates both copies of the storage-trie-roots loop (lines ~1572–1591 and ~1801–1820), but the near-identical duplication remains a maintenance hazard. This is a pre-existing issue and out of scope for this PR, but worth tracking.

---

Positive Changes

• The key type change from Address to H256 in storage_trie_roots, storage_tries, and verified_storage_roots is correct. The state trie uses keccak256(address) as the path key, so keying by H256 is semantically accurate.
• hash_address returning H256 instead of Vec<u8> eliminates repeated allocation and makes the type safer.
• The *self.account_hashes_by_address.entry(...).or_insert_with(...) dereference (exploiting Copy on H256) is idiomatic and clean.
• Adding #[serde(default)] to RpcExecutionWitness::keys preserves backward compatibility with nodes that still emit keys while allowing nodes that omit it.
• The workflow trigger additions in .github/workflows/pr-main_l2.yaml are appropriate.

---

Verdict

The PR must not be merged as-is. The Nibbles::from_bytes(&[]) bug makes execution_witness_from_rpc_chain_config produce empty storage tries for all inputs, which will break ethrex-replay. Fixing that one call site unblocks the rest of the PR, which is otherwise well-structured.

---

Automated review by Claude (Anthropic) · custom prompt

[greptile-apps]

Greptile Summary

This PR removes the keys field from ExecutionWitness/GuestProgramState (keeping it in RpcExecutionWitness with #[serde(default)] for backward compat), switches storage_trie_roots and related maps to H256-keyed (keccak256 of address), and replaces the keys-based storage-trie-root discovery with a recursive embedded-trie walker (collect_accounts_from_node). The internal refactoring in block_execution_witness.rs and blockchain.rs is correct and clean, but the new trie walker in execution_witness.rs contains a critical bug.

Key changes:

• ExecutionWitness.storage_trie_roots key type changed from Address → H256 (keccak256 hash), aligning with how the state trie stores paths.
• GuestProgramState.storage_tries, verified_storage_roots, account_hashes_by_address updated for consistency.
• hash_address return type simplified from Vec<u8> to H256, eliminating redundant allocations.
• collect_accounts_from_node (new function in execution_witness.rs) walks embedded trie nodes to discover (hashed_address, storage_root) pairs. However, it is called with Nibbles::from_bytes(&[]) as the initial path, which appends a leaf-terminator nibble 16 (see Nibbles::from_raw with is_leaf=true). This spurious nibble makes every accumulated leaf path 33 bytes long after to_bytes() instead of 32, so the if path_bytes.len() == 32 guard never fires. No accounts are ever collected and storage_trie_roots is always empty, silently reproducing the same regression the PR was designed to fix. The fix is to use Nibbles::from_hex(vec![]).

Confidence Score: 1/5

• Not safe to merge — the trie walker's wrong initial Nibbles silently produces empty storage_trie_roots for every execution witness reconstructed via execution_witness_from_rpc_chain_config.
• One-line fix required: the initial Nibbles::from_bytes(&[]) in the trie walker inserts a leaf-flag nibble that makes every path check fail, causing the main feature of the PR (discovering storage roots without relying on keys) to produce no output at all.
• crates/networking/rpc/debug/execution_witness.rs — line 71, the Nibbles::from_bytes(&[]) initial argument to collect_accounts_from_node.

Important Files Changed

Filename	Overview
crates/networking/rpc/debug/execution_witness.rs	Replaces keys-based account discovery with a recursive state-trie walker, but the walker is initialized with Nibbles::from_bytes(&[]) which inserts a spurious leaf-terminator nibble causing all path length checks to fail — no storage trie roots are ever discovered.
crates/common/types/block_execution_witness.rs	Correctly migrates storage_tries, verified_storage_roots, and account_hashes_by_address from Address/Vec keys to H256; copy-semantics fix for hashed_address avoids borrow-checker conflicts; hash_address return type simplified to H256.
crates/blockchain/blockchain.rs	Removes the redundant keys Vec construction and replaces Address-keyed storage_trie_roots with H256 (hashed address) keys; logic is symmetric across both witness-generation functions.
.github/workflows/pr-main_l2.yaml	Adds the two changed source files to the L2 CI trigger list so the workflow runs on relevant file changes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["execution_witness_from_rpc_chain_config(rpc_witness, ...)"] --> B["Build nodes: BTreeMap<H256, Node>"]
    B --> C["get_embedded_root → state_trie_root: Option<Node>"]
    C --> D{"state_trie_root\npresent?"}
    D -- "Yes" --> E["collect_accounts_from_node(root, Nibbles::from_bytes(&[]), accounts)\n⚠️ initial path has stray leaf flag 16"]
    D -- "No" --> H["storage_trie_roots = empty"]
    E --> F["Walk Branch/Extension/Leaf nodes recursively"]
    F --> G{"leaf path\nto_bytes().len()\n== 32?"}
    G -- "NEVER TRUE\n(always 33 due to bug)" --> F
    G -- "Should be true" --> I["Push (hashed_address, storage_root)"]
    I --> J["Lookup storage root in nodes map"]
    J --> K["Insert into storage_trie_roots"]
    K --> L["Return ExecutionWitness { storage_trie_roots }"]
    H --> L
    style E fill:#f88,stroke:#c00
    style G fill:#f88,stroke:#c00

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: crates/networking/rpc/debug/execution_witness.rs
Line: 71

Comment:
**Wrong initial `Nibbles` — trie walker never finds any account**

`Nibbles::from_bytes(&[])` calls `from_raw(&[], is_leaf=true)`, which appends a **leaf-terminator nibble `16`** to the data (see `nibbles.rs`). This means the initial path is `data = [16]` instead of `data = []`.

As the trie walk accumulates branch/extension nibbles via `append_new` and `concat`, that stray `16` ends up in the middle of the path. When a leaf is reached, `path.concat(&leaf.partial)` yields **65 nibbles** (1 spurious + 64 actual + 1 leaf flag), and after `to_bytes()` trims the trailing leaf flag and packs: ⌈65/2⌉ = **33 bytes**. The guard `if path_bytes.len() == 32` therefore **never** passes, no accounts are ever pushed, and `storage_trie_roots` is always empty.

This defeats the entire purpose of replacing the `keys`-based lookup and silently reproduces the same empty-storage-trie regression the PR was designed to fix.

The correct initial value is an empty nibble sequence with no leaf flag:

```suggestion
        collect_accounts_from_node(state_trie_root, Nibbles::from_hex(vec![]), &mut accounts);
```

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

_{Last reviewed commit: adf0711}

[greptile-apps]

Wrong initial Nibbles — trie walker never finds any account

Nibbles::from_bytes(&[]) calls from_raw(&[], is_leaf=true), which appends a leaf-terminator nibble 16 to the data (see nibbles.rs). This means the initial path is data = [16] instead of data = [].

As the trie walk accumulates branch/extension nibbles via append_new and concat, that stray 16 ends up in the middle of the path. When a leaf is reached, path.concat(&leaf.partial) yields 65 nibbles (1 spurious + 64 actual + 1 leaf flag), and after to_bytes() trims the trailing leaf flag and packs: ⌈65/2⌉ = 33 bytes. The guard if path_bytes.len() == 32 therefore never passes, no accounts are ever pushed, and storage_trie_roots is always empty.

This defeats the entire purpose of replacing the keys-based lookup and silently reproduces the same empty-storage-trie regression the PR was designed to fix.

The correct initial value is an empty nibble sequence with no leaf flag:

Suggested change

	collect_accounts_from_node(state_trie_root, Nibbles::from_bytes(&[]), &mut accounts);
	collect_accounts_from_node(state_trie_root, Nibbles::from_hex(vec![]), &mut accounts);

Prompt To Fix With AI

This is a comment left during a code review.
Path: crates/networking/rpc/debug/execution_witness.rs
Line: 71

Comment:
**Wrong initial `Nibbles` — trie walker never finds any account**

`Nibbles::from_bytes(&[])` calls `from_raw(&[], is_leaf=true)`, which appends a **leaf-terminator nibble `16`** to the data (see `nibbles.rs`). This means the initial path is `data = [16]` instead of `data = []`.

As the trie walk accumulates branch/extension nibbles via `append_new` and `concat`, that stray `16` ends up in the middle of the path. When a leaf is reached, `path.concat(&leaf.partial)` yields **65 nibbles** (1 spurious + 64 actual + 1 leaf flag), and after `to_bytes()` trims the trailing leaf flag and packs: ⌈65/2⌉ = **33 bytes**. The guard `if path_bytes.len() == 32` therefore **never** passes, no accounts are ever pushed, and `storage_trie_roots` is always empty.

This defeats the entire purpose of replacing the `keys`-based lookup and silently reproduces the same empty-storage-trie regression the PR was designed to fix.

The correct initial value is an empty nibble sequence with no leaf flag:

```suggestion
        collect_accounts_from_node(state_trie_root, Nibbles::from_hex(vec![]), &mut accounts);
```

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

[Copilot]

Pull request overview

Refactors L1 execution witness handling to remove the deprecated keys dependency by deriving required storage trie roots directly from the embedded state trie, and aligns witness/state structures to key storage tries by keccak256(address) (H256).

Changes:

• Remove keys from ExecutionWitness and ensure RpcExecutionWitness.keys remains optional/defaulted but is always emitted empty.
• Change storage_trie_roots / related guest state maps to be keyed by hashed address (H256) instead of Address.
• Rebuild storage_trie_roots in execution_witness_from_rpc_chain_config by walking the embedded state trie leaves to discover (hashed_address, storage_root) pairs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
crates/networking/rpc/debug/execution_witness.rs	Replaces keys-based reconstruction with a state-trie walker to find account storage roots.
crates/common/types/block_execution_witness.rs	Updates witness/guest-program state types to use H256 hashed-address keys; removes ExecutionWitness.keys; keeps RPC keys defaulted and always empty on output.
crates/blockchain/blockchain.rs	Stops building/emitting keys; stores storage_trie_roots keyed by hashed address (H256).
.github/workflows/pr-main_l2.yaml	Ensures L2 CI runs when execution-witness-related files change.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ElFantasma]

nit: when path_bytes.len() != 32 the leaf is silently skipped. For consistency with the decode-failure log below (line 159), consider adding a debug! in the else branch — it would help diagnose corrupted or unexpected witness data:

} else {
    debug!("Unexpected state trie leaf path length: {} (expected 32)", path_bytes.len());
}

[avilagaston9]

Addressed in 58d60a1.