Skip to content

Conversation

@JereSalo
Copy link
Contributor

@JereSalo JereSalo commented Nov 25, 2025

Motivation

Description

  • from_nodes doesn't decode the nodes anymore, this is useful because when called multiple times with same set of nodes we were decoding them more than once unnecessarily.
  • execution_witness_from_rpc_chain_config now gets the initial state root from the rpc_witness itself. This should ALWAYS be in the block headers, otherwise it returns an error.

Related Replay PR: lambdaclass/ethrex-replay#50

@github-actions github-actions bot added the L1 Ethereum client label Nov 25, 2025
@github-actions
Copy link

github-actions bot commented Nov 25, 2025

Lines of code report

Total lines added: 13
Total lines removed: 4
Total lines changed: 17

Detailed view
+---------------------------------------------------------+-------+------+
| File                                                    | Lines | Diff |
+---------------------------------------------------------+-------+------+
| ethrex/crates/common/trie/db.rs                         | 92    | -4   |
+---------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/debug/execution_witness.rs | 220   | +13  |
+---------------------------------------------------------+-------+------+

@github-actions
Copy link

Benchmark for d980dad

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 32.5±1.86ms 32.5±1.68ms 0.00%
Trie/cita-trie insert 1k 2.7±0.02ms 2.8±0.08ms +3.70%
Trie/ethrex-trie insert 10k 28.4±0.77ms 27.5±0.83ms -3.17%
Trie/ethrex-trie insert 1k 2.2±0.02ms 2.2±0.06ms 0.00%

@github-actions
Copy link

Benchmark for 73f4104

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 28.0±0.60ms 28.3±1.36ms +1.07%
Trie/cita-trie insert 1k 2.9±0.01ms 2.9±0.05ms 0.00%
Trie/ethrex-trie insert 10k 24.7±0.41ms 24.5±0.40ms -0.81%
Trie/ethrex-trie insert 1k 2.2±0.01ms 2.2±0.02ms 0.00%

@github-actions
Copy link

Benchmark for 97aa0aa

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 27.7±0.98ms 28.4±1.53ms +2.53%
Trie/cita-trie insert 1k 2.8±0.01ms 2.9±0.09ms +3.57%
Trie/ethrex-trie insert 10k 25.3±1.08ms 25.5±0.77ms +0.79%
Trie/ethrex-trie insert 1k 2.2±0.01ms 2.2±0.05ms 0.00%

@JereSalo JereSalo marked this pull request as ready for review November 26, 2025 22:07
@JereSalo JereSalo requested a review from a team as a code owner November 26, 2025 22:07
Copilot AI review requested due to automatic review settings November 26, 2025 22:07
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Nov 26, 2025
Copy link
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 refactors two public functions used by ethrex_replay to improve performance and reduce parameter coupling. The from_nodes function no longer decodes nodes internally (avoiding redundant decoding when called multiple times), and execution_witness_from_rpc_chain_config now extracts the initial state root directly from the witness headers rather than requiring it as a parameter.

  • Performance improvement: from_nodes now accepts pre-decoded nodes to avoid redundant decoding
  • Simplified API: execution_witness_from_rpc_chain_config extracts initial state root from headers instead of requiring it as a parameter
  • Better encapsulation: Initial state root logic is now self-contained within the witness conversion function

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/networking/rpc/debug/execution_witness.rs Removes initial_state_root parameter and adds logic to extract it from witness headers; adds BlockHeader import
crates/common/trie/db.rs Changes from_nodes signature to accept BTreeMap<H256, Node> instead of BTreeMap<H256, NodeRLP>, removing internal decoding logic and unused imports

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

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copy link
Contributor

@tomip01 tomip01 left a comment

Choose a reason for hiding this comment

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

Looks good

@ilitteri ilitteri requested a review from xqft November 27, 2025 19:43
@ilitteri ilitteri added this pull request to the merge queue Nov 27, 2025
Merged via the queue into main with commit f2b2006 Nov 27, 2025
60 checks passed
@ilitteri ilitteri deleted the replay/fix_replay_nov_25 branch November 27, 2025 21:09
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Nov 27, 2025
github-merge-queue bot pushed a commit to lambdaclass/ethrex-replay that referenced this pull request Nov 28, 2025
**Motivation**

<!-- Why does this pull request exist? What are its goals? -->

**Description**

<!-- A clear and concise general description of the changes this PR
introduces -->
- Use simplified `execution_witness_from_rpc_chain_config` function.
  - The advantage of this is that we get to remove a lot of code 😃 
- Decode all nodes just once. This is a huge improvement in storage
preparation performance for no-zkvm executions. Before when ran in
`debug` mode the storage preparation was taking almost a **whole
minute** because nodes were being decoded over and over again every time
we ran `from_nodes`, now this function in the correspondent ethrex PR
receives the nodes already decoded.
- This started being a problem because of
[this](https://github.com/lambdaclass/ethrex/pull/5224/files#diff-5586f62a6533f683576523ff0bfcb3d336411d569f12062649dbb34cd30c33adR67).
Before we didn't need to decode the nodes in order to generate the
`InMemoryTrieDB`.


Ethrex related PR: lambdaclass/ethrex#5416
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants