Addressed PR comments; added regression test that's currently failing

src/consistency.rs

@@ -240,7 +240,11 @@ fn last_common_ancestor(mut idx: InternalIdx, num_leaves1: u64, num_leaves2: u64
 
 #[cfg(test)]
 pub(crate) mod test {
-    use crate::mem_backed_tree::test::{rand_tree, rand_val};
+    use crate::{
+        mem_backed_tree::test::{rand_tree, rand_val},
+        RootHash,
+    };
+    use sha2::Sha256;
 
     // Tests that an honestly generated consistency proof verifies
     #[test]
@@ -270,6 +274,13 @@ pub(crate) mod test {
                             initial_size + num_to_add
                         )
                     });
+
+                // Make a nonsense initial root. This should fail to verify
+                let modified_initial_root =
+                    RootHash::<Sha256>::new(initial_root.root_hash, (initial_size as u64) - 1);
+                assert!(new_root
+                    .verify_consistency(&modified_initial_root, &proof)
+                    .is_err())
             }
         }
     }

src/lib.rs

@@ -31,17 +31,17 @@ use subtle::ConstantTimeEq;
 #[derive(Clone, Debug)]
 pub struct RootHash<H: Digest> {
     /// The root hash of the Merkle tree that this root represents
-    pub(crate) root_hash: digest::Output<H>,
+    root_hash: digest::Output<H>,
 
     /// The number of leaves in the Merkle tree that this root represents. That is, the number of
     /// items inserted into the [`CtMerkleTree`] that created with `RootHash`.
-    pub(crate) num_leaves: u64,
+    num_leaves: u64,
 }
 
 impl<H: Digest> PartialEq for RootHash<H> {
     /// Compares this `RootHash` to another in constant time.
     fn eq(&self, other: &RootHash<H>) -> bool {
-        self.root_hash.ct_eq(&other.root_hash).into()
+        self.num_leaves == other.num_leaves() && self.root_hash.ct_eq(&other.root_hash).into()
     }
 }
 

src/mem_backed_tree.rs

@@ -120,7 +120,7 @@ where
         let new_leaf_idx = LeafIdx::new(num_leaves - 1);
         // recalculate_path() requires its leaf idx to be less than usize::MAX. This is guaranteed
         // because it's self.len() - 1.
-        self.recaluclate_path(new_leaf_idx)
+        self.recalculate_path(new_leaf_idx)
     }
 
     /// Checks that this tree is well-formed. This can take a while if the tree is large. Run this
@@ -151,10 +151,10 @@ where
             let expected_hash = leaf_hash::<H, _>(leaf);
             // We can unwrap() because we checked above that the number of nodes necessary for this
             // tree fits in memory
-            let stored_hash = match self.internal_nodes.get(leaf_hash_idx.as_usize().unwrap()) {
-                None => Err(SelfCheckError::MissingNode(leaf_hash_idx.as_u64())),
-                Some(h) => Ok(h),
-            }?;
+            let Some(stored_hash) = self.internal_nodes.get(leaf_hash_idx.as_usize().unwrap())
+            else {
+                return Err(SelfCheckError::MissingNode(leaf_hash_idx.as_u64()));
+            };
 
             // If the hashes don't match, that's an error
             if stored_hash != &expected_hash {
@@ -175,7 +175,7 @@ where
                 let left_child_idx = parent_idx.left_child();
                 let right_child_idx = parent_idx.right_child(num_leaves);
 
-                // We may unwrap the .as_usize() computations because we alreayd know from the check
+                // We may unwrap the .as_usize() computations because we already know from the check
                 // above that self.internal_nodes.len() == num_nodes, i.e., the total number of
                 // nodes in the tree fits in memory, and therefore all the indices are at most
                 // `usize::MAX`.
@@ -206,10 +206,12 @@ where
         Ok(())
     }
 
-    /// Recalculates the hashes on the path from `leaf_idx` to the root. Panics if the path doesn't
-    /// exist. In other words, this tree MUST NOT be missing internal nodes or leaves. Also panics
-    /// if the given leaf index exceeds `usize::MAX`.
-    fn recaluclate_path(&mut self, leaf_idx: LeafIdx) {
+    /// Recalculates the hashes on the path from `leaf_idx` to the root.
+    ///
+    /// # Panics
+    /// Panics if the path doesn't exist. In other words, this tree MUST NOT be missing internal
+    /// nodes or leaves. Also panics if the given leaf index exceeds `usize::MAX`.
+    fn recalculate_path(&mut self, leaf_idx: LeafIdx) {
         // First update the leaf hash
         let leaf = &self.leaves[leaf_idx.as_usize().unwrap()];
         let mut cur_idx: InternalIdx = leaf_idx.into();

src/tree_util.rs

@@ -29,7 +29,7 @@ impl LeafIdx {
     }
 }
 
-// I know I could just expose the underlying usize. But making it an opaque type with a
+// I know I could just expose the underlying u64. But making it an opaque type with a
 // constructor and a getter seems safer
 impl InternalIdx {
     pub(crate) fn new(idx: u64) -> Self {