shardtree: Simplify `LocatedPrunableTree::insert_subtree` node logic

The previous logic had the non-obvious behaviour that if the inserted
subtree was a leaf, then the `.iter().all()` was not rejecting subtrees
with non-matching values, because `Node::annotation` returns `None` for
leaves. This didn't cause a bug, because this behaviour is actually
intentionally triggered by the previous `is_complete` branch of the
conditional (as leaves are by definition complete subtrees).

The simplified logic is more obvious, because `PrunableTree::node_value`
returns the value of leaves and the annotation of nodes (which are the
same type for `PrunableTree`). It does change how leaves are treated by
this branch of the conditional, so a test has been added to ensure that
the intended behaviour is preserved if the `is_complete` branch is
refactored in future.

Co-authored-by: Kris Nuttycombe <kris@nutty.land>

shardtree/src/prunable.rs

@@ -700,14 +700,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
                             // It is safe to replace the existing root unannotated, because we
                             // can always recompute the root from a complete subtree.
                             Ok((subtree.root, vec![]))
-                        } else if subtree
-                            .root
-                            .0
-                            .annotation()
-                            .and_then(|ann| ann.as_ref())
-                            .iter()
-                            .all(|v| v.as_ref() == value)
-                        {
+                        } else if subtree.root.node_value().iter().all(|v| v == &value) {
                             Ok((
                                 // at this point we statically know the root to be a parent
                                 subtree.root.reannotate_root(Some(Rc::new(value.clone()))),
@@ -1592,6 +1585,31 @@ mod tests {
         );
     }
 
+    #[test]
+    fn located_insert_subtree_leaf_overwrites() {
+        let t: LocatedPrunableTree<String> = LocatedTree {
+            root_addr: Address::from_parts(2.into(), 1),
+            root: parent(leaf(("a".to_string(), RetentionFlags::MARKED)), nil()),
+        };
+
+        assert_eq!(
+            t.insert_subtree(
+                LocatedTree {
+                    root_addr: Address::from_parts(1.into(), 2),
+                    root: leaf(("b".to_string(), RetentionFlags::EPHEMERAL)),
+                },
+                false,
+            ),
+            Ok((
+                LocatedTree {
+                    root_addr: Address::from_parts(2.into(), 1),
+                    root: parent(leaf(("b".to_string(), RetentionFlags::EPHEMERAL)), nil()),
+                },
+                vec![],
+            )),
+        );
+    }
+
     #[test]
     fn located_witness() {
         let t: LocatedPrunableTree<String> = LocatedTree {