Skip to content

Commit

Permalink
WIP: eliminate input selection trait inheritance
Browse files Browse the repository at this point in the history
  • Loading branch information
nuttycom committed Nov 1, 2023
1 parent c3adebf commit 98b14b3
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 83 deletions.
36 changes: 4 additions & 32 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,19 +245,12 @@ pub trait TransparentInputSource {
) -> Result<Vec<WalletTransparentOutput>, Self::Error>;
}

pub trait InputSource:
SaplingInputSource<Error = <Self as InputSource>::Error>
+ TransparentInputSource<Error = <Self as InputSource>::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 = <Self as WalletRead>::Error> {
pub trait WalletRead {
type Error;

/// Returns the height of the chain as known to the wallet as of the most recent call to
Expand Down Expand Up @@ -405,14 +398,6 @@ pub trait WalletRead: InputSource<Error = <Self as WalletRead>::Error> {
query: NullifierQuery,
) -> Result<Vec<(AccountId, sapling::Nullifier)>, <Self as WalletRead>::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 as WalletRead>::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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1052,10 +1037,6 @@ pub mod testing {
}
}

impl InputSource for MockWalletDb {
type Error = ();
}

impl WalletRead for MockWalletDb {
type Error = ();

Expand Down Expand Up @@ -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<Vec<ReceivedSaplingNote<Self::NoteRef>>, <Self as WalletRead>::Error> {
Ok(Vec::new())
}

fn get_transparent_receivers(
&self,
_account: AccountId,
Expand Down
28 changes: 14 additions & 14 deletions zcash_client_backend/src/data_api/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};

Expand Down Expand Up @@ -210,7 +212,7 @@ 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
Expand Down Expand Up @@ -313,10 +315,10 @@ 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())
Expand Down Expand Up @@ -365,10 +367,10 @@ pub fn propose_transfer<DbT, ParamsT, InputsT, CommitmentTreeErrT>(
>,
>
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>,
{
use self::input_selection::InputSelectorError;
let (target_height, anchor_height) = wallet_db
Expand Down Expand Up @@ -430,7 +432,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 {
Expand Down Expand Up @@ -470,7 +472,7 @@ pub fn propose_shielding<DbT, ParamsT, InputsT, CommitmentTreeErrT>(
from_addrs: &[TransparentAddress],
min_confirmations: NonZeroU32,
) -> Result<
Proposal<InputsT::FeeRule, DbT::NoteRef>,
Proposal<InputsT::FeeRule, Infallible>,
Error<
<DbT as WalletRead>::Error,
CommitmentTreeErrT,
Expand All @@ -480,9 +482,8 @@ pub fn propose_shielding<DbT, ParamsT, InputsT, CommitmentTreeErrT>(
>
where
ParamsT: consensus::Parameters,
DbT: WalletWrite,
DbT::NoteRef: Copy + Eq + Ord,
InputsT: InputSelector<DataSource = DbT>,
DbT: WalletRead + TransparentInputSource<Error = <DbT as WalletRead>::Error>,
InputsT: InputSelector<InputSource = DbT>,
{
use self::input_selection::InputSelectorError;
let (target_height, anchor_height) = wallet_db
Expand Down Expand Up @@ -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<DbT, ParamsT, InputsErrT, FeeRuleT>(
pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT, N>(
wallet_db: &mut DbT,
params: &ParamsT,
prover: impl SaplingProver,
usk: &UnifiedSpendingKey,
ovk_policy: OvkPolicy,
proposal: Proposal<FeeRuleT, DbT::NoteRef>,
proposal: Proposal<FeeRuleT, N>,
min_confirmations: NonZeroU32,
) -> Result<
TxId,
Expand All @@ -531,7 +532,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,
{
Expand Down
67 changes: 31 additions & 36 deletions zcash_client_backend/src/data_api/wallet/input_selection.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand All @@ -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<DbErrT, SelectorErrT> {
/// An error occurred accessing the underlying data store.
Expand Down Expand Up @@ -153,12 +157,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
/// 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;

Expand All @@ -181,23 +185,18 @@ pub trait InputSelector {
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,
) -> Result<
Proposal<
Self::FeeRule,
<<Self as InputSelector>::DataSource as SaplingInputSource>::NoteRef,
>,
InputSelectorError<
<<Self as InputSelector>::DataSource as InputSource>::Error,
Self::Error,
>,
Proposal<Self::FeeRule, <Self::InputSource as SaplingInputSource>::NoteRef>,
InputSelectorError<<Self::InputSource as SaplingInputSource>::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.
Expand All @@ -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<ParamsT>(
&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,
<<Self as InputSelector>::DataSource as SaplingInputSource>::NoteRef,
>,
InputSelectorError<
<<Self as InputSelector>::DataSource as InputSource>::Error,
Self::Error,
>,
Proposal<Self::FeeRule, Infallible>,
InputSelectorError<<Self::InputSource as TransparentInputSource>::Error, Self::Error>,
>
where
ParamsT: consensus::Parameters;
ParamsT: consensus::Parameters,
Self::InputSource: TransparentInputSource;
}

/// Errors that can occur as a consequence of greedy input selection.
Expand Down Expand Up @@ -327,29 +320,30 @@ impl<DbT, ChangeT: ChangeStrategy> GreedyInputSelector<DbT, ChangeT> {

impl<DbT, ChangeT> InputSelector for GreedyInputSelector<DbT, ChangeT>
where
DbT: InputSource,
DbT: TransparentInputSource + 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,
) -> Result<
Proposal<Self::FeeRule, DbT::NoteRef>,
InputSelectorError<<DbT as InputSource>::Error, Self::Error>,
InputSelectorError<<DbT as SaplingInputSource>::Error, Self::Error>,
>
where
ParamsT: consensus::Parameters,
Self::InputSource: SaplingInputSource,
{
let mut transparent_outputs = vec![];
let mut sapling_outputs = vec![];
Expand Down Expand Up @@ -453,17 +447,18 @@ where
}
}

#[cfg(feature = "transparent-inputs")]
#[allow(clippy::type_complexity)]
fn propose_shielding<ParamsT>(
&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, DbT::NoteRef>,
Proposal<Self::FeeRule, Infallible>,
InputSelectorError<<DbT as TransparentInputSource>::Error, Self::Error>,
>
where
Expand Down
2 changes: 1 addition & 1 deletion zcash_client_backend/src/fees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 98b14b3

Please sign in to comment.