Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spending pool selection #1244

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ and this library adheres to Rust's notion of
changes related to `Orchard` below are introduced under this feature
flag.
- `zcash_client_backend::data_api`:
- `Account`
- `AccountBalance::with_orchard_balance_mut`
- `AccountBirthday::orchard_frontier`
- `AccountSources`
- `BlockMetadata::orchard_tree_size`
- `DecryptedTransaction::{new, tx(), orchard_outputs()}`
- `ScannedBlock::orchard`
Expand Down Expand Up @@ -48,11 +50,17 @@ and this library adheres to Rust's notion of
- Arguments to `BlockMetadata::from_parts` have changed.
- Arguments to `ScannedBlock::from_parts` have changed.
- Changes to the `WalletRead` trait:
- Added `get_orchard_nullifiers`
- Added `Account` associated type.
- Added `get_orchard_nullifiers` method.
- `get_account_for_ufvk` now returns an `Self::Account` instead of a bare
`AccountId`
- Added `get_seed_account` method.
- Changes to the `InputSource` trait:
- `select_spendable_notes` now takes its `target_value` argument as a
`NonNegativeAmount`. Also, the values of the returned map are also
`NonNegativeAmount`s instead of `Amount`s.
- `select_spendable_notes` has changed:
- It now takes its `target_value` argument as a `NonNegativeAmount`.
- Instead of an `AccountId`, it takes an `AccountSources` argument. The
separate `sources` argument has been removed.
- The values of the returned map are `NonNegativeAmount`s instead of `Amount`s.
- Fields of `DecryptedTransaction` are now private. Use `DecryptedTransaction::new`
and the newly provided accessors instead.
- Fields of `SentTransaction` are now private. Use `SentTransaction::new`
Expand All @@ -64,6 +72,10 @@ and this library adheres to Rust's notion of
- `fn put_orchard_subtree_roots`
- Added method `WalletRead::validate_seed`
- Removed `Error::AccountNotFound` variant.
- `wallet::input_selection::InputSelector::propose_transaction` now takes an
`AccountSources` rather than a bare `AccountId`.
- `wallet::{propose_transfer, propose_standard_transfer_to_address}` now
each take an `AccountSources` instead of a bare `AccountId`.
- `zcash_client_backend::decrypt`:
- Fields of `DecryptedOutput` are now private. Use `DecryptedOutput::new`
and the newly provided accessors instead.
Expand Down
154 changes: 144 additions & 10 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
use incrementalmerkletree::{frontier::Frontier, Retention};
use secrecy::SecretVec;
use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
use zcash_keys::keys::HdSeedFingerprint;

use self::{chain::CommitmentTreeRoot, scanning::ScanRange};
use crate::{
Expand Down Expand Up @@ -308,6 +309,119 @@
}
}

/// A set of capabilities that a client account must provide.
pub trait Account<AccountId: Copy> {
/// Returns the unique identifier for the account.
fn id(&self) -> AccountId;

/// Returns the UFVK that the wallet backend has stored for the account, if any.
fn ufvk(&self) -> Option<&UnifiedFullViewingKey>;

/// Returns the default sources of funds that are available to spending by the account.
///
/// This corresponds to the set of shielded pools for which the account's UFVK can
/// maintain balance.
fn default_sources(&self) -> Option<AccountSources<AccountId>> {
self.ufvk().map(|ufvk| {
#[cfg(not(feature = "orchard"))]
let use_orchard = false;
#[cfg(feature = "orchard")]
let use_orchard = ufvk.orchard().is_some();

let use_sapling = ufvk.sapling().is_some();

AccountSources {
account_id: self.id(),
use_orchard,
use_sapling,
use_transparent: false,
}
})
}
}

