perf(l1): avoid validating node hash

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

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<Option<Arc<Node>>, 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<Option<Arc<Node>>, 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);

crates/common/trie/trie.rs

@@ -194,11 +194,13 @@ impl Trie {
     }
 
     pub fn get_root_node(&self, path: Nibbles) -> Result<Arc<Node>, 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

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) {

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 {

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();