From cc0cc2de8459ec83d4b8e672394533c5f8f8d9ca Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 12 Oct 2023 13:48:56 -0600 Subject: [PATCH] zcash_primitives: add StandardFeeRule `StandardFeeRule` is an enumeration of the standard fees that have existed in the history of Zcash zips. It is provided to simplify transition to new fee strategies; legacy elements of this enumeration are introduced already-deprecated. --- zcash_client_backend/CHANGELOG.md | 1 + zcash_client_backend/src/data_api/wallet.rs | 17 ++-- zcash_client_backend/src/fees.rs | 23 +++++ zcash_client_backend/src/fees/standard.rs | 101 ++++++++++++++++++++ zcash_client_sqlite/src/testing.rs | 20 ++-- zcash_client_sqlite/src/wallet/sapling.rs | 22 +++-- zcash_primitives/CHANGELOG.md | 2 + zcash_primitives/src/transaction/fees.rs | 40 ++++++++ 8 files changed, 196 insertions(+), 30 deletions(-) create mode 100644 zcash_client_backend/src/fees/standard.rs diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 9c1da9ada..013662a97 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -14,6 +14,7 @@ and this library adheres to Rust's notion of before it is fully supported. - `zcash_client_backend::data_api::error::Error` has new error variant: - `Error::UnsupportedPoolType(zcash_client_backend::data_api::PoolType)` +- Added module `zcash_client_backend::fees::standard` - Added methods to `zcash_client_backend::wallet::ReceivedSaplingNote`: `{from_parts, txid, output_index, diversifier, rseed, note_commitment_tree_position}`. diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 98454d355..4179c3cd9 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -1,7 +1,6 @@ -use std::{convert::Infallible, num::NonZeroU32}; +use std::num::NonZeroU32; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; -use zcash_primitives::transaction::TxId; use zcash_primitives::{ consensus::{self, BlockHeight, NetworkUpgrade}, memo::MemoBytes, @@ -13,9 +12,9 @@ use zcash_primitives::{ }, transaction::{ builder::Builder, - components::amount::{Amount, BalanceError, NonNegativeAmount}, - fees::{fixed, FeeRule}, - Transaction, + components::amount::{Amount, NonNegativeAmount}, + fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule}, + Transaction, TxId, }, zip32::{sapling::DiversifiableFullViewingKey, sapling::ExtendedSpendingKey, AccountId, Scope}, }; @@ -204,8 +203,8 @@ pub fn create_spend_to_address( Error< ::Error, ::Error, - GreedyInputSelectorError, - Infallible, + GreedyInputSelectorError, + Zip317FeeError, >, > where @@ -226,8 +225,8 @@ where ); #[allow(deprecated)] - let fee_rule = fixed::FeeRule::standard(); - let change_strategy = fees::fixed::SingleOutputChangeStrategy::new(fee_rule, change_memo); + let fee_rule = StandardFeeRule::PreZip313; + let change_strategy = fees::standard::SingleOutputChangeStrategy::new(fee_rule, change_memo); spend( wallet_db, params, diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index 5ddcffc1b..2f72158d0 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -15,6 +15,7 @@ use zcash_primitives::{ }; pub mod fixed; +pub mod standard; pub mod zip317; /// A proposed change amount and output pool. @@ -118,6 +119,28 @@ pub enum ChangeError { StrategyError(E), } +impl ChangeError { + pub fn map E0>(self, f: F) -> ChangeError { + match self { + ChangeError::InsufficientFunds { + available, + required, + } => ChangeError::InsufficientFunds { + available, + required, + }, + ChangeError::DustInputs { + transparent, + sapling, + } => ChangeError::DustInputs { + transparent, + sapling, + }, + ChangeError::StrategyError(e) => ChangeError::StrategyError(f(e)), + } + } +} + impl fmt::Display for ChangeError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match &self { diff --git a/zcash_client_backend/src/fees/standard.rs b/zcash_client_backend/src/fees/standard.rs new file mode 100644 index 000000000..c0443cd09 --- /dev/null +++ b/zcash_client_backend/src/fees/standard.rs @@ -0,0 +1,101 @@ +//! Change strategies designed for use with a standard fee. + +use zcash_primitives::{ + consensus::{self, BlockHeight}, + memo::MemoBytes, + transaction::{ + components::{ + amount::NonNegativeAmount, sapling::fees as sapling, transparent::fees as transparent, + }, + fees::{ + fixed::FeeRule as FixedFeeRule, + zip317::{FeeError as Zip317FeeError, FeeRule as Zip317FeeRule}, + StandardFeeRule, + }, + }, +}; + +use super::{fixed, zip317, ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance}; + +/// A change strategy that and proposes change as a single output to the most current supported +/// shielded pool and delegates fee calculation to the provided fee rule. +pub struct SingleOutputChangeStrategy { + fee_rule: StandardFeeRule, + change_memo: Option, +} + +impl SingleOutputChangeStrategy { + /// Constructs a new [`SingleOutputChangeStrategy`] with the specified ZIP 317 + /// fee parameters. + pub fn new(fee_rule: StandardFeeRule, change_memo: Option) -> Self { + Self { + fee_rule, + change_memo, + } + } +} + +impl ChangeStrategy for SingleOutputChangeStrategy { + type FeeRule = StandardFeeRule; + type Error = Zip317FeeError; + + fn fee_rule(&self) -> &Self::FeeRule { + &self.fee_rule + } + + fn compute_balance( + &self, + params: &P, + target_height: BlockHeight, + transparent_inputs: &[impl transparent::InputView], + transparent_outputs: &[impl transparent::OutputView], + sapling_inputs: &[impl sapling::InputView], + sapling_outputs: &[impl sapling::OutputView], + dust_output_policy: &DustOutputPolicy, + ) -> Result> { + #[allow(deprecated)] + match self.fee_rule() { + StandardFeeRule::PreZip313 => fixed::SingleOutputChangeStrategy::new( + FixedFeeRule::non_standard(NonNegativeAmount::const_from_u64(10000)), + self.change_memo.clone(), + ) + .compute_balance( + params, + target_height, + transparent_inputs, + transparent_outputs, + sapling_inputs, + sapling_outputs, + dust_output_policy, + ) + .map_err(|e| e.map(Zip317FeeError::Balance)), + StandardFeeRule::Zip313 => fixed::SingleOutputChangeStrategy::new( + FixedFeeRule::non_standard(NonNegativeAmount::const_from_u64(1000)), + self.change_memo.clone(), + ) + .compute_balance( + params, + target_height, + transparent_inputs, + transparent_outputs, + sapling_inputs, + sapling_outputs, + dust_output_policy, + ) + .map_err(|e| e.map(Zip317FeeError::Balance)), + StandardFeeRule::Zip317 => zip317::SingleOutputChangeStrategy::new( + Zip317FeeRule::standard(), + self.change_memo.clone(), + ) + .compute_balance( + params, + target_height, + transparent_inputs, + transparent_outputs, + sapling_inputs, + sapling_outputs, + dust_output_policy, + ), + } + } +} diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index 49dd50a8d..06dc2a456 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -14,20 +14,18 @@ use tempfile::NamedTempFile; #[cfg(feature = "unstable")] use tempfile::TempDir; -use zcash_client_backend::data_api::chain::ScanSummary; -use zcash_client_backend::data_api::{AccountBalance, WalletRead}; #[allow(deprecated)] use zcash_client_backend::{ address::RecipientAddress, data_api::{ self, - chain::{scan_cached_blocks, BlockSource}, + chain::{scan_cached_blocks, BlockSource, ScanSummary}, wallet::{ create_proposed_transaction, create_spend_to_address, input_selection::{GreedyInputSelectorError, InputSelector, Proposal}, propose_transfer, spend, }, - AccountBirthday, WalletSummary, WalletWrite, + AccountBalance, AccountBirthday, WalletRead, WalletSummary, WalletWrite, }, keys::UnifiedSpendingKey, proto::compact_formats::{ @@ -48,8 +46,8 @@ use zcash_primitives::{ Note, Nullifier, PaymentAddress, }, transaction::{ - components::amount::{BalanceError, NonNegativeAmount}, - fees::FeeRule, + components::amount::NonNegativeAmount, + fees::{zip317::FeeError as Zip317FeeError, FeeRule}, Transaction, TxId, }, zip32::{sapling::DiversifiableFullViewingKey, DiversifierIndex}, @@ -442,8 +440,8 @@ impl TestState { data_api::error::Error< SqliteClientError, commitment_tree::Error, - GreedyInputSelectorError, - Infallible, + GreedyInputSelectorError, + Zip317FeeError, >, > { let params = self.network(); @@ -560,7 +558,7 @@ impl TestState { } /// Invokes [`create_proposed_transaction`] with the given arguments. - pub(crate) fn create_proposed_transaction( + pub(crate) fn create_proposed_transaction( &mut self, usk: &UnifiedSpendingKey, ovk_policy: OvkPolicy, @@ -571,7 +569,7 @@ impl TestState { data_api::error::Error< SqliteClientError, commitment_tree::Error, - Infallible, + InputsErrT, FeeRuleT::Error, >, > @@ -579,7 +577,7 @@ impl TestState { FeeRuleT: FeeRule, { let params = self.network(); - create_proposed_transaction::<_, _, Infallible, _>( + create_proposed_transaction( &mut self.db_data, ¶ms, test_prover(), diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index c41a91463..8a9c809a0 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -456,11 +456,12 @@ pub(crate) mod tests { PaymentAddress, }, transaction::{ - components::{ - amount::{BalanceError, NonNegativeAmount}, - Amount, + components::{amount::NonNegativeAmount, Amount}, + fees::{ + fixed::FeeRule as FixedFeeRule, + zip317::{FeeError as Zip317FeeError, FeeRule as Zip317FeeRule}, + StandardFeeRule, }, - fees::{fixed::FeeRule as FixedFeeRule, zip317::FeeRule as Zip317FeeRule}, Transaction, }, zip32::{sapling::ExtendedSpendingKey, Scope}, @@ -477,7 +478,7 @@ pub(crate) mod tests { WalletWrite, }, decrypt_transaction, - fees::{fixed, zip317, DustOutputPolicy}, + fees::{fixed, standard, zip317, DustOutputPolicy}, keys::UnifiedSpendingKey, wallet::OvkPolicy, zip321::{self, Payment, TransactionRequest}, @@ -547,10 +548,10 @@ pub(crate) mod tests { }]) .unwrap(); - let fee_rule = FixedFeeRule::standard(); + let fee_rule = StandardFeeRule::PreZip313; let change_memo = "Test change memo".parse::().unwrap(); let change_strategy = - fixed::SingleOutputChangeStrategy::new(fee_rule, Some(change_memo.clone().into())); + standard::SingleOutputChangeStrategy::new(fee_rule, Some(change_memo.clone().into())); let input_selector = &GreedyInputSelector::new(change_strategy, DustOutputPolicy::default()); let proposal_result = st.propose_transfer( @@ -561,7 +562,7 @@ pub(crate) mod tests { ); assert_matches!(proposal_result, Ok(_)); - let create_proposed_result = st.create_proposed_transaction( + let create_proposed_result = st.create_proposed_transaction::( &usk, OvkPolicy::Sender, proposal_result.unwrap(), @@ -655,6 +656,7 @@ pub(crate) mod tests { } #[test] + #[allow(deprecated)] fn create_to_address_fails_on_incorrect_usk() { let mut st = TestBuilder::new() .with_test_account(AccountBirthday::from_sapling_activation) @@ -998,8 +1000,8 @@ pub(crate) mod tests { Error< SqliteClientError, commitment_tree::Error, - GreedyInputSelectorError, - Infallible, + GreedyInputSelectorError, + Zip317FeeError, >, > { let txid = st.create_spend_to_address( diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 4a4ff34fe..ea11f1e97 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -78,6 +78,8 @@ and this library adheres to Rust's notion of - All `const` values (moved to `zcash_primitives::sapling::constants`). - `impl From for u64` +### Added +- `transaction::fees::StandardFeeRule` ## [0.13.0] - 2023-09-25 ### Added - `zcash_primitives::consensus::BlockHeight::saturating_sub` diff --git a/zcash_primitives/src/transaction/fees.rs b/zcash_primitives/src/transaction/fees.rs index 9ca89b9c6..a2c7a4501 100644 --- a/zcash_primitives/src/transaction/fees.rs +++ b/zcash_primitives/src/transaction/fees.rs @@ -56,3 +56,43 @@ pub trait FutureFeeRule: FeeRule { tze_outputs: &[impl tze::OutputView], ) -> Result; } + +/// An enumeration of the standard fee rules supported by the wallet. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum StandardFeeRule { + #[deprecated(note = "It is recommended to use `StandardFeeRule::Zip317` instead.")] + PreZip313, + #[deprecated(note = "It is recommended to use `StandardFeeRule::Zip317` instead.")] + Zip313, + Zip317, +} + +impl FeeRule for StandardFeeRule { + type Error = zip317::FeeError; + + fn fee_required( + &self, + params: &P, + target_height: BlockHeight, + transparent_inputs: &[impl transparent::InputView], + transparent_outputs: &[impl transparent::OutputView], + sapling_input_count: usize, + sapling_output_count: usize, + orchard_action_count: usize, + ) -> Result { + #[allow(deprecated)] + match self { + Self::PreZip313 => Ok(zip317::MINIMUM_FEE), + Self::Zip313 => Ok(NonNegativeAmount::const_from_u64(1000)), + Self::Zip317 => zip317::FeeRule::standard().fee_required( + params, + target_height, + transparent_inputs, + transparent_outputs, + sapling_input_count, + sapling_output_count, + orchard_action_count, + ), + } + } +}