zcash_client_sqlite: Bound truncation to checkpointed heights.

zcash_client_backend/src/data_api.rs

@@ -2038,20 +2038,27 @@ pub trait WalletWrite: WalletRead {
         transactions: &[SentTransaction<Self::AccountId>],
     ) -> Result<(), Self::Error>;
 
-    /// Truncates the wallet database to the specified height.
+    /// Truncates the wallet database to at most the specified height.
     ///
-    /// This method assumes that the state of the underlying data store is
+    /// Implementations of this method may choose a lower block height to which the data store will
-    /// consistent up to a particular block height. Since it is possible that
+    /// be truncated if it is not possible to truncate exactly to the specified height. Upon
-    /// a chain reorg might invalidate some stored state, this method must be
+    /// successful truncation, this method returns the height to which the data store was actually
-    /// implemented in order to allow users of this API to "reset" the data store
+    /// truncated.
-    /// to correctly represent chainstate as of a specified block height.
     ///
-    /// After calling this method, the block at the given height will be the
+    /// This method assumes that the state of the underlying data store is consistent up to a
-    /// most recent block and all other operations will treat this block
+    /// particular block height. Since it is possible that a chain reorg might invalidate some
-    /// as the chain tip for balance determination purposes.
+    /// stored state, this method must be implemented in order to allow users of this API to
+    /// "reset" the data store to correctly represent chainstate as of at most the requested block
+    /// height.
     ///
-    /// There may be restrictions on heights to which it is possible to truncate.
+    /// After calling this method, the block at the returned height will be the most recent block
-    fn truncate_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error>;
+    /// and all other operations will treat this block as the chain tip for balance determination
+    /// purposes.
+    ///
+    /// There may be restrictions on heights to which it is possible to truncate. Specifically, it
+    /// will only be possible to truncate to heights at which is is possible to create a witness
+    /// given the current state of the wallet's note commitment tree.
+    fn truncate_to_height(&mut self, max_height: BlockHeight) -> Result<BlockHeight, Self::Error>;
 
     /// Reserves the next `n` available ephemeral addresses for the given account.
     /// This cannot be undone, so as far as possible, errors associated with transaction

zcash_client_backend/src/data_api/testing.rs

@@ -2633,8 +2633,11 @@ impl WalletWrite for MockWalletDb {
         Ok(())
     }
 
