Skip to content

fix(trie): Reveal extension child when extension is last remaining child of a branch #18891

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

Merged
mediocregopher merged 4 commits into from
Oct 8, 2025
Merged

fix(trie): Reveal extension child when extension is last remaining child of a branch #18891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c00a113
fix(trie): reveal extension node children on leaf removal
mediocregopher Oct 7, 2025
fc4ff99
serial sparse trie fix
mediocregopher Oct 7, 2025
36f650b
Test
mediocregopher Oct 7, 2025
aff3886
More comments
mediocregopher Oct 8, 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
309 changes: 270 additions & 39 deletions crates/trie/sparse-parallel/src/trie.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,51 +623,19 @@ impl SparseTrieInterface for ParallelSparseTrie {
"Branch node has only one child",
);

let remaining_child_subtrie = self.subtrie_for_path_mut(&remaining_child_path);

// If the remaining child node is not yet revealed then we have to reveal it here,
// otherwise it's not possible to know how to collapse the branch.
let remaining_child_node =
match remaining_child_subtrie.nodes.get(&remaining_child_path).unwrap() {
SparseNode::Hash(_) => {
debug!(
target: "trie::parallel_sparse",
child_path = ?remaining_child_path,
leaf_full_path = ?full_path,
"Branch node child not revealed in remove_leaf, falling back to db",
);
if let Some(RevealedNode { node, tree_mask, hash_mask }) =
provider.trie_node(&remaining_child_path)?
{
let decoded = TrieNode::decode(&mut &node[..])?;
trace!(
target: "trie::parallel_sparse",
?remaining_child_path,
?decoded,
?tree_mask,
?hash_mask,
"Revealing remaining blinded branch child"
);
remaining_child_subtrie.reveal_node(
remaining_child_path,
&decoded,
TrieMasks { hash_mask, tree_mask },
)?;
remaining_child_subtrie.nodes.get(&remaining_child_path).unwrap()
} else {
return Err(SparseTrieErrorKind::NodeNotFoundInProvider {
path: remaining_child_path,
}
.into())
}
}
node => node,
};
let remaining_child_node = self.reveal_remaining_child_on_leaf_removal(
provider,
full_path,
&remaining_child_path,
true, // recurse_into_extension
)?;

let (new_branch_node, remove_child) = Self::branch_changes_on_leaf_removal(
branch_path,
&remaining_child_path,
remaining_child_node,
&remaining_child_node,
);

if remove_child {
Expand Down Expand Up @@ -1228,6 +1196,90 @@ impl ParallelSparseTrie {
}
}

/// Called when a leaf is removed on a branch which has only one other remaining child. That
/// child must be revealed in order to properly collapse the branch.
///
/// If `recurse_into_extension` is true, and the remaining child is an extension node, then its
/// child will be ensured to be revealed as well.
///
/// ## Returns
///
/// The node of the remaining child, whether it was already revealed or not.
fn reveal_remaining_child_on_leaf_removal<P: TrieNodeProvider>(
&mut self,
provider: P,
full_path: &Nibbles, // only needed for logs
remaining_child_path: &Nibbles,
recurse_into_extension: bool,
) -> SparseTrieResult<SparseNode> {
let remaining_child_subtrie = self.subtrie_for_path_mut(remaining_child_path);
Comment on lines +1199 to +1215
Copy link
Copy Markdown
Contributor

@yongkangc yongkangc Oct 8, 2025

Choose a reason for hiding this comment

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

much clearer logic


let remaining_child_node =
match remaining_child_subtrie.nodes.get(remaining_child_path).unwrap() {
SparseNode::Hash(_) => {
debug!(
target: "trie::parallel_sparse",
child_path = ?remaining_child_path,
leaf_full_path = ?full_path,
"Node child not revealed in remove_leaf, falling back to db",
);
if let Some(RevealedNode { node, tree_mask, hash_mask }) =
provider.trie_node(remaining_child_path)?
{
let decoded = TrieNode::decode(&mut &node[..])?;
trace!(
target: "trie::parallel_sparse",
?remaining_child_path,
?decoded,
?tree_mask,
?hash_mask,
"Revealing remaining blinded branch child"
);
remaining_child_subtrie.reveal_node(
*remaining_child_path,
&decoded,
TrieMasks { hash_mask, tree_mask },
)?;
remaining_child_subtrie.nodes.get(remaining_child_path).unwrap().clone()
} else {
return Err(SparseTrieErrorKind::NodeNotFoundInProvider {
path: *remaining_child_path,
}
.into())
}
}
node => node.clone(),
};

// If `recurse_into_extension` is true, and the remaining child is an extension node, then
// its child will be ensured to be revealed as well. This is required for generation of
// trie updates; without revealing the grandchild branch it's not always possible to know
// if the tree mask bit should be set for the child extension on its parent branch.
if let SparseNode::Extension { key, .. } = &remaining_child_node &&
recurse_into_extension
{
let mut remaining_grandchild_path = *remaining_child_path;
remaining_grandchild_path.extend(key);

trace!(
target: "trie::parallel_sparse",
remaining_grandchild_path = ?remaining_grandchild_path,
child_path = ?remaining_child_path,
leaf_full_path = ?full_path,
"Revealing child of extension node, which is the last remaining child of the branch"
);

self.reveal_remaining_child_on_leaf_removal(
provider,
full_path,
&remaining_grandchild_path,
false, // recurse_into_extension
)?;
}

Ok(remaining_child_node)
}

