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 2 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
4 changes: 2 additions & 2 deletions crates/common/trie/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ pub enum TrieError {
RLPDecode(#[from] RLPDecodeError),
#[error("Verification Error: {0}")]
Verify(String),
#[error("Inconsistent internal tree structure")]
InconsistentTree,
#[error("Inconsistent internal tree structure {0}")]
InconsistentTree(String),
#[error("Lock Error: Panicked when trying to acquire a lock")]
LockError,
#[error("Database error: {0}")]
Expand Down
48 changes: 38 additions & 10 deletions crates/common/trie/node/branch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,13 @@ 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)?.ok_or(TrieError::InconsistentTree)?;
let child_node =
child_ref
.get_node(db)?
.ok_or(TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
child_ref.compute_hash()
)))?;
child_node.get(db, path)
} else {
Ok(None)
Expand Down Expand Up @@ -73,9 +79,13 @@ impl BranchNode {
}
// Insert into existing child and then update it
(choice_ref, ValueOrHash::Value(value)) => {
let child_node = choice_ref
.get_node(db)?
.ok_or(TrieError::InconsistentTree)?;
let child_node =
choice_ref
.get_node(db)?
.ok_or(TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
choice_ref.compute_hash()
)))?;

*choice_ref = child_node.insert(db, path, value)?.into();
}
Expand All @@ -90,7 +100,10 @@ impl BranchNode {
} else {
*choice_ref = choice_ref
.get_node(db)?
.ok_or(TrieError::InconsistentTree)?
.ok_or(TrieError::InconsistentTree(format!(
"Node hash {:?} not found in db",
choice_ref.compute_hash()
)))?
.insert(db, path, value)?
.into();
}
Expand Down Expand Up @@ -135,9 +148,13 @@ impl BranchNode {
// Check if the value is located in a child subtrie
let value = if let Some(choice_index) = path.next_choice() {
if self.choices[choice_index].is_valid() {
let child_node = self.choices[choice_index]
.get_node(db)?
.ok_or(TrieError::InconsistentTree)?;
let child_node =
self.choices[choice_index]
.get_node(db)?
.ok_or(TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
self.choices[choice_index].compute_hash()
)))?;
// 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 @@ -176,7 +193,12 @@ impl BranchNode {
// If this node doesn't have a value and has only one child, replace it with its child node
(1, false) => {
let (choice_index, child_ref) = children[0];
let child = child_ref.get_node(db)?.ok_or(TrieError::InconsistentTree)?;
let child = child_ref
.get_node(db)?
.ok_or(TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
child_ref.compute_hash()
)))?;
match child {
// Replace self with an extension node leading to the child
Node::Branch(_) => ExtensionNode::new(
Expand Down Expand Up @@ -243,7 +265,13 @@ impl BranchNode {
// Continue to child
let child_ref = &self.choices[choice];
if child_ref.is_valid() {
let child_node = child_ref.get_node(db)?.ok_or(TrieError::InconsistentTree)?;
let child_node =
child_ref
.get_node(db)?
.ok_or(TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
child_ref.compute_hash()
)))?;
child_node.get_path(db, path, node_path)?;
}
}
Expand Down
51 changes: 34 additions & 17 deletions crates/common/trie/node/extension.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@ 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)?
.ok_or(TrieError::InconsistentTree)?;
let child_node =
self.child
.get_node(db)?
.ok_or(TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
self.child.compute_hash()
)))?;

