diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 2780788b2..c2fa863e2 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -27,7 +27,8 @@ and this library adheres to Rust's notion of - `wallet::input_selection::ShieldingSelector` has been factored out from the `InputSelector` trait to separate out transparent functionality and move it behind the `transparent-inputs` feature flag. -- `zcash_client_backend::fees::{standard, sapling}` +- `zcash_client_backend::fees::{standard, orchard, sapling}` +- `zcash_client_backend::fees::ChangeValue::{new, orchard}` - `zcash_client_backend::wallet`: - `Note` - `ReceivedNote` @@ -141,9 +142,10 @@ and this library adheres to Rust's notion of for its `InputSource` associated type. - `zcash_client_backend::fees`: - - `ChangeValue::Sapling` is now a structured variant. In addition to the - existing change value, it now also carries an optional memo to be associated - with the change output. + - `ChangeStrategy::compute_balance` arguments have changed. + - `ChangeValue` is now a struct. In addition to the existing change value, it + now also provides the output pool to which change should be sent and an + optional memo to be associated with the change output. - `fixed::SingleOutputChangeStrategy::new` and `zip317::SingleOutputChangeStrategy::new` each now accept an additional `change_memo` argument. @@ -157,7 +159,6 @@ and this library adheres to Rust's notion of - `error::Error::InsufficientFunds.{available, required}` - `wallet::input_selection::InputSelectorError::InsufficientFunds.{available, required}` - `zcash_client_backend::fees`: - - `ChangeValue::Sapling.value` - `ChangeError::InsufficientFunds.{available, required}` - `zcash_client_backend::zip321::Payment.amount` - The following methods now take `NonNegativeAmount` instead of `Amount`: diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 24e10367c..20c46ec62 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -24,7 +24,7 @@ use crate::{ SentTransactionOutput, WalletCommitmentTrees, WalletRead, WalletWrite, }, decrypt_transaction, - fees::{self, ChangeValue, DustOutputPolicy}, + fees::{self, DustOutputPolicy}, keys::UnifiedSpendingKey, wallet::{Note, OvkPolicy, Recipient}, zip321::{self, Payment}, @@ -718,13 +718,15 @@ where } for change_value in proposal.balance().proposed_change() { - match change_value { - ChangeValue::Sapling { value, memo } => { - let memo = memo.as_ref().map_or_else(MemoBytes::empty, |m| m.clone()); + let memo = change_value + .memo() + .map_or_else(MemoBytes::empty, |m| m.clone()); + match change_value.output_pool() { + ShieldedProtocol::Sapling => { builder.add_sapling_output( internal_ovk(), dfvk.change_address().1, - *value, + change_value.value(), memo.clone(), )?; sapling_output_meta.push(( @@ -732,10 +734,19 @@ where account, PoolType::Shielded(ShieldedProtocol::Sapling), ), - *value, + change_value.value(), Some(memo), )) } + ShieldedProtocol::Orchard => { + #[cfg(not(feature = "orchard"))] + return Err(Error::UnsupportedPoolType(PoolType::Shielded( + ShieldedProtocol::Orchard, + ))); + + #[cfg(feature = "orchard")] + unimplemented!("FIXME: implement Orchard change output creation.") + } } } diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index c2973341f..550b96f3b 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -27,10 +27,10 @@ use crate::{ }; #[cfg(feature = "transparent-inputs")] -use { - std::collections::BTreeSet, std::convert::Infallible, - zcash_primitives::transaction::components::OutPoint, -}; +use {std::collections::BTreeSet, zcash_primitives::transaction::components::OutPoint}; + +#[cfg(feature = "orchard")] +use {crate::fees::orchard as orchard_fees, std::convert::Infallible}; /// The type of errors that may be produced in input selection. pub enum InputSelectorError { @@ -447,6 +447,23 @@ impl sapling::OutputView for SaplingPayment { } } +pub(crate) struct OrchardPayment(NonNegativeAmount); + +// TODO: introduce this method when it is needed for testing. +// #[cfg(test)] +// impl OrchardPayment { +// pub(crate) fn new(amount: NonNegativeAmount) -> Self { +// OrchardPayment(amount) +// } +// } + +#[cfg(feature = "orchard")] +impl orchard_fees::OutputView for OrchardPayment { + fn value(&self) -> NonNegativeAmount { + self.0 + } +} + /// An [`InputSelector`] implementation that uses a greedy strategy to select between available /// notes. /// @@ -499,6 +516,7 @@ where { let mut transparent_outputs = vec![]; let mut sapling_outputs = vec![]; + let mut orchard_outputs = vec![]; for payment in transaction_request.payments() { let mut push_transparent = |taddr: TransparentAddress| { transparent_outputs.push(TxOut { @@ -509,6 +527,9 @@ where let mut push_sapling = || { sapling_outputs.push(SaplingPayment(payment.amount)); }; + let mut push_orchard = || { + orchard_outputs.push(OrchardPayment(payment.amount)); + }; match &payment.recipient_address { Address::Transparent(addr) => { @@ -518,7 +539,14 @@ where push_sapling(); } Address::Unified(addr) => { - if addr.sapling().is_some() { + #[cfg(feature = "orchard")] + let has_orchard = addr.orchard().is_some(); + #[cfg(not(feature = "orchard"))] + let has_orchard = false; + + if has_orchard { + push_orchard(); + } else if addr.sapling().is_some() { push_sapling(); } else if let Some(addr) = addr.transparent() { push_transparent(*addr); @@ -544,8 +572,17 @@ where target_height, &Vec::::new(), &transparent_outputs, - &sapling_inputs, - &sapling_outputs, + &( + ::sapling::builder::BundleType::DEFAULT, + &sapling_inputs[..], + &sapling_outputs[..], + ), + #[cfg(feature = "orchard")] + &( + ::orchard::builder::BundleType::DEFAULT, + &Vec::::new()[..], + &orchard_outputs[..], + ), &self.dust_output_policy, ); @@ -652,8 +689,17 @@ where target_height, &transparent_inputs, &Vec::::new(), - &Vec::>::new(), - &Vec::::new(), + &( + ::sapling::builder::BundleType::DEFAULT, + &Vec::::new()[..], + &Vec::::new()[..], + ), + #[cfg(feature = "orchard")] + &( + orchard::builder::BundleType::DEFAULT, + &Vec::::new()[..], + &Vec::::new()[..], + ), &self.dust_output_policy, ); @@ -668,8 +714,17 @@ where target_height, &transparent_inputs, &Vec::::new(), - &Vec::>::new(), - &Vec::::new(), + &( + ::sapling::builder::BundleType::DEFAULT, + &Vec::::new()[..], + &Vec::::new()[..], + ), + #[cfg(feature = "orchard")] + &( + orchard::builder::BundleType::DEFAULT, + &Vec::::new()[..], + &Vec::::new()[..], + ), &self.dust_output_policy, )? } diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index 1aca511d2..25f2f9402 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -12,38 +12,69 @@ use zcash_primitives::{ }, }; +use crate::ShieldedProtocol; + pub(crate) mod common; pub mod fixed; +pub mod orchard; pub mod sapling; pub mod standard; pub mod zip317; /// A proposed change amount and output pool. #[derive(Clone, Debug, PartialEq, Eq)] -pub enum ChangeValue { - Sapling { - value: NonNegativeAmount, - memo: Option, - }, +pub struct ChangeValue { + output_pool: ShieldedProtocol, + value: NonNegativeAmount, + memo: Option, } impl ChangeValue { + /// Constructs a new change value from its constituent parts. + pub fn new( + output_pool: ShieldedProtocol, + value: NonNegativeAmount, + memo: Option, + ) -> Self { + Self { + output_pool, + value, + memo, + } + } + + /// Constructs a new change value that will be created as a Sapling output. pub fn sapling(value: NonNegativeAmount, memo: Option) -> Self { - Self::Sapling { value, memo } + Self { + output_pool: ShieldedProtocol::Sapling, + value, + memo, + } + } + + /// Constructs a new change value that will be created as an Orchard output. + #[cfg(feature = "orchard")] + pub fn orchard(value: NonNegativeAmount, memo: Option) -> Self { + Self { + output_pool: ShieldedProtocol::Orchard, + value, + memo, + } + } + + /// Returns the pool to which the change output should be sent. + pub fn output_pool(&self) -> ShieldedProtocol { + self.output_pool } /// Returns the value of the change output to be created, in zatoshis. pub fn value(&self) -> NonNegativeAmount { - match self { - ChangeValue::Sapling { value, .. } => *value, - } + self.value } /// Returns the memo to be associated with the change output. pub fn memo(&self) -> Option<&MemoBytes> { - match self { - ChangeValue::Sapling { memo, .. } => memo.as_ref(), - } + self.memo.as_ref() } } @@ -120,6 +151,8 @@ pub enum ChangeError { }, /// An error occurred that was specific to the change selection strategy in use. StrategyError(E), + /// The proposed bundle structure would violate bundle type construction rules. + BundleError(&'static str), } impl ChangeError { @@ -140,6 +173,7 @@ impl ChangeError { sapling, }, ChangeError::StrategyError(e) => ChangeError::StrategyError(f(e)), + ChangeError::BundleError(e) => ChangeError::BundleError(e), } } } @@ -167,6 +201,13 @@ impl fmt::Display for ChangeError { ChangeError::StrategyError(err) => { write!(f, "{}", err) } + ChangeError::BundleError(err) => { + write!( + f, + "The proposed transaction structure violates bundle type constrints: {}", + err + ) + } } } } @@ -252,8 +293,8 @@ pub trait ChangeStrategy { target_height: BlockHeight, transparent_inputs: &[impl transparent::InputView], transparent_outputs: &[impl transparent::OutputView], - sapling_inputs: &[impl sapling::InputView], - sapling_outputs: &[impl sapling::OutputView], + sapling: &impl sapling::BundleView, + #[cfg(feature = "orchard")] orchard: &impl orchard::BundleView, dust_output_policy: &DustOutputPolicy, ) -> Result>; } diff --git a/zcash_client_backend/src/fees/common.rs b/zcash_client_backend/src/fees/common.rs index 4368bda4d..63a5a1280 100644 --- a/zcash_client_backend/src/fees/common.rs +++ b/zcash_client_backend/src/fees/common.rs @@ -7,7 +7,15 @@ use zcash_primitives::{ }, }; -use super::{sapling, ChangeError, ChangeValue, DustAction, DustOutputPolicy, TransactionBalance}; +use crate::ShieldedProtocol; + +use super::{ + sapling as sapling_fees, ChangeError, ChangeValue, DustAction, DustOutputPolicy, + TransactionBalance, +}; + +#[cfg(feature = "orchard")] +use super::orchard as orchard_fees; #[allow(clippy::too_many_arguments)] pub(crate) fn single_change_output_balance< @@ -21,8 +29,8 @@ pub(crate) fn single_change_output_balance< target_height: BlockHeight, transparent_inputs: &[impl transparent::InputView], transparent_outputs: &[impl transparent::OutputView], - sapling_inputs: &[impl sapling::InputView], - sapling_outputs: &[impl sapling::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, @@ -43,40 +51,88 @@ where .map(|t_out| t_out.value()) .sum::>() .ok_or_else(overflow)?; - let sapling_in = sapling_inputs + let sapling_in = sapling + .inputs() .iter() - .map(|s_in| s_in.value()) + .map(sapling_fees::InputView::::value) .sum::>() .ok_or_else(overflow)?; - let sapling_out = sapling_outputs + let sapling_out = sapling + .outputs() .iter() - .map(|s_out| s_out.value()) + .map(sapling_fees::OutputView::value) .sum::>() .ok_or_else(overflow)?; + #[cfg(feature = "orchard")] + let orchard_in = orchard + .inputs() + .iter() + .map(orchard_fees::InputView::::value) + .sum::>() + .ok_or_else(overflow)?; + #[cfg(not(feature = "orchard"))] + let orchard_in = NonNegativeAmount::ZERO; + + #[cfg(feature = "orchard")] + let orchard_out = orchard + .outputs() + .iter() + .map(orchard_fees::OutputView::value) + .sum::>() + .ok_or_else(overflow)?; + #[cfg(not(feature = "orchard"))] + let orchard_out = NonNegativeAmount::ZERO; + + // 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 > NonNegativeAmount::ZERO || orchard_out > NonNegativeAmount::ZERO { + // Send change to Orchard if we're spending any Orchard inputs or creating any Orchard outputs + (ShieldedProtocol::Orchard, 0, 1) + } else if sapling_in > NonNegativeAmount::ZERO || sapling_out > NonNegativeAmount::ZERO { + // 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) + } else { + // For all other transactions, send change to Sapling. + // FIXME: Change this to Orchard once Orchard outputs are enabled. + (ShieldedProtocol::Sapling, 1, 0) + }; + #[cfg(not(feature = "orchard"))] + let (change_pool, sapling_change) = (ShieldedProtocol::Sapling, 1); + + #[cfg(feature = "orchard")] + let orchard_num_actions = orchard + .bundle_type() + .num_actions( + orchard.inputs().len(), + orchard.outputs().len() + orchard_change, + ) + .map_err(ChangeError::BundleError)?; + #[cfg(not(feature = "orchard"))] + let orchard_num_actions = 0; + let fee_amount = fee_rule .fee_required( params, target_height, transparent_inputs, transparent_outputs, - sapling_inputs.len(), - if sapling_inputs.is_empty() { - sapling_outputs.len() + 1 - } else { - std::cmp::max(sapling_outputs.len() + 1, 2) - }, - //Orchard is not yet supported in zcash_client_backend - 0, + sapling.inputs().len(), + sapling + .bundle_type() + .num_outputs( + sapling.inputs().len(), + sapling.outputs().len() + sapling_change, + ) + .map_err(ChangeError::BundleError)?, + orchard_num_actions, ) .map_err(|fee_error| ChangeError::StrategyError(E::from(fee_error)))?; - let total_in = (t_in + sapling_in).ok_or_else(overflow)?; - - let total_out = [t_out, sapling_out, fee_amount] - .iter() - .sum::>() - .ok_or_else(overflow)?; + 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 proposed_change = (total_in - total_out).ok_or(ChangeError::InsufficientFunds { available: total_in, @@ -101,7 +157,7 @@ where }) } DustAction::AllowDustChange => TransactionBalance::new( - vec![ChangeValue::sapling(proposed_change, change_memo)], + vec![ChangeValue::new(change_pool, proposed_change, change_memo)], fee_amount, ) .map_err(|_| overflow()), @@ -113,7 +169,7 @@ where } } else { TransactionBalance::new( - vec![ChangeValue::sapling(proposed_change, change_memo)], + vec![ChangeValue::new(change_pool, proposed_change, change_memo)], fee_amount, ) .map_err(|_| overflow()) diff --git a/zcash_client_backend/src/fees/fixed.rs b/zcash_client_backend/src/fees/fixed.rs index b17863ec2..ec4fef86d 100644 --- a/zcash_client_backend/src/fees/fixed.rs +++ b/zcash_client_backend/src/fees/fixed.rs @@ -10,10 +10,13 @@ use zcash_primitives::{ }; use super::{ - common::single_change_output_balance, sapling, ChangeError, ChangeStrategy, DustOutputPolicy, - TransactionBalance, + common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy, + DustOutputPolicy, TransactionBalance, }; +#[cfg(feature = "orchard")] +use super::orchard as orchard_fees; + /// A change strategy that and proposes change as a single output to the most current supported /// shielded pool and delegates fee calculation to the provided fee rule. pub struct SingleOutputChangeStrategy { @@ -46,8 +49,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy { target_height: BlockHeight, transparent_inputs: &[impl transparent::InputView], transparent_outputs: &[impl transparent::OutputView], - sapling_inputs: &[impl sapling::InputView], - sapling_outputs: &[impl sapling::OutputView], + sapling: &impl sapling_fees::BundleView, + #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, dust_output_policy: &DustOutputPolicy, ) -> Result> { single_change_output_balance( @@ -56,8 +59,9 @@ impl ChangeStrategy for SingleOutputChangeStrategy { target_height, transparent_inputs, transparent_outputs, - sapling_inputs, - sapling_outputs, + sapling, + #[cfg(feature = "orchard")] + orchard, dust_output_policy, self.fee_rule().fixed_fee(), self.change_memo.clone(), @@ -67,6 +71,9 @@ impl ChangeStrategy for SingleOutputChangeStrategy { #[cfg(test)] mod tests { + #[cfg(feature = "orchard")] + use std::convert::Infallible; + use zcash_primitives::{ consensus::{Network, NetworkUpgrade, Parameters}, transaction::{ @@ -98,13 +105,22 @@ mod tests { .unwrap(), &Vec::::new(), &Vec::::new(), - &[TestSaplingInput { - note_id: 0, - value: NonNegativeAmount::const_from_u64(60000), - }], - &[SaplingPayment::new(NonNegativeAmount::const_from_u64( - 40000, - ))], + &( + sapling::builder::BundleType::DEFAULT, + &[TestSaplingInput { + note_id: 0, + value: NonNegativeAmount::const_from_u64(60000), + }][..], + &[SaplingPayment::new(NonNegativeAmount::const_from_u64( + 40000, + ))][..], + ), + #[cfg(feature = "orchard")] + &( + orchard::builder::BundleType::DEFAULT, + &[] as &[Infallible], + &[] as &[Infallible], + ), &DustOutputPolicy::default(), ); @@ -130,20 +146,29 @@ mod tests { .unwrap(), &Vec::::new(), &Vec::::new(), - &[ - TestSaplingInput { - note_id: 0, - value: NonNegativeAmount::const_from_u64(40000), - }, - // enough to pay a fee, plus dust - TestSaplingInput { - note_id: 0, - value: NonNegativeAmount::const_from_u64(10100), - }, - ], - &[SaplingPayment::new(NonNegativeAmount::const_from_u64( - 40000, - ))], + &( + sapling::builder::BundleType::DEFAULT, + &[ + TestSaplingInput { + note_id: 0, + value: NonNegativeAmount::const_from_u64(40000), + }, + // enough to pay a fee, plus dust + TestSaplingInput { + note_id: 0, + value: NonNegativeAmount::const_from_u64(10100), + }, + ][..], + &[SaplingPayment::new(NonNegativeAmount::const_from_u64( + 40000, + ))][..], + ), + #[cfg(feature = "orchard")] + &( + orchard::builder::BundleType::DEFAULT, + &[] as &[Infallible], + &[] as &[Infallible], + ), &DustOutputPolicy::default(), ); diff --git a/zcash_client_backend/src/fees/orchard.rs b/zcash_client_backend/src/fees/orchard.rs new file mode 100644 index 000000000..790bdff78 --- /dev/null +++ b/zcash_client_backend/src/fees/orchard.rs @@ -0,0 +1,76 @@ +//! Types related to computation of fees and change related to the Orchard components +//! of a transaction. + +use std::convert::Infallible; +use zcash_primitives::transaction::components::amount::NonNegativeAmount; + +#[cfg(feature = "orchard")] +use orchard::builder::BundleType; + +/// A trait that provides a minimized view of Orchard bundle configuration +/// suitable for use in fee and change calculation. +#[cfg(feature = "orchard")] +pub trait BundleView { + /// The type of inputs to the bundle. + type In: InputView; + /// The type of inputs of the bundle. + type Out: OutputView; + + /// Returns the type of the bundle + fn bundle_type(&self) -> BundleType; + /// Returns the inputs to the bundle. + fn inputs(&self) -> &[Self::In]; + /// Returns the outputs of the bundle. + fn outputs(&self) -> &[Self::Out]; +} + +#[cfg(feature = "orchard")] +impl<'a, NoteRef, In: InputView, Out: OutputView> BundleView + for (BundleType, &'a [In], &'a [Out]) +{ + type In = In; + type Out = Out; + + fn bundle_type(&self) -> BundleType { + self.0 + } + + fn inputs(&self) -> &[In] { + self.1 + } + + fn outputs(&self) -> &[Out] { + self.2 + } +} + +/// A trait that provides a minimized view of an Orchard input suitable for use in fee and change +/// calculation. +pub trait InputView { + /// An identifier for the input being spent. + fn note_id(&self) -> &NoteRef; + /// The value of the input being spent. + fn value(&self) -> NonNegativeAmount; +} + +impl InputView for Infallible { + fn note_id(&self) -> &N { + unreachable!() + } + fn value(&self) -> NonNegativeAmount { + unreachable!() + } +} + +/// A trait that provides a minimized view of a Orchard output suitable for use in fee and change +/// calculation. +pub trait OutputView { + /// The value of the output being produced. + fn value(&self) -> NonNegativeAmount; +} + +impl OutputView for Infallible { + fn value(&self) -> NonNegativeAmount { + unreachable!() + } +} diff --git a/zcash_client_backend/src/fees/sapling.rs b/zcash_client_backend/src/fees/sapling.rs index df2702b19..2653310a9 100644 --- a/zcash_client_backend/src/fees/sapling.rs +++ b/zcash_client_backend/src/fees/sapling.rs @@ -3,9 +3,44 @@ use std::convert::Infallible; -use sapling::builder::{OutputInfo, SpendInfo}; +use sapling::builder::{BundleType, OutputInfo, SpendInfo}; use zcash_primitives::transaction::components::amount::NonNegativeAmount; +/// A trait that provides a minimized view of Sapling bundle configuration +/// suitable for use in fee and change calculation. +pub trait BundleView { + /// The type of inputs to the bundle. + type In: InputView; + /// The type of inputs of the bundle. + type Out: OutputView; + + /// Returns the type of the bundle + fn bundle_type(&self) -> BundleType; + /// Returns the inputs to the bundle. + fn inputs(&self) -> &[Self::In]; + /// Returns the outputs of the bundle. + fn outputs(&self) -> &[Self::Out]; +} + +impl<'a, NoteRef, In: InputView, Out: OutputView> BundleView + for (BundleType, &'a [In], &'a [Out]) +{ + type In = In; + type Out = Out; + + fn bundle_type(&self) -> BundleType { + self.0 + } + + fn inputs(&self) -> &[In] { + self.1 + } + + fn outputs(&self) -> &[Out] { + self.2 + } +} + /// A trait that provides a minimized view of a Sapling input suitable for use in /// fee and change calculation. pub trait InputView { diff --git a/zcash_client_backend/src/fees/standard.rs b/zcash_client_backend/src/fees/standard.rs index 7dbd740ef..d9d40b1c3 100644 --- a/zcash_client_backend/src/fees/standard.rs +++ b/zcash_client_backend/src/fees/standard.rs @@ -15,9 +15,13 @@ use zcash_primitives::{ }; use super::{ - fixed, sapling, zip317, ChangeError, ChangeStrategy, DustOutputPolicy, TransactionBalance, + fixed, sapling as sapling_fees, zip317, ChangeError, ChangeStrategy, DustOutputPolicy, + TransactionBalance, }; +#[cfg(feature = "orchard")] +use super::orchard as orchard_fees; + /// A change strategy that proposes change as a single output to the most current supported /// shielded pool and delegates fee calculation to the provided fee rule. pub struct SingleOutputChangeStrategy { @@ -50,8 +54,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy { target_height: BlockHeight, transparent_inputs: &[impl transparent::InputView], transparent_outputs: &[impl transparent::OutputView], - sapling_inputs: &[impl sapling::InputView], - sapling_outputs: &[impl sapling::OutputView], + sapling: &impl sapling_fees::BundleView, + #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, dust_output_policy: &DustOutputPolicy, ) -> Result> { #[allow(deprecated)] @@ -65,8 +69,9 @@ impl ChangeStrategy for SingleOutputChangeStrategy { target_height, transparent_inputs, transparent_outputs, - sapling_inputs, - sapling_outputs, + sapling, + #[cfg(feature = "orchard")] + orchard, dust_output_policy, ) .map_err(|e| e.map(Zip317FeeError::Balance)), @@ -79,8 +84,9 @@ impl ChangeStrategy for SingleOutputChangeStrategy { target_height, transparent_inputs, transparent_outputs, - sapling_inputs, - sapling_outputs, + sapling, + #[cfg(feature = "orchard")] + orchard, dust_output_policy, ) .map_err(|e| e.map(Zip317FeeError::Balance)), @@ -93,8 +99,9 @@ impl ChangeStrategy for SingleOutputChangeStrategy { target_height, transparent_inputs, transparent_outputs, - sapling_inputs, - sapling_outputs, + sapling, + #[cfg(feature = "orchard")] + orchard, dust_output_policy, ), } diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs index ab88bcd26..fe08501e6 100644 --- a/zcash_client_backend/src/fees/zip317.rs +++ b/zcash_client_backend/src/fees/zip317.rs @@ -14,10 +14,13 @@ use zcash_primitives::{ }; use super::{ - common::single_change_output_balance, sapling, ChangeError, ChangeStrategy, DustOutputPolicy, - TransactionBalance, + common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy, + DustOutputPolicy, TransactionBalance, }; +#[cfg(feature = "orchard")] +use super::orchard as orchard_fees; + /// A change strategy that and proposes change as a single output to the most current supported /// shielded pool and delegates fee calculation to the provided fee rule. pub struct SingleOutputChangeStrategy { @@ -50,8 +53,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy { target_height: BlockHeight, transparent_inputs: &[impl transparent::InputView], transparent_outputs: &[impl transparent::OutputView], - sapling_inputs: &[impl sapling::InputView], - sapling_outputs: &[impl sapling::OutputView], + sapling: &impl sapling_fees::BundleView, + #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, dust_output_policy: &DustOutputPolicy, ) -> Result> { let mut transparent_dust: Vec<_> = transparent_inputs @@ -67,11 +70,12 @@ impl ChangeStrategy for SingleOutputChangeStrategy { }) .collect(); - let mut sapling_dust: Vec<_> = sapling_inputs + let mut sapling_dust: Vec<_> = sapling + .inputs() .iter() .filter_map(|i| { - if i.value() < self.fee_rule.marginal_fee() { - Some(i.note_id().clone()) + if sapling_fees::InputView::::value(i) < self.fee_rule.marginal_fee() { + Some(sapling_fees::InputView::::note_id(i).clone()) } else { None } @@ -88,8 +92,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy { // 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. - 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_non_dust = sapling.inputs().len() - sapling_dust.len(); + let s_allowed_dust = (sapling.outputs().len() + 1).saturating_sub(s_non_dust); let available_grace_inputs = self .fee_rule @@ -135,8 +139,9 @@ impl ChangeStrategy for SingleOutputChangeStrategy { target_height, transparent_inputs, transparent_outputs, - sapling_inputs, - sapling_outputs, + sapling, + #[cfg(feature = "orchard")] + orchard, dust_output_policy, self.fee_rule.marginal_fee(), self.change_memo.clone(), @@ -147,6 +152,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy { #[cfg(test)] mod tests { + use std::convert::Infallible; + use zcash_primitives::{ consensus::{Network, NetworkUpgrade, Parameters}, legacy::Script, @@ -177,13 +184,22 @@ mod tests { .unwrap(), &Vec::::new(), &Vec::::new(), - &[TestSaplingInput { - note_id: 0, - value: NonNegativeAmount::const_from_u64(55000), - }], - &[SaplingPayment::new(NonNegativeAmount::const_from_u64( - 40000, - ))], + &( + sapling::builder::BundleType::DEFAULT, + &[TestSaplingInput { + note_id: 0, + value: NonNegativeAmount::const_from_u64(55000), + }][..], + &[SaplingPayment::new(NonNegativeAmount::const_from_u64( + 40000, + ))][..], + ), + #[cfg(feature = "orchard")] + &( + orchard::builder::BundleType::DEFAULT, + &Vec::::new()[..], + &Vec::::new()[..], + ), &DustOutputPolicy::default(), ); @@ -210,11 +226,20 @@ mod tests { value: NonNegativeAmount::const_from_u64(40000), script_pubkey: Script(vec![]), }], - &[TestSaplingInput { - note_id: 0, - value: NonNegativeAmount::const_from_u64(55000), - }], - &Vec::::new(), + &( + sapling::builder::BundleType::DEFAULT, + &[TestSaplingInput { + note_id: 0, + value: NonNegativeAmount::const_from_u64(55000), + }][..], + &Vec::::new()[..], + ), + #[cfg(feature = "orchard")] + &( + orchard::builder::BundleType::DEFAULT, + &Vec::::new()[..], + &Vec::::new()[..], + ), &DustOutputPolicy::default(), ); @@ -237,19 +262,28 @@ mod tests { .unwrap(), &Vec::::new(), &Vec::::new(), - &[ - TestSaplingInput { - note_id: 0, - value: NonNegativeAmount::const_from_u64(49000), - }, - TestSaplingInput { - note_id: 1, - value: NonNegativeAmount::const_from_u64(1000), - }, - ], - &[SaplingPayment::new(NonNegativeAmount::const_from_u64( - 40000, - ))], + &( + sapling::builder::BundleType::DEFAULT, + &[ + TestSaplingInput { + note_id: 0, + value: NonNegativeAmount::const_from_u64(49000), + }, + TestSaplingInput { + note_id: 1, + value: NonNegativeAmount::const_from_u64(1000), + }, + ][..], + &[SaplingPayment::new(NonNegativeAmount::const_from_u64( + 40000, + ))][..], + ), + #[cfg(feature = "orchard")] + &( + orchard::builder::BundleType::DEFAULT, + &Vec::::new()[..], + &Vec::::new()[..], + ), &DustOutputPolicy::default(), ); @@ -272,23 +306,32 @@ mod tests { .unwrap(), &Vec::::new(), &Vec::::new(), - &[ - TestSaplingInput { - note_id: 0, - value: NonNegativeAmount::const_from_u64(29000), - }, - TestSaplingInput { - note_id: 1, - value: NonNegativeAmount::const_from_u64(20000), - }, - TestSaplingInput { - note_id: 2, - value: NonNegativeAmount::const_from_u64(1000), - }, - ], - &[SaplingPayment::new(NonNegativeAmount::const_from_u64( - 40000, - ))], + &( + sapling::builder::BundleType::DEFAULT, + &[ + TestSaplingInput { + note_id: 0, + value: NonNegativeAmount::const_from_u64(29000), + }, + TestSaplingInput { + note_id: 1, + value: NonNegativeAmount::const_from_u64(20000), + }, + TestSaplingInput { + note_id: 2, + value: NonNegativeAmount::const_from_u64(1000), + }, + ][..], + &[SaplingPayment::new(NonNegativeAmount::const_from_u64( + 40000, + ))][..], + ), + #[cfg(feature = "orchard")] + &( + orchard::builder::BundleType::DEFAULT, + &Vec::::new()[..], + &Vec::::new()[..], + ), &DustOutputPolicy::default(), ); diff --git a/zcash_client_backend/src/proto.rs b/zcash_client_backend/src/proto.rs index 13af86322..1c9f33a16 100644 --- a/zcash_client_backend/src/proto.rs +++ b/zcash_client_backend/src/proto.rs @@ -341,17 +341,20 @@ impl proposal::Proposal { .balance() .proposed_change() .iter() - .map(|change| match change { - ChangeValue::Sapling { value, memo } => proposal::ChangeValue { + .map(|change| match change.output_pool() { + ShieldedProtocol::Sapling => proposal::ChangeValue { value: Some(proposal::change_value::Value::SaplingValue( proposal::SaplingChange { - amount: (*value).into(), - memo: memo.as_ref().map(|memo_bytes| proposal::MemoBytes { + amount: change.value().into(), + memo: change.memo().map(|memo_bytes| proposal::MemoBytes { value: memo_bytes.as_slice().to_vec(), }), }, )), }, + ShieldedProtocol::Orchard => { + unimplemented!("FIXME: implement Orchard change outputs!") + } }) .collect(), fee_required: value.balance().fee_required().into(), diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 6cc4d3090..84cf87f89 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -44,9 +44,11 @@ and this library adheres to Rust's notion of - `impl From<&NonNegativeAmount> for Amount` - `impl From for u64` - `impl From for zcash_primitives::sapling::value::NoteValue` + - `impl From for orchard::::NoteValue` - `impl Sum for Option` - `impl<'a> Sum<&'a NonNegativeAmount> for Option` - `impl TryFrom for NonNegativeAmount` + - `impl TryFrom for NonNegativeAmount` - `impl {Clone, PartialEq, Eq} for zcash_primitives::memo::Error` - `impl {PartialEq, Eq} for zcash_primitives::sapling::note::Rseed` - `impl From for [u8; 32]` diff --git a/zcash_primitives/src/transaction/components/amount.rs b/zcash_primitives/src/transaction/components/amount.rs index b005bd43d..3474e74ea 100644 --- a/zcash_primitives/src/transaction/components/amount.rs +++ b/zcash_primitives/src/transaction/components/amount.rs @@ -331,6 +331,20 @@ impl TryFrom for NonNegativeAmount { } } +impl From for orchard::NoteValue { + fn from(n: NonNegativeAmount) -> Self { + orchard::NoteValue::from_raw(n.into()) + } +} + +impl TryFrom for NonNegativeAmount { + type Error = (); + + fn try_from(value: orchard::NoteValue) -> Result { + Self::from_u64(value.inner()) + } +} + impl TryFrom for NonNegativeAmount { type Error = ();