From 9a58a1dafe541f91504db492ea0d0fd9abc67840 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 7 Oct 2024 14:58:50 -0600 Subject: [PATCH] zcash_client_backend: Clean up arguments to `single_change_output_balance` --- zcash_client_backend/src/fees/common.rs | 66 ++++++++++++++++++------- zcash_client_backend/src/fees/fixed.rs | 21 +++++--- zcash_client_backend/src/fees/zip317.rs | 21 +++++--- 3 files changed, 73 insertions(+), 35 deletions(-) diff --git a/zcash_client_backend/src/fees/common.rs b/zcash_client_backend/src/fees/common.rs index 99dcf046b..59600cc9b 100644 --- a/zcash_client_backend/src/fees/common.rs +++ b/zcash_client_backend/src/fees/common.rs @@ -164,6 +164,38 @@ impl OutputManifest { } } +pub(crate) struct SinglePoolBalanceConfig<'a, P, F> { + params: &'a P, + fee_rule: &'a F, + dust_output_policy: &'a DustOutputPolicy, + default_dust_threshold: NonNegativeAmount, + fallback_change_pool: ShieldedProtocol, + marginal_fee: NonNegativeAmount, + grace_actions: usize, +} + +impl<'a, P, F> SinglePoolBalanceConfig<'a, P, F> { + pub(crate) fn new( + params: &'a P, + fee_rule: &'a F, + dust_output_policy: &'a DustOutputPolicy, + default_dust_threshold: NonNegativeAmount, + fallback_change_pool: ShieldedProtocol, + marginal_fee: NonNegativeAmount, + grace_actions: usize, + ) -> Self { + Self { + params, + fee_rule, + dust_output_policy, + default_dust_threshold, + fallback_change_pool, + marginal_fee, + grace_actions, + } + } +} + #[allow(clippy::too_many_arguments)] pub(crate) fn single_change_output_balance< P: consensus::Parameters, @@ -171,19 +203,13 @@ pub(crate) fn single_change_output_balance< F: FeeRule, E, >( - params: &P, - fee_rule: &F, + cfg: SinglePoolBalanceConfig, 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, - dust_output_policy: &DustOutputPolicy, - default_dust_threshold: NonNegativeAmount, change_memo: Option<&MemoBytes>, - fallback_change_pool: ShieldedProtocol, - marginal_fee: NonNegativeAmount, - grace_actions: usize, ephemeral_balance: Option<&EphemeralBalance>, ) -> Result> where @@ -208,7 +234,7 @@ where #[allow(unused_variables)] let (change_pool, sapling_change, orchard_change) = - single_change_output_policy(&net_flows, fallback_change_pool); + single_change_output_policy(&net_flows, cfg.fallback_change_pool); // We don't create a fully-transparent transaction if a change memo is used. let transparent = net_flows.is_transparent() && change_memo.is_none(); @@ -216,13 +242,13 @@ where // If we have a non-zero marginal fee, we need to check for uneconomic inputs. // This is basically assuming that fee rules with non-zero marginal fee are // "ZIP 317-like", but we can generalize later if needed. - if marginal_fee.is_positive() { + if cfg.marginal_fee.is_positive() { // Is it certain that there will be a change output? If it is not certain, // we should call `check_for_uneconomic_inputs` with `possible_change` // including both possibilities. let possible_change = // These are the situations where we might not have a change output. - if transparent || (dust_output_policy.action() == DustAction::AddDustToFee && change_memo.is_none()) { + if transparent || (cfg.dust_output_policy.action() == DustAction::AddDustToFee && change_memo.is_none()) { vec![ OutputManifest::ZERO, OutputManifest { @@ -241,8 +267,8 @@ where sapling, #[cfg(feature = "orchard")] orchard, - marginal_fee, - grace_actions, + cfg.marginal_fee, + cfg.grace_actions, &possible_change[..], ephemeral_balance, )?; @@ -331,9 +357,10 @@ where .map(|_| P2PKH_STANDARD_OUTPUT_SIZE), ); - let fee_without_change = fee_rule + let fee_without_change = cfg + .fee_rule .fee_required( - params, + cfg.params, target_height, transparent_input_sizes.clone(), transparent_output_sizes.clone(), @@ -345,9 +372,9 @@ where let fee_with_change = max( fee_without_change, - fee_rule + cfg.fee_rule .fee_required( - params, + cfg.params, target_height, transparent_input_sizes, transparent_output_sizes, @@ -394,12 +421,13 @@ where ) }; - let change_dust_threshold = dust_output_policy + let change_dust_threshold = cfg + .dust_output_policy .dust_threshold() - .unwrap_or(default_dust_threshold); + .unwrap_or(cfg.default_dust_threshold); if proposed_change < change_dust_threshold { - match dust_output_policy.action() { + match cfg.dust_output_policy.action() { DustAction::Reject => { // Always allow zero-valued change even for the `Reject` policy: // * it should be allowed in order to record change memos and to improve diff --git a/zcash_client_backend/src/fees/fixed.rs b/zcash_client_backend/src/fees/fixed.rs index 48da67d53..2d2938cff 100644 --- a/zcash_client_backend/src/fees/fixed.rs +++ b/zcash_client_backend/src/fees/fixed.rs @@ -14,8 +14,9 @@ use zcash_primitives::{ use crate::{data_api::InputSource, ShieldedProtocol}; use super::{ - common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy, - DustOutputPolicy, EphemeralBalance, TransactionBalance, + common::{single_change_output_balance, SinglePoolBalanceConfig}, + sapling as sapling_fees, ChangeError, ChangeStrategy, DustOutputPolicy, EphemeralBalance, + TransactionBalance, }; #[cfg(feature = "orchard")] @@ -85,21 +86,25 @@ impl ChangeStrategy for SingleOutputChangeStrategy { ephemeral_balance: Option<&EphemeralBalance>, _wallet_meta: Option<&Self::WalletMeta>, ) -> Result> { - single_change_output_balance( + let cfg = SinglePoolBalanceConfig::new( params, &self.fee_rule, + &self.dust_output_policy, + self.fee_rule.fixed_fee(), + self.fallback_change_pool, + NonNegativeAmount::ZERO, + 0, + ); + + single_change_output_balance( + cfg, target_height, transparent_inputs, transparent_outputs, sapling, #[cfg(feature = "orchard")] orchard, - &self.dust_output_policy, - self.fee_rule.fixed_fee(), self.change_memo.as_ref(), - self.fallback_change_pool, - NonNegativeAmount::ZERO, - 0, ephemeral_balance, ) } diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs index 56f79cca6..673dc7cf7 100644 --- a/zcash_client_backend/src/fees/zip317.rs +++ b/zcash_client_backend/src/fees/zip317.rs @@ -18,8 +18,9 @@ use zcash_primitives::{ use crate::{data_api::InputSource, ShieldedProtocol}; use super::{ - common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy, - DustOutputPolicy, EphemeralBalance, TransactionBalance, + common::{single_change_output_balance, SinglePoolBalanceConfig}, + sapling as sapling_fees, ChangeError, ChangeStrategy, DustOutputPolicy, EphemeralBalance, + TransactionBalance, }; #[cfg(feature = "orchard")] @@ -89,21 +90,25 @@ impl ChangeStrategy for SingleOutputChangeStrategy { ephemeral_balance: Option<&EphemeralBalance>, _wallet_meta: Option<&Self::WalletMeta>, ) -> Result> { - single_change_output_balance( + let cfg = SinglePoolBalanceConfig::new( params, &self.fee_rule, + &self.dust_output_policy, + self.fee_rule.marginal_fee(), + self.fallback_change_pool, + self.fee_rule.marginal_fee(), + self.fee_rule.grace_actions(), + ); + + single_change_output_balance( + cfg, target_height, transparent_inputs, transparent_outputs, sapling, #[cfg(feature = "orchard")] orchard, - &self.dust_output_policy, - self.fee_rule.marginal_fee(), self.change_memo.as_ref(), - self.fallback_change_pool, - self.fee_rule.marginal_fee(), - self.fee_rule.grace_actions(), ephemeral_balance, ) }