zcash_client_backend: Factor out input source traits 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. In order to minimize the operations of the newly-created `InputSource` and `ShieldingSource` traits, this change also removes the `min_confirmations` field from transaction proposals, in favor of storing explicit target and anchor heights. This has the effect of limiting the lifetime of transaction proposals to `PRUNING_DEPTH - min_confirmations` blocks.
nuttycom · Nov 3, 2023 · ef1d985 · ef1d985
1 parent 13fb0a4
commit ef1d985
Show file tree

Hide file tree

Showing 14 changed files with 439 additions and 422 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -108,3 +108,7 @@ fpe = "0.6"
 lto = true
 panic = 'abort'
 codegen-units = 1
+
+[patch.crates-io]
+incrementalmerkletree = { git = "https://github.com/nuttycom/incrementalmerkletree.git", rev = "c8351fc4f0863b8e69d7c0617bf5c82fc0035ce4" }
+shardtree = { git = "https://github.com/nuttycom/incrementalmerkletree.git", rev = "c8351fc4f0863b8e69d7c0617bf5c82fc0035ce4" }
diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md
@@ -15,13 +15,19 @@ and this library adheres to Rust's notion of
 - `zcash_client_backend::data_api::error::Error` has new error variant:
   - `Error::UnsupportedPoolType(zcash_client_backend::data_api::PoolType)`
 - Added module `zcash_client_backend::fees::standard`
-- Added `zcash_client_backend::wallet::input_selection::Proposal::min_confirmations`
 - Added methods to `zcash_client_backend::wallet::ReceivedSaplingNote`:
   `{from_parts, txid, output_index, diversifier, rseed, note_commitment_tree_position}`.
+- `zcash_client_backend::data_api::TransparentInputSource`
+- `zcash_client_backend::data_api::SaplingInputSource`
+- `zcash_client_backend::data_api::wallet::input_selection::SaplingInputs`
+- `zcash_client_backend::data_api::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.
 
 ### 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.
@@ -49,7 +55,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.
 - `zcash_client_backend::fees::ChangeValue::Sapling` is now a structured variant.
@@ -68,11 +74,44 @@ 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::wallet::input_selection::InputSelector::propose_shielding`,
+  has been moved out to the newly-created `ShieldingSelector` trait.
+- The `zcash_client_backend::data_api::wallet::input_selection::InputSelector::DataSource`
+  associated type has been renamed to `InputSource`.
+- The signature of `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.
+- `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`.
+- `zcash_client_backend::data_api::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.
+- `zcash_client_backend::data_api::wallet::create_proposed_transaction` now forces
+  implementations to ignore the database identifiers for its contained notes by
+  universally quantifying the `NoteRef` type parameter.
+- `zcash_client_backend::data_api::wallet::input_selection::Proposal::sapling_inputs`
+  now returns type `Option<&SaplingInputs>`.
+- `zcash_client_backend::data_api::wallet::input_selection::Proposal::min_anchor_height`
+  has been removed in favor of storing this value in `SaplingInputs`.
+- `zcash_client_backend::data_api::wallet::input_selection::GreedyInputSelector`
+  now has relaxed requirements for its `InputSource` associated type.
 
 ### 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
 

diff --git a/zcash_client_backend/Cargo.toml b/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 = "0.7"
 
 # - 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"] }
 

diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs
@@ -210,12 +210,7 @@ impl WalletSummary {
     }
 }
 
-/// 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 {
+pub trait SaplingInputSource {
     /// The type of errors produced by a wallet backend.
     type Error;
 
@@ -225,6 +220,39 @@ 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>;
+}
+
+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 {
+    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 +379,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 +389,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 +898,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(
@@ -954,8 +946,9 @@ pub mod testing {
 
     use super::{
         chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, BlockMetadata,
-        DecryptedTransaction, NoteId, NullifierQuery, ScannedBlock, SentTransaction,
-        WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT,
+        DecryptedTransaction, NoteId, NullifierQuery, SaplingInputSource, ScannedBlock,
+        SentTransaction, TransparentInputSource, WalletCommitmentTrees, WalletRead, WalletSummary,
+        WalletWrite, SAPLING_SHARD_HEIGHT,
     };
 
     pub struct MockWalletDb {
@@ -976,10 +969,37 @@ 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())
+        }
+    }
+
+    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,41 +1099,13 @@ 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,
         ) -> Result<HashMap<TransparentAddress, AddressMetadata>, Self::Error> {
             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 +1206,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())
-        }
     }
 }