Skip to content

Conversation

@cdiielsi
Copy link
Contributor

@cdiielsi cdiielsi commented Oct 9, 2025

Motivation

Adding some metadata to TrieError::InconsistentTree to have more information when we don't find a node in the database.

Description

This pr introduces a new enum to make the TrieError::InconsistentTree more specific:

#[derive(Debug, Error)]
pub enum InconsistentTreeError {
    #[error("Child node of {0}, differs from expected")]
    ExtensionNodeChildDiffers(ExtensionNodeErrorData),
    #[error("No Child Node found of {0}")]
    ExtensionNodeChildNotFound(ExtensionNodeErrorData),
    #[error("Node with hash {0:#x} not found in Branch Node with hash {1:#x} using path {2:?}")]
    NodeNotFoundOnBranchNode(H256, H256, Nibbles),
    #[error("Root node with hash {0:#x} not found")]
    RootNotFound(H256),
}

#[derive(Debug)]
pub struct ExtensionNodeErrorData {
    pub node_hash: H256,
    pub extension_node_hash: H256,
    pub extension_node_prefix: Nibbles,
    pub node_path: Nibbles,
}

Then I got InconsistentTreeError boxed because for some error variants it surpasses the 168 bytes limit allowed by clippy on results.

Closes #4786

@github-actions github-actions bot added the L1 Ethereum client label Oct 9, 2025
@github-actions
Copy link

github-actions bot commented Oct 9, 2025

Lines of code report

Total lines added: 150
Total lines removed: 0
Total lines changed: 150

Detailed view
+---------------------------------------------+-------+------+
| File                                        | Lines | Diff |
+---------------------------------------------+-------+------+
| ethrex/crates/common/trie/error.rs          | 46    | +29  |
+---------------------------------------------+-------+------+
| ethrex/crates/common/trie/node/branch.rs    | 574   | +45  |
+---------------------------------------------+-------+------+
| ethrex/crates/common/trie/node/extension.rs | 482   | +51  |
+---------------------------------------------+-------+------+
| ethrex/crates/common/trie/trie.rs           | 954   | +25  |
+---------------------------------------------+-------+------+

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

Benchmark for e4c2504

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 34.9±0.36ms 34.4±0.33ms -1.43%
Trie/cita-trie insert 1k 3.6±0.03ms 3.5±0.02ms -2.78%
Trie/ethrex-trie insert 10k 55.4±1.04ms 733.2±9.16ms +1223.47%
Trie/ethrex-trie insert 1k 6.1±0.03ms 62.0±1.51ms +916.39%

@github-actions
Copy link

Benchmark for cbc8b39

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 36.1±1.26ms 35.0±0.62ms -3.05%
Trie/cita-trie insert 1k 3.6±0.02ms 3.5±0.02ms -2.78%
Trie/ethrex-trie insert 10k 55.0±0.41ms 738.5±12.91ms +1242.73%
Trie/ethrex-trie insert 1k 6.1±0.01ms 62.2±0.57ms +919.67%

@cdiielsi cdiielsi marked this pull request as ready for review October 13, 2025 14:17
@cdiielsi cdiielsi requested a review from a team as a code owner October 13, 2025 14:17
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Oct 13, 2025
Copy link
Contributor

@JereSalo JereSalo left a comment

Choose a reason for hiding this comment

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

I believe ok_or() eagerly builds the error, so even when the trie node is found both format! and compute_hash() will run, this means we'll be hashing every time.
I suggest replacing ok_or for ok_or_else( || { TrieError::... }) as this one takes a closure that is only executed if the Option is None.

@github-project-automation github-project-automation bot moved this from In Review to In Progress in ethrex_l1 Oct 13, 2025
@github-actions
Copy link

Benchmark for 786dd32

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 36.9±2.47ms 36.8±2.74ms -0.27%
Trie/cita-trie insert 1k 3.6±0.03ms 3.5±0.06ms -2.78%
Trie/ethrex-trie insert 10k 55.6±1.09ms 55.5±0.61ms -0.18%
Trie/ethrex-trie insert 1k 6.1±0.11ms 6.1±0.03ms 0.00%

@github-actions
Copy link

Benchmark for 4765370

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 36.5±1.00ms 36.6±1.17ms +0.27%
Trie/cita-trie insert 1k 3.6±0.02ms 3.5±0.05ms -2.78%
Trie/ethrex-trie insert 10k 56.3±1.24ms 56.0±1.56ms -0.53%
Trie/ethrex-trie insert 1k 6.2±0.13ms 6.2±0.02ms 0.00%

@github-actions
Copy link

Benchmark for 3ea5de1

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 35.7±1.16ms 35.2±0.86ms -1.40%
Trie/cita-trie insert 1k 3.6±0.02ms 3.6±0.21ms 0.00%
Trie/ethrex-trie insert 10k 62.5±0.59ms 63.2±0.83ms +1.12%
Trie/ethrex-trie insert 1k 8.1±0.02ms 8.0±0.10ms -1.23%

@github-actions
Copy link

Benchmark for 6e31a3d

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 36.9±2.43ms 36.5±1.71ms -1.08%
Trie/cita-trie insert 1k 3.5±0.09ms 3.6±0.16ms +2.86%
Trie/ethrex-trie insert 10k 62.7±1.51ms 64.4±2.31ms +2.71%
Trie/ethrex-trie insert 1k 7.9±0.06ms 8.1±0.04ms +2.53%

@github-actions
Copy link

