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): retain updates only for sparse branch nodes in the prefix set #13234

Merged
merged 1 commit into from
Dec 9, 2024
+3 −2
Merged

fix(trie): retain updates only for sparse branch nodes in the prefix set #13234

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions crates/trie/sparse/src/trie.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,7 @@ impl<P> RevealedSparseTrie<P> {
));
continue
}
let retain_updates = self.updates.is_some() && prefix_set_contains(&path);
Copy link
Member

Choose a reason for hiding this comment

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

prefix_set_contains might be called twice (see above). let's assign to var

Copy link
Collaborator Author

@shekhirin shekhirin Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

the value is already memoized, so we actually should call prefix_set_contains twice to avoid doingprefix_set.contains twice


buffers.branch_child_buf.clear();
// Walk children in a reverse order from `f` to `0`, so we pop the `0` first
Expand Down Expand Up @@ -650,7 +651,7 @@ impl<P> RevealedSparseTrie<P> {
buffers.rlp_node_stack.pop().unwrap();

// Update the masks only if we need to retain trie updates
if self.updates.is_some() {
if retain_updates {
// Set the trie mask
let tree_mask_value = if node_type.store_in_db_trie() {
// A branch or an extension node explicitly set the
Expand Down Expand Up @@ -716,7 +717,7 @@ impl<P> RevealedSparseTrie<P> {
// Save a branch node update only if it's not a root node, and we need to
// persist updates.
let store_in_db_trie_value = if let Some(updates) =
self.updates.as_mut().filter(|_| !path.is_empty())
self.updates.as_mut().filter(|_| retain_updates && !path.is_empty())
{
let mut tree_mask_values = tree_mask_values.into_iter().rev();
let mut hash_mask_values = hash_mask_values.into_iter().rev();
Expand Down
Loading