Merge pull request #1003 from nuttycom/wallet/reusable_input_selection

zcash_client_backend: Factor out `InputSource` from `WalletRead`

.github/workflows/ci.yml

@@ -132,7 +132,7 @@ jobs:
       - run: cargo fetch
       # Requires #![deny(rustdoc::broken_intra_doc_links)] in crates.
       - name: Check intra-doc links
-        run: cargo doc --all --document-private-items
+        run: cargo doc --all --all-features --document-private-items
 
   fmt:
     name: Rustfmt

Cargo.lock

@@ -2183,9 +2183,9 @@ dependencies = [
 
 [[package]]
 name = "shardtree"
-version = "0.1.0"
+version = "0.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c19f96dde3a8693874f7e7c53d95616569b4009379a903789efbd448f4ea9cc7"
+checksum = "dbf20c7a2747d9083092e3a3eeb9a7ed75577ae364896bebbc5e0bdcd4e97735"
 dependencies = [
  "assert_matches",
  "bitflags 2.4.0",
@@ -2963,6 +2963,7 @@ dependencies = [
  "jubjub",
  "memuse",
  "nom",
+ "nonempty",
  "orchard",
  "percent-encoding",
  "proptest",

Cargo.toml

@@ -39,7 +39,7 @@ zcash_proofs = { version = "0.13", path = "zcash_proofs", default-features = fal
 ff = "0.13"
 group = "0.13"
 incrementalmerkletree = "0.5"
-shardtree = "0.1"
+shardtree = "0.2"
 
 # Payment protocols
 # - Sapling

zcash_client_backend/CHANGELOG.md

@@ -8,10 +8,16 @@ and this library adheres to Rust's notion of
 ## [Unreleased]
 
 ### Added
-- `zcash_client_backend::data_api::wallet::propose_standard_transfer_to_address`
+- `zcash_client_backend::data_api`:
+  - `TransparentInputSource`
+  - `SaplingInputSource`
+  - `wallet::propose_standard_transfer_to_address`
+  - `wallet::input_selection::SaplingInputs`
+  - `wallet::input_selection::ShieldingSelector` has been
+    factored out from the `InputSelector` trait to separate out transparent
+    functionality and move it behind the `transparent-inputs` feature flag.
 - `zcash_client_backend::fees::standard`
 - `zcash_client_backend::wallet`:
-  - `input_selection::Proposal::min_confirmations`
   - `ReceivedSaplingNote::from_parts`
   - `ReceivedSaplingNote::{txid, output_index, diversifier, rseed, note_commitment_tree_position}`
 
@@ -39,7 +45,7 @@ and this library adheres to Rust's notion of
   - `wallet::create_proposed_transaction` now takes its `proposal` argument
     by reference instead of as an owned value.
   - `wallet::create_proposed_transaction` no longer takes a `min_confirmations`
-    argument. Instead, `min_confirmations` is stored in the `Proposal`
+    argument. Instead, it uses the anchor height from its `proposal` argument.
   - `wallet::create_spend_to_address` now takes an additional
     `change_memo` argument.
   - The error type of `wallet::create_spend_to_address` has been changed to use
@@ -51,6 +57,39 @@ and this library adheres to Rust's notion of
     - `wallet::create_spend_to_address`
     - `wallet::shield_transparent_funds`
     - `wallet::spend`
+  - In order to support better reusability for input selection code, two new
+    traits have been factored out from `WalletRead`:
+    - `zcash_client_backend::data_api::TransparentInputSource`
+    - `zcash_client_backend::data_api::SaplingInputSource`
+  - `wallet::input_selection::InputSelector::propose_shielding`,
+    has been moved out to the newly-created `ShieldingSelector` trait.
+    - `ShieldingSelector::propose_shielding` has been altered such that it takes
+      an explicit `target_height` in order to minimize the capabilities that the
+      `data_api::TransparentInputSource` trait must expose. Also, it now takes its
+      `min_confirmations` argument as `u32` instead of `NonZeroU32`.
+  - The `wallet::input_selection::InputSelector::DataSource`
+    associated type has been renamed to `InputSource`.
+  - The signature of `wallet:input_selection::InputSelector::propose_transaction` 
+    has been altered such that it longer takes `min_confirmations` as an
+    argument, instead taking explicit `target_height` and `anchor_height`
+    arguments. This helps to minimize the set of capabilities that the
+    `data_api::SaplingInputSource` must expose.
+  - `WalletRead::get_checkpoint_depth` has been removed without replacement. This
+    is no longer needed given the change to use the stored anchor height for transaction
+    proposal execution.
+  - `wallet::{propose_shielding, shield_transparent_funds}` now takes their
+    `min_confirmations` arguments as `u32` rather than a `NonZeroU32` to permit
+    implmentations to enable zero-conf shielding.
+  - `wallet::create_proposed_transaction` now forces implementations to ignore
+    the database identifiers for its contained notes by universally quantifying
+    the `NoteRef` type parameter.
+  - `wallet::input_selection::Proposal::sapling_inputs` now returns type
+    `Option<&SaplingInputs>`.
+  - `wallet::input_selection::Proposal::min_anchor_height` has been removed in
+    favor of storing this value in `SaplingInputs`.
+  - `wallet::input_selection::GreedyInputSelector` now has relaxed requirements
+    for its `InputSource` associated type.
+
 - `zcash_client_backend::fees`:
   - `ChangeValue::Sapling` is now a structured variant. In addition to the
     existing change value, it now also carries an optional memo to be associated
@@ -92,8 +131,12 @@ and this library adheres to Rust's notion of
 
 ### Removed
 - `zcash_client_backend::data_api::WalletRead::is_valid_account_extfvk` has been
-  removed; it was unused in the ECC mobile wallet SDKs and has been superseded by 
+  removed; it was unused in the ECC mobile wallet SDKs and has been superseded by
   `get_account_for_ufvk`.
+- `zcash_client_backend::data_api::WalletRead::get_spendable_sapling_notes` has been
+  removed without replacement as it was unused, and its functionality will be
+  fully reproduced by `SaplingInputSource::select_spendable_sapling_notes` in a future
+  change.
 
 ## [0.10.0] - 2023-09-25
 

zcash_client_backend/Cargo.toml

@@ -30,6 +30,7 @@ zcash_primitives.workspace = true
 # (Breaking upgrades to these require a breaking upgrade to this crate.)
 # - Data Access API
 time = "0.3.22"
+nonempty.workspace = true
 
 # - Encodings
 base64.workspace = true
@@ -87,6 +88,7 @@ gumdrop = "0.8"
 jubjub.workspace = true
 proptest.workspace = true
 rand_core.workspace = true
+shardtree = { workspace = true, features = ["test-dependencies"] }
 zcash_proofs.workspace = true
 zcash_address = { workspace = true, features = ["test-dependencies"] }
 

zcash_client_backend/src/data_api.rs

@@ -17,10 +17,7 @@ use zcash_primitives::{
     memo::{Memo, MemoBytes},
     sapling::{self, Node, NOTE_COMMITMENT_TREE_DEPTH},
     transaction::{
-        components::{
+        components::amount::{Amount, NonNegativeAmount},
-            amount::{Amount, NonNegativeAmount},
-            OutPoint,
-        },
         Transaction, TxId,
     },
     zip32::AccountId,
@@ -34,6 +31,9 @@ use crate::{
     wallet::{ReceivedSaplingNote, WalletTransparentOutput, WalletTx},
 };
 
+#[cfg(feature = "transparent-inputs")]
+use zcash_primitives::transaction::components::OutPoint;
+
 use self::chain::CommitmentTreeRoot;
 use self::scanning::ScanRange;
 
@@ -210,12 +210,9 @@ impl WalletSummary {
     }
 }
 
-/// Read-only operations required for light wallet functions.
+/// A trait representing the capability to query a data store for unspent Sapling notes belonging
-///
+/// to a wallet.
-/// This trait defines the read-only portion of the storage interface atop which
+pub trait SaplingInputSource {
-/// higher-level wallet operations are implemented. It serves to allow wallet functions to
-/// be abstracted away from any particular data storage substrate.
-pub trait WalletRead {
     /// The type of errors produced by a wallet backend.
     type Error;
 
@@ -225,6 +222,43 @@ pub trait WalletRead {
     /// or a UUID.
     type NoteRef: Copy + Debug + Eq + Ord;
 
+    /// Returns a list of spendable Sapling notes sufficient to cover the specified target value,
+    /// if possible.
+    fn select_spendable_sapling_notes(
+        &self,
+        account: AccountId,
+        target_value: Amount,
+        anchor_height: BlockHeight,
+        exclude: &[Self::NoteRef],
+    ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error>;
+}
+
+/// A trait representing the capability to query a data store for unspent transparent UTXOs
+/// belonging to a wallet.
+#[cfg(feature = "transparent-inputs")]
+pub trait TransparentInputSource {
+    /// The type of errors produced by a wallet backend.
+    type Error;
+
+    /// Returns a list of unspent transparent UTXOs that appear in the chain at heights up to and
+    /// including `max_height`.
+    fn get_unspent_transparent_outputs(
+        &self,
+        address: &TransparentAddress,
+        max_height: BlockHeight,
+        exclude: &[OutPoint],
+    ) -> Result<Vec<WalletTransparentOutput>, Self::Error>;
+}
+
+/// Read-only operations required for light wallet functions.
+///
+/// This trait defines the read-only portion of the storage interface atop which
+/// higher-level wallet operations are implemented. It serves to allow wallet functions to
+/// be abstracted away from any particular data storage substrate.
+pub trait WalletRead {
+    /// The type of errors that may be generated when querying a wallet data store.
+    type Error;
+
     /// Returns the height of the chain as known to the wallet as of the most recent call to
     /// [`WalletWrite::update_chain_tip`].
     ///
@@ -351,24 +385,6 @@ pub trait WalletRead {
         query: NullifierQuery,
     ) -> Result<Vec<(AccountId, sapling::Nullifier)>, Self::Error>;
 
-    /// Return all unspent Sapling notes, excluding the specified note IDs.
-    fn get_spendable_sapling_notes(
-        &self,
-        account: AccountId,
-        anchor_height: BlockHeight,
-        exclude: &[Self::NoteRef],
-    ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error>;
-
-    /// Returns a list of spendable Sapling notes sufficient to cover the specified target value,
-    /// if possible.
-    fn select_spendable_sapling_notes(
-        &self,
-        account: AccountId,
-        target_value: Amount,
-        anchor_height: BlockHeight,
-        exclude: &[Self::NoteRef],
-    ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error>;
-
     /// Returns the set of all transparent receivers associated with the given account.
     ///
     /// The set contains all transparent receivers that are known to have been derived
@@ -379,15 +395,6 @@ pub trait WalletRead {
         account: AccountId,
     ) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error>;
 
-    /// Returns a list of unspent transparent UTXOs that appear in the chain at heights up to and
-    /// including `max_height`.
-    fn get_unspent_transparent_outputs(
-        &self,
-        address: &TransparentAddress,
-        max_height: BlockHeight,
-        exclude: &[OutPoint],
-    ) -> Result<Vec<WalletTransparentOutput>, Self::Error>;
-
     /// Returns a mapping from transparent receiver to not-yet-shielded UTXO balance,
     /// for each address associated with a nonzero balance.
     fn get_transparent_balances(
@@ -897,16 +904,7 @@ 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(
@@ -939,10 +937,7 @@ pub mod testing {
         legacy::TransparentAddress,
         memo::Memo,
         sapling,
-        transaction::{
+        transaction::{components::Amount, Transaction, TxId},
-            components::{Amount, OutPoint},
-            Transaction, TxId,
-        },
         zip32::AccountId,
     };
 
@@ -954,8 +949,9 @@ pub mod testing {
 
     use super::{
         chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, BlockMetadata,
-        DecryptedTransaction, NoteId, NullifierQuery, ScannedBlock, SentTransaction,
+        DecryptedTransaction, NoteId, NullifierQuery, SaplingInputSource, ScannedBlock,
-        WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT,
+        SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite,
+        SAPLING_SHARD_HEIGHT,
     };
 
     pub struct MockWalletDb {
@@ -967,6 +963,9 @@ pub mod testing {
         >,
     }
 
+    #[cfg(feature = "transparent-inputs")]
+    use {super::TransparentInputSource, zcash_primitives::transaction::components::OutPoint};
+
     impl MockWalletDb {
         pub fn new(network: Network) -> Self {
             Self {
@@ -976,10 +975,38 @@ pub mod testing {
         }
     }
 
-    impl WalletRead for MockWalletDb {
+    impl SaplingInputSource for MockWalletDb {
         type Error = ();
         type NoteRef = u32;
 
+        fn select_spendable_sapling_notes(
+            &self,
+            _account: AccountId,
+            _target_value: Amount,
+            _anchor_height: BlockHeight,
+            _exclude: &[Self::NoteRef],
+        ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error> {
+            Ok(Vec::new())
+        }
+    }
+
+    #[cfg(feature = "transparent-inputs")]
+    impl TransparentInputSource for MockWalletDb {
+        type Error = ();
+
+        fn get_unspent_transparent_outputs(
+            &self,
+            _address: &TransparentAddress,
+            _anchor_height: BlockHeight,
+            _exclude: &[OutPoint],
+        ) -> Result<Vec<WalletTransparentOutput>, Self::Error> {
+            Ok(Vec::new())
+        }
+    }
+
+    impl WalletRead for MockWalletDb {
+        type Error = ();
+
         fn chain_height(&self) -> Result<Option<BlockHeight>, Self::Error> {
             Ok(None)
         }
@@ -1079,25 +1106,6 @@ pub mod testing {
             Ok(Vec::new())
         }
 
-        fn get_spendable_sapling_notes(
-            &self,
-            _account: AccountId,
-            _anchor_height: BlockHeight,
-            _exclude: &[Self::NoteRef],
-        ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error> {
-            Ok(Vec::new())
-        }
-
-        fn select_spendable_sapling_notes(
-            &self,
-            _account: AccountId,
-            _target_value: Amount,
-            _anchor_height: BlockHeight,
-            _exclude: &[Self::NoteRef],
-        ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error> {
-            Ok(Vec::new())
-        }
-
         fn get_transparent_receivers(
             &self,
             _account: AccountId,
@@ -1105,15 +1113,6 @@ pub mod testing {
             Ok(HashMap::new())
         }
 
-        fn get_unspent_transparent_outputs(
-            &self,
-            _address: &TransparentAddress,
-            _anchor_height: BlockHeight,
-            _exclude: &[OutPoint],
-        ) -> Result<Vec<WalletTransparentOutput>, Self::Error> {
-            Ok(Vec::new())
-        }
-
         fn get_transparent_balances(
             &self,
             _account: AccountId,
@@ -1214,12 +1213,5 @@ 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)).unwrap())
-        }
     }
 }

zcash_client_backend/src/data_api/wallet.rs

@@ -35,13 +35,18 @@ use crate::{
 };
 
 pub mod input_selection;
-use input_selection::{GreedyInputSelector, GreedyInputSelectorError, InputSelector};
+use input_selection::{
+    GreedyInputSelector, GreedyInputSelectorError, InputSelector, InputSelectorError,
+};
 
-use super::{NoteId, ShieldedProtocol};
+use super::{NoteId, SaplingInputSource, ShieldedProtocol};
 
 #[cfg(feature = "transparent-inputs")]
 use {
+    super::TransparentInputSource,
     crate::wallet::WalletTransparentOutput,
+    input_selection::ShieldingSelector,
+    std::convert::Infallible,
     zcash_primitives::{legacy::TransparentAddress, sapling::keys::OutgoingViewingKey},
 };
 
@@ -216,7 +221,9 @@ pub fn create_spend_to_address<DbT, ParamsT>(
 >
 where
     ParamsT: consensus::Parameters + Clone,
-    DbT: WalletWrite + WalletCommitmentTrees,
+    DbT: WalletWrite
+        + WalletCommitmentTrees
+        + SaplingInputSource<Error = <DbT as WalletRead>::Error>,
     DbT::NoteRef: Copy + Eq + Ord,
 {
     let account = wallet_db
@@ -324,10 +331,12 @@ pub fn spend<DbT, ParamsT, InputsT>(
     >,
 >
 where
-    DbT: WalletWrite + WalletCommitmentTrees,
+    DbT: WalletWrite
+        + WalletCommitmentTrees
+        + SaplingInputSource<Error = <DbT as WalletRead>::Error>,
     DbT::NoteRef: Copy + Eq + Ord,
     ParamsT: consensus::Parameters + Clone,
-    InputsT: InputSelector<DataSource = DbT>,
+    InputsT: InputSelector<InputSource = DbT>,
 {
     let account = wallet_db
         .get_account_for_ufvk(&usk.to_unified_full_viewing_key())
@@ -368,21 +377,32 @@ pub fn propose_transfer<DbT, ParamsT, InputsT, CommitmentTreeErrT>(
     min_confirmations: NonZeroU32,
 ) -> Result<
     Proposal<InputsT::FeeRule, DbT::NoteRef>,
-    Error<DbT::Error, CommitmentTreeErrT, InputsT::Error, <InputsT::FeeRule as FeeRule>::Error>,
+    Error<
+        <DbT as WalletRead>::Error,
+        CommitmentTreeErrT,
+        InputsT::Error,
+        <InputsT::FeeRule as FeeRule>::Error,
+    >,
 >
 where
-    DbT: WalletWrite,
+    DbT: WalletRead + SaplingInputSource<Error = <DbT as WalletRead>::Error>,
     DbT::NoteRef: Copy + Eq + Ord,
     ParamsT: consensus::Parameters + Clone,
-    InputsT: InputSelector<DataSource = DbT>,
+    InputsT: InputSelector<InputSource = DbT>,
 {
+    let (target_height, anchor_height) = wallet_db
+        .get_target_and_anchor_heights(min_confirmations)
+        .map_err(|e| Error::from(InputSelectorError::DataSource(e)))?
+        .ok_or_else(|| Error::from(InputSelectorError::SyncRequired))?;
+
     input_selector
         .propose_transaction(
             params,
             wallet_db,
+            target_height,
+            anchor_height,
             spend_from_account,
             request,
-            min_confirmations,
         )
         .map_err(Error::from)
 }
@@ -421,7 +441,7 @@ pub fn propose_standard_transfer_to_address<DbT, ParamsT, CommitmentTreeErrT>(
 ) -> Result<
     Proposal<StandardFeeRule, DbT::NoteRef>,
     Error<
-        DbT::Error,
+        <DbT as WalletRead>::Error,
         CommitmentTreeErrT,
         GreedyInputSelectorError<Zip317FeeError, DbT::NoteRef>,
         Zip317FeeError,
@@ -429,7 +449,7 @@ pub fn propose_standard_transfer_to_address<DbT, ParamsT, CommitmentTreeErrT>(
 >
 where
     ParamsT: consensus::Parameters + Clone,
-    DbT: WalletWrite,
+    DbT: WalletRead + SaplingInputSource<Error = <DbT as WalletRead>::Error>,
     DbT::NoteRef: Copy + Eq + Ord,
 {
     let request = zip321::TransactionRequest::new(vec![Payment {
@@ -467,23 +487,33 @@ pub fn propose_shielding<DbT, ParamsT, InputsT, CommitmentTreeErrT>(
     input_selector: &InputsT,
     shielding_threshold: NonNegativeAmount,
     from_addrs: &[TransparentAddress],
-    min_confirmations: NonZeroU32,
+    min_confirmations: u32,
 ) -> Result<
-    Proposal<InputsT::FeeRule, DbT::NoteRef>,
+    Proposal<InputsT::FeeRule, Infallible>,
-    Error<DbT::Error, CommitmentTreeErrT, InputsT::Error, <InputsT::FeeRule as FeeRule>::Error>,
+    Error<
+        <DbT as WalletRead>::Error,
+        CommitmentTreeErrT,
+        InputsT::Error,
+        <InputsT::FeeRule as FeeRule>::Error,
+    >,
 >
 where
     ParamsT: consensus::Parameters,
-    DbT: WalletWrite,
+    DbT: WalletRead + TransparentInputSource<Error = <DbT as WalletRead>::Error>,
-    DbT::NoteRef: Copy + Eq + Ord,
+    InputsT: ShieldingSelector<InputSource = DbT>,
-    InputsT: InputSelector<DataSource = DbT>,
 {
+    let chain_tip_height = wallet_db
+        .chain_height()
+        .map_err(|e| Error::from(InputSelectorError::DataSource(e)))?
+        .ok_or_else(|| Error::from(InputSelectorError::SyncRequired))?;
+
     input_selector
         .propose_shielding(
             params,
             wallet_db,
             shielding_threshold,
             from_addrs,
+            chain_tip_height + 1,
             min_confirmations,
         )
         .map_err(Error::from)
@@ -499,14 +529,14 @@ where
 /// to fall back to the transparent receiver until full Orchard support is implemented.
 #[allow(clippy::too_many_arguments)]
 #[allow(clippy::type_complexity)]
-pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT>(
+pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT, N>(
     wallet_db: &mut DbT,
     params: &ParamsT,
     spend_prover: &impl SpendProver,
     output_prover: &impl OutputProver,
     usk: &UnifiedSpendingKey,
     ovk_policy: OvkPolicy,
-    proposal: &Proposal<FeeRuleT, DbT::NoteRef>,
+    proposal: &Proposal<FeeRuleT, N>,
 ) -> Result<
     TxId,
     Error<
@@ -518,7 +548,6 @@ pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT>(
 >
 where
     DbT: WalletWrite + WalletCommitmentTrees,
-    DbT::NoteRef: Copy + Eq + Ord,
     ParamsT: consensus::Parameters + Clone,
     FeeRuleT: FeeRule,
 {
@@ -557,28 +586,29 @@ 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(proposal.min_confirmations())?;
+    if let Some(sapling_inputs) = proposal.sapling_inputs() {
-    wallet_db.with_sapling_tree_mut::<_, _, Error<_, _, _, _>>(|sapling_tree| {
+        wallet_db.with_sapling_tree_mut::<_, _, Error<_, _, _, _>>(|sapling_tree| {
-        for selected in proposal.sapling_inputs() {
+            for selected in sapling_inputs.notes() {
-            let (note, key, merkle_path) = select_key_for_note(
+                let (note, key, merkle_path) = select_key_for_note(
-                sapling_tree,
+                    sapling_tree,
-                selected,
+                    selected,
-                usk.sapling(),
+                    usk.sapling(),
-                &dfvk,
+                    &dfvk,
-                checkpoint_depth,
+                    sapling_inputs.anchor_height(),
-            )?
+                )?
-            .ok_or_else(|| {
+                .ok_or_else(|| {
-                Error::NoteMismatch(NoteId {
+                    Error::NoteMismatch(NoteId {
-                    txid: *selected.txid(),
+                        txid: *selected.txid(),
-                    protocol: ShieldedProtocol::Sapling,
+                        protocol: ShieldedProtocol::Sapling,
-                    output_index: selected.output_index(),
+                        output_index: selected.output_index(),
-                })
+                    })
-            })?;
+                })?;
 
-            builder.add_sapling_spend(key, selected.diversifier(), note, merkle_path)?;
+                builder.add_sapling_spend(key, selected.diversifier(), note, merkle_path)?;
-        }
+            }
-        Ok(())
+            Ok(())
-    })?;
+        })?;
+    }
 
     #[cfg(feature = "transparent-inputs")]
     let utxos = {
@@ -807,7 +837,7 @@ pub fn shield_transparent_funds<DbT, ParamsT, InputsT>(
     shielding_threshold: NonNegativeAmount,
     usk: &UnifiedSpendingKey,
     from_addrs: &[TransparentAddress],
-    min_confirmations: NonZeroU32,
+    min_confirmations: u32,
 ) -> Result<
     TxId,
     Error<
@@ -819,9 +849,10 @@ pub fn shield_transparent_funds<DbT, ParamsT, InputsT>(
 >
 where
     ParamsT: consensus::Parameters,
-    DbT: WalletWrite + WalletCommitmentTrees,
+    DbT: WalletWrite
-    DbT::NoteRef: Copy + Eq + Ord,
+        + WalletCommitmentTrees
-    InputsT: InputSelector<DataSource = DbT>,
+        + TransparentInputSource<Error = <DbT as WalletRead>::Error>,
+    InputsT: ShieldingSelector<InputSource = DbT>,
 {
     let proposal = propose_shielding(
         wallet_db,
@@ -853,7 +884,7 @@ fn select_key_for_note<N, S: ShardStore<H = Node, CheckpointId = BlockHeight>>(
     selected: &ReceivedSaplingNote<N>,
     extsk: &ExtendedSpendingKey,
     dfvk: &DiversifiableFullViewingKey,
-    checkpoint_depth: usize,
+    anchor_height: BlockHeight,
 ) -> Result<
     Option<(sapling::Note, ExtendedSpendingKey, sapling::MerklePath)>,
     ShardTreeError<S::Error>,
@@ -868,9 +899,11 @@ fn select_key_for_note<N, S: ShardStore<H = Node, CheckpointId = BlockHeight>>(
         .diversified_change_address(selected.diversifier())
         .map(|addr| addr.create_note(selected.value().try_into().unwrap(), selected.rseed()));
 
-    let expected_root = commitment_tree.root_at_checkpoint(checkpoint_depth)?;
+    let expected_root = commitment_tree.root_at_checkpoint_id(&anchor_height)?;
-    let merkle_path = commitment_tree
+    let merkle_path = commitment_tree.witness_at_checkpoint_id_caching(
-        .witness_caching(selected.note_commitment_tree_position(), checkpoint_depth)?;
+        selected.note_commitment_tree_position(),
+        &anchor_height,
+    )?;
 
     Ok(external_note
         .filter(|n| expected_root == merkle_path.root(Node::from_cmu(&n.cmu())))

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -1,10 +1,9 @@
 //! Types related to the process of selecting inputs to be spent given a transaction request.
 
 use core::marker::PhantomData;
-use std::fmt;
+use std::fmt::{self, Debug};
-use std::num::NonZeroU32;
-use std::{collections::BTreeSet, fmt::Debug};
 
+use nonempty::NonEmpty;
 use zcash_primitives::{
     consensus::{self, BlockHeight},
     legacy::TransparentAddress,
@@ -12,7 +11,7 @@ use zcash_primitives::{
         components::{
             amount::{BalanceError, NonNegativeAmount},
             sapling::fees as sapling,
-            OutPoint, TxOut,
+            TxOut,
         },
         fees::FeeRule,
     },
@@ -21,12 +20,18 @@ use zcash_primitives::{
 
 use crate::{
     address::{RecipientAddress, UnifiedAddress},
-    data_api::WalletRead,
+    data_api::SaplingInputSource,
     fees::{ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance},
     wallet::{ReceivedSaplingNote, WalletTransparentOutput},
     zip321::TransactionRequest,
 };
 
+#[cfg(feature = "transparent-inputs")]
+use {
+    crate::data_api::TransparentInputSource, std::collections::BTreeSet, std::convert::Infallible,
+    zcash_primitives::transaction::components::OutPoint,
+};
+
 /// The type of errors that may be produced in input selection.
 pub enum InputSelectorError<DbErrT, SelectorErrT> {
     /// An error occurred accessing the underlying data store.
@@ -73,17 +78,14 @@ impl<DE: fmt::Display, SE: fmt::Display> fmt::Display for InputSelectorError<DE,
     }
 }
 
-/// A data structure that describes the inputs to be consumed and outputs to
+/// The inputs to be consumed and outputs to be produced in a proposed transaction.
-/// be produced in a proposed transaction.
 pub struct Proposal<FeeRuleT, NoteRef> {
     transaction_request: TransactionRequest,
     transparent_inputs: Vec<WalletTransparentOutput>,
-    sapling_inputs: Vec<ReceivedSaplingNote<NoteRef>>,
+    sapling_inputs: Option<SaplingInputs<NoteRef>>,
     balance: TransactionBalance,
     fee_rule: FeeRuleT,
-    min_confirmations: NonZeroU32,
     min_target_height: BlockHeight,
-    min_anchor_height: BlockHeight,
     is_shielding: bool,
 }
 
@@ -97,8 +99,8 @@ impl<FeeRuleT, NoteRef> Proposal<FeeRuleT, NoteRef> {
         &self.transparent_inputs
     }
     /// Returns the Sapling inputs that have been selected to fund the transaction.
-    pub fn sapling_inputs(&self) -> &[ReceivedSaplingNote<NoteRef>] {
+    pub fn sapling_inputs(&self) -> Option<&SaplingInputs<NoteRef>> {
-        &self.sapling_inputs
+        self.sapling_inputs.as_ref()
     }
     /// Returns the change outputs to be added to the transaction and the fee to be paid.
     pub fn balance(&self) -> &TransactionBalance {
@@ -108,10 +110,6 @@ impl<FeeRuleT, NoteRef> Proposal<FeeRuleT, NoteRef> {
     pub fn fee_rule(&self) -> &FeeRuleT {
         &self.fee_rule
     }
-    /// Returns the value of `min_confirmations` used in input selection.
-    pub fn min_confirmations(&self) -> NonZeroU32 {
-        self.min_confirmations
-    }
     /// Returns the target height for which the proposal was prepared.
     ///
     /// The chain must contain at least this many blocks in order for the proposal to
@@ -119,14 +117,6 @@ impl<FeeRuleT, NoteRef> Proposal<FeeRuleT, NoteRef> {
     pub fn min_target_height(&self) -> BlockHeight {
         self.min_target_height
     }
-    /// Returns the anchor height used in preparing the proposal.
-    ///
-    /// If, at the time that the proposal is executed, the anchor height required to satisfy
-    /// the minimum confirmation depth is less than this height, the proposal execution
-    /// API should return an error.
-    pub fn min_anchor_height(&self) -> BlockHeight {
-        self.min_anchor_height
-    }
     /// Returns a flag indicating whether or not the proposed transaction
     /// is exclusively wallet-internal (if it does not involve any external
     /// recipients).
@@ -140,16 +130,41 @@ impl<FeeRuleT, NoteRef> Debug for Proposal<FeeRuleT, NoteRef> {
         f.debug_struct("Proposal")
             .field("transaction_request", &self.transaction_request)
             .field("transparent_inputs", &self.transparent_inputs)
-            .field("sapling_inputs", &self.sapling_inputs.len())
+            .field(
+                "sapling_inputs",
+                &self.sapling_inputs().map(|i| i.notes.len()),
+            )
+            .field(
+                "sapling_anchor_height",
+                &self.sapling_inputs().map(|i| i.anchor_height),
+            )
             .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_non_exhaustive()
     }
 }
 
+/// The Sapling inputs to a proposed transaction.
+pub struct SaplingInputs<NoteRef> {
+    anchor_height: BlockHeight,
+    notes: NonEmpty<ReceivedSaplingNote<NoteRef>>,
+}
+
+impl<NoteRef> SaplingInputs<NoteRef> {
+    /// Returns the anchor height for Sapling inputs that should be used when constructing the
+    /// proposed transaction.
+    pub fn anchor_height(&self) -> BlockHeight {
+        self.anchor_height
+    }
+
+    /// Returns the list of Sapling notes to be used as inputs to the proposed transaction.
+    pub fn notes(&self) -> &NonEmpty<ReceivedSaplingNote<NoteRef>> {
+        &self.notes
+    }
+}
+
 /// A strategy for selecting transaction inputs and proposing transaction outputs.
 ///
 /// Proposals should include only economically useful inputs, as determined by `Self::FeeRule`;
@@ -158,12 +173,12 @@ impl<FeeRuleT, NoteRef> Debug for Proposal<FeeRuleT, NoteRef> {
 pub trait InputSelector {
     /// The type of errors that may be generated in input selection
     type Error;
-    /// The type of data source that the input selector expects to access to obtain input notes and
+    /// The type of data source that the input selector expects to access to obtain input Sapling
-    /// UTXOs. This associated type permits input selectors that may use specialized knowledge of
+    /// notes. This associated type permits input selectors that may use specialized knowledge of
-    /// the internals of a particular backing data store, if the generic API of `WalletRead` does
+    /// the internals of a particular backing data store, if the generic API of
-    /// not provide sufficiently fine-grained operations for a particular backing store to
+    /// `SaplingInputSource` does not provide sufficiently fine-grained operations for a particular
-    /// optimally perform input selection.
+    /// backing store to optimally perform input selection.
-    type DataSource: WalletRead;
+    type InputSource: SaplingInputSource;
     /// The type of the fee rule that this input selector uses when computing fees.
     type FeeRule: FeeRule;
 
@@ -181,21 +196,38 @@ pub trait InputSelector {
     ///
     /// If insufficient funds are available to satisfy the required outputs for the shielding
     /// request, this operation must fail and return [`InputSelectorError::InsufficientFunds`].
-    #[allow(clippy::too_many_arguments)]
     #[allow(clippy::type_complexity)]
     fn propose_transaction<ParamsT>(
         &self,
         params: &ParamsT,
-        wallet_db: &Self::DataSource,
+        wallet_db: &Self::InputSource,
+        target_height: BlockHeight,
+        anchor_height: BlockHeight,
         account: AccountId,
         transaction_request: TransactionRequest,
-        min_confirmations: NonZeroU32,
     ) -> Result<
-        Proposal<Self::FeeRule, <<Self as InputSelector>::DataSource as WalletRead>::NoteRef>,
+        Proposal<Self::FeeRule, <Self::InputSource as SaplingInputSource>::NoteRef>,
-        InputSelectorError<<<Self as InputSelector>::DataSource as WalletRead>::Error, Self::Error>,
+        InputSelectorError<<Self::InputSource as SaplingInputSource>::Error, Self::Error>,
     >
     where
         ParamsT: consensus::Parameters;
+}
+
+/// A strategy for selecting transaction inputs and proposing transaction outputs
+/// for shielding-only transactions (transactions which spend transparent UTXOs and
+/// send all transaction outputs to the wallet's shielded internal address(es)).
+#[cfg(feature = "transparent-inputs")]
+pub trait ShieldingSelector {
+    /// The type of errors that may be generated in input selection
+    type Error;
+    /// The type of data source that the input selector expects to access to obtain input
+    /// transparent UTXOs. This associated type permits input selectors that may use specialized
+    /// knowledge of the internals of a particular backing data store, if the generic API of
+    /// `TransparentInputSource` does not provide sufficiently fine-grained operations for a
+    /// particular backing store to optimally perform input selection.
+    type InputSource: TransparentInputSource;
+    /// The type of the fee rule that this input selector uses when computing fees.
+    type FeeRule: FeeRule;
 
     /// Performs input selection and returns a proposal for the construction of a shielding
     /// transaction.
@@ -205,18 +237,18 @@ pub trait InputSelector {
     /// specified source addresses. If insufficient funds are available to satisfy the required
     /// outputs for the shielding request, this operation must fail and return
     /// [`InputSelectorError::InsufficientFunds`].
-    #[allow(clippy::too_many_arguments)]
     #[allow(clippy::type_complexity)]
     fn propose_shielding<ParamsT>(
         &self,
         params: &ParamsT,
-        wallet_db: &Self::DataSource,
+        wallet_db: &Self::InputSource,
         shielding_threshold: NonNegativeAmount,
         source_addrs: &[TransparentAddress],
-        min_confirmations: NonZeroU32,
+        target_height: BlockHeight,
+        min_confirmations: u32,
     ) -> Result<
-        Proposal<Self::FeeRule, <<Self as InputSelector>::DataSource as WalletRead>::NoteRef>,
+        Proposal<Self::FeeRule, Infallible>,
-        InputSelectorError<<<Self as InputSelector>::DataSource as WalletRead>::Error, Self::Error>,
+        InputSelectorError<<Self::InputSource as TransparentInputSource>::Error, Self::Error>,
     >
     where
         ParamsT: consensus::Parameters;
@@ -296,8 +328,8 @@ impl sapling::OutputView for SaplingPayment {
 /// An [`InputSelector`] implementation that uses a greedy strategy to select between available
 /// notes.
 ///
-/// This implementation performs input selection using methods available via the [`WalletRead`]
+/// This implementation performs input selection using methods available via the
-/// interface.
+/// [`SaplingInputSource`] and [`TransparentInputSource`] interfaces.
 pub struct GreedyInputSelector<DbT, ChangeT> {
     change_strategy: ChangeT,
     dust_output_policy: DustOutputPolicy,
@@ -318,32 +350,31 @@ impl<DbT, ChangeT: ChangeStrategy> GreedyInputSelector<DbT, ChangeT> {
 
 impl<DbT, ChangeT> InputSelector for GreedyInputSelector<DbT, ChangeT>
 where
-    DbT: WalletRead,
+    DbT: SaplingInputSource,
     ChangeT: ChangeStrategy,
     ChangeT::FeeRule: Clone,
 {
     type Error = GreedyInputSelectorError<ChangeT::Error, DbT::NoteRef>;
-    type DataSource = DbT;
+    type InputSource = DbT;
     type FeeRule = ChangeT::FeeRule;
 
     #[allow(clippy::type_complexity)]
     fn propose_transaction<ParamsT>(
         &self,
         params: &ParamsT,
-        wallet_db: &Self::DataSource,
+        wallet_db: &Self::InputSource,
+        target_height: BlockHeight,
+        anchor_height: BlockHeight,
         account: AccountId,
         transaction_request: TransactionRequest,
-        min_confirmations: NonZeroU32,
+    ) -> Result<
-    ) -> Result<Proposal<Self::FeeRule, DbT::NoteRef>, InputSelectorError<DbT::Error, Self::Error>>
+        Proposal<Self::FeeRule, DbT::NoteRef>,
+        InputSelectorError<<DbT as SaplingInputSource>::Error, Self::Error>,
+    >
     where
         ParamsT: consensus::Parameters,
+        Self::InputSource: SaplingInputSource,
     {
-        // Target the next block, assuming we are up-to-date.
-        let (target_height, anchor_height) = wallet_db
-            .get_target_and_anchor_heights(min_confirmations)
-            .map_err(InputSelectorError::DataSource)
-            .and_then(|x| x.ok_or(InputSelectorError::SyncRequired))?;
-
         let mut transparent_outputs = vec![];
         let mut sapling_outputs = vec![];
         for payment in transaction_request.payments() {
@@ -401,12 +432,15 @@ where
                     return Ok(Proposal {
                         transaction_request,
                         transparent_inputs: vec![],
-                        sapling_inputs,
+                        sapling_inputs: NonEmpty::from_vec(sapling_inputs).map(|notes| {
+                            SaplingInputs {
+                                anchor_height,
+                                notes,
+                            }
+                        }),
                         balance,
                         fee_rule: (*self.change_strategy.fee_rule()).clone(),
-                        min_confirmations,
                         min_target_height: target_height,
-                        min_anchor_height: anchor_height,
                         is_shielding: false,
                     });
                 }
@@ -446,27 +480,44 @@ where
             }
         }
     }
+}
+
+#[cfg(feature = "transparent-inputs")]
+impl<DbT, ChangeT> ShieldingSelector for GreedyInputSelector<DbT, ChangeT>
+where
+    DbT: TransparentInputSource,
+    ChangeT: ChangeStrategy,
+    ChangeT::FeeRule: Clone,
+{
+    type Error = GreedyInputSelectorError<ChangeT::Error, Infallible>;
+    type InputSource = DbT;
+    type FeeRule = ChangeT::FeeRule;
 
     #[allow(clippy::type_complexity)]
     fn propose_shielding<ParamsT>(
         &self,
         params: &ParamsT,
-        wallet_db: &Self::DataSource,
+        wallet_db: &Self::InputSource,
         shielding_threshold: NonNegativeAmount,
         source_addrs: &[TransparentAddress],
-        min_confirmations: NonZeroU32,
+        target_height: BlockHeight,
-    ) -> Result<Proposal<Self::FeeRule, DbT::NoteRef>, InputSelectorError<DbT::Error, Self::Error>>
+        min_confirmations: u32,
+    ) -> Result<
+        Proposal<Self::FeeRule, Infallible>,
+        InputSelectorError<<DbT as TransparentInputSource>::Error, Self::Error>,
+    >
     where
         ParamsT: consensus::Parameters,
     {
-        let (target_height, latest_anchor) = wallet_db
-            .get_target_and_anchor_heights(min_confirmations)
-            .map_err(InputSelectorError::DataSource)
-            .and_then(|x| x.ok_or(InputSelectorError::SyncRequired))?;
-
         let mut transparent_inputs: Vec<WalletTransparentOutput> = source_addrs
             .iter()
-            .map(|taddr| wallet_db.get_unspent_transparent_outputs(taddr, latest_anchor, &[]))
+            .map(|taddr| {
+                wallet_db.get_unspent_transparent_outputs(
+                    taddr,
+                    target_height - min_confirmations,
+                    &[],
+                )
+            })
             .collect::<Result<Vec<Vec<_>>, _>>()
             .map_err(InputSelectorError::DataSource)?
             .into_iter()
@@ -478,7 +529,7 @@ where
             target_height,
             &transparent_inputs,
             &Vec::<TxOut>::new(),
-            &Vec::<ReceivedSaplingNote<DbT::NoteRef>>::new(),
+            &Vec::<ReceivedSaplingNote<Infallible>>::new(),
             &Vec::<SaplingPayment>::new(),
             &self.dust_output_policy,
         );
@@ -494,7 +545,7 @@ where
                     target_height,
                     &transparent_inputs,
                     &Vec::<TxOut>::new(),
-                    &Vec::<ReceivedSaplingNote<DbT::NoteRef>>::new(),
+                    &Vec::<ReceivedSaplingNote<Infallible>>::new(),
                     &Vec::<SaplingPayment>::new(),
                     &self.dust_output_policy,
                 )?
@@ -508,12 +559,10 @@ where
             Ok(Proposal {
                 transaction_request: TransactionRequest::empty(),
                 transparent_inputs,
-                sapling_inputs: vec![],
+                sapling_inputs: None,
                 balance,
                 fee_rule: (*self.change_strategy.fee_rule()).clone(),
-                min_confirmations,
                 min_target_height: target_height,
-                min_anchor_height: latest_anchor,
                 is_shielding: true,
             })
         } else {

zcash_client_backend/src/fees.rs

@@ -112,7 +112,7 @@ pub enum ChangeError<E, NoteRefT> {
     DustInputs {
         /// The outpoints corresponding to transparent inputs having no current economic value.
         transparent: Vec<OutPoint>,
-        /// The identifiers for Sapling inputs having not current economic value
+        /// The identifiers for Sapling inputs having no current economic value
         sapling: Vec<NoteRefT>,
     },
     /// An error occurred that was specific to the change selection strategy in use.

zcash_client_sqlite/src/lib.rs

@@ -44,10 +44,7 @@ use std::{
 };
 
 use incrementalmerkletree::Position;
-use shardtree::{
+use shardtree::{error::ShardTreeError, ShardTree};
-    error::{QueryError, ShardTreeError},
-    ShardTree,
-};
 use zcash_primitives::{
     block::BlockHash,
     consensus::{self, BlockHeight},
@@ -55,10 +52,7 @@ use zcash_primitives::{
     memo::{Memo, MemoBytes},
     sapling,
     transaction::{
-        components::{
+        components::amount::{Amount, NonNegativeAmount},
-            amount::{Amount, NonNegativeAmount},
-            OutPoint,
-        },
         Transaction, TxId,
     },
     zip32::{AccountId, DiversifierIndex},
@@ -71,8 +65,8 @@ use zcash_client_backend::{
         chain::{BlockSource, CommitmentTreeRoot},
         scanning::{ScanPriority, ScanRange},
         AccountBirthday, BlockMetadata, DecryptedTransaction, NoteId, NullifierQuery, PoolType,
-        Recipient, ScannedBlock, SentTransaction, ShieldedProtocol, WalletCommitmentTrees,
+        Recipient, SaplingInputSource, ScannedBlock, SentTransaction, ShieldedProtocol,
-        WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT,
+        WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT,
     },
     keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
     proto::compact_formats::CompactBlock,
@@ -82,6 +76,12 @@ use zcash_client_backend::{
 
 use crate::{error::SqliteClientError, wallet::commitment_tree::SqliteShardStore};
 
+#[cfg(feature = "transparent-inputs")]
+use {
+    zcash_client_backend::data_api::TransparentInputSource,
+    zcash_primitives::transaction::components::OutPoint,
+};
+
 #[cfg(feature = "unstable")]
 use {
     crate::chain::{fsblockdb_with_blocks, BlockMeta},
@@ -94,7 +94,7 @@ pub mod error;
 
 pub mod wallet;
 use wallet::{
-    commitment_tree::{self, get_checkpoint_depth, put_shard_roots},
+    commitment_tree::{self, put_shard_roots},
     SubtreeScanProgress,
 };
 
@@ -167,10 +167,54 @@ impl<P: consensus::Parameters + Clone> WalletDb<Connection, P> {
     }
 }
 
-impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for WalletDb<C, P> {
+impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> SaplingInputSource
+    for WalletDb<C, P>
+{
     type Error = SqliteClientError;
     type NoteRef = ReceivedNoteId;
 
+    fn select_spendable_sapling_notes(
+        &self,
+        account: AccountId,
+        target_value: Amount,
+        anchor_height: BlockHeight,
+        exclude: &[Self::NoteRef],
+    ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error> {
+        wallet::sapling::select_spendable_sapling_notes(
+            self.conn.borrow(),
+            account,
+            target_value,
+            anchor_height,
+            exclude,
+        )
+    }
+}
+
+#[cfg(feature = "transparent-inputs")]
+impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> TransparentInputSource
+    for WalletDb<C, P>
+{
+    type Error = SqliteClientError;
+
+    fn get_unspent_transparent_outputs(
+        &self,
+        address: &TransparentAddress,
+        max_height: BlockHeight,
+        exclude: &[OutPoint],
+    ) -> Result<Vec<WalletTransparentOutput>, Self::Error> {
+        wallet::get_unspent_transparent_outputs(
+            self.conn.borrow(),
+            &self.params,
+            address,
+            max_height,
+            exclude,
+        )
+    }
+}
+
+impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for WalletDb<C, P> {
+    type Error = SqliteClientError;
+
     fn chain_height(&self) -> Result<Option<BlockHeight>, Self::Error> {
         wallet::scan_queue_extrema(self.conn.borrow())
             .map(|h| h.map(|range| *range.end()))
@@ -277,36 +321,6 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
         }
     }
 
-    fn get_spendable_sapling_notes(
-        &self,
-        account: AccountId,
-        anchor_height: BlockHeight,
-        exclude: &[Self::NoteRef],
-    ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error> {
-        wallet::sapling::get_spendable_sapling_notes(
-            self.conn.borrow(),
-            account,
-            anchor_height,
-            exclude,
-        )
-    }
-
-    fn select_spendable_sapling_notes(
-        &self,
-        account: AccountId,
-        target_value: Amount,
-        anchor_height: BlockHeight,
-        exclude: &[Self::NoteRef],
-    ) -> Result<Vec<ReceivedSaplingNote<Self::NoteRef>>, Self::Error> {
-        wallet::sapling::select_spendable_sapling_notes(
-            self.conn.borrow(),
-            account,
-            target_value,
-            anchor_height,
-            exclude,
-        )
-    }
-
     fn get_transparent_receivers(
         &self,
         _account: AccountId,
@@ -320,27 +334,6 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
         );
     }
 
-    fn get_unspent_transparent_outputs(
-        &self,
-        _address: &TransparentAddress,
-        _max_height: BlockHeight,
-        _exclude: &[OutPoint],
-    ) -> Result<Vec<WalletTransparentOutput>, Self::Error> {
-        #[cfg(feature = "transparent-inputs")]
-        return wallet::get_unspent_transparent_outputs(
-            self.conn.borrow(),
-            &self.params,
-            _address,
-            _max_height,
-            _exclude,
-        );
-
-        #[cfg(not(feature = "transparent-inputs"))]
-        panic!(
-            "The wallet must be compiled with the transparent-inputs feature to use this method."
-        );
-    }
-
     fn get_transparent_balances(
         &self,
         _account: AccountId,
@@ -534,7 +527,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
 
                 // Update the Sapling note commitment tree with all newly read note commitments
                 let mut subtrees = subtrees.into_iter();
-                wdb.with_sapling_tree_mut::<_, _, SqliteClientError>(move |sapling_tree| {
+                wdb.with_sapling_tree_mut::<_, _, Self::Error>(move |sapling_tree| {
                     for (tree, checkpoints) in &mut subtrees {
                         sapling_tree.insert_tree(tree, checkpoints)?;
                     }
@@ -792,18 +785,6 @@ 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)))?
-            // `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))
-    }
 }
 
 impl<'conn, P: consensus::Parameters> WalletCommitmentTrees for WalletDb<SqlTransaction<'conn>, P> {
@@ -844,18 +825,6 @@ 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/testing.rs

@@ -70,7 +70,9 @@ use super::BlockDb;
 
 #[cfg(feature = "transparent-inputs")]
 use {
-    zcash_client_backend::data_api::wallet::{propose_shielding, shield_transparent_funds},
+    zcash_client_backend::data_api::wallet::{
+        input_selection::ShieldingSelector, propose_shielding, shield_transparent_funds,
+    },
     zcash_primitives::legacy::TransparentAddress,
 };
 
@@ -483,7 +485,7 @@ impl<Cache> TestState<Cache> {
         >,
     >
     where
-        InputsT: InputSelector<DataSource = WalletDb<Connection, Network>>,
+        InputsT: InputSelector<InputSource = WalletDb<Connection, Network>>,
     {
         let params = self.network();
         let prover = test_prover();
@@ -518,7 +520,7 @@ impl<Cache> TestState<Cache> {
         >,
     >
     where
-        InputsT: InputSelector<DataSource = WalletDb<Connection, Network>>,
+        InputsT: InputSelector<InputSource = WalletDb<Connection, Network>>,
     {
         let params = self.network();
         propose_transfer::<_, _, _, Infallible>(
@@ -575,9 +577,9 @@ impl<Cache> TestState<Cache> {
         input_selector: &InputsT,
         shielding_threshold: NonNegativeAmount,
         from_addrs: &[TransparentAddress],
-        min_confirmations: NonZeroU32,
+        min_confirmations: u32,
     ) -> Result<
-        Proposal<InputsT::FeeRule, ReceivedNoteId>,
+        Proposal<InputsT::FeeRule, Infallible>,
         data_api::error::Error<
             SqliteClientError,
             Infallible,
@@ -586,7 +588,7 @@ impl<Cache> TestState<Cache> {
         >,
     >
     where
-        InputsT: InputSelector<DataSource = WalletDb<Connection, Network>>,
+        InputsT: ShieldingSelector<InputSource = WalletDb<Connection, Network>>,
     {
         let params = self.network();
         propose_shielding::<_, _, _, Infallible>(
@@ -639,7 +641,7 @@ impl<Cache> TestState<Cache> {
         shielding_threshold: NonNegativeAmount,
         usk: &UnifiedSpendingKey,
         from_addrs: &[TransparentAddress],
-        min_confirmations: NonZeroU32,
+        min_confirmations: u32,
     ) -> Result<
         TxId,
         data_api::error::Error<
@@ -650,7 +652,7 @@ impl<Cache> TestState<Cache> {
         >,
     >
     where
-        InputsT: InputSelector<DataSource = WalletDb<Connection, Network>>,
+        InputsT: ShieldingSelector<InputSource = WalletDb<Connection, Network>>,
     {
         let params = self.network();
         let prover = test_prover();

zcash_client_sqlite/src/wallet.rs

@@ -67,7 +67,6 @@
 use incrementalmerkletree::Retention;
 use rusqlite::{self, named_params, OptionalExtension};
 use shardtree::ShardTree;
-use std::cmp;
 use std::collections::{BTreeMap, HashMap};
 use std::convert::TryFrom;
 use std::io::{self, Cursor};
@@ -100,7 +99,7 @@ use zcash_client_backend::{
     wallet::WalletTx,
 };
 
-use crate::wallet::commitment_tree::SqliteShardStore;
+use crate::wallet::commitment_tree::{get_max_checkpointed_height, SqliteShardStore};
 use crate::{
     error::SqliteClientError, SqlTransaction, WalletCommitmentTrees, WalletDb, PRUNING_DEPTH,
     SAPLING_TABLES_PREFIX,
@@ -932,19 +931,19 @@ pub(crate) fn get_target_and_anchor_heights(
     conn: &rusqlite::Connection,
     min_confirmations: NonZeroU32,
 ) -> Result<Option<(BlockHeight, BlockHeight)>, rusqlite::Error> {
-    scan_queue_extrema(conn).map(|heights| {
+    match scan_queue_extrema(conn)?.map(|range| *range.end()) {
-        heights.map(|range| {
+        Some(chain_tip_height) => {
-            let target_height = *range.end() + 1;
+            let sapling_anchor_height = get_max_checkpointed_height(
-            // Select an anchor min_confirmations back from the target block,
+                conn,
-            // unless that would be before the earliest block we have.
+                SAPLING_TABLES_PREFIX,
-            let anchor_height = BlockHeight::from(cmp::max(
+                chain_tip_height,
-                u32::from(target_height).saturating_sub(min_confirmations.into()),
+                min_confirmations,
-                u32::from(*range.start()),
+            )?;
-            ));
 
-            (target_height, anchor_height)
+            Ok(sapling_anchor_height.map(|h| (chain_tip_height + 1, h)))
-        })
+        }
-    })
+        None => Ok(None),
+    }
 }
 
 fn parse_block_metadata(
@@ -1915,7 +1914,9 @@ mod tests {
             PRUNING_DEPTH,
         },
         zcash_client_backend::{
-            data_api::{wallet::input_selection::GreedyInputSelector, WalletWrite},
+            data_api::{
+                wallet::input_selection::GreedyInputSelector, TransparentInputSource, WalletWrite,
+            },
             encoding::AddressCodec,
             fees::{fixed, DustOutputPolicy},
             wallet::WalletTransparentOutput,
@@ -2141,13 +2142,7 @@ mod tests {
             DustOutputPolicy::default(),
         );
         let txid = st
-            .shield_transparent_funds(
+            .shield_transparent_funds(&input_selector, value, &usk, &[*taddr], 1)
-                &input_selector,
-                value,
-                &usk,
-                &[*taddr],
-                NonZeroU32::new(1).unwrap(),
-            )
             .unwrap();
 
         // The wallet should have zero transparent balance, because the shielding

zcash_client_sqlite/src/wallet/commitment_tree.rs

@@ -21,8 +21,6 @@ use zcash_primitives::{consensus::BlockHeight, merkle_tree::HashSer};
 
 use zcash_client_backend::serialization::shardtree::{read_shard, write_shard};
 
-use super::scan_queue_extrema;
-
 /// Errors that can appear in SQLite-back [`ShardStore`] implementation operations.
 #[derive(Debug)]
 pub enum Error {
@@ -173,6 +171,7 @@ impl<'conn, 'a: 'conn, H: HashSer, const SHARD_HEIGHT: u8> ShardStore
         checkpoint_depth: usize,
     ) -> Result<Option<(Self::CheckpointId, Checkpoint)>, Self::Error> {
         get_checkpoint_at_depth(self.conn, self.table_prefix, checkpoint_depth)
+            .map_err(Error::Query)
     }
 
     fn get_checkpoint(
@@ -280,6 +279,7 @@ impl<H: HashSer, const SHARD_HEIGHT: u8> ShardStore
         checkpoint_depth: usize,
     ) -> Result<Option<(Self::CheckpointId, Checkpoint)>, Self::Error> {
         get_checkpoint_at_depth(&self.conn, self.table_prefix, checkpoint_depth)
+            .map_err(Error::Query)
     }
 
     fn get_checkpoint(
@@ -750,38 +750,37 @@ pub(crate) fn get_checkpoint(
         .transpose()
 }
 
-pub(crate) fn get_checkpoint_depth(
+pub(crate) fn get_max_checkpointed_height(
     conn: &rusqlite::Connection,
     table_prefix: &'static str,
+    chain_tip_height: BlockHeight,
     min_confirmations: NonZeroU32,
-) -> Result<Option<usize>, rusqlite::Error> {
+) -> Result<Option<BlockHeight>, rusqlite::Error> {
-    scan_queue_extrema(conn)?
+    let max_checkpoint_height =
-        .map(|range| *range.end())
+        u32::from(chain_tip_height).saturating_sub(u32::from(min_confirmations) - 1);
-        .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
+    // We exclude from consideration all checkpoints having heights greater than the maximum
-            // checkpoint height. The checkpoint depth is the number of excluded checkpoints + 1.
+    // checkpoint height. The checkpoint depth is the number of excluded checkpoints + 1.
-            conn.query_row(
+    conn.query_row(
-                &format!(
+        &format!(
-                    "SELECT COUNT(*) 
+            "SELECT checkpoint_id
-                     FROM {}_tree_checkpoints
+             FROM {}_tree_checkpoints
-                     WHERE checkpoint_id > :max_checkpoint_height",
+             WHERE checkpoint_id <= :max_checkpoint_height
-                    table_prefix
+             ORDER BY checkpoint_id DESC
-                ),
+             LIMIT 1",
-                named_params![":max_checkpoint_height": max_checkpoint_height],
+            table_prefix
-                |row| row.get::<_, usize>(0).map(|s| s + 1),
+        ),
-            )
+        named_params![":max_checkpoint_height": max_checkpoint_height],
-        })
+        |row| row.get::<_, u32>(0).map(BlockHeight::from),
-        .transpose()
+    )
+    .optional()
 }
 
 pub(crate) fn get_checkpoint_at_depth(
     conn: &rusqlite::Connection,
     table_prefix: &'static str,
     checkpoint_depth: usize,
-) -> Result<Option<(BlockHeight, Checkpoint)>, Error> {
+) -> Result<Option<(BlockHeight, Checkpoint)>, rusqlite::Error> {
     if checkpoint_depth == 0 {
         return Ok(None);
     }
@@ -806,27 +805,21 @@ pub(crate) fn get_checkpoint_at_depth(
                 ))
             },
         )
-        .optional()
+        .optional()?;
-        .map_err(Error::Query)?;
 
     checkpoint_parts
         .map(|(checkpoint_id, pos_opt)| {
-            let mut stmt = conn
+            let mut stmt = conn.prepare_cached(&format!(
-                .prepare_cached(&format!(
+                "SELECT mark_removed_position
-                    "SELECT mark_removed_position
                     FROM {}_tree_checkpoint_marks_removed
                     WHERE checkpoint_id = ?",
-                    table_prefix
+                table_prefix
-                ))
+            ))?;
-                .map_err(Error::Query)?;
+            let mark_removed_rows = stmt.query([u32::from(checkpoint_id)])?;
-            let mark_removed_rows = stmt
-                .query([u32::from(checkpoint_id)])
-                .map_err(Error::Query)?;
 
             let marks_removed = mark_removed_rows
                 .mapped(|row| row.get::<_, u64>(0).map(Position::from))
-                .collect::<Result<BTreeSet<_>, _>>()
+                .collect::<Result<BTreeSet<_>, _>>()?;
-                .map_err(Error::Query)?;
 
             Ok((
                 checkpoint_id,
@@ -1168,6 +1161,7 @@ mod tests {
 
         // simulate discovery of a note
         let mut tree = ShardTree::<_, 6, 3>::new(store, 10);
+        let checkpoint_height = BlockHeight::from(3);
         tree.batch_insert(
             Position::from(24),
             ('a'..='h').into_iter().map(|c| {
@@ -1176,7 +1170,7 @@ mod tests {
                     match c {
                         'c' => Retention::Marked,
                         'h' => Retention::Checkpoint {
-                            id: BlockHeight::from(3),
+                            id: checkpoint_height,
                             is_marked: false,
                         },
                         _ => Retention::Ephemeral,
@@ -1187,7 +1181,9 @@ mod tests {
         .unwrap();
 
         // construct a witness for the note
-        let witness = tree.witness(Position::from(26), 0).unwrap();
+        let witness = tree
+            .witness_at_checkpoint_id(Position::from(26), &checkpoint_height)
+            .unwrap();
         assert_eq!(
             witness.path_elems(),
             &[

zcash_client_sqlite/src/wallet/sapling.rs

@@ -159,62 +159,6 @@ fn unscanned_tip_exists(
     )
 }
 
-pub(crate) fn get_spendable_sapling_notes(
-    conn: &Connection,
-    account: AccountId,
-    anchor_height: BlockHeight,
-    exclude: &[ReceivedNoteId],
-) -> Result<Vec<ReceivedSaplingNote<ReceivedNoteId>>, SqliteClientError> {
-    let birthday_height = match wallet_birthday(conn)? {
-        Some(birthday) => birthday,
-        None => {
-            // the wallet birthday can only be unknown if there are no accounts in the wallet; in
-            // such a case, the wallet has no notes to spend.
-            return Ok(vec![]);
-        }
-    };
-
-    if unscanned_tip_exists(conn, anchor_height)? {
-        return Ok(vec![]);
-    }
-
-    let mut stmt_select_notes = conn.prepare_cached(
-        "SELECT id_note, txid, output_index, diversifier, value, rcm, commitment_tree_position
-         FROM sapling_received_notes
-         INNER JOIN transactions ON transactions.id_tx = sapling_received_notes.tx
-         WHERE account = :account
-         AND commitment_tree_position IS NOT NULL
-         AND spent IS NULL
-         AND transactions.block <= :anchor_height
-         AND id_note NOT IN rarray(:exclude)
-         AND NOT EXISTS (
-            SELECT 1 FROM v_sapling_shard_unscanned_ranges unscanned
-            -- select all the unscanned ranges involving the shard containing this note
-            WHERE sapling_received_notes.commitment_tree_position >= unscanned.start_position
-            AND sapling_received_notes.commitment_tree_position < unscanned.end_position_exclusive
-            -- exclude unscanned ranges that start above the anchor height (they don't affect spendability)
-            AND unscanned.block_range_start <= :anchor_height
-            -- exclude unscanned ranges that end below the wallet birthday
-            AND unscanned.block_range_end > :wallet_birthday
-         )",
-    )?;
-
-    let excluded: Vec<Value> = exclude.iter().map(|n| Value::from(n.0)).collect();
-    let excluded_ptr = Rc::new(excluded);
-
-    let notes = stmt_select_notes.query_and_then(
-        named_params![
-            ":account": u32::from(account),
-            ":anchor_height": u32::from(anchor_height),
-            ":exclude": &excluded_ptr,
-            ":wallet_birthday": u32::from(birthday_height)
-        ],
-        to_spendable_note,
-    )?;
-
-    notes.collect::<Result<_, _>>()
-}
-
 pub(crate) fn select_spendable_sapling_notes(
     conn: &Connection,
     account: AccountId,
@@ -764,7 +708,7 @@ pub(crate) mod tests {
             st.propose_standard_transfer::<Infallible>(
                 account,
                 StandardFeeRule::Zip317,
-                NonZeroU32::new(10).unwrap(),
+                NonZeroU32::new(2).unwrap(),
                 &to,
                 NonNegativeAmount::const_from_u64(70000),
                 None,
@@ -1463,7 +1407,7 @@ pub(crate) mod tests {
                 NonNegativeAmount::from_u64(10000).unwrap(),
                 &usk,
                 &[*taddr],
-                NonZeroU32::new(1).unwrap()
+                1
             ),
             Ok(_)
         );

zcash_primitives/src/transaction/sighash.rs

@@ -5,7 +5,7 @@ use super::{
     components::{
         amount::NonNegativeAmount,
         sapling::{self, GrothProofBytes},
-        transparent, Amount,
+        transparent,
     },
     sighash_v4::v4_signature_hash,
     sighash_v5::v5_signature_hash,
@@ -13,7 +13,7 @@ use super::{
 };
 
 #[cfg(feature = "zfuture")]
-use crate::extensions::transparent::Precondition;
+use {super::components::Amount, crate::extensions::transparent::Precondition};
 
 pub const SIGHASH_ALL: u8 = 0x01;
 pub const SIGHASH_NONE: u8 = 0x02;