test: Cover all errors in concurrent Merkle tree

Make sure all errors are covered:

* `HeightZero` - add new tests.
* `ChangelogZero` - add new tests.
* `RootsZero` - add new tests and fix the checks.
* `TreeFull` - was already tested.
* `BatchGreatedThanChangelog` - was already tested.
* `InvalidProofLength` - add new tests.
* `InvalidProof` - add new tests.
* `CannotUpdateEmpty` - add new tests.
* `EmptyLeaves` - add new tests.
* `StructBufferSize` - add new tests.
* `FilledSubtreesBufferSize` - add new tests.
* `ChangelogBufferSize` - add new tests.
* `RootBufferSize` - add new tests.
* `CanopyBufferSize` - add new tests.

Also, remove error variants which we don't want anymore:

* `HeightHigherThanMax` - not used anymore, there is no limit of height.
* `RootHigherThanMax` - it was an error we used to return in `root()`
  method. It's removed here, because it's not reproducable at all.
  We always ensure the bound of roots, so it's fine to panic in `root()`.
* `BytesRead` - not used anymore. All methods for byte serialization
  return specific errors like `StructBufferSize` or `ChangelogBufferSize`
  when the sizes of provided byte buffers are incorrect.
* `CannotUpdateLeaf` - not used anymore. The original idea behing this
  error comes from SPL concurrent Merkle tree whitepaper and the fact
  that patching the Merkle proof when the same leaf was updated, can't
  guarantee the correctness of the patched proof. But performing such
  patching needs to be done anyway for indexed Merkle trees.
* `AppendOnly` - not used anymore. Our concurrent Merkle tree always
  needs to have a changelog. The reason is that we use changelog also
  as a storage for Merkle paths that we compute during appends and
  updates.
* `EmptyChangelogEntries` - not used anymore.
* `MerklePathsEmptyNode` - not used anymore.

Error variant which is not tested:

* `BoundedVecError` - it's not reproducable. This error variant is
  returned only when bounds of roots, changelogs, subtrees etc. are
  violated. Which should never happen - if it does, it's a bug.

merkle-tree/concurrent/src/changelog.rs

