diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 9c1da9adac..013662a97b 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 98454d3555..c2de33e26d 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -1,7 +1,7 @@ -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 +13,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 +204,8 @@ pub fn create_spend_to_address( Error< ::Error, ::Error, - GreedyInputSelectorError, - Infallible, + GreedyInputSelectorError, + Zip317FeeError, >, > where @@ -213,29 +213,31 @@ where DbT: WalletWrite + WalletCommitmentTrees, DbT::NoteRef: Copy + Eq + Ord, { - let req = zip321::TransactionRequest::new(vec![Payment { - recipient_address: to.clone(), + let account = wallet_db + .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) + .map_err(Error::DataSource)? + .ok_or(Error::KeyNotRecognized)?; + + #[allow(deprecated)] + let proposal = propose_standard_transfer_to_address( + wallet_db, + params, + StandardFeeRule::PreZip313, + account, + min_confirmations, + to, amount, memo, - label: None, - message: None, - other_params: vec![], - }]) - .expect( - "It should not be possible for this to violate ZIP 321 request construction invariants.", - ); + change_memo, + )?; - #[allow(deprecated)] - let fee_rule = fixed::FeeRule::standard(); - let change_strategy = fees::fixed::SingleOutputChangeStrategy::new(fee_rule, change_memo); - spend( + create_proposed_transaction( wallet_db, params, prover, - &GreedyInputSelector::::new(change_strategy, DustOutputPolicy::default()), usk, - req, ovk_policy, + proposal, min_confirmations, ) } @@ -374,6 +376,77 @@ where .map_err(Error::from) } +/// Proposes a transaction paying the specified address from the given account. +/// +/// Returns the proposal, which may then be executed using [`create_proposed_transaction`] +/// +/// Parameters: +/// * `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. +/// * `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 +/// not supported. +/// * `to`: The address to which `amount` will be paid. +/// * `amount`: The amount to send. +/// * `memo`: A memo to be included in the output to the recipient. +/// * `change_memo`: A memo to be included in any change output that is created. +#[allow(clippy::too_many_arguments)] +#[allow(clippy::type_complexity)] +pub fn propose_standard_transfer_to_address( + wallet_db: &mut DbT, + params: &ParamsT, + fee_rule: StandardFeeRule, + spend_from_account: AccountId, + min_confirmations: NonZeroU32, + to: &RecipientAddress, + amount: NonNegativeAmount, + memo: Option, + change_memo: Option, +) -> Result< + Proposal, + Error< + DbT::Error, + CommitmentTreeErrT, + GreedyInputSelectorError, + Zip317FeeError, + >, +> +where + ParamsT: consensus::Parameters + Clone, + DbT: WalletWrite, + DbT::NoteRef: Copy + Eq + Ord, +{ + let request = zip321::TransactionRequest::new(vec![Payment { + recipient_address: to.clone(), + amount, + memo, + label: None, + message: None, + other_params: vec![], + }]) + .expect( + "It should not be possible for this to violate ZIP 321 request construction invariants.", + ); + + let change_strategy = fees::standard::SingleOutputChangeStrategy::new(fee_rule, change_memo); + let input_selector = + GreedyInputSelector::::new(change_strategy, DustOutputPolicy::default()); + + propose_transfer( + wallet_db, + params, + spend_from_account, + &input_selector, + request, + min_confirmations, + ) +} + #[cfg(feature = "transparent-inputs")] #[allow(clippy::too_many_arguments)] #[allow(clippy::type_complexity)] diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index 5ddcffc1bf..2f72158d01 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 0000000000..572bb814bd --- /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 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 49dd50a8db..c14b6d44f5 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -14,20 +14,21 @@ 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}; +use zcash_client_backend::fees::{standard, DustOutputPolicy}; #[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, + input_selection::{ + GreedyInputSelector, GreedyInputSelectorError, InputSelector, Proposal, + }, + propose_standard_transfer_to_address, propose_transfer, spend, }, - AccountBirthday, WalletSummary, WalletWrite, + AccountBalance, AccountBirthday, WalletRead, WalletSummary, WalletWrite, }, keys::UnifiedSpendingKey, proto::compact_formats::{ @@ -40,7 +41,7 @@ use zcash_note_encryption::Domain; use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight, Network, NetworkUpgrade, Parameters}, - memo::MemoBytes, + memo::{Memo, MemoBytes}, sapling::{ note_encryption::{sapling_note_encryption, SaplingDomain}, util::generate_random_rseed, @@ -48,8 +49,8 @@ use zcash_primitives::{ Note, Nullifier, PaymentAddress, }, transaction::{ - components::amount::{BalanceError, NonNegativeAmount}, - fees::FeeRule, + components::amount::NonNegativeAmount, + fees::{zip317::FeeError as Zip317FeeError, FeeRule, StandardFeeRule}, Transaction, TxId, }, zip32::{sapling::DiversifiableFullViewingKey, DiversifierIndex}, @@ -442,8 +443,8 @@ impl TestState { data_api::error::Error< SqliteClientError, commitment_tree::Error, - GreedyInputSelectorError, - Infallible, + GreedyInputSelectorError, + Zip317FeeError, >, > { let params = self.network(); @@ -526,6 +527,41 @@ impl TestState { ) } + /// Invokes [`propose_standard_transfer`] with the given arguments. + #[allow(clippy::type_complexity)] + #[allow(clippy::too_many_arguments)] + pub(crate) fn propose_standard_transfer( + &mut self, + spend_from_account: AccountId, + fee_rule: StandardFeeRule, + min_confirmations: NonZeroU32, + to: &RecipientAddress, + amount: NonNegativeAmount, + memo: Option, + change_memo: Option, + ) -> Result< + Proposal, + data_api::error::Error< + SqliteClientError, + CommitmentTreeErrT, + GreedyInputSelectorError, + Zip317FeeError, + >, + > { + let params = self.network(); + propose_standard_transfer_to_address::<_, _, CommitmentTreeErrT>( + &mut self.db_data, + ¶ms, + fee_rule, + spend_from_account, + min_confirmations, + to, + amount, + memo, + change_memo, + ) + } + /// Invokes [`propose_shielding`] with the given arguments. #[cfg(feature = "transparent-inputs")] #[allow(clippy::type_complexity)] @@ -560,7 +596,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 +607,7 @@ impl TestState { data_api::error::Error< SqliteClientError, commitment_tree::Error, - Infallible, + InputsErrT, FeeRuleT::Error, >, > @@ -579,7 +615,7 @@ impl TestState { FeeRuleT: FeeRule, { let params = self.network(); - create_proposed_transaction::<_, _, Infallible, _>( + create_proposed_transaction( &mut self.db_data, ¶ms, test_prover(), @@ -995,3 +1031,15 @@ impl TestCache for FsBlockCache { meta } } + +pub(crate) fn input_selector( + fee_rule: StandardFeeRule, + change_memo: Option<&str>, +) -> GreedyInputSelector< + WalletDb, + standard::SingleOutputChangeStrategy, +> { + let change_memo = change_memo.map(|m| MemoBytes::from(m.parse::().unwrap())); + let change_strategy = standard::SingleOutputChangeStrategy::new(fee_rule, change_memo); + GreedyInputSelector::new(change_strategy, DustOutputPolicy::default()) +} diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index c41a914633..c0cc7cf983 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -456,11 +456,10 @@ pub(crate) mod tests { PaymentAddress, }, transaction::{ - components::{ - amount::{BalanceError, NonNegativeAmount}, - Amount, + components::{amount::NonNegativeAmount, Amount}, + fees::{ + fixed::FeeRule as FixedFeeRule, zip317::FeeError as Zip317FeeError, StandardFeeRule, }, - fees::{fixed::FeeRule as FixedFeeRule, zip317::FeeRule as Zip317FeeRule}, Transaction, }, zip32::{sapling::ExtendedSpendingKey, Scope}, @@ -477,7 +476,7 @@ pub(crate) mod tests { WalletWrite, }, decrypt_transaction, - fees::{fixed, zip317, DustOutputPolicy}, + fees::{fixed, standard, DustOutputPolicy}, keys::UnifiedSpendingKey, wallet::OvkPolicy, zip321::{self, Payment, TransactionRequest}, @@ -485,7 +484,7 @@ pub(crate) mod tests { use crate::{ error::SqliteClientError, - testing::{AddressType, BlockCache, TestBuilder, TestState}, + testing::{input_selector, AddressType, BlockCache, TestBuilder, TestState}, wallet::{ block_max_scanned, commitment_tree, sapling::select_spendable_sapling_notes, scanning::tests::test_with_canopy_birthday, @@ -547,10 +546,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 +560,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 +654,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) @@ -687,7 +687,7 @@ pub(crate) mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let (_, usk, _) = st.test_account().unwrap(); + let (account, _, _) = st.test_account().unwrap(); let dfvk = st.test_account_sapling().unwrap(); let to = dfvk.default_address().1.into(); @@ -696,13 +696,13 @@ pub(crate) mod tests { // We cannot do anything if we aren't synchronised assert_matches!( - st.create_spend_to_address( - &usk, + st.propose_standard_transfer::( + account, + StandardFeeRule::PreZip313, + NonZeroU32::new(1).unwrap(), &to, NonNegativeAmount::const_from_u64(1), None, - OvkPolicy::Sender, - NonZeroU32::new(1).unwrap(), None ), Err(data_api::error::Error::ScanRequired) @@ -710,7 +710,7 @@ pub(crate) mod tests { } #[test] - fn create_to_address_fails_on_unverified_notes() { + fn spend_fails_on_unverified_notes() { let mut st = TestBuilder::new() .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) @@ -763,13 +763,13 @@ pub(crate) mod tests { let extsk2 = ExtendedSpendingKey::master(&[]); let to = extsk2.default_address().1.into(); assert_matches!( - st.create_spend_to_address( - &usk, + st.propose_standard_transfer::( + account, + StandardFeeRule::Zip317, + NonZeroU32::new(10).unwrap(), &to, NonNegativeAmount::const_from_u64(70000), None, - OvkPolicy::Sender, - NonZeroU32::new(10).unwrap(), None ), Err(data_api::error::Error::InsufficientFunds { @@ -792,13 +792,13 @@ pub(crate) mod tests { // Spend still fails assert_matches!( - st.create_spend_to_address( - &usk, + st.propose_standard_transfer::( + account, + StandardFeeRule::Zip317, + NonZeroU32::new(10).unwrap(), &to, NonNegativeAmount::const_from_u64(70000), None, - OvkPolicy::Sender, - NonZeroU32::new(10).unwrap(), None ), Err(data_api::error::Error::InsufficientFunds { @@ -822,20 +822,31 @@ pub(crate) mod tests { (value * 9).unwrap() ); - // Spend should now succeed + // Should now be able to generate a proposal let amount_sent = NonNegativeAmount::from_u64(70000).unwrap(); - let txid = st - .create_spend_to_address( - &usk, + let min_confirmations = NonZeroU32::new(10).unwrap(); + let proposal = st + .propose_standard_transfer::( + account, + StandardFeeRule::Zip317, + min_confirmations, &to, amount_sent, None, - OvkPolicy::Sender, - NonZeroU32::new(10).unwrap(), None, ) .unwrap(); + // Executing the proposal should succeed + let txid = st + .create_proposed_transaction::( + &usk, + OvkPolicy::Sender, + proposal, + min_confirmations, + ) + .unwrap(); + let (h, _) = st.generate_next_block_including(txid); st.scan_cached_blocks(h, 1); @@ -849,7 +860,7 @@ pub(crate) mod tests { } #[test] - fn create_to_address_fails_on_locked_notes() { + fn spend_fails_on_locked_notes() { let mut st = TestBuilder::new() .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) @@ -858,6 +869,11 @@ pub(crate) mod tests { let (account, usk, _) = st.test_account().unwrap(); let dfvk = st.test_account_sapling().unwrap(); + // TODO: This test was originally written to use the pre-zip-313 fee rule + // and has not yet been updated. + #[allow(deprecated)] + let fee_rule = StandardFeeRule::PreZip313; + // Add funds to the wallet in a single note let value = NonNegativeAmount::const_from_u64(50000); let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); @@ -870,28 +886,39 @@ pub(crate) mod tests { // Send some of the funds to another address, but don't mine the tx. let extsk2 = ExtendedSpendingKey::master(&[]); let to = extsk2.default_address().1.into(); - assert_matches!( - st.create_spend_to_address( - &usk, + let min_confirmations = NonZeroU32::new(1).unwrap(); + let proposal = st + .propose_standard_transfer::( + account, + fee_rule, + min_confirmations, &to, NonNegativeAmount::const_from_u64(15000), None, + None, + ) + .unwrap(); + + // Executing the proposal should succeed + assert_matches!( + st.create_proposed_transaction::( + &usk, OvkPolicy::Sender, - NonZeroU32::new(1).unwrap(), - None + proposal, + min_confirmations ), Ok(_) ); - // A second spend fails because there are no usable notes + // A second proposal fails because there are no usable notes assert_matches!( - st.create_spend_to_address( - &usk, + st.propose_standard_transfer::( + account, + fee_rule, + NonZeroU32::new(1).unwrap(), &to, NonNegativeAmount::const_from_u64(2000), None, - OvkPolicy::Sender, - NonZeroU32::new(1).unwrap(), None ), Err(data_api::error::Error::InsufficientFunds { @@ -912,15 +939,15 @@ pub(crate) mod tests { } st.scan_cached_blocks(h1 + 1, 41); - // Second spend still fails + // Second proposal still fails assert_matches!( - st.create_spend_to_address( - &usk, + st.propose_standard_transfer::( + account, + fee_rule, + NonZeroU32::new(1).unwrap(), &to, NonNegativeAmount::const_from_u64(2000), None, - OvkPolicy::Sender, - NonZeroU32::new(1).unwrap(), None ), Err(data_api::error::Error::InsufficientFunds { @@ -943,19 +970,29 @@ pub(crate) mod tests { assert_eq!(st.get_spendable_balance(account, 1), value); // Second spend should now succeed - let amount_sent2 = NonNegativeAmount::from_u64(2000).unwrap(); - let txid2 = st - .create_spend_to_address( - &usk, + let amount_sent2 = NonNegativeAmount::const_from_u64(2000); + let min_confirmations = NonZeroU32::new(1).unwrap(); + let proposal = st + .propose_standard_transfer::( + account, + fee_rule, + min_confirmations, &to, amount_sent2, None, - OvkPolicy::Sender, - NonZeroU32::new(1).unwrap(), None, ) .unwrap(); + let txid2 = st + .create_proposed_transaction::( + &usk, + OvkPolicy::Sender, + proposal, + min_confirmations, + ) + .unwrap(); + let (h, _) = st.generate_next_block_including(txid2); st.scan_cached_blocks(h, 1); @@ -990,6 +1027,11 @@ pub(crate) mod tests { let addr2 = extsk2.default_address().1; let to = addr2.into(); + // TODO: This test was originally written to use the pre-zip-313 fee rule + // and has not yet been updated. + #[allow(deprecated)] + let fee_rule = StandardFeeRule::PreZip313; + #[allow(clippy::type_complexity)] let send_and_recover_with_policy = |st: &mut TestState, ovk_policy| @@ -998,20 +1040,25 @@ pub(crate) mod tests { Error< SqliteClientError, commitment_tree::Error, - GreedyInputSelectorError, - Infallible, + GreedyInputSelectorError, + Zip317FeeError, >, > { - let txid = st.create_spend_to_address( - &usk, + let min_confirmations = NonZeroU32::new(1).unwrap(); + let proposal = st.propose_standard_transfer( + account, + fee_rule, + min_confirmations, &to, NonNegativeAmount::const_from_u64(15000), None, - ovk_policy, - NonZeroU32::new(1).unwrap(), None, )?; + // Executing the proposal should succeed + let txid = + st.create_proposed_transaction(&usk, ovk_policy, proposal, min_confirmations)?; + // Fetch the transaction from the database let raw_tx: Vec<_> = st .wallet() @@ -1069,7 +1116,7 @@ pub(crate) mod tests { } #[test] - fn create_to_address_succeeds_to_t_addr_zero_change() { + fn spend_succeeds_to_t_addr_zero_change() { let mut st = TestBuilder::new() .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) @@ -1087,24 +1134,40 @@ pub(crate) mod tests { assert_eq!(st.get_total_balance(account), value); assert_eq!(st.get_spendable_balance(account, 1), value); + // TODO: This test was originally written to use the pre-zip-313 fee rule + // and has not yet been updated. + #[allow(deprecated)] + let fee_rule = StandardFeeRule::PreZip313; + // TODO: generate_next_block_from_tx does not currently support transparent outputs. let to = TransparentAddress::PublicKey([7; 20]).into(); - assert_matches!( - st.create_spend_to_address( - &usk, + let min_confirmations = NonZeroU32::new(1).unwrap(); + let proposal = st + .propose_standard_transfer::( + account, + fee_rule, + min_confirmations, &to, NonNegativeAmount::const_from_u64(50000), None, + None, + ) + .unwrap(); + + // Executing the proposal should succeed + assert_matches!( + st.create_proposed_transaction::( + &usk, OvkPolicy::Sender, - NonZeroU32::new(1).unwrap(), - None + proposal, + min_confirmations ), Ok(_) ); } #[test] - fn create_to_address_spends_a_change_note() { + fn change_note_spends_succeed() { let mut st = TestBuilder::new() .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) @@ -1129,17 +1192,33 @@ pub(crate) mod tests { NonNegativeAmount::ZERO ); + // TODO: This test was originally written to use the pre-zip-313 fee rule + // and has not yet been updated. + #[allow(deprecated)] + let fee_rule = StandardFeeRule::PreZip313; + // TODO: generate_next_block_from_tx does not currently support transparent outputs. let to = TransparentAddress::PublicKey([7; 20]).into(); - assert_matches!( - st.create_spend_to_address( - &usk, + let min_confirmations = NonZeroU32::new(1).unwrap(); + let proposal = st + .propose_standard_transfer::( + account, + fee_rule, + min_confirmations, &to, NonNegativeAmount::const_from_u64(50000), None, + None, + ) + .unwrap(); + + // Executing the proposal should succeed + assert_matches!( + st.create_proposed_transaction::( + &usk, OvkPolicy::Sender, - NonZeroU32::new(1).unwrap(), - None + proposal, + min_confirmations ), Ok(_) ); @@ -1203,6 +1282,7 @@ pub(crate) mod tests { ]) .unwrap(); + #[allow(deprecated)] let fee_rule = FixedFeeRule::standard(); let input_selector = GreedyInputSelector::new( fixed::SingleOutputChangeStrategy::new(fee_rule, None), @@ -1296,10 +1376,7 @@ pub(crate) mod tests { assert_eq!(st.get_total_balance(account), total); assert_eq!(st.get_spendable_balance(account, 1), total); - let input_selector = GreedyInputSelector::new( - zip317::SingleOutputChangeStrategy::new(Zip317FeeRule::standard(), None), - DustOutputPolicy::default(), - ); + let input_selector = input_selector(StandardFeeRule::Zip317, None); // This first request will fail due to insufficient non-dust funds let req = TransactionRequest::new(vec![Payment { diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 4a4ff34fec..c13fd259c9 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -78,6 +78,9 @@ 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 9ca89b9c66..fbdda8f7b5 100644 --- a/zcash_primitives/src/transaction/fees.rs +++ b/zcash_primitives/src/transaction/fees.rs @@ -56,3 +56,47 @@ 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 = "Using this fee rule violates ZIP 317, and might cause transactions built with it to fail. Use `StandardFeeRule::Zip317` instead." + )] + PreZip313, + #[deprecated( + note = "Using this fee rule violates ZIP 317, and might cause transactions built with it to fail. 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, + ), + } + } +}