fix(trie): remove branch nodes in sparse trie that shouldn't be stored (#13808)

This commit is contained in:
Alexey Shekhirin
2025-01-16 08:47:17 +00:00
committed by GitHub
parent ac73b52079
commit bbc592c5bf
2 changed files with 33 additions and 22 deletions

View File

@ -246,7 +246,7 @@ impl<F: BlindedProviderFactory> SparseStateTrie<F> {
}; };
trace!(target: "trie::sparse", ?path, ?node, ?hash_mask, ?tree_mask, "Revealing account node"); trace!(target: "trie::sparse", ?path, ?node, ?hash_mask, ?tree_mask, "Revealing account node");
trie.reveal_node(path, node, hash_mask, tree_mask)?; trie.reveal_node(path, node, tree_mask, hash_mask)?;
} }
} }
@ -277,7 +277,7 @@ impl<F: BlindedProviderFactory> SparseStateTrie<F> {
}; };
trace!(target: "trie::sparse", ?account, ?path, ?node, ?hash_mask, ?tree_mask, "Revealing storage node"); trace!(target: "trie::sparse", ?account, ?path, ?node, ?hash_mask, ?tree_mask, "Revealing storage node");
trie.reveal_node(path, node, hash_mask, tree_mask)?; trie.reveal_node(path, node, tree_mask, hash_mask)?;
} }
} }
} }

View File

