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

Use `EphemeralBalance` instead of `EphemeralParameters` 

The `EphemeralParameters` type makes too many states representable, in
particular, it represents that it is possible for a proposal step to
have both ephemeral inputs and ephemeral outputs. This change reduces
the representable state space to make it so that only one or the other
is true, and also makes clear that the change memo must be attached to
the change output of the intermediate step, and not to the ultimate
output to the final recipient (where we expect there to be no shielded
change, and therefore it is not possible to attach a memo.)
This commit is contained in:
Kris Nuttycombe 2024-07-16 13:53:31 -06:00
parent 56aa348a41
commit aa4312326d
7 changed files with 80 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
zcash_client_backend/CHANGELOG.md
View File

@ -28,6 +28,7 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
- `chain::BlockCache` trait, behind the `sync` feature flag.
- `WalletRead::get_spendable_transparent_outputs`.
- `zcash_client_backend::fees`:
- `EphemeralBalance`
- `ChangeValue::shielded, is_ephemeral`
- `ChangeValue::ephemeral_transparent` (when "transparent-inputs" is enabled)
- `sapling::EmptyBundleView`
@ -67,10 +68,10 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
return type of `ChangeValue::output_pool` has (unconditionally) changed
from `ShieldedProtocol` to `zcash_protocol::PoolType`.
- `ChangeStrategy::compute_balance`: this trait method has an additional
`&EphemeralParameters` parameter. If the "transparent-inputs" feature is
`Option<&EphemeralBalance>` parameter. If the "transparent-inputs" feature is
enabled, this can be used to specify whether the change memo should be
ignored, and the amounts of additional transparent P2PKH inputs and
outputs. Passing `&EphemeralParameters::NONE` will retain the previous
outputs. Passing `None` will retain the previous
behaviour (and is necessary when the "transparent-inputs" feature is
not enabled).
- `zcash_client_backend::input_selection::GreedyInputSelectorError` has a

27
zcash_client_backend/src/data_api/wallet/input_selection.rs
View File