impl<A: Copy> Account<A> for (A, UnifiedFullViewingKey) {
fn id(&self) -> A {
self.0

Check warning on line 345 in zcash_client_backend/src/data_api.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api.rs#L344-L345

Added lines #L344 - L345 were not covered by tests
}

fn ufvk(&self) -> Option<&UnifiedFullViewingKey> {
Some(&self.1)

Check warning on line 349 in zcash_client_backend/src/data_api.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api.rs#L348-L349

Added lines #L348 - L349 were not covered by tests
}
}

impl<A: Copy> Account<A> for (A, Option<UnifiedFullViewingKey>) {
fn id(&self) -> A {
self.0
}

fn ufvk(&self) -> Option<&UnifiedFullViewingKey> {
self.1.as_ref()
}
}

/// A type that describes what FVK components are known to the wallet for an account.
#[derive(Clone, Copy, Debug)]
pub struct AccountSources<AccountId> {
account_id: AccountId,
use_orchard: bool,
use_sapling: bool,
use_transparent: bool,
}

impl<AccountId: Copy> AccountSources<AccountId> {
/// Constructs AccountSources from its constituent parts
pub fn new(
account_id: AccountId,
use_orchard: bool,
use_sapling: bool,
use_transparent: bool,
) -> Self {
Self {
account_id,
use_orchard,
use_sapling,
use_transparent,
}
}

/// Returns the id for the account to which this metadata applies.
pub fn account_id(&self) -> AccountId {
self.account_id
}

/// Returns whether the account has an Orchard balance and spendability determination
/// capability.
pub fn use_orchard(&self) -> bool {
self.use_orchard

Check warning on line 396 in zcash_client_backend/src/data_api.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api.rs#L395-L396

Added lines #L395 - L396 were not covered by tests
}

/// Returns whether the account has an Sapling balance and spendability determination
/// capability.
pub fn use_sapling(&self) -> bool {
self.use_sapling
}

/// Returns whether the account has a Transparent balance determination capability.
pub fn use_transparent(&self) -> bool {
self.use_transparent

Check warning on line 407 in zcash_client_backend/src/data_api.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api.rs#L406-L407

Added lines #L406 - L407 were not covered by tests
}

/// Restricts the sources to be used to those for which the given [`UnifiedSpendingKey`]
/// provides a spending capability.
pub fn filter_with_usk(&mut self, usk: &UnifiedSpendingKey) {
self.use_orchard &= usk.has_orchard();
self.use_sapling &= usk.has_sapling();
self.use_transparent &= usk.has_transparent();

Check warning on line 415 in zcash_client_backend/src/data_api.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api.rs#L412-L415

Added lines #L412 - L415 were not covered by tests
}

/// Returns the [`UnifiedAddressRequest`] that will produce a [`UnifiedAddress`] having
/// receivers corresponding to the spending capabilities described by this value.
pub fn to_unified_address_request(&self) -> Option<UnifiedAddressRequest> {
UnifiedAddressRequest::new(self.use_orchard, self.use_sapling, self.use_transparent)
}
}

/// A polymorphic ratio type, usually used for rational numbers.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct Ratio<T> {
Expand Down Expand Up @@ -444,9 +558,8 @@
/// be included.
fn select_spendable_notes(
&self,
account: Self::AccountId,
inputs_from: AccountSources<Self::AccountId>,
target_value: NonNegativeAmount,
sources: &[ShieldedProtocol],
anchor_height: BlockHeight,
exclude: &[Self::NoteRef],
) -> Result<Vec<ReceivedNote<Self::NoteRef, Note>>, Self::Error>;
Expand Down Expand Up @@ -492,6 +605,9 @@
/// will be interpreted as belonging to that account.
type AccountId: Copy + Debug + Eq + Hash;

/// The concrete account type used by this wallet backend.
type Account: Account<Self::AccountId>;

/// Verifies that the given seed corresponds to the viewing key for the specified account.
///
/// Returns:
Expand Down Expand Up @@ -602,11 +718,19 @@
&self,
) -> Result<HashMap<Self::AccountId, UnifiedFullViewingKey>, Self::Error>;

