From 98b14b3c8b7237d43e736b8ca36eda7016fd0f7c Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 1 Nov 2023 12:23:09 -0600 Subject: [PATCH] WIP: eliminate input selection trait inheritance --- zcash_client_backend/src/data_api.rs | 36 ++-------- zcash_client_backend/src/data_api/wallet.rs | 28 ++++---- .../src/data_api/wallet/input_selection.rs | 67 +++++++++---------- zcash_client_backend/src/fees.rs | 2 +- 4 files changed, 50 insertions(+), 83 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index d9b4dd2f5a..718ba51984 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -245,19 +245,12 @@ pub trait TransparentInputSource { ) -> Result, Self::Error>; } -pub trait InputSource: - SaplingInputSource::Error> - + TransparentInputSource::Error> -{ - type 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: InputSource::Error> { +pub trait WalletRead { type Error; /// Returns the height of the chain as known to the wallet as of the most recent call to @@ -405,14 +398,6 @@ pub trait WalletRead: InputSource::Error> { query: NullifierQuery, ) -> Result, ::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>, ::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 @@ -1001,9 +986,9 @@ pub mod testing { use super::{ chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, BlockMetadata, - DecryptedTransaction, InputSource, NoteId, NullifierQuery, SaplingInputSource, - ScannedBlock, SentTransaction, TransparentInputSource, WalletCommitmentTrees, WalletRead, - WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + DecryptedTransaction, NoteId, NullifierQuery, SaplingInputSource, ScannedBlock, + SentTransaction, TransparentInputSource, WalletCommitmentTrees, WalletRead, WalletSummary, + WalletWrite, SAPLING_SHARD_HEIGHT, }; pub struct MockWalletDb { @@ -1052,10 +1037,6 @@ pub mod testing { } } - impl InputSource for MockWalletDb { - type Error = (); - } - impl WalletRead for MockWalletDb { type Error = (); @@ -1179,15 +1160,6 @@ pub mod testing { Ok(Vec::new()) } - fn get_spendable_sapling_notes( - &self, - _account: AccountId, - _anchor_height: BlockHeight, - _exclude: &[Self::NoteRef], - ) -> Result>, ::Error> { - Ok(Vec::new()) - } - fn get_transparent_receivers( &self, _account: AccountId, diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 8ebbd478af..91bcd81339 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -37,11 +37,13 @@ use crate::{ pub mod input_selection; use input_selection::{GreedyInputSelector, GreedyInputSelectorError, InputSelector}; -use super::{NoteId, ShieldedProtocol}; +use super::{NoteId, SaplingInputSource, ShieldedProtocol}; #[cfg(feature = "transparent-inputs")] use { crate::wallet::WalletTransparentOutput, + super::TransparentInputSource, + std::convert::Infallible, zcash_primitives::{legacy::TransparentAddress, sapling::keys::OutgoingViewingKey}, }; @@ -210,7 +212,7 @@ pub fn create_spend_to_address( > where ParamsT: consensus::Parameters + Clone, - DbT: WalletWrite + WalletCommitmentTrees, + DbT: WalletWrite + WalletCommitmentTrees + SaplingInputSource::Error>, DbT::NoteRef: Copy + Eq + Ord, { let account = wallet_db @@ -313,10 +315,10 @@ pub fn spend( >, > where - DbT: WalletWrite + WalletCommitmentTrees, + DbT: WalletWrite + WalletCommitmentTrees + SaplingInputSource::Error>, DbT::NoteRef: Copy + Eq + Ord, ParamsT: consensus::Parameters + Clone, - InputsT: InputSelector, + InputsT: InputSelector, { let account = wallet_db .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) @@ -365,10 +367,10 @@ pub fn propose_transfer( >, > where - DbT: WalletWrite, + DbT: WalletRead + SaplingInputSource::Error>, DbT::NoteRef: Copy + Eq + Ord, ParamsT: consensus::Parameters + Clone, - InputsT: InputSelector, + InputsT: InputSelector, { use self::input_selection::InputSelectorError; let (target_height, anchor_height) = wallet_db @@ -430,7 +432,7 @@ pub fn propose_standard_transfer_to_address( > where ParamsT: consensus::Parameters + Clone, - DbT: WalletWrite, + DbT: WalletRead + SaplingInputSource::Error>, DbT::NoteRef: Copy + Eq + Ord, { let request = zip321::TransactionRequest::new(vec![Payment { @@ -470,7 +472,7 @@ pub fn propose_shielding( from_addrs: &[TransparentAddress], min_confirmations: NonZeroU32, ) -> Result< - Proposal, + Proposal, Error< ::Error, CommitmentTreeErrT, @@ -480,9 +482,8 @@ pub fn propose_shielding( > where ParamsT: consensus::Parameters, - DbT: WalletWrite, - DbT::NoteRef: Copy + Eq + Ord, - InputsT: InputSelector, + DbT: WalletRead + TransparentInputSource::Error>, + InputsT: InputSelector, { use self::input_selection::InputSelectorError; let (target_height, anchor_height) = wallet_db @@ -512,13 +513,13 @@ 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( +pub fn create_proposed_transaction( wallet_db: &mut DbT, params: &ParamsT, prover: impl SaplingProver, usk: &UnifiedSpendingKey, ovk_policy: OvkPolicy, - proposal: Proposal, + proposal: Proposal, min_confirmations: NonZeroU32, ) -> Result< TxId, @@ -531,7 +532,6 @@ pub fn create_proposed_transaction( > where DbT: WalletWrite + WalletCommitmentTrees, - DbT::NoteRef: Copy + Eq + Ord, ParamsT: consensus::Parameters + Clone, FeeRuleT: FeeRule, { diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index bdfe56bcd6..bc7d6502db 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -1,8 +1,7 @@ //! Types related to the process of selecting inputs to be spent given a transaction request. use core::marker::PhantomData; -use std::fmt; -use std::{collections::BTreeSet, fmt::Debug}; +use std::fmt::{self, Debug}; use zcash_primitives::{ consensus::{self, BlockHeight}, @@ -11,22 +10,27 @@ use zcash_primitives::{ components::{ amount::{BalanceError, NonNegativeAmount}, sapling::fees as sapling, - OutPoint, TxOut, + TxOut, }, fees::FeeRule, }, zip32::AccountId, }; -use crate::data_api::{SaplingInputSource, TransparentInputSource}; use crate::{ address::{RecipientAddress, UnifiedAddress}, - data_api::InputSource, + data_api::{SaplingInputSource, TransparentInputSource}, fees::{ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance}, wallet::{ReceivedSaplingNote, WalletTransparentOutput}, zip321::TransactionRequest, }; +#[cfg(feature = "transparent-inputs")] +use { + 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 { /// An error occurred accessing the underlying data store. @@ -153,12 +157,12 @@ impl Debug for Proposal { 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 - /// UTXOs. This associated type permits input selectors that may use specialized knowledge of + /// The type of data source that the input selector expects to access to obtain input Sapling + /// 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 /// not provide sufficiently fine-grained operations for a particular backing store to /// optimally perform input selection. - type DataSource: InputSource; + type InputSource; /// The type of the fee rule that this input selector uses when computing fees. type FeeRule: FeeRule; @@ -181,23 +185,18 @@ pub trait InputSelector { fn propose_transaction( &self, params: &ParamsT, - wallet_db: &Self::DataSource, + wallet_db: &Self::InputSource, target_height: BlockHeight, anchor_height: BlockHeight, account: AccountId, transaction_request: TransactionRequest, ) -> Result< - Proposal< - Self::FeeRule, - <::DataSource as SaplingInputSource>::NoteRef, - >, - InputSelectorError< - <::DataSource as InputSource>::Error, - Self::Error, - >, + Proposal::NoteRef>, + InputSelectorError<::Error, Self::Error>, > where - ParamsT: consensus::Parameters; + ParamsT: consensus::Parameters, + Self::InputSource: SaplingInputSource; /// Performs input selection and returns a proposal for the construction of a shielding /// transaction. @@ -207,28 +206,22 @@ 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)] + #[cfg(feature = "transparent-inputs")] fn propose_shielding( &self, params: &ParamsT, - wallet_db: &Self::DataSource, + wallet_db: &Self::InputSource, shielding_threshold: NonNegativeAmount, target_height: BlockHeight, latest_anchor: BlockHeight, source_addrs: &[TransparentAddress], ) -> Result< - Proposal< - Self::FeeRule, - <::DataSource as SaplingInputSource>::NoteRef, - >, - InputSelectorError< - <::DataSource as InputSource>::Error, - Self::Error, - >, + Proposal, + InputSelectorError<::Error, Self::Error>, > where - ParamsT: consensus::Parameters; + ParamsT: consensus::Parameters, + Self::InputSource: TransparentInputSource; } /// Errors that can occur as a consequence of greedy input selection. @@ -327,29 +320,30 @@ impl GreedyInputSelector { impl InputSelector for GreedyInputSelector where - DbT: InputSource, + DbT: TransparentInputSource + SaplingInputSource, ChangeT: ChangeStrategy, ChangeT::FeeRule: Clone, { type Error = GreedyInputSelectorError; - type DataSource = DbT; + type InputSource = DbT; type FeeRule = ChangeT::FeeRule; #[allow(clippy::type_complexity)] fn propose_transaction( &self, params: &ParamsT, - wallet_db: &Self::DataSource, + wallet_db: &Self::InputSource, target_height: BlockHeight, anchor_height: BlockHeight, account: AccountId, transaction_request: TransactionRequest, ) -> Result< Proposal, - InputSelectorError<::Error, Self::Error>, + InputSelectorError<::Error, Self::Error>, > where ParamsT: consensus::Parameters, + Self::InputSource: SaplingInputSource, { let mut transparent_outputs = vec![]; let mut sapling_outputs = vec![]; @@ -453,17 +447,18 @@ where } } + #[cfg(feature = "transparent-inputs")] #[allow(clippy::type_complexity)] fn propose_shielding( &self, params: &ParamsT, - wallet_db: &Self::DataSource, + wallet_db: &Self::InputSource, shielding_threshold: NonNegativeAmount, target_height: BlockHeight, latest_anchor: BlockHeight, source_addrs: &[TransparentAddress], ) -> Result< - Proposal, + Proposal, InputSelectorError<::Error, Self::Error>, > where diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index 2f72158d01..3f9f9ea5c1 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -112,7 +112,7 @@ pub enum ChangeError { DustInputs { /// The outpoints corresponding to transparent inputs having no current economic value. transparent: Vec, - /// The identifiers for Sapling inputs having not current economic value + /// The identifiers for Sapling inputs having no current economic value sapling: Vec, }, /// An error occurred that was specific to the change selection strategy in use.