From 3fe1cdb9a36f0fd9543268c561b29a9719f97d95 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 17 Oct 2024 11:55:47 -0600 Subject: [PATCH] WIP: zcash_client_backend: Allow change strategies to act based on wallet balance. --- zcash_client_backend/src/data_api.rs | 57 ++++++++++- zcash_client_backend/src/fees.rs | 60 ++++++++---- zcash_client_backend/src/fees/zip317.rs | 12 ++- zcash_client_sqlite/src/error.rs | 4 + zcash_client_sqlite/src/lib.rs | 35 ++++--- zcash_client_sqlite/src/wallet/common.rs | 119 ++++++++++++++++------- 6 files changed, 212 insertions(+), 75 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index d56465413c..dcddcc654a 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -804,20 +804,28 @@ impl SpendableNotes { /// the wallet. pub struct WalletMeta { sapling_note_count: usize, + sapling_total_value: NonNegativeAmount, #[cfg(feature = "orchard")] orchard_note_count: usize, + #[cfg(feature = "orchard")] + orchard_total_value: NonNegativeAmount, } impl WalletMeta { /// Constructs a new [`WalletMeta`] value from its constituent parts. pub fn new( sapling_note_count: usize, + sapling_total_value: NonNegativeAmount, #[cfg(feature = "orchard")] orchard_note_count: usize, + #[cfg(feature = "orchard")] orchard_total_value: NonNegativeAmount, ) -> Self { Self { sapling_note_count, + sapling_total_value, #[cfg(feature = "orchard")] orchard_note_count, + #[cfg(feature = "orchard")] + orchard_total_value, } } @@ -838,6 +846,11 @@ impl WalletMeta { self.sapling_note_count } + /// Returns the total value of Sapling notes represented by [`Self::sapling_note_count`]. + pub fn sapling_total_value(&self) -> NonNegativeAmount { + self.sapling_total_value + } + /// Returns the number of unspent Orchard notes belonging to the account for which this was /// generated. #[cfg(feature = "orchard")] @@ -845,11 +858,48 @@ impl WalletMeta { self.orchard_note_count } + /// Returns the total value of Orchard notes represented by [`Self::orchard_note_count`]. + #[cfg(feature = "orchard")] + pub fn orchard_total_value(&self) -> NonNegativeAmount { + self.orchard_total_value + } + /// 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) } + + /// Returns the total value of shielded notes represented by [`Self::total_note_count`] + pub fn total_value(&self) -> NonNegativeAmount { + #[cfg(feature = "orchard")] + let orchard_value = self.orchard_total_value; + #[cfg(not(feature = "orchard"))] + let orchard_value = NonNegativeAmount::ZERO; + + (self.sapling_total_value + orchard_value).expect("Does not overflow Zcash maximum value.") + } +} + +/// A small query language for filtering notes belonging to an account. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum NoteSelector { + /// Selects notes having value greater than or equal to the provided value. + MinValue(NonNegativeAmount), + /// Selects notes having value greater than or equal to the n'th percentile of previously sent + /// notes in the wallet. The wrapped value must be in the range `1..=100` + PriorSendPercentile(u8), + /// Selects notes having value greater than or equal to the specified percentage of the wallet + /// balance. The wrapped value must be in the range `1..=100` + BalancePercentage(u8), + /// A note will be selected if it satisfies the first condition; if it is not possible to + /// evaaluate that condition (for example, [`NoteSelector::PriorSendPercentile`] cannot be + /// evaluated if no sends have been performed) then the second condition will be used for + /// evaluation. + Try { + condition: Box, + fallback: Box, + }, } /// A trait representing the capability to query a data store for unspent transaction outputs @@ -900,12 +950,15 @@ pub trait InputSource { /// /// The returned metadata value must exclude: /// - spent notes; - /// - unspent notes having value less than the specified minimum value; + /// - unspent notes excluded by the provided selector; /// - unspent notes identified in the given `exclude` list. + /// + /// Implementations of this method may limit the complexity of supported queries. Such + /// limitations should be clearly documented for the implementing type. fn get_wallet_metadata( &self, account: Self::AccountId, - min_value: NonNegativeAmount, + selector: &NoteSelector, exclude: &[Self::NoteRef], ) -> Result; diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index a10136db51..31e4537c03 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -16,7 +16,7 @@ use zcash_primitives::{ }; use zcash_protocol::{PoolType, ShieldedProtocol}; -use crate::data_api::InputSource; +use crate::data_api::{InputSource, Ratio}; pub mod common; #[cfg(feature = "non-standard-fees")] @@ -355,14 +355,14 @@ impl Default for DustOutputPolicy { #[derive(Clone, Copy, Debug)] pub struct SplitPolicy { target_output_count: NonZeroUsize, - min_split_output_size: NonNegativeAmount, + min_split_output_size: Option, } impl SplitPolicy { /// Constructs a new [`SplitPolicy`] from its constituent parts. pub fn new( target_output_count: NonZeroUsize, - min_split_output_size: NonNegativeAmount, + min_split_output_size: Option, ) -> Self { Self { target_output_count, @@ -374,7 +374,7 @@ impl SplitPolicy { pub fn single_output() -> Self { Self { target_output_count: NonZeroUsize::MIN, - min_split_output_size: NonNegativeAmount::ZERO, + min_split_output_size: None, } } @@ -382,7 +382,7 @@ impl SplitPolicy { /// /// If splitting change would result in notes of value less than the minimum split output size, /// a smaller number of splits should be chosen. - pub fn min_split_output_size(&self) -> NonNegativeAmount { + pub fn min_split_output_size(&self) -> Option { self.min_split_output_size } @@ -391,27 +391,49 @@ impl SplitPolicy { pub fn split_count( &self, existing_notes: usize, + existing_notes_total: NonNegativeAmount, total_change: NonNegativeAmount, ) -> NonZeroUsize { + fn to_nonzero_u64(value: usize) -> NonZeroU64 { + NonZeroU64::new(u64::try_from(value).expect("usize fits into u64")) + .expect("NonZeroU64 input derived from NonZeroUsize") + } + let mut split_count = NonZeroUsize::new(usize::from(self.target_output_count).saturating_sub(existing_notes)) .unwrap_or(NonZeroUsize::MIN); - loop { - let per_output_change = total_change.div_with_remainder( - NonZeroU64::new( - u64::try_from(usize::from(split_count)).expect("usize fits into u64"), - ) - .unwrap(), - ); - if *per_output_change.quotient() >= self.min_split_output_size { - return split_count; - } else if let Some(new_count) = NonZeroUsize::new(usize::from(split_count) - 1) { - split_count = new_count; - } else { - // We always create at least one change output. - return NonZeroUsize::MIN; + let min_split_output_size = self.min_split_output_size.or_else(|| { + // If no minimum split output size is set, we choose the minimum split size to be a + // quarter of the average value of notes in the wallet after the transaction. + (existing_notes_total + total_change).map(|total| { + *total + .div_with_remainder(to_nonzero_u64( + usize::from(self.target_output_count).saturating_mul(4), + )) + .quotient() + }) + }); + + if let Some(min_split_output_size) = min_split_output_size { + loop { + let per_output_change = + total_change.div_with_remainder(to_nonzero_u64(usize::from(split_count))); + if *per_output_change.quotient() >= self.min_split_output_size { + return split_count; + } else if let Some(new_count) = NonZeroUsize::new(usize::from(split_count) - 1) { + split_count = new_count; + } else { + // We always create at least one change output. + return NonZeroUsize::MIN; + } } + } else { + // This is purely defensive; this case would only arise in the case that the addition + // of the existing notes with the total change overflows the maximum monetary amount. + // Since it's always safe to fall back to a single change value, this is better than a + // panic. + return NonZeroUsize::MIN; } } } diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs index 7d89897be6..45ce31006c 100644 --- a/zcash_client_backend/src/fees/zip317.rs +++ b/zcash_client_backend/src/fees/zip317.rs @@ -342,7 +342,7 @@ mod tests { { // spend a single Sapling note and produce 5 outputs - let balance = |existing_notes| { + let balance = |existing_notes, total| { change_strategy.compute_balance( &Network::TestNetwork, Network::TestNetwork @@ -365,14 +365,17 @@ mod tests { None, &WalletMeta::new( existing_notes, + total, #[cfg(feature = "orchard")] 0, + #[cfg(feature = "orchard")] + NonNegativeAmount::ZERO, ), ) }; assert_matches!( - balance(0), + balance(0, NonNegativeAmount::ZERO), Ok(balance) if balance.proposed_change() == [ ChangeValue::sapling(NonNegativeAmount::const_from_u64(129_4000), None), @@ -385,7 +388,7 @@ mod tests { ); assert_matches!( - balance(2), + balance(2, NonNegativeAmount::const_from_u64(100_0000)), Ok(balance) if balance.proposed_change() == [ ChangeValue::sapling(NonNegativeAmount::const_from_u64(216_0000), None), @@ -421,8 +424,11 @@ mod tests { None, &WalletMeta::new( 0, + NonNegativeAmount::ZERO, #[cfg(feature = "orchard")] 0, + #[cfg(feature = "orchard")] + NonNegativeAmount::ZERO, ), ); diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 741003b6ea..f08af7979d 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -121,6 +121,9 @@ pub enum SqliteClientError { /// An error occurred in computing wallet balance BalanceError(BalanceError), + /// A note selection query contained an invalid constant or was otherwise not supported. + NoteSelectorInvalid(NoteSelector), + /// The proposal cannot be constructed until transactions with previously reserved /// ephemeral address outputs have been mined. The parameters are the account id and /// the index that could not safely be reserved. @@ -187,6 +190,7 @@ impl fmt::Display for SqliteClientError { SqliteClientError::ChainHeightUnknown => write!(f, "Chain height unknown; please call `update_chain_tip`"), SqliteClientError::UnsupportedPoolType(t) => write!(f, "Pool type is not currently supported: {}", t), SqliteClientError::BalanceError(e) => write!(f, "Balance error: {}", e), + SqliteClientError::NoteSelectorInvalid(s) => write!(f, "Could not evaluate selection query: {:?}", s), #[cfg(feature = "transparent-inputs")] SqliteClientError::ReachedGapLimit(account_id, bad_index) => write!(f, "The proposal cannot be constructed until transactions with previously reserved ephemeral address outputs have been mined. \ diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index f110229273..226d038226 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -51,9 +51,10 @@ use zcash_client_backend::{ chain::{BlockSource, ChainState, CommitmentTreeRoot}, scanning::{ScanPriority, ScanRange}, Account, AccountBirthday, AccountPurpose, AccountSource, BlockMetadata, - DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SeedRelevance, - SentTransaction, SpendableNotes, TransactionDataRequest, WalletCommitmentTrees, WalletMeta, - WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + DecryptedTransaction, InputSource, NoteSelector, NullifierQuery, ScannedBlock, + SeedRelevance, SentTransaction, SpendableNotes, TransactionDataRequest, + WalletCommitmentTrees, WalletMeta, WalletRead, WalletSummary, WalletWrite, + SAPLING_SHARD_HEIGHT, }, keys::{ AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey, @@ -128,7 +129,7 @@ pub mod error; pub mod wallet; use wallet::{ commitment_tree::{self, put_shard_roots}, - common::count_outputs, + common::spendable_notes_meta, SubtreeProgressEstimator, }; @@ -351,35 +352,39 @@ impl, P: consensus::Parameters> InputSource for fn get_wallet_metadata( &self, account_id: Self::AccountId, - min_value: NonNegativeAmount, + selector: &NoteSelector, + exclude: &[Self::NoteRef], exclude: &[Self::NoteRef], ) -> Result { let chain_tip_height = wallet::chain_tip_height(self.conn.borrow())? .ok_or(SqliteClientError::ChainHeightUnknown)?; - let sapling_note_count = count_outputs( + let sapling_pool_meta = spendable_notes_meta( self.conn.borrow(), - account_id, - min_value, - exclude, ShieldedProtocol::Sapling, chain_tip_height, + account_id, + selector, + exclude, )?; #[cfg(feature = "orchard")] - let orchard_note_count = count_outputs( + let orchard_pool_meta = spendable_notes_meta( self.conn.borrow(), - account_id, - min_value, - exclude, ShieldedProtocol::Orchard, chain_tip_height, + account_id, + selector, + exclude, )?; Ok(WalletMeta::new( - sapling_note_count, + sapling_pool_meta.note_count, + sapling_pool_meta.total_value, + #[cfg(feature = "orchard")] + orchard_pool_meta.note_count, #[cfg(feature = "orchard")] - orchard_note_count, + orchard_pool_meta.total_value, )) } } diff --git a/zcash_client_sqlite/src/wallet/common.rs b/zcash_client_sqlite/src/wallet/common.rs index 378abbe1a1..b0ca750189 100644 --- a/zcash_client_sqlite/src/wallet/common.rs +++ b/zcash_client_sqlite/src/wallet/common.rs @@ -3,8 +3,8 @@ use rusqlite::{named_params, types::Value, Connection, Row}; use std::rc::Rc; -use zcash_client_backend::{wallet::ReceivedNote, ShieldedProtocol}; -use zcash_primitives::transaction::{components::amount::NonNegativeAmount, TxId}; +use zcash_client_backend::{data_api::NoteSelector, wallet::ReceivedNote, ShieldedProtocol}; +use zcash_primitives::transaction::{components::amount::NonNegativeAmount, fees::zip317, TxId}; use zcash_protocol::consensus::{self, BlockHeight}; use super::wallet_birthday; @@ -226,16 +226,83 @@ where .collect::>() } -pub(crate) fn count_outputs( +pub(crate) struct PoolMeta { + pub(crate) note_count: usize, + pub(crate) total_value: NonNegativeAmount, +} + +pub(crate) fn spendable_notes_meta( conn: &rusqlite::Connection, - account: AccountId, - min_value: NonNegativeAmount, - exclude: &[ReceivedNoteId], protocol: ShieldedProtocol, chain_tip_height: BlockHeight, -) -> Result { + account: AccountId, + selector: &NoteSelector, + exclude: &[ReceivedNoteId], +) -> Result { let (table_prefix, _, _) = per_protocol_names(protocol); + let run_selection = |min_value| { + conn.query_row( + &format!( + "SELECT COUNT(*), SUM(rn.value) + FROM {table_prefix}_received_notes rn + INNER JOIN accounts ON accounts.id = rn.account_id + INNER JOIN transactions ON transactions.id_tx = rn.tx + WHERE rn.value >= :min_value + AND accounts.id = :account_id + AND accounts.ufvk IS NOT NULL + AND rn.recipient_key_scope IS NOT NULL + AND transactions.mined_height IS NOT NULL + AND rn.id NOT IN rarray(:exclude) + AND rn.id NOT IN ( + SELECT {table_prefix}_received_note_id + FROM {table_prefix}_received_note_spends rns + JOIN transactions stx ON stx.id_tx = rns.transaction_id + WHERE stx.block IS NOT NULL -- the spending tx is mined + OR stx.expiry_height IS NULL -- the spending tx will not expire + OR stx.expiry_height > :chain_tip_height -- the spending tx is unexpired + )" + ), + named_params![ + ":account_id": account.0, + ":min_value": u64::from(min_value), + ":exclude": &excluded_ptr, + ":chain_tip_height": u32::from(chain_tip_height) + ], + |row| Ok((row.get::<_, usize>(0)?, row.get::<_, Option>(1)?)), + ) + }; + + fn min_note_value( + selector: &NoteSelector, + ) -> Result, SqliteClientError> { + match selector { + NoteSelector::MinValue(v) => Ok(Some(v)), + NoteSelector::PriorSendPercentile(n) => { + if *n < 1 || *n > 100 { + return Err(SqliteClientError::NoteSelectorInvalid(selector.clone())); + } + + todo!() + } + NoteSelector::BalancePercentage(n) => { + if *n < 1 || *n > 100 { + return Err(SqliteClientError::NoteSelectorInvalid(selector.clone())); + } + + Ok(run_selection(zip317::MARGINAL_FEE)? + .and_then(|(_, total)| (total * 100).checked_div(n))) + } + NoteSelector::Try { + condition, + fallback, + } => match min_value(condition)? { + Some(n) => Ok(Some(n)), + None => min_value(fallback), + }, + } + } + let excluded: Vec = exclude .iter() .filter_map(|ReceivedNoteId(p, n)| { @@ -248,33 +315,13 @@ pub(crate) fn count_outputs( .collect(); let excluded_ptr = Rc::new(excluded); - conn.query_row( - &format!( - "SELECT COUNT(*) - FROM {table_prefix}_received_notes rn - INNER JOIN accounts ON accounts.id = rn.account_id - INNER JOIN transactions ON transactions.id_tx = rn.tx - WHERE value >= :min_value - AND accounts.id = :account_id - AND accounts.ufvk IS NOT NULL - AND recipient_key_scope IS NOT NULL - AND transactions.mined_height IS NOT NULL - AND rn.id NOT IN rarray(:exclude) - AND rn.id NOT IN ( - SELECT {table_prefix}_received_note_id - FROM {table_prefix}_received_note_spends - JOIN transactions stx ON stx.id_tx = transaction_id - WHERE stx.block IS NOT NULL -- the spending tx is mined - OR stx.expiry_height IS NULL -- the spending tx will not expire - OR stx.expiry_height > :chain_tip_height -- the spending tx is unexpired - )" - ), - named_params![ - ":account_id": account.0, - ":min_value": u64::from(min_value), - ":exclude": &excluded_ptr, - ":chain_tip_height": u32::from(chain_tip_height) - ], - |row| row.get(0), - ) + let min_value = min_note_value(selector)?.unwrap_or(zip317::MARGINAL_FEE * 10); + let (note_count, total_value) = run_selection(min_value)?; + + Ok(PoolMeta { + note_count, + total_value: total_value.map_or(Ok(NonNegativeAmount::ZERO), |v| { + NonNegativeAmount::from_nonnegative_i64(v).map_err(SqliteClientError::BalanceError) + })?, + }) }