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

zcash_client_backend: Implement note management via change splitting. #1579

Merged
merged 7 commits into from
Oct 24, 2024
8 changes: 6 additions & 2 deletions components/zcash_protocol/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this library adheres to Rust's notion of

## [Unreleased]

### Added
- `zcash_protocol::value::QuotRem`
- `zcash_protocol::value::Zatoshis::div_with_remainder`

### Changed
- MSRV is now 1.77.0.

Expand Down Expand Up @@ -80,9 +84,9 @@ The entries below are relative to the `zcash_primitives` crate as of the tag
- `zcash_protocol::consensus::Parameters` has been split into two traits, with
the newly added `NetworkConstants` trait providing all network constant
accessors. Also, the `address_network` method has been replaced with a new
`network_type` method that serves the same purpose. A blanket impl of
`network_type` method that serves the same purpose. A blanket impl of
`NetworkConstants` is provided for all types that implement `Parameters`,
so call sites for methods that have moved to `NetworkConstants` should
so call sites for methods that have moved to `NetworkConstants` should
remain unchanged (though they may require an additional `use` statement.)

### Removed
Expand Down
28 changes: 28 additions & 0 deletions components/zcash_protocol/src/value.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::convert::{Infallible, TryFrom};
use std::error;
use std::iter::Sum;
use std::num::NonZeroU64;
use std::ops::{Add, Mul, Neg, Sub};

