diff --git a/src/consistency.rs b/src/consistency.rs index fab3e98..fe70b3b 100644 --- a/src/consistency.rs +++ b/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::::new(initial_root.root_hash, (initial_size as u64) - 1); + assert!(new_root + .verify_consistency(&modified_initial_root, &proof) + .is_err()) } } } diff --git a/src/lib.rs b/src/lib.rs index 24795ed..9f1260d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -31,17 +31,17 @@ use subtle::ConstantTimeEq; #[derive(Clone, Debug)] pub struct RootHash { /// The root hash of the Merkle tree that this root represents - pub(crate) root_hash: digest::Output, + root_hash: digest::Output, /// 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 PartialEq for RootHash { /// Compares this `RootHash` to another in constant time. fn eq(&self, other: &RootHash) -> 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() } } diff --git a/src/mem_backed_tree.rs b/src/mem_backed_tree.rs index 75aa0e6..3ee5e7e 100644 --- a/src/mem_backed_tree.rs +++ b/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::(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(); diff --git a/src/tree_util.rs b/src/tree_util.rs index ecfdf9e..1990849 100644 --- a/src/tree_util.rs +++ b/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 {