diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 7de96c9a9..49e285285 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -71,11 +71,12 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail if a memo is given for the transparent pool. Use `ChangeValue::shielded` to avoid this error case when creating a `ChangeValue` known to be for a shielded pool. - - `ChangeStrategy::compute_balance`: this trait method has two additional + - `ChangeStrategy::compute_balance`: this trait method has three additional parameters when the "transparent-inputs" feature is enabled. These are - used to specify amounts of additional transparent P2PKH inputs and outputs, - for which `InputView` or `OutputView` instances may not be available. - Empty slices can be passed to obtain the previous behaviour. + used to specify whether the change memo should be ignored, and the amounts + of additional transparent P2PKH inputs and outputs. Passing `false` for + `ignore_change_memo` and empty slices for the other two arguments will + retain the previous behaviour. - `zcash_client_backend::input_selection::GreedyInputSelectorError` has a new variant `UnsupportedTexAddress`. - `zcash_client_backend::proto::ProposalDecodingError` has a new variant 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 6a8ece442..bec5283ec 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/zcash_client_backend/src/data_api/wallet/input_selection.rs @@ -475,6 +475,8 @@ where &orchard_fees::EmptyBundleView, &self.dust_output_policy, #[cfg(feature = "transparent-inputs")] + true, + #[cfg(feature = "transparent-inputs")] &[NonNegativeAmount::ZERO], #[cfg(feature = "transparent-inputs")] &[], @@ -494,6 +496,8 @@ where &orchard_fees::EmptyBundleView, &self.dust_output_policy, #[cfg(feature = "transparent-inputs")] + true, + #[cfg(feature = "transparent-inputs")] &[tr1_required_input_value], #[cfg(feature = "transparent-inputs")] &[], @@ -574,6 +578,8 @@ where ), &self.dust_output_policy, #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &ephemeral_output_amounts, @@ -764,6 +770,8 @@ where &orchard_fees::EmptyBundleView, &self.dust_output_policy, #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], @@ -785,6 +793,8 @@ where &orchard_fees::EmptyBundleView, &self.dust_output_policy, #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], diff --git a/zcash_client_backend/src/fees.rs b/zcash_client_backend/src/fees.rs index 7e5e966a5..a85acbdf2 100644 --- a/zcash_client_backend/src/fees.rs +++ b/zcash_client_backend/src/fees.rs @@ -356,6 +356,7 @@ pub trait ChangeStrategy { sapling: &impl sapling::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard::BundleView, dust_output_policy: &DustOutputPolicy, + #[cfg(feature = "transparent-inputs")] ignore_change_memo: bool, #[cfg(feature = "transparent-inputs")] ephemeral_input_amounts: &[NonNegativeAmount], #[cfg(feature = "transparent-inputs")] ephemeral_output_amounts: &[NonNegativeAmount], ) -> Result>; diff --git a/zcash_client_backend/src/fees/fixed.rs b/zcash_client_backend/src/fees/fixed.rs index 1a340c7a2..f9ab9a9ca 100644 --- a/zcash_client_backend/src/fees/fixed.rs +++ b/zcash_client_backend/src/fees/fixed.rs @@ -66,9 +66,13 @@ impl ChangeStrategy for SingleOutputChangeStrategy { sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, dust_output_policy: &DustOutputPolicy, + #[cfg(feature = "transparent-inputs")] ignore_change_memo: bool, #[cfg(feature = "transparent-inputs")] ephemeral_input_amounts: &[NonNegativeAmount], #[cfg(feature = "transparent-inputs")] ephemeral_output_amounts: &[NonNegativeAmount], ) -> Result> { + #[cfg(not(feature = "transparent-inputs"))] + let ignore_change_memo = false; + single_change_output_balance( params, &self.fee_rule, @@ -80,7 +84,11 @@ impl ChangeStrategy for SingleOutputChangeStrategy { orchard, dust_output_policy, self.fee_rule().fixed_fee(), - self.change_memo.clone(), + if ignore_change_memo { + None + } else { + self.change_memo.clone() + }, self.fallback_change_pool, #[cfg(feature = "transparent-inputs")] ephemeral_input_amounts, @@ -142,6 +150,8 @@ mod tests { &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], @@ -191,6 +201,8 @@ mod tests { &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], diff --git a/zcash_client_backend/src/fees/standard.rs b/zcash_client_backend/src/fees/standard.rs index c5fc6cb9f..8d53ecb66 100644 --- a/zcash_client_backend/src/fees/standard.rs +++ b/zcash_client_backend/src/fees/standard.rs @@ -68,6 +68,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, dust_output_policy: &DustOutputPolicy, + #[cfg(feature = "transparent-inputs")] ignore_change_memo: bool, #[cfg(feature = "transparent-inputs")] ephemeral_input_amounts: &[NonNegativeAmount], #[cfg(feature = "transparent-inputs")] ephemeral_output_amounts: &[NonNegativeAmount], ) -> Result> { @@ -88,6 +89,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy { orchard, dust_output_policy, #[cfg(feature = "transparent-inputs")] + ignore_change_memo, + #[cfg(feature = "transparent-inputs")] ephemeral_input_amounts, #[cfg(feature = "transparent-inputs")] ephemeral_output_amounts, @@ -108,6 +111,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy { orchard, dust_output_policy, #[cfg(feature = "transparent-inputs")] + ignore_change_memo, + #[cfg(feature = "transparent-inputs")] ephemeral_input_amounts, #[cfg(feature = "transparent-inputs")] ephemeral_output_amounts, @@ -128,6 +133,8 @@ impl ChangeStrategy for SingleOutputChangeStrategy { orchard, dust_output_policy, #[cfg(feature = "transparent-inputs")] + ignore_change_memo, + #[cfg(feature = "transparent-inputs")] ephemeral_input_amounts, #[cfg(feature = "transparent-inputs")] ephemeral_output_amounts, diff --git a/zcash_client_backend/src/fees/zip317.rs b/zcash_client_backend/src/fees/zip317.rs index 63ce70c4a..e6c8d7858 100644 --- a/zcash_client_backend/src/fees/zip317.rs +++ b/zcash_client_backend/src/fees/zip317.rs @@ -68,6 +68,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy { sapling: &impl sapling_fees::BundleView, #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView, dust_output_policy: &DustOutputPolicy, + #[cfg(feature = "transparent-inputs")] ignore_change_memo: bool, #[cfg(feature = "transparent-inputs")] ephemeral_input_amounts: &[NonNegativeAmount], #[cfg(feature = "transparent-inputs")] ephemeral_output_amounts: &[NonNegativeAmount], ) -> Result> { @@ -203,6 +204,9 @@ impl ChangeStrategy for SingleOutputChangeStrategy { } } + #[cfg(not(feature = "transparent-inputs"))] + let ignore_change_memo = false; + single_change_output_balance( params, &self.fee_rule, @@ -214,7 +218,11 @@ impl ChangeStrategy for SingleOutputChangeStrategy { orchard, dust_output_policy, self.fee_rule.marginal_fee(), - self.change_memo.clone(), + if ignore_change_memo { + None + } else { + self.change_memo.clone() + }, self.fallback_change_pool, #[cfg(feature = "transparent-inputs")] ephemeral_input_amounts, @@ -283,6 +291,8 @@ mod tests { &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], @@ -330,6 +340,8 @@ mod tests { ), &DustOutputPolicy::default(), #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], @@ -386,6 +398,8 @@ mod tests { &orchard_fees::EmptyBundleView, dust_output_policy, #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], @@ -433,6 +447,8 @@ mod tests { &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], @@ -480,6 +496,8 @@ mod tests { &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], @@ -533,6 +551,8 @@ mod tests { Some(NonNegativeAmount::const_from_u64(1000)), ), #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], @@ -597,6 +617,8 @@ mod tests { &orchard_fees::EmptyBundleView, dust_output_policy, #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], @@ -653,6 +675,8 @@ mod tests { &orchard_fees::EmptyBundleView, &DustOutputPolicy::default(), #[cfg(feature = "transparent-inputs")] + false, + #[cfg(feature = "transparent-inputs")] &[], #[cfg(feature = "transparent-inputs")] &[], diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index cb0292bd2..ed6cb158e 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -302,6 +302,8 @@ pub(crate) fn send_single_step_proposed_transfer() { #[cfg(feature = "transparent-inputs")] pub(crate) fn send_multi_step_proposed_transfer() { + use std::str::FromStr; + use zcash_client_backend::fees::ChangeValue; let mut st = TestBuilder::new() @@ -346,8 +348,7 @@ pub(crate) fn send_multi_step_proposed_transfer() { TransparentAddress::PublicKeyHash(data) => Address::Tex(data), _ => unreachable!(), }; - // As of this commit, change memos are not correctly handled in ZIP 320 proposals. - let change_memo = None; + let change_memo = Some(Memo::from_str("change").expect("valid memo").encode()); // We use `st.propose_standard_transfer` here in order to also test round-trip // serialization of the proposal.