/// Drains any [`SparseTrieUpdatesAction`]s from the given subtrie, and applies each action to
/// the given `updates` set. If the given set is None then this is a no-op.
fn apply_subtrie_update_actions(
Expand Down Expand Up @@ -4076,6 +4128,185 @@ mod tests {
);
}

#[test]
fn test_remove_leaf_remaining_extension_node_child_is_revealed() {
let branch_path = Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7]);
let removed_branch_path = Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7, 0x2]);

// Convert the logs into reveal_nodes call on a fresh ParallelSparseTrie
let nodes = vec![
// Branch at 0x4f8807
RevealedSparseNode {
path: branch_path,
node: {
TrieNode::Branch(BranchNode::new(
vec![
RlpNode::word_rlp(&B256::from(hex!(
"dede882d52f0e0eddfb5b89293a10c87468b4a73acd0d4ae550054a92353f6d5"
))),
RlpNode::word_rlp(&B256::from(hex!(
"8746f18e465e2eed16117306b6f2eef30bc9d2978aee4a7838255e39c41a3222"
))),
RlpNode::word_rlp(&B256::from(hex!(
"35a4ea861548af5f0262a9b6d619b4fc88fce6531cbd004eab1530a73f34bbb1"
))),
RlpNode::word_rlp(&B256::from(hex!(
"47d5c2bf9eea5c1ee027e4740c2b86159074a27d52fd2f6a8a8c86c77e48006f"
))),
RlpNode::word_rlp(&B256::from(hex!(
"eb76a359b216e1d86b1f2803692a9fe8c3d3f97a9fe6a82b396e30344febc0c1"
))),
RlpNode::word_rlp(&B256::from(hex!(
"437656f2697f167b23e33cb94acc8550128cfd647fc1579d61e982cb7616b8bc"
))),
RlpNode::word_rlp(&B256::from(hex!(
"45a1ac2faf15ea8a4da6f921475974e0379f39c3d08166242255a567fa88ce6c"
))),
RlpNode::word_rlp(&B256::from(hex!(
"7dbb299d714d3dfa593f53bc1b8c66d5c401c30a0b5587b01254a56330361395"
))),
RlpNode::word_rlp(&B256::from(hex!(
"ae407eb14a74ed951c9949c1867fb9ee9ba5d5b7e03769eaf3f29c687d080429"
))),
RlpNode::word_rlp(&B256::from(hex!(
"768d0fe1003f0e85d3bc76e4a1fa0827f63b10ca9bca52d56c2b1cceb8eb8b08"
))),
RlpNode::word_rlp(&B256::from(hex!(
"e5127935143493d5094f4da6e4f7f5a0f62d524fbb61e7bb9fb63d8a166db0f3"
))),
RlpNode::word_rlp(&B256::from(hex!(
"7f3698297308664fbc1b9e2c41d097fbd57d8f364c394f6ad7c71b10291fbf42"
))),
RlpNode::word_rlp(&B256::from(hex!(
"4a2bc7e19cec63cb5ef5754add0208959b50bcc79f13a22a370f77b277dbe6db"
))),
RlpNode::word_rlp(&B256::from(hex!(
"40764b8c48de59258e62a3371909a107e76e1b5e847cfa94dbc857e9fd205103"
))),
RlpNode::word_rlp(&B256::from(hex!(
"2985dca29a7616920d95c43ab62eb013a40e6a0c88c284471e4c3bd22f3b9b25"
))),
RlpNode::word_rlp(&B256::from(hex!(
"1b6511f7a385e79477239f7dd4a49f52082ecac05aa5bd0de18b1d55fe69d10c"
))),
],
TrieMask::new(0b1111111111111111),
))
},
masks: TrieMasks {
hash_mask: Some(TrieMask::new(0b1111111111111111)),
tree_mask: Some(TrieMask::new(0b0011110100100101)),
},
},
// Branch at 0x4f88072
RevealedSparseNode {
path: removed_branch_path,
node: {
let stack = vec![
RlpNode::word_rlp(&B256::from(hex!(
"15fd4993a41feff1af3b629b32572ab05acddd97c681d82ec2eb89c8a8e3ab9e"
))),
RlpNode::word_rlp(&B256::from(hex!(
"a272b0b94ced4e6ec7adb41719850cf4a167ad8711d0dda6a810d129258a0d94"
))),
];
let branch_node = BranchNode::new(stack, TrieMask::new(0b0001000000000100));
TrieNode::Branch(branch_node)
},
masks: TrieMasks {
hash_mask: Some(TrieMask::new(0b0000000000000000)),
tree_mask: Some(TrieMask::new(0b0000000000000100)),
},
},
// Extension at 0x4f880722
RevealedSparseNode {
path: Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7, 0x2, 0x2]),
node: {
let extension_node = ExtensionNode::new(
Nibbles::from_nibbles([0x6]),
RlpNode::word_rlp(&B256::from(hex!(
"56fab2b106a97eae9c7197f86d03bca292da6e0ac725b783082f7d950cc4e0fc"
))),
);
TrieNode::Extension(extension_node)
},
masks: TrieMasks { hash_mask: None, tree_mask: None },
},
// Leaf at 0x4f88072c
RevealedSparseNode {
path: Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7, 0x2, 0xc]),
node: {
let leaf_node = LeafNode::new(
Nibbles::from_nibbles([
0x0, 0x7, 0x7, 0xf, 0x8, 0x6, 0x6, 0x1, 0x3, 0x0, 0x8, 0x8, 0xd, 0xf,
0xc, 0xa, 0xe, 0x6, 0x4, 0x8, 0xa, 0xb, 0xe, 0x8, 0x3, 0x1, 0xf, 0xa,
0xd, 0xc, 0xa, 0x5, 0x5, 0xa, 0xd, 0x4, 0x3, 0xa, 0xb, 0x1, 0x6, 0x5,
0xd, 0x1, 0x6, 0x8, 0x0, 0xd, 0xd, 0x5, 0x6, 0x7, 0xb, 0x5, 0xd, 0x6,
]),
hex::decode("8468d3971d").unwrap(),
);
TrieNode::Leaf(leaf_node)
},
masks: TrieMasks { hash_mask: None, tree_mask: None },
},
];