/// Returns the account id corresponding to a given [`UnifiedFullViewingKey`], if any.
/// Returns the account corresponding to a given [`UnifiedFullViewingKey`], if any.
fn get_account_for_ufvk(
&self,
ufvk: &UnifiedFullViewingKey,
) -> Result<Option<Self::AccountId>, Self::Error>;
) -> Result<Option<Self::Account>, Self::Error>;

/// Returns the account corresponding to a given [`HdSeedFingerprint`] and
/// [`zip32::AccountId`], if any.
fn get_seed_account(
&self,
seed: &HdSeedFingerprint,
account_id: zip32::AccountId,
) -> Result<Option<Self::Account>, Self::Error>;

/// Returns the wallet balances and sync status for an account given the specified minimum
/// number of confirmations, or `Ok(None)` if the wallet has no balance data available.
Expand Down Expand Up @@ -1356,6 +1480,7 @@
use secrecy::{ExposeSecret, SecretVec};
use shardtree::{error::ShardTreeError, store::memory::MemoryShardStore, ShardTree};
use std::{collections::HashMap, convert::Infallible, num::NonZeroU32};
use zcash_keys::keys::HdSeedFingerprint;

use zcash_primitives::{
block::BlockHash,
Expand All @@ -1372,9 +1497,10 @@
};

use super::{
chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, BlockMetadata,
DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SentTransaction,
WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT,
chain::CommitmentTreeRoot, scanning::ScanRange, AccountBirthday, AccountSources,
BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock,
SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite,
SAPLING_SHARD_HEIGHT,
};

#[cfg(feature = "transparent-inputs")]
Expand Down Expand Up @@ -1425,9 +1551,8 @@

