Skip to content

Commit 273ee08

Browse files
mediocregophermediocregopher
authored
fix(trie): Reveal extension child when extension is last remaining child of a branch (#18891)
1 parent b82ad07 commit 273ee08

File tree

2 files changed

+360
-69
lines changed

2 files changed

+360
-69
lines changed

‎crates/trie/sparse-parallel/src/trie.rs‎

Lines changed: 270 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -623,51 +623,19 @@ impl SparseTrieInterface for ParallelSparseTrie {
623623
"Branch node has only one child",
624624
);
625625

626-
let remaining_child_subtrie = self.subtrie_for_path_mut(&remaining_child_path);
627-
628626
// If the remaining child node is not yet revealed then we have to reveal it here,
629627
// otherwise it's not possible to know how to collapse the branch.
630-
let remaining_child_node =
631-
match remaining_child_subtrie.nodes.get(&remaining_child_path).unwrap() {
632-
SparseNode::Hash(_) => {
633-
debug!(
634-
target: "trie::parallel_sparse",
635-
child_path = ?remaining_child_path,
636-
leaf_full_path = ?full_path,
637-
"Branch node child not revealed in remove_leaf, falling back to db",
638-
);
639-
if let Some(RevealedNode { node, tree_mask, hash_mask }) =
640-
provider.trie_node(&remaining_child_path)?
641-
{
642-
let decoded = TrieNode::decode(&mut &node[..])?;
643-
trace!(
644-
target: "trie::parallel_sparse",
645-
?remaining_child_path,
646-
?decoded,
647-
?tree_mask,
648-
?hash_mask,
649-
"Revealing remaining blinded branch child"
650-
);
651-
remaining_child_subtrie.reveal_node(
652-
remaining_child_path,
653-
&decoded,
654-
TrieMasks { hash_mask, tree_mask },
655-
)?;
656-
remaining_child_subtrie.nodes.get(&remaining_child_path).unwrap()
657-
} else {
658-
return Err(SparseTrieErrorKind::NodeNotFoundInProvider {
659-
path: remaining_child_path,
660-
}
661-
.into())
662-
}
663-
}
664-
node => node,
665-
};
628+
let remaining_child_node = self.reveal_remaining_child_on_leaf_removal(
629+
provider,
630+
full_path,
631+
&remaining_child_path,
632+
true, // recurse_into_extension
633+
)?;
666634

667635
let (new_branch_node, remove_child) = Self::branch_changes_on_leaf_removal(
668636
branch_path,
669637
&remaining_child_path,
670-
remaining_child_node,
638+
&remaining_child_node,
671639
);
672640

673641
if remove_child {
@@ -1228,6 +1196,90 @@ impl ParallelSparseTrie {
12281196
}
12291197
}
12301198

1199+
/// Called when a leaf is removed on a branch which has only one other remaining child. That
1200+
/// child must be revealed in order to properly collapse the branch.
1201+
///
1202+
/// If `recurse_into_extension` is true, and the remaining child is an extension node, then its
1203+
/// child will be ensured to be revealed as well.
1204+
///
1205+
/// ## Returns
1206+
///
1207+
/// The node of the remaining child, whether it was already revealed or not.
1208+
fn reveal_remaining_child_on_leaf_removal<P: TrieNodeProvider>(
1209+
&mut self,
1210+
provider: P,
1211+
full_path: &Nibbles, // only needed for logs
1212+
remaining_child_path: &Nibbles,
1213+
recurse_into_extension: bool,
1214+
) -> SparseTrieResult<SparseNode> {
1215+
let remaining_child_subtrie = self.subtrie_for_path_mut(remaining_child_path);
1216+
1217+
let remaining_child_node =
1218+
match remaining_child_subtrie.nodes.get(remaining_child_path).unwrap() {
1219+
SparseNode::Hash(_) => {
1220+
debug!(
1221+
target: "trie::parallel_sparse",
1222+
child_path = ?remaining_child_path,
1223+
leaf_full_path = ?full_path,
1224+
"Node child not revealed in remove_leaf, falling back to db",
1225+
);
1226+
if let Some(RevealedNode { node, tree_mask, hash_mask }) =
1227+
provider.trie_node(remaining_child_path)?
1228+
{
1229+
let decoded = TrieNode::decode(&mut &node[..])?;
1230+
trace!(
1231+
target: "trie::parallel_sparse",
1232+
?remaining_child_path,
1233+
?decoded,
1234+
?tree_mask,
1235+
?hash_mask,
1236+
"Revealing remaining blinded branch child"
1237+
);
1238+
remaining_child_subtrie.reveal_node(
1239+
*remaining_child_path,
1240+
&decoded,
1241+
TrieMasks { hash_mask, tree_mask },
1242+
)?;
1243+
remaining_child_subtrie.nodes.get(remaining_child_path).unwrap().clone()
1244+
} else {
1245+
return Err(SparseTrieErrorKind::NodeNotFoundInProvider {
1246+
path: *remaining_child_path,
1247+
}
1248+
.into())
1249+
}
1250+
}
1251+
node => node.clone(),
1252+
};
1253+
1254+
// If `recurse_into_extension` is true, and the remaining child is an extension node, then
1255+
// its child will be ensured to be revealed as well. This is required for generation of
1256+
// trie updates; without revealing the grandchild branch it's not always possible to know
1257+
// if the tree mask bit should be set for the child extension on its parent branch.
1258+
if let SparseNode::Extension { key, .. } = &remaining_child_node &&
1259+
recurse_into_extension
1260+
{
1261+
let mut remaining_grandchild_path = *remaining_child_path;
1262+
remaining_grandchild_path.extend(key);
1263+
1264+
trace!(
1265+
target: "trie::parallel_sparse",
1266+
remaining_grandchild_path = ?remaining_grandchild_path,
1267+
child_path = ?remaining_child_path,
1268+
leaf_full_path = ?full_path,
1269+
"Revealing child of extension node, which is the last remaining child of the branch"
1270+
);
1271+
1272+
self.reveal_remaining_child_on_leaf_removal(
1273+
provider,
1274+
full_path,
1275+
&remaining_grandchild_path,
1276+
false, // recurse_into_extension
1277+
)?;
1278+
}
1279+
1280+
Ok(remaining_child_node)
1281+
}
1282+
12311283
/// Drains any [`SparseTrieUpdatesAction`]s from the given subtrie, and applies each action to
12321284
/// the given `updates` set. If the given set is None then this is a no-op.
12331285
fn apply_subtrie_update_actions(
@@ -4076,6 +4128,185 @@ mod tests {
40764128
);
40774129
}
40784130

4131+
#[test]
4132+
fn test_remove_leaf_remaining_extension_node_child_is_revealed() {
4133+
let branch_path = Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7]);
4134+
let removed_branch_path = Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7, 0x2]);
4135+
4136+
// Convert the logs into reveal_nodes call on a fresh ParallelSparseTrie
4137+
let nodes = vec![
4138+
// Branch at 0x4f8807
4139+
RevealedSparseNode {
4140+
path: branch_path,
4141+
node: {
4142+
TrieNode::Branch(BranchNode::new(
4143+
vec![
4144+
RlpNode::word_rlp(&B256::from(hex!(
4145+
"dede882d52f0e0eddfb5b89293a10c87468b4a73acd0d4ae550054a92353f6d5"
4146+
))),
4147+
RlpNode::word_rlp(&B256::from(hex!(
4148+
"8746f18e465e2eed16117306b6f2eef30bc9d2978aee4a7838255e39c41a3222"
4149+
))),
4150+
RlpNode::word_rlp(&B256::from(hex!(
4151+
"35a4ea861548af5f0262a9b6d619b4fc88fce6531cbd004eab1530a73f34bbb1"
4152+
))),
4153+
RlpNode::word_rlp(&B256::from(hex!(
4154+
"47d5c2bf9eea5c1ee027e4740c2b86159074a27d52fd2f6a8a8c86c77e48006f"
4155+
))),
4156+
RlpNode::word_rlp(&B256::from(hex!(
4157+
"eb76a359b216e1d86b1f2803692a9fe8c3d3f97a9fe6a82b396e30344febc0c1"
4158+
))),
4159+
RlpNode::word_rlp(&B256::from(hex!(
4160+
"437656f2697f167b23e33cb94acc8550128cfd647fc1579d61e982cb7616b8bc"
4161+
))),
4162+
RlpNode::word_rlp(&B256::from(hex!(
4163+
"45a1ac2faf15ea8a4da6f921475974e0379f39c3d08166242255a567fa88ce6c"
4164+
))),
4165+
RlpNode::word_rlp(&B256::from(hex!(
4166+
"7dbb299d714d3dfa593f53bc1b8c66d5c401c30a0b5587b01254a56330361395"
4167+
))),
4168+
RlpNode::word_rlp(&B256::from(hex!(
4169+
"ae407eb14a74ed951c9949c1867fb9ee9ba5d5b7e03769eaf3f29c687d080429"
4170+
))),
4171+
RlpNode::word_rlp(&B256::from(hex!(
4172+
"768d0fe1003f0e85d3bc76e4a1fa0827f63b10ca9bca52d56c2b1cceb8eb8b08"
4173+
))),
4174+
RlpNode::word_rlp(&B256::from(hex!(
4175+
"e5127935143493d5094f4da6e4f7f5a0f62d524fbb61e7bb9fb63d8a166db0f3"
4176+
))),
4177+
RlpNode::word_rlp(&B256::from(hex!(
4178+
"7f3698297308664fbc1b9e2c41d097fbd57d8f364c394f6ad7c71b10291fbf42"
4179+
))),
4180+
RlpNode::word_rlp(&B256::from(hex!(
4181+
"4a2bc7e19cec63cb5ef5754add0208959b50bcc79f13a22a370f77b277dbe6db"
4182+
))),
4183+
RlpNode::word_rlp(&B256::from(hex!(
4184+
"40764b8c48de59258e62a3371909a107e76e1b5e847cfa94dbc857e9fd205103"
4185+
))),
4186+
RlpNode::word_rlp(&B256::from(hex!(
4187+
"2985dca29a7616920d95c43ab62eb013a40e6a0c88c284471e4c3bd22f3b9b25"
4188+
))),
4189+
RlpNode::word_rlp(&B256::from(hex!(
4190+
"1b6511f7a385e79477239f7dd4a49f52082ecac05aa5bd0de18b1d55fe69d10c"
4191+
))),
4192+
],
4193+
TrieMask::new(0b1111111111111111),
4194+
))
4195+
},
4196+
masks: TrieMasks {
4197+
hash_mask: Some(TrieMask::new(0b1111111111111111)),
4198+
tree_mask: Some(TrieMask::new(0b0011110100100101)),
4199+
},
4200+
},
4201+
// Branch at 0x4f88072
4202+
RevealedSparseNode {
4203+
path: removed_branch_path,
4204+
node: {
4205+
let stack = vec![
4206+
RlpNode::word_rlp(&B256::from(hex!(
4207+
"15fd4993a41feff1af3b629b32572ab05acddd97c681d82ec2eb89c8a8e3ab9e"
4208+
))),
4209+
RlpNode::word_rlp(&B256::from(hex!(
4210+
"a272b0b94ced4e6ec7adb41719850cf4a167ad8711d0dda6a810d129258a0d94"
4211+
))),
4212+
];
4213+
let branch_node = BranchNode::new(stack, TrieMask::new(0b0001000000000100));
4214+
TrieNode::Branch(branch_node)
4215+
},
4216+
masks: TrieMasks {
4217+
hash_mask: Some(TrieMask::new(0b0000000000000000)),
4218+
tree_mask: Some(TrieMask::new(0b0000000000000100)),
4219+
},
4220+
},
4221+
// Extension at 0x4f880722
4222+
RevealedSparseNode {
4223+
path: Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7, 0x2, 0x2]),
4224+
node: {
4225+
let extension_node = ExtensionNode::new(
4226+
Nibbles::from_nibbles([0x6]),
4227+
RlpNode::word_rlp(&B256::from(hex!(
4228+
"56fab2b106a97eae9c7197f86d03bca292da6e0ac725b783082f7d950cc4e0fc"
4229+
))),
4230+
);
4231+
TrieNode::Extension(extension_node)
4232+
},
4233+
masks: TrieMasks { hash_mask: None, tree_mask: None },
4234+
},
4235+
// Leaf at 0x4f88072c
4236+
RevealedSparseNode {
4237+
path: Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7, 0x2, 0xc]),
4238+
node: {
4239+
let leaf_node = LeafNode::new(
4240+
Nibbles::from_nibbles([
4241+
0x0, 0x7, 0x7, 0xf, 0x8, 0x6, 0x6, 0x1, 0x3, 0x0, 0x8, 0x8, 0xd, 0xf,
4242+
0xc, 0xa, 0xe, 0x6, 0x4, 0x8, 0xa, 0xb, 0xe, 0x8, 0x3, 0x1, 0xf, 0xa,
4243+
0xd, 0xc, 0xa, 0x5, 0x5, 0xa, 0xd, 0x4, 0x3, 0xa, 0xb, 0x1, 0x6, 0x5,
4244+
0xd, 0x1, 0x6, 0x8, 0x0, 0xd, 0xd, 0x5, 0x6, 0x7, 0xb, 0x5, 0xd, 0x6,
4245+
]),
4246+
hex::decode("8468d3971d").unwrap(),
4247+
);
4248+
TrieNode::Leaf(leaf_node)
4249+
},
4250+
masks: TrieMasks { hash_mask: None, tree_mask: None },
4251+
},
4252+
];
4253+
4254+
// Create a fresh ParallelSparseTrie
4255+
let mut trie = ParallelSparseTrie::from_root(
4256+
TrieNode::Extension(ExtensionNode::new(
4257+
Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7]),
4258+
RlpNode::word_rlp(&B256::from(hex!(
4259+
"56fab2b106a97eae9c7197f86d03bca292da6e0ac725b783082f7d950cc4e0fc"
4260+
))),
4261+
)),
4262+
TrieMasks::none(),
4263+
true,
4264+
)
4265+
.unwrap();
4266+
4267+
// Call reveal_nodes
4268+
trie.reveal_nodes(nodes).unwrap();
4269+
4270+
// Remove the leaf at "0x4f88072c077f86613088dfcae648abe831fadca55ad43ab165d1680dd567b5d6"
4271+
let leaf_key = Nibbles::from_nibbles([
4272+
0x4, 0xf, 0x8, 0x8, 0x0, 0x7, 0x2, 0xc, 0x0, 0x7, 0x7, 0xf, 0x8, 0x6, 0x6, 0x1, 0x3,
4273+
0x0, 0x8, 0x8, 0xd, 0xf, 0xc, 0xa, 0xe, 0x6, 0x4, 0x8, 0xa, 0xb, 0xe, 0x8, 0x3, 0x1,
4274+
0xf, 0xa, 0xd, 0xc, 0xa, 0x5, 0x5, 0xa, 0xd, 0x4, 0x3, 0xa, 0xb, 0x1, 0x6, 0x5, 0xd,
4275+
0x1, 0x6, 0x8, 0x0, 0xd, 0xd, 0x5, 0x6, 0x7, 0xb, 0x5, 0xd, 0x6,
4276+
]);
4277+
4278+
let mut provider = MockTrieNodeProvider::new();
4279+
let revealed_branch = create_branch_node_with_children(&[], []);
4280+
let mut encoded = Vec::new();
4281+
revealed_branch.encode(&mut encoded);
4282+
provider.add_revealed_node(
4283+
Nibbles::from_nibbles([0x4, 0xf, 0x8, 0x8, 0x0, 0x7, 0x2, 0x2, 0x6]),
4284+
RevealedNode {
4285+
node: encoded.into(),
4286+
tree_mask: None,
4287+
// Give it a fake hashmask so that it appears like it will be stored in the db
4288+
hash_mask: Some(TrieMask::new(0b1111)),
4289+
},
4290+
);
4291+
4292+
trie.remove_leaf(&leaf_key, provider).unwrap();
4293+
4294+
// Calculate root so that updates are calculated.
4295+
trie.root();
4296+
4297+
// Take updates and assert they are correct
4298+
let updates = trie.take_updates();
4299+
assert_eq!(
4300+
updates.removed_nodes.into_iter().collect::<Vec<_>>(),
4301+
vec![removed_branch_path]
4302+
);
4303+
assert_eq!(updates.updated_nodes.len(), 1);
4304+
let updated_node = updates.updated_nodes.get(&branch_path).unwrap();
4305+
4306+
// Second bit must be set, indicating that the extension's child is in the db
4307+
assert_eq!(updated_node.tree_mask, TrieMask::new(0b011110100100101),)
4308+
}
4309+
40794310
#[test]
40804311
fn test_parallel_sparse_trie_root() {
40814312
// Step 1: Create the trie structure

0 commit comments

Comments
 (0)