Use const generics to set commitment tree & incremental witness depths.

This is in preparation for extraction into the `incrementalmerkletree`
crate, which is not Sapling-specific and therefore cannot hard-code
the depths of these data structures.

zcash_client_backend/src/data_api.rs

@@ -10,8 +10,7 @@ use zcash_primitives::{
     consensus::BlockHeight,
     legacy::TransparentAddress,
     memo::{Memo, MemoBytes},
-    merkle_tree::{CommitmentTree, IncrementalWitness},
+    sapling::{self, Nullifier, PaymentAddress},
-    sapling::{self, Node, Nullifier, PaymentAddress},
     transaction::{
         components::{amount::Amount, OutPoint},
         Transaction, TxId,
@@ -166,14 +165,14 @@ pub trait WalletRead {
     fn get_commitment_tree(
         &self,
         block_height: BlockHeight,
-    ) -> Result<Option<CommitmentTree<Node>>, Self::Error>;
+    ) -> Result<Option<sapling::CommitmentTree>, Self::Error>;
 
     /// Returns the incremental witnesses as of the specified block height.
     #[allow(clippy::type_complexity)]
     fn get_witnesses(
         &self,
         block_height: BlockHeight,
-    ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error>;
+    ) -> Result<Vec<(Self::NoteRef, sapling::IncrementalWitness)>, Self::Error>;
 
     /// Returns the nullifiers for notes that the wallet is tracking, along with their
     /// associated account IDs, that are either unspent or have not yet been confirmed as
@@ -239,7 +238,7 @@ pub struct PrunedBlock<'a> {
     pub block_height: BlockHeight,
     pub block_hash: BlockHash,
     pub block_time: u32,
-    pub commitment_tree: &'a CommitmentTree<Node>,
+    pub commitment_tree: &'a sapling::CommitmentTree,
     pub transactions: &'a Vec<WalletTx<Nullifier>>,
 }
 
@@ -388,8 +387,8 @@ pub trait WalletWrite: WalletRead {
     fn advance_by_block(
         &mut self,
         block: &PrunedBlock,
-        updated_witnesses: &[(Self::NoteRef, IncrementalWitness<Node>)],
+        updated_witnesses: &[(Self::NoteRef, sapling::IncrementalWitness)],
-    ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error>;
+    ) -> Result<Vec<(Self::NoteRef, sapling::IncrementalWitness)>, Self::Error>;
 
     /// Caches a decrypted transaction in the persistent wallet store.
     fn store_decrypted_tx(
@@ -433,8 +432,7 @@ pub mod testing {
         consensus::{BlockHeight, Network},
         legacy::TransparentAddress,
         memo::Memo,
-        merkle_tree::{CommitmentTree, IncrementalWitness},
+        sapling::{self, Nullifier},
-        sapling::{Node, Nullifier},
         transaction::{
             components::{Amount, OutPoint},
             Transaction, TxId,
@@ -525,7 +523,7 @@ pub mod testing {
         fn get_commitment_tree(
             &self,
             _block_height: BlockHeight,
-        ) -> Result<Option<CommitmentTree<Node>>, Self::Error> {
+        ) -> Result<Option<sapling::CommitmentTree>, Self::Error> {
             Ok(None)
         }
 
@@ -533,7 +531,7 @@ pub mod testing {
         fn get_witnesses(
             &self,
             _block_height: BlockHeight,
-        ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error> {
+        ) -> Result<Vec<(Self::NoteRef, sapling::IncrementalWitness)>, Self::Error> {
             Ok(Vec::new())
         }
 
@@ -613,8 +611,8 @@ pub mod testing {
         fn advance_by_block(
             &mut self,
             _block: &PrunedBlock,
-            _updated_witnesses: &[(Self::NoteRef, IncrementalWitness<Node>)],
+            _updated_witnesses: &[(Self::NoteRef, sapling::IncrementalWitness)],
-        ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error> {
+        ) -> Result<Vec<(Self::NoteRef, sapling::IncrementalWitness)>, Self::Error> {
             Ok(vec![])
         }
 

zcash_client_backend/src/data_api/wallet.rs

@@ -4,7 +4,6 @@ use std::fmt::Debug;
 use zcash_primitives::{
     consensus::{self, NetworkUpgrade},
     memo::MemoBytes,
-    merkle_tree::MerklePath,
     sapling::{
         self,
         note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey},
@@ -701,11 +700,7 @@ fn select_key_for_note<N>(
     selected: &SpendableNote<N>,
     extsk: &ExtendedSpendingKey,
     dfvk: &DiversifiableFullViewingKey,
-) -> Option<(
+) -> Option<(sapling::Note, ExtendedSpendingKey, sapling::MerklePath)> {
-    sapling::Note,
-    ExtendedSpendingKey,
-    MerklePath<sapling::Node>,
-)> {
     let merkle_path = selected.witness.path().expect("the tree is not empty");
 
     // Attempt to reconstruct the note being spent using both the internal and external dfvks

zcash_client_backend/src/wallet.rs

@@ -6,7 +6,6 @@ use zcash_primitives::{
     consensus::BlockHeight,
     keys::OutgoingViewingKey,
     legacy::TransparentAddress,
-    merkle_tree::IncrementalWitness,
     sapling,
     transaction::{
         components::{
@@ -118,7 +117,7 @@ pub struct WalletSaplingOutput<N> {
     account: AccountId,
     note: sapling::Note,
     is_change: bool,
-    witness: IncrementalWitness<sapling::Node>,
+    witness: sapling::IncrementalWitness,
     nf: N,
 }
 
@@ -132,7 +131,7 @@ impl<N> WalletSaplingOutput<N> {
         account: AccountId,
         note: sapling::Note,
         is_change: bool,
-        witness: IncrementalWitness<sapling::Node>,
+        witness: sapling::IncrementalWitness,
         nf: N,
     ) -> Self {
         Self {
@@ -165,10 +164,10 @@ impl<N> WalletSaplingOutput<N> {
     pub fn is_change(&self) -> bool {
         self.is_change
     }
-    pub fn witness(&self) -> &IncrementalWitness<sapling::Node> {
+    pub fn witness(&self) -> &sapling::IncrementalWitness {
         &self.witness
     }
-    pub fn witness_mut(&mut self) -> &mut IncrementalWitness<sapling::Node> {
+    pub fn witness_mut(&mut self) -> &mut sapling::IncrementalWitness {
         &mut self.witness
     }
     pub fn nf(&self) -> &N {
@@ -183,7 +182,7 @@ pub struct SpendableNote<NoteRef> {
     pub diversifier: sapling::Diversifier,
     pub note_value: Amount,
     pub rseed: sapling::Rseed,
-    pub witness: IncrementalWitness<sapling::Node>,
+    pub witness: sapling::IncrementalWitness,
 }
 
 impl<NoteRef> sapling_fees::InputView<NoteRef> for SpendableNote<NoteRef> {

zcash_client_backend/src/welding_rig.rs

@@ -7,7 +7,6 @@ use subtle::{ConditionallySelectable, ConstantTimeEq, CtOption};
 use zcash_note_encryption::batch;
 use zcash_primitives::{
     consensus,
-    merkle_tree::{CommitmentTree, IncrementalWitness},
     sapling::{
         self,
         note_encryption::{PreparedIncomingViewingKey, SaplingDomain},
@@ -60,7 +59,7 @@ pub trait ScanningKey {
     fn sapling_nf(
         key: &Self::SaplingNk,
         note: &Note,
-        witness: &IncrementalWitness<Node>,
+        witness: &sapling::IncrementalWitness,
     ) -> Self::Nf;
 }
 
@@ -88,7 +87,7 @@ impl ScanningKey for DiversifiableFullViewingKey {
     fn sapling_nf(
         key: &Self::SaplingNk,
         note: &Note,
-        witness: &IncrementalWitness<Node>,
+        witness: &sapling::IncrementalWitness,
     ) -> Self::Nf {
         note.nf(key, witness.position() as u64)
     }
@@ -108,7 +107,7 @@ impl ScanningKey for SaplingIvk {
         [((), self.clone(), ())]
     }
 
-    fn sapling_nf(_key: &Self::SaplingNk, _note: &Note, _witness: &IncrementalWitness<Node>) {}
+    fn sapling_nf(_key: &Self::SaplingNk, _note: &Note, _witness: &sapling::IncrementalWitness) {}
 }
 
 /// Scans a [`CompactBlock`] with a set of [`ScanningKey`]s.
@@ -130,8 +129,8 @@ impl ScanningKey for SaplingIvk {
 /// [`SaplingIvk`]: zcash_primitives::sapling::SaplingIvk
 /// [`CompactBlock`]: crate::proto::compact_formats::CompactBlock
 /// [`ScanningKey`]: crate::welding_rig::ScanningKey
-/// [`CommitmentTree`]: zcash_primitives::merkle_tree::CommitmentTree
+/// [`CommitmentTree`]: zcash_primitives::sapling::CommitmentTree
-/// [`IncrementalWitness`]: zcash_primitives::merkle_tree::IncrementalWitness
+/// [`IncrementalWitness`]: zcash_primitives::sapling::IncrementalWitness
 /// [`WalletSaplingOutput`]: crate::wallet::WalletSaplingOutput
 /// [`WalletTx`]: crate::wallet::WalletTx
 pub fn scan_block<P: consensus::Parameters + Send + 'static, K: ScanningKey>(
@@ -139,8 +138,8 @@ pub fn scan_block<P: consensus::Parameters + Send + 'static, K: ScanningKey>(
     block: CompactBlock,
     vks: &[(&AccountId, &K)],
     nullifiers: &[(AccountId, Nullifier)],
-    tree: &mut CommitmentTree<Node>,
+    tree: &mut sapling::CommitmentTree,
-    existing_witnesses: &mut [&mut IncrementalWitness<Node>],
+    existing_witnesses: &mut [&mut sapling::IncrementalWitness],
 ) -> Vec<WalletTx<K::Nf>> {
     scan_block_with_runner::<_, _, ()>(
         params,
@@ -200,8 +199,8 @@ pub(crate) fn scan_block_with_runner<
     block: CompactBlock,
     vks: &[(&AccountId, &K)],
     nullifiers: &[(AccountId, Nullifier)],
-    tree: &mut CommitmentTree<Node>,
+    tree: &mut sapling::CommitmentTree,
-    existing_witnesses: &mut [&mut IncrementalWitness<Node>],
+    existing_witnesses: &mut [&mut sapling::IncrementalWitness],
     mut batch_runner: Option<&mut TaggedBatchRunner<P, K::Scope, T>>,
 ) -> Vec<WalletTx<K::Nf>> {
     let mut wtxs: Vec<WalletTx<K::Nf>> = vec![];
@@ -350,7 +349,7 @@ pub(crate) fn scan_block_with_runner<
                     // - Notes created by consolidation transactions.
                     // - Notes sent from one account to itself.
                     let is_change = spent_from_accounts.contains(&account);
-                    let witness = IncrementalWitness::from_tree(tree);
+                    let witness = sapling::IncrementalWitness::from_tree(tree.clone());
                     let nf = K::sapling_nf(&nk, &note, &witness);
 
                     shielded_outputs.push(WalletSaplingOutput::from_parts(

zcash_client_sqlite/src/lib.rs

@@ -43,8 +43,7 @@ use zcash_primitives::{
     consensus::{self, BlockHeight},
     legacy::TransparentAddress,
     memo::{Memo, MemoBytes},
-    merkle_tree::{CommitmentTree, IncrementalWitness},
+    sapling::{self, Nullifier},
-    sapling::{Node, Nullifier},
     transaction::{
         components::{amount::Amount, OutPoint},
         Transaction, TxId,
@@ -201,7 +200,7 @@ impl<P: consensus::Parameters> WalletRead for WalletDb<P> {
     fn get_commitment_tree(
         &self,
         block_height: BlockHeight,
-    ) -> Result<Option<CommitmentTree<Node>>, Self::Error> {
+    ) -> Result<Option<sapling::CommitmentTree>, Self::Error> {
         wallet::get_sapling_commitment_tree(self, block_height)
     }
 
@@ -209,7 +208,7 @@ impl<P: consensus::Parameters> WalletRead for WalletDb<P> {
     fn get_witnesses(
         &self,
         block_height: BlockHeight,
-    ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error> {
+    ) -> Result<Vec<(Self::NoteRef, sapling::IncrementalWitness)>, Self::Error> {
         wallet::get_sapling_witnesses(self, block_height)
     }
 
@@ -357,7 +356,7 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> {
     fn get_commitment_tree(
         &self,
         block_height: BlockHeight,
-    ) -> Result<Option<CommitmentTree<Node>>, Self::Error> {
+    ) -> Result<Option<sapling::CommitmentTree>, Self::Error> {
         self.wallet_db.get_commitment_tree(block_height)
     }
 
@@ -365,7 +364,7 @@ impl<'a, P: consensus::Parameters> WalletRead for DataConnStmtCache<'a, P> {
     fn get_witnesses(
         &self,
         block_height: BlockHeight,
-    ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error> {
+    ) -> Result<Vec<(Self::NoteRef, sapling::IncrementalWitness)>, Self::Error> {
         self.wallet_db.get_witnesses(block_height)
     }
 
@@ -515,8 +514,8 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> {
     fn advance_by_block(
         &mut self,
         block: &PrunedBlock,
-        updated_witnesses: &[(Self::NoteRef, IncrementalWitness<Node>)],
+        updated_witnesses: &[(Self::NoteRef, sapling::IncrementalWitness)],
-    ) -> Result<Vec<(Self::NoteRef, IncrementalWitness<Node>)>, Self::Error> {
+    ) -> Result<Vec<(Self::NoteRef, sapling::IncrementalWitness)>, Self::Error> {
         // database updates for each block are transactional
         self.transactionally(|up| {
             // Insert the block into the database.

zcash_client_sqlite/src/prepared.rs

@@ -12,8 +12,7 @@ use zcash_primitives::{
     block::BlockHash,
     consensus::{self, BlockHeight},
     memo::MemoBytes,
-    merkle_tree::{CommitmentTree, IncrementalWitness},
+    sapling::{self, Diversifier, Nullifier},
-    sapling::{Diversifier, Node, Nullifier},
     transaction::{components::Amount, TxId},
     zip32::{AccountId, DiversifierIndex},
 };
@@ -277,7 +276,7 @@ impl<'a, P> DataConnStmtCache<'a, P> {
         block_height: BlockHeight,
         block_hash: BlockHash,
         block_time: u32,
-        commitment_tree: &CommitmentTree<Node>,
+        commitment_tree: &sapling::CommitmentTree,
     ) -> Result<(), SqliteClientError> {
         let mut encoded_tree = Vec::new();
         commitment_tree.write(&mut encoded_tree).unwrap();
@@ -777,7 +776,7 @@ impl<'a, P> DataConnStmtCache<'a, P> {
         &mut self,
         note_id: NoteId,
         height: BlockHeight,
-        witness: &IncrementalWitness<Node>,
+        witness: &sapling::IncrementalWitness,
     ) -> Result<(), SqliteClientError> {
         let note_id = match note_id {
             NoteId::ReceivedNoteId(note_id) => Ok(note_id),

zcash_client_sqlite/src/wallet.rs

@@ -73,8 +73,7 @@ use zcash_primitives::{
     block::BlockHash,
     consensus::{self, BlockHeight, BranchId, NetworkUpgrade, Parameters},
     memo::{Memo, MemoBytes},
-    merkle_tree::{CommitmentTree, IncrementalWitness},
+    sapling::{self, Note, Nullifier},
-    sapling::{Node, Note, Nullifier},
     transaction::{components::Amount, Transaction, TxId},
     zip32::{
         sapling::{DiversifiableFullViewingKey, ExtendedFullViewingKey},
@@ -687,14 +686,14 @@ pub(crate) fn truncate_to_height<P: consensus::Parameters>(
 pub(crate) fn get_sapling_commitment_tree<P>(
     wdb: &WalletDb<P>,
     block_height: BlockHeight,
-) -> Result<Option<CommitmentTree<Node>>, SqliteClientError> {
+) -> Result<Option<sapling::CommitmentTree>, SqliteClientError> {
     wdb.conn
         .query_row_and_then(
             "SELECT sapling_tree FROM blocks WHERE height = ?",
             [u32::from(block_height)],
             |row| {
                 let row_data: Vec<u8> = row.get(0)?;
-                CommitmentTree::read(&row_data[..]).map_err(|e| {
+                sapling::CommitmentTree::read(&row_data[..]).map_err(|e| {
                     rusqlite::Error::FromSqlConversionFailure(
                         row_data.len(),
                         rusqlite::types::Type::Blob,
@@ -712,7 +711,7 @@ pub(crate) fn get_sapling_commitment_tree<P>(
 pub(crate) fn get_sapling_witnesses<P>(
     wdb: &WalletDb<P>,
     block_height: BlockHeight,
-) -> Result<Vec<(NoteId, IncrementalWitness<Node>)>, SqliteClientError> {
+) -> Result<Vec<(NoteId, sapling::IncrementalWitness)>, SqliteClientError> {
     let mut stmt_fetch_witnesses = wdb
         .conn
         .prepare("SELECT note, witness FROM sapling_witnesses WHERE block = ?")?;
@@ -720,7 +719,7 @@ pub(crate) fn get_sapling_witnesses<P>(
         .query_map([u32::from(block_height)], |row| {
             let id_note = NoteId::ReceivedNoteId(row.get(0)?);
             let wdb: Vec<u8> = row.get(1)?;
-            Ok(IncrementalWitness::read(&wdb[..]).map(|witness| (id_note, witness)))
+            Ok(sapling::IncrementalWitness::read(&wdb[..]).map(|witness| (id_note, witness)))
         })
         .map_err(SqliteClientError::from)?;
 
@@ -884,7 +883,7 @@ pub(crate) fn insert_block<'a, P>(
     block_height: BlockHeight,
     block_hash: BlockHash,
     block_time: u32,
-    commitment_tree: &CommitmentTree<Node>,
+    commitment_tree: &sapling::CommitmentTree,
 ) -> Result<(), SqliteClientError> {
     stmts.stmt_insert_block(block_height, block_hash, block_time, commitment_tree)
 }
@@ -1049,7 +1048,7 @@ pub(crate) fn put_received_note<'a, P, T: ReceivedSaplingOutput>(
 pub(crate) fn insert_witness<'a, P>(
     stmts: &mut DataConnStmtCache<'a, P>,
     note_id: i64,
-    witness: &IncrementalWitness<Node>,
+    witness: &sapling::IncrementalWitness,
     height: BlockHeight,
 ) -> Result<(), SqliteClientError> {
     stmts.stmt_insert_witness(NoteId::ReceivedNoteId(note_id), height, witness)

zcash_extensions/src/transparent/demo.rs

@@ -820,7 +820,7 @@ mod tests {
         // fake that the note appears in some previous
         // shielded output
         tree.append(cm1).unwrap();
-        let witness1 = IncrementalWitness::from_tree(&tree);
+        let witness1 = IncrementalWitness::from_tree(tree);
 
         let mut builder_a = demo_builder(tx_height);
         builder_a

zcash_primitives/CHANGELOG.md

@@ -12,17 +12,22 @@ and this library adheres to Rust's notion of
   `incrementalmerkletree::Hashable` and `merkle_tree::HashSer`.
 - The `Hashable` bound on the `Node` parameter to the `IncrementalWitness`
   type has been removed.
+- `sapling::SAPLING_COMMITMENT_TREE_DEPTH_U8` and `sapling::SAPLING_COMMITMENT_TREE_DEPTH` 
+  have been removed; use `sapling::NOTE_COMMITMENT_TREE_DEPTH` instead.
 - `merkle_tree::incremental::write_auth_fragment_v1`
 
 ### Added
 - `merkle_tree::incremental::{read_address, write_address}`
 - `merkle_tree::incremental::read_bridge_v2`
+- `sapling::{CommitmentTree, IncrementalWitness, MerklePath, NOTE_COMMITMENT_TREE_DEPTH}`
 
 ### Changed
 - The bounds on the `H` parameter to the following methods have changed:
   - `merkle_tree::incremental::read_frontier_v0`
   - `merkle_tree::incremental::read_auth_fragment_v1`
-
+- The depth of the `merkle_tree::CommitmentTree`, `merkle_tree::IncrementalWitness` 
+  and `merkle_tree::incremental::MerklePath` data types are now statically constrained 
+  using const generic type parameters.
 
 ## [0.11.0] - 2023-04-15
 ### Added

zcash_primitives/src/merkle_tree.rs

@@ -1,15 +1,14 @@
 //! Implementation of a Merkle tree of commitments used to prove the existence of notes.
 
+use bridgetree::Frontier;
 use byteorder::{LittleEndian, ReadBytesExt};
-use incrementalmerkletree::Hashable;
+use incrementalmerkletree::{Hashable, Level};
 use std::collections::VecDeque;
 use std::convert::TryInto;
 use std::io::{self, Read, Write};
 use std::iter::repeat;
 use zcash_encoding::{Optional, Vector};
 
-use crate::sapling::SAPLING_COMMITMENT_TREE_DEPTH_U8;
-
 pub mod incremental;
 
 /// A hashable node within a Merkle tree.
@@ -34,25 +33,22 @@ impl<Node: Hashable> PathFiller<Node> {
         }
     }
 
-    fn next(&mut self, depth: u8) -> Node {
+    fn next(&mut self, level: Level) -> Node {
         self.queue
             .pop_front()
-            .unwrap_or_else(|| Node::empty_root(depth.into()))
+            .unwrap_or_else(|| Node::empty_root(level))
     }
 }
 
 /// A Merkle tree of note commitments.
-///
-/// The depth of the Merkle tree is fixed at 32, equal to the depth of the Sapling
-/// commitment tree.
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub struct CommitmentTree<Node> {
+pub struct CommitmentTree<Node, const DEPTH: u8> {
     pub(crate) left: Option<Node>,
     pub(crate) right: Option<Node>,
     pub(crate) parents: Vec<Option<Node>>,
 }
 
-impl<Node> CommitmentTree<Node> {
+impl<Node, const DEPTH: u8> CommitmentTree<Node, DEPTH> {
     /// Creates an empty tree.
     pub fn empty() -> Self {
         CommitmentTree {
@@ -62,7 +58,7 @@ impl<Node> CommitmentTree<Node> {
         }
     }
 
-    pub fn from_frontier<const DEPTH: u8>(frontier: &bridgetree::Frontier<Node, DEPTH>) -> Self
+    pub fn from_frontier(frontier: &Frontier<Node, DEPTH>) -> Self
     where
         Node: Clone,
     {
@@ -97,12 +93,12 @@ impl<Node> CommitmentTree<Node> {
         })
     }
 
-    pub fn to_frontier<const DEPTH: u8>(&self) -> bridgetree::Frontier<Node, DEPTH>
+    pub fn to_frontier(&self) -> Frontier<Node, DEPTH>
     where
         Node: Hashable + Clone,
     {
         if self.size() == 0 {
-            bridgetree::Frontier::empty()
+            Frontier::empty()
         } else {
             let ommers_iter = self.parents.iter().filter_map(|v| v.as_ref()).cloned();
             let (leaf, ommers) = match (self.left.as_ref(), self.right.as_ref()) {
@@ -116,7 +112,7 @@ impl<Node> CommitmentTree<Node> {
 
             // If a frontier cannot be successfully constructed from the
             // parts of a commitment tree, it is a programming error.
-            bridgetree::Frontier::from_parts((self.size() - 1).into(), leaf, ommers)
+            Frontier::from_parts((self.size() - 1).into(), leaf, ommers)
                 .expect("Frontier should be constructable from CommitmentTree.")
         }
     }
@@ -154,9 +150,12 @@ impl<Node> CommitmentTree<Node> {
     }
 }
 
-impl<Node: Hashable + HashSer> CommitmentTree<Node> {
+impl<Node: Hashable + Clone, const DEPTH: u8> CommitmentTree<Node, DEPTH> {
     /// Reads a `CommitmentTree` from its serialized form.
-    pub fn read<R: Read>(mut reader: R) -> io::Result<Self> {
+    pub fn read<R: Read>(mut reader: R) -> io::Result<Self>
+    where
+        Node: HashSer,
+    {
         let left = Optional::read(&mut reader, Node::read)?;
         let right = Optional::read(&mut reader, Node::read)?;
         let parents = Vector::read(&mut reader, |r| Optional::read(r, Node::read))?;
@@ -169,23 +168,26 @@ impl<Node: Hashable + HashSer> CommitmentTree<Node> {
     }
 
     /// Serializes this tree as an array of bytes.
-    pub fn write<W: Write>(&self, mut writer: W) -> io::Result<()> {
+    pub fn write<W: Write>(&self, mut writer: W) -> io::Result<()>
+    where
+        Node: HashSer,
+    {
         Optional::write(&mut writer, self.left.as_ref(), |w, n| n.write(w))?;
         Optional::write(&mut writer, self.right.as_ref(), |w, n| n.write(w))?;
         Vector::write(&mut writer, &self.parents, |w, e| {
             Optional::write(w, e.as_ref(), |w, n| n.write(w))
         })
     }
-}
 
-impl<Node: Hashable + Clone> CommitmentTree<Node> {
     /// Adds a leaf node to the tree.
     ///
     /// Returns an error if the tree is full.
+    #[allow(clippy::result_unit_err)]
     pub fn append(&mut self, node: Node) -> Result<(), ()> {
-        self.append_inner(node, SAPLING_COMMITMENT_TREE_DEPTH_U8)
+        self.append_inner(node, DEPTH)
     }
 
+    #[allow(clippy::result_unit_err)]
     fn append_inner(&mut self, node: Node, depth: u8) -> Result<(), ()> {
         if self.is_complete(depth) {
             // Tree is full
@@ -223,7 +225,7 @@ impl<Node: Hashable + Clone> CommitmentTree<Node> {
 
     /// Returns the current root of the tree.
     pub fn root(&self) -> Node {
-        self.root_inner(SAPLING_COMMITMENT_TREE_DEPTH_U8, PathFiller::empty())
+        self.root_inner(DEPTH, PathFiller::empty())
     }
 
     fn root_inner(&self, depth: u8, mut filler: PathFiller<Node>) -> Node {
@@ -231,16 +233,17 @@ impl<Node: Hashable + Clone> CommitmentTree<Node> {
 
         // 1) Hash left and right leaves together.
         //    - Empty leaves are used as needed.
+        //    - Note that `filler.next` is side-effecting and so cannot be factored out.
         let leaf_root = Node::combine(
             0.into(),
             &self
                 .left
                 .as_ref()
-                .map_or_else(|| filler.next(0), |n| n.clone()),
+                .map_or_else(|| filler.next(0.into()), |n| n.clone()),
             &self
                 .right
                 .as_ref()
-                .map_or_else(|| filler.next(0), |n| n.clone()),
+                .map_or_else(|| filler.next(0.into()), |n| n.clone()),
         );
 
         // 2) Extend the parents to the desired depth with None values, then hash from leaf to
@@ -251,10 +254,10 @@ impl<Node: Hashable + Clone> CommitmentTree<Node> {
             .take((depth - 1).into())
             .zip(0u8..)
             .fold(leaf_root, |root, (p, i)| {
-                let parent_level = i + 1;
+                let level = Level::from(i + 1);
                 match p {
-                    Some(node) => Node::combine(parent_level.into(), node, &root),
+                    Some(node) => Node::combine(level, node, &root),
-                    None => Node::combine(parent_level.into(), &root, &filler.next(parent_level)),
+                    None => Node::combine(level, &root, &filler.next(level)),
                 }
             })
     }
@@ -269,54 +272,53 @@ impl<Node: Hashable + Clone> CommitmentTree<Node> {
 /// # Examples
 ///
 /// ```
-/// use ff::{Field, PrimeField};
+/// use zcash_primitives::merkle_tree::{
-/// use rand_core::OsRng;
+///   CommitmentTree,
-/// use zcash_primitives::{
+///   IncrementalWitness,
-///     merkle_tree::{CommitmentTree, IncrementalWitness},
+///   testing::TestNode
-///     sapling::Node,
 /// };
 ///
-/// let mut rng = OsRng;
+/// let mut tree = CommitmentTree::<TestNode, 8>::empty();
 ///
-/// let mut tree = CommitmentTree::<Node>::empty();
+/// tree.append(TestNode(0));
-///
+/// tree.append(TestNode(1));
-/// tree.append(Node::from_scalar(bls12_381::Scalar::random(&mut rng)));
+/// let mut witness = IncrementalWitness::from_tree(tree.clone());
-/// tree.append(Node::from_scalar(bls12_381::Scalar::random(&mut rng)));
-/// let mut witness = IncrementalWitness::from_tree(&tree);
 /// assert_eq!(witness.position(), 1);
 /// assert_eq!(tree.root(), witness.root());
 ///
-/// let cmu = Node::from_scalar(bls12_381::Scalar::random(&mut rng));
+/// let next = TestNode(2);
-/// tree.append(cmu);
+/// tree.append(next.clone());
-/// witness.append(cmu);
+/// witness.append(next);
 /// assert_eq!(tree.root(), witness.root());
 /// ```
 #[derive(Clone, Debug)]
-pub struct IncrementalWitness<Node> {
+pub struct IncrementalWitness<Node, const DEPTH: u8> {
-    tree: CommitmentTree<Node>,
+    tree: CommitmentTree<Node, DEPTH>,
     filled: Vec<Node>,
     cursor_depth: u8,
-    cursor: Option<CommitmentTree<Node>>,
+    cursor: Option<CommitmentTree<Node, DEPTH>>,
 }
 
-impl<Node: Hashable + HashSer + Clone> IncrementalWitness<Node> {
+impl<Node, const DEPTH: u8> IncrementalWitness<Node, DEPTH> {
     /// Creates an `IncrementalWitness` for the most recent commitment added to the given
     /// [`CommitmentTree`].
-    pub fn from_tree(tree: &CommitmentTree<Node>) -> IncrementalWitness<Node> {
+    pub fn from_tree(tree: CommitmentTree<Node, DEPTH>) -> IncrementalWitness<Node, DEPTH> {
         IncrementalWitness {
-            tree: tree.clone(),
+            tree,
             filled: vec![],
             cursor_depth: 0,
             cursor: None,
         }
     }
+}
 
+impl<Node: Hashable + HashSer + Clone, const DEPTH: u8> IncrementalWitness<Node, DEPTH> {
     /// Reads an `IncrementalWitness` from its serialized form.
     #[allow(clippy::redundant_closure)]
-    pub fn read<R: Read>(mut reader: R) -> io::Result<Self> {
+    pub fn read<R: Read>(mut reader: R) -> io::Result<IncrementalWitness<Node, DEPTH>> {
-        let tree = CommitmentTree::read(&mut reader)?;
+        let tree = CommitmentTree::<Node, DEPTH>::read(&mut reader)?;
         let filled = Vector::read(&mut reader, |r| Node::read(r))?;
-        let cursor = Optional::read(&mut reader, CommitmentTree::read)?;
+        let cursor = Optional::read(&mut reader, CommitmentTree::<Node, DEPTH>::read)?;
 
         let mut witness = IncrementalWitness {
             tree,
@@ -395,10 +397,12 @@ impl<Node: Hashable + HashSer + Clone> IncrementalWitness<Node> {
     /// Tracks a leaf node that has been added to the underlying tree.
     ///
     /// Returns an error if the tree is full.
+    #[allow(clippy::result_unit_err)]
     pub fn append(&mut self, node: Node) -> Result<(), ()> {
-        self.append_inner(node, SAPLING_COMMITMENT_TREE_DEPTH_U8)
+        self.append_inner(node, DEPTH)
     }
 
+    #[allow(clippy::result_unit_err)]
     fn append_inner(&mut self, node: Node, depth: u8) -> Result<(), ()> {
         if let Some(mut cursor) = self.cursor.take() {
             cursor
@@ -433,7 +437,7 @@ impl<Node: Hashable + HashSer + Clone> IncrementalWitness<Node> {
 
     /// Returns the current root of the tree corresponding to the witness.
     pub fn root(&self) -> Node {
-        self.root_inner(SAPLING_COMMITMENT_TREE_DEPTH_U8)
+        self.root_inner(DEPTH)
     }
 
     fn root_inner(&self, depth: u8) -> Node {
@@ -441,11 +445,11 @@ impl<Node: Hashable + HashSer + Clone> IncrementalWitness<Node> {
     }
 
     /// Returns the current witness, or None if the tree is empty.
-    pub fn path(&self) -> Option<MerklePath<Node>> {
+    pub fn path(&self) -> Option<MerklePath<Node, DEPTH>> {
-        self.path_inner(SAPLING_COMMITMENT_TREE_DEPTH_U8)
+        self.path_inner(DEPTH)
     }
 
-    fn path_inner(&self, depth: u8) -> Option<MerklePath<Node>> {
+    fn path_inner(&self, depth: u8) -> Option<MerklePath<Node, DEPTH>> {
         let mut filler = self.filler();
         let mut auth_path = Vec::new();
 
@@ -453,7 +457,7 @@ impl<Node: Hashable + HashSer + Clone> IncrementalWitness<Node> {
             if self.tree.right.is_some() {
                 auth_path.push((node.clone(), true));
             } else {
-                auth_path.push((filler.next(0), false));
+                auth_path.push((filler.next(0.into()), false));
             }
         } else {
             // Can't create an authentication path for the beginning of the tree
@@ -470,41 +474,51 @@ impl<Node: Hashable + HashSer + Clone> IncrementalWitness<Node> {
         {
             auth_path.push(match p {
                 Some(node) => (node.clone(), true),
-                None => (filler.next(i + 1), false),
+                None => (filler.next(Level::from(i + 1)), false),
             });
         }
 
-        assert_eq!(auth_path.len(), depth.into());
+        assert_eq!(auth_path.len(), usize::from(depth));
 
-        Some(MerklePath::from_path(auth_path, self.position() as u64))
+        MerklePath::from_path(auth_path, self.position() as u64).ok()
     }
 }
 
 /// A path from a position in a particular commitment tree to the root of that tree.
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub struct MerklePath<Node: Hashable> {
+pub struct MerklePath<Node, const DEPTH: u8> {
-    pub auth_path: Vec<(Node, bool)>,
+    auth_path: Vec<(Node, bool)>,
-    pub position: u64,
+    position: u64,
 }
 
-impl<Node: Hashable + HashSer> MerklePath<Node> {
+impl<Node, const DEPTH: u8> MerklePath<Node, DEPTH> {
+    pub fn auth_path(&self) -> &[(Node, bool)] {
+        &self.auth_path
+    }
+
+    pub fn position(&self) -> u64 {
+        self.position
+    }
+
     /// Constructs a Merkle path directly from a path and position.
-    pub fn from_path(auth_path: Vec<(Node, bool)>, position: u64) -> Self {
+    pub fn from_path(auth_path: Vec<(Node, bool)>, position: u64) -> Result<Self, ()> {
-        MerklePath {
+        if auth_path.len() == usize::from(DEPTH) {
-            auth_path,
+            Ok(MerklePath {
-            position,
+                auth_path,
+                position,
+            })
+        } else {
+            Err(())
         }
     }
+}
 
+impl<Node: Hashable + HashSer, const DEPTH: u8> MerklePath<Node, DEPTH> {
     /// Reads a Merkle path from its serialized form.
-    pub fn from_slice(witness: &[u8]) -> Result<Self, ()> {
+    pub fn from_slice(mut witness: &[u8]) -> Result<Self, ()> {
-        Self::from_slice_with_depth(witness, SAPLING_COMMITMENT_TREE_DEPTH_U8)
+        // Skip the first byte, which should be DEPTH to signify the length of
-    }
-
-    fn from_slice_with_depth(mut witness: &[u8], depth: u8) -> Result<Self, ()> {
-        // Skip the first byte, which should be "depth" to signify the length of
         // the following vector of Pedersen hashes.
-        if witness[0] != depth {
+        if witness[0] != DEPTH {
             return Err(());
         }
         witness = &witness[1..];
@@ -533,7 +547,7 @@ impl<Node: Hashable + HashSer> MerklePath<Node> {
                 }
             })
             .collect::<Result<Vec<_>, _>>()?;
-        if auth_path.len() != depth.into() {
+        if auth_path.len() != usize::from(DEPTH) {
             return Err(());
         }
 
@@ -583,9 +597,8 @@ mod tests {
     use incrementalmerkletree::Hashable;
     use proptest::prelude::*;
     use proptest::strategy::Strategy;
-    use std::io::{self, Read, Write};
 
-    use crate::sapling::{testing::arb_node, Node};
+    use crate::sapling::{self, testing::arb_node, Node};
 
     use super::{
         testing::{arb_commitment_tree, TestNode},
@@ -628,68 +641,6 @@ mod tests {
         "fbc2f4300c01f0b7820d00e3347c8da4ee614674376cbc45359daa54f9b5493e",
     ];
 
-    const TESTING_DEPTH: u8 = 4;
-
-    #[derive(Debug)]
-    struct TestCommitmentTree(CommitmentTree<Node>);
-
-    impl TestCommitmentTree {
-        fn new() -> Self {
-            TestCommitmentTree(CommitmentTree::empty())
-        }
-
-        pub fn read<R: Read>(reader: R) -> io::Result<Self> {
-            let tree = CommitmentTree::read(reader)?;
-            Ok(TestCommitmentTree(tree))
-        }
-
-        pub fn write<W: Write>(&self, writer: W) -> io::Result<()> {
-            self.0.write(writer)
-        }
-
-        fn size(&self) -> usize {
-            self.0.size()
-        }
-
-        fn append(&mut self, node: Node) -> Result<(), ()> {
-            self.0.append_inner(node, TESTING_DEPTH)
-        }
-
-        fn root(&self) -> Node {
-            self.0.root_inner(TESTING_DEPTH, PathFiller::empty())
-        }
-    }
-
-    #[derive(Debug)]
-    struct TestIncrementalWitness(IncrementalWitness<Node>);
-
-    impl TestIncrementalWitness {
-        fn from_tree(tree: &TestCommitmentTree) -> Self {
-            TestIncrementalWitness(IncrementalWitness::from_tree(&tree.0))
-        }
-
-        pub fn read<R: Read>(reader: R) -> io::Result<Self> {
-            let witness = IncrementalWitness::read(reader)?;
-            Ok(TestIncrementalWitness(witness))
-        }
-
-        pub fn write<W: Write>(&self, writer: W) -> io::Result<()> {
-            self.0.write(writer)
-        }
-
-        fn append(&mut self, node: Node) -> Result<(), ()> {
-            self.0.append_inner(node, TESTING_DEPTH)
-        }
-
-        fn root(&self) -> Node {
-            self.0.root_inner(TESTING_DEPTH)
-        }
-
-        fn path(&self) -> Option<MerklePath<Node>> {
-            self.0.path_inner(TESTING_DEPTH)
-        }
-    }
-
     #[test]
     fn empty_root_test_vectors() {
         let mut tmp = [0u8; 32];
@@ -704,7 +655,7 @@ mod tests {
     #[test]
     fn sapling_empty_root() {
         let mut tmp = [0u8; 32];
-        CommitmentTree::<Node>::empty()
+        sapling::CommitmentTree::empty()
             .root()
             .write(&mut tmp[..])
             .expect("length is 32 bytes");
@@ -716,7 +667,7 @@ mod tests {
 
     #[test]
     fn empty_commitment_tree_roots() {
-        let tree = CommitmentTree::<Node>::empty();
+        let tree = sapling::CommitmentTree::empty();
         let mut tmp = [0u8; 32];
         for (&expected, i) in HEX_EMPTY_ROOTS.iter().zip(0u8..).skip(1) {
             tree.root_inner(i, PathFiller::empty())
@@ -1053,40 +1004,46 @@ mod tests {
             "01f43e3aac61e5a753062d4d0508c26ceaf5e4c0c58ba3c956e104b5d2cf67c41c0003015991131c5c25911b35fcea2a8343e2dfd7a4d5b45493390e0cb184394d91c34901002df68503da9247dfde6585cb8c9fa94897cf21735f8fc1b32116ef474de05c010d6b42350c11df4fcc17987c13d8492ba4e8b3f31eb9baff9be5d8890cfa512d013a3661bc12b72646c94bc6c92796e81953985ee62d80a9ec3645a9a95740ac1500",
         ];
 
+        const TESTING_DEPTH: u8 = 4;
+
         fn assert_root_eq(root: Node, expected: &str) {
             let mut tmp = [0u8; 32];
             root.write(&mut tmp[..]).expect("length is 32 bytes");
             assert_eq!(hex::encode(tmp), expected);
         }
 
-        fn assert_tree_ser_eq(tree: &TestCommitmentTree, expected: &str) {
+        fn assert_tree_ser_eq(tree: &CommitmentTree<Node, TESTING_DEPTH>, expected: &str) {
             // Check that the tree matches its encoding
             let mut tmp = Vec::new();
             tree.write(&mut tmp).unwrap();
             assert_eq!(hex::encode(&tmp[..]), expected);
 
             // Check round-trip encoding
-            let decoded = TestCommitmentTree::read(&hex::decode(expected).unwrap()[..]).unwrap();
+            let decoded: CommitmentTree<Node, TESTING_DEPTH> =
+                CommitmentTree::read(&hex::decode(expected).unwrap()[..]).unwrap();
             tmp.clear();
             decoded.write(&mut tmp).unwrap();
             assert_eq!(hex::encode(tmp), expected);
         }
 
-        fn assert_witness_ser_eq(witness: &TestIncrementalWitness, expected: &str) {
+        fn assert_witness_ser_eq(
+            witness: &IncrementalWitness<Node, TESTING_DEPTH>,
+            expected: &str,
+        ) {
             // Check that the witness matches its encoding
             let mut tmp = Vec::new();
             witness.write(&mut tmp).unwrap();
             assert_eq!(hex::encode(&tmp[..]), expected);
 
             // Check round-trip encoding
-            let decoded =
+            let decoded: IncrementalWitness<Node, TESTING_DEPTH> =
-                TestIncrementalWitness::read(&hex::decode(expected).unwrap()[..]).unwrap();
+                IncrementalWitness::read(&hex::decode(expected).unwrap()[..]).unwrap();
             tmp.clear();
             decoded.write(&mut tmp).unwrap();
             assert_eq!(hex::encode(tmp), expected);
         }
 
-        let mut tree = TestCommitmentTree::new();
+        let mut tree = CommitmentTree::<Node, TESTING_DEPTH>::empty();
         assert_eq!(tree.size(), 0);
 
         let mut witnesses = vec![];
@@ -1099,7 +1056,7 @@ mod tests {
             let cmu = Node::new(cmu[..].try_into().unwrap());
 
             // Witness here
-            witnesses.push((TestIncrementalWitness::from_tree(&tree), last_cmu));
+            witnesses.push((IncrementalWitness::from_tree(tree.clone()), last_cmu));
 
             // Now append a commitment to the tree
             assert!(tree.append(cmu).is_ok());
@@ -1119,11 +1076,8 @@ mod tests {
 
                 if let Some(leaf) = leaf {
                     let path = witness.path().expect("should be able to create a path");
-                    let expected = MerklePath::from_slice_with_depth(
+                    let expected =
-                        &hex::decode(paths[paths_i]).unwrap(),
+                        MerklePath::from_slice(&hex::decode(paths[paths_i]).unwrap()).unwrap();
-                        TESTING_DEPTH,
-                    )
-                    .unwrap();
                     assert_eq!(path, expected);
 
                     assert_eq!(path.root(*leaf), witness.root());
@@ -1176,7 +1130,7 @@ mod tests {
 
     proptest! {
         #[test]
-        fn prop_commitment_tree_roundtrip_str(ct in arb_commitment_tree(32, any::<char>().prop_map(|c| c.to_string()), 8)) {
+        fn prop_commitment_tree_roundtrip_str(ct in arb_commitment_tree::<_, _, 8>(32, any::<char>().prop_map(|c| c.to_string()))) {
             let frontier: Frontier<String, 8> = ct.to_frontier();
             let ct0 = CommitmentTree::from_frontier(&frontier);
             assert_eq!(ct, ct0);
@@ -1185,7 +1139,7 @@ mod tests {
         }
 
         #[test]
-        fn prop_commitment_tree_roundtrip_node(ct in arb_commitment_tree(32, arb_node(), 8)) {
+        fn prop_commitment_tree_roundtrip_node(ct in arb_commitment_tree::<_, _, 8>(32, arb_node())) {
             let frontier: Frontier<Node, 8> = ct.to_frontier();
             let ct0 = CommitmentTree::from_frontier(&frontier);
             assert_eq!(ct, ct0);
@@ -1194,21 +1148,21 @@ mod tests {
         }
 
         #[test]
-        fn prop_commitment_tree_roundtrip_ser(ct in arb_commitment_tree(32, arb_node(), 8)) {
+        fn prop_commitment_tree_roundtrip_ser(ct in arb_commitment_tree::<_, _, 8>(32, arb_node())) {
             let mut serialized = vec![];
             assert_matches!(ct.write(&mut serialized), Ok(()));
-            assert_matches!(CommitmentTree::read(&serialized[..]), Ok(ct_out) if ct == ct_out);
+            assert_matches!(CommitmentTree::<_, 8>::read(&serialized[..]), Ok(ct_out) if ct == ct_out);
         }
     }
 
     #[test]
     fn test_commitment_tree_complete() {
-        let mut t: CommitmentTree<TestNode> = CommitmentTree::empty();
+        let mut t: CommitmentTree<TestNode, 32> = CommitmentTree::empty();
         for n in 1u64..=32 {
             t.append(TestNode(n)).unwrap();
             // every tree of a power-of-two height is complete
             let is_complete = n.count_ones() == 1;
-            let level = 63 - n.leading_zeros(); //log2
+            let level = usize::BITS - 1 - n.leading_zeros(); //log2
             assert_eq!(
                 is_complete,
                 t.is_complete(level.try_into().unwrap()),
@@ -1238,18 +1192,21 @@ pub mod testing {
 
     use super::{CommitmentTree, HashSer};
 
-    pub fn arb_commitment_tree<Node: Hashable + Clone + Debug, T: Strategy<Value = Node>>(
+    pub fn arb_commitment_tree<
+        Node: Hashable + Clone + Debug,
+        T: Strategy<Value = Node>,
+        const DEPTH: u8,
+    >(
         min_size: usize,
         arb_node: T,
-        depth: u8,
+    ) -> impl Strategy<Value = CommitmentTree<Node, DEPTH>> {
-    ) -> impl Strategy<Value = CommitmentTree<Node>> {
+        assert!((1 << DEPTH) >= min_size + 100);
-        assert!((1 << depth) >= min_size + 100);
         vec(arb_node, min_size..(min_size + 100)).prop_map(move |v| {
             let mut tree = CommitmentTree::empty();
             for node in v.into_iter() {
                 tree.append(node).unwrap();
             }
-            tree.parents.resize_with((depth - 1).into(), || None);
+            tree.parents.resize_with((DEPTH - 1).into(), || None);
             tree
         })
     }

zcash_primitives/src/merkle_tree/incremental.rs

@@ -10,6 +10,7 @@ use orchard::tree::MerkleHashOrchard;
 use zcash_encoding::{Optional, Vector};
 
 use super::{CommitmentTree, HashSer};
+use crate::sapling;
 
 pub const SER_V1: u8 = 1;
 pub const SER_V2: u8 = 2;
@@ -78,8 +79,8 @@ pub fn read_address<R: Read>(mut reader: R) -> io::Result<Address> {
 
 pub fn read_frontier_v0<H: Hashable + HashSer + Clone, R: Read>(
     mut reader: R,
-) -> io::Result<Frontier<H, 32>> {
+) -> io::Result<Frontier<H, { sapling::NOTE_COMMITMENT_TREE_DEPTH }>> {
-    let tree = CommitmentTree::read(&mut reader)?;
+    let tree = CommitmentTree::<H, { sapling::NOTE_COMMITMENT_TREE_DEPTH }>::read(&mut reader)?;
 
     Ok(tree.to_frontier())
 }
@@ -317,7 +318,7 @@ mod tests {
 
     proptest! {
         #[test]
-        fn frontier_serialization_v0(t in arb_commitment_tree(0, sapling::arb_node(), 32))
+        fn frontier_serialization_v0(t in arb_commitment_tree::<_, _, 32>(0, sapling::arb_node()))
         {
             let mut buffer = vec![];
             t.write(&mut buffer).unwrap();
@@ -328,7 +329,7 @@ mod tests {
         }
 
         #[test]
-        fn frontier_serialization_v1(t in arb_commitment_tree(1, sapling::arb_node(), 32))
+        fn frontier_serialization_v1(t in arb_commitment_tree::<_, _, 32>(1, sapling::arb_node()))
         {
             let original: Frontier<Node, 32> = t.to_frontier();
 

zcash_primitives/src/sapling.rs

@@ -24,7 +24,7 @@ pub use address::PaymentAddress;
 pub use keys::{Diversifier, NullifierDerivingKey, ProofGenerationKey, SaplingIvk, ViewingKey};
 pub use note::{nullifier::Nullifier, Note, Rseed};
 pub use tree::{
-    merkle_hash, Node, SAPLING_COMMITMENT_TREE_DEPTH, SAPLING_COMMITMENT_TREE_DEPTH_U8,
+    merkle_hash, CommitmentTree, IncrementalWitness, MerklePath, Node, NOTE_COMMITMENT_TREE_DEPTH,
 };
 
 /// Create the spendAuthSig for a Sapling SpendDescription.

zcash_primitives/src/sapling/prover.rs

@@ -1,11 +1,10 @@
 //! Abstractions over the proving system and parameters.
 
 use crate::{
-    merkle_tree::MerklePath,
     sapling::{
+        self,
         redjubjub::{PublicKey, Signature},
         value::ValueCommitment,
-        Node,
     },
     transaction::components::{Amount, GROTH_PROOF_SIZE},
 };
@@ -35,7 +34,7 @@ pub trait TxProver {
         ar: jubjub::Fr,
         value: u64,
         anchor: bls12_381::Scalar,
-        merkle_path: MerklePath<Node>,
+        merkle_path: sapling::MerklePath,
     ) -> Result<([u8; GROTH_PROOF_SIZE], ValueCommitment, PublicKey), ()>;
 
     /// Create the value commitment and proof for a Sapling [`OutputDescription`],
@@ -69,11 +68,11 @@ pub mod mock {
 
     use crate::{
         constants::SPENDING_KEY_GENERATOR,
-        merkle_tree::MerklePath,
         sapling::{
+            self,
             redjubjub::{PublicKey, Signature},
             value::{NoteValue, ValueCommitTrapdoor, ValueCommitment},
-            Diversifier, Node, PaymentAddress, ProofGenerationKey, Rseed,
+            Diversifier, PaymentAddress, ProofGenerationKey, Rseed,
         },
         transaction::components::{Amount, GROTH_PROOF_SIZE},
     };
@@ -96,7 +95,7 @@ pub mod mock {
             ar: jubjub::Fr,
             value: u64,
             _anchor: bls12_381::Scalar,
-            _merkle_path: MerklePath<Node>,
+            _merkle_path: sapling::MerklePath,
         ) -> Result<([u8; GROTH_PROOF_SIZE], ValueCommitment, PublicKey), ()> {
             let mut rng = OsRng;
 

zcash_primitives/src/sapling/tree.rs

@@ -8,16 +8,18 @@ use super::{
     note::ExtractedNoteCommitment,
     pedersen_hash::{pedersen_hash, Personalization},
 };
-use crate::merkle_tree::HashSer;
+use crate::merkle_tree::{self, HashSer};
 
-pub const SAPLING_COMMITMENT_TREE_DEPTH: usize = 32;
+pub const NOTE_COMMITMENT_TREE_DEPTH: u8 = 32;
-pub const SAPLING_COMMITMENT_TREE_DEPTH_U8: u8 = 32;
+pub type CommitmentTree = merkle_tree::CommitmentTree<Node, NOTE_COMMITMENT_TREE_DEPTH>;
+pub type IncrementalWitness = merkle_tree::IncrementalWitness<Node, NOTE_COMMITMENT_TREE_DEPTH>;
+pub type MerklePath = merkle_tree::MerklePath<Node, NOTE_COMMITMENT_TREE_DEPTH>;
 
 lazy_static! {
     static ref UNCOMMITTED_SAPLING: bls12_381::Scalar = bls12_381::Scalar::one();
     static ref EMPTY_ROOTS: Vec<Node> = {
         let mut v = vec![Node::empty_leaf()];
-        for d in 0..SAPLING_COMMITMENT_TREE_DEPTH_U8 {
+        for d in 0..NOTE_COMMITMENT_TREE_DEPTH {
             let next = Node::combine(d.into(), &v[usize::from(d)], &v[usize::from(d)]);
             v.push(next);
         }

zcash_primitives/src/transaction/builder.rs

@@ -5,9 +5,6 @@ use std::error;
 use std::fmt;
 use std::sync::mpsc::Sender;
 
-#[cfg(not(feature = "zfuture"))]
-use std::marker::PhantomData;
-
 use rand::{rngs::OsRng, CryptoRng, RngCore};
 
 use crate::{
@@ -15,14 +12,13 @@ use crate::{
     keys::OutgoingViewingKey,
     legacy::TransparentAddress,
     memo::MemoBytes,
-    merkle_tree::MerklePath,
+    sapling::{self, prover::TxProver, value::NoteValue, Diversifier, Note, PaymentAddress},
-    sapling::{prover::TxProver, value::NoteValue, Diversifier, Node, Note, PaymentAddress},
     transaction::{
         components::{
             amount::{Amount, BalanceError},
             sapling::{
-                self,
+                builder::{self as sapling_builder, SaplingBuilder, SaplingMetadata},
-                builder::{SaplingBuilder, SaplingMetadata},
+                fees as sapling_fees,
             },
             transparent::{self, builder::TransparentBuilder},
         },
@@ -67,7 +63,7 @@ pub enum Error<FeeError> {
     /// An error occurred in constructing the transparent parts of a transaction.
     TransparentBuild(transparent::builder::Error),
     /// An error occurred in constructing the Sapling parts of a transaction.
-    SaplingBuild(sapling::builder::Error),
+    SaplingBuild(sapling_builder::Error),
     /// An error occurred in constructing the TZE parts of a transaction.
     #[cfg(feature = "zfuture")]
     TzeBuild(tze::builder::Error),
@@ -144,7 +140,7 @@ pub struct Builder<'a, P, R> {
     #[cfg(feature = "zfuture")]
     tze_builder: TzeBuilder<'a, TransactionData<Unauthorized>>,
     #[cfg(not(feature = "zfuture"))]
-    tze_builder: PhantomData<&'a ()>,
+    tze_builder: std::marker::PhantomData<&'a ()>,
     progress_notifier: Option<Sender<Progress>>,
 }
 
@@ -173,13 +169,13 @@ impl<'a, P, R> Builder<'a, P, R> {
 
     /// Returns the set of Sapling inputs currently committed to be consumed
     /// by the transaction.
-    pub fn sapling_inputs(&self) -> &[impl sapling::fees::InputView<()>] {
+    pub fn sapling_inputs(&self) -> &[impl sapling_fees::InputView<()>] {
         self.sapling_builder.inputs()
     }
 
     /// Returns the set of Sapling outputs currently set to be produced by
     /// the transaction.
-    pub fn sapling_outputs(&self) -> &[impl sapling::fees::OutputView] {
+    pub fn sapling_outputs(&self) -> &[impl sapling_fees::OutputView] {
         self.sapling_builder.outputs()
     }
 }
@@ -226,7 +222,7 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> {
             #[cfg(feature = "zfuture")]
             tze_builder: TzeBuilder::empty(),
             #[cfg(not(feature = "zfuture"))]
-            tze_builder: PhantomData,
+            tze_builder: std::marker::PhantomData,
             progress_notifier: None,
         }
     }
@@ -240,8 +236,8 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> {
         extsk: ExtendedSpendingKey,
         diversifier: Diversifier,
         note: Note,
-        merkle_path: MerklePath<Node>,
+        merkle_path: sapling::MerklePath,
-    ) -> Result<(), sapling::builder::Error> {
+    ) -> Result<(), sapling_builder::Error> {
         self.sapling_builder
             .add_spend(&mut self.rng, extsk, diversifier, note, merkle_path)
     }
@@ -253,9 +249,9 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> {
         to: PaymentAddress,
         value: Amount,
         memo: MemoBytes,
-    ) -> Result<(), sapling::builder::Error> {
+    ) -> Result<(), sapling_builder::Error> {
         if value.is_negative() {
-            return Err(sapling::builder::Error::InvalidAmount);
+            return Err(sapling_builder::Error::InvalidAmount);
         }
         self.sapling_builder.add_output(
             &mut self.rng,
@@ -555,8 +551,8 @@ mod tests {
         sapling::{Node, Rseed},
         transaction::components::{
             amount::{Amount, DEFAULT_FEE},
-            sapling::builder::{self as build_s},
+            sapling::builder::{self as sapling_builder},
-            transparent::builder::{self as build_t},
+            transparent::builder::{self as transparent_builder},
         },
         zip32::ExtendedSpendingKey,
     };
@@ -567,9 +563,6 @@ mod tests {
     #[cfg(feature = "transparent-inputs")]
     use super::TzeBuilder;
 
-    #[cfg(not(feature = "zfuture"))]
-    use std::marker::PhantomData;
-
     #[cfg(feature = "transparent-inputs")]
     use crate::{
         legacy::keys::{AccountPrivKey, IncomingViewingKey},
@@ -599,7 +592,7 @@ mod tests {
                 Amount::from_i64(-1).unwrap(),
                 MemoBytes::empty()
             ),
-            Err(build_s::Error::InvalidAmount)
+            Err(sapling_builder::Error::InvalidAmount)
         );
     }
 
@@ -626,7 +619,7 @@ mod tests {
             #[cfg(feature = "zfuture")]
             tze_builder: TzeBuilder::empty(),
             #[cfg(not(feature = "zfuture"))]
-            tze_builder: PhantomData,
+            tze_builder: std::marker::PhantomData,
             progress_notifier: None,
         };
 
@@ -672,9 +665,9 @@ mod tests {
 
         let note1 = to.create_note(50000, Rseed::BeforeZip212(jubjub::Fr::random(&mut rng)));
         let cmu1 = Node::from_cmu(&note1.cmu());
-        let mut tree = CommitmentTree::empty();
+        let mut tree = CommitmentTree::<Node, 32>::empty();
         tree.append(cmu1).unwrap();
-        let witness1 = IncrementalWitness::from_tree(&tree);
+        let witness1 = IncrementalWitness::from_tree(tree);
 
         let tx_height = TEST_NETWORK
             .activation_height(NetworkUpgrade::Sapling)
@@ -697,7 +690,7 @@ mod tests {
         // that a binding signature was attempted
         assert_eq!(
             builder.mock_build(),
-            Err(Error::SaplingBuild(build_s::Error::BindingSig))
+            Err(Error::SaplingBuild(sapling_builder::Error::BindingSig))
         );
     }
 
@@ -712,7 +705,7 @@ mod tests {
                 &TransparentAddress::PublicKey([0; 20]),
                 Amount::from_i64(-1).unwrap(),
             ),
-            Err(build_t::Error::InvalidAmount)
+            Err(transparent_builder::Error::InvalidAmount)
         );
     }
 
@@ -780,9 +773,9 @@ mod tests {
 
         let note1 = to.create_note(50999, Rseed::BeforeZip212(jubjub::Fr::random(&mut rng)));
         let cmu1 = Node::from_cmu(&note1.cmu());
-        let mut tree = CommitmentTree::empty();
+        let mut tree = CommitmentTree::<Node, 32>::empty();
         tree.append(cmu1).unwrap();
-        let mut witness1 = IncrementalWitness::from_tree(&tree);
+        let mut witness1 = IncrementalWitness::from_tree(tree.clone());
 
         // Fail if there is insufficient input
         // 0.0003 z-ZEC out, 0.0002 t-ZEC out, 0.00001 t-ZEC fee, 0.00050999 z-ZEC in
@@ -820,7 +813,7 @@ mod tests {
         let cmu2 = Node::from_cmu(&note2.cmu());
         tree.append(cmu2).unwrap();
         witness1.append(cmu2).unwrap();
-        let witness2 = IncrementalWitness::from_tree(&tree);
+        let witness2 = IncrementalWitness::from_tree(tree);
 
         // Succeeds if there is sufficient input
         // 0.0003 z-ZEC out, 0.0002 t-ZEC out, 0.0001 t-ZEC fee, 0.0006 z-ZEC in
@@ -856,7 +849,7 @@ mod tests {
                 .unwrap();
             assert_eq!(
                 builder.mock_build(),
-                Err(Error::SaplingBuild(build_s::Error::BindingSig))
+                Err(Error::SaplingBuild(sapling_builder::Error::BindingSig))
             )
         }
     }

zcash_primitives/src/transaction/components/sapling/builder.rs

@@ -10,7 +10,6 @@ use crate::{
     consensus::{self, BlockHeight},
     keys::OutgoingViewingKey,
     memo::MemoBytes,
-    merkle_tree::MerklePath,
     sapling::{
         keys::SaplingIvk,
         note_encryption::sapling_note_encryption,
@@ -19,7 +18,7 @@ use crate::{
         spend_sig_internal,
         util::generate_random_rseed_internal,
         value::{NoteValue, ValueSum},
-        Diversifier, Node, Note, PaymentAddress,
+        Diversifier, MerklePath, Node, Note, PaymentAddress,
     },
     transaction::{
         builder::Progress,
@@ -67,7 +66,7 @@ pub struct SpendDescriptionInfo {
     diversifier: Diversifier,
     note: Note,
     alpha: jubjub::Fr,
-    merkle_path: MerklePath<Node>,
+    merkle_path: MerklePath,
 }
 
 impl fees::InputView<()> for SpendDescriptionInfo {
@@ -280,7 +279,7 @@ impl<P: consensus::Parameters> SaplingBuilder<P> {
         extsk: ExtendedSpendingKey,
         diversifier: Diversifier,
         note: Note,
-        merkle_path: MerklePath<Node>,
+        merkle_path: MerklePath,
     ) -> Result<(), Error> {
         // Consistency check: all anchors must equal the first one
         let node = Node::from_cmu(&note.cmu());
@@ -392,7 +391,7 @@ impl<P: consensus::Parameters> SaplingBuilder<P> {
 
                     let nullifier = spend.note.nf(
                         &proof_generation_key.to_viewing_key().nk,
-                        spend.merkle_path.position,
+                        spend.merkle_path.position(),
                     );
 
                     let (zkproof, cv, rk) = prover
@@ -610,8 +609,8 @@ pub mod testing {
                 n_notes
             ),
             commitment_trees in vec(
-                arb_commitment_tree(n_notes, arb_node(), 32).prop_map(
+                arb_commitment_tree::<_, _, 32>(n_notes, arb_node()).prop_map(
-                    |t| IncrementalWitness::from_tree(&t).path().unwrap()
+                    |t| IncrementalWitness::from_tree(t).path().unwrap()
                 ),
                 n_notes
             ),

zcash_proofs/CHANGELOG.md

@@ -7,6 +7,9 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 
+### Removed
+- `circuit::sapling::TREE_DEPTH` use `zcash_primitives::sapling::NOTE_COMMITMENT_TREE_DEPTH` instead
+
 ## [0.11.0] - 2023-04-15
 ### Changed
 - Bumped dependencies to `bls12_381 0.8`, `group 0.13`, `jubjub 0.10`,

zcash_proofs/src/circuit/sapling.rs

@@ -6,9 +6,7 @@ use bellman::{Circuit, ConstraintSystem, SynthesisError};
 
 use zcash_primitives::constants;
 
-use zcash_primitives::sapling::{
+use zcash_primitives::sapling::{PaymentAddress, ProofGenerationKey};
-    PaymentAddress, ProofGenerationKey, SAPLING_COMMITMENT_TREE_DEPTH,
-};
 
 use super::ecc;
 use super::pedersen_hash;
@@ -29,8 +27,6 @@ use group::ff::PrimeFieldBits;
 #[cfg(test)]
 use zcash_primitives::sapling::value::NoteValue;
 
-pub const TREE_DEPTH: usize = SAPLING_COMMITMENT_TREE_DEPTH;
-
 /// The opening (value and randomness) of a Sapling value commitment.
 #[derive(Clone)]
 pub struct ValueCommitmentOpening {

zcash_proofs/src/prover.rs

@@ -4,12 +4,11 @@ use bellman::groth16::{Parameters, PreparedVerifyingKey};
 use bls12_381::Bls12;
 use std::path::Path;
 use zcash_primitives::{
-    merkle_tree::MerklePath,
     sapling::{
         prover::TxProver,
         redjubjub::{PublicKey, Signature},
         value::ValueCommitment,
-        Diversifier, Node, PaymentAddress, ProofGenerationKey, Rseed,
+        Diversifier, MerklePath, PaymentAddress, ProofGenerationKey, Rseed,
     },
     transaction::components::{Amount, GROTH_PROOF_SIZE},
 };
@@ -154,7 +153,7 @@ impl TxProver for LocalTxProver {
         ar: jubjub::Fr,
         value: u64,
         anchor: bls12_381::Scalar,
-        merkle_path: MerklePath<Node>,
+        merkle_path: MerklePath,
     ) -> Result<([u8; GROTH_PROOF_SIZE], ValueCommitment, PublicKey), ()> {
         let (proof, cv, rk) = ctx.spend_proof(
             proof_generation_key,

zcash_proofs/src/sapling/prover.rs

@@ -7,11 +7,10 @@ use group::{Curve, GroupEncoding};
 use rand_core::OsRng;
 use zcash_primitives::{
     constants::{SPENDING_KEY_GENERATOR, VALUE_COMMITMENT_RANDOMNESS_GENERATOR},
-    merkle_tree::MerklePath,
     sapling::{
         redjubjub::{PublicKey, Signature},
         value::{CommitmentSum, NoteValue, TrapdoorSum, ValueCommitTrapdoor, ValueCommitment},
-        Diversifier, Node, Note, PaymentAddress, ProofGenerationKey, Rseed,
+        Diversifier, MerklePath, Note, PaymentAddress, ProofGenerationKey, Rseed,
     },
     transaction::components::Amount,
 };
@@ -52,7 +51,7 @@ impl SaplingProvingContext {
         ar: jubjub::Fr,
         value: u64,
         anchor: bls12_381::Scalar,
-        merkle_path: MerklePath<Node>,
+        merkle_path: MerklePath,
         proving_key: &Parameters<Bls12>,
         verifying_key: &PreparedVerifyingKey<Bls12>,
     ) -> Result<(Proof<Bls12>, ValueCommitment, PublicKey), ()> {
@@ -84,7 +83,7 @@ impl SaplingProvingContext {
         // Let's compute the nullifier while we have the position
         let note = Note::from_parts(payment_address, NoteValue::from_raw(value), rseed);
 
-        let nullifier = note.nf(&viewing_key.nk, merkle_path.position);
+        let nullifier = note.nf(&viewing_key.nk, merkle_path.position());
 
         // We now have the full witness for our circuit
         let instance = Spend {
@@ -94,7 +93,7 @@ impl SaplingProvingContext {
             commitment_randomness: Some(note.rcm()),
             ar: Some(ar),
             auth_path: merkle_path
-                .auth_path
+                .auth_path()
                 .iter()
                 .map(|(node, b)| Some(((*node).into(), *b)))
                 .collect(),