// Create a fresh ParallelSparseTrie
let mut trie = ParallelSparseTrie::from_root(
TrieNode::Extension(ExtensionNode::new(
Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7]),
RlpNode::word_rlp(&B256::from(hex!(
"56fab2b106a97eae9c7197f86d03bca292da6e0ac725b783082f7d950cc4e0fc"
))),
)),
TrieMasks::none(),
true,
)
.unwrap();

// Call reveal_nodes
trie.reveal_nodes(nodes).unwrap();

// Remove the leaf at "0x4f88072c077f86613088dfcae648abe831fadca55ad43ab165d1680dd567b5d6"
let leaf_key = Nibbles::from_nibbles([
0x4, 0xf, 0x8, 0x8, 0x0, 0x7, 0x2, 0xc, 0x0, 0x7, 0x7, 0xf, 0x8, 0x6, 0x6, 0x1, 0x3,
0x0, 0x8, 0x8, 0xd, 0xf, 0xc, 0xa, 0xe, 0x6, 0x4, 0x8, 0xa, 0xb, 0xe, 0x8, 0x3, 0x1,
0xf, 0xa, 0xd, 0xc, 0xa, 0x5, 0x5, 0xa, 0xd, 0x4, 0x3, 0xa, 0xb, 0x1, 0x6, 0x5, 0xd,
0x1, 0x6, 0x8, 0x0, 0xd, 0xd, 0x5, 0x6, 0x7, 0xb, 0x5, 0xd, 0x6,
]);

let mut provider = MockTrieNodeProvider::new();
let revealed_branch = create_branch_node_with_children(&[], []);
let mut encoded = Vec::new();
revealed_branch.encode(&mut encoded);
provider.add_revealed_node(
Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7, 0x2, 0x2, 0x6]),
RevealedNode {
node: encoded.into(),
tree_mask: None,
// Give it a fake hashmask so that it appears like it will be stored in the db
hash_mask: Some(TrieMask::new(0b1111)),
},
);

trie.remove_leaf(&leaf_key, provider).unwrap();

// Calculate root so that updates are calculated.
trie.root();

// Take updates and assert they are correct
let updates = trie.take_updates();
assert_eq!(
updates.removed_nodes.into_iter().collect::<Vec<_>>(),
vec![removed_branch_path]
);
assert_eq!(updates.updated_nodes.len(), 1);
let updated_node = updates.updated_nodes.get(&branch_path).unwrap();

// Second bit must be set, indicating that the extension's child is in the db
assert_eq!(updated_node.tree_mask, TrieMask::new(0b011110100100101),)
}

#[test]
fn test_parallel_sparse_trie_root() {
// Step 1: Create the trie structure
Expand Down
Loading
Loading