From b3015c75b1f56fdfbbcce8cdb97a3cde1c642985 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Tue, 22 Oct 2024 14:31:37 +0100 Subject: [PATCH] fix(trie): removing a blinded leaf should result in an error (#11869) Co-authored-by: Roman Krasiuk --- crates/trie/sparse/src/trie.rs | 68 ++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 16 deletions(-) diff --git a/crates/trie/sparse/src/trie.rs b/crates/trie/sparse/src/trie.rs index 6680d7e9b6..42f90d34c8 100644 --- a/crates/trie/sparse/src/trie.rs +++ b/crates/trie/sparse/src/trie.rs @@ -267,11 +267,11 @@ impl RevealedSparseTrie { /// Remove leaf node from the trie. pub fn remove_leaf(&mut self, path: &Nibbles) -> SparseTrieResult<()> { self.prefix_set.insert(path.clone()); - let existing = self.values.remove(path); - if existing.is_none() { - // trie structure unchanged, return immediately - return Ok(()) - } + self.values.remove(path); + + // If the path wasn't present in `values`, we still need to walk the trie and ensure that + // there is no node at the path. When a leaf node is a blinded `Hash`, it will have an entry + // in `nodes`, but not in the `values`. let mut removed_nodes = self.take_nodes_for_path(path)?; debug!(target: "trie::sparse", ?path, ?removed_nodes, "Removed nodes for path"); @@ -726,6 +726,7 @@ mod tests { use super::*; use alloy_primitives::U256; + use assert_matches::assert_matches; use itertools::Itertools; use prop::sample::SizeRange; use proptest::prelude::*; @@ -960,7 +961,7 @@ mod tests { pretty_assertions::assert_eq!( sparse.nodes.clone().into_iter().collect::>(), BTreeMap::from_iter([ - (Nibbles::new(), SparseNode::new_ext(Nibbles::from_nibbles([0x5]))), + (Nibbles::default(), SparseNode::new_ext(Nibbles::from_nibbles([0x5]))), (Nibbles::from_nibbles([0x5]), SparseNode::new_branch(0b1101.into())), ( Nibbles::from_nibbles([0x5, 0x0]), @@ -972,11 +973,11 @@ mod tests { ), ( Nibbles::from_nibbles([0x5, 0x0, 0x2, 0x3, 0x1]), - SparseNode::new_leaf(Nibbles::new()) + SparseNode::new_leaf(Nibbles::default()) ), ( Nibbles::from_nibbles([0x5, 0x0, 0x2, 0x3, 0x3]), - SparseNode::new_leaf(Nibbles::new()) + SparseNode::new_leaf(Nibbles::default()) ), ( Nibbles::from_nibbles([0x5, 0x2]), @@ -1015,7 +1016,7 @@ mod tests { pretty_assertions::assert_eq!( sparse.nodes.clone().into_iter().collect::>(), BTreeMap::from_iter([ - (Nibbles::new(), SparseNode::new_ext(Nibbles::from_nibbles([0x5]))), + (Nibbles::default(), SparseNode::new_ext(Nibbles::from_nibbles([0x5]))), (Nibbles::from_nibbles([0x5]), SparseNode::new_branch(0b1001.into())), ( Nibbles::from_nibbles([0x5, 0x0]), @@ -1027,11 +1028,11 @@ mod tests { ), ( Nibbles::from_nibbles([0x5, 0x0, 0x2, 0x3, 0x1]), - SparseNode::new_leaf(Nibbles::new()) + SparseNode::new_leaf(Nibbles::default()) ), ( Nibbles::from_nibbles([0x5, 0x0, 0x2, 0x3, 0x3]), - SparseNode::new_leaf(Nibbles::new()) + SparseNode::new_leaf(Nibbles::default()) ), (Nibbles::from_nibbles([0x5, 0x3]), SparseNode::new_branch(0b1010.into())), ( @@ -1063,7 +1064,7 @@ mod tests { pretty_assertions::assert_eq!( sparse.nodes.clone().into_iter().collect::>(), BTreeMap::from_iter([ - (Nibbles::new(), SparseNode::new_ext(Nibbles::from_nibbles([0x5]))), + (Nibbles::default(), SparseNode::new_ext(Nibbles::from_nibbles([0x5]))), (Nibbles::from_nibbles([0x5]), SparseNode::new_branch(0b1001.into())), ( Nibbles::from_nibbles([0x5, 0x0]), @@ -1097,7 +1098,7 @@ mod tests { pretty_assertions::assert_eq!( sparse.nodes.clone().into_iter().collect::>(), BTreeMap::from_iter([ - (Nibbles::new(), SparseNode::new_ext(Nibbles::from_nibbles([0x5]))), + (Nibbles::default(), SparseNode::new_ext(Nibbles::from_nibbles([0x5]))), (Nibbles::from_nibbles([0x5]), SparseNode::new_branch(0b1001.into())), ( Nibbles::from_nibbles([0x5, 0x0]), @@ -1128,7 +1129,7 @@ mod tests { pretty_assertions::assert_eq!( sparse.nodes.clone().into_iter().collect::>(), BTreeMap::from_iter([ - (Nibbles::new(), SparseNode::new_ext(Nibbles::from_nibbles([0x5]))), + (Nibbles::default(), SparseNode::new_ext(Nibbles::from_nibbles([0x5]))), (Nibbles::from_nibbles([0x5]), SparseNode::new_branch(0b1001.into())), ( Nibbles::from_nibbles([0x5, 0x0]), @@ -1147,7 +1148,7 @@ mod tests { pretty_assertions::assert_eq!( sparse.nodes.clone().into_iter().collect::>(), BTreeMap::from_iter([( - Nibbles::new(), + Nibbles::default(), SparseNode::new_leaf(Nibbles::from_nibbles([0x5, 0x3, 0x3, 0x0, 0x2])) ),]) ); @@ -1157,7 +1158,42 @@ mod tests { // Empty pretty_assertions::assert_eq!( sparse.nodes.clone().into_iter().collect::>(), - BTreeMap::from_iter([(Nibbles::new(), SparseNode::Empty),]) + BTreeMap::from_iter([(Nibbles::default(), SparseNode::Empty),]) + ); + } + + #[test] + fn sparse_trie_remove_leaf_blinded() { + let mut sparse = RevealedSparseTrie::default(); + + let leaf = LeafNode::new( + Nibbles::default(), + alloy_rlp::encode_fixed_size(&U256::from(1)).to_vec(), + ); + + // Reveal a branch node and one of its children + // + // Branch (Mask = 11) + // ├── 0 -> Hash (Path = 0) + // └── 1 -> Leaf (Path = 1) + sparse + .reveal_node( + Nibbles::default(), + TrieNode::Branch(BranchNode::new( + vec![ + RlpNode::word_rlp(&B256::repeat_byte(1)), + RlpNode::from_raw_rlp(&alloy_rlp::encode(leaf.clone())).unwrap(), + ], + TrieMask::new(0b11), + )), + ) + .unwrap(); + sparse.reveal_node(Nibbles::from_nibbles([0x1]), TrieNode::Leaf(leaf)).unwrap(); + + // Removing a blinded leaf should result in an error + assert_matches!( + sparse.remove_leaf(&Nibbles::from_nibbles([0x0])), + Err(SparseTrieError::BlindedNode { path, hash }) if path == Nibbles::from_nibbles([0x0]) && hash == B256::repeat_byte(1) ); }