Skip to content

Conversation

@fedacking
Copy link
Contributor

@fedacking fedacking commented Dec 23, 2025

Motivation

We want to validate the tree quicker and consume less memory.

Description

  • Made validate state and storage root not recompute the entire tree, instead just traverse the trie to found the missing nodes.

@github-actions github-actions bot added the L1 Ethereum client label Dec 23, 2025
@github-actions
Copy link

github-actions bot commented Dec 23, 2025

Lines of code report

Total lines added: 25
Total lines removed: 10
Total lines changed: 35

Detailed view
+--------------------------------------+-------+------+
| File                                 | Lines | Diff |
+--------------------------------------+-------+------+
| ethrex/crates/common/trie/trie.rs    | 1004  | +25  |
+--------------------------------------+-------+------+
| ethrex/crates/networking/p2p/sync.rs | 1359  | -10  |
+--------------------------------------+-------+------+

@github-actions
Copy link

Benchmark for 2c37b03

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 29.9±1.63ms 30.4±3.30ms +1.67%
Trie/cita-trie insert 1k 2.9±0.01ms 2.9±0.14ms 0.00%
Trie/ethrex-trie insert 10k 25.5±1.02ms 25.2±1.84ms -1.18%
Trie/ethrex-trie insert 1k 2.2±0.01ms 2.2±0.03ms 0.00%

@github-actions
Copy link

Benchmark for 398d407

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 27.4±0.40ms 28.1±0.76ms +2.55%
Trie/cita-trie insert 1k 2.8±0.01ms 2.9±0.10ms +3.57%
Trie/ethrex-trie insert 10k 24.6±0.37ms 25.4±0.83ms +3.25%
Trie/ethrex-trie insert 1k 2.2±0.02ms 2.1±0.05ms -4.55%

@github-actions
Copy link

Benchmark for e28a603

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 28.4±0.99ms 28.9±1.15ms +1.76%
Trie/cita-trie insert 1k 2.8±0.05ms 2.8±0.07ms 0.00%
Trie/ethrex-trie insert 10k 25.4±0.86ms 26.0±0.84ms +2.36%
Trie/ethrex-trie insert 1k 2.2±0.01ms 2.2±0.01ms 0.00%

@fedacking fedacking marked this pull request as ready for review December 23, 2025 21:45
@fedacking fedacking requested a review from a team as a code owner December 23, 2025 21:45
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Dec 23, 2025
@github-actions
Copy link

Benchmark for 60c8be6

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 28.0±0.71ms 28.1±0.65ms +0.36%
Trie/cita-trie insert 1k 2.8±0.01ms 2.9±0.12ms +3.57%
Trie/ethrex-trie insert 10k 24.6±0.51ms 24.8±0.48ms +0.81%
Trie/ethrex-trie insert 1k 2.2±0.05ms 2.2±0.01ms 0.00%

Comment on lines +534 to +549
for (_, node) in self.into_iter() {
expected_count -= 1;
match node {
Node::Branch(branch_node) => {
expected_count += branch_node
.choices
.iter()
.filter(|child| child.is_valid())
.count();
}
Node::Extension(_) => {
expected_count += 1;
}
Node::Leaf(_) => {}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we checking the hashes match at some point? Does the iterator still do that?

Copy link
Contributor Author

@fedacking fedacking Dec 23, 2025

Choose a reason for hiding this comment

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

The iterator uses get_node_checked, which will stop the iteration, which will make expected_count return a value indicating that the trie is not valid.

Copy link
Contributor

@Oppen Oppen left a comment

Choose a reason for hiding this comment

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

LGTM but see comment.

@github-actions
Copy link

github-actions bot commented Jan 5, 2026

Benchmark for 8b9433a

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 34.8±1.53ms 35.1±1.65ms +0.86%
Trie/cita-trie insert 1k 2.8±0.02ms 2.8±0.10ms 0.00%
Trie/ethrex-trie insert 10k 28.6±0.91ms 28.8±0.81ms +0.70%
Trie/ethrex-trie insert 1k 2.2±0.02ms 2.1±0.02ms -4.55%

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.

3 participants