@ -23,7 +23,7 @@ use zcash_primitives::{
use crate::{
address::{Address, UnifiedAddress},
data_api::{InputSource, SimpleNoteRetention, SpendableNotes},
fees::{sapling, ChangeError, ChangeStrategy, DustOutputPolicy, EphemeralParameters},
fees::{sapling, ChangeError, ChangeStrategy, DustOutputPolicy},
proposal::{Proposal, ProposalError, ShieldedInputs},
wallet::WalletTransparentOutput,
zip321::TransactionRequest,
@ -33,7 +33,7 @@ use crate::{
#[cfg(feature = "transparent-inputs")]
use {
crate::{
fees::ChangeValue,
fees::{ChangeValue, EphemeralBalance},
proposal::{Step, StepOutput, StepOutputIndex},
zip321::Payment,
},
@ -460,12 +460,12 @@ where
}
#[cfg(not(feature = "transparent-inputs"))]
let ephemeral_parameters = EphemeralParameters::NONE;
let ephemeral_balance = None;
#[cfg(feature = "transparent-inputs")]
let (ephemeral_parameters, tr1_balance_opt) = {
let (ephemeral_balance, tr1_balance_opt) = {
if tr1_transparent_outputs.is_empty() {
(EphemeralParameters::NONE, None)
(None, None)
} else {
// The ephemeral input going into transaction 1 must be able to pay that
// transaction's fee, as well as the TEX address payments.
@ -484,7 +484,7 @@ where
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
&self.dust_output_policy,
&EphemeralParameters::new(true, Some(NonNegativeAmount::ZERO), None),
Some(&EphemeralBalance::Input(NonNegativeAmount::ZERO)),
) {
Err(ChangeError::InsufficientFunds { required, .. }) => required,
Ok(_) => NonNegativeAmount::ZERO, // shouldn't happen
@ -502,12 +502,12 @@ where
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
&self.dust_output_policy,
&EphemeralParameters::new(true, Some(tr1_required_input_value), None),
Some(&EphemeralBalance::Input(tr1_required_input_value)),
)?;
assert_eq!(tr1_balance.total(), tr1_balance.fee_required());
(
EphemeralParameters::new(false, None, Some(tr1_required_input_value)),
Some(EphemeralBalance::Output(tr1_required_input_value)),
Some(tr1_balance),
)
}
@ -582,7 +582,7 @@ where
&orchard_outputs[..],
),
&self.dust_output_policy,
&ephemeral_parameters,
ephemeral_balance.as_ref(),
);
match balance {
@ -609,8 +609,8 @@ where
assert_eq!(
*balance.proposed_change().last().expect("nonempty"),
ChangeValue::ephemeral_transparent(
ephemeral_parameters
.ephemeral_output_amount()
ephemeral_balance
.and_then(|b| b.ephemeral_output_amount())
.expect("ephemeral output is present")
)
);
@ -773,7 +773,7 @@ where
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
&self.dust_output_policy,
&EphemeralParameters::NONE,
None,
);
let balance = match trial_balance {
@ -791,8 +791,7 @@ where
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
&self.dust_output_policy,
#[cfg(feature = "transparent-inputs")]
&EphemeralParameters::NONE,
None,
)?
}
Err(other) => {

69
zcash_client_backend/src/fees.rs
View File

@ -330,61 +330,36 @@ impl Default for DustOutputPolicy {
}
}
/// `EphemeralParameters` can be used to specify variations on how balance
/// and fees are computed that are relevant to transactions using ephemeral
/// transparent outputs. These are only relevant when the "transparent-inputs"
/// feature is enabled, but to reduce feature-dependent boilerplate, the type
/// and the `EphemeralParameters::NONE` constant are present unconditionally.
/// `EphemeralBalance` describes the ephemeral input or output value for
/// a transaction. It is use in the computation of fees are relevant to transactions using
/// ephemeral transparent outputs.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct EphemeralParameters {
ignore_change_memo: bool,
ephemeral_input_amount: Option<NonNegativeAmount>,
ephemeral_output_amount: Option<NonNegativeAmount>,
pub enum EphemeralBalance {
Input(NonNegativeAmount),
Output(NonNegativeAmount),
}
impl EphemeralParameters {
/// An `EphemeralParameters` indicating no use of ephemeral inputs
/// or outputs. It has:
///
/// * `ignore_change_memo: false`,
/// * `ephemeral_input_amount: None`,
/// * `ephemeral_output_amount: None`.
pub const NONE: Self = Self {
ignore_change_memo: false,
ephemeral_input_amount: None,
ephemeral_output_amount: None,
};
/// Returns an `EphemeralParameters` with the following parameters:
///
/// * `ignore_change_memo`: `true` if the change memo should be
/// ignored for the purpose of deciding whether there will be a
/// change output.
/// * `ephemeral_input_amount`: specifies that there will be an
/// additional P2PKH input of the given amount.
/// * `ephemeral_output_amount`: specifies that there will be an
/// additional P2PKH output of the given amount.
#[cfg(feature = "transparent-inputs")]
pub const fn new(
ignore_change_memo: bool,
ephemeral_input_amount: Option<NonNegativeAmount>,
ephemeral_output_amount: Option<NonNegativeAmount>,
) -> Self {
Self {
ignore_change_memo,
ephemeral_input_amount,
ephemeral_output_amount,
}
impl EphemeralBalance {
pub fn is_input(&self) -> bool {
matches!(self, EphemeralBalance::Input(_))
}
pub fn ignore_change_memo(&self) -> bool {
self.ignore_change_memo
pub fn is_output(&self) -> bool {
matches!(self, EphemeralBalance::Output(_))
}
pub fn ephemeral_input_amount(&self) -> Option<NonNegativeAmount> {
self.ephemeral_input_amount
match self {
EphemeralBalance::Input(v) => Some(*v),
EphemeralBalance::Output(_) => None,
}
}
pub fn ephemeral_output_amount(&self) -> Option<NonNegativeAmount> {
self.ephemeral_output_amount
match self {
EphemeralBalance::Input(_) => None,
EphemeralBalance::Output(v) => Some(*v),
}
}
}
@ -433,7 +408,7 @@ pub trait ChangeStrategy {
sapling: &impl sapling::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard::BundleView<NoteRefT>,
dust_output_policy: &DustOutputPolicy,
ephemeral_parameters: &EphemeralParameters,
ephemeral_balance: Option<&EphemeralBalance>,
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>>;
}

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

@ -12,7 +12,7 @@ use zcash_protocol::ShieldedProtocol;
use super::{
sapling as sapling_fees, ChangeError, ChangeValue, DustAction, DustOutputPolicy,
EphemeralParameters, TransactionBalance,
EphemeralBalance, TransactionBalance,
};
#[cfg(feature = "orchard")]
@ -49,8 +49,7 @@ pub(crate) fn calculate_net_flows<NoteRefT: Clone, F: FeeRule, E>(
transparent_outputs: &[impl transparent::OutputView],
sapling: &impl sapling_fees::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
ephemeral_input_amount: Option<NonNegativeAmount>,
ephemeral_output_amount: Option<NonNegativeAmount>,
ephemeral_balance: Option<&EphemeralBalance>,
) -> Result<NetFlows, ChangeError<E, NoteRefT>>
where
E: From<F::Error> + From<BalanceError>,
@ -60,13 +59,13 @@ where
let t_in = transparent_inputs
.iter()
.map(|t_in| t_in.coin().value)
.chain(ephemeral_input_amount)
.chain(ephemeral_balance.and_then(|b| b.ephemeral_input_amount()))
.sum::<Option<_>>()
.ok_or_else(overflow)?;
let t_out = transparent_outputs
.iter()
.map(|t_out| t_out.value())
.chain(ephemeral_output_amount)
.chain(ephemeral_balance.and_then(|b| b.ephemeral_output_amount()))
.sum::<Option<_>>()
.ok_or_else(overflow)?;
let sapling_in = sapling
@ -162,12 +161,15 @@ pub(crate) fn single_change_output_balance<
fallback_change_pool: ShieldedProtocol,
marginal_fee: NonNegativeAmount,
grace_actions: usize,
ephemeral_parameters: &EphemeralParameters,
ephemeral_balance: Option<&EphemeralBalance>,
) -> Result<TransactionBalance, ChangeError<E, NoteRefT>>
where
E: From<F::Error> + From<BalanceError>,
{
let change_memo = change_memo.filter(|_| !ephemeral_parameters.ignore_change_memo());
// The change memo, if any, must be attached to the change in the intermediate step that
// produces the ephemeral output, and so it should be discarded in the ultimate step; this is
// distinguished by identifying that this transaction has ephemeral inputs.
let change_memo = change_memo.filter(|_| ephemeral_balance.map_or(true, |b| !b.is_input()));
let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow));
let underflow = || ChangeError::StrategyError(E::from(BalanceError::Underflow));
@ -178,8 +180,7 @@ where
sapling,
#[cfg(feature = "orchard")]
orchard,
ephemeral_parameters.ephemeral_input_amount(),
ephemeral_parameters.ephemeral_output_amount(),
ephemeral_balance,
)?;
#[allow(unused_variables)]
@ -213,7 +214,7 @@ where
marginal_fee,
grace_actions,
&possible_change[..],
ephemeral_parameters,
ephemeral_balance,
)?;
}
@ -287,16 +288,16 @@ where
.iter()
.map(|i| i.serialized_size())
.chain(
ephemeral_parameters
.ephemeral_input_amount()
ephemeral_balance
.and_then(|b| b.ephemeral_input_amount())
.map(|_| transparent::InputSize::STANDARD_P2PKH),
);
let transparent_output_sizes = transparent_outputs
.iter()
.map(|i| i.serialized_size())
.chain(
ephemeral_parameters
.ephemeral_output_amount()
ephemeral_balance
.and_then(|b| b.ephemeral_output_amount())
.map(|_| P2PKH_STANDARD_OUTPUT_SIZE),
);
@ -426,8 +427,8 @@ where
};
#[cfg(feature = "transparent-inputs")]
change.extend(
ephemeral_parameters
.ephemeral_output_amount()
ephemeral_balance
.and_then(|b| b.ephemeral_output_amount())
.map(ChangeValue::ephemeral_transparent),
);
@ -456,7 +457,7 @@ pub(crate) fn check_for_uneconomic_inputs<NoteRefT: Clone, E>(
marginal_fee: NonNegativeAmount,
grace_actions: usize,
possible_change: &[(usize, usize, usize)],
ephemeral_parameters: &EphemeralParameters,
ephemeral_balance: Option<&EphemeralBalance>,
) -> Result<(), ChangeError<E, NoteRefT>> {
let mut t_dust: Vec<_> = transparent_inputs
.iter()
@ -504,10 +505,8 @@ pub(crate) fn check_for_uneconomic_inputs<NoteRefT: Clone, E>(
}
let (t_inputs_len, t_outputs_len) = (
transparent_inputs.len()
+ usize::from(ephemeral_parameters.ephemeral_input_amount().is_some()),
transparent_outputs.len()
+ usize::from(ephemeral_parameters.ephemeral_output_amount().is_some()),
transparent_inputs.len() + usize::from(ephemeral_balance.map_or(false, |b| b.is_input())),
transparent_outputs.len() + usize::from(ephemeral_balance.map_or(false, |b| b.is_output())),
);
let (s_inputs_len, s_outputs_len) = (sapling.inputs().len(), sapling.outputs().len());
#[cfg(feature = "orchard")]

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

@ -13,7 +13,7 @@ use crate::ShieldedProtocol;
use super::{
common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy,
DustOutputPolicy, EphemeralParameters, TransactionBalance,
DustOutputPolicy, EphemeralBalance, TransactionBalance,
};
#[cfg(feature = "orchard")]
@ -63,7 +63,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
sapling: &impl sapling_fees::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
dust_output_policy: &DustOutputPolicy,
ephemeral_parameters: &EphemeralParameters,
ephemeral_balance: Option<&EphemeralBalance>,
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
single_change_output_balance(
params,
@ -80,7 +80,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
self.fallback_change_pool,
NonNegativeAmount::ZERO,
0,
ephemeral_parameters,
ephemeral_balance,
)
}
}
@ -100,7 +100,7 @@ mod tests {
data_api::wallet::input_selection::SaplingPayment,
fees::{
tests::{TestSaplingInput, TestTransparentInput},
ChangeError, ChangeStrategy, ChangeValue, DustOutputPolicy, EphemeralParameters,
ChangeError, ChangeStrategy, ChangeValue, DustOutputPolicy,
},
ShieldedProtocol,
};
@ -136,7 +136,7 @@ mod tests {
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
&EphemeralParameters::NONE,
None,
);
assert_matches!(
@ -182,7 +182,7 @@ mod tests {
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
&EphemeralParameters::NONE,
None,
);
assert_matches!(

10
zcash_client_backend/src/fees/standard.rs
View File

@ -18,7 +18,7 @@ use crate::ShieldedProtocol;
use super::{
fixed, sapling as sapling_fees, zip317, ChangeError, ChangeStrategy, DustOutputPolicy,
EphemeralParameters, TransactionBalance,
EphemeralBalance, TransactionBalance,
};
#[cfg(feature = "orchard")]
@ -68,7 +68,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
sapling: &impl sapling_fees::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
dust_output_policy: &DustOutputPolicy,
ephemeral_parameters: &EphemeralParameters,
ephemeral_balance: Option<&EphemeralBalance>,
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
#[allow(deprecated)]
match self.fee_rule() {
@ -86,7 +86,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
#[cfg(feature = "orchard")]
orchard,
dust_output_policy,
ephemeral_parameters,
ephemeral_balance,
)
.map_err(|e| e.map(Zip317FeeError::Balance)),
StandardFeeRule::Zip313 => fixed::SingleOutputChangeStrategy::new(
@ -103,7 +103,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
#[cfg(feature = "orchard")]
orchard,
dust_output_policy,
ephemeral_parameters,
ephemeral_balance,
)
.map_err(|e| e.map(Zip317FeeError::Balance)),
StandardFeeRule::Zip317 => zip317::SingleOutputChangeStrategy::new(
@ -120,7 +120,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
#[cfg(feature = "orchard")]
orchard,
dust_output_policy,
ephemeral_parameters,
ephemeral_balance,
),
}
}

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

