Skip to content

refactor(l1): make TrieError::InconsistentTree more specific #4836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 22 commits into from
Oct 22, 2025
Merged

refactor(l1): make TrieError::InconsistentTree more specific #4836

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
3f423cf
make TrieError::InconsistentTree point the node's hash
cdiielsi Oct 9, 2025
a0d62b6
Merge branch 'main' into more-specific-trie-error
cdiielsi Oct 13, 2025
335cac6
add more errors
cdiielsi Oct 13, 2025
8f37de6
upgrade error name to be shorter
cdiielsi Oct 13, 2025
91f0139
print NodeRef instead of the hash for all cases
cdiielsi Oct 14, 2025
d1ba664
merge main into branch
cdiielsi Oct 15, 2025
782f3bf
Merge branch 'main' into more-specific-trie-error
cdiielsi Oct 16, 2025
61c17d6
show node hashes for any type of node
cdiielsi Oct 16, 2025
569a4c1
fix for clippy
cdiielsi Oct 16, 2025
c96891c
drop impl Debug for NodeHash
cdiielsi Oct 16, 2025
5f0c9bf
new enum for inconsistent tree error more specific
cdiielsi Oct 17, 2025
3118135
new struct for ExtensionNodeError metadata boxed to not mess with per…
cdiielsi Oct 17, 2025
7d44ad2
new get_root_node method
cdiielsi Oct 17, 2025
2b94661
change errors for insertion on Extension node
cdiielsi Oct 21, 2025
47863ea
Merge branch 'main' into more-specific-trie-error
cdiielsi Oct 21, 2025
ab4f7cd
Box InconsistentTreeError
cdiielsi Oct 21, 2025
e721265
drop Box used in InconsistentTreeError and add comment on why Box
cdiielsi Oct 21, 2025
aa26484
Merge branch 'main' into more-specific-trie-error
cdiielsi Oct 21, 2025
ff58529
Merge branch 'main' into more-specific-trie-error
cdiielsi Oct 21, 2025
dad633a
add comment on why Box
cdiielsi Oct 21, 2025
7d2cc47
Merge branch 'more-specific-trie-error' of github.com:lambdaclass/eth…
cdiielsi Oct 21, 2025
8a1b84a
Merge branch 'main' into more-specific-trie-error
cdiielsi Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions crates/common/trie/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,51 @@
use ethereum_types::H256;
use ethrex_rlp::error::RLPDecodeError;
use thiserror::Error;

use crate::Nibbles;

#[derive(Debug, Error)]
pub enum TrieError {
#[error(transparent)]
RLPDecode(#[from] RLPDecodeError),
#[error("Verification Error: {0}")]
Verify(String),
#[error("Inconsistent internal tree structure")]
InconsistentTree,
#[error("Inconsistent internal tree structure: {0}")]
InconsistentTree(Box<InconsistentTreeError>),
#[error("Lock Error: Panicked when trying to acquire a lock")]
LockError,
#[error("Database error: {0}")]
DbError(anyhow::Error),
#[error("Invalid trie input")]
InvalidInput,
}

#[derive(Debug, Error)]
pub enum InconsistentTreeError {
#[error("Child node of {0}, differs from expected")]
ExtensionNodeChildDiffers(Box<ExtensionNodeErrorData>),
#[error("No Child Node found of {0}")]
ExtensionNodeChildNotFound(Box<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,
}

impl std::fmt::Display for ExtensionNodeErrorData {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
"Node with hash {:#x}, child of the Extension Node (hash {:#x}, prefix {:?}) on path {:?}",
self.node_hash, self.extension_node_hash, self.extension_node_hash, self.node_path
)
}
}
85 changes: 65 additions & 20 deletions crates/common/trie/node/branch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use ethrex_rlp::structs::Encoder;

use crate::{TrieDB, ValueRLP, error::TrieError, nibbles::Nibbles, node_hash::NodeHash};
use crate::{
InconsistentTreeError, TrieDB, ValueRLP, error::TrieError, nibbles::Nibbles,
node_hash::NodeHash,
};

use super::{ExtensionNode, LeafNode, Node, NodeRef, ValueOrHash};

