zec
/
librustzcash
mirror of https://github.com/zcash/librustzcash.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Restore the logic to determine whether we are spending inputs that are 

uneconomic to spend.

Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>
This commit is contained in:
Daira-Emma Hopwood 2024-06-28 17:00:37 +01:00
parent feabe6df93
commit baccb4361b
4 changed files with 269 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
zcash_client_backend/src/fees.rs
View File

@ -206,15 +206,25 @@ pub enum ChangeError<E, NoteRefT> {
/// including the required fees.
required: NonNegativeAmount,
},
/// Some of the inputs provided to the transaction were determined to currently have no
/// economic value (i.e. their inclusion in a transaction causes fees to rise in an amount
/// greater than their value.)
/// Some of the inputs provided to the transaction have value less than the
/// marginal fee, and could not be determined to have any economic value in
/// the context of this input selection.
///
/// This determination is potentially conservative in the sense that inputs
/// with value less than or equal to the marginal fee might be excluded, even
/// though in practice they would not cause the fee to increase. Inputs with
/// value greater than the marginal fee will never be excluded.
///
/// The ordering of the inputs in each list is unspecified.
DustInputs {
/// The outpoints corresponding to transparent inputs having no current economic value.
/// The outpoints for transparent inputs that could not be determined to
/// have economic value in the context of this input selection.
transparent: Vec<OutPoint>,
/// The identifiers for Sapling inputs having no current economic value
/// The identifiers for Sapling inputs that could not be determined to
/// have economic value in the context of this input selection.
sapling: Vec<NoteRefT>,
/// The identifiers for Orchard inputs having no current economic value
/// The identifiers for Orchard inputs that could not be determined to
/// have economic value in the context of this input selection.
#[cfg(feature = "orchard")]
orchard: Vec<NoteRefT>,
},
@ -416,6 +426,13 @@ pub trait ChangeStrategy {
/// supply the requested outputs and required fees, implementations should return
/// [`ChangeError::InsufficientFunds`].
///
/// If the inputs include notes or UTXOs that are not economic to spend in the context
/// of this input selection, a [`ChangeError::DustInputs`] error can be returned
/// indicating inputs that should be removed from the selection (all of which will
/// have value less than or equal to the marginal fee). The caller should order the
/// inputs from most to least preferred to spend within each pool, so that the most
/// preferred ones are less likely to be indicated to remove.
///
#[cfg_attr(
feature = "transparent-inputs",
doc = "`ephemeral_parameters` can be used to specify variations on how balance

247
zcash_client_backend/src/fees/common.rs
View File

@ -1,4 +1,4 @@
use core::cmp::max;
use core::cmp::{max, min};
use zcash_primitives::{
consensus::{self, BlockHeight},
@ -171,6 +171,8 @@ pub(crate) fn single_change_output_balance<
default_dust_threshold: NonNegativeAmount,
change_memo: Option<&MemoBytes>,
fallback_change_pool: ShieldedProtocol,
marginal_fee: NonNegativeAmount,
grace_actions: usize,
#[cfg(feature = "transparent-inputs")] ephemeral_parameters: &EphemeralParameters,
) -> Result<TransactionBalance, ChangeError<E, NoteRefT>>
where
@ -193,6 +195,43 @@ where
#[cfg(feature = "transparent-inputs")]
ephemeral_parameters.ephemeral_output_amount(),
)?;
#[allow(unused_variables)]
let (change_pool, sapling_change, orchard_change) =
single_change_output_policy(&net_flows, 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() {
// 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()) {
vec![(0, 0, 0), (0, sapling_change, orchard_change)]
} else {
vec![(0, sapling_change, orchard_change)]
};
check_for_uneconomic_inputs(
transparent_inputs,
transparent_outputs,
sapling,
#[cfg(feature = "orchard")]
orchard,
marginal_fee,
grace_actions,
&possible_change[..],
#[cfg(feature = "transparent-inputs")]
ephemeral_parameters,
)?;
}
let total_in = net_flows
.total_in()
.map_err(|e| ChangeError::StrategyError(E::from(e)))?;
@ -200,10 +239,6 @@ where
.total_out()
.map_err(|e| ChangeError::StrategyError(E::from(e)))?;
#[allow(unused_variables)]
let (change_pool, sapling_change, orchard_change) =
single_change_output_policy(&net_flows, fallback_change_pool);
let sapling_input_count = sapling
.bundle_type()
.num_spends(sapling.inputs().len())
@ -305,9 +340,6 @@ where
.map_err(|fee_error| ChangeError::StrategyError(E::from(fee_error)))?,
);
// We don't create a fully-transparent transaction if a change memo is used.
let transparent = net_flows.is_transparent() && change_memo.is_none();
let total_out_plus_fee_without_change =
(total_out + fee_without_change).ok_or_else(overflow)?;
let total_out_plus_fee_with_change = (total_out + fee_with_change).ok_or_else(overflow)?;
@ -344,11 +376,11 @@ where
)
};
let dust_threshold = dust_output_policy
let change_dust_threshold = dust_output_policy
.dust_threshold()
.unwrap_or(default_dust_threshold);
if proposed_change < dust_threshold {
if proposed_change < change_dust_threshold {
match dust_output_policy.action() {
DustAction::Reject => {
// Always allow zero-valued change even for the `Reject` policy:
@ -361,7 +393,7 @@ where
simple_case()
} else {
let shortfall =
(dust_threshold - proposed_change).ok_or_else(underflow)?;
(change_dust_threshold - proposed_change).ok_or_else(underflow)?;
return Err(ChangeError::InsufficientFunds {
available: total_in,
@ -414,3 +446,196 @@ where
TransactionBalance::new(change, fee).map_err(|_| overflow())
}
/// Returns a `[ChangeStrategy::DustInputs]` error if some of the inputs provided
/// to the transaction have value less than the marginal fee, and could not be
/// determined to have any economic value in the context of this input selection.
///
/// This determination is potentially conservative in the sense that outputs
/// with value less than the marginal fee might be excluded, even though in
/// practice they would not cause the fee to increase. Outputs with value
/// greater than the marginal fee will never be excluded.
#[allow(clippy::too_many_arguments)]
pub(crate) fn check_for_uneconomic_inputs<NoteRefT: Clone, E>(
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>,
marginal_fee: NonNegativeAmount,
grace_actions: usize,
possible_change: &[(usize, usize, usize)],
#[cfg(feature = "transparent-inputs")] ephemeral_parameters: &EphemeralParameters,
) -> Result<(), ChangeError<E, NoteRefT>> {
let mut t_dust: Vec<_> = transparent_inputs
.iter()
.filter_map(|i| {
// For now, we're just assuming P2PKH inputs, so we don't check the
// size of the input script.
if i.coin().value <= marginal_fee {
Some(i.outpoint().clone())
} else {
None
}
})
.collect();
let mut s_dust: Vec<_> = sapling
.inputs()
.iter()
.filter_map(|i| {
if sapling_fees::InputView::<NoteRefT>::value(i) <= marginal_fee {
Some(sapling_fees::InputView::<NoteRefT>::note_id(i).clone())
} else {
None
}
})
.collect();
#[cfg(feature = "orchard")]
let mut o_dust: Vec<NoteRefT> = orchard
.inputs()
.iter()
.filter_map(|i| {
if orchard_fees::InputView::<NoteRefT>::value(i) <= marginal_fee {
Some(orchard_fees::InputView::<NoteRefT>::note_id(i).clone())
} else {
None
}
})
.collect();
#[cfg(not(feature = "orchard"))]
let mut o_dust: Vec<NoteRefT> = vec![];
// If we don't have any dust inputs, there is nothing to check.
if t_dust.is_empty() && s_dust.is_empty() && o_dust.is_empty() {
return Ok(());
}
let (t_inputs_len, t_outputs_len) = (transparent_inputs.len(), transparent_outputs.len());
#[cfg(feature = "transparent-inputs")]
let (t_inputs_len, t_outputs_len) = (
t_inputs_len + usize::from(ephemeral_parameters.ephemeral_input_amount().is_some()),
t_outputs_len + usize::from(ephemeral_parameters.ephemeral_output_amount().is_some()),
);
let (s_inputs_len, s_outputs_len) = (sapling.inputs().len(), sapling.outputs().len());
#[cfg(feature = "orchard")]
let (o_inputs_len, o_outputs_len) = (orchard.inputs().len(), orchard.outputs().len());
#[cfg(not(feature = "orchard"))]
let (o_inputs_len, o_outputs_len) = (0usize, 0usize);
let t_non_dust = t_inputs_len.checked_sub(t_dust.len()).unwrap();
let s_non_dust = s_inputs_len.checked_sub(s_dust.len()).unwrap();
let o_non_dust = o_inputs_len.checked_sub(o_dust.len()).unwrap();
// Return the number of allowed dust inputs from each pool.
let allowed_dust = |(t_change, s_change, o_change): &(usize, usize, usize)| {
// What is the maximum number of dust inputs in each pool, out of the ones we
// actually have, that can be economically spent along with the non-dust inputs?
// Get an initial estimate by calculating the number of dust inputs in each pool
// that will be allowed regardless of padding or grace.
let t_allowed = min(
t_dust.len(),
(t_outputs_len + t_change).saturating_sub(t_non_dust),
);
let s_allowed = min(
s_dust.len(),
(s_outputs_len + s_change).saturating_sub(s_non_dust),
);
let o_allowed = min(
o_dust.len(),
(o_outputs_len + o_change).saturating_sub(o_non_dust),
);
// We'll be spending the non-dust and allowed dust in each pool.
let t_req_inputs = t_non_dust + t_allowed;
let s_req_inputs = s_non_dust + s_allowed;
#[cfg(feature = "orchard")]
let o_req_inputs = o_non_dust + o_allowed;
// This calculates the hypothetical number of actions with given extra inputs.
// The padding rules are subtle (they also depend on `bundle_required` for example).
// To reliably tell whether we can add an extra input of a given type, we will need
// to call them both with without that input; if the number of actions does not
// increase, then the input is free to add.
let hypothetical_actions = |t_extra, s_extra, _o_extra| {
let s_spend_count = sapling
.bundle_type()
.num_spends(s_req_inputs + s_extra)
.map_err(ChangeError::BundleError)?;
let s_output_count = sapling
.bundle_type()
.num_outputs(s_req_inputs + s_extra, s_outputs_len + s_change)
.map_err(ChangeError::BundleError)?;
#[cfg(feature = "orchard")]
let o_action_count = orchard
.bundle_type()
.num_actions(o_req_inputs + _o_extra, o_outputs_len + o_change)
.map_err(ChangeError::BundleError)?;
#[cfg(not(feature = "orchard"))]
let o_action_count = 0;
// To calculate the number of unused actions, we assume that transparent inputs
// and outputs are P2PKH.
Ok(max(t_req_inputs + t_extra, t_outputs_len + t_change)
+ max(s_spend_count, s_output_count)
+ o_action_count)
};
// First calculate the baseline number of logical actions with only the definitely
// allowed inputs estimated above. If it is less than `grace_actions`, try to allocate
// a grace input first to transparent dust, then to Sapling dust, then to Orchard dust.
// If the number of actions increases, it was not possible to allocate that input for
// free. This approach is sufficient because at most one such input can be allocated,
// since `grace_actions` is at most 2 for ZIP 317 and there must be at least one
// logical action. (If `grace_actions` were greater than 2 then the code would still
// be correct, it would just not find all potential extra inputs.)
let baseline = hypothetical_actions(0, 0, 0)?;
let (t_extra, s_extra, o_extra) = if baseline >= grace_actions {
(0, 0, 0)
} else if t_dust.len() > t_allowed && hypothetical_actions(1, 0, 0)? <= baseline {
(1, 0, 0)
} else if s_dust.len() > s_allowed && hypothetical_actions(0, 1, 0)? <= baseline {
(0, 1, 0)
} else if o_dust.len() > o_allowed && hypothetical_actions(0, 0, 1)? <= baseline {
(0, 0, 1)
} else {
(0, 0, 0)
};
Ok((
t_allowed + t_extra,
s_allowed + s_extra,
o_allowed + o_extra,
))
};
// Find the least number of allowed dust inputs for each pool for any `possible_change`.
let (t_allowed, s_allowed, o_allowed) = possible_change
.iter()
.map(allowed_dust)
.collect::<Result<Vec<_>, _>>()?
.into_iter()
.reduce(|(a, b, c), (x, y, z)| (min(a, x), min(b, y), min(c, z)))
.expect("possible_change is nonempty");
// The inputs in the tail of each list after the first `*_allowed` are returned as uneconomic.
// The caller should order the inputs from most to least preferred to spend.
let t_dust = t_dust.split_off(t_allowed);
let s_dust = s_dust.split_off(s_allowed);
let o_dust = o_dust.split_off(o_allowed);
if t_dust.is_empty() && s_dust.is_empty() && o_dust.is_empty() {
Ok(())
} else {
Err(ChangeError::DustInputs {
transparent: t_dust,
sapling: s_dust,
#[cfg(feature = "orchard")]
orchard: o_dust,
})
}
}

6
zcash_client_backend/src/fees/fixed.rs
View File

@ -4,7 +4,7 @@ use zcash_primitives::{
consensus::{self, BlockHeight},
memo::MemoBytes,
transaction::{
components::amount::BalanceError,
components::amount::{BalanceError, NonNegativeAmount},
fees::{fixed::FeeRule as FixedFeeRule, transparent},
},
};
@ -78,9 +78,11 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
#[cfg(feature = "orchard")]
orchard,
dust_output_policy,
self.fee_rule().fixed_fee(),
self.fee_rule.fixed_fee(),
self.change_memo.as_ref(),
self.fallback_change_pool,
NonNegativeAmount::ZERO,
0,
#[cfg(feature = "transparent-inputs")]
ephemeral_parameters,
)

13
zcash_client_backend/src/fees/zip317.rs
View File

@ -72,7 +72,6 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
dust_output_policy: &DustOutputPolicy,
#[cfg(feature = "transparent-inputs")] ephemeral_parameters: &EphemeralParameters,
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
// TODO: consider opportunistic dust spends (#1316).
single_change_output_balance(
params,
&self.fee_rule,
@ -86,6 +85,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
self.fee_rule.marginal_fee(),
self.change_memo.as_ref(),
self.fallback_change_pool,
self.fee_rule.marginal_fee(),
self.fee_rule.grace_actions(),
#[cfg(feature = "transparent-inputs")]
ephemeral_parameters,
)
@ -472,10 +473,8 @@ mod tests {
ShieldedProtocol::Sapling,
);
// Attempt to spend three Sapling notes, one of them dust. Taking into account
// Sapling output padding, the dust note would be free to add to the transaction
// if there were only two notes (as in the `change_with_allowable_dust` test), but
// it is not free when there are three notes.
// Attempt to spend three Sapling notes, one of them dust. Adding the third
// note increases the number of actions, and so it is uneconomic to spend it.
let result = change_strategy.compute_balance(
&Network::TestNetwork,
Network::TestNetwork
@ -500,7 +499,7 @@ mod tests {
},
][..],
&[SaplingPayment::new(NonNegativeAmount::const_from_u64(
40000,
30000,
))][..],
),
#[cfg(feature = "orchard")]
@ -510,7 +509,7 @@ mod tests {
&Default::default(),
);
// We will get an error here, because the dust input now isn't free to add
// We will get an error here, because the dust input isn't free to add
// to the transaction.
assert_matches!(
result,