Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(trie): use correct store_in_db_trie value for sparse extension nodes #13826

Merged
merged 5 commits into from
Jan 16, 2025
Merged

fix(trie): use correct store_in_db_trie value for sparse extension nodes #13826

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e83bae5
fix(trie): reveal blinded node along with masks in sparse trie
shekhirin Jan 16, 2025
16ccf41
extension node walking logs
shekhirin Jan 16, 2025
84806e7
use correct store in db trie value for extension node
shekhirin Jan 16, 2025
0f131d8
Revert "fix(trie): reveal blinded node along with masks in sparse trie"
shekhirin Jan 16, 2025
f04d67a
add why none
shekhirin Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 27 additions & 12 deletions crates/trie/sparse/src/trie.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ impl<P> RevealedSparseTrie<P> {
// Memoize the hash of a previously blinded node in a new extension
// node.
hash: Some(*hash),
store_in_db_trie: None,
});
self.reveal_node_or_hash(child_path, &ext.child)?;
}
Expand Down Expand Up @@ -602,14 +603,14 @@ impl<P> RevealedSparseTrie<P> {
while let Some((mut path, level)) = paths.pop() {
match self.nodes.get(&path).unwrap() {
SparseNode::Empty | SparseNode::Hash(_) => {}
SparseNode::Leaf { hash, .. } => {
SparseNode::Leaf { key: _, hash } => {
if hash.is_some() && !prefix_set.contains(&path) {
continue
}

targets.push(path);
}
SparseNode::Extension { key, hash } => {
SparseNode::Extension { key, hash, store_in_db_trie: _ } => {
if hash.is_some() && !prefix_set.contains(&path) {
continue
}
Expand All @@ -621,7 +622,7 @@ impl<P> RevealedSparseTrie<P> {
paths.push((path, level + 1));
}
}
SparseNode::Branch { state_mask, hash, .. } => {
SparseNode::Branch { state_mask, hash, store_in_db_trie: _ } => {
if hash.is_some() && !prefix_set.contains(&path) {
continue
}
Expand Down Expand Up @@ -673,26 +674,37 @@ impl<P> RevealedSparseTrie<P> {
(rlp_node, SparseNodeType::Leaf)
}
}
SparseNode::Extension { key, hash } => {
SparseNode::Extension { key, hash, store_in_db_trie } => {
let mut child_path = path.clone();
child_path.extend_from_slice_unchecked(key);
if let Some(hash) = hash.filter(|_| !prefix_set_contains(&path)) {
(
RlpNode::word_rlp(&hash),
SparseNodeType::Extension { store_in_db_trie: true },
)
if let Some((hash, store_in_db_trie)) =
hash.zip(*store_in_db_trie).filter(|_| !prefix_set_contains(&path))
{
(RlpNode::word_rlp(&hash), SparseNodeType::Extension { store_in_db_trie })
} else if buffers.rlp_node_stack.last().is_some_and(|e| e.0 == child_path) {
let (_, child, child_node_type) = buffers.rlp_node_stack.pop().unwrap();
self.rlp_buf.clear();
let rlp_node = ExtensionNodeRef::new(key, &child).rlp(&mut self.rlp_buf);
*hash = rlp_node.as_hash();

let store_in_db_trie_value = child_node_type.store_in_db_trie();

trace!(
target: "trie::sparse",
?path,
?child_path,
?child_node_type,
"Extension node"
);

*store_in_db_trie = Some(store_in_db_trie_value);

(
rlp_node,
SparseNodeType::Extension {
// Inherit the `store_in_db_trie` flag from the child node, which is
// always the branch node
store_in_db_trie: child_node_type.store_in_db_trie(),
store_in_db_trie: store_in_db_trie_value,
},
)
} else {
Expand Down Expand Up @@ -1217,6 +1229,9 @@ pub enum SparseNode {
/// Pre-computed hash of the sparse node.
/// Can be reused unless this trie path has been updated.
hash: Option<B256>,
/// Pre-computed flag indicating whether the trie node should be stored in the database.
/// Can be reused unless this trie path has been updated.
store_in_db_trie: Option<bool>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unclear here how None differs from false?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a comment to all fields

},
/// Sparse branch node with state mask.
Branch {
Expand Down Expand Up @@ -1258,7 +1273,7 @@ impl SparseNode {

/// Create new [`SparseNode::Extension`] from the key slice.
pub const fn new_ext(key: Nibbles) -> Self {
Self::Extension { key, hash: None }
Self::Extension { key, hash: None, store_in_db_trie: None }
}

/// Create new [`SparseNode::Leaf`] from leaf key and value.
Expand Down Expand Up @@ -2296,7 +2311,7 @@ mod tests {
// Check that the root extension node exists
assert_matches!(
sparse.nodes.get(&Nibbles::default()),
Some(SparseNode::Extension { key, hash: None }) if *key == Nibbles::from_nibbles([0x00])
Some(SparseNode::Extension { key, hash: None, store_in_db_trie: None }) if *key == Nibbles::from_nibbles([0x00])
);

// Insert the leaf with a different prefix
Expand Down
Loading