fn select_spendable_notes(
&self,
_account: Self::AccountId,
_inputs_from: AccountSources<Self::AccountId>,
_target_value: NonNegativeAmount,
_sources: &[ShieldedProtocol],
_anchor_height: BlockHeight,
_exclude: &[Self::NoteRef],
) -> Result<Vec<ReceivedNote<Self::NoteRef, Note>>, Self::Error> {
Expand All @@ -1438,6 +1563,7 @@
impl WalletRead for MockWalletDb {
type Error = ();
type AccountId = u32;
type Account = (Self::AccountId, UnifiedFullViewingKey);

fn validate_seed(
&self,
Expand Down Expand Up @@ -1523,7 +1649,15 @@
fn get_account_for_ufvk(
&self,
_ufvk: &UnifiedFullViewingKey,
) -> Result<Option<Self::AccountId>, Self::Error> {
) -> Result<Option<Self::Account>, Self::Error> {
Ok(None)

Check warning on line 1653 in zcash_client_backend/src/data_api.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api.rs#L1653

Added line #L1653 was not covered by tests
}

fn get_seed_account(

Check warning on line 1656 in zcash_client_backend/src/data_api.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api.rs#L1656

Added line #L1656 was not covered by tests
&self,
_seed: &HdSeedFingerprint,
_account_id: zip32::AccountId,
) -> Result<Option<Self::Account>, Self::Error> {
Ok(None)
}

Expand Down
9 changes: 9 additions & 0 deletions zcash_client_backend/src/data_api/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@
/// No account could be found corresponding to a provided spending key.
KeyNotRecognized,

/// No shielded source of funds was available for an account.
NoShieldedSources,

/// Zcash amount computation encountered an overflow or underflow.
BalanceError(BalanceError),

Expand Down Expand Up @@ -122,6 +125,12 @@
"Wallet does not contain an account corresponding to the provided spending key"
)
}
Error::NoShieldedSources => {
write!(
f,

Check warning on line 130 in zcash_client_backend/src/data_api/error.rs

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api/error.rs#L128-L130

Added lines #L128 - L130 were not covered by tests
"A wallet account contains no shielded sources of funds."
)
}
Error::BalanceError(e) => write!(
f,
"The value lies outside the valid range of Zcash amounts: {:?}.",
Expand Down
28 changes: 15 additions & 13 deletions zcash_client_backend/src/data_api/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ use sapling::{
};
use std::num::NonZeroU32;

use super::InputSource;
use super::{AccountSources, InputSource};
use crate::{
address::Address,
data_api::{
error::Error, SentTransaction, SentTransactionOutput, WalletCommitmentTrees, WalletRead,
WalletWrite,
error::Error, Account, SentTransaction, SentTransactionOutput, WalletCommitmentTrees,
WalletRead, WalletWrite,
},
decrypt_transaction,
fees::{self, DustOutputPolicy},
Expand Down Expand Up @@ -269,7 +269,7 @@ where
wallet_db,
params,
StandardFeeRule::PreZip313,
account,
account.default_sources().ok_or(Error::NoShieldedSources)?,
min_confirmations,
to,
amount,
Expand Down Expand Up @@ -380,7 +380,7 @@ where
let proposal = propose_transfer(
wallet_db,
params,
account,
account.default_sources().ok_or(Error::NoShieldedSources)?,
input_selector,
request,
min_confirmations,
Expand All @@ -405,7 +405,7 @@ where
pub fn propose_transfer<DbT, ParamsT, InputsT, CommitmentTreeErrT>(
wallet_db: &mut DbT,
params: &ParamsT,
spend_from_account: <DbT as InputSource>::AccountId,
sources: AccountSources<<DbT as InputSource>::AccountId>,
input_selector: &InputsT,
request: zip321::TransactionRequest,
min_confirmations: NonZeroU32,
Expand Down Expand Up @@ -435,7 +435,7 @@ where
wallet_db,
target_height,
anchor_height,
spend_from_account,
sources,
request,
)
.map_err(Error::from)
Expand All @@ -453,9 +453,10 @@ where
/// * `wallet_db`: A read/write reference to the wallet database.
/// * `params`: Consensus parameters.
/// * `fee_rule`: The fee rule to use in creating the transaction.
/// * `spend_from_account`: The unified account that controls the funds that will be spent
/// in the resulting transaction. This procedure will return an error if the
/// account ID does not correspond to an account known to the wallet.
/// * `sources`: Metadata that describes the unified account and the pools from which
/// funds may be spent in the resulting transaction. This procedure will return an
/// error if the contained account ID does not correspond to an account known to
/// the wallet.
/// * `min_confirmations`: The minimum number of confirmations that a previously
/// received note must have in the blockchain in order to be considered for being
/// spent. A value of 10 confirmations is recommended and 0-conf transactions are
Expand All @@ -472,7 +473,7 @@ pub fn propose_standard_transfer_to_address<DbT, ParamsT, CommitmentTreeErrT>(
wallet_db: &mut DbT,
params: &ParamsT,
fee_rule: StandardFeeRule,
spend_from_account: <DbT as InputSource>::AccountId,
sources: AccountSources<<DbT as InputSource>::AccountId>,
min_confirmations: NonZeroU32,
to: &Address,
amount: NonNegativeAmount,
Expand Down Expand Up @@ -520,7 +521,7 @@ where
propose_transfer(
wallet_db,
params,
spend_from_account,
sources,
&input_selector,
request,
min_confirmations,
Expand Down Expand Up @@ -694,7 +695,8 @@ where
let account = wallet_db
.get_account_for_ufvk(&usk.to_unified_full_viewing_key())
.map_err(Error::DataSource)?
.ok_or(Error::KeyNotRecognized)?;
.ok_or(Error::KeyNotRecognized)?
.id();

let (sapling_anchor, sapling_inputs) =
if proposal_step.involves(PoolType::Shielded(ShieldedProtocol::Sapling)) {
Expand Down
Loading
Loading