use memuse::DynamicUsage;
Expand Down Expand Up @@ -229,6 +230,24 @@ impl Mul<usize> for ZatBalance {
#[derive(Clone, Copy, Debug, PartialEq, PartialOrd, Eq, Ord)]
pub struct Zatoshis(u64);

/// A struct that provides both the quotient and remainder of a division operation.
pub struct QuotRem<A> {
quotient: A,
remainder: A,
}

impl<A> QuotRem<A> {
/// Returns the quotient portion of the value.
pub fn quotient(&self) -> &A {
&self.quotient
}

/// Returns the remainder portion of the value.
pub fn remainder(&self) -> &A {
&self.remainder
}
}

impl Zatoshis {
/// Returns the identity `Zatoshis`
pub const ZERO: Self = Zatoshis(0);
Expand Down Expand Up @@ -298,6 +317,15 @@ impl Zatoshis {
pub fn is_positive(&self) -> bool {
self > &Zatoshis::ZERO
}

/// Divides this `Zatoshis` value by the given divisor and returns the quotient and remainder.
pub fn div_with_remainder(&self, divisor: NonZeroU64) -> QuotRem<Zatoshis> {
let divisor = u64::from(divisor);
QuotRem {
quotient: Zatoshis(self.0 / divisor),
remainder: Zatoshis(self.0 % divisor),
}
}
}

impl From<Zatoshis> for ZatBalance {
Expand Down
54 changes: 53 additions & 1 deletion zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,54 @@ and this library adheres to Rust's notion of
- `zcash_client_backend::data_api`:
- `Progress`
- `WalletSummary::progress`
- `WalletMeta`
- `impl Default for wallet::input_selection::GreedyInputSelector`
- `zcash_client_backend::fees::SplitPolicy`
- `zcash_client_backend::fees::zip317::MultiOutputChangeStrategy`

### Changed
- `zcash_client_backend::data_api`:
- `InputSource` has an added method `get_wallet_metadata`
- `error::Error` has additional variant `Error::Change`. This necessitates
the addition of two type parameters to the `Error` type,
`ChangeErrT` and `NoteRefT`.
- The following methods each now take an additional `change_strategy`
argument, along with an associated `ChangeT` type parameter:
- `wallet::spend`
- `wallet::propose_transfer`
- `wallet::propose_shielding`. This method also now takes an additional
`to_account` argument.
- `wallet::shield_transparent_funds`. This method also now takes an
additional `to_account` argument.
- `wallet::input_selection::InputSelectionError` now has an additional `Change`
variant. This necessitates the addition of two type parameters.
- `wallet::input_selection::InputSelector::propose_transaction` takes an
additional `change_strategy` argument, along with an associated `ChangeT`
type parameter.
- The `wallet::input_selection::InputSelector::FeeRule` associated type has
been removed. The fee rule is now part of the change strategy passed to
`propose_transaction`.
- `wallet::input_selection::ShieldingSelector::propose_shielding` takes an
additional `change_strategy` argument, along with an associated `ChangeT`
type parameter. In addition, it also takes a new `to_account` argument
that identifies the destination account for the shielded notes.
- The `wallet::input_selection::ShieldingSelector::FeeRule` associated type
has been removed. The fee rule is now part of the change strategy passed to
`propose_shielding`.
- The `Change` variant of `wallet::input_selection::GreedyInputSelectorError`
has been removed, along with the additional type parameters it necessitated.
- The arguments to `wallet::input_selection::GreedyInputSelector::new` have
changed.
- `zcash_client_backend::fees`:
- `ChangeStrategy` has changed. It has two new associated types, `MetaSource`
and `WalletMeta`, and its `FeeRule` associated type now has an additional
`Clone` bound. In addition, it defines a new `fetch_wallet_meta` method, and
the arguments to `compute_balance` have changed.
- The following methods now take an additional `DustOutputPolicy` argument,
and carry an additional type parameter:
- `fixed::SingleOutputChangeStrategy::new`
- `standard::SingleOutputChangeStrategy::new`
- `zip317::SingleOutputChangeStrategy::new`

### Changed
- MSRV is now 1.77.0.
Expand All @@ -20,6 +68,10 @@ and this library adheres to Rust's notion of
- `zcash_client_backend::data_api`:
- `WalletSummary::scan_progress` and `WalletSummary::recovery_progress` have
been removed. Use `WalletSummary::progress` instead.
- `testing::input_selector` use explicit `InputSelector` constructors
directly instead.
- `zcash_client_backend::fees`:
- `impl From<BalanceError> for ChangeError<...>`

## [0.14.0] - 2024-10-04

Expand All @@ -28,7 +80,7 @@ and this library adheres to Rust's notion of
- `GAP_LIMIT`
- `WalletSummary::recovery_progress`
- `SpendableNotes::{take_sapling, take_orchard}`
- Tests and testing infrastructure have been migrated from the
- Tests and testing infrastructure have been migrated from the
`zcash_client_sqlite` internal tests to the `testing` module, and have been
generalized so that they may be used for testing arbitrary implementations
of the `zcash_client_backend::data_api` interfaces. The following have been
Expand Down
71 changes: 71 additions & 0 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,64 @@
}
}

/// Metadata about the structure of the wallet for a particular account.
///
/// At present this just contains counts of unspent outputs in each pool, but it may be extended in
/// the future to contain note values or other more detailed information about wallet structure.
///
/// Values of this type are intended to be used in selection of change output values. A value of
/// this type may represent filtered data, and may therefore not count all of the unspent notes in
/// the wallet.
pub struct WalletMeta {
sapling_note_count: usize,
#[cfg(feature = "orchard")]
daira marked this conversation as resolved.
Show resolved Hide resolved
orchard_note_count: usize,
}

impl WalletMeta {
nuttycom marked this conversation as resolved.
Show resolved Hide resolved
/// Constructs a new [`WalletMeta`] value from its constituent parts.
pub fn new(
sapling_note_count: usize,
#[cfg(feature = "orchard")] orchard_note_count: usize,
) -> Self {
Self {
sapling_note_count,
#[cfg(feature = "orchard")]
orchard_note_count,
}
}

/// Returns the number of unspent notes in the wallet for the given shielded protocol.
pub fn note_count(&self, protocol: ShieldedProtocol) -> usize {
match protocol {
ShieldedProtocol::Sapling => self.sapling_note_count,

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

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api.rs#L827

Added line #L827 was not covered by tests
#[cfg(feature = "orchard")]
ShieldedProtocol::Orchard => self.orchard_note_count,
#[cfg(not(feature = "orchard"))]
ShieldedProtocol::Orchard => 0,
}
}

/// Returns the number of unspent Sapling notes belonging to the account for which this was
/// generated.
pub fn sapling_note_count(&self) -> usize {
self.sapling_note_count

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

View check run for this annotation

Codecov / codecov/patch

zcash_client_backend/src/data_api.rs#L837-L838

Added lines #L837 - L838 were not covered by tests
}

/// Returns the number of unspent Orchard notes belonging to the account for which this was
/// generated.
#[cfg(feature = "orchard")]
pub fn orchard_note_count(&self) -> usize {
self.orchard_note_count
}

/// Returns the total number of unspent shielded notes belonging to the account for which this
/// was generated.
pub fn total_note_count(&self) -> usize {
self.sapling_note_count + self.note_count(ShieldedProtocol::Orchard)
}
}

/// A trait representing the capability to query a data store for unspent transaction outputs
/// belonging to a wallet.
#[cfg_attr(feature = "test-dependencies", delegatable_trait)]
Expand Down Expand Up @@ -838,6 +896,19 @@
exclude: &[Self::NoteRef],
) -> Result<SpendableNotes<Self::NoteRef>, Self::Error>;

/// Returns metadata describing the structure of the wallet for the specified account.
///
/// The returned metadata value must exclude:
/// - spent notes;
/// - unspent notes having value less than the specified minimum value;
/// - unspent notes identified in the given `exclude` list.
fn get_wallet_metadata(
&self,
account: Self::AccountId,
min_value: NonNegativeAmount,
exclude: &[Self::NoteRef],
) -> Result<WalletMeta, Self::Error>;

/// Fetches the transparent output corresponding to the provided `outpoint`.
///
/// Returns `Ok(None)` if the UTXO is not known to belong to the wallet or is not
Expand Down
45 changes: 30 additions & 15 deletions zcash_client_backend/src/data_api/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use crate::address::UnifiedAddress;
use crate::data_api::wallet::input_selection::InputSelectorError;
use crate::fees::ChangeError;
use crate::proposal::ProposalError;
use crate::PoolType;

Expand All @@ -23,7 +24,8 @@

/// Errors that can occur as a consequence of wallet operations.
#[derive(Debug)]
pub enum Error<DataSourceError, CommitmentTreeError, SelectionError, FeeError> {
pub enum Error<DataSourceError, CommitmentTreeError, SelectionError, FeeError, ChangeErrT, NoteRefT>
{
/// An error occurred retrieving data from the underlying data source
DataSource(DataSourceError),

Expand All @@ -33,6 +35,9 @@
/// An error in note selection
NoteSelection(SelectionError),

/// An error in change selection during transaction proposal construction
Change(ChangeError<ChangeErrT, NoteRefT>),
daira marked this conversation as resolved.
Show resolved Hide resolved

/// An error in transaction proposal construction
Proposal(ProposalError),

Expand Down Expand Up @@ -98,12 +103,14 @@
PaysEphemeralTransparentAddress(String),
}

impl<DE, CE, SE, FE> fmt::Display for Error<DE, CE, SE, FE>
impl<DE, TE, SE, FE, CE, N> fmt::Display for Error<DE, TE, SE, FE, CE, N>
where
DE: fmt::Display,
CE: fmt::Display,
TE: fmt::Display,
SE: fmt::Display,
FE: fmt::Display,
CE: fmt::Display,
N: fmt::Display,
{
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
use fmt::Write;
Expand All @@ -122,6 +129,9 @@
Error::NoteSelection(e) => {
write!(f, "Note selection encountered the following error: {}", e)
}
Error::Change(e) => {
write!(f, "Change output generation failed: {}", e)

Check warning on line 133 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#L132-L133

Added lines #L132 - L133 were not covered by tests
}
Error::Proposal(e) => {
write!(f, "Input selection attempted to construct an invalid proposal: {}", e)
}
Expand Down Expand Up @@ -178,12 +188,14 @@
}
}