@ -170,10 +170,10 @@ pub struct RevealedSparseTrie<P = DefaultBlindedProvider> {
provider: P, provider: P,
/// All trie nodes. /// All trie nodes.
nodes: HashMap<Nibbles, SparseNode>, nodes: HashMap<Nibbles, SparseNode>,
/// All branch node hash masks.
branch_node_hash_masks: HashMap<Nibbles, TrieMask>,
/// All branch node tree masks. /// All branch node tree masks.
branch_node_tree_masks: HashMap<Nibbles, TrieMask>, branch_node_tree_masks: HashMap<Nibbles, TrieMask>,
/// All branch node hash masks.
branch_node_hash_masks: HashMap<Nibbles, TrieMask>,
/// All leaf values. /// All leaf values.
values: HashMap<Nibbles, Vec<u8>>, values: HashMap<Nibbles, Vec<u8>>,
/// Prefix set. /// Prefix set.
@ -188,8 +188,8 @@ impl<P> fmt::Debug for RevealedSparseTrie<P> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("RevealedSparseTrie") f.debug_struct("RevealedSparseTrie")
.field("nodes", &self.nodes) .field("nodes", &self.nodes)
.field("branch_hash_masks", &self.branch_node_hash_masks)
.field("branch_tree_masks", &self.branch_node_tree_masks) .field("branch_tree_masks", &self.branch_node_tree_masks)
.field("branch_hash_masks", &self.branch_node_hash_masks)
.field("values", &self.values) .field("values", &self.values)
.field("prefix_set", &self.prefix_set) .field("prefix_set", &self.prefix_set)
.field("updates", &self.updates) .field("updates", &self.updates)
@ -203,8 +203,8 @@ impl Default for RevealedSparseTrie {
Self { Self {
provider: Default::default(), provider: Default::default(),
nodes: HashMap::from_iter([(Nibbles::default(), SparseNode::Empty)]), nodes: HashMap::from_iter([(Nibbles::default(), SparseNode::Empty)]),
branch_node_hash_masks: HashMap::default(),
branch_node_tree_masks: HashMap::default(), branch_node_tree_masks: HashMap::default(),
branch_node_hash_masks: HashMap::default(),
values: HashMap::default(), values: HashMap::default(),
prefix_set: PrefixSetMut::default(), prefix_set: PrefixSetMut::default(),
updates: None, updates: None,
@ -224,15 +224,15 @@ impl RevealedSparseTrie {
let mut this = Self { let mut this = Self {
provider: Default::default(), provider: Default::default(),
nodes: HashMap::default(), nodes: HashMap::default(),
branch_node_hash_masks: HashMap::default(),
branch_node_tree_masks: HashMap::default(), branch_node_tree_masks: HashMap::default(),
branch_node_hash_masks: HashMap::default(),
values: HashMap::default(), values: HashMap::default(),
prefix_set: PrefixSetMut::default(), prefix_set: PrefixSetMut::default(),
rlp_buf: Vec::new(), rlp_buf: Vec::new(),
updates: None, updates: None,
} }
.with_updates(retain_updates); .with_updates(retain_updates);
this.reveal_node(Nibbles::default(), node, hash_mask, tree_mask)?; this.reveal_node(Nibbles::default(), node, tree_mask, hash_mask)?;
Ok(this) Ok(this)
} }
} }
@ -249,15 +249,15 @@ impl<P> RevealedSparseTrie<P> {
let mut this = Self { let mut this = Self {
provider, provider,
nodes: HashMap::default(), nodes: HashMap::default(),
branch_node_hash_masks: HashMap::default(),
branch_node_tree_masks: HashMap::default(), branch_node_tree_masks: HashMap::default(),
branch_node_hash_masks: HashMap::default(),
values: HashMap::default(), values: HashMap::default(),
prefix_set: PrefixSetMut::default(), prefix_set: PrefixSetMut::default(),
rlp_buf: Vec::new(), rlp_buf: Vec::new(),
updates: None, updates: None,
} }
.with_updates(retain_updates); .with_updates(retain_updates);
this.reveal_node(Nibbles::default(), node, hash_mask, tree_mask)?; this.reveal_node(Nibbles::default(), node, tree_mask, hash_mask)?;
Ok(this) Ok(this)
} }
@ -266,8 +266,8 @@ impl<P> RevealedSparseTrie<P> {
RevealedSparseTrie { RevealedSparseTrie {
provider, provider,
nodes: self.nodes, nodes: self.nodes,
branch_node_hash_masks: self.branch_node_hash_masks,
branch_node_tree_masks: self.branch_node_tree_masks, branch_node_tree_masks: self.branch_node_tree_masks,
branch_node_hash_masks: self.branch_node_hash_masks,
values: self.values, values: self.values,
prefix_set: self.prefix_set, prefix_set: self.prefix_set,
updates: self.updates, updates: self.updates,
@ -303,20 +303,20 @@ impl<P> RevealedSparseTrie<P> {
&mut self, &mut self,
path: Nibbles, path: Nibbles,
node: TrieNode, node: TrieNode,
hash_mask: Option<TrieMask>,
tree_mask: Option<TrieMask>, tree_mask: Option<TrieMask>,
hash_mask: Option<TrieMask>,
) -> SparseTrieResult<()> { ) -> SparseTrieResult<()> {
// If the node is already revealed and it's not a hash node, do nothing. // If the node is already revealed and it's not a hash node, do nothing.
if self.nodes.get(&path).is_some_and(|node| !node.is_hash()) { if self.nodes.get(&path).is_some_and(|node| !node.is_hash()) {
return Ok(()) return Ok(())
} }
if let Some(hash_mask) = hash_mask {
self.branch_node_hash_masks.insert(path.clone(), hash_mask);
}
if let Some(tree_mask) = tree_mask { if let Some(tree_mask) = tree_mask {
self.branch_node_tree_masks.insert(path.clone(), tree_mask); self.branch_node_tree_masks.insert(path.clone(), tree_mask);
} }
if let Some(hash_mask) = hash_mask {
self.branch_node_hash_masks.insert(path.clone(), hash_mask);
}
match node { match node {
TrieNode::EmptyRoot => { TrieNode::EmptyRoot => {
@ -807,10 +807,10 @@ impl<P> RevealedSparseTrie<P> {
let store_in_db_trie_value = if let Some(updates) = let store_in_db_trie_value = if let Some(updates) =
self.updates.as_mut().filter(|_| retain_updates && !path.is_empty()) self.updates.as_mut().filter(|_| retain_updates && !path.is_empty())
{ {
// Store in DB trie if there are either any children that are stored in the
// DB trie, or any children represent hashed values
let store_in_db_trie = !tree_mask.is_empty() || !hash_mask.is_empty(); let store_in_db_trie = !tree_mask.is_empty() || !hash_mask.is_empty();
if store_in_db_trie { if store_in_db_trie {
// Store in DB trie if there are either any children that are stored in
// the DB trie, or any children represent hashed values
hashes.reverse(); hashes.reverse();
let branch_node = BranchNodeCompact::new( let branch_node = BranchNodeCompact::new(
*state_mask, *state_mask,
@ -820,6 +820,17 @@ impl<P> RevealedSparseTrie<P> {
hash.filter(|_| path.is_empty()), hash.filter(|_| path.is_empty()),
); );
updates.updated_nodes.insert(path.clone(), branch_node); updates.updated_nodes.insert(path.clone(), branch_node);
} else if self
.branch_node_tree_masks
.get(&path)
.is_some_and(|mask| !mask.is_empty()) ||
self.branch_node_hash_masks
.get(&path)
.is_some_and(|mask| !mask.is_empty())
{
// If new tree and hash masks are empty, but previously they weren't, we
// need to remove the node.
updates.removed_nodes.insert(path.clone());
} }
store_in_db_trie store_in_db_trie
@ -2111,7 +2122,7 @@ mod tests {
let hash_mask = branch_node_hash_masks.get(&path).copied(); let hash_mask = branch_node_hash_masks.get(&path).copied();
let tree_mask = branch_node_tree_masks.get(&path).copied(); let tree_mask = branch_node_tree_masks.get(&path).copied();
sparse sparse
.reveal_node(path, TrieNode::decode(&mut &node[..]).unwrap(), hash_mask, tree_mask) .reveal_node(path, TrieNode::decode(&mut &node[..]).unwrap(), tree_mask, hash_mask)
.unwrap(); .unwrap();
} }
@ -2137,7 +2148,7 @@ mod tests {
let hash_mask = branch_node_hash_masks.get(&path).copied(); let hash_mask = branch_node_hash_masks.get(&path).copied();
let tree_mask = branch_node_tree_masks.get(&path).copied(); let tree_mask = branch_node_tree_masks.get(&path).copied();
sparse sparse
.reveal_node(path, TrieNode::decode(&mut &node[..]).unwrap(), hash_mask, tree_mask) .reveal_node(path, TrieNode::decode(&mut &node[..]).unwrap(), tree_mask, hash_mask)
.unwrap(); .unwrap();
} }
@ -2202,7 +2213,7 @@ mod tests {
let hash_mask = branch_node_hash_masks.get(&path).copied(); let hash_mask = branch_node_hash_masks.get(&path).copied();
let tree_mask = branch_node_tree_masks.get(&path).copied(); let tree_mask = branch_node_tree_masks.get(&path).copied();
sparse sparse
.reveal_node(path, TrieNode::decode(&mut &node[..]).unwrap(), hash_mask, tree_mask) .reveal_node(path, TrieNode::decode(&mut &node[..]).unwrap(), tree_mask, hash_mask)
.unwrap(); .unwrap();
} }
@ -2232,7 +2243,7 @@ mod tests {
let hash_mask = branch_node_hash_masks.get(&path).copied(); let hash_mask = branch_node_hash_masks.get(&path).copied();
let tree_mask = branch_node_tree_masks.get(&path).copied(); let tree_mask = branch_node_tree_masks.get(&path).copied();
sparse sparse
.reveal_node(path, TrieNode::decode(&mut &node[..]).unwrap(), hash_mask, tree_mask) .reveal_node(path, TrieNode::decode(&mut &node[..]).unwrap(), tree_mask, hash_mask)
.unwrap(); .unwrap();
} }
@ -2300,7 +2311,7 @@ mod tests {
let hash_mask = branch_node_hash_masks.get(&path).copied(); let hash_mask = branch_node_hash_masks.get(&path).copied();
let tree_mask = branch_node_tree_masks.get(&path).copied(); let tree_mask = branch_node_tree_masks.get(&path).copied();
sparse sparse
.reveal_node(path, TrieNode::decode(&mut &node[..]).unwrap(), hash_mask, tree_mask) .reveal_node(path, TrieNode::decode(&mut &node[..]).unwrap(), tree_mask, hash_mask)
.unwrap(); .unwrap();
} }