child_node.get(db, path)
} else {
Expand Down Expand Up @@ -57,10 +60,13 @@ impl ExtensionNode {
let match_index = path.count_prefix(&self.prefix);
if match_index == self.prefix.len() {
// Insert into child node
let child_node = self
.child
.get_node(db)?
.ok_or(TrieError::InconsistentTree)?;
let child_node =
self.child
.get_node(db)?
.ok_or(TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
self.child.compute_hash()
)))?;
let new_child_node = child_node.insert(db, path.offset(match_index), value)?;
self.child = new_child_node.into();
Ok(self.into())
Expand All @@ -74,7 +80,12 @@ impl ExtensionNode {
let branch_node = if self.prefix.at(0) == 16 {
match new_node.get_node(db)? {
Some(Node::Leaf(leaf)) => BranchNode::new_with_value(choices, leaf.value),
_ => return Err(TrieError::InconsistentTree),
_ => {
return Err(TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
new_node.compute_hash()
)));
}
}
} else {
choices[self.prefix.at(0)] = new_node;
Expand Down Expand Up @@ -105,10 +116,13 @@ 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)?
.ok_or(TrieError::InconsistentTree)?;
let child_node =
self.child
.get_node(db)?
.ok_or(TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
self.child.compute_hash()
)))?;
// Remove value from child subtrie
let (child_node, old_value) = child_node.remove(db, path)?;
// Restructure node based on removal
Expand Down Expand Up @@ -172,10 +186,13 @@ impl ExtensionNode {
};
// Continue to child
if path.skip_prefix(&self.prefix) {
let child_node = self
.child
.get_node(db)?
.ok_or(TrieError::InconsistentTree)?;
let child_node =
self.child
.get_node(db)?
.ok_or(TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
self.child.compute_hash()
)))?;
child_node.get_path(db, path, node_path)?;
}
Ok(())
Expand Down
54 changes: 41 additions & 13 deletions crates/common/trie/trie.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,12 @@ impl Trie {
Ok(match self.root {
NodeRef::Node(ref node, _) => node.get(self.db.as_ref(), Nibbles::from_bytes(path))?,
NodeRef::Hash(hash) if hash.is_valid() => {
let rlp = self.db.get(hash)?.ok_or(TrieError::InconsistentTree)?;
let rlp = self
.db
.get(hash)?
.ok_or(TrieError::InconsistentTree(format!(
"Node hash {hash:?} not found in db"
)))?;
let node = Node::decode(&rlp).map_err(TrieError::RLPDecode)?;
node.get(self.db.as_ref(), Nibbles::from_bytes(path))?
}
Expand All @@ -112,7 +117,10 @@ impl Trie {
// If the trie is not empty, call the root node's insertion logic.
self.root
.get_node(self.db.as_ref())?
.ok_or(TrieError::InconsistentTree)?
.ok_or(TrieError::InconsistentTree(format!(
"Root node with hash {:?} not found in db",
self.root.compute_hash()
)))?
.insert(self.db.as_ref(), path, value)?
.into()
} else {
Expand All @@ -134,7 +142,10 @@ impl Trie {
let (node, value) = self
.root
.get_node(self.db.as_ref())?
.ok_or(TrieError::InconsistentTree)?
.ok_or(TrieError::InconsistentTree(format!(
"Root node with hash {:?} not found in db",
self.root.compute_hash()
)))?
.remove(self.db.as_ref(), Nibbles::from_bytes(path))?;
self.root = node.map(Into::into).unwrap_or_default();

Expand Down Expand Up @@ -234,7 +245,10 @@ impl Trie {
let encoded_root = self
.root
.get_node(self.db.as_ref())?
.ok_or(TrieError::InconsistentTree)?
.ok_or(TrieError::InconsistentTree(format!(
"Root node with hash {:?} not found in db",
self.root.compute_hash()
)))?
.encode_raw();

let mut node_path = HashSet::new();
Expand Down Expand Up @@ -263,7 +277,9 @@ impl Trie {
) -> Result<NodeRef, TrieError> {
let root_rlp = all_nodes
.get(&root_hash)
.ok_or(TrieError::InconsistentTree)?;
.ok_or(TrieError::InconsistentTree(format!(
"Root node {root_hash:?} not found in db"
)))?;

fn get_embedded_node(
all_nodes: &BTreeMap<H256, Vec<u8>>,
Expand Down Expand Up @@ -385,8 +401,12 @@ impl Trie {
Some(idx) => {
let child_ref = &branch_node.choices[idx];
if child_ref.is_valid() {
let child_node =
child_ref.get_node(db)?.ok_or(TrieError::InconsistentTree)?;
let child_node = child_ref.get_node(db)?.ok_or(
TrieError::InconsistentTree(format!(
"Node with hash {:?} not found in db",
child_ref.compute_hash()
)),
)?;
get_node_inner(db, child_node, partial_path)
} else {
Ok(vec![])
Expand All @@ -398,10 +418,12 @@ impl Trie {
if partial_path.skip_prefix(&extension_node.prefix)
&& extension_node.child.is_valid()
{
let child_node = extension_node
.child
.get_node(db)?
.ok_or(TrieError::InconsistentTree)?;
let child_node = extension_node.child.get_node(db)?.ok_or(
TrieError::InconsistentTree(format!(
"Extension Node child with hash {:?} with not found in db",
extension_node.child.compute_hash()
)),
)?;
get_node_inner(db, child_node, partial_path)
} else {
Ok(vec![])
Expand All @@ -417,7 +439,10 @@ impl Trie {
self.db.as_ref(),
self.root
.get_node(self.db.as_ref())?
.ok_or(TrieError::InconsistentTree)?,
.ok_or(TrieError::InconsistentTree(format!(
"Root node with hash {:?} not found in db",
self.root.compute_hash()
)))?,
partial_path,
)
} else {
Expand Down Expand Up @@ -468,7 +493,10 @@ impl ProofTrie {
self.0
.root
.get_node(self.0.db.as_ref())?
.ok_or(TrieError::InconsistentTree)?
.ok_or(TrieError::InconsistentTree(format!(
"Root node with hash {:?} not found in db",
self.0.root.compute_hash()
)))?
.insert(self.0.db.as_ref(), partial_path, external_ref)?
.into()
} else {
Expand Down
2 changes: 1 addition & 1 deletion crates/networking/p2p/rlpx/connection/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ impl GenServer for RLPxConnection {
);
return CastResponse::Stop;
}
RLPxError::StoreError(StoreError::Trie(TrieError::InconsistentTree)) => {
RLPxError::StoreError(StoreError::Trie(TrieError::InconsistentTree(_))) => {
if established_state.blockchain.is_synced() {
log_peer_error(
&established_state.node,
Expand Down
Loading