-    fn truncate_to_height(&mut self, _block_height: BlockHeight) -> Result<(), Self::Error> {
+    fn truncate_to_height(
-        Ok(())
+        &mut self,
+        _block_height: BlockHeight,
+    ) -> Result<BlockHeight, Self::Error> {
+        Err(())
     }
 
     /// Adds a transparent UTXO received by the wallet to the data store.

zcash_client_sqlite/CHANGELOG.md

@@ -7,12 +7,15 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 
+- `zcash_client_sqlite::error::SqliteClientError::RequestedRewindInvalid`
+  is now a structured variant.
+
 ## [0.11.2] - 2024-08-21
 
 ### Changed
 - The `v_tx_outputs` view was modified slightly to support older versions of
   `sqlite`. Queries to the exposed `v_tx_outputs` and `v_transactions` views
-  are supported for SQLite versions back to `3.19.x`. 
+  are supported for SQLite versions back to `3.19.x`.
 - `zcash_client_sqlite::wallet::init::WalletMigrationError` has an additional
   variant, `DatabaseNotSupported`. The `init_wallet_db` function now checks
   that the sqlite version in use is compatible with the features required by

zcash_client_sqlite/src/error.rs

@@ -12,7 +12,6 @@ use zcash_primitives::{consensus::BlockHeight, transaction::components::amount::
 
 use crate::wallet::commitment_tree;
 use crate::AccountId;
-use crate::PRUNING_DEPTH;
 
 #[cfg(feature = "transparent-inputs")]
 use {
@@ -64,8 +63,12 @@ pub enum SqliteClientError {
     NonSequentialBlocks,
 
     /// A requested rewind would violate invariants of the storage layer. The payload returned with
-    /// this error is (safe rewind height, requested height).
+    /// this error is (safe rewind height, requested height). If no safe rewind height can be
-    RequestedRewindInvalid(BlockHeight, BlockHeight),
+    /// determined, the safe rewind height member will be `None`.
+    RequestedRewindInvalid {
+        safe_rewind_height: Option<BlockHeight>,
+        requested_height: BlockHeight,
+    },
 
     /// An error occurred in generating a Zcash address.
     AddressGeneration(AddressGenerationError),
@@ -152,8 +155,12 @@ impl fmt::Display for SqliteClientError {
             }
             SqliteClientError::Protobuf(e) => write!(f, "Failed to parse protobuf-encoded record: {}", e),
             SqliteClientError::InvalidNote => write!(f, "Invalid note"),
-            SqliteClientError::RequestedRewindInvalid(h, r) =>
+            SqliteClientError::RequestedRewindInvalid { safe_rewind_height,  requested_height } => write!(
-                write!(f, "A rewind must be either of less than {} blocks, or at least back to block {} for your wallet; the requested height was {}.", PRUNING_DEPTH, h, r),
+                f,
+                "A rewind for your wallet may only target height {} or greater; the requested height was {}.",
+                safe_rewind_height.map_or("<unavailable>".to_owned(), |h0| format!("{}", h0)),
+               requested_height
+            ),
             SqliteClientError::DecodingError(e) => write!(f, "{}", e),
             #[cfg(feature = "transparent-inputs")]
             SqliteClientError::TransparentDerivation(e) => write!(f, "{:?}", e),

zcash_client_sqlite/src/lib.rs

@@ -1335,10 +1335,8 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
         })
     }
 
-    fn truncate_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error> {
+    fn truncate_to_height(&mut self, max_height: BlockHeight) -> Result<BlockHeight, Self::Error> {
-        self.transactionally(|wdb| {
+        self.transactionally(|wdb| wallet::truncate_to_height(wdb.conn.0, &wdb.params, max_height))
-            wallet::truncate_to_height(wdb.conn.0, &wdb.params, block_height)
-        })
     }
 
     #[cfg(feature = "transparent-inputs")]

zcash_client_sqlite/src/wallet.rs

@@ -2105,7 +2105,7 @@ pub(crate) fn get_max_height_hash(
 }
 
 /// Gets the height to which the database must be truncated if any truncation that would remove a
-/// number of blocks greater than the pruning height is attempted.
+/// number of blocks greater than the note commitment tree pruning depth is attempted.
 pub(crate) fn get_min_unspent_height(
     conn: &rusqlite::Connection,
 ) -> Result<Option<BlockHeight>, SqliteClientError> {
@@ -2357,34 +2357,86 @@ pub(crate) fn set_transaction_status(
     Ok(())
 }
 
-/// Truncates the database to the given height.
+/// Truncates the database to at most the given height.
 ///
 /// If the requested height is greater than or equal to the height of the last scanned
 /// block, this function does nothing.
 ///
 /// This should only be executed inside a transactional context.
+///
+/// Returns the block height to which the database was truncated.
 pub(crate) fn truncate_to_height<P: consensus::Parameters>(
     conn: &rusqlite::Transaction,
     params: &P,
-    block_height: BlockHeight,
+    max_height: BlockHeight,
-) -> Result<(), SqliteClientError> {
+) -> Result<BlockHeight, SqliteClientError> {
-    let sapling_activation_height = params
+    // Determine a checkpoint to which we can rewind, if any.
-        .activation_height(NetworkUpgrade::Sapling)
+    #[cfg(not(feature = "orchard"))]
-        .expect("Sapling activation height must be available.");
+    let truncation_height_query = r#"
+        SELECT MAX(height) FROM blocks
+        JOIN sapling_tree_checkpoints ON checkpoint_id = blocks.height
+        WHERE blocks.height <= :block_height
+    "#;
+
+    #[cfg(feature = "orchard")]
+    let truncation_height_query = r#"
+        SELECT MAX(height) FROM blocks
+        JOIN sapling_tree_checkpoints sc ON sc.checkpoint_id = blocks.height
+        JOIN orchard_tree_checkpoints oc ON oc.checkpoint_id = blocks.height
+        WHERE blocks.height <= :block_height
+    "#;
+
+    let truncation_height = conn
+        .query_row(
+            truncation_height_query,
+            named_params! {":block_height": u32::from(max_height)},
+            |row| row.get::<_, Option<u32>>(0),
+        )
+        .optional()?
+        .flatten()
+        .map_or_else(
+            || {
+                // If we don't have a checkpoint at a height less than or equal to the requested
+                // truncation height, query for the minimum height to which it's possible for us to
+                // truncate so that we can report it to the caller.
+                #[cfg(not(feature = "orchard"))]
+                let min_checkpoint_height_query =
+                    "SELECT MIN(checkpoint_id) FROM sapling_tree_checkpoints";
+                #[cfg(feature = "orchard")]
+                let min_checkpoint_height_query = "SELECT MIN(checkpoint_id) 
+                     FROM sapling_tree_checkpoints sc
+                     JOIN orchard_tree_checkpoints oc
+                     ON oc.checkpoint_id = sc.checkpoint_id";
+
+                let min_truncation_height = conn
+                    .query_row(min_checkpoint_height_query, [], |row| {
+                        row.get::<_, Option<u32>>(0)
+                    })
+                    .optional()?
+                    .flatten()
+                    .map(BlockHeight::from);
+
+                Err(SqliteClientError::RequestedRewindInvalid {
+                    safe_rewind_height: min_truncation_height,
+                    requested_height: max_height,
+                })
+            },
+            |h| Ok(BlockHeight::from(h)),
+        )?;
 
-    // Recall where we synced up to previously.
     let last_scanned_height = conn.query_row("SELECT MAX(height) FROM blocks", [], |row| {
-        row.get::<_, Option<u32>>(0)
+        let h = row.get::<_, Option<u32>>(0)?;
-            .map(|opt| opt.map_or_else(|| sapling_activation_height - 1, BlockHeight::from))
-    })?;
 
-    if block_height < last_scanned_height - PRUNING_DEPTH {
+        Ok(h.map_or_else(
-        if let Some(h) = get_min_unspent_height(conn)? {
+            || {
-            if block_height > h {
+                params
-                return Err(SqliteClientError::RequestedRewindInvalid(h, block_height));
+                    .activation_height(NetworkUpgrade::Sapling)
-            }
+                    .expect("Sapling activation height must be available.")
-        }
+                    - 1
-    }
+            },
+            BlockHeight::from,
+        ))
+    })?;
 
     // Delete from the scanning queue any range with a start height greater than the
     // truncation height, and then truncate any remaining range by setting the end
@@ -2393,13 +2445,13 @@ pub(crate) fn truncate_to_height<P: consensus::Parameters>(
     conn.execute(
         "DELETE FROM scan_queue
         WHERE block_range_start >= :new_end_height",
-        named_params![":new_end_height": u32::from(block_height + 1)],
+        named_params![":new_end_height": u32::from(truncation_height + 1)],
     )?;
     conn.execute(
         "UPDATE scan_queue
         SET block_range_end = :new_end_height
         WHERE block_range_end > :new_end_height",
-        named_params![":new_end_height": u32::from(block_height + 1)],
+        named_params![":new_end_height": u32::from(truncation_height + 1)],
     )?;
 
     // Mark transparent utxos as un-mined. Since the TXO is now not mined, it would ideally be
@@ -2413,7 +2465,7 @@ pub(crate) fn truncate_to_height<P: consensus::Parameters>(
          FROM transactions tx
          WHERE tx.id_tx = transaction_id
          AND max_observed_unspent_height > :height",
-        named_params![":height": u32::from(block_height)],
+        named_params![":height": u32::from(truncation_height)],
     )?;
 
     // Un-mine transactions. This must be done outside of the last_scanned_height check because
@@ -2422,32 +2474,32 @@ pub(crate) fn truncate_to_height<P: consensus::Parameters>(
         "UPDATE transactions
          SET block = NULL, mined_height = NULL, tx_index = NULL
          WHERE mined_height > :height",
-        named_params![":height": u32::from(block_height)],
+        named_params![":height": u32::from(truncation_height)],
     )?;
 
     // If we're removing scanned blocks, we need to truncate the note commitment tree and remove
     // affected block records from the database.
-    if block_height < last_scanned_height {
+    if truncation_height < last_scanned_height {
         // Truncate the note commitment trees
         let mut wdb = WalletDb {
             conn: SqlTransaction(conn),
             params: params.clone(),
         };
         wdb.with_sapling_tree_mut(|tree| {
-            tree.truncate_removing_checkpoint(&block_height)?;
+            tree.truncate_removing_checkpoint(&truncation_height)?;
             // We do want a checkpoint preserved at the end of the block, but it should have no
             // other data associated with it. TODO: `truncate_removing_checkpoint` is an awkward
             // API to work with, and should be replaced with `truncate_to_checkpoint`.
-            tree.checkpoint(block_height)?;
+            tree.checkpoint(truncation_height)?;
             Ok::<_, SqliteClientError>(())
         })?;
         #[cfg(feature = "orchard")]
         wdb.with_orchard_tree_mut(|tree| {
-            tree.truncate_removing_checkpoint(&block_height)?;
+            tree.truncate_removing_checkpoint(&truncation_height)?;
             // We do want a checkpoint preserved at the end of the block, but it should have no
             // other data associated with it. TODO: `truncate_removing_checkpoint` is an awkward
             // API to work with, and should be replaced with `truncate_to_checkpoint`.
-            tree.checkpoint(block_height)?;
+            tree.checkpoint(truncation_height)?;
             Ok::<_, SqliteClientError>(())
         })?;
 
@@ -2461,7 +2513,7 @@ pub(crate) fn truncate_to_height<P: consensus::Parameters>(
         // Now that they aren't depended on, delete un-mined blocks.
         conn.execute(
             "DELETE FROM blocks WHERE height > ?",
-            [u32::from(block_height)],
+            [u32::from(truncation_height)],
         )?;
 
         // Delete from the nullifier map any entries with a locator referencing a block
@@ -2469,11 +2521,11 @@ pub(crate) fn truncate_to_height<P: consensus::Parameters>(
         conn.execute(
             "DELETE FROM tx_locator_map
             WHERE block_height > :block_height",
-            named_params![":block_height": u32::from(block_height)],
+            named_params![":block_height": u32::from(truncation_height)],
         )?;
     }
 
-    Ok(())
+    Ok(truncation_height)
 }
 
 /// Returns a vector with the IDs of all accounts known to this wallet.

zcash_client_sqlite/src/wallet/init.rs

@@ -197,7 +197,7 @@ fn sqlite_client_error_to_wallet_migration_error(e: SqliteClientError) -> Wallet
         SqliteClientError::TableNotEmpty => unreachable!("wallet already initialized"),
         SqliteClientError::BlockConflict(_)
         | SqliteClientError::NonSequentialBlocks
-        | SqliteClientError::RequestedRewindInvalid(_, _)
+        | SqliteClientError::RequestedRewindInvalid { .. }
         | SqliteClientError::KeyDerivationError(_)
         | SqliteClientError::AccountIdDiscontinuity
         | SqliteClientError::AccountIdOutOfRange