zcash_client_backend: Replace `WalletWrite::advance_by_block` with `WalletWrite::put_block`

Also, add assertions to prevent attempting the creation of zero-conf
shielded spends.

zcash_client_backend/CHANGELOG.md

@@ -7,9 +7,11 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 ### Added
-- `impl Eq for zcash_client_backend::address::RecipientAddress`
+- `impl Eq for address::RecipientAddress`
-- `impl Eq for zcash_client_backend::zip321::{Payment, TransactionRequest}`
+- `impl Eq for zip321::{Payment, TransactionRequest}`
 - `data_api::NullifierQuery` for use with `WalletRead::get_sapling_nullifiers`
+- `WalletWrite::put_block`
+- `impl Debug` for `{data_api::wallet::input_selection::Proposal, wallet::ReceivedSaplingNote}
 
 ### Changed
 - MSRV is now 1.65.0.
@@ -21,9 +23,14 @@ and this library adheres to Rust's notion of
 - `WalletRead::get_nullifiers` has been renamed to `WalletRead::get_sapling_nullifiers`
   and its signature has changed; it now subsumes the removed `WalletRead::get_all_nullifiers`.
 - `wallet::SpendableNote` has been renamed to `wallet::ReceivedSaplingNote`.
+- `data_api::chain::scan_cached_blocks` now takes a `from_height` argument that
+  permits the caller to control the starting position of the scan range.
+- `WalletWrite::advance_by_block` has been replaced by `WalletWrite::put_block`
+  to reflect the semantic change that scanning is no longer a linear operation.
 
 ### Removed
 - `WalletRead::get_all_nullifiers`
+- `WalletWrite::advance_by_block` 
 
 ## [0.9.0] - 2023-04-28
 ### Added

zcash_client_backend/src/data_api.rs

@@ -401,7 +401,7 @@ pub trait WalletWrite: WalletRead {
     /// along with the note commitments that were detected when scanning the block for transactions
     /// pertaining to this wallet.
     #[allow(clippy::type_complexity)]
-    fn advance_by_block(
+    fn put_block(
         &mut self,
         block: PrunedBlock<sapling::Nullifier>,
     ) -> Result<Vec<Self::NoteRef>, Self::Error>;
@@ -660,7 +660,7 @@ pub mod testing {
         }
 
         #[allow(clippy::type_complexity)]
-        fn advance_by_block(
+        fn put_block(
             &mut self,
             _block: PrunedBlock<sapling::Nullifier>,
         ) -> Result<Vec<Self::NoteRef>, Self::Error> {

zcash_client_backend/src/data_api/chain.rs

@@ -175,7 +175,7 @@ where
     // comparing against the `validate_from` hash.
 
     block_source.with_blocks::<_, Infallible, Infallible>(
-        validate_from.map(|(h, _)| h),
+        validate_from.map(|(h, _)| h + 1),
         limit,
         move |block| {
             if let Some((valid_height, valid_hash)) = validate_from {
@@ -260,18 +260,20 @@ where
     );
 
     // Start at either the provided height, or where we synced up to previously.
-    let (last_scanned_height, commitment_tree_meta) = from_height.map_or_else(
+    let (from_height, commitment_tree_meta) = from_height.map_or_else(
         || {
             data_db.fully_scanned_height().map_or_else(
                 |e| Err(Error::Wallet(e)),
-                |next| Ok(next.map_or_else(|| (None, None), |(h, m)| (Some(h), Some(m)))),
+                |last_scanned| {
+                    Ok(last_scanned.map_or_else(|| (None, None), |(h, m)| (Some(h + 1), Some(m))))
+                },
             )
         },
         |h| Ok((Some(h), None)),
     )?;
 
     block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>(
-        last_scanned_height,
+        from_height,
         limit,
         |block: CompactBlock| {
             add_block_to_runner(params, block, &mut batch_runner);
@@ -282,7 +284,7 @@ where
     batch_runner.flush();
 
     block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>(
-        last_scanned_height,
+        from_height,
         limit,
         |block: CompactBlock| {
             let pruned_block = scan_block_with_runner(
@@ -308,9 +310,7 @@ where
                     .map(|out| (out.account(), *out.nf()))
             }));
 
-            data_db
+            data_db.put_block(pruned_block).map_err(Error::Wallet)?;
-                .advance_by_block(pruned_block)
-                .map_err(Error::Wallet)?;
 
             Ok(())
         },

zcash_client_backend/src/data_api/wallet.rs

@@ -119,7 +119,8 @@ where
 ///   can allow the sender to view the resulting notes on the blockchain.
 /// * `min_confirmations`: The minimum number of confirmations that a previously
 ///   received note must have in the blockchain in order to be considered for being
-///   spent. A value of 10 confirmations is recommended.
+///   spent. A value of 10 confirmations is recommended and 0-conf transactions are
+///   not supported.
 ///
 /// # Examples
 ///
@@ -318,6 +319,10 @@ where
     ParamsT: consensus::Parameters + Clone,
     InputsT: InputSelector<DataSource = DbT>,
 {
+    assert!(
+        min_confirmations > 0,
+        "zero-conf transactions are not supported"
+    );
     let account = wallet_db
         .get_account_for_ufvk(&usk.to_unified_full_viewing_key())
         .map_err(Error::DataSource)?
@@ -372,6 +377,10 @@ where
     ParamsT: consensus::Parameters + Clone,
     InputsT: InputSelector<DataSource = DbT>,
 {
+    assert!(
+        min_confirmations > 0,
+        "zero-conf transactions are not supported"
+    );
     input_selector
         .propose_transaction(
             params,
@@ -409,6 +418,10 @@ where
     DbT::NoteRef: Copy + Eq + Ord,
     InputsT: InputSelector<DataSource = DbT>,
 {
+    assert!(
+        min_confirmations > 0,
+        "zero-conf transactions are not supported"
+    );
     input_selector
         .propose_shielding(
             params,
@@ -453,6 +466,10 @@ where
     ParamsT: consensus::Parameters + Clone,
     FeeRuleT: FeeRule,
 {
+    assert!(
+        min_confirmations > 0,
+        "zero-conf transactions are not supported"
+    );
     let account = wallet_db
         .get_account_for_ufvk(&usk.to_unified_full_viewing_key())
         .map_err(Error::DataSource)?
@@ -495,8 +512,7 @@ where
                 selected,
                 usk.sapling(),
                 &dfvk,
-                min_confirmations
+                usize::try_from(min_confirmations - 1)
-                    .try_into()
                     .expect("min_confirmations should never be anywhere close to usize::MAX"),
             )?
             .ok_or(Error::NoteMismatch(selected.note_id))?;

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -1,8 +1,8 @@
 //! Types related to the process of selecting inputs to be spent given a transaction request.
 
 use core::marker::PhantomData;
-use std::collections::BTreeSet;
 use std::fmt;
+use std::{collections::BTreeSet, fmt::Debug};
 
 use zcash_primitives::{
     consensus::{self, BlockHeight},
@@ -124,6 +124,21 @@ impl<FeeRuleT, NoteRef> Proposal<FeeRuleT, NoteRef> {
     }
 }
 
+impl<FeeRuleT, NoteRef> Debug for Proposal<FeeRuleT, NoteRef> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("Proposal")
+            .field("transaction_request", &self.transaction_request)
+            .field("transparent_inputs", &self.transparent_inputs)
+            .field("sapling_inputs", &self.sapling_inputs.len())
+            .field("balance", &self.balance)
+            //.field("fee_rule", &self.fee_rule)
+            .field("min_target_height", &self.min_target_height)
+            .field("min_anchor_height", &self.min_anchor_height)
+            .field("is_shielding", &self.is_shielding)
+            .finish()
+    }
+}
+
 /// A strategy for selecting transaction inputs and proposing transaction outputs.
 ///
 /// Proposals should include only economically useful inputs, as determined by `Self::FeeRule`;

zcash_client_backend/src/wallet.rs

@@ -175,6 +175,7 @@ impl<N> WalletSaplingOutput<N> {
 
 /// Information about a note that is tracked by the wallet that is available for spending,
 /// with sufficient information for use in note selection.
+#[derive(Debug)]
 pub struct ReceivedSaplingNote<NoteRef> {
     pub note_id: NoteRef,
     pub diversifier: sapling::Diversifier,

zcash_client_sqlite/src/lib.rs

@@ -392,7 +392,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
 
     #[tracing::instrument(skip_all, fields(height = u32::from(block.block_height)))]
     #[allow(clippy::type_complexity)]
-    fn advance_by_block(
+    fn put_block(
         &mut self,
         block: PrunedBlock<sapling::Nullifier>,
     ) -> Result<Vec<Self::NoteRef>, Self::Error> {