zcash_client_backend: Get commitment tree depth for a given number of confirmations from the database.

This fixes the following bug:

Due to complexities related to non-linear scanning, checkpoints are only
added to the wallet's commitment tree in cases where there are notes
discovered within a scanned block. At present, the `shardtree` API only
makes it possible to add multiple checkpoints of the same tree state
when adding checkpoints at the chain tip, and this functionality is not
used by `zcash_client_backend` because we perform checkpoint insertion
in batches that may contain gaps in the case that multiple blocks
contain no Sapling notes. While it would be possible to fix this by
altering the `shardtree` API to permit explicit insertion of multiple
checkpoints of the same tree state at a given note position, this fix
takes a simpler approach.

Instead of ensuring that a checkpoint exists at every block and
computing the required checkpoint depth directly from the minimum number
of confirmations required when attempting a spend, we alter the
`WalletCommitmentTrees` API to allow internal information of the note
commitment tree to be used to determine this checkpoint depth, given the
minimum number of commitments as an argument. This allows us to select a
usable checkpoint from the sparse checkpoint set that resulted from the
sparse insertion of checkpoints described above.

zcash_client_backend/src/data_api.rs

@@ -867,6 +867,16 @@ pub trait WalletCommitmentTrees {
         Error = Self::Error,
     >;
 
+    /// Returns the depth of the checkpoint in the tree that can be used to create a witness at the
+    /// anchor having the given number of confirmations.
+    ///
+    /// This assumes that at any time a note is added to the tree, a checkpoint is created for the
+    /// end of the block in which that note was discovered.
+    fn get_checkpoint_depth(
+        &self,
+        min_confirmations: NonZeroU32,
+    ) -> Result<usize, ShardTreeError<Self::Error>>;
+
     fn with_sapling_tree_mut<F, A, E>(&mut self, callback: F) -> Result<A, E>
     where
         for<'a> F: FnMut(
@@ -1182,5 +1192,12 @@ pub mod testing {
 
             Ok(())
         }
+
+        fn get_checkpoint_depth(
+            &self,
+            min_confirmations: NonZeroU32,
+        ) -> Result<usize, ShardTreeError<Self::Error>> {
+            Ok(usize::try_from(u32::from(min_confirmations) - 1).unwrap())
+        }
     }
 }

zcash_client_backend/src/data_api/wallet.rs

@@ -488,6 +488,8 @@ where
     // are no possible transparent inputs, so we ignore those
     let mut builder = Builder::new(params.clone(), proposal.min_target_height(), None);
 
+    let checkpoint_depth = wallet_db.get_checkpoint_depth(min_confirmations)?;
+
     wallet_db.with_sapling_tree_mut::<_, _, Error<_, _, _, _, _>>(|sapling_tree| {
         for selected in proposal.sapling_inputs() {
             let (note, key, merkle_path) = select_key_for_note(
@@ -495,7 +497,7 @@ where
                 selected,
                 usk.sapling(),
                 &dfvk,
-                usize::try_from(u32::from(min_confirmations) - 1).unwrap(),
+                checkpoint_depth,
             )?
             .ok_or(Error::NoteMismatch(selected.note_id))?;
 

zcash_client_sqlite/src/lib.rs

@@ -44,7 +44,10 @@ use std::{
 };
 
 use incrementalmerkletree::Position;
-use shardtree::{error::ShardTreeError, ShardTree};
+use shardtree::{
+    error::{QueryError, ShardTreeError},
+    ShardTree,
+};
 use zcash_primitives::{
     block::BlockHash,
     consensus::{self, BlockHeight},
@@ -89,7 +92,7 @@ pub mod serialization;
 
 pub mod wallet;
 use wallet::{
-    commitment_tree::{self, put_shard_roots},
+    commitment_tree::{self, get_checkpoint_depth, put_shard_roots},
     SubtreeScanProgress,
 };
 
@@ -795,6 +798,15 @@ impl<P: consensus::Parameters> WalletCommitmentTrees for WalletDb<rusqlite::Conn
             .map_err(|e| ShardTreeError::Storage(commitment_tree::Error::Query(e)))?;
         Ok(())
     }
+
+    fn get_checkpoint_depth(
+        &self,
+        min_confirmations: NonZeroU32,
+    ) -> Result<usize, ShardTreeError<Self::Error>> {
+        get_checkpoint_depth(&self.conn, SAPLING_TABLES_PREFIX, min_confirmations)
+            .map_err(|e| ShardTreeError::Storage(commitment_tree::Error::Query(e)))?
+            .ok_or(ShardTreeError::Query(QueryError::CheckpointPruned))
+    }
 }
 
 impl<'conn, P: consensus::Parameters> WalletCommitmentTrees for WalletDb<SqlTransaction<'conn>, P> {
@@ -835,6 +847,18 @@ impl<'conn, P: consensus::Parameters> WalletCommitmentTrees for WalletDb<SqlTran
             roots,
         )
     }
+
+    fn get_checkpoint_depth(
+        &self,
+        min_confirmations: NonZeroU32,
+    ) -> Result<usize, ShardTreeError<Self::Error>> {
+        get_checkpoint_depth(self.conn.0, SAPLING_TABLES_PREFIX, min_confirmations)
+            .map_err(|e| ShardTreeError::Storage(commitment_tree::Error::Query(e)))?
+            // `CheckpointPruned` is perhaps a little misleading; in this case it's that
+            // the chain tip is unknown, but if that were the case we should never have been
+            // calling this anyway.
+            .ok_or(ShardTreeError::Query(QueryError::CheckpointPruned))
+    }
 }
 
 /// A handle for the SQLite block source.

zcash_client_sqlite/src/wallet/commitment_tree.rs

@@ -4,6 +4,7 @@ use std::{
     error, fmt,
     io::{self, Cursor},
     marker::PhantomData,
+    num::NonZeroU32,
     ops::Range,
     sync::Arc,
 };
@@ -20,6 +21,8 @@ use zcash_primitives::{consensus::BlockHeight, merkle_tree::HashSer};
 
 use crate::serialization::{read_shard, write_shard};
 
+use super::scan_queue_extrema;
+
 /// Errors that can appear in SQLite-back [`ShardStore`] implementation operations.
 #[derive(Debug)]
 pub enum Error {
@@ -724,8 +727,8 @@ pub(crate) fn get_checkpoint(
         .query_row(
             &format!(
                 "SELECT position
-            FROM {}_tree_checkpoints
-            WHERE checkpoint_id = ?",
+                 FROM {}_tree_checkpoints
+                 WHERE checkpoint_id = ?",
                 table_prefix
             ),
             [u32::from(checkpoint_id)],
@@ -747,6 +750,33 @@ pub(crate) fn get_checkpoint(
         .transpose()
 }
 
+pub(crate) fn get_checkpoint_depth(
+    conn: &rusqlite::Connection,
+    table_prefix: &'static str,
+    min_confirmations: NonZeroU32,
+) -> Result<Option<usize>, rusqlite::Error> {
+    scan_queue_extrema(conn)?
+        .map(|(_, max)| max)
+        .map(|chain_tip| {
+            let max_checkpoint_height =
+                u32::from(chain_tip).saturating_sub(u32::from(min_confirmations) - 1);
+
+            // We exclude from consideration all checkpoints having heights greater than the maximum
+            // checkpoint height. The checkpoint depth is the number of excluded checkpoints + 1.
+            conn.query_row(
+                &format!(
+                    "SELECT COUNT(*) 
+                     FROM {}_tree_checkpoints
+                     WHERE checkpoint_id > :max_checkpoint_height",
+                    table_prefix
+                ),
+                named_params![":max_checkpoint_height": max_checkpoint_height],
+                |row| row.get::<_, usize>(0).map(|s| s + 1),
+            )
+        })
+        .transpose()
+}
+
 pub(crate) fn get_checkpoint_at_depth(
     conn: &rusqlite::Connection,
     table_prefix: &'static str,