Skip to content

refactor(l1): remove keys dependency, pin ethrex deps, and add NativeCrypto#68

Open
avilagaston9 wants to merge 5 commits intomainfrom
fix/remove-keys-dependency
Open

refactor(l1): remove keys dependency, pin ethrex deps, and add NativeCrypto#68
avilagaston9 wants to merge 5 commits intomainfrom
fix/remove-keys-dependency

Conversation

@avilagaston9
Copy link
Copy Markdown

@avilagaston9 avilagaston9 commented Mar 9, 2026

Motivation

The ExecutionWitness struct in ethrex no longer includes a keys field (removed in lambdaclass/ethrex#6356). The --no-zkvm replay path relied on witness.keys to discover account addresses and set up storage tries. This PR adapts ethrex-replay to the new API and pins dependencies to the merge commit.

Description

  1. Remove keys dependency in --no-zkvm path: Instead of iterating witness.keys to get addresses and then looking up each in the state trie, enumerate accounts directly from the state trie's InMemoryTrieDB. After commit(), leaf entries have paths of 65 nibbles (64 for keccak256(address) + 1 leaf flag 0x10) with raw RLP-encoded AccountState as values. The first 64 nibbles are converted back to the 32-byte hashed address needed by the Store.

  2. Add NativeCrypto parameter: The ethrex refactor made the Crypto provider injectable. All call sites that require it (sender, execute_block, prepare_block, get_transactions_with_sender, new_for_l1, new_for_l2) now pass NativeCrypto. Added ethrex-crypto as a direct dependency.

  3. Pin ethrex deps to a specific commit: Dependencies now use rev = "b962fbfb..." (the merge commit of #6356 on main) instead of tracking a branch. This avoids unexpected breakage from upstream changes.

  4. Remove Holesky references: HOLESKY_CHAIN_ID and PublicNetwork::Holesky were removed from ethrex at the pinned commit (replaced by Hoodi).

Instead of iterating witness.keys to get addresses and then looking up each
in the state trie, enumerate accounts directly from the state trie's
InMemoryTrieDB. After commit(), leaf entries have paths of 65 nibbles
(64 for keccak256(address) + 1 leaf flag 0x10) with raw RLP-encoded
AccountState as values. The first 64 nibbles are converted back to the
32-byte hashed address needed by the Store.
is_empty() check on storage_trie_nodes since get_trie_nodes_with_dummies
always injects dummy branch nodes for every existing path.
@avilagaston9 avilagaston9 marked this pull request as ready for review March 9, 2026 20:57
Copilot AI review requested due to automatic review settings March 9, 2026 20:57
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the L1 --no-zkvm replay state preparation to stop depending on the soon-to-be-removed ExecutionWitness.keys field by deriving the set of accounts from the state trie instead.

Changes:

  • Removes usage of witness.keys (and the ethrex_storage::hash_address import) in prepare_no_zkvm_state.
  • Attempts to enumerate state-trie leaf entries from InMemoryTrieDB and derive the hashed-address from the leaf path to write storage trie nodes.

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

Comment on lines +1056 to 1059
let Ok(account_state) = AccountState::decode(account_state_rlp) else {
continue;
};

Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

account_state_rlp here is coming directly from InMemoryTrieDB::inner() which (as used in get_trie_nodes_with_dummies) stores RLP-encoded trie nodes at each path, not raw RLP-encoded AccountState. Decoding it as AccountState will fail (and you continue), so no storage tries get written and add_block_pipeline will likely read missing storage.

Instead, decode the value as a trie Node first, select only Leaf nodes, extract the leaf’s embedded value bytes, and then RLP-decode those bytes into AccountState before proceeding.

Suggested change
let Ok(account_state) = AccountState::decode(account_state_rlp) else {
continue;
};
// `account_state_rlp` is actually an RLP-encoded trie `Node`, not a raw `AccountState`.
let Ok(node) = Node::decode(account_state_rlp) else {
continue;
};
// Only leaf nodes contain account state; extract the embedded value bytes.
let leaf_value = match &node {
Node::Leaf(_, value) => value,
_ => continue,
};
let Ok(account_state) = AccountState::decode(leaf_value) else {
continue;
};

Copilot uses AI. Check for mistakes.
…ness branch

and add NativeCrypto parameter to all call sites that now require it (sender,
execute_block, prepare_block, get_transactions_with_sender, new_for_l1, new_for_l2).
Also add ethrex-crypto as a direct dependency.
github-merge-queue bot pushed a commit to lambdaclass/ethrex that referenced this pull request Mar 16, 2026
## 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):
   ```bash
   make -C crates/l2 init-l1
   ```

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

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](lambdaclass/ethrex-replay#68)):
   ```bash
   cd ~/Documents/ethrex-replay
   git checkout fix/remove-keys-dependency
   cargo build --release
   ```

4. **Run replay — no-zkvm path** (uses `Blockchain::add_block`
directly):
   ```bash
   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):
   ```bash
   RUST_LOG=info ./target/release/ethrex-replay block <BLOCK_NUMBER> \
     --rpc-url http://localhost:8545
   ```

Both should exit 0 with a successful replay.
Muzry pushed a commit to Muzry/ethrex that referenced this pull request Mar 17, 2026
…ss#6356)

## 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 lambdaclass#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):
   ```bash
   make -C crates/l2 init-l1
   ```

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

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
lambdaclass#68](lambdaclass/ethrex-replay#68)):
   ```bash
   cd ~/Documents/ethrex-replay
   git checkout fix/remove-keys-dependency
   cargo build --release
   ```

4. **Run replay — no-zkvm path** (uses `Blockchain::add_block`
directly):
   ```bash
   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):
   ```bash
   RUST_LOG=info ./target/release/ethrex-replay block <BLOCK_NUMBER> \
     --rpc-url http://localhost:8545
   ```

Both should exit 0 with a successful replay.
on main instead of tracking the now-merged branch. Also remove
HOLESKY_CHAIN_ID and PublicNetwork::Holesky references which were
removed from ethrex at that commit.
@avilagaston9 avilagaston9 changed the title refactor(l1): remove dependency on ExecutionWitness keys field in --no-zkvm path refactor(l1): remove keys dependency, pin ethrex deps, and add NativeCrypto Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants