shardtree: Fix `LocatedPrunableTree::from_iter`

The general logic in this function is:
- Build a stack that contains all of the complete subtrees contained
  within the iterator.
- Take nodes off the top of the stack, and combine them together with
  each other and empty nodes until the resulting right-hand subtree is
  larger than all remaining subtrees on the stack.
- Make a single pass over the stack from bottom to top, accumulating
  subtrees and adding empty nodes on the left until a single tree is
  obtained.

The bug was that the second step was not leaving the right-hand subtree
sufficiently large. The final single-pass step expects every subtree in
the stack to be no smaller than the accumulated tree up to that point,
and in particular the right-hand subtree must be the direct right child
of the final tree. Some specific sequences of leaves were leaving the
right-hand subtree in a state where it was the same size as the largest
individual subtree of the rest of the stack, but smaller than their
accumulated tree.

The solution is to pre-compute what the address of the final tree will
be, and then add empty nodes on the right to the right-hand subtree
until it is the direct right child.

Co-authored-by: Kris Nuttycombe <kris@nutty.land>
This commit is contained in:
Jack Grigg 2023-07-06 13:02:40 +00:00
parent bddf6684e3
commit 278e4d2320
1 changed files with 35 additions and 10 deletions

View File

@ -911,6 +911,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
// [`Node::Nil`] nodes that were introduced in the process of constructing the tree.
fn build_minimal_tree<H: Hashable + Clone + PartialEq>(
mut xs: Vec<(LocatedPrunableTree<H>, bool)>,
root_addr: Address,
prune_below: Level,
) -> Option<(LocatedPrunableTree<H>, bool, Vec<IncompleteAt>)> {
// First, consume the stack from the right, building up a single tree
@ -967,6 +968,22 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
}
}
while cur.root_addr.level() + 1 < root_addr.level() {
let sibling_addr = cur.root_addr.sibling();
incomplete.push(IncompleteAt {
address: sibling_addr,
required_for_witness: contains_marked,
});
cur = unite(
cur,
LocatedTree {
root_addr: sibling_addr,
root: Tree(Node::Nil),
},
prune_below,
);
}
// push our accumulated max-height right hand node back on to the stack.
xs.push((cur, contains_marked));
@ -1050,16 +1067,24 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
}
trace!("Initial fragments: {:?}", fragments);
build_minimal_tree(fragments, prune_below).map(
|(to_insert, contains_marked, incomplete)| BatchInsertionResult {
subtree: to_insert,
contains_marked,
incomplete,
max_insert_position: Some(position - 1),
checkpoints,
remainder: values,
},
)
if position > position_range.start {
let last_position = position - 1;
let minimal_tree_addr =
Address::from(position_range.start).common_ancestor(&last_position.into());
trace!("Building minimal tree at {:?}", minimal_tree_addr);
build_minimal_tree(fragments, minimal_tree_addr, prune_below).map(
|(to_insert, contains_marked, incomplete)| BatchInsertionResult {
subtree: to_insert,
contains_marked,
incomplete,
max_insert_position: Some(last_position),
checkpoints,
remainder: values,
},
)
} else {
None
}
}
/// Put a range of values into the subtree by consuming the given iterator, starting at the