zcash_client_backend: Move change memos into the ChangeValue components of Proposals

zcash_client_backend/CHANGELOG.md

@@ -26,6 +26,23 @@ and this library adheres to Rust's notion of
     an `Amount`.
   - All uses of `Amount` in `data_api::wallet::input_selection` have been replaced
     with `NonNegativeAmount`.
+  - `wallet::shield_transparent_funds` no longer
+    takes a `memo` argument; instead, memos to be associated with the shielded
+    outputs should be specified in the construction of the value of the
+    `input_selector` argument, which is used to construct the proposed shielded
+    values as internal "change" outputs.
+  - `wallet::create_proposed_transaction` no longer takes a
+    `change_memo` argument; instead, change memos are represented in the
+    individual values of the `proposed_change` field of the `Proposal`'s
+    `TransactionBalance`.
+  - `wallet::create_spend_to_address` now takes an additional
+    `change_memo` argument.
+- `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.
+- `zcash_client_backend::fees::fixed::SingleOutputChangeStrategy::new` and
+  `zcash_client_backend::fees::zip317::SingleOutputChangeStrategy::new` each now
+  accept an additional `change_memo` argument.
 - All uses of `Amount` in `zcash_client_backend::fees` have been replaced
   with `NonNegativeAmount`.
 

zcash_client_backend/src/data_api/wallet.rs

@@ -118,6 +118,7 @@ where
 ///   received note must have in the blockchain in order to be considered for being
 ///   spent. A value of 10 confirmations is recommended and 0-conf transactions are
 ///   not supported.
+/// * `change_memo`: A memo to be included in the change output
 ///
 /// # Examples
 ///
@@ -174,7 +175,8 @@ where
 ///     Amount::from_u64(1).unwrap(),
 ///     None,
 ///     OvkPolicy::Sender,
-///     10
+///     10,
+///     None
 /// )
 ///
 /// # }
@@ -196,6 +198,7 @@ pub fn create_spend_to_address<DbT, ParamsT>(
     memo: Option<MemoBytes>,
     ovk_policy: OvkPolicy,
     min_confirmations: NonZeroU32,
+    change_memo: Option<MemoBytes>,
 ) -> Result<
     TxId,
     Error<
@@ -224,7 +227,7 @@ where
 
     #[allow(deprecated)]
     let fee_rule = fixed::FeeRule::standard();
-    let change_strategy = fees::fixed::SingleOutputChangeStrategy::new(fee_rule);
+    let change_strategy = fees::fixed::SingleOutputChangeStrategy::new(fee_rule, change_memo);
     spend(
         wallet_db,
         params,
@@ -335,7 +338,6 @@ where
         ovk_policy,
         proposal,
         min_confirmations,
-        None,
     )
 }
 
@@ -418,7 +420,6 @@ pub fn create_proposed_transaction<DbT, ParamsT, InputsErrT, FeeRuleT>(
     ovk_policy: OvkPolicy,
     proposal: Proposal<FeeRuleT, DbT::NoteRef>,
     min_confirmations: NonZeroU32,
-    change_memo: Option<MemoBytes>,
 ) -> Result<
     TxId,
     Error<
@@ -566,22 +567,20 @@ where
 
     for change_value in proposal.balance().proposed_change() {
         match change_value {
-            ChangeValue::Sapling(amount) => {
-                let memo = change_memo
-                    .as_ref()
-                    .map_or_else(MemoBytes::empty, |m| m.clone());
+            ChangeValue::Sapling { value, memo } => {
+                let memo = memo.as_ref().map_or_else(MemoBytes::empty, |m| m.clone());
                 builder.add_sapling_output(
                     internal_ovk(),
                     dfvk.change_address().1,
-                    amount.into(),
+                    value.into(),
                     memo.clone(),
                 )?;
                 sapling_output_meta.push((
                     Recipient::InternalAccount(
                         account,
                         PoolType::Shielded(ShieldedProtocol::Sapling),
                     ),
-                    amount.into(),
+                    value.into(),
                     Some(memo),
                 ))
             }
@@ -682,10 +681,6 @@ where
 /// * `from_addrs`: The list of transparent addresses that will be used to filter transaparent
 ///   UTXOs received by the wallet. Only UTXOs received at one of the provided addresses will
 ///   be selected to be shielded.
-/// * `memo`: A memo to be included in the output to the (internal) recipient.
-///   This can be used to take notes about auto-shielding operations internal
-///   to the wallet that the wallet can use to improve how it represents those
-///   shielding transactions to the user.
 /// * `min_confirmations`: The minimum number of confirmations that a previously
 ///   received note must have in the blockchain in order to be considered for being
 ///   spent. A value of 10 confirmations is recommended and 0-conf transactions are
@@ -703,7 +698,6 @@ pub fn shield_transparent_funds<DbT, ParamsT, InputsT>(
     shielding_threshold: NonNegativeAmount,
     usk: &UnifiedSpendingKey,
     from_addrs: &[TransparentAddress],
-    memo: &MemoBytes,
     min_confirmations: NonZeroU32,
 ) -> Result<
     TxId,
@@ -737,7 +731,6 @@ where
         OvkPolicy::Sender,
         proposal,
         min_confirmations,
-        Some(memo.clone()),
     )
 }
 

zcash_client_backend/src/fees.rs

@@ -2,6 +2,7 @@
 
 use zcash_primitives::{
     consensus::{self, BlockHeight},
+    memo::MemoBytes,
     transaction::{
         components::{
             amount::{Amount, BalanceError, NonNegativeAmount},
@@ -19,13 +20,28 @@
 /// A proposed change amount and output pool.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum ChangeValue {
-    Sapling(NonNegativeAmount),
+    Sapling {
+        value: NonNegativeAmount,
+        memo: Option<MemoBytes>,
+    },
 }
 
 impl ChangeValue {
+    pub fn sapling(value: NonNegativeAmount, memo: Option<MemoBytes>) -> Self {
+        Self::Sapling { value, memo }
+    }
+
+    /// Returns the value of the change output to be created, in zatoshis.
     pub fn value(&self) -> NonNegativeAmount {
         match self {
-            ChangeValue::Sapling(value) => *value,
+            ChangeValue::Sapling { value, .. } => *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(),
         }
     }
 }
@@ -60,7 +76,7 @@
         })
     }
 
-    /// The change values proposed by the [`ChangeStrategy`] that computed this balance.  
+    /// The change values proposed by the [`ChangeStrategy`] that computed this balance.
     pub fn proposed_change(&self) -> &[ChangeValue] {
         &self.proposed_change
     }

zcash_client_backend/src/fees/fixed.rs

@@ -3,6 +3,7 @@
 
 use zcash_primitives::{
     consensus::{self, BlockHeight},
+    memo::MemoBytes,
     transaction::{
         components::{
             amount::{Amount, BalanceError, NonNegativeAmount},
@@ -21,12 +22,17 @@
 /// shielded pool and delegates fee calculation to the provided fee rule.
 pub struct SingleOutputChangeStrategy {
     fee_rule: FixedFeeRule,
+    change_memo: Option<MemoBytes>,
 }
 
 impl SingleOutputChangeStrategy {
-    /// Constructs a new [`SingleOutputChangeStrategy`] with the specified fee rule.
-    pub fn new(fee_rule: FixedFeeRule) -> Self {
-        Self { fee_rule }
+    /// Constructs a new [`SingleOutputChangeStrategy`] with the specified fee rule
+    /// and change memo.
+    pub fn new(fee_rule: FixedFeeRule, change_memo: Option<MemoBytes>) -> Self {
+        Self {
+            fee_rule,
+            change_memo,
+        }
     }
 }
 
@@ -136,7 +142,10 @@
                                 })
                             }
                             DustAction::AllowDustChange => TransactionBalance::new(
-                                vec![ChangeValue::Sapling(proposed_change)],
+                                vec![ChangeValue::sapling(
+                                    proposed_change,
+                                    self.change_memo.clone(),
+                                )],
                                 fee_amount,
                             )
                             .map_err(overflow),
@@ -148,7 +157,10 @@
                         }
                     } else {
                         TransactionBalance::new(
-                            vec![ChangeValue::Sapling(proposed_change)],
+                            vec![ChangeValue::sapling(
+                                proposed_change,
+                                self.change_memo.clone(),
+                            )],
                             fee_amount,
                         )
                         .map_err(overflow)
@@ -185,7 +197,7 @@
     fn change_without_dust() {
         #[allow(deprecated)]
         let fee_rule = FixedFeeRule::standard();
-        let change_strategy = SingleOutputChangeStrategy::new(fee_rule);
+        let change_strategy = SingleOutputChangeStrategy::new(fee_rule, None);
 
         // spend a single Sapling note that is sufficient to pay the fee
         let result = change_strategy.compute_balance(
@@ -205,16 +217,17 @@
 
         assert_matches!(
             result,
-            Ok(balance) if balance.proposed_change() == [ChangeValue::Sapling(NonNegativeAmount::const_from_u64(10000))]
-                && balance.fee_required() == NonNegativeAmount::const_from_u64(10000)
+            Ok(balance) if
+                balance.proposed_change() == [ChangeValue::sapling(NonNegativeAmount::const_from_u64(10000), None)] &&
+                balance.fee_required() == NonNegativeAmount::const_from_u64(10000)
         );
     }
 
     #[test]
     fn dust_change() {
         #[allow(deprecated)]
         let fee_rule = FixedFeeRule::standard();
-        let change_strategy = SingleOutputChangeStrategy::new(fee_rule);
+        let change_strategy = SingleOutputChangeStrategy::new(fee_rule, None);
 
         // spend a single Sapling note that is sufficient to pay the fee
         let result = change_strategy.compute_balance(

zcash_client_backend/src/fees/zip317.rs

@@ -7,6 +7,7 @@
 
 use zcash_primitives::{
     consensus::{self, BlockHeight},
+    memo::MemoBytes,
     transaction::{
         components::{
             amount::{Amount, BalanceError, NonNegativeAmount},
@@ -28,13 +29,17 @@
 /// shielded pool and delegates fee calculation to the provided fee rule.
 pub struct SingleOutputChangeStrategy {
     fee_rule: Zip317FeeRule,
+    change_memo: Option<MemoBytes>,
 }
 
 impl SingleOutputChangeStrategy {
     /// Constructs a new [`SingleOutputChangeStrategy`] with the specified ZIP 317
-    /// fee parameters.
-    pub fn new(fee_rule: Zip317FeeRule) -> Self {
-        Self { fee_rule }
+    /// fee parameters and change memo.
+    pub fn new(fee_rule: Zip317FeeRule, change_memo: Option<MemoBytes>) -> Self {
+        Self {
+            fee_rule,
+            change_memo,
+        }
     }
 }
 
@@ -204,7 +209,10 @@
                             })
                         }
                         DustAction::AllowDustChange => TransactionBalance::new(
-                            vec![ChangeValue::Sapling(proposed_change)],
+                            vec![ChangeValue::sapling(
+                                proposed_change,
+                                self.change_memo.clone(),
+                            )],
                             fee_amount,
                         )
                         .map_err(|_| overflow()),
@@ -215,8 +223,14 @@
                         .map_err(|_| overflow()),
                     }
                 } else {
-                    TransactionBalance::new(vec![ChangeValue::Sapling(proposed_change)], fee_amount)
-                        .map_err(|_| overflow())
+                    TransactionBalance::new(
+                        vec![ChangeValue::sapling(
+                            proposed_change,
+                            self.change_memo.clone(),
+                        )],
+                        fee_amount,
+                    )
+                    .map_err(|_| overflow())
                 }
             }
         }
@@ -249,7 +263,7 @@
 
     #[test]
     fn change_without_dust() {
-        let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard());
+        let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard(), None);
 
         // spend a single Sapling note that is sufficient to pay the fee
         let result = change_strategy.compute_balance(
@@ -269,14 +283,15 @@
 
         assert_matches!(
             result,
-            Ok(balance) if balance.proposed_change() == [ChangeValue::Sapling(NonNegativeAmount::const_from_u64(5000))]
-                && balance.fee_required() == NonNegativeAmount::const_from_u64(10000)
+            Ok(balance) if
+                balance.proposed_change() == [ChangeValue::sapling(NonNegativeAmount::const_from_u64(5000), None)] &&
+                balance.fee_required() == NonNegativeAmount::const_from_u64(10000)
         );
     }
 
     #[test]
     fn change_with_transparent_payments() {
-        let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard());
+        let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard(), None);
 
         // spend a single Sapling note that is sufficient to pay the fee
         let result = change_strategy.compute_balance(
@@ -306,7 +321,7 @@
 
     #[test]
     fn change_with_allowable_dust() {
-        let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard());
+        let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard(), None);
 
         // spend a single Sapling note that is sufficient to pay the fee
         let result = change_strategy.compute_balance(
@@ -339,7 +354,7 @@
 
     #[test]
     fn change_with_disallowed_dust() {
-        let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard());
+        let change_strategy = SingleOutputChangeStrategy::new(Zip317FeeRule::standard(), None);
 
         // spend a single Sapling note that is sufficient to pay the fee
         let result = change_strategy.compute_balance(

zcash_client_sqlite/src/chain.rs

@@ -532,7 +532,7 @@ mod tests {
         }])
         .unwrap();
         let input_selector = GreedyInputSelector::new(
-            SingleOutputChangeStrategy::new(FeeRule::standard()),
+            SingleOutputChangeStrategy::new(FeeRule::standard(), None),
             DustOutputPolicy::default(),
         );
         assert_matches!(