zcash_client_backend: Detect Orchard dust in `zip317::SingleOutputChangeStrategy`

This commit is contained in:
Jack Grigg 2024-03-11 00:56:54 +00:00
parent 8e09b78ca1
commit 5a6057b8fb
5 changed files with 166 additions and 34 deletions

View File

@ -76,6 +76,8 @@ and this library adheres to Rust's notion of
constraint on its `<AccountId>` parameter has been strengthened to `Copy`.
- `zcash_client_backend::fees`:
- Arguments to `ChangeStrategy::compute_balance` have changed.
- `ChangeError::DustInputs` now has an `orchard` field behind the `orchard`
feature flag.
- `zcash_client_backend::proto`:
- `ProposalDecodingError` has a new variant `TransparentMemo`.
- `zcash_client_backend::zip321::render::amount_str` now takes a

View File

@ -445,8 +445,15 @@ where
)
.map_err(InputSelectorError::Proposal);
}
Err(ChangeError::DustInputs { mut sapling, .. }) => {
Err(ChangeError::DustInputs {
mut sapling,
#[cfg(feature = "orchard")]
mut orchard,
..
}) => {
exclude.append(&mut sapling);
#[cfg(feature = "orchard")]
exclude.append(&mut orchard);
}
Err(ChangeError::InsufficientFunds { required, .. }) => {
amount_required = required;

View File

@ -149,6 +149,9 @@ pub enum ChangeError<E, NoteRefT> {
transparent: Vec<OutPoint>,
/// The identifiers for Sapling inputs having no current economic value
sapling: Vec<NoteRefT>,
/// The identifiers for Orchard inputs having no current economic value
#[cfg(feature = "orchard")]
orchard: Vec<NoteRefT>,
},
/// An error occurred that was specific to the change selection strategy in use.
StrategyError(E),
@ -169,9 +172,13 @@ impl<E, NoteRefT> ChangeError<E, NoteRefT> {
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),
@ -194,10 +201,21 @@ impl<CE: fmt::Display, N: fmt::Display> fmt::Display for ChangeError<CE, N> {
ChangeError::DustInputs {
transparent,
sapling,
#[cfg(feature = "orchard")]
orchard,
} => {
#[cfg(feature = "orchard")]
let orchard_len = orchard.len();
#[cfg(not(feature = "orchard"))]
let orchard_len = 0;
// we can't encode the UA to its string representation because we
// don't have network parameters here
write!(f, "Insufficient funds: {} dust inputs were present, but would cost more to spend than they are worth.", transparent.len() + sapling.len())
write!(
f,
"Insufficient funds: {} dust inputs were present, but would cost more to spend than they are worth.",
transparent.len() + sapling.len() + orchard_len,
)
}
ChangeError::StrategyError(err) => {
write!(f, "{}", err)

View File

@ -17,30 +17,26 @@ use super::{
#[cfg(feature = "orchard")]
use super::orchard as orchard_fees;
pub(crate) struct NetFlows {
t_in: NonNegativeAmount,
t_out: NonNegativeAmount,
sapling_in: NonNegativeAmount,
sapling_out: NonNegativeAmount,
orchard_in: NonNegativeAmount,
orchard_out: NonNegativeAmount,
}
#[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,
target_height: BlockHeight,
pub(crate) fn calculate_net_flows<NoteRefT: Clone, F: FeeRule, 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>,
dust_output_policy: &DustOutputPolicy,
default_dust_threshold: NonNegativeAmount,
change_memo: Option<MemoBytes>,
_fallback_change_pool: ShieldedProtocol,
) -> Result<TransactionBalance, ChangeError<E, NoteRefT>>
) -> Result<NetFlows, ChangeError<E, NoteRefT>>
where
E: From<F::Error> + From<BalanceError>,
{
let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow));
let underflow = || ChangeError::StrategyError(E::from(BalanceError::Underflow));
let t_in = transparent_inputs
.iter()
@ -85,13 +81,30 @@ where
#[cfg(not(feature = "orchard"))]
let orchard_out = NonNegativeAmount::ZERO;
Ok(NetFlows {
t_in,
t_out,
sapling_in,
sapling_out,
orchard_in,
orchard_out,
})
}
pub(crate) fn single_change_output_policy<NoteRefT: Clone, F: FeeRule, E>(
_net_flows: &NetFlows,
_fallback_change_pool: ShieldedProtocol,
) -> Result<(ShieldedProtocol, usize, usize), ChangeError<E, NoteRefT>>
where
E: From<F::Error> + From<BalanceError>,
{
// TODO: implement a less naive strategy for selecting the pool to which change will be sent.
#[cfg(feature = "orchard")]
let (change_pool, sapling_change, orchard_change) =
if orchard_in.is_positive() || orchard_out.is_positive() {
if _net_flows.orchard_in.is_positive() || _net_flows.orchard_out.is_positive() {
// Send change to Orchard if we're spending any Orchard inputs or creating any Orchard outputs
(ShieldedProtocol::Orchard, 0, 1)
} else if sapling_in.is_positive() || sapling_out.is_positive() {
} else if _net_flows.sapling_in.is_positive() || _net_flows.sapling_out.is_positive() {
// Otherwise, send change to Sapling if we're spending any Sapling inputs or creating any
// Sapling outputs, so that we avoid pool-crossing.
(ShieldedProtocol::Sapling, 1, 0)
@ -104,7 +117,45 @@ where
}
};
#[cfg(not(feature = "orchard"))]
let (change_pool, sapling_change) = (ShieldedProtocol::Sapling, 1);
let (change_pool, sapling_change, orchard_change) = (ShieldedProtocol::Sapling, 1, 0);
Ok((change_pool, sapling_change, orchard_change))
}
#[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,
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,
) -> Result<TransactionBalance, ChangeError<E, NoteRefT>>
where
E: From<F::Error> + From<BalanceError>,
{
let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow));
let underflow = || ChangeError::StrategyError(E::from(BalanceError::Underflow));
let net_flows = calculate_net_flows::<NoteRefT, F, E>(
transparent_inputs,
transparent_outputs,
sapling,
#[cfg(feature = "orchard")]
orchard,
)?;
let (change_pool, sapling_change, _orchard_change) =
single_change_output_policy::<NoteRefT, F, E>(&net_flows, _fallback_change_pool)?;
let sapling_input_count = sapling
.bundle_type()
@ -123,7 +174,7 @@ where
.bundle_type()
.num_actions(
orchard.inputs().len(),
orchard.outputs().len() + orchard_change,
orchard.outputs().len() + _orchard_change,
)
.map_err(ChangeError::BundleError)?;
#[cfg(not(feature = "orchard"))]
@ -141,8 +192,10 @@ where
)
.map_err(|fee_error| ChangeError::StrategyError(E::from(fee_error)))?;
let total_in = (t_in + sapling_in + orchard_in).ok_or_else(overflow)?;
let total_out = (t_out + sapling_out + orchard_out + fee_amount).ok_or_else(overflow)?;
let total_in =
(net_flows.t_in + net_flows.sapling_in + net_flows.orchard_in).ok_or_else(overflow)?;
let total_out = (net_flows.t_out + net_flows.sapling_out + net_flows.orchard_out + fee_amount)
.ok_or_else(overflow)?;
let proposed_change = (total_in - total_out).ok_or(ChangeError::InsufficientFunds {
available: total_in,

View File

@ -16,8 +16,8 @@ use zcash_primitives::{
use crate::ShieldedProtocol;
use super::{
common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy,
DustOutputPolicy, TransactionBalance,
common::{calculate_net_flows, single_change_output_balance, single_change_output_policy},
sapling as sapling_fees, ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance,
};
#[cfg(feature = "orchard")]
@ -93,32 +93,73 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
})
.collect();
#[cfg(feature = "orchard")]
let mut orchard_dust: Vec<NoteRefT> = orchard
.inputs()
.iter()
.filter_map(|i| {
if orchard_fees::InputView::<NoteRefT>::value(i) < self.fee_rule.marginal_fee() {
Some(orchard_fees::InputView::<NoteRefT>::note_id(i).clone())
} else {
None
}
})
.collect();
#[cfg(not(feature = "orchard"))]
let mut orchard_dust: Vec<NoteRefT> = vec![];
// Depending on the shape of the transaction, we may be able to spend up to
// `grace_actions - 1` dust inputs. If we don't have any dust inputs though,
// we don't need to worry about any of that.
if !(transparent_dust.is_empty() && sapling_dust.is_empty()) {
if !(transparent_dust.is_empty() && sapling_dust.is_empty() && orchard_dust.is_empty()) {
let t_non_dust = transparent_inputs.len() - transparent_dust.len();
let t_allowed_dust = transparent_outputs.len().saturating_sub(t_non_dust);
// We add one to the sapling outputs for the (single) change output Note that this
// means that wallet-internal shielding transactions are an opportunity to spend a dust
// note.
// We add one to either the Sapling or Orchard outputs for the (single)
// change output. Note that this means that wallet-internal shielding
// transactions are an opportunity to spend a dust note.
let net_flows = calculate_net_flows::<NoteRefT, Self::FeeRule, Self::Error>(
transparent_inputs,
transparent_outputs,
sapling,
#[cfg(feature = "orchard")]
orchard,
)?;
let (_, sapling_change, orchard_change) =
single_change_output_policy::<NoteRefT, Self::FeeRule, Self::Error>(
&net_flows,
self.fallback_change_pool,
)?;
let s_non_dust = sapling.inputs().len() - sapling_dust.len();
let s_allowed_dust = (sapling.outputs().len() + 1).saturating_sub(s_non_dust);
let s_allowed_dust =
(sapling.outputs().len() + sapling_change).saturating_sub(s_non_dust);
#[cfg(feature = "orchard")]
let (orchard_inputs_len, orchard_outputs_len) =
(orchard.inputs().len(), orchard.outputs().len());
#[cfg(not(feature = "orchard"))]
let (orchard_inputs_len, orchard_outputs_len) = (0, 0);
let o_non_dust = orchard_inputs_len - orchard_dust.len();
let o_allowed_dust = (orchard_outputs_len + orchard_change).saturating_sub(o_non_dust);
let available_grace_inputs = self
.fee_rule
.grace_actions()
.saturating_sub(t_non_dust)
.saturating_sub(s_non_dust);
.saturating_sub(s_non_dust)
.saturating_sub(o_non_dust);
let mut t_disallowed_dust = transparent_dust.len().saturating_sub(t_allowed_dust);
let mut s_disallowed_dust = sapling_dust.len().saturating_sub(s_allowed_dust);
let mut o_disallowed_dust = orchard_dust.len().saturating_sub(o_allowed_dust);
if available_grace_inputs > 0 {
// If we have available grace inputs, allocate them first to transparent dust
// and then to Sapling dust. The caller has provided inputs that it is willing
// to spend, so we don't need to consider privacy effects at this layer.
// and then to Sapling dust followed by Orchard dust. The caller has provided
// inputs that it is willing to spend, so we don't need to consider privacy
// effects at this layer.
let t_grace_dust = available_grace_inputs.saturating_sub(t_disallowed_dust);
t_disallowed_dust = t_disallowed_dust.saturating_sub(t_grace_dust);
@ -126,6 +167,12 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
.saturating_sub(t_grace_dust)
.saturating_sub(s_disallowed_dust);
s_disallowed_dust = s_disallowed_dust.saturating_sub(s_grace_dust);
let o_grace_dust = available_grace_inputs
.saturating_sub(t_grace_dust)
.saturating_sub(s_grace_dust)
.saturating_sub(o_disallowed_dust);
o_disallowed_dust = o_disallowed_dust.saturating_sub(o_grace_dust);
}
// Truncate the lists of inputs to be disregarded in input selection to just the
@ -135,11 +182,16 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
transparent_dust.truncate(t_disallowed_dust);
sapling_dust.reverse();
sapling_dust.truncate(s_disallowed_dust);
orchard_dust.reverse();
orchard_dust.truncate(o_disallowed_dust);
if !(transparent_dust.is_empty() && sapling_dust.is_empty()) {
if !(transparent_dust.is_empty() && sapling_dust.is_empty() && orchard_dust.is_empty())
{
return Err(ChangeError::DustInputs {
transparent: transparent_dust,
sapling: sapling_dust,
#[cfg(feature = "orchard")]
orchard: orchard_dust,
});
}
}