diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 7a7ef321fb..b08cd2e498 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -13,10 +13,20 @@ and this library adheres to Rust's notion of - `WalletSummary::progress` - `WalletMeta` - `impl Default for wallet::input_selection::GreedyInputSelector` -- `zcash_client_backend::fees::SplitPolicy` -- `zcash_client_backend::fees::zip317::MultiOutputChangeStrategy` +- `zcash_client_backend::fees` + - `SplitPolicy` + - `StandardFeeRule` has been moved here from `zcash_primitives::fees`. Relative + to that type, the deprecated `PreZip313` and `Zip313` variants have been + removed. + - `zip317::{MultiOutputChangeStrategy, Zip317FeeRule}` + - `standard::MultiOutputChangeStrategy` +- A new feature flag, `non-standard-fees`, has been added. This flag is now + required in order to make use of any types or methods that enable non-standard + fee calculation. ### Changed +- MSRV is now 1.77.0. +- Migrated to `arti-client 0.23`. - `zcash_client_backend::data_api`: - `InputSource` has an added method `get_wallet_metadata` - `error::Error` has additional variant `Error::Change`. This necessitates @@ -54,15 +64,18 @@ and this library adheres to Rust's notion of 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. + - `zip317::SingleOutputChangeStrategy` has been made polymorphic in the fee + rule type, and takes an additional type parameter as a consequence. - 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. -- Migrated to `arti-client 0.23`. +- `zcash_client_backend::proto::ProposalDecodingError` has modified variants. + `ProposalDecodingError::FeeRuleNotSpecified` has been removed, and + `ProposalDecodingError::FeeRuleNotSupported` has been added to replace it. +- `zcash_client_backend::data_api::fees::fixed` is now available only via the + use of the `non-standard-fees` feature flag. ### Removed - `zcash_client_backend::data_api`: @@ -70,6 +83,9 @@ and this library adheres to Rust's notion of been removed. Use `WalletSummary::progress` instead. - `testing::input_selector` use explicit `InputSelector` constructors directly instead. + - The deprecated `wallet::create_spend_to_address` and `wallet::spend` + methods have been removed. Use `propose_transfer` and + `create_proposed_transaction` instead. - `zcash_client_backend::fees`: - `impl From for ChangeError<...>` diff --git a/zcash_client_backend/Cargo.toml b/zcash_client_backend/Cargo.toml index d350f00cdc..ef01a76658 100644 --- a/zcash_client_backend/Cargo.toml +++ b/zcash_client_backend/Cargo.toml @@ -211,6 +211,9 @@ test-dependencies = [ "incrementalmerkletree/test-dependencies", ] +## Exposes APIs that allow calculation of non-standard fees. +non-standard-fees = ["zcash_primitives/non-standard-fees"] + #! ### Experimental features ## Exposes unstable APIs. Their behaviour may change at any time. diff --git a/zcash_client_backend/src/data_api/testing.rs b/zcash_client_backend/src/data_api/testing.rs index 9e99355034..0c117c7c8e 100644 --- a/zcash_client_backend/src/data_api/testing.rs +++ b/zcash_client_backend/src/data_api/testing.rs @@ -31,7 +31,7 @@ use zcash_primitives::{ memo::Memo, transaction::{ components::{amount::NonNegativeAmount, sapling::zip212_enforcement}, - fees::{FeeRule, StandardFeeRule}, + fees::FeeRule, Transaction, TxId, }, }; @@ -48,7 +48,7 @@ use crate::{ address::UnifiedAddress, fees::{ standard::{self, SingleOutputChangeStrategy}, - ChangeStrategy, DustOutputPolicy, + ChangeStrategy, DustOutputPolicy, StandardFeeRule, }, keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey}, proposal::Proposal, @@ -59,14 +59,14 @@ use crate::{ ShieldedProtocol, }; -#[allow(deprecated)] +use super::error::Error; use super::{ chain::{scan_cached_blocks, BlockSource, ChainState, CommitmentTreeRoot, ScanSummary}, scanning::ScanRange, wallet::{ - create_proposed_transactions, create_spend_to_address, + create_proposed_transactions, input_selection::{GreedyInputSelector, InputSelector}, - propose_standard_transfer_to_address, propose_transfer, spend, + propose_standard_transfer_to_address, propose_transfer, }, Account, AccountBalance, AccountBirthday, AccountPurpose, AccountSource, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SeedRelevance, @@ -861,43 +861,7 @@ where + WalletCommitmentTrees, ::AccountId: ConditionallySelectable + Default + Send + 'static, { - /// Invokes [`create_spend_to_address`] with the given arguments. - #[allow(deprecated)] - #[allow(clippy::type_complexity)] - #[allow(clippy::too_many_arguments)] - pub fn create_spend_to_address( - &mut self, - usk: &UnifiedSpendingKey, - to: &Address, - amount: NonNegativeAmount, - memo: Option, - ovk_policy: OvkPolicy, - min_confirmations: NonZeroU32, - change_memo: Option, - fallback_change_pool: ShieldedProtocol, - ) -> Result< - NonEmpty, - super::wallet::TransferErrT, SingleOutputChangeStrategy>, - > { - let prover = LocalTxProver::bundled(); - let network = self.network().clone(); - create_spend_to_address( - self.wallet_mut(), - &network, - &prover, - &prover, - usk, - to, - amount, - memo, - ovk_policy, - min_confirmations, - change_memo, - fallback_change_pool, - ) - } - - /// Invokes [`spend`] with the given arguments. + /// Prepares and executes the given [`zip321::TransactionRequest`] in a single step. #[allow(clippy::type_complexity)] pub fn spend( &mut self, @@ -912,20 +876,33 @@ where InputsT: InputSelector, ChangeT: ChangeStrategy, { - #![allow(deprecated)] let prover = LocalTxProver::bundled(); let network = self.network().clone(); - spend( + + let account = self + .wallet() + .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) + .map_err(Error::DataSource)? + .ok_or(Error::KeyNotRecognized)?; + + let proposal = propose_transfer( self.wallet_mut(), &network, - &prover, - &prover, + account.id(), input_selector, change_strategy, - usk, request, - ovk_policy, min_confirmations, + )?; + + create_proposed_transactions( + self.wallet_mut(), + &network, + &prover, + &prover, + usk, + ovk_policy, + &proposal, ) } diff --git a/zcash_client_backend/src/data_api/testing/pool.rs b/zcash_client_backend/src/data_api/testing/pool.rs index a104e8c4c3..846a681136 100644 --- a/zcash_client_backend/src/data_api/testing/pool.rs +++ b/zcash_client_backend/src/data_api/testing/pool.rs @@ -17,7 +17,7 @@ use zcash_primitives::{ legacy::TransparentAddress, transaction::{ components::amount::NonNegativeAmount, - fees::{fixed::FeeRule as FixedFeeRule, zip317::FeeRule as Zip317FeeRule, StandardFeeRule}, + fees::zip317::{FeeRule as Zip317FeeRule, MARGINAL_FEE, MINIMUM_FEE}, Transaction, }, }; @@ -50,7 +50,7 @@ use crate::{ fees::{ self, standard::{self, SingleOutputChangeStrategy}, - DustOutputPolicy, SplitPolicy, + DustOutputPolicy, SplitPolicy, StandardFeeRule, }, scanning::ScanError, wallet::{Note, NoteId, OvkPolicy, ReceivedNote}, @@ -961,9 +961,8 @@ pub fn proposal_fails_if_not_all_ephemeral_outputs_consumed( - ds_factory: impl DataStoreFactory, +pub fn create_to_address_fails_on_incorrect_usk( + ds_factory: DSF, ) { let mut st = TestBuilder::new() .with_data_store_factory(ds_factory) @@ -976,23 +975,30 @@ pub fn create_to_address_fails_on_incorrect_usk( let acct1 = zip32::AccountId::try_from(1).unwrap(); let usk1 = UnifiedSpendingKey::from_seed(st.network(), &[1u8; 32], acct1).unwrap(); + let input_selector = GreedyInputSelector::::new(); + let change_strategy = + single_output_change_strategy(StandardFeeRule::Zip317, None, T::SHIELDED_PROTOCOL); + + let req = TransactionRequest::new(vec![Payment::without_memo( + to.to_zcash_address(st.network()), + NonNegativeAmount::const_from_u64(1), + )]) + .unwrap(); + // Attempting to spend with a USK that is not in the wallet results in an error assert_matches!( - st.create_spend_to_address( + st.spend( + &input_selector, + &change_strategy, &usk1, - &to, - NonNegativeAmount::const_from_u64(1), - None, + req, OvkPolicy::Sender, NonZeroU32::new(1).unwrap(), - None, - T::SHIELDED_PROTOCOL, ), Err(data_api::error::Error::KeyNotRecognized) ); } -#[allow(deprecated)] pub fn proposal_fails_with_no_blocks(ds_factory: DSF) where DSF: DataStoreFactory, @@ -1014,7 +1020,7 @@ where assert_matches!( st.propose_standard_transfer::( account_id, - StandardFeeRule::PreZip313, + StandardFeeRule::Zip317, NonZeroU32::new(1).unwrap(), &to, NonNegativeAmount::const_from_u64(1), @@ -1581,10 +1587,8 @@ pub fn external_address_change_spends_detected_in_restore_from_seed( assert_eq!(spendable.len(), 1); } -pub fn checkpoint_gaps( - ds_factory: impl DataStoreFactory, +pub fn checkpoint_gaps( + ds_factory: DSF, cache: impl TestCache, ) { let mut st = TestBuilder::new() @@ -1982,18 +1986,26 @@ pub fn checkpoint_gaps( .unwrap(); assert_eq!(spendable.len(), 1); - // Attempt to spend the note with 5 confirmations + let input_selector = GreedyInputSelector::::new(); + let change_strategy = + single_output_change_strategy(StandardFeeRule::Zip317, None, T::SHIELDED_PROTOCOL); + let to = T::fvk_default_address(¬_our_key); + let req = TransactionRequest::new(vec![Payment::without_memo( + to.to_zcash_address(st.network()), + NonNegativeAmount::const_from_u64(10000), + )]) + .unwrap(); + + // Attempt to spend the note with 5 confirmations assert_matches!( - st.create_spend_to_address( + st.spend( + &input_selector, + &change_strategy, account.usk(), - &to, - NonNegativeAmount::const_from_u64(10000), - None, + req, OvkPolicy::Sender, NonZeroU32::new(5).unwrap(), - None, - T::SHIELDED_PROTOCOL, ), Ok(_) ); @@ -2799,7 +2811,6 @@ pub fn scan_cached_blocks_allows_blocks_out_of_order( )]) .unwrap(); - #[allow(deprecated)] let input_selector = GreedyInputSelector::new(); let change_strategy = single_output_change_strategy(StandardFeeRule::Zip317, None, T::SHIELDED_PROTOCOL); diff --git a/zcash_client_backend/src/data_api/testing/transparent.rs b/zcash_client_backend/src/data_api/testing/transparent.rs index 9cfd6838a9..d663dc44a6 100644 --- a/zcash_client_backend/src/data_api/testing/transparent.rs +++ b/zcash_client_backend/src/data_api/testing/transparent.rs @@ -1,21 +1,19 @@ use crate::{ data_api::{ - testing::{AddressType, TestBuilder, TestState}, - testing::{DataStoreFactory, ShieldedProtocol, TestCache}, + testing::{ + AddressType, DataStoreFactory, ShieldedProtocol, TestBuilder, TestCache, TestState, + }, wallet::input_selection::GreedyInputSelector, Account as _, InputSource, WalletRead, WalletWrite, }, - fees::{fixed, DustOutputPolicy}, + fees::{standard, DustOutputPolicy, StandardFeeRule}, wallet::WalletTransparentOutput, }; use assert_matches::assert_matches; use sapling::zip32::ExtendedSpendingKey; use zcash_primitives::{ block::BlockHash, - transaction::{ - components::{amount::NonNegativeAmount, OutPoint, TxOut}, - fees::fixed::FeeRule as FixedFeeRule, - }, + transaction::components::{amount::NonNegativeAmount, OutPoint, TxOut}, }; pub fn put_received_transparent_utxo(dsf: DSF) @@ -198,8 +196,8 @@ where // Shield the output. let input_selector = GreedyInputSelector::new(); - let change_strategy = fixed::SingleOutputChangeStrategy::new( - FixedFeeRule::non_standard(NonNegativeAmount::ZERO), + let change_strategy = standard::SingleOutputChangeStrategy::new( + StandardFeeRule::Zip317, None, ShieldedProtocol::Sapling, DustOutputPolicy::default(), diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 6d9bf6f3a8..0d43844116 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -50,7 +50,9 @@ use crate::{ WalletRead, WalletWrite, }, decrypt_transaction, - fees::{standard::SingleOutputChangeStrategy, ChangeStrategy, DustOutputPolicy}, + fees::{ + standard::SingleOutputChangeStrategy, ChangeStrategy, DustOutputPolicy, StandardFeeRule, + }, keys::UnifiedSpendingKey, proposal::{Proposal, ProposalError, Step, StepOutputIndex}, wallet::{Note, OvkPolicy, Recipient}, @@ -62,7 +64,7 @@ use zcash_primitives::{ transaction::{ builder::{BuildConfig, BuildResult, Builder}, components::{amount::NonNegativeAmount, sapling::zip212_enforcement, OutPoint}, - fees::{FeeRule, StandardFeeRule}, + fees::FeeRule, Transaction, TxId, }, }; @@ -169,284 +171,6 @@ pub type ShieldErrT = Error< Infallible, >; -#[allow(clippy::needless_doctest_main)] -/// Creates a transaction or series of transactions paying the specified address from -/// the given account, and the [`TxId`] corresponding to each newly-created transaction. -/// -/// These transactions can be retrieved from the underlying data store using the -/// [`WalletRead::get_transaction`] method. -/// -/// Do not call this multiple times in parallel, or you will generate transactions that -/// double-spend the same notes. -/// -/// # Transaction privacy -/// -/// `ovk_policy` specifies the desired policy for which outgoing viewing key should be -/// able to decrypt the outputs of this transaction. This is primarily relevant to -/// wallet recovery from backup; in particular, [`OvkPolicy::Discard`] will prevent the -/// recipient's address, and the contents of `memo`, from ever being recovered from the -/// block chain. (The total value sent can always be inferred by the sender from the spent -/// notes and received change.) -/// -/// Regardless of the specified policy, `create_spend_to_address` saves `to`, `value`, and -/// `memo` in `db_data`. This can be deleted independently of `ovk_policy`. -/// -/// For details on what transaction information is visible to the holder of a full or -/// outgoing viewing key, refer to [ZIP 310]. -/// -/// [ZIP 310]: https://zips.z.cash/zip-0310 -/// -/// Parameters: -/// * `wallet_db`: A read/write reference to the wallet database -/// * `params`: Consensus parameters -/// * `spend_prover`: The [`sapling::SpendProver`] to use in constructing the shielded -/// transaction. -/// * `output_prover`: The [`sapling::OutputProver`] to use in constructing the shielded -/// transaction. -/// * `usk`: The unified spending key that controls the funds that will be spent -/// in the resulting transaction. This procedure will return an error if the -/// USK does not correspond to an account known to the wallet. -/// * `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. -/// * `ovk_policy`: The policy to use for constructing outgoing viewing keys that -/// can allow the sender to view the resulting notes on the blockchain. -/// * `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. -/// * `change_memo`: A memo to be included in the change output -/// -/// # Examples -/// -/// ``` -/// # #[cfg(all(feature = "test-dependencies", feature = "local-prover"))] -/// # { -/// use zcash_primitives::{ -/// consensus::{self, Network}, -/// constants::testnet::COIN_TYPE, -/// transaction::{TxId, components::Amount}, -/// zip32::AccountId, -/// }; -/// use zcash_proofs::prover::LocalTxProver; -/// use zcash_client_backend::{ -/// keys::{UnifiedSpendingKey, UnifiedAddressRequest}, -/// data_api::{wallet::create_spend_to_address, error::Error, testing}, -/// wallet::OvkPolicy, -/// }; -/// -/// # use std::convert::Infallible; -/// # use zcash_primitives::transaction::components::amount::BalanceError; -/// # use zcash_client_backend::{ -/// # data_api::wallet::input_selection::GreedyInputSelectorError, -/// # }; -/// # -/// # fn main() { -/// # test(); -/// # } -/// # -/// # #[allow(deprecated)] -/// # fn test() -> Result, Infallible, u32>> { -/// -/// let tx_prover = match LocalTxProver::with_default_location() { -/// Some(tx_prover) => tx_prover, -/// None => { -/// panic!("Cannot locate the Zcash parameters. Please run zcash-fetch-params or fetch-params.sh to download the parameters, and then re-run the tests."); -/// } -/// }; -/// -/// let account = AccountId::from(0); -/// let req = UnifiedAddressRequest::new(false, true, true); -/// let usk = UnifiedSpendingKey::from_seed(&Network::TestNetwork, &[0; 32][..], account).unwrap(); -/// let to = usk.to_unified_full_viewing_key().default_address(req).0.into(); -/// -/// let mut db_read = testing::MockWalletDb { -/// network: Network::TestNetwork -/// }; -/// -/// create_spend_to_address( -/// &mut db_read, -/// &Network::TestNetwork, -/// tx_prover, -/// &usk, -/// &to, -/// Amount::from_u64(1).unwrap(), -/// None, -/// OvkPolicy::Sender, -/// 10, -/// None -/// ) -/// -/// # } -/// # } -/// ``` -/// -/// [`sapling::SpendProver`]: sapling::prover::SpendProver -/// [`sapling::OutputProver`]: sapling::prover::OutputProver -#[allow(clippy::too_many_arguments)] -#[allow(clippy::type_complexity)] -#[deprecated( - note = "Use `propose_transfer` and `create_proposed_transactions` instead. `create_spend_to_address` uses a fixed fee of 10000 zatoshis, which is not compliant with ZIP 317." -)] -pub fn create_spend_to_address( - wallet_db: &mut DbT, - params: &ParamsT, - spend_prover: &impl SpendProver, - output_prover: &impl OutputProver, - usk: &UnifiedSpendingKey, - to: &Address, - amount: NonNegativeAmount, - memo: Option, - ovk_policy: OvkPolicy, - min_confirmations: NonZeroU32, - change_memo: Option, - fallback_change_pool: ShieldedProtocol, -) -> Result< - NonEmpty, - TransferErrT, SingleOutputChangeStrategy>, -> -where - ParamsT: consensus::Parameters + Clone, - DbT: InputSource, - DbT: WalletWrite< - Error = ::Error, - AccountId = ::AccountId, - >, - DbT: WalletCommitmentTrees, -{ - 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.id(), - min_confirmations, - to, - amount, - memo, - change_memo, - fallback_change_pool, - )?; - - create_proposed_transactions( - wallet_db, - params, - spend_prover, - output_prover, - usk, - ovk_policy, - &proposal, - ) -} - -/// Constructs a transaction or series of transactions that send funds as specified -/// by the `request` argument, stores them to the wallet's "sent transactions" data -/// store, and returns the [`TxId`] for each transaction constructed. -/// -/// The newly-created transactions can be retrieved from the underlying data store using the -/// [`WalletRead::get_transaction`] method. -/// -/// Do not call this multiple times in parallel, or you will generate transactions that -/// double-spend the same notes. -/// -/// # Transaction privacy -/// -/// `ovk_policy` specifies the desired policy for which outgoing viewing key should be -/// able to decrypt the outputs of this transaction. This is primarily relevant to -/// wallet recovery from backup; in particular, [`OvkPolicy::Discard`] will prevent the -/// recipient's address, and the contents of `memo`, from ever being recovered from the -/// block chain. (The total value sent can always be inferred by the sender from the spent -/// notes and received change.) -/// -/// Regardless of the specified policy, `create_spend_to_address` saves `to`, `value`, and -/// `memo` in `db_data`. This can be deleted independently of `ovk_policy`. -/// -/// For details on what transaction information is visible to the holder of a full or -/// outgoing viewing key, refer to [ZIP 310]. -/// -/// [ZIP 310]: https://zips.z.cash/zip-0310 -/// -/// Parameters: -/// * `wallet_db`: A read/write reference to the wallet database -/// * `params`: Consensus parameters -/// * `spend_prover`: The [`sapling::SpendProver`] to use in constructing the shielded -/// transaction. -/// * `output_prover`: The [`sapling::OutputProver`] to use in constructing the shielded -/// transaction. -/// * `input_selector`: The [`InputSelector`] that will be used to select available -/// inputs from the wallet database, choose change amounts and compute required -/// transaction fees. -/// * `usk`: The unified spending key that controls the funds that will be spent -/// in the resulting transaction. This procedure will return an error if the -/// USK does not correspond to an account known to the wallet. -/// * `request`: The ZIP-321 payment request specifying the recipients and amounts -/// for the transaction. -/// * `ovk_policy`: The policy to use for constructing outgoing viewing keys that -/// can allow the sender to view the resulting notes on the blockchain. -/// * `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. -/// -/// [`sapling::SpendProver`]: sapling::prover::SpendProver -/// [`sapling::OutputProver`]: sapling::prover::OutputProver -#[allow(clippy::too_many_arguments)] -#[allow(clippy::type_complexity)] -#[deprecated(note = "Use `propose_transfer` and `create_proposed_transactions` instead.")] -pub fn spend( - wallet_db: &mut DbT, - params: &ParamsT, - spend_prover: &impl SpendProver, - output_prover: &impl OutputProver, - input_selector: &InputsT, - change_strategy: &ChangeT, - usk: &UnifiedSpendingKey, - request: zip321::TransactionRequest, - ovk_policy: OvkPolicy, - min_confirmations: NonZeroU32, -) -> Result, TransferErrT> -where - DbT: InputSource, - DbT: WalletWrite< - Error = ::Error, - AccountId = ::AccountId, - >, - DbT: WalletCommitmentTrees, - ParamsT: consensus::Parameters + Clone, - InputsT: InputSelector, - ChangeT: ChangeStrategy, -{ - let account = wallet_db - .get_account_for_ufvk(&usk.to_unified_full_viewing_key()) - .map_err(Error::DataSource)? - .ok_or(Error::KeyNotRecognized)?; - - let proposal = propose_transfer( - wallet_db, - params, - account.id(), - input_selector, - change_strategy, - request, - min_confirmations, - )?; - - create_proposed_transactions( - wallet_db, - params, - spend_prover, - output_prover, - usk, - ovk_policy, - &proposal, - ) -} - /// Select transaction inputs, compute fees, and construct a proposal for a transaction or series /// of transactions that can then be authorized and made ready for submission to the network with /// [`create_proposed_transactions`]. diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index e855a0462a..a10136db51 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -8,14 +8,18 @@ use zcash_primitives::{ memo::MemoBytes, transaction::{ components::{amount::NonNegativeAmount, OutPoint}, - fees::{transparent, FeeRule}, + fees::{ + transparent::{self, InputSize}, + zip317 as prim_zip317, FeeRule, + }, }, }; use zcash_protocol::{PoolType, ShieldedProtocol}; use crate::data_api::InputSource; -pub(crate) mod common; +pub mod common; +#[cfg(feature = "non-standard-fees")] pub mod fixed; #[cfg(feature = "orchard")] pub mod orchard; @@ -23,6 +27,40 @@ pub mod sapling; pub mod standard; pub mod zip317; +/// An enumeration of the standard fee rules supported by the wallet backend. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum StandardFeeRule { + Zip317, +} + +impl FeeRule for StandardFeeRule { + type Error = prim_zip317::FeeError; + + fn fee_required( + &self, + params: &P, + target_height: BlockHeight, + transparent_input_sizes: impl IntoIterator, + transparent_output_sizes: impl IntoIterator, + sapling_input_count: usize, + sapling_output_count: usize, + orchard_action_count: usize, + ) -> Result { + #[allow(deprecated)] + match self { + Self::Zip317 => prim_zip317::FeeRule::standard().fee_required( + params, + target_height, + transparent_input_sizes, + transparent_output_sizes, + sapling_input_count, + sapling_output_count, + orchard_action_count, + ), + } + } +} + /// `ChangeValue` represents either a proposed change output to a shielded pool /// (with an optional change memo), or if the "transparent-inputs" feature is /// enabled, an ephemeral output to the transparent pool. @@ -203,33 +241,6 @@ pub enum ChangeError { BundleError(&'static str), } -impl ChangeError { - pub(crate) fn map E0>(self, f: F) -> ChangeError { - match self { - ChangeError::InsufficientFunds { - available, - required, - } => ChangeError::InsufficientFunds { - available, - required, - }, - ChangeError::DustInputs { - transparent, - sapling, - #[cfg(feature = "orchard")] - orchard, - } => ChangeError::DustInputs { - transparent, - sapling, - #[cfg(feature = "orchard")] - orchard, - }, - ChangeError::StrategyError(e) => ChangeError::StrategyError(f(e)), - ChangeError::BundleError(e) => ChangeError::BundleError(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/common.rs b/zcash_client_backend/src/fees/common.rs index 45b77e5dfa..2a1a4a4e2d 100644 --- a/zcash_client_backend/src/fees/common.rs +++ b/zcash_client_backend/src/fees/common.rs @@ -500,7 +500,7 @@ where .dust_threshold() .unwrap_or(cfg.default_dust_threshold); - if per_output_change.quotient() < &change_dust_threshold { + if proposed_change < change_dust_threshold { match cfg.dust_output_policy.action() { DustAction::Reject => { // Always allow zero-valued change even for the `Reject` policy: @@ -509,11 +509,11 @@ where // * this case occurs in practice when sending all funds from an account; // * zero-valued notes do not require witness tracking; // * the effect on trial decryption overhead is small. - if per_output_change.quotient().is_zero() { + if proposed_change.is_zero() && excess_fee.is_zero() { simple_case() } else { - let shortfall = (change_dust_threshold - *per_output_change.quotient()) - .ok_or_else(underflow)?; + let shortfall = + (change_dust_threshold - proposed_change).ok_or_else(underflow)?; return Err(ChangeError::InsufficientFunds { available: total_in, diff --git a/zcash_client_backend/src/fees/fixed.rs b/zcash_client_backend/src/fees/fixed.rs index 452135fbba..49cd861148 100644 --- a/zcash_client_backend/src/fees/fixed.rs +++ b/zcash_client_backend/src/fees/fixed.rs @@ -119,7 +119,7 @@ mod tests { consensus::{Network, NetworkUpgrade, Parameters}, transaction::{ components::{amount::NonNegativeAmount, transparent::TxOut}, - fees::fixed::FeeRule as FixedFeeRule, + fees::{fixed::FeeRule as FixedFeeRule, zip317::MINIMUM_FEE}, }, }; @@ -138,8 +138,7 @@ mod tests { #[test] fn change_without_dust() { - #[allow(deprecated)] - let fee_rule = FixedFeeRule::standard(); + let fee_rule = FixedFeeRule::non_standard(MINIMUM_FEE); let change_strategy = SingleOutputChangeStrategy::::new( fee_rule, None, @@ -175,14 +174,13 @@ mod tests { result, Ok(balance) if balance.proposed_change() == [ChangeValue::sapling(NonNegativeAmount::const_from_u64(10000), None)] && - balance.fee_required() == NonNegativeAmount::const_from_u64(10000) + balance.fee_required() == MINIMUM_FEE ); } #[test] fn dust_change() { - #[allow(deprecated)] - let fee_rule = FixedFeeRule::standard(); + let fee_rule = FixedFeeRule::non_standard(MINIMUM_FEE); let change_strategy = SingleOutputChangeStrategy::::new( fee_rule, None, diff --git a/zcash_client_backend/src/fees/standard.rs b/zcash_client_backend/src/fees/standard.rs index c3e46d3875..f9a14a8514 100644 --- a/zcash_client_backend/src/fees/standard.rs +++ b/zcash_client_backend/src/fees/standard.rs @@ -1,150 +1,17 @@ //! Change strategies designed for use with a standard fee. -use std::marker::PhantomData; - -use zcash_primitives::{ - consensus::{self, BlockHeight}, - memo::MemoBytes, - transaction::{ - components::amount::NonNegativeAmount, - fees::{ - fixed::FeeRule as FixedFeeRule, - transparent, - zip317::{FeeError as Zip317FeeError, FeeRule as Zip317FeeRule}, - StandardFeeRule, - }, - }, -}; - -use crate::{data_api::InputSource, ShieldedProtocol}; - -use super::{ - fixed, sapling as sapling_fees, zip317, ChangeError, ChangeStrategy, DustOutputPolicy, - EphemeralBalance, TransactionBalance, -}; - -#[cfg(feature = "orchard")] -use super::orchard as orchard_fees; +use super::StandardFeeRule; /// A change strategy that proposes change as a single output. The output pool is chosen /// as the most current pool that avoids unnecessary pool-crossing (with a specified /// fallback when the transaction has no shielded inputs). Fee calculation is delegated /// to the provided fee rule. -pub struct SingleOutputChangeStrategy { - fee_rule: StandardFeeRule, - change_memo: Option, - fallback_change_pool: ShieldedProtocol, - dust_output_policy: DustOutputPolicy, - meta_source: PhantomData, -} - -impl SingleOutputChangeStrategy { - /// Constructs a new [`SingleOutputChangeStrategy`] with the specified ZIP 317 - /// fee parameters. - /// - /// `fallback_change_pool` is used when more than one shielded pool is enabled via - /// feature flags, and the transaction has no shielded inputs. - pub fn new( - fee_rule: StandardFeeRule, - change_memo: Option, - fallback_change_pool: ShieldedProtocol, - dust_output_policy: DustOutputPolicy, - ) -> Self { - Self { - fee_rule, - change_memo, - fallback_change_pool, - dust_output_policy, - meta_source: PhantomData, - } - } -} - -impl ChangeStrategy for SingleOutputChangeStrategy { - type FeeRule = StandardFeeRule; - type Error = Zip317FeeError; - type MetaSource = I; - type WalletMetaT = (); - - fn fee_rule(&self) -> &Self::FeeRule { - &self.fee_rule - } - - fn fetch_wallet_meta( - &self, - _meta_source: &Self::MetaSource, - _account: ::AccountId, - _exclude: &[::NoteRef], - ) -> Result::Error> { - Ok(()) - } - - fn compute_balance( - &self, - params: &P, - target_height: BlockHeight, - transparent_inputs: &[impl transparent::InputView], - transparent_outputs: &[impl transparent::OutputView], - sapling: &impl sapling_fees::BundleView, - #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, - ephemeral_balance: Option<&EphemeralBalance>, - wallet_meta: &Self::WalletMetaT, - ) -> Result> { - #[allow(deprecated)] - match self.fee_rule() { - StandardFeeRule::PreZip313 => fixed::SingleOutputChangeStrategy::::new( - FixedFeeRule::non_standard(NonNegativeAmount::const_from_u64(10000)), - self.change_memo.clone(), - self.fallback_change_pool, - self.dust_output_policy, - ) - .compute_balance( - params, - target_height, - transparent_inputs, - transparent_outputs, - sapling, - #[cfg(feature = "orchard")] - orchard, - ephemeral_balance, - wallet_meta, - ) - .map_err(|e| e.map(Zip317FeeError::Balance)), - StandardFeeRule::Zip313 => fixed::SingleOutputChangeStrategy::::new( - FixedFeeRule::non_standard(NonNegativeAmount::const_from_u64(1000)), - self.change_memo.clone(), - self.fallback_change_pool, - self.dust_output_policy, - ) - .compute_balance( - params, - target_height, - transparent_inputs, - transparent_outputs, - sapling, - #[cfg(feature = "orchard")] - orchard, - ephemeral_balance, - wallet_meta, - ) - .map_err(|e| e.map(Zip317FeeError::Balance)), - StandardFeeRule::Zip317 => zip317::SingleOutputChangeStrategy::::new( - Zip317FeeRule::standard(), - self.change_memo.clone(), - self.fallback_change_pool, - self.dust_output_policy, - ) - .compute_balance( - params, - target_height, - transparent_inputs, - transparent_outputs, - sapling, - #[cfg(feature = "orchard")] - orchard, - ephemeral_balance, - wallet_meta, - ), - } - } -} +pub type SingleOutputChangeStrategy = + super::zip317::SingleOutputChangeStrategy; + +/// A change strategy that proposes change as potentially multiple evenly-sized outputs having at +/// least a threshold value. The output pool is chosen as the most current pool that avoids +/// unnecessary pool-crossing (with a specified fallback when the transaction has no shielded +/// inputs). Fee calculation is delegated to the provided fee rule. +pub type MultiOutputChangeStrategy = + super::zip317::MultiOutputChangeStrategy; diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs index 0706bfaf81..7d89897be6 100644 --- a/zcash_client_backend/src/fees/zip317.rs +++ b/zcash_client_backend/src/fees/zip317.rs @@ -9,14 +9,13 @@ use std::marker::PhantomData; use zcash_primitives::{ consensus::{self, BlockHeight}, memo::MemoBytes, - transaction::fees::{ - transparent, - zip317::{FeeError as Zip317FeeError, FeeRule as Zip317FeeRule}, - }, + transaction::fees::{transparent, zip317 as prim_zip317, FeeRule}, }; +use zcash_protocol::value::{BalanceError, Zatoshis}; use crate::{ data_api::{InputSource, WalletMeta}, + fees::StandardFeeRule, ShieldedProtocol, }; @@ -29,26 +28,56 @@ use super::{ #[cfg(feature = "orchard")] use super::orchard as orchard_fees; +/// An extension to the [`FeeRule`] trait that exposes methods required for +/// ZIP 317 fee calculation. +pub trait Zip317FeeRule: FeeRule { + /// Returns the ZIP 317 marginal fee. + fn marginal_fee(&self) -> Zatoshis; + + /// Returns the ZIP 317 number of grace actions + fn grace_actions(&self) -> usize; +} + +impl Zip317FeeRule for prim_zip317::FeeRule { + fn marginal_fee(&self) -> Zatoshis { + self.marginal_fee() + } + + fn grace_actions(&self) -> usize { + self.grace_actions() + } +} + +impl Zip317FeeRule for StandardFeeRule { + fn marginal_fee(&self) -> Zatoshis { + prim_zip317::FeeRule::standard().marginal_fee() + } + + fn grace_actions(&self) -> usize { + prim_zip317::FeeRule::standard().grace_actions() + } +} + /// A change strategy that proposes change as a single output. The output pool is chosen /// as the most current pool that avoids unnecessary pool-crossing (with a specified /// fallback when the transaction has no shielded inputs). Fee calculation is delegated /// to the provided fee rule. -pub struct SingleOutputChangeStrategy { - fee_rule: Zip317FeeRule, +pub struct SingleOutputChangeStrategy { + fee_rule: R, change_memo: Option, fallback_change_pool: ShieldedProtocol, dust_output_policy: DustOutputPolicy, meta_source: PhantomData, } -impl SingleOutputChangeStrategy { +impl SingleOutputChangeStrategy { /// Constructs a new [`SingleOutputChangeStrategy`] with the specified ZIP 317 /// fee parameters and change memo. /// /// `fallback_change_pool` is used when more than one shielded pool is enabled via /// feature flags, and the transaction has no shielded inputs. pub fn new( - fee_rule: Zip317FeeRule, + fee_rule: R, change_memo: Option, fallback_change_pool: ShieldedProtocol, dust_output_policy: DustOutputPolicy, @@ -63,9 +92,14 @@ impl SingleOutputChangeStrategy { } } -impl ChangeStrategy for SingleOutputChangeStrategy { - type FeeRule = Zip317FeeRule; - type Error = Zip317FeeError; +impl ChangeStrategy for SingleOutputChangeStrategy +where + R: Zip317FeeRule + Clone, + I: InputSource, + ::Error: From, +{ + type FeeRule = R; + type Error = ::Error; type MetaSource = I; type WalletMetaT = (); @@ -122,8 +156,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy { /// A change strategy that attempts to split the change value into some number of equal-sized notes /// as dictated by the included [`SplitPolicy`] value. -pub struct MultiOutputChangeStrategy { - fee_rule: Zip317FeeRule, +pub struct MultiOutputChangeStrategy { + fee_rule: R, change_memo: Option, fallback_change_pool: ShieldedProtocol, dust_output_policy: DustOutputPolicy, @@ -131,7 +165,7 @@ pub struct MultiOutputChangeStrategy { meta_source: PhantomData, } -impl MultiOutputChangeStrategy { +impl MultiOutputChangeStrategy { /// Constructs a new [`MultiOutputChangeStrategy`] with the specified ZIP 317 /// fee parameters, change memo, and change splitting policy. /// @@ -144,7 +178,7 @@ impl MultiOutputChangeStrategy { /// - `split_policy`: A policy value describing how the change value should be returned as /// multiple notes. pub fn new( - fee_rule: Zip317FeeRule, + fee_rule: R, change_memo: Option, fallback_change_pool: ShieldedProtocol, dust_output_policy: DustOutputPolicy, @@ -161,9 +195,14 @@ impl MultiOutputChangeStrategy { } } -impl ChangeStrategy for MultiOutputChangeStrategy { - type FeeRule = Zip317FeeRule; - type Error = Zip317FeeError; +impl ChangeStrategy for MultiOutputChangeStrategy +where + R: Zip317FeeRule + Clone, + I: InputSource, + ::Error: From, +{ + type FeeRule = R; + type Error = ::Error; type MetaSource = I; type WalletMetaT = WalletMeta; @@ -249,7 +288,7 @@ mod tests { #[test] fn change_without_dust() { - let change_strategy = SingleOutputChangeStrategy::::new( + let change_strategy = SingleOutputChangeStrategy::<_, MockWalletDb>::new( Zip317FeeRule::standard(), None, ShieldedProtocol::Sapling, @@ -290,7 +329,7 @@ mod tests { #[test] fn change_without_dust_multi() { - let change_strategy = MultiOutputChangeStrategy::::new( + let change_strategy = MultiOutputChangeStrategy::<_, MockWalletDb>::new( Zip317FeeRule::standard(), None, ShieldedProtocol::Sapling, @@ -404,7 +443,7 @@ mod tests { #[test] #[cfg(feature = "orchard")] fn cross_pool_change_without_dust() { - let change_strategy = SingleOutputChangeStrategy::::new( + let change_strategy = SingleOutputChangeStrategy::<_, MockWalletDb>::new( Zip317FeeRule::standard(), None, ShieldedProtocol::Orchard, @@ -460,7 +499,7 @@ mod tests { } fn change_with_transparent_payments(dust_output_policy: DustOutputPolicy) { - let change_strategy = SingleOutputChangeStrategy::::new( + let change_strategy = SingleOutputChangeStrategy::<_, MockWalletDb>::new( Zip317FeeRule::standard(), None, ShieldedProtocol::Sapling, @@ -506,7 +545,7 @@ mod tests { use crate::fees::sapling as sapling_fees; use zcash_primitives::{legacy::TransparentAddress, transaction::components::OutPoint}; - let change_strategy = SingleOutputChangeStrategy::::new( + let change_strategy = SingleOutputChangeStrategy::<_, MockWalletDb>::new( Zip317FeeRule::standard(), None, ShieldedProtocol::Sapling, @@ -551,7 +590,7 @@ mod tests { use crate::fees::sapling as sapling_fees; use zcash_primitives::{legacy::TransparentAddress, transaction::components::OutPoint}; - let change_strategy = SingleOutputChangeStrategy::::new( + let change_strategy = SingleOutputChangeStrategy::<_, MockWalletDb>::new( Zip317FeeRule::standard(), None, ShieldedProtocol::Sapling, @@ -596,7 +635,7 @@ mod tests { use crate::fees::sapling as sapling_fees; use zcash_primitives::{legacy::TransparentAddress, transaction::components::OutPoint}; - let change_strategy = SingleOutputChangeStrategy::::new( + let change_strategy = SingleOutputChangeStrategy::<_, MockWalletDb>::new( Zip317FeeRule::standard(), None, ShieldedProtocol::Sapling, @@ -655,7 +694,7 @@ mod tests { } fn change_with_allowable_dust(dust_output_policy: DustOutputPolicy) { - let change_strategy = SingleOutputChangeStrategy::::new( + let change_strategy = SingleOutputChangeStrategy::<_, MockWalletDb>::new( Zip317FeeRule::standard(), None, ShieldedProtocol::Sapling, @@ -705,7 +744,7 @@ mod tests { #[test] fn change_with_disallowed_dust() { - let change_strategy = SingleOutputChangeStrategy::::new( + let change_strategy = SingleOutputChangeStrategy::<_, MockWalletDb>::new( Zip317FeeRule::standard(), None, ShieldedProtocol::Sapling, diff --git a/zcash_client_backend/src/proto.rs b/zcash_client_backend/src/proto.rs index c832fb44f6..79813d7c4c 100644 --- a/zcash_client_backend/src/proto.rs +++ b/zcash_client_backend/src/proto.rs @@ -16,12 +16,12 @@ use zcash_primitives::{ consensus::BlockHeight, memo::{self, MemoBytes}, merkle_tree::read_commitment_tree, - transaction::{components::amount::NonNegativeAmount, fees::StandardFeeRule, TxId}, + transaction::{components::amount::NonNegativeAmount, TxId}, }; use crate::{ data_api::{chain::ChainState, InputSource}, - fees::{ChangeValue, TransactionBalance}, + fees::{ChangeValue, StandardFeeRule, TransactionBalance}, proposal::{Proposal, ProposalError, ShieldedInputs, Step, StepOutput, StepOutputIndex}, zip321::{TransactionRequest, Zip321Error}, PoolType, ShieldedProtocol, @@ -356,8 +356,8 @@ pub enum ProposalDecodingError { MemoInvalid(memo::Error), /// The serialization version returned by the protobuf was not recognized. VersionInvalid(u32), - /// The proposal did not correctly specify a standard fee rule. - FeeRuleNotSpecified, + /// The fee rule specified by the proposal is not supported by the wallet. + FeeRuleNotSupported(proposal::FeeRule), /// The proposal violated balance or structural constraints. ProposalInvalid(ProposalError), /// An inputs field for the given protocol was present, but contained no input note references. @@ -409,8 +409,12 @@ impl Display for ProposalDecodingError { ProposalDecodingError::VersionInvalid(v) => { write!(f, "Unrecognized proposal version {}", v) } - ProposalDecodingError::FeeRuleNotSpecified => { - write!(f, "Proposal did not specify a known fee rule.") + ProposalDecodingError::FeeRuleNotSupported(r) => { + write!( + f, + "Fee calculation using the {:?} fee rule is not supported.", + r + ) } ProposalDecodingError::ProposalInvalid(err) => write!(f, "{}", err), ProposalDecodingError::EmptyShieldedInputs(protocol) => write!( @@ -596,12 +600,9 @@ impl proposal::Proposal { }) .collect(); - #[allow(deprecated)] proposal::Proposal { proto_version: PROPOSAL_SER_V1, fee_rule: match value.fee_rule() { - StandardFeeRule::PreZip313 => proposal::FeeRule::PreZip313, - StandardFeeRule::Zip313 => proposal::FeeRule::Zip313, StandardFeeRule::Zip317 => proposal::FeeRule::Zip317, } .into(), @@ -622,13 +623,10 @@ impl proposal::Proposal { use self::proposal::proposed_input::Value::*; match self.proto_version { PROPOSAL_SER_V1 => { - #[allow(deprecated)] let fee_rule = match self.fee_rule() { - proposal::FeeRule::PreZip313 => StandardFeeRule::PreZip313, - proposal::FeeRule::Zip313 => StandardFeeRule::Zip313, proposal::FeeRule::Zip317 => StandardFeeRule::Zip317, - proposal::FeeRule::NotSpecified => { - return Err(ProposalDecodingError::FeeRuleNotSpecified); + other => { + return Err(ProposalDecodingError::FeeRuleNotSupported(other)); } }; diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 03e6f05af6..9c3793ba5b 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -94,7 +94,7 @@ tempfile = "3.5.0" zcash_keys = { workspace = true, features = ["test-dependencies"] } zcash_note_encryption.workspace = true zcash_proofs = { workspace = true, features = ["bundled-prover"] } -zcash_primitives = { workspace = true, features = ["test-dependencies"] } +zcash_primitives = { workspace = true, features = ["test-dependencies", "non-standard-fees"] } zcash_protocol = { workspace = true, features = ["local-consensus"] } zcash_client_backend = { workspace = true, features = ["test-dependencies", "unstable-serialization", "unstable-spanning-tree"] } zcash_address = { workspace = true, features = ["test-dependencies"] } diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 01209e181c..42e111d629 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -320,7 +320,6 @@ where } #[cfg(test)] -#[allow(deprecated)] mod tests { use zcash_client_backend::data_api::testing::sapling::SaplingPoolTester; @@ -340,7 +339,6 @@ mod tests { testing::pool::valid_chain_states::() } - // FIXME: This requires test framework fixes to pass. #[test] #[cfg(feature = "orchard")] fn invalid_chain_cache_disconnected_sapling() { diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index 508961b890..ab043e3d43 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -2,10 +2,10 @@ use prost::Message; use rusqlite::params; use tempfile::NamedTempFile; -use zcash_client_backend::data_api::testing::{NoteCommitments, TestCache}; - -#[allow(deprecated)] -use zcash_client_backend::proto::compact_formats::CompactBlock; +use zcash_client_backend::{ + data_api::testing::{NoteCommitments, TestCache}, + proto::compact_formats::CompactBlock, +}; use crate::{chain::init::init_cache_database, error::SqliteClientError}; diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 9d5d0a3c61..8465a582a4 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -65,14 +65,12 @@ pub(crate) fn proposal_fails_if_not_all_ephemeral_outputs_consumed() { - zcash_client_backend::data_api::testing::pool::create_to_address_fails_on_incorrect_usk::( + zcash_client_backend::data_api::testing::pool::create_to_address_fails_on_incorrect_usk::( TestDbFactory::default(), ) } -#[allow(deprecated)] pub(crate) fn proposal_fails_with_no_blocks() { zcash_client_backend::data_api::testing::pool::proposal_fails_with_no_blocks::( TestDbFactory::default(), @@ -149,7 +147,7 @@ pub(crate) fn birthday_in_anchor_shard() { } pub(crate) fn checkpoint_gaps() { - zcash_client_backend::data_api::testing::pool::checkpoint_gaps::( + zcash_client_backend::data_api::testing::pool::checkpoint_gaps::( TestDbFactory::default(), BlockCache::new(), ) diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index f9d7ecbdc6..2fa90e826f 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -409,7 +409,6 @@ fn verify_sqlite_version_compatibility( } #[cfg(test)] -#[allow(deprecated)] mod tests { use rusqlite::{self, named_params, Connection, ToSql}; use secrecy::Secret; @@ -676,6 +675,7 @@ mod tests { let seed = [0xab; 32]; let account = AccountId::ZERO; let secret_key = sapling::spending_key(&seed, db_data.params.coin_type(), account); + #[allow(deprecated)] let extfvk = secret_key.to_extended_full_viewing_key(); init_0_3_0(&mut db_data, &extfvk, account).unwrap(); @@ -847,6 +847,7 @@ mod tests { let seed = [0xab; 32]; let account = AccountId::ZERO; let secret_key = sapling::spending_key(&seed, db_data.params.coin_type(), account); + #[allow(deprecated)] let extfvk = secret_key.to_extended_full_viewing_key(); init_autoshielding(&mut db_data, &extfvk, account).unwrap(); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs b/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs index 9773ab19bd..efab795a65 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs @@ -86,15 +86,12 @@ mod tests { wallet::input_selection::GreedyInputSelector, Account as _, WalletRead as _, WalletWrite as _, }, - fees::{standard, DustOutputPolicy}, + fees::{standard, DustOutputPolicy, StandardFeeRule}, wallet::WalletTransparentOutput, }, zcash_primitives::{ block::BlockHash, - transaction::{ - components::{OutPoint, TxOut}, - fees::StandardFeeRule, - }, + transaction::components::{OutPoint, TxOut}, }, zcash_protocol::value::Zatoshis, }; diff --git a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs index 987f50f6a9..cfae9d438d 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/receiving_key_scopes.rs @@ -405,6 +405,7 @@ mod tests { OsRng, &prover, &prover, + #[allow(deprecated)] &fixed::FeeRule::non_standard(NonNegativeAmount::ZERO), ) .unwrap(); diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 321b1fcabd..046b81f6b2 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -401,7 +401,7 @@ pub(crate) mod tests { #[test] fn send_with_multiple_change_outputs() { - testing::pool::send_with_multiple_change_outputs::() + testing::pool::send_with_multiple_change_outputs::() } #[test] @@ -417,13 +417,11 @@ pub(crate) mod tests { } #[test] - #[allow(deprecated)] fn create_to_address_fails_on_incorrect_usk() { testing::pool::create_to_address_fails_on_incorrect_usk::() } #[test] - #[allow(deprecated)] fn proposal_fails_with_no_blocks() { testing::pool::proposal_fails_with_no_blocks::() } diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index b4df74fbe4..c9cf157a4a 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -430,13 +430,11 @@ pub(crate) mod tests { } #[test] - #[allow(deprecated)] fn create_to_address_fails_on_incorrect_usk() { testing::pool::create_to_address_fails_on_incorrect_usk::() } #[test] - #[allow(deprecated)] fn proposal_fails_with_no_blocks() { testing::pool::proposal_fails_with_no_blocks::() } diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index fd4fac0af5..c41ed06849 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -599,10 +599,10 @@ pub(crate) mod tests { testing::orchard::OrchardPoolTester, wallet::input_selection::GreedyInputSelector, WalletCommitmentTrees, }, - fees::{standard, DustOutputPolicy}, + fees::{standard, DustOutputPolicy, StandardFeeRule}, wallet::OvkPolicy, }, - zcash_primitives::{memo::Memo, transaction::fees::StandardFeeRule}, + zcash_primitives::memo::Memo, }; #[test] diff --git a/zcash_extensions/src/transparent/demo.rs b/zcash_extensions/src/transparent/demo.rs index 597bda6a5b..42b29d2b4b 100644 --- a/zcash_extensions/src/transparent/demo.rs +++ b/zcash_extensions/src/transparent/demo.rs @@ -493,7 +493,7 @@ mod tests { amount::{Amount, NonNegativeAmount}, tze::{Authorized, Bundle, OutPoint, TzeIn, TzeOut}, }, - fees::fixed, + fees::{fixed, zip317::MINIMUM_FEE}, Transaction, TransactionData, TxVersion, }, }; @@ -794,7 +794,7 @@ mod tests { // FIXME: implement zcash_primitives::transaction::fees::FutureFeeRule for zip317::FeeRule. #[allow(deprecated)] - let fee_rule = fixed::FeeRule::standard(); + let fee_rule = fixed::FeeRule::non_standard(MINIMUM_FEE); // create some inputs to spend let extsk = ExtendedSpendingKey::master(&[]); diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 073c1815fb..8c357adc56 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -7,8 +7,37 @@ and this library adheres to Rust's notion of ## [Unreleased] +### Added +- A new feature flag, `non-standard-fees`, has been added. This flag is now + required in order to make use of any types or methods that enable non-standard + fee calculation. + ### Changed - MSRV is now 1.77.0. +- `zcash_primitives::transaction::fees`: + - The `fixed` module has been moved behind the `non-standard-fees` feature + flag. Using a fixed fee may result in a transaction that cannot be mined on + the current Zcash network. To calculate the ZIP 317 fee, use + `zip317::FeeRule::standard()`. + - `zip317::FeeRule::non_standard` has been moved behind the `non-standard-fees` + feature flag. Using a non-standard fee may result in a transaction that cannot + be mined on the current `Zcash` network. + +### Deprecated +- `zcash_primitives::transaction::fees`: + - `StandardFeeRule` has been deprecated. It was never used within `zcash_primitives` + and should have been a member of `zcash_client_backend::fees` instead. + +### Removed +- `zcash_primitives::transaction::fees`: + - `StandardFeeRule` itself has been removed; it was not used in this crate. + Its use in `zcash_client_backend` has been replaced with + `zcash_client_backend::fees::StandardFeeRule`. + - `fixed::FeeRule::standard`. This constructor was misleadingly named: using a + fixed fee does not conform to any current Zcash standard. To calculate the + ZIP 317 fee, use `zip317::FeeRule::standard()`. To preserve the current + behaviour, use `fixed::FeeRule::non_standard(zip317::MINIMUM_FEE)`, + but note that this is likely to result in transactions that cannot be mined. ## [0.19.0] - 2024-10-02 diff --git a/zcash_primitives/Cargo.toml b/zcash_primitives/Cargo.toml index b70609c052..d2b235dbe6 100644 --- a/zcash_primitives/Cargo.toml +++ b/zcash_primitives/Cargo.toml @@ -125,6 +125,10 @@ test-dependencies = [ ## A feature used to isolate tests that are expensive to run. Test-only. expensive-tests = [] +## A feature that provides `FeeRule` implementations and constructors for +## non-standard fees. +non-standard-fees = [] + [lib] bench = false diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index b40655dc55..dda20bcec9 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -874,7 +874,6 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Extensio mod testing { use rand::RngCore; use rand_core::CryptoRng; - use std::convert::Infallible; use super::{BuildResult, Builder, Error}; use crate::{ @@ -883,13 +882,16 @@ mod testing { self, prover::mock::{MockOutputProver, MockSpendProver}, }, - transaction::fees::fixed, + transaction::fees::zip317, }; impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<'a, P, U> { /// Build the transaction using mocked randomness and proving capabilities. /// DO NOT USE EXCEPT FOR UNIT TESTING. - pub fn mock_build(self, rng: R) -> Result> { + pub fn mock_build( + self, + rng: R, + ) -> Result> { struct FakeCryptoRng(R); impl CryptoRng for FakeCryptoRng {} impl RngCore for FakeCryptoRng { @@ -910,12 +912,12 @@ mod testing { } } - #[allow(deprecated)] self.build( FakeCryptoRng(rng), &MockSpendProver, &MockOutputProver, - &fixed::FeeRule::standard(), + #[allow(deprecated)] + &zip317::FeeRule::standard(), ) } } @@ -1056,7 +1058,7 @@ mod tests { builder .add_transparent_output( &TransparentAddress::PublicKeyHash([0; 20]), - NonNegativeAmount::const_from_u64(40000), + NonNegativeAmount::const_from_u64(35000), ) .unwrap(); @@ -1170,7 +1172,7 @@ mod tests { builder .add_transparent_output( &TransparentAddress::PublicKeyHash([0; 20]), - NonNegativeAmount::const_from_u64(20000), + NonNegativeAmount::const_from_u64(15000), ) .unwrap(); assert_matches!( @@ -1189,7 +1191,7 @@ mod tests { let witness2 = IncrementalWitness::from_tree(tree); // Succeeds if there is sufficient input - // 0.0003 z-ZEC out, 0.0002 t-ZEC out, 0.0001 t-ZEC fee, 0.0006 z-ZEC in + // 0.0003 z-ZEC out, 0.00015 t-ZEC out, 0.00015 t-ZEC fee, 0.0006 z-ZEC in { let build_config = BuildConfig::Standard { sapling_anchor: Some(witness1.root().into()), @@ -1213,12 +1215,15 @@ mod tests { builder .add_transparent_output( &TransparentAddress::PublicKeyHash([0; 20]), - NonNegativeAmount::const_from_u64(20000), + NonNegativeAmount::const_from_u64(15000), ) .unwrap(); - assert_matches!( - builder.mock_build(OsRng), - Ok(res) if res.transaction().fee_paid(|_| Err(BalanceError::Overflow)).unwrap() == Amount::const_from_i64(10_000) + let res = builder.mock_build(OsRng).unwrap(); + assert_eq!( + res.transaction() + .fee_paid(|_| Err(BalanceError::Overflow)) + .unwrap(), + Amount::const_from_i64(15_000) ); } } diff --git a/zcash_primitives/src/transaction/fees.rs b/zcash_primitives/src/transaction/fees.rs index ae996de6ce..4a4cb6aa8b 100644 --- a/zcash_primitives/src/transaction/fees.rs +++ b/zcash_primitives/src/transaction/fees.rs @@ -5,6 +5,7 @@ use crate::{ transaction::{components::amount::NonNegativeAmount, fees::transparent::InputSize}, }; +#[cfg(feature = "non-standard-fees")] pub mod fixed; pub mod transparent; pub mod zip317; @@ -58,47 +59,3 @@ 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_input_sizes: impl IntoIterator, - transparent_output_sizes: impl IntoIterator, - 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_input_sizes, - transparent_output_sizes, - sapling_input_count, - sapling_output_count, - orchard_action_count, - ), - } - } -} diff --git a/zcash_primitives/src/transaction/fees/fixed.rs b/zcash_primitives/src/transaction/fees/fixed.rs index 82e8301ee0..56fc9a1ecd 100644 --- a/zcash_primitives/src/transaction/fees/fixed.rs +++ b/zcash_primitives/src/transaction/fees/fixed.rs @@ -1,7 +1,7 @@ use crate::{ consensus::{self, BlockHeight}, transaction::components::amount::NonNegativeAmount, - transaction::fees::{transparent, zip317}, + transaction::fees::transparent, }; #[cfg(zcash_unstable = "zfuture")] @@ -20,25 +20,6 @@ impl FeeRule { Self { fixed_fee } } - /// Creates a new fixed fee rule with the minimum possible [ZIP 317] fee, - /// i.e. 10000 zatoshis. - /// - /// Note that using a fixed fee is not compliant with [ZIP 317]; consider - /// using [`zcash_primitives::transaction::fees::zip317::FeeRule::standard()`] - /// instead. - /// - /// [`zcash_primitives::transaction::fees::zip317::FeeRule::standard()`]: crate::transaction::fees::zip317::FeeRule::standard - /// [ZIP 317]: https://zips.z.cash/zip-0317 - #[deprecated( - since = "0.12.0", - note = "To calculate the ZIP 317 fee, use `transaction::fees::zip317::FeeRule::standard()`. For a fixed fee, use the `non_standard` constructor." - )] - pub fn standard() -> Self { - Self { - fixed_fee: zip317::MINIMUM_FEE, - } - } - /// Returns the fixed fee amount which this rule was configured. pub fn fixed_fee(&self) -> NonNegativeAmount { self.fixed_fee diff --git a/zcash_primitives/src/transaction/fees/zip317.rs b/zcash_primitives/src/transaction/fees/zip317.rs index 48457b8007..63c8089d07 100644 --- a/zcash_primitives/src/transaction/fees/zip317.rs +++ b/zcash_primitives/src/transaction/fees/zip317.rs @@ -76,15 +76,17 @@ impl FeeRule { /// Construct a new FeeRule instance with the specified parameter values. /// + /// Using this fee rule with + /// ```compile_fail + /// marginal_fee < 5000 || grace_actions < 2 \ + /// || p2pkh_standard_input_size > P2PKH_STANDARD_INPUT_SIZE \ + /// || p2pkh_standard_output_size > P2PKH_STANDARD_OUTPUT_SIZE + /// ``` + /// violates ZIP 317, and might cause transactions built with it to fail. + /// /// Returns `None` if either `p2pkh_standard_input_size` or `p2pkh_standard_output_size` are /// zero. - #[deprecated( - note = "Using this fee rule with `marginal_fee < 5000 || grace_actions < 2 \ - || p2pkh_standard_input_size > P2PKH_STANDARD_INPUT_SIZE \ - || p2pkh_standard_output_size > P2PKH_STANDARD_OUTPUT_SIZE` \ - violates ZIP 317, and might cause transactions built with it to fail. \ - This API is likely to be removed. Use `[FeeRule::standard]` instead." - )] + #[cfg(feature = "non-standard-fees")] pub fn non_standard( marginal_fee: NonNegativeAmount, grace_actions: usize,