@@ -62,21 +62,10 @@ impl<const HEIGHT: usize> ChangelogEntry<HEIGHT> {
         &self,
         leaf_index: usize,
         proof: &mut BoundedVec<[u8; 32]>,
-        _allow_updates_in_changelog: bool,
     ) -> Result<(), ConcurrentMerkleTreeError> {
         if leaf_index != self.index() {
             let intersection_index = self.intersection_index(leaf_index);
             proof[intersection_index] = self.path[intersection_index];
-        } else {
-            // This case means that the leaf we are trying to update was
-            // already updated. Therefore, updating the proof is impossible.
-            // We need to return an error and request the caller
-            // to retry the update with a new proof.
-            //
-            // TODO(vadorovsky): Re-visit optional throwing of this error.
-            // if !allow_updates_in_changelog {
-            //     return Err(ConcurrentMerkleTreeError::CannotUpdateLeaf);
-            // }
         }
 
         Ok(())

merkle-tree/concurrent/src/errors.rs

@@ -10,14 +10,8 @@ pub enum ConcurrentMerkleTreeError {
     HeightZero,
     #[error("Invalid changelog size, it has to be greater than 0. Changelog is used for storing Merkle paths during appends.")]
     ChangelogZero,
-    #[error("Invalid height, it cannot exceed the maximum allowed height")]
-    HeightHigherThanMax,
     #[error("Invalid number of roots, it has to be greater than 0")]
     RootsZero,
-    #[error("Invalid root index, it exceeds the root buffer size")]
-    RootHigherThanMax,
-    #[error("Failed to read a value from bytes")]
-    BytesRead,
     #[error("Merkle tree is full, cannot append more leaves.")]
     TreeFull,
     #[error("Number of leaves ({0}) exceeds the changelog capacity ({1}).")]
@@ -26,20 +20,10 @@ pub enum ConcurrentMerkleTreeError {
     InvalidProofLength(usize, usize),
     #[error("Invalid Merkle proof, expected root: {0:?}, the provided proof produces root: {1:?}")]
     InvalidProof([u8; 32], [u8; 32]),
-    #[error("Attempting to update the leaf which was updated by an another newest change.")]
-    CannotUpdateLeaf,
     #[error("Cannot update the empty leaf")]
     CannotUpdateEmpty,
-    #[error("Cannot update tree without changelog, only `append` is supported.")]
-    AppendOnly,
     #[error("The batch of leaves is empty")]
     EmptyLeaves,
-    #[error("The vector of changelog entries is empty")]
-    EmptyChangelogEntries,
-    #[error(
-        "Found an empty node in the Merkle path buffer, where we expected all nodes to be filled"
-    )]
-    MerklePathsEmptyNode,
     #[error("Invalid struct buffer size, expected {0}, got {1}")]
     StructBufferSize(usize, usize),
     #[error("Invalid filled subtrees buffer size, expected {0}, got {1}")]
@@ -65,25 +49,18 @@ impl From<ConcurrentMerkleTreeError> for u32 {
             ConcurrentMerkleTreeError::IntegerOverflow => 2001,
             ConcurrentMerkleTreeError::HeightZero => 2002,
             ConcurrentMerkleTreeError::ChangelogZero => 2003,
-            ConcurrentMerkleTreeError::HeightHigherThanMax => 2004,
-            ConcurrentMerkleTreeError::RootsZero => 2005,
-            ConcurrentMerkleTreeError::RootHigherThanMax => 2006,
-            ConcurrentMerkleTreeError::BytesRead => 2007,
-            ConcurrentMerkleTreeError::TreeFull => 2008,
-            ConcurrentMerkleTreeError::BatchGreaterThanChangelog(_, _) => 2009,
-            ConcurrentMerkleTreeError::InvalidProofLength(_, _) => 2010,
-            ConcurrentMerkleTreeError::InvalidProof(_, _) => 2011,
-            ConcurrentMerkleTreeError::CannotUpdateLeaf => 2012,
-            ConcurrentMerkleTreeError::CannotUpdateEmpty => 2013,
-            ConcurrentMerkleTreeError::AppendOnly => 2014,
-            ConcurrentMerkleTreeError::EmptyLeaves => 2015,
-            ConcurrentMerkleTreeError::EmptyChangelogEntries => 2016,
-            ConcurrentMerkleTreeError::MerklePathsEmptyNode => 2017,
-            ConcurrentMerkleTreeError::StructBufferSize(_, _) => 2018,
-            ConcurrentMerkleTreeError::FilledSubtreesBufferSize(_, _) => 2019,
-            ConcurrentMerkleTreeError::ChangelogBufferSize(_, _) => 2020,
-            ConcurrentMerkleTreeError::RootBufferSize(_, _) => 2021,
-            ConcurrentMerkleTreeError::CanopyBufferSize(_, _) => 2022,
+            ConcurrentMerkleTreeError::RootsZero => 2004,
+            ConcurrentMerkleTreeError::TreeFull => 2005,
+            ConcurrentMerkleTreeError::BatchGreaterThanChangelog(_, _) => 2006,
+            ConcurrentMerkleTreeError::InvalidProofLength(_, _) => 2007,
+            ConcurrentMerkleTreeError::InvalidProof(_, _) => 2008,
+            ConcurrentMerkleTreeError::CannotUpdateEmpty => 2009,
+            ConcurrentMerkleTreeError::EmptyLeaves => 2010,
+            ConcurrentMerkleTreeError::StructBufferSize(_, _) => 2011,
+            ConcurrentMerkleTreeError::FilledSubtreesBufferSize(_, _) => 2012,
+            ConcurrentMerkleTreeError::ChangelogBufferSize(_, _) => 2013,
+            ConcurrentMerkleTreeError::RootBufferSize(_, _) => 2014,
+            ConcurrentMerkleTreeError::CanopyBufferSize(_, _) => 2015,
             ConcurrentMerkleTreeError::Hasher(e) => e.into(),
             ConcurrentMerkleTreeError::BoundedVec(e) => e.into(),
         }

merkle-tree/concurrent/src/lib.rs

@@ -93,6 +93,9 @@ where
         if changelog_size == 0 {
             return Err(ConcurrentMerkleTreeError::ChangelogZero);
         }
+        if roots_size == 0 {
+            return Err(ConcurrentMerkleTreeError::RootsZero);
+        }
         Ok(Self {
             height,
 
@@ -374,6 +377,9 @@ where
         if tree.changelog_capacity == 0 {
             return Err(ConcurrentMerkleTreeError::ChangelogZero);
         }
+        if tree.roots_capacity == 0 {
+            return Err(ConcurrentMerkleTreeError::RootsZero);
+        }
 
         // Restore the vectors correctly, by pointing them to the appropriate
         // byte slices as underlying data. The most unsafe part of this code.
@@ -562,6 +568,9 @@ where
         if changelog_size == 0 {
             return Err(ConcurrentMerkleTreeError::ChangelogZero);
         }
+        if roots_size == 0 {
+            return Err(ConcurrentMerkleTreeError::RootsZero);
+        }
 
         let tree = ConcurrentMerkleTree::struct_from_bytes_mut(bytes_struct)?;
 
@@ -720,11 +729,10 @@ where
     }
 
     /// Returns the current root.
-    pub fn root(&self) -> Result<[u8; 32], ConcurrentMerkleTreeError> {
-        self.roots
-            .get(self.root_index())
-            .ok_or(ConcurrentMerkleTreeError::RootHigherThanMax)
-            .copied()
+    pub fn root(&self) -> [u8; 32] {
+        // PANICS: This should never happen - there is always a root in the
+        // tree and `self.root_index()` should always point to an existing index.
+        self.roots[self.root_index()]
     }
 
     pub fn current_index(&self) -> usize {
@@ -783,10 +791,9 @@ where
         changelog_index: usize,
         leaf_index: usize,
         proof: &mut BoundedVec<[u8; 32]>,
-        allow_updates_changelog: bool,
     ) -> Result<(), ConcurrentMerkleTreeError> {
         for changelog_entry in self.changelog.iter_from(changelog_index) {
-            changelog_entry.update_proof(leaf_index, proof, allow_updates_changelog)?;
+            changelog_entry.update_proof(leaf_index, proof)?;
         }
 
         Ok(())
@@ -801,7 +808,7 @@ where
         leaf_index: usize,
         proof: &BoundedVec<[u8; 32]>,
     ) -> Result<(), ConcurrentMerkleTreeError> {
-        let expected_root = self.root()?;
+        let expected_root = self.root();
         let computed_root = compute_root::<H>(leaf, leaf_index, proof)?;
         if computed_root == expected_root {
             Ok(())
@@ -876,7 +883,6 @@ where
         new_leaf: &[u8; 32],
         leaf_index: usize,
         proof: &mut BoundedVec<[u8; 32]>,
-        allow_updates_changelog: bool,
     ) -> Result<(usize, usize), ConcurrentMerkleTreeError> {
         let expected_proof_len = self.height - self.canopy_depth;
         if proof.len() != expected_proof_len {
@@ -893,12 +899,7 @@ where
             self.update_proof_from_canopy(leaf_index, proof)?;
         }
         if self.changelog_capacity > 0 && changelog_index != self.changelog_index() {
-            self.update_proof_from_changelog(
-                changelog_index,
-                leaf_index,
-                proof,
-                allow_updates_changelog,
-            )?;
+            self.update_proof_from_changelog(changelog_index, leaf_index, proof)?;
         }
         self.validate_proof(old_leaf, leaf_index, proof)?;
         self.update_leaf_in_tree(new_leaf, leaf_index, proof)
@@ -914,6 +915,9 @@ where
         &mut self,
         leaves: &[&[u8; 32]],
     ) -> Result<(usize, usize), ConcurrentMerkleTreeError> {
+        if leaves.is_empty() {
+            return Err(ConcurrentMerkleTreeError::EmptyLeaves);
+        }
         if (self.next_index + leaves.len() - 1) >= 1 << self.height {
             return Err(ConcurrentMerkleTreeError::TreeFull);
         }