Expand Down Expand Up @@ -44,9 +47,15 @@ impl BranchNode {
// Delegate to children if present
let child_ref = &self.choices[choice];
if child_ref.is_valid() {
let child_node = child_ref
.get_node(db, path.current())?
.ok_or(TrieError::InconsistentTree)?;
let child_node = child_ref.get_node(db, path.current())?.ok_or_else(|| {
TrieError::InconsistentTree(Box::new(
InconsistentTreeError::NodeNotFoundOnBranchNode(
child_ref.compute_hash().finalize(),
self.compute_hash().finalize(),
path.current(),
),
))
})?;
child_node.get(db, path)
} else {
Ok(None)
Expand All @@ -67,34 +76,48 @@ impl BranchNode {
// If path is at the end, insert or replace its own value.
// Otherwise, check the corresponding choice and insert or delegate accordingly.
if let Some(choice) = path.next_choice() {
match (&mut self.choices[choice], value) {
self.choices[choice] = match (&self.choices[choice], value) {
// Create new child (leaf node)
(choice_ref, ValueOrHash::Value(value)) if !choice_ref.is_valid() => {
let new_leaf = LeafNode::new(path, value);
*choice_ref = Node::from(new_leaf).into();
Node::from(new_leaf).into()
}
// Insert into existing child and then update it
(choice_ref, ValueOrHash::Value(value)) => {
let child_node = choice_ref
.get_node(db, path.current())?
.ok_or(TrieError::InconsistentTree)?;

*choice_ref = child_node.insert(db, path, value)?.into();
let child_node = choice_ref.get_node(db, path.current())?.ok_or_else(|| {
TrieError::InconsistentTree(Box::new(
InconsistentTreeError::NodeNotFoundOnBranchNode(
choice_ref.compute_hash().finalize(),
self.compute_hash().finalize(),
path.current(),
),
))
})?;

child_node.insert(db, path, value)?.into()
}
// Insert external node hash if there are no overrides.
(choice_ref, value @ ValueOrHash::Hash(hash)) => {
if !choice_ref.is_valid() {
*choice_ref = hash.into();
hash.into()
} else if path.is_empty() {
return Err(TrieError::Verify(
"attempt to override proof node with external hash".to_string(),
));
} else {
*choice_ref = choice_ref
choice_ref
.get_node(db, path.current())?
.ok_or(TrieError::InconsistentTree)?
.ok_or_else(|| {
TrieError::InconsistentTree(Box::new(
InconsistentTreeError::NodeNotFoundOnBranchNode(
choice_ref.compute_hash().finalize(),
self.compute_hash().finalize(),
path.current(),
),
))
})?
.insert(db, path, value)?
.into();
.into()
}
}
}
Expand Down Expand Up @@ -141,7 +164,15 @@ impl BranchNode {
if self.choices[choice_index].is_valid() {
let child_node = self.choices[choice_index]
.get_node(db, path.current())?
.ok_or(TrieError::InconsistentTree)?;
.ok_or_else(|| {
TrieError::InconsistentTree(Box::new(
InconsistentTreeError::NodeNotFoundOnBranchNode(
self.choices[choice_index].compute_hash().finalize(),
self.compute_hash().finalize(),
path.current(),
),
))
})?;
// Remove value from child node
let (child_node, old_value) = child_node.remove(db, path.clone())?;
if let Some(child_node) = child_node {
Expand Down Expand Up @@ -182,7 +213,15 @@ impl BranchNode {
let (choice_index, child_ref) = children[0];
let child = child_ref
.get_node(db, base_path.current().append_new(choice_index as u8))?
.ok_or(TrieError::InconsistentTree)?;
.ok_or_else(|| {
TrieError::InconsistentTree(Box::new(
InconsistentTreeError::NodeNotFoundOnBranchNode(
child_ref.compute_hash().finalize(),
self.compute_hash().finalize(),
base_path.current(),
),
))
})?;
match child {
// Replace self with an extension node leading to the child
Node::Branch(_) => ExtensionNode::new(
Expand Down Expand Up @@ -249,9 +288,15 @@ impl BranchNode {
// Continue to child
let child_ref = &self.choices[choice];
if child_ref.is_valid() {
let child_node = child_ref
.get_node(db, path.current())?
.ok_or(TrieError::InconsistentTree)?;
let child_node = child_ref.get_node(db, path.current())?.ok_or_else(|| {
TrieError::InconsistentTree(Box::new(
InconsistentTreeError::NodeNotFoundOnBranchNode(
child_ref.compute_hash().finalize(),
self.compute_hash().finalize(),
path.current(),
),
))
})?;
child_node.get_path(db, path, node_path)?;
}
}
Expand Down
95 changes: 77 additions & 18 deletions crates/common/trie/node/extension.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ use ethrex_rlp::structs::Encoder;
use crate::ValueRLP;
use crate::nibbles::Nibbles;
use crate::node_hash::NodeHash;
use crate::{TrieDB, error::TrieError};
use crate::{
TrieDB,
error::{ExtensionNodeErrorData, InconsistentTreeError, TrieError},
};

use super::{BranchNode, Node, NodeRef, ValueOrHash};

Expand All @@ -26,10 +29,18 @@ impl ExtensionNode {
// If the path is prefixed by this node's prefix, delegate to its child.
// Otherwise, no value is present.
if path.skip_prefix(&self.prefix) {
let child_node = self
.child
.get_node(db, path.current())?
.ok_or(TrieError::InconsistentTree)?;
let child_node = self.child.get_node(db, path.current())?.ok_or_else(|| {
TrieError::InconsistentTree(Box::new(
InconsistentTreeError::ExtensionNodeChildNotFound(Box::new(
ExtensionNodeErrorData {
node_hash: self.child.compute_hash().finalize(),
extension_node_hash: self.compute_hash().finalize(),
extension_node_prefix: self.prefix.clone(),
node_path: path.current(),
},
)),
))
})?;

child_node.get(db, path)
} else {
Expand Down Expand Up @@ -58,14 +69,23 @@ impl ExtensionNode {
if match_index == self.prefix.len() {
let path = path.offset(match_index);
// Insert into child node
let child_node = self
.child
.get_node(db, path.current())?
.ok_or(TrieError::InconsistentTree)?;
let child_node = self.child.get_node(db, path.current())?.ok_or_else(|| {
TrieError::InconsistentTree(Box::new(
InconsistentTreeError::ExtensionNodeChildNotFound(Box::new(
ExtensionNodeErrorData {
node_hash: self.child.compute_hash().finalize(),
extension_node_hash: self.compute_hash().finalize(),
extension_node_prefix: self.prefix.clone(),
node_path: path.current(),
},
)),
))
})?;
let new_child_node = child_node.insert(db, path, value)?;
self.child = new_child_node.into();
Ok(self.into())
} else if match_index == 0 {
let current_node_hash = self.compute_hash().finalize();
let new_node = if self.prefix.len() == 1 {
self.child
} else {
Expand All @@ -75,7 +95,30 @@ impl ExtensionNode {
let branch_node = if self.prefix.at(0) == 16 {
match new_node.get_node(db, path.current())? {
Some(Node::Leaf(leaf)) => BranchNode::new_with_value(choices, leaf.value),
_ => return Err(TrieError::InconsistentTree),
Some(_) => {
return Err(TrieError::InconsistentTree(Box::new(
InconsistentTreeError::ExtensionNodeChildDiffers(Box::new(
ExtensionNodeErrorData {
node_hash: new_node.compute_hash().finalize(),
extension_node_hash: current_node_hash,
extension_node_prefix: self.prefix,
node_path: path.current(),
},
)),
)));
}
None => {
return Err(TrieError::InconsistentTree(Box::new(
InconsistentTreeError::ExtensionNodeChildNotFound(Box::new(
ExtensionNodeErrorData {
node_hash: new_node.compute_hash().finalize(),
extension_node_hash: current_node_hash,
extension_node_prefix: self.prefix,
node_path: path.current(),
},
)),
)));
}
}
} else {
choices[self.prefix.at(0)] = new_node;
Expand Down Expand Up @@ -106,10 +149,18 @@ impl ExtensionNode {

// Check if the value is part of the child subtrie according to the prefix
if path.skip_prefix(&self.prefix) {
let child_node = self
.child
.get_node(db, path.current())?
.ok_or(TrieError::InconsistentTree)?;
let child_node = self.child.get_node(db, path.current())?.ok_or_else(|| {
TrieError::InconsistentTree(Box::new(
InconsistentTreeError::ExtensionNodeChildNotFound(Box::new(
ExtensionNodeErrorData {
node_hash: self.child.compute_hash().finalize(),
extension_node_hash: self.compute_hash().finalize(),
extension_node_prefix: self.prefix.clone(),
node_path: path.current(),
},
)),
))
})?;
// Remove value from child subtrie
let (child_node, old_value) = child_node.remove(db, path)?;
// Restructure node based on removal
Expand Down Expand Up @@ -173,10 +224,18 @@ impl ExtensionNode {
};
// Continue to child
if path.skip_prefix(&self.prefix) {
let child_node = self
.child
.get_node(db, path.current())?
.ok_or(TrieError::InconsistentTree)?;
let child_node = self.child.get_node(db, path.current())?.ok_or_else(|| {
TrieError::InconsistentTree(Box::new(
InconsistentTreeError::ExtensionNodeChildNotFound(Box::new(
ExtensionNodeErrorData {
node_hash: self.child.clone().compute_hash().finalize(),
extension_node_hash: self.compute_hash().finalize(),
extension_node_prefix: self.prefix.clone(),
node_path: path.current(),
},
)),
))
})?;
child_node.get_path(db, path, node_path)?;
}
Ok(())
Expand Down
Loading