Benchmark for 636eb5c

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 42.3±2.90ms 38.4±2.60ms -9.22%
Trie/cita-trie insert 1k 3.6±0.01ms 3.6±0.03ms 0.00%
Trie/ethrex-trie insert 10k 64.1±1.19ms 63.7±1.38ms -0.62%
Trie/ethrex-trie insert 1k 8.0±0.04ms 8.3±0.37ms +3.75%

@github-actions
Copy link

Benchmark for 1291411

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 35.0±1.58ms 33.7±1.43ms -3.71%
Trie/cita-trie insert 1k 2.9±0.01ms 2.8±0.01ms -3.45%
Trie/ethrex-trie insert 10k 66.1±1.56ms 66.0±1.69ms -0.15%
Trie/ethrex-trie insert 1k 7.4±0.02ms 7.2±0.02ms -2.70%

@github-project-automation github-project-automation bot moved this from In Review to In Progress in ethrex_l1 Oct 17, 2025
@github-actions
Copy link

Benchmark for 37f9166

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 37.9±2.37ms 36.7±2.03ms -3.17%
Trie/cita-trie insert 1k 3.5±0.01ms 3.5±0.02ms 0.00%
Trie/ethrex-trie insert 10k 63.7±1.79ms 255.9±2.86ms +301.73%
Trie/ethrex-trie insert 1k 8.0±0.05ms 21.9±0.79ms +173.75%

@github-actions
Copy link

Benchmark for 1360e78

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 39.1±2.02ms 38.8±1.62ms -0.77%
Trie/cita-trie insert 1k 3.5±0.01ms 3.5±0.03ms 0.00%
Trie/ethrex-trie insert 10k 63.9±1.24ms 258.3±3.04ms +304.23%
Trie/ethrex-trie insert 1k 8.0±0.19ms 21.8±0.56ms +172.50%

@github-actions
Copy link

Benchmark for b594f6a

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 36.1±1.36ms 35.1±0.75ms -2.77%
Trie/cita-trie insert 1k 3.6±0.02ms 3.5±0.12ms -2.78%
Trie/ethrex-trie insert 10k 65.8±1.24ms 67.1±0.91ms +1.98%
Trie/ethrex-trie insert 1k 8.4±0.23ms 8.4±0.03ms 0.00%

@github-actions
Copy link

Benchmark for 7658ea2

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 35.8±1.39ms 34.4±2.45ms -3.91%
Trie/cita-trie insert 1k 3.6±0.01ms 3.6±0.19ms 0.00%
Trie/ethrex-trie insert 10k 66.3±1.16ms 69.6±2.39ms +4.98%
Trie/ethrex-trie insert 1k 8.4±0.09ms 8.6±0.05ms +2.38%

@github-actions
Copy link

Benchmark for 4087ed1

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 34.7±0.28ms 34.4±0.29ms -0.86%
Trie/cita-trie insert 1k 3.5±0.01ms 3.6±0.14ms +2.86%
Trie/ethrex-trie insert 10k 64.4±0.22ms 65.5±0.23ms +1.71%
Trie/ethrex-trie insert 1k 8.2±0.02ms 8.4±0.02ms +2.44%

@github-actions
Copy link

Benchmark for 842abe6

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 37.1±1.39ms 36.3±1.10ms -2.16%
Trie/cita-trie insert 1k 3.5±0.02ms 3.5±0.01ms 0.00%
Trie/ethrex-trie insert 10k 66.3±0.71ms 67.4±1.14ms +1.66%
Trie/ethrex-trie insert 1k 8.3±0.19ms 8.5±0.37ms +2.41%

@github-project-automation github-project-automation bot moved this from In Progress to In Review in ethrex_l1 Oct 21, 2025
@github-actions
Copy link

Benchmark for 599e1dc

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 35.1±0.70ms 35.2±0.85ms +0.28%
Trie/cita-trie insert 1k 3.6±0.24ms 3.5±0.13ms -2.78%
Trie/ethrex-trie insert 10k 65.0±0.72ms 68.0±1.57ms +4.62%
Trie/ethrex-trie insert 1k 8.3±0.02ms 8.4±0.02ms +1.20%

@github-actions
Copy link

Benchmark for 6d061a3

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 35.0±1.16ms 37.6±2.96ms +7.43%
Trie/cita-trie insert 1k 3.5±0.01ms 3.6±0.19ms +2.86%
Trie/ethrex-trie insert 10k 65.1±1.28ms 69.9±2.59ms +7.37%
Trie/ethrex-trie insert 1k 8.3±0.27ms 8.6±0.06ms +3.61%

@github-actions
Copy link

Benchmark for 3be8b5c

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 35.2±0.28ms 35.1±1.22ms -0.28%
Trie/cita-trie insert 1k 3.5±0.01ms 3.6±0.16ms +2.86%
Trie/ethrex-trie insert 10k 64.7±0.64ms 67.1±0.87ms +3.71%
Trie/ethrex-trie insert 1k 8.3±0.03ms 8.5±0.11ms +2.41%

@github-actions
Copy link

Benchmark for 8a71b46

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 35.1±0.15ms 34.3±0.26ms -2.28%
Trie/cita-trie insert 1k 3.6±0.01ms 3.5±0.01ms -2.78%
Trie/ethrex-trie insert 10k 64.4±0.50ms 65.7±0.37ms +2.02%
Trie/ethrex-trie insert 1k 8.3±0.09ms 8.6±0.28ms +3.61%

@cdiielsi cdiielsi added this pull request to the merge queue Oct 22, 2025
Merged via the queue into main with commit c3e30f4 Oct 22, 2025
30 checks passed
@cdiielsi cdiielsi deleted the more-specific-trie-error branch October 22, 2025 14:24
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Oct 22, 2025
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.

Improve TrieError::InconsistentTree error reporting

4 participants