Skip to content

Commit

Permalink
shardtree: Use annotated parent nodes for frontier ommers.
Browse files Browse the repository at this point in the history
The semantics of `Node::Leaf` in `PrunableTree` are that the leaf
has been produced by pruning of observed children. In the case of
an inserted frontier, the values of the ommers have not been directly
observed and thus should be represented by `Parent` nodes with `Nil`
children.
  • Loading branch information
nuttycom committed Jun 26, 2024
1 parent 1ab5969 commit 55d46a5
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 22 deletions.
3 changes: 3 additions & 0 deletions incrementalmerkletree/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ impl TryFrom<Position> for usize {
pub struct Level(u8);

impl Level {
/// Level 0 corresponds to a leaf of the tree.
pub const ZERO: Self = Level(0);

pub const fn new(value: u8) -> Self {
Self(value)
}
Expand Down
1 change: 1 addition & 0 deletions shardtree/src/legacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
marking: Marking::None,
},
self.root_addr.level(),
true,
)
});

Expand Down
18 changes: 6 additions & 12 deletions shardtree/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,17 +321,8 @@ impl<
.map_err(ShardTreeError::Storage)?
.unwrap_or_else(|| LocatedTree::empty(subtree_root_addr));

trace!(
max_position = ?current_shard.max_position(),
subtree = ?current_shard,
"Current shard");
let (updated_subtree, supertree) =
current_shard.insert_frontier_nodes(frontier, &leaf_retention)?;
trace!(
max_position = ?updated_subtree.max_position(),
subtree = ?updated_subtree,
"Replacement shard"
);
self.store
.put_shard(updated_subtree)
.map_err(ShardTreeError::Storage)?;
Expand Down Expand Up @@ -1465,7 +1456,8 @@ mod tests {
id: 1,
marking: frontier_marking,
},
)?;
)
.unwrap();

// Insert a few leaves beginning at the subsequent position, so as to cross the shard
// boundary.
Expand All @@ -1474,7 +1466,8 @@ mod tests {
('g'..='j')
.into_iter()
.map(|c| (c.to_string(), Retention::Ephemeral)),
)?;
)
.unwrap();

// Trigger pruning by adding 5 more checkpoints
for i in 2..7 {
Expand All @@ -1487,7 +1480,8 @@ mod tests {
('e'..='f')
.into_iter()
.map(|c| (c.to_string(), Retention::Marked)),
)?;
)
.unwrap();

// Compute the witness
tree.witness_at_checkpoint_id(frontier_end, &6)
Expand Down
40 changes: 30 additions & 10 deletions shardtree/src/prunable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -559,12 +559,6 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
subtree: LocatedPrunableTree<H>,
contains_marked: bool,
) -> Result<(PrunableTree<H>, Vec<IncompleteAt>), InsertionError> {
trace!(
root_addr = ?root_addr,
max_position = ?LocatedTree::max_position_internal(root_addr, into),
to_insert = ?subtree.root_addr(),
"Subtree insert"
);
// An empty tree cannot replace any other type of tree.
if subtree.root().is_nil() {
Ok((into.clone(), vec![]))
Expand Down Expand Up @@ -769,7 +763,14 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
split_at: Level,
) -> (Self, Option<Self>) {
let (position, leaf, ommers) = frontier.into_parts();
Self::from_frontier_parts(position, leaf, ommers.into_iter(), leaf_retention, split_at)
Self::from_frontier_parts(
position,
leaf,
ommers.into_iter(),
leaf_retention,
split_at,
false,
)
}

// Permits construction of a subtree from legacy `CommitmentTree` data that may
Expand All @@ -781,6 +782,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
mut ommers: impl Iterator<Item = H>,
leaf_retention: &Retention<C>,
split_at: Level,
ommers_fully_observed: bool,
) -> (Self, Option<Self>) {
let mut addr = Address::from(position);
let mut subtree = Tree::leaf((leaf, leaf_retention.into()));
Expand All @@ -793,8 +795,19 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
} else if let Some(left) = ommers.next() {
// the current address corresponds to a right child, so create a parent that
// takes the left sibling to that child from the ommers
subtree =
Tree::parent(None, Tree::leaf((left, RetentionFlags::EPHEMERAL)), subtree);
subtree = Tree::parent(
None,
// For the leaf level of the tree, or if all ommers are fully observed,
// return a leaf; otherwise return an annotated empty parent node to
// signify that the value of that subtree root has not actually been
// observed.
if ommers_fully_observed || addr.level() == Level::ZERO {
Tree::leaf((left, RetentionFlags::EPHEMERAL))
} else {
Tree::empty_annotated(Some(Arc::new(left)))
},
subtree,
);
} else {
break;
}
Expand All @@ -821,7 +834,14 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
// left-hand sibling
supertree = Some(Tree::parent(
None,
Tree::leaf((left, RetentionFlags::EPHEMERAL)),
// For the leaf level of the supertree, or if all ommers are fully observed,
// return a leaf; otherwise return an annotated empty parent node to signify
// that the value of that root has not actually been observed.
if ommers_fully_observed || addr.level() == split_at {
Tree::leaf((left, RetentionFlags::EPHEMERAL))
} else {
Tree::empty_annotated(Some(Arc::new(left)))
},
supertree.unwrap_or_else(Tree::empty),
));
addr = addr.parent();
Expand Down

0 comments on commit 55d46a5

Please sign in to comment.