proper errors

crates/trie/sparse/Cargo.toml

@@ -34,6 +34,7 @@ smallvec = { workspace = true, features = ["const_new"] }
 reth-primitives = { workspace = true, features = ["test-utils", "arbitrary"] }
 reth-trie-common = { workspace = true, features = ["test-utils", "arbitrary"] }
 reth-trie = { workspace = true, features = ["test-utils"] }
+assert_matches.workspace = true
 itertools.workspace = true
 proptest.workspace = true
 criterion.workspace = true

crates/trie/sparse/src/errors.rs

@@ -0,0 +1,49 @@
+//! Errors for sparse trie.
+
+use alloy_primitives::{Bytes, B256};
+use reth_trie::Nibbles;
+use thiserror::Error;
+
+/// Result type with [`SparseStateTrieError`] as error.
+pub type SparseStateTrieResult<Ok> = Result<Ok, SparseStateTrieError>;
+
+/// Error encountered in [`crate::SparseStateTrie`].
+#[derive(Error, Debug)]
+pub enum SparseStateTrieError {
+    /// Encountered invalid root node.
+    #[error("invalid root node at {path:?}: {node:?}")]
+    InvalidRootNode {
+        /// Path to first proof node.
+        path: Nibbles,
+        /// Encoded first proof node.
+        node: Bytes,
+    },
+    /// Sparse trie error.
+    #[error(transparent)]
+    Sparse(#[from] SparseTrieError),
+    /// RLP error.
+    #[error(transparent)]
+    Rlp(#[from] alloy_rlp::Error),
+}
+
+/// Result type with [`SparseTrieError`] as error.
+pub type SparseTrieResult<Ok> = Result<Ok, SparseTrieError>;
+
+/// Error encountered in [`crate::SparseTrie`].
+#[derive(Error, Debug)]
+pub enum SparseTrieError {
+    /// Sparse trie is still blind. Thrown on attempt to update it.
+    #[error("sparse trie is blind")]
+    Blind,
+    /// Encountered blinded node on update.
+    #[error("attempted to update blind node at {path:?}: {hash}")]
+    BlindedNode {
+        /// Blind node path.
+        path: Nibbles,
+        /// Node hash
+        hash: B256,
+    },
+    /// RLP error.
+    #[error(transparent)]
+    Rlp(#[from] alloy_rlp::Error),
+}

crates/trie/sparse/src/lib.rs

@@ -5,3 +5,6 @@ pub use state::*;
 
 mod trie;
 pub use trie::*;
+
+mod errors;
+pub use errors::*;

crates/trie/sparse/src/state.rs

@@ -1,4 +1,4 @@
-use crate::SparseTrie;
+use crate::{SparseStateTrieError, SparseStateTrieResult, SparseTrie};
 use alloy_primitives::{
     map::{HashMap, HashSet},
     Bytes, B256,
@@ -36,26 +36,27 @@ impl SparseStateTrie {
     }
 
     /// Reveal unknown trie paths from provided leaf path and its proof.
-    ///
-    /// # Panics
-    ///
-    /// This method panics on invalid proof if `debug_assertions` are enabled.
-    /// However, it does not extensively validate the proof.
+    /// NOTE: This method does not extensively validate the proof.
     pub fn reveal_account(
         &mut self,
         account: B256,
         proof: impl IntoIterator<Item = (Nibbles, Bytes)>,
-    ) -> alloy_rlp::Result<()> {
+    ) -> SparseStateTrieResult<()> {
         let mut proof = proof.into_iter().peekable();
 
-        // reveal root and initialize the trie of not already
-        let Some((path, root)) = proof.next() else { return Ok(()) };
-        debug_assert!(path.is_empty(), "first proof node is not root");
-        let root_node = TrieNode::decode(&mut &root[..])?;
-        debug_assert!(
-            !matches!(root_node, TrieNode::EmptyRoot) || proof.peek().is_none(),
-            "invalid proof"
-        );
+        // reveal root and initialize the trie if not already
+        let Some((path, node)) = proof.next() else { return Ok(()) };
+        if !path.is_empty() {
+            return Err(SparseStateTrieError::InvalidRootNode { path, node })
+        }
+
+        // Decode root node and perform sanity check.
+        let root_node = TrieNode::decode(&mut &node[..])?;
+        if matches!(root_node, TrieNode::EmptyRoot) && proof.peek().is_some() {
+            return Err(SparseStateTrieError::InvalidRootNode { path, node })
+        }
+
+        // Reveal root node if it wasn't already.
         let trie = self.state.reveal_root(root_node)?;
 
         // add the remaining proof nodes
@@ -70,20 +71,24 @@ impl SparseStateTrie {
         Ok(())
     }
 
-    /// Returns sparse trie root if the the trie has been revealed.
-    pub fn root(&mut self) -> Option<B256> {
-        self.state.root()
+    /// Update the leaf node.
+    pub fn update_leaf(&mut self, path: Nibbles, value: Vec<u8>) -> SparseStateTrieResult<()> {
+        self.state.update_leaf(path, value)?;
+        Ok(())
     }
 
-    /// Update the leaf node
-    pub fn update_leaf(&mut self, path: Nibbles, value: Vec<u8>) {
-        self.state.as_revealed_mut().unwrap().update_leaf(path, value);
+    /// Returns sparse trie root if the trie has been revealed.
+    pub fn root(&mut self) -> Option<B256> {
+        self.state.root()
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use alloy_primitives::Bytes;
+    use alloy_rlp::EMPTY_STRING_CODE;
+    use assert_matches::assert_matches;
     use reth_trie::HashBuilder;
     use reth_trie_common::proof::ProofRetainer;
 
@@ -101,29 +106,26 @@ mod tests {
         assert_eq!(sparse.state, SparseTrie::revealed_empty());
     }
 
-    #[cfg(debug_assertions)]
-    mod debug_assertions {
-        use super::*;
-        use alloy_primitives::Bytes;
-        use alloy_rlp::EMPTY_STRING_CODE;
-
-        #[test]
-        #[should_panic]
-        fn reveal_first_node_not_root() {
-            let mut sparse = SparseStateTrie::default();
-            let proof = [(Nibbles::from_nibbles([0x1]), Bytes::from([EMPTY_STRING_CODE]))];
-            sparse.reveal_account(Default::default(), proof).unwrap();
-        }
+    #[test]
+    fn reveal_first_node_not_root() {
+        let mut sparse = SparseStateTrie::default();
+        let proof = [(Nibbles::from_nibbles([0x1]), Bytes::from([EMPTY_STRING_CODE]))];
+        assert_matches!(
+            sparse.reveal_account(Default::default(), proof),
+            Err(SparseStateTrieError::InvalidRootNode { .. })
+        );
+    }
 
-        #[test]
-        #[should_panic]
-        fn reveal_invalid_proof_with_empty_root() {
-            let mut sparse = SparseStateTrie::default();
-            let proof = [
-                (Nibbles::default(), Bytes::from([EMPTY_STRING_CODE])),
-                (Nibbles::from_nibbles([0x1]), Bytes::new()),
-            ];
-            sparse.reveal_account(Default::default(), proof).unwrap();
-        }
+    #[test]
+    fn reveal_invalid_proof_with_empty_root() {
+        let mut sparse = SparseStateTrie::default();
+        let proof = [
+            (Nibbles::default(), Bytes::from([EMPTY_STRING_CODE])),
+            (Nibbles::from_nibbles([0x1]), Bytes::new()),
+        ];
+        assert_matches!(
+            sparse.reveal_account(Default::default(), proof),
+            Err(SparseStateTrieError::InvalidRootNode { .. })
+        );
     }
 }

crates/trie/sparse/src/trie.rs

@@ -1,3 +1,4 @@
+use crate::{SparseTrieError, SparseTrieResult};
 use alloy_primitives::{hex, keccak256, map::HashMap, B256};
 use alloy_rlp::Decodable;
 use reth_trie::prefix_set::{PrefixSet, PrefixSetMut};
@@ -44,13 +45,20 @@ impl SparseTrie {
     /// # Returns
     ///
     /// Mutable reference to [`RevealedSparseTrie`].
-    pub fn reveal_root(&mut self, root: TrieNode) -> alloy_rlp::Result<&mut RevealedSparseTrie> {
+    pub fn reveal_root(&mut self, root: TrieNode) -> SparseTrieResult<&mut RevealedSparseTrie> {
         if self.is_blind() {
             *self = Self::Revealed(RevealedSparseTrie::from_root(root)?)
         }
         Ok(self.as_revealed_mut().unwrap())
     }
 
+    /// Update the leaf node.
+    pub fn update_leaf(&mut self, path: Nibbles, value: Vec<u8>) -> SparseTrieResult<()> {
+        let revealed = self.as_revealed_mut().ok_or(SparseTrieError::Blind)?;
+        revealed.update_leaf(path, value)?;
+        Ok(())
+    }
+
     /// Calculates and returns the trie root if the trie has been revealed.
     pub fn root(&mut self) -> Option<B256> {
         Some(self.as_revealed_mut()?.root())
@@ -94,7 +102,7 @@ impl Default for RevealedSparseTrie {
 
 impl RevealedSparseTrie {
     /// Create new revealed sparse trie from the given root node.
-    pub fn from_root(node: TrieNode) -> alloy_rlp::Result<Self> {
+    pub fn from_root(node: TrieNode) -> SparseTrieResult<Self> {
         let mut this = Self {
             nodes: HashMap::default(),
             values: HashMap::default(),
@@ -106,7 +114,7 @@ impl RevealedSparseTrie {
     }
 
     /// Reveal the trie node only if it was not known already.
-    pub fn reveal_node(&mut self, path: Nibbles, node: TrieNode) -> alloy_rlp::Result<()> {
+    pub fn reveal_node(&mut self, path: Nibbles, node: TrieNode) -> SparseTrieResult<()> {
         // TODO: revise all inserts to not overwrite existing entries
         match node {
             TrieNode::EmptyRoot => {
@@ -143,7 +151,7 @@ impl RevealedSparseTrie {
         Ok(())
     }
 
-    fn reveal_node_or_hash(&mut self, path: Nibbles, child: &[u8]) -> alloy_rlp::Result<()> {
+    fn reveal_node_or_hash(&mut self, path: Nibbles, child: &[u8]) -> SparseTrieResult<()> {
         if child.len() == B256::len_bytes() + 1 {
             // TODO: revise insert to not overwrite existing entries
             self.nodes.insert(path, SparseNode::Hash(B256::from_slice(&child[1..])));
@@ -154,12 +162,12 @@ impl RevealedSparseTrie {
     }
 
     /// Update the leaf node with provided value.
-    pub fn update_leaf(&mut self, path: Nibbles, value: Vec<u8>) {
+    pub fn update_leaf(&mut self, path: Nibbles, value: Vec<u8>) -> SparseTrieResult<()> {
         self.prefix_set.insert(path.clone());
         let existing = self.values.insert(path.clone(), value);
         if existing.is_some() {
             // trie structure unchanged, return immediately
-            return
+            return Ok(())
         }
 
         let mut current = Nibbles::default();
@@ -169,16 +177,15 @@ impl RevealedSparseTrie {
                     *node = SparseNode::new_leaf(path);
                     break
                 }
-                SparseNode::Hash(_) => {
-                    unimplemented!() // TODO: error out
+                SparseNode::Hash(hash) => {
+                    return Err(SparseTrieError::BlindedNode { path: current, hash: *hash })
                 }
                 SparseNode::Leaf { key: current_key, .. } => {
                     current.extend_from_slice_unchecked(current_key);
 
                     // this leaf is being updated
                     if current == path {
-                        // TODO: unreachable
-                        break
+                        unreachable!("we already checked leaf presence in the beginning");
                     }
 
                     // find the common prefix
@@ -242,6 +249,8 @@ impl RevealedSparseTrie {
                 }
             };
         }
+
+        Ok(())
     }
 
     /// Remove leaf node from the trie.
@@ -487,7 +496,7 @@ mod tests {
         let expected = hash_builder.root();
 
         let mut sparse = RevealedSparseTrie::default();
-        sparse.update_leaf(path, value.to_vec());
+        sparse.update_leaf(path, value.to_vec()).unwrap();
         let root = sparse.root();
         assert_eq!(root, expected);
     }
@@ -505,7 +514,7 @@ mod tests {
 
         let mut sparse = RevealedSparseTrie::default();
         for path in &paths {
-            sparse.update_leaf(path.clone(), value.to_vec());
+            sparse.update_leaf(path.clone(), value.to_vec()).unwrap();
         }
         let root = sparse.root();
         assert_eq!(root, expected);
@@ -524,7 +533,7 @@ mod tests {
 
         let mut sparse = RevealedSparseTrie::default();
         for path in &paths {
-            sparse.update_leaf(path.clone(), value.to_vec());
+            sparse.update_leaf(path.clone(), value.to_vec()).unwrap();
         }
         let root = sparse.root();
         assert_eq!(root, expected);
@@ -551,7 +560,7 @@ mod tests {
 
         let mut sparse = RevealedSparseTrie::default();
         for path in &paths {
-            sparse.update_leaf(path.clone(), value.to_vec());
+            sparse.update_leaf(path.clone(), value.to_vec()).unwrap();
         }
         let root = sparse.root();
         assert_eq!(root, expected);
@@ -571,7 +580,7 @@ mod tests {
 
         let mut sparse = RevealedSparseTrie::default();
         for path in &paths {
-            sparse.update_leaf(path.clone(), old_value.to_vec());
+            sparse.update_leaf(path.clone(), old_value.to_vec()).unwrap();
         }
         let root = sparse.root();
         assert_eq!(root, expected);
@@ -583,7 +592,7 @@ mod tests {
         let expected = hash_builder.root();
 
         for path in &paths {
-            sparse.update_leaf(path.clone(), new_value.to_vec());
+            sparse.update_leaf(path.clone(), new_value.to_vec()).unwrap();
         }
         let root = sparse.root();
         assert_eq!(root, expected);
@@ -597,7 +606,7 @@ mod tests {
 
             for update in updates {
                 for (key, value) in &update {
-                    sparse.update_leaf(Nibbles::unpack(key), alloy_rlp::encode_fixed_size(value).to_vec());
+                    sparse.update_leaf(Nibbles::unpack(key), alloy_rlp::encode_fixed_size(value).to_vec()).unwrap();
                 }
                 let root = sparse.root();