impl<DE, CE, SE, FE> error::Error for Error<DE, CE, SE, FE>
impl<DE, TE, SE, FE, CE, N> error::Error for Error<DE, TE, SE, FE, CE, N>
where
DE: Debug + Display + error::Error + 'static,
CE: Debug + Display + error::Error + 'static,
TE: Debug + Display + error::Error + 'static,
SE: Debug + Display + error::Error + 'static,
FE: Debug + Display + 'static,
CE: Debug + Display + error::Error + 'static,
N: Debug + Display + 'static,
{
fn source(&self) -> Option<&(dyn error::Error + 'static)> {
match &self {
Expand All @@ -197,35 +209,38 @@
}
}

impl<DE, CE, SE, FE> From<builder::Error<FE>> for Error<DE, CE, SE, FE> {
impl<DE, TE, SE, FE, CE, N> From<builder::Error<FE>> for Error<DE, TE, SE, FE, CE, N> {
fn from(e: builder::Error<FE>) -> Self {
Error::Builder(e)
}
}

impl<DE, CE, SE, FE> From<ProposalError> for Error<DE, CE, SE, FE> {
impl<DE, TE, SE, FE, CE, N> From<ProposalError> for Error<DE, TE, SE, FE, CE, N> {
fn from(e: ProposalError) -> Self {
Error::Proposal(e)
}
}

impl<DE, CE, SE, FE> From<BalanceError> for Error<DE, CE, SE, FE> {
impl<DE, TE, SE, FE, CE, N> From<BalanceError> for Error<DE, TE, SE, FE, CE, N> {
fn from(e: BalanceError) -> Self {
Error::BalanceError(e)
}
}

impl<DE, CE, SE, FE> From<ConversionError<&'static str>> for Error<DE, CE, SE, FE> {
impl<DE, TE, SE, FE, CE, N> From<ConversionError<&'static str>> for Error<DE, TE, SE, FE, CE, N> {
fn from(value: ConversionError<&'static str>) -> Self {
Error::Address(value)
}
}

impl<DE, CE, SE, FE> From<InputSelectorError<DE, SE>> for Error<DE, CE, SE, FE> {
fn from(e: InputSelectorError<DE, SE>) -> Self {
impl<DE, TE, SE, FE, CE, N> From<InputSelectorError<DE, SE, CE, N>>
for Error<DE, TE, SE, FE, CE, N>
{
fn from(e: InputSelectorError<DE, SE, CE, N>) -> Self {
match e {
InputSelectorError::DataSource(e) => Error::DataSource(e),
InputSelectorError::Selection(e) => Error::NoteSelection(e),
InputSelectorError::Change(e) => Error::Change(e),

Check warning on line 243 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#L243

Added line #L243 was not covered by tests
InputSelectorError::Proposal(e) => Error::Proposal(e),
InputSelectorError::InsufficientFunds {
available,
Expand All @@ -240,20 +255,20 @@
}
}

impl<DE, CE, SE, FE> From<sapling::builder::Error> for Error<DE, CE, SE, FE> {
impl<DE, TE, SE, FE, CE, N> From<sapling::builder::Error> for Error<DE, TE, SE, FE, CE, N> {
fn from(e: sapling::builder::Error) -> Self {
Error::Builder(builder::Error::SaplingBuild(e))
}
}

impl<DE, CE, SE, FE> From<transparent::builder::Error> for Error<DE, CE, SE, FE> {
impl<DE, TE, SE, FE, CE, N> From<transparent::builder::Error> for Error<DE, TE, SE, FE, CE, N> {
fn from(e: transparent::builder::Error) -> Self {
Error::Builder(builder::Error::TransparentBuild(e))
}
}

impl<DE, CE, SE, FE> From<ShardTreeError<CE>> for Error<DE, CE, SE, FE> {
fn from(e: ShardTreeError<CE>) -> Self {
impl<DE, TE, SE, FE, CE, N> From<ShardTreeError<TE>> for Error<DE, TE, SE, FE, CE, N> {
fn from(e: ShardTreeError<TE>) -> Self {

Check warning on line 271 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#L271

Added line #L271 was not covered by tests
Error::CommitmentTree(e)
}
}
Loading
Loading