From 1ab596938aec6c949140ceba6877a8fc98957d0a Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@nutty.land>
Date: Tue, 25 Jun 2024 15:04:12 -0600
Subject: [PATCH] shardtree: Make it possible to construct empty annotated
 parent nodes.

This is needed for inserting shard root values; inserting these roots as
`Node::Leaf` values incorrectly indicates that the children have been
observed.
---
 shardtree/src/lib.rs      |  3 ++-
 shardtree/src/prunable.rs | 16 ++--------------
 shardtree/src/tree.rs     | 21 ++++++++++++++++++---
 3 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/shardtree/src/lib.rs b/shardtree/src/lib.rs
index 398b24b..22f35d5 100644
--- a/shardtree/src/lib.rs
+++ b/shardtree/src/lib.rs
@@ -210,7 +210,8 @@ impl<
         Ok(())
     }
 
-    /// Append a single value at the first available position in the tree.
+    /// Append a single value at the first unfilled position greater than the maximum position of
+    /// any previously inserted leaf.
     ///
     /// Prefer to use [`Self::batch_insert`] when appending multiple values, as these operations
     /// require fewer traversals of the tree than are necessary when performing multiple sequential
diff --git a/shardtree/src/prunable.rs b/shardtree/src/prunable.rs
index d230093..8a6b617 100644
--- a/shardtree/src/prunable.rs
+++ b/shardtree/src/prunable.rs
@@ -312,7 +312,7 @@ impl<H: Hashable + Clone + PartialEq> PrunableTree<H> {
     /// `level` must be the level of the two nodes that are being joined.
     pub(crate) fn unite(level: Level, ann: Option<Arc<H>>, left: Self, right: Self) -> Self {
         match (left, right) {
-            (Tree(Node::Nil), Tree(Node::Nil)) => Tree::empty(),
+            (Tree(Node::Nil), Tree(Node::Nil)) if ann.is_none() => Tree::empty(),
             (Tree(Node::Leaf { value: lv }), Tree(Node::Leaf { value: rv }))
                 // we can prune right-hand leaves that are not marked or reference leaves; if a
                 // leaf is a checkpoint then that information will be propagated to the replacement
@@ -322,11 +322,7 @@ impl<H: Hashable + Clone + PartialEq> PrunableTree<H> {
             {
                 Tree::leaf((H::combine(level, &lv.0, &rv.0), rv.1))
             }
-            (left, right) => Tree::parent(
-                    ann,
-                    left,
-                    right,
-            ),
+            (left, right) => Tree::parent(ann, left, right),
         }
     }
 }
@@ -721,13 +717,6 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
             }
         }
 
-        let max_position = self.max_position();
-        trace!(
-            max_position = ?max_position,
-            tree = ?self,
-            to_insert = ?subtree,
-            "Current shard"
-        );
         let LocatedTree { root_addr, root } = self;
         if root_addr.contains(&subtree.root_addr) {
             go(*root_addr, root, subtree, contains_marked).map(|(root, incomplete)| {
@@ -735,7 +724,6 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
                     root_addr: *root_addr,
                     root,
                 };
-                assert!(new_tree.max_position() >= max_position);
                 (new_tree, incomplete)
             })
         } else {
diff --git a/shardtree/src/tree.rs b/shardtree/src/tree.rs
index 6682c7b..e92fd6c 100644
--- a/shardtree/src/tree.rs
+++ b/shardtree/src/tree.rs
@@ -94,16 +94,31 @@ impl<A, V> Deref for Tree<A, V> {
 
 impl<A, V> Tree<A, V> {
     /// Constructs the empty tree.
-    pub fn empty() -> Self {
+    ///
+    /// This represents a tree for which we have no information.
+    pub const fn empty() -> Self {
         Tree(Node::Nil)
     }
 
+    /// Constructs an empty annotated parent node (a [`Node::Parent`] having both
+    /// [`Node::Nil`] children.
+    pub fn empty_annotated(ann: A) -> Self {
+        Self::parent(ann, Tree::empty(), Tree::empty())
+    }
+
     /// Constructs the empty tree consisting of a single pruned node.
-    pub fn empty_pruned() -> Self {
+    ///
+    /// This represents a tree for which we have previously observed all of the
+    /// children of its root, and have pruned away all of the data of this tree
+    /// as irrelevant.
+    pub const fn empty_pruned() -> Self {
         Tree(Node::Pruned)
     }
 
     /// Constructs a tree containing a single leaf.
+    ///
+    /// This represents either leaf of the tree, or an internal parent node of the
+    /// tree having all [`Node::Pruned`] children.
     pub fn leaf(value: V) -> Self {
         Tree(Node::Leaf { value })
     }
@@ -117,7 +132,7 @@ impl<A, V> Tree<A, V> {
         })
     }
 
-    /// Returns `true` if the tree has no leaves.
+    /// Returns `true` if the tree is the [`Node::Nil`] node.
     pub fn is_empty(&self) -> bool {
         self.0.is_nil()
     }