Skip to content

Commit

Permalink
zcash_client_backend: Clean up arguments to `single_change_output_bal…
Browse files Browse the repository at this point in the history
…ance`
  • Loading branch information
nuttycom committed Oct 17, 2024
1 parent ecb5a9b commit 5738990
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 35 deletions.
66 changes: 47 additions & 19 deletions zcash_client_backend/src/fees/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,26 +164,52 @@ 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,
NoteRefT: Clone,
F: FeeRule,
E,
>(
params: &P,
fee_rule: &F,
cfg: SinglePoolBalanceConfig<P, F>,
target_height: BlockHeight,
transparent_inputs: &[impl transparent::InputView],
transparent_outputs: &[impl transparent::OutputView],
sapling: &impl sapling_fees::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
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<TransactionBalance, ChangeError<E, NoteRefT>>
where
Expand All @@ -208,21 +234,21 @@ 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();

// 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 {
Expand All @@ -241,8 +267,8 @@ where
sapling,
#[cfg(feature = "orchard")]
orchard,
marginal_fee,
grace_actions,
cfg.marginal_fee,
cfg.grace_actions,
&possible_change[..],
ephemeral_balance,
)?;
Expand Down Expand Up @@ -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(),
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down
21 changes: 13 additions & 8 deletions zcash_client_backend/src/fees/fixed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -85,21 +86,25 @@ impl<I: InputSource> ChangeStrategy for SingleOutputChangeStrategy<I> {
ephemeral_balance: Option<&EphemeralBalance>,
_wallet_meta: Option<&Self::WalletMeta>,
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
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,
)
}
Expand Down
21 changes: 13 additions & 8 deletions zcash_client_backend/src/fees/zip317.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -89,21 +90,25 @@ impl<I: InputSource> ChangeStrategy for SingleOutputChangeStrategy<I> {
ephemeral_balance: Option<&EphemeralBalance>,
_wallet_meta: Option<&Self::WalletMeta>,
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
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,
)
}
Expand Down

0 comments on commit 5738990

Please sign in to comment.