zcash_client_backend: Factor out InputSource from WalletRead

Prior to this change, it's necessary to implement the entirety of the
`WalletRead` trait in order to be able to use the input selection
functionality provided by `zcash_client_backend::data_api::input_selection`.
This change factors out the minimal operations required for transaction
proposal construction to better reflect the principle of least authority
and make the input selection code reusable in more contexts.

zcash_client_backend/CHANGELOG.md

@@ -21,6 +21,7 @@ and this library adheres to Rust's notion of
 ### Changed
 - `zcash_client_backend::data_api::chain::scan_cached_blocks` now returns
   a `ScanSummary` containing metadata about the scanned blocks on success.
+
 - The fields of `zcash_client_backend::wallet::ReceivedSaplingNote` are now
   private. Use `ReceivedSaplingNote::from_parts` for construction instead.
   Accessor methods are provided for each previously-public field.
@@ -63,6 +64,16 @@ and this library adheres to Rust's notion of
   updated accordingly.
 - Almost all uses of `Amount` in `zcash_client_backend::zip321` have been replaced
   with `NonNegativeAmount`.
+- In order to support better reusability for input selection code, three new
+  supertraits have been factored out from `zcash_client_backend::data_api::WalletRead`:
+  - `zcash_client_backend::data_api::TransparentInputSource`
+  - `zcash_client_backend::data_api::SaplingInputSource`
+  - `zcash_client_backend::data_api::InputSource`
+- In `zcash_client_backend::data_api::wallet::input_selection::InputSelector`,
+  the signatures of `propose_transaction` and `propose_shielding` have been altered
+  such that these methods no longer take `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::InputSource` trait must expose.
 
 ## [0.10.0] - 2023-09-25
 

zcash_client_backend/Cargo.toml

@@ -87,6 +87,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/chain.rs

@@ -163,6 +163,8 @@ use crate::{
 pub mod error;
 use error::Error;
 
+use super::WalletRead;
+
 /// A struct containing metadata about a subtree root of the note commitment tree.
 ///
 /// This stores the block height at which the leaf that completed the subtree was
@@ -274,7 +276,7 @@ pub fn scan_cached_blocks<ParamsT, DbT, BlockSourceT>(
     data_db: &mut DbT,
     from_height: BlockHeight,
     limit: usize,
-) -> Result<ScanSummary, Error<DbT::Error, BlockSourceT::Error>>
+) -> Result<ScanSummary, Error<<DbT as WalletRead>::Error, BlockSourceT::Error>>
 where
     ParamsT: consensus::Parameters + Send + 'static,
     BlockSourceT: BlockSource,
@@ -327,7 +329,7 @@ where
     };
 
     let mut continuity_check_metadata = prior_block_metadata;
