diff --git a/crates/common/trie/error.rs b/crates/common/trie/error.rs index 1cfe1ca345b..3833abb6a0f 100644 --- a/crates/common/trie/error.rs +++ b/crates/common/trie/error.rs @@ -1,14 +1,18 @@ +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), + // Box was added to make the error smaller since the InconsistentTreeError variants size vary up to more than 168 bytes. #[error("Lock Error: Panicked when trying to acquire a lock")] LockError, #[error("Database error: {0}")] @@ -16,3 +20,33 @@ pub enum TrieError { #[error("Invalid trie input")] InvalidInput, } + +#[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, +} + +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 + ) + } +} diff --git a/crates/common/trie/node/branch.rs b/crates/common/trie/node/branch.rs index 270f7599642..65434383bc4 100644 --- a/crates/common/trie/node/branch.rs +++ b/crates/common/trie/node/branch.rs @@ -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}; @@ -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) @@ -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() } } } @@ -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 { @@ -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( @@ -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)?; } } diff --git a/crates/common/trie/node/extension.rs b/crates/common/trie/node/extension.rs index 00248990044..4a99a3ff7ca 100644 --- a/crates/common/trie/node/extension.rs +++ b/crates/common/trie/node/extension.rs @@ -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}; @@ -26,10 +29,16 @@ 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(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 { @@ -58,14 +67,21 @@ 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(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 { @@ -75,7 +91,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( + 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( + 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; @@ -106,10 +145,16 @@ 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(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 @@ -173,10 +218,16 @@ 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(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(()) diff --git a/crates/common/trie/trie.rs b/crates/common/trie/trie.rs index eea5b1eba6f..2907d1a600b 100644 --- a/crates/common/trie/trie.rs +++ b/crates/common/trie/trie.rs @@ -25,7 +25,7 @@ pub use self::{ node_hash::NodeHash, }; -pub use self::error::TrieError; +pub use self::error::{ExtensionNodeErrorData, InconsistentTreeError, TrieError}; use self::{node::LeafNode, trie_iter::TrieIterator}; use ethrex_rlp::decode::RLPDecode; @@ -113,14 +113,15 @@ impl Trie { Ok(match self.root { NodeRef::Node(ref node, _) => node.get(self.db.as_ref(), path)?, - NodeRef::Hash(hash) if hash.is_valid() => Node::decode( - &self - .db - .get(Nibbles::default())? - .ok_or(TrieError::InconsistentTree)?, - ) - .map_err(TrieError::RLPDecode)? - .get(self.db.as_ref(), path)?, + NodeRef::Hash(hash) if hash.is_valid() => { + Node::decode(&self.db.get(Nibbles::default())?.ok_or_else(|| { + TrieError::InconsistentTree(Box::new(InconsistentTreeError::RootNotFound( + hash.finalize(), + ))) + })?) + .map_err(TrieError::RLPDecode)? + .get(self.db.as_ref(), path)? + } _ => None, }) } @@ -132,9 +133,7 @@ impl Trie { self.root = if self.root.is_valid() { // If the trie is not empty, call the root node's insertion logic. - self.root - .get_node(self.db.as_ref(), Nibbles::default())? - .ok_or(TrieError::InconsistentTree)? + self.get_root_node(Nibbles::default())? .insert(self.db.as_ref(), path, value)? .into() } else { @@ -157,9 +156,7 @@ impl Trie { // If the trie is not empty, call the root node's removal logic. let (node, value) = self - .root - .get_node(self.db.as_ref(), Nibbles::default())? - .ok_or(TrieError::InconsistentTree)? + .get_root_node(Nibbles::default())? .remove(self.db.as_ref(), Nibbles::from_bytes(path))?; self.root = node.map(Into::into).unwrap_or_default(); @@ -184,6 +181,14 @@ impl Trie { } } + pub fn get_root_node(&self, path: Nibbles) -> Result { + self.root.get_node(self.db.as_ref(), path)?.ok_or_else(|| { + TrieError::InconsistentTree(Box::new(InconsistentTreeError::RootNotFound( + self.root.compute_hash().finalize(), + ))) + }) + } + /// Returns a list of changes in a TrieNode format since last root hash processed. /// /// # Returns @@ -257,11 +262,7 @@ impl Trie { paths: &[PathRLP], ) -> Result<(Option, Vec), TrieError> { if self.root.is_valid() { - let encoded_root = self - .root - .get_node(self.db.as_ref(), Nibbles::default())? - .ok_or(TrieError::InconsistentTree)? - .encode_raw(); + let encoded_root = self.get_root_node(Nibbles::default())?.encode_raw(); let mut node_path = HashSet::new(); for path in paths { @@ -292,9 +293,9 @@ impl Trie { return Ok(NodeRef::default()); } - let root_rlp = all_nodes - .get(&root_hash) - .ok_or(TrieError::InconsistentTree)?; + let root_rlp = all_nodes.get(&root_hash).ok_or_else(|| { + TrieError::InconsistentTree(Box::new(InconsistentTreeError::RootNotFound(root_hash))) + })?; fn get_embedded_node( all_nodes: &BTreeMap>, @@ -418,9 +419,16 @@ impl Trie { let child_ref = &branch_node.choices[idx]; if child_ref.is_valid() { let child_path = current_path.append_new(idx as u8); - let child_node = child_ref - .get_node(db, child_path.clone())? - .ok_or(TrieError::InconsistentTree)?; + let child_node = + child_ref.get_node(db, child_path.clone())?.ok_or_else(|| { + TrieError::InconsistentTree(Box::new( + InconsistentTreeError::NodeNotFoundOnBranchNode( + child_ref.compute_hash().finalize(), + branch_node.compute_hash().finalize(), + child_path.clone(), + ), + )) + })?; get_node_inner(db, child_path, child_node, partial_path) } else { Ok(vec![]) @@ -436,7 +444,23 @@ impl Trie { let child_node = extension_node .child .get_node(db, child_path.clone())? - .ok_or(TrieError::InconsistentTree)?; + .ok_or_else(|| { + TrieError::InconsistentTree(Box::new( + InconsistentTreeError::ExtensionNodeChildNotFound( + ExtensionNodeErrorData { + node_hash: extension_node + .child + .compute_hash() + .finalize(), + extension_node_hash: extension_node + .compute_hash() + .finalize(), + extension_node_prefix: extension_node.prefix, + node_path: child_path.clone(), + }, + ), + )) + })?; get_node_inner(db, child_path, child_node, partial_path) } else { Ok(vec![]) @@ -451,9 +475,7 @@ impl Trie { get_node_inner( self.db.as_ref(), Default::default(), - self.root - .get_node(self.db.as_ref(), Default::default())? - .ok_or(TrieError::InconsistentTree)?, + self.get_root_node(Default::default())?, partial_path, ) } else { @@ -498,7 +520,11 @@ impl ProofTrie { self.0 .root .get_node(self.0.db.as_ref(), Nibbles::default())? - .ok_or(TrieError::InconsistentTree)? + .ok_or_else(|| { + TrieError::InconsistentTree(Box::new(InconsistentTreeError::RootNotFound( + self.0.root.compute_hash().finalize(), + ))) + })? .insert(self.0.db.as_ref(), partial_path, external_ref)? .into() } else { diff --git a/crates/networking/p2p/rlpx/connection/server.rs b/crates/networking/p2p/rlpx/connection/server.rs index ff90309b761..fdb25d765d3 100644 --- a/crates/networking/p2p/rlpx/connection/server.rs +++ b/crates/networking/p2p/rlpx/connection/server.rs @@ -415,7 +415,7 @@ impl GenServer for PeerConnectionServer { return CastResponse::Stop; } PeerConnectionError::StoreError(StoreError::Trie( - TrieError::InconsistentTree, + TrieError::InconsistentTree(_), )) => { if established_state.blockchain.is_synced() { log_peer_error!(