@ -17,7 +17,7 @@ use crate::ShieldedProtocol;
use super::{
common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy,
DustOutputPolicy, EphemeralParameters, TransactionBalance,
DustOutputPolicy, EphemeralBalance, TransactionBalance,
};
#[cfg(feature = "orchard")]
@ -67,7 +67,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
sapling: &impl sapling_fees::BundleView<NoteRefT>,
#[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
dust_output_policy: &DustOutputPolicy,
ephemeral_parameters: &EphemeralParameters,
ephemeral_parameters: Option<&EphemeralBalance>,
) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
single_change_output_balance(
params,
@ -108,7 +108,6 @@ mod tests {
fees::{
tests::{TestSaplingInput, TestTransparentInput},
ChangeError, ChangeStrategy, ChangeValue, DustAction, DustOutputPolicy,
EphemeralParameters,
},
ShieldedProtocol,
};
@ -148,7 +147,7 @@ mod tests {
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
&EphemeralParameters::NONE,
None,
);
assert_matches!(
@ -192,7 +191,7 @@ mod tests {
))][..],
),
&DustOutputPolicy::default(),
&EphemeralParameters::NONE,
None,
);
assert_matches!(
@ -245,7 +244,7 @@ mod tests {
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
dust_output_policy,
&EphemeralParameters::NONE,
None,
);
assert_matches!(
@ -289,7 +288,7 @@ mod tests {
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
&EphemeralParameters::NONE,
None,
);
assert_matches!(
@ -333,7 +332,7 @@ mod tests {
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
&EphemeralParameters::NONE,
None,
);
assert_matches!(
@ -383,7 +382,7 @@ mod tests {
DustAction::AllowDustChange,
Some(NonNegativeAmount::const_from_u64(1000)),
),
&EphemeralParameters::NONE,
None,
);
assert_matches!(
@ -444,7 +443,7 @@ mod tests {
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
dust_output_policy,
&EphemeralParameters::NONE,
None,
);
assert_matches!(
@ -495,7 +494,7 @@ mod tests {
#[cfg(feature = "orchard")]
&orchard_fees::EmptyBundleView,
&DustOutputPolicy::default(),
&EphemeralParameters::NONE,
None,
);
// We will get an error here, because the dust input isn't free to add