diff --git a/CHANGELOG.md b/CHANGELOG.md index db8263f6e34..d439fbe30bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ ## Perf ### 2025-11-03 + +- Avoid unnecessary hash validations [#5167](https://github.com/lambdaclass/ethrex/pull/5167) - Merge execution with some post-execution validations [#5170](https://github.com/lambdaclass/ethrex/pull/5170) ### 2025-10-31 diff --git a/crates/common/trie/node.rs b/crates/common/trie/node.rs index bb98030549c..3bcb65d1bbd 100644 --- a/crates/common/trie/node.rs +++ b/crates/common/trie/node.rs @@ -24,7 +24,29 @@ pub enum NodeRef { impl NodeRef { /// Gets a shared reference to the inner node. + /// Requires that the trie is in a consistent state, ie that all leaves being pointed are in the database. + /// Outside of snapsync this should always be the case. pub fn get_node(&self, db: &dyn TrieDB, path: Nibbles) -> Result>, TrieError> { + match self { + NodeRef::Node(node, _) => Ok(Some(node.clone())), + NodeRef::Hash(hash @ NodeHash::Inline(_)) => { + Ok(Some(Arc::new(Node::decode(hash.as_ref())?))) + } + NodeRef::Hash(_) => db + .get(path)? + .filter(|rlp| !rlp.is_empty()) + .map(|rlp| Ok(Arc::new(Node::decode(&rlp)?))) + .transpose(), + } + } + + /// Gets a shared reference to the inner node, checking it's hash. + /// Returns `Ok(None)` if the hash is invalid. + pub fn get_node_checked( + &self, + db: &dyn TrieDB, + path: Nibbles, + ) -> Result>, TrieError> { match self { NodeRef::Node(node, _) => Ok(Some(node.clone())), NodeRef::Hash(hash @ NodeHash::Inline(_)) => { @@ -63,10 +85,7 @@ impl NodeRef { let Some(node) = db .get(path.clone())? .filter(|rlp| !rlp.is_empty()) - .and_then(|rlp| match Node::decode(&rlp) { - Ok(node) => (node.compute_hash() == *hash).then_some(Ok(node)), - Err(err) => Some(Err(TrieError::RLPDecode(err))), - }) + .map(|rlp| Node::decode(&rlp).map_err(TrieError::RLPDecode)) .transpose()? else { return Ok(None); diff --git a/crates/common/trie/trie.rs b/crates/common/trie/trie.rs index 4e2a2674f21..4102fdbe43b 100644 --- a/crates/common/trie/trie.rs +++ b/crates/common/trie/trie.rs @@ -194,11 +194,13 @@ impl Trie { } pub fn get_root_node(&self, path: Nibbles) -> Result, TrieError> { - self.root.get_node(self.db.as_ref(), path)?.ok_or_else(|| { - TrieError::InconsistentTree(Box::new(InconsistentTreeError::RootNotFound( - self.root.compute_hash().finalize(), - ))) - }) + self.root + .get_node_checked(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. @@ -254,7 +256,10 @@ impl Trie { node_path.push(data[..len as usize].to_vec()); } - let root = match self.root.get_node(self.db.as_ref(), Nibbles::default())? { + let root = match self + .root + .get_node_checked(self.db.as_ref(), Nibbles::default())? + { Some(x) => x, None => return Ok(Vec::new()), }; @@ -431,8 +436,9 @@ 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_else(|| { + let child_node = child_ref + .get_node_checked(db, child_path.clone())? + .ok_or_else(|| { TrieError::InconsistentTree(Box::new( InconsistentTreeError::NodeNotFoundOnBranchNode( child_ref.compute_hash().finalize(), @@ -455,7 +461,7 @@ impl Trie { let child_path = partial_path.concat(&extension_node.prefix); let child_node = extension_node .child - .get_node(db, child_path.clone())? + .get_node_checked(db, child_path.clone())? .ok_or_else(|| { TrieError::InconsistentTree(Box::new( InconsistentTreeError::ExtensionNodeChildNotFound( @@ -500,7 +506,8 @@ impl Trie { if self.hash_no_commit() == *EMPTY_TRIE_HASH { return Ok(None); } - self.root.get_node(self.db.as_ref(), Nibbles::default()) + self.root + .get_node_checked(self.db.as_ref(), Nibbles::default()) } /// Creates a new Trie based on a temporary InMemory DB diff --git a/crates/common/trie/trie_iter.rs b/crates/common/trie/trie_iter.rs index 90c1365a3fc..8a47bec524c 100644 --- a/crates/common/trie/trie_iter.rs +++ b/crates/common/trie/trie_iter.rs @@ -1,4 +1,4 @@ -use std::cmp::Ordering; +use std::{cmp::Ordering, sync::Arc}; use crate::{ PathRLP, Trie, TrieDB, TrieError, ValueRLP, @@ -39,14 +39,17 @@ impl TrieIterator { db: &dyn TrieDB, prefix_nibbles: Nibbles, mut target_nibbles: Nibbles, - mut node: NodeRef, + node: NodeRef, new_stack: &mut Vec<(Nibbles, NodeRef)>, ) -> Result<(), TrieError> { - let Some(next_node) = node.get_node_mut(db, prefix_nibbles.clone()).ok().flatten() + let Some(mut next_node) = node + .get_node_checked(db, prefix_nibbles.clone()) + .ok() + .flatten() else { return Ok(()); }; - match &next_node { + match Arc::make_mut(&mut next_node) { Node::Branch(branch_node) => { // Add all children to the stack (in reverse order so we process first child frist) let Some(choice) = target_nibbles.next_choice() else { @@ -141,7 +144,7 @@ impl Iterator for TrieIterator { // Fetch the last node in the stack let (mut path, next_node_ref) = self.stack.pop()?; let next_node = next_node_ref - .get_node(self.db.as_ref(), path.clone()) + .get_node_checked(self.db.as_ref(), path.clone()) .ok() .flatten()?; match &(*next_node) { diff --git a/crates/networking/p2p/sync/state_healing.rs b/crates/networking/p2p/sync/state_healing.rs index ff69e547f53..6aa2e84b68a 100644 --- a/crates/networking/p2p/sync/state_healing.rs +++ b/crates/networking/p2p/sync/state_healing.rs @@ -414,7 +414,7 @@ pub fn node_missing_children( continue; } let validity = child - .get_node(trie_state, child_path.clone()) + .get_node_checked(trie_state, child_path.clone()) .inspect_err(|_| { debug!("Malformed data when doing get child of a branch node") })? @@ -438,7 +438,7 @@ pub fn node_missing_children( } let validity = node .child - .get_node(trie_state, child_path.clone()) + .get_node_checked(trie_state, child_path.clone()) .inspect_err(|_| debug!("Malformed data when doing get child of a branch node"))? .is_some(); if validity { diff --git a/crates/networking/p2p/sync/storage_healing.rs b/crates/networking/p2p/sync/storage_healing.rs index a8bab86bf6e..37412a1ae5f 100644 --- a/crates/networking/p2p/sync/storage_healing.rs +++ b/crates/networking/p2p/sync/storage_healing.rs @@ -617,7 +617,7 @@ pub fn determine_missing_children( continue; } let validity = child - .get_node(trie_state, child_path.clone()) + .get_node_checked(trie_state, child_path.clone()) .inspect_err(|_| { debug!("Malformed data when doing get child of a branch node") })? @@ -643,7 +643,7 @@ pub fn determine_missing_children( } let validity = node .child - .get_node(trie_state, child_path.clone()) + .get_node_checked(trie_state, child_path.clone()) .inspect_err(|_| debug!("Malformed data when doing get child of a branch node"))? .is_some();