-    block_source.with_blocks::<_, DbT::Error>(
+    block_source.with_blocks::<_, <DbT as WalletRead>::Error>(
         Some(from_height),
         Some(limit),
         |block: CompactBlock| {
@@ -378,7 +380,7 @@ where
     let mut scan_end_height = from_height;
     let mut received_note_count = 0;
     let mut spent_note_count = 0;
-    block_source.with_blocks::<_, DbT::Error>(
+    block_source.with_blocks::<_, <DbT as WalletRead>::Error>(
         Some(from_height),
         Some(limit),
         |block: CompactBlock| {

zcash_client_backend/src/data_api/wallet.rs

@@ -51,7 +51,7 @@ pub fn decrypt_and_store_transaction<ParamsT, DbT>(
     params: &ParamsT,
     data: &mut DbT,
     tx: &Transaction,
-) -> Result<(), DbT::Error>
+) -> Result<(), <DbT as WalletRead>::Error>
 where
     ParamsT: consensus::Parameters,
     DbT: WalletWrite,
@@ -357,21 +357,33 @@ 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::NoteRef: Copy + Eq + Ord,
     ParamsT: consensus::Parameters + Clone,
     InputsT: InputSelector<DataSource = DbT>,
 {
+    use self::input_selection::InputSelectorError;
+    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)
 }
@@ -410,7 +422,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,
@@ -459,21 +471,33 @@ pub fn propose_shielding<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
     ParamsT: consensus::Parameters,
     DbT: WalletWrite,
     DbT::NoteRef: Copy + Eq + Ord,
     InputsT: InputSelector<DataSource = DbT>,
 {
+    use self::input_selection::InputSelectorError;
+    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_shielding(
             params,
             wallet_db,
             shielding_threshold,
+            target_height,
+            anchor_height,
             from_addrs,
-            min_confirmations,
         )
         .map_err(Error::from)
 }

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -2,7 +2,6 @@
 
 use core::marker::PhantomData;
 use std::fmt;
-use std::num::NonZeroU32;
 use std::{collections::BTreeSet, fmt::Debug};
 
 use zcash_primitives::{
@@ -19,9 +18,10 @@ use zcash_primitives::{
     zip32::AccountId,
 };
 
+use crate::data_api::{SaplingInputSource, TransparentInputSource};
 use crate::{
     address::{RecipientAddress, UnifiedAddress},
-    data_api::WalletRead,
+    data_api::InputSource,
     fees::{ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance},
     wallet::{ReceivedSaplingNote, WalletTransparentOutput},
     zip321::TransactionRequest,
@@ -158,7 +158,7 @@ pub trait InputSelector {
     /// the internals of a particular backing data store, if the generic API of `WalletRead` does
     /// not provide sufficiently fine-grained operations for a particular backing store to
     /// optimally perform input selection.
-    type DataSource: WalletRead;
+    type DataSource: InputSource;
     /// The type of the fee rule that this input selector uses when computing fees.
     type FeeRule: FeeRule;
 
@@ -182,12 +182,19 @@ pub trait InputSelector {
         &self,
         params: &ParamsT,
         wallet_db: &Self::DataSource,
+        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>,
-        InputSelectorError<<<Self as InputSelector>::DataSource as WalletRead>::Error, Self::Error>,
+        Proposal<
+            Self::FeeRule,
+            <<Self as InputSelector>::DataSource as SaplingInputSource>::NoteRef,
+        >,
+        InputSelectorError<
+            <<Self as InputSelector>::DataSource as InputSource>::Error,
+            Self::Error,
+        >,
     >
     where
         ParamsT: consensus::Parameters;
@@ -207,11 +214,18 @@ pub trait InputSelector {
         params: &ParamsT,
         wallet_db: &Self::DataSource,
         shielding_threshold: NonNegativeAmount,
+        target_height: BlockHeight,
+        latest_anchor: BlockHeight,
         source_addrs: &[TransparentAddress],
-        min_confirmations: NonZeroU32,
     ) -> Result<
-        Proposal<Self::FeeRule, <<Self as InputSelector>::DataSource as WalletRead>::NoteRef>,
-        InputSelectorError<<<Self as InputSelector>::DataSource as WalletRead>::Error, Self::Error>,
+        Proposal<
+            Self::FeeRule,
+            <<Self as InputSelector>::DataSource as SaplingInputSource>::NoteRef,
+        >,
+        InputSelectorError<
+            <<Self as InputSelector>::DataSource as InputSource>::Error,
+            Self::Error,
+        >,
     >
     where
         ParamsT: consensus::Parameters;
@@ -313,7 +327,7 @@ impl<DbT, ChangeT: ChangeStrategy> GreedyInputSelector<DbT, ChangeT> {
 
 impl<DbT, ChangeT> InputSelector for GreedyInputSelector<DbT, ChangeT>
 where
-    DbT: WalletRead,
+    DbT: InputSource,
     ChangeT: ChangeStrategy,
     ChangeT::FeeRule: Clone,
 {
@@ -326,19 +340,17 @@ where
         &self,
         params: &ParamsT,
         wallet_db: &Self::DataSource,
+        target_height: BlockHeight,
+        anchor_height: BlockHeight,
         account: AccountId,
         transaction_request: TransactionRequest,
-        min_confirmations: NonZeroU32,
-    ) -> Result<Proposal<Self::FeeRule, DbT::NoteRef>, InputSelectorError<DbT::Error, Self::Error>>
+    ) -> Result<
+        Proposal<Self::FeeRule, DbT::NoteRef>,
+        InputSelectorError<<DbT as InputSource>::Error, Self::Error>,
+    >
     where
         ParamsT: consensus::Parameters,
     {
-        // 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() {
@@ -447,17 +459,16 @@ where
         params: &ParamsT,
         wallet_db: &Self::DataSource,
         shielding_threshold: NonNegativeAmount,
+        target_height: BlockHeight,
+        latest_anchor: BlockHeight,
         source_addrs: &[TransparentAddress],
-        min_confirmations: NonZeroU32,
-    ) -> Result<Proposal<Self::FeeRule, DbT::NoteRef>, InputSelectorError<DbT::Error, Self::Error>>
+    ) -> Result<
+        Proposal<Self::FeeRule, DbT::NoteRef>,
+        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, &[]))