zcash_client_backend: Add fees::zip317::MultiOutputChangeStrategy.

components/zcash_protocol/CHANGELOG.md

@@ -7,6 +7,10 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 
+### Added
+- `zcash_protocol::value::DivRem`
+- `zcash_protocol::value::Zatoshis::div_with_remainder`
+
 ## [0.4.0] - 2024-10-02
 ### Added
 - `impl Sub<BlockHeight> for BlockHeight` unlike the implementation that was

components/zcash_protocol/src/value.rs

@@ -1,6 +1,7 @@
 use std::convert::{Infallible, TryFrom};
 use std::error;
 use std::iter::Sum;
+use std::num::NonZeroUsize;
 use std::ops::{Add, Mul, Neg, Sub};
 
 use memuse::DynamicUsage;
@@ -229,6 +230,24 @@ impl Mul<usize> for ZatBalance {
 #[derive(Clone, Copy, Debug, PartialEq, PartialOrd, Eq, Ord)]
 pub struct Zatoshis(u64);
 
+/// A struct that provides both the dividend and remainder of a division operation.
+pub struct DivRem<A> {
+    dividend: A,
+    remainder: A,
+}
+
+impl<A> DivRem<A> {
+    /// Returns the dividend portion of the value.
+    pub fn dividend(&self) -> &A {
+        &self.dividend
+    }
+
+    /// Returns the remainder portion of the value.
+    pub fn remainder(&self) -> &A {
+        &self.remainder
+    }
+}
+
 impl Zatoshis {
     /// Returns the identity `Zatoshis`
     pub const ZERO: Self = Zatoshis(0);
@@ -298,6 +317,15 @@ impl Zatoshis {
     pub fn is_positive(&self) -> bool {
         self > &Zatoshis::ZERO
     }
+
+    /// Divides this `Zatoshis` value by the given divisor and returns the dividend and remainder.
+    pub fn div_with_remainder(&self, divisor: NonZeroUsize) -> DivRem<Zatoshis> {
+        let divisor = u64::try_from(usize::from(divisor)).expect("divisor fits into a u64");
+        DivRem {
+            dividend: Zatoshis(self.0 / divisor),
+            remainder: Zatoshis(self.0 % divisor),
+        }
+    }
 }
 
 impl From<Zatoshis> for ZatBalance {

zcash_client_backend/CHANGELOG.md

@@ -11,6 +11,8 @@ and this library adheres to Rust's notion of
 - `zcash_client_backend::data_api`:
   - `WalletMeta`
   - `impl Default for wallet::input_selection::GreedyInputSelector`
+- `zcash_client_backend::fees::SplitPolicy`
+- `zcash_client_backend::fees::zip317::MultiOutputChangeStrategy`
 
 ### Changed
 - `zcash_client_backend::data_api`:

zcash_client_backend/src/data_api.rs

@@ -778,7 +778,7 @@ impl<NoteRef> SpendableNotes<NoteRef> {
     }
 }
 
-/// Metadata about the structure of the wallet for a particular account. 
+/// Metadata about the structure of the wallet for a particular account.
 ///
 /// At present this just contains counts of unspent outputs in each pool, but it may be extended in
 /// the future to contain note values or other more detailed information about wallet structure.

zcash_client_backend/src/fees.rs

@@ -1,4 +1,7 @@
-use std::fmt::{self, Debug, Display};
+use std::{
+    fmt::{self, Debug, Display},
+    num::NonZeroUsize,
+};
 
 use zcash_primitives::{
     consensus::{self, BlockHeight},
@@ -336,6 +339,67 @@ impl Default for DustOutputPolicy {
     }
 }
 
+/// A policy that describes how change output should be split into multiple notes for the purpose
+/// of note management.
+#[derive(Clone, Copy, Debug)]
+pub struct SplitPolicy {
+    target_output_count: NonZeroUsize,
+    min_split_output_size: NonNegativeAmount,
+}
+
+impl SplitPolicy {
+    /// Constructs a new [`SplitPolicy`] from its constituent parts.
+    pub fn new(
+        target_output_count: NonZeroUsize,
+        min_split_output_size: NonNegativeAmount,
+    ) -> Self {
+        Self {
+            target_output_count,
+            min_split_output_size,
+        }
+    }
+
+    /// Constructs a [`SplitPolicy`] that prescribes a single output (no splitting).
+    pub fn single_output() -> Self {
+        Self {
+            target_output_count: NonZeroUsize::MIN,
+            min_split_output_size: NonNegativeAmount::ZERO,
+        }
+    }
+
+    /// Returns the minimum value for a note resulting from splitting of change.
+    ///
+    /// If splitting change would result in notes of value less than the minimum split output size,
+    /// a smaller number of splits should be chosen.
+    pub fn min_split_output_size(&self) -> NonNegativeAmount {
+        self.min_split_output_size
+    }
+
+    /// Returns the number of output notes to produce from the given total change value, given the
+    /// number of existing unspent notes in the account and this policy.
+    pub fn split_count(
+        &self,
+        existing_notes: usize,
+        total_change: NonNegativeAmount,
+    ) -> NonZeroUsize {
+        let mut split_count =
+            NonZeroUsize::new(usize::from(self.target_output_count).saturating_sub(existing_notes))
+                .unwrap_or(NonZeroUsize::MIN);
+
+        loop {
+            let per_output_change = total_change.div_with_remainder(split_count);
+            if split_count > NonZeroUsize::MIN
+                && *per_output_change.dividend() < self.min_split_output_size
+            {
+                // safety: `split_count` has just been verified to be > 1
+                split_count = unsafe { NonZeroUsize::new_unchecked(usize::from(split_count) - 1) };
+            } else {
+                return split_count;
+            }
+        }
+    }
+}
+
 /// `EphemeralBalance` describes the ephemeral input or output value for a transaction. It is used
 /// in fee computation for series of transactions that use an ephemeral transparent output in an
 /// intermediate step, such as when sending from a shielded pool to a [ZIP 320] "TEX" address.

zcash_client_backend/src/fees/common.rs

@@ -1,4 +1,5 @@
 use core::cmp::{max, min};
+use std::num::NonZeroUsize;
 
 use zcash_primitives::{
     consensus::{self, BlockHeight},
@@ -10,9 +11,11 @@ use zcash_primitives::{
 };
 use zcash_protocol::ShieldedProtocol;
 
+use crate::data_api::WalletMeta;
+
 use super::{
     sapling as sapling_fees, ChangeError, ChangeValue, DustAction, DustOutputPolicy,
-    EphemeralBalance, TransactionBalance,
+    EphemeralBalance, SplitPolicy, TransactionBalance,
 };
 
 #[cfg(feature = "orchard")]
@@ -112,33 +115,26 @@ where
 }
 
 /// Decide which shielded pool change should go to if there is any.
-pub(crate) fn single_change_output_policy(
+pub(crate) fn select_change_pool(
     _net_flows: &NetFlows,
     _fallback_change_pool: ShieldedProtocol,
-) -> (ShieldedProtocol, usize, usize) {
+) -> ShieldedProtocol {
     // TODO: implement a less naive strategy for selecting the pool to which change will be sent.
-    let change_pool = {
-        #[cfg(feature = "orchard")]
-        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
-        } 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
-        } else {
-            // The flows are transparent, so there may not be change. If there is, the caller
-            // gets to decide where to shield it.
-            _fallback_change_pool
-        }
-        #[cfg(not(feature = "orchard"))]
+    #[cfg(feature = "orchard")]
+    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
+    } 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
-    };
-    (
-        change_pool,
-        (change_pool == ShieldedProtocol::Sapling).into(),
-        (change_pool == ShieldedProtocol::Orchard).into(),
-    )
+    } else {
+        // The flows are transparent, so there may not be change. If there is, the caller
+        // gets to decide where to shield it.
+        _fallback_change_pool
+    }
+    #[cfg(not(feature = "orchard"))]
+    ShieldedProtocol::Sapling
 }
 
 #[derive(Clone, Copy, Debug)]
@@ -169,6 +165,7 @@ pub(crate) struct SinglePoolBalanceConfig<'a, P, F> {
     fee_rule: &'a F,
     dust_output_policy: &'a DustOutputPolicy,
     default_dust_threshold: NonNegativeAmount,
+    split_policy: &'a SplitPolicy,
     fallback_change_pool: ShieldedProtocol,
     marginal_fee: NonNegativeAmount,
     grace_actions: usize,
@@ -180,6 +177,7 @@ impl<'a, P, F> SinglePoolBalanceConfig<'a, P, F> {
         fee_rule: &'a F,
         dust_output_policy: &'a DustOutputPolicy,
         default_dust_threshold: NonNegativeAmount,
+        split_policy: &'a SplitPolicy,
         fallback_change_pool: ShieldedProtocol,
         marginal_fee: NonNegativeAmount,
         grace_actions: usize,
@@ -189,6 +187,7 @@ impl<'a, P, F> SinglePoolBalanceConfig<'a, P, F> {
             fee_rule,
             dust_output_policy,
             default_dust_threshold,
+            split_policy,
             fallback_change_pool,
             marginal_fee,
             grace_actions,
@@ -197,13 +196,9 @@ impl<'a, P, F> SinglePoolBalanceConfig<'a, P, F> {
 }
 
 #[allow(clippy::too_many_arguments)]
-pub(crate) fn single_change_output_balance<
-    P: consensus::Parameters,
-    NoteRefT: Clone,
-    F: FeeRule,
-    E,
->(
+pub(crate) fn single_pool_output_balance<P: consensus::Parameters, NoteRefT: Clone, F: FeeRule, E>(
     cfg: SinglePoolBalanceConfig<P, F>,
+    wallet_meta: Option<&WalletMeta>,
     target_height: BlockHeight,
     transparent_inputs: &[impl transparent::InputView],
     transparent_outputs: &[impl transparent::OutputView],
@@ -232,9 +227,16 @@ where
         ephemeral_balance,
     )?;
 
-    #[allow(unused_variables)]
-    let (change_pool, sapling_change, orchard_change) =
-        single_change_output_policy(&net_flows, cfg.fallback_change_pool);
+    let change_pool = select_change_pool(&net_flows, cfg.fallback_change_pool);
+    let target_change_counts = OutputManifest {
+        transparent: 0,
+        sapling: (change_pool == ShieldedProtocol::Sapling)
+            .then(|| cfg.split_policy.target_output_count.into())
+            .unwrap_or(0),
+        orchard: (change_pool == ShieldedProtocol::Orchard)
+            .then(|| cfg.split_policy.target_output_count.into())
+            .unwrap_or(0),
+    };
 
     // We don't create a fully-transparent transaction if a change memo is used.
     let transparent = net_flows.is_transparent() && change_memo.is_none();
@@ -246,20 +248,17 @@ where
         // Is it certain that there will be a change output? If it is not certain,
         // we should call `check_for_uneconomic_inputs` with `possible_change`
         // including both possibilities.
-        let possible_change =
+        let possible_change = {
             // These are the situations where we might not have a change output.
-            if transparent || (cfg.dust_output_policy.action() == DustAction::AddDustToFee && change_memo.is_none()) {
-                vec![
-                    OutputManifest::ZERO,
-                    OutputManifest {
-                        transparent: 0,
-                        sapling: sapling_change,
-                        orchard: orchard_change
-                    }
-                ]
+            if transparent
+                || (cfg.dust_output_policy.action() == DustAction::AddDustToFee
+                    && change_memo.is_none())
+            {
+                vec![OutputManifest::ZERO, target_change_counts]
             } else {
-                vec![OutputManifest { transparent: 0, sapling: sapling_change, orchard: orchard_change}]
-            };
+                vec![target_change_counts]
+            }
+        };
 
         check_for_uneconomic_inputs(
             transparent_inputs,
@@ -293,7 +292,7 @@ where
         .bundle_type()
         .num_outputs(
             sapling.inputs().len(),
-            sapling.outputs().len() + sapling_change,
+            sapling.outputs().len() + target_change_counts.sapling(),
         )
         .map_err(ChangeError::BundleError)?;
 
@@ -307,7 +306,7 @@ where
         .bundle_type()
         .num_actions(
             orchard.inputs().len(),
-            orchard.outputs().len() + orchard_change,
+            orchard.outputs().len() + target_change_counts.orchard(),
         )
         .map_err(ChangeError::BundleError)?;
     #[cfg(not(feature = "orchard"))]
@@ -410,13 +409,35 @@ where
             // Case 3b or 3c.
             let proposed_change =
                 (total_in - total_out_plus_fee_with_change).expect("checked above");
+
+            // We obtain a split count based on the total number of notes of sufficient size
+            // available in the wallet, irrespective of pool. If we don't have any wallet metadata
+            // available, we fall back to generating a single change output.
+            let split_count = wallet_meta.map_or(NonZeroUsize::MIN, |wm| {
+                cfg.split_policy
+                    .split_count(wm.total_note_count(), proposed_change)
+            });
+            let per_output_change = proposed_change.div_with_remainder(split_count);
+
             let simple_case = || {
                 (
-                    vec![ChangeValue::shielded(
-                        change_pool,
-                        proposed_change,
-                        change_memo.cloned(),
-                    )],
+                    (0usize..split_count.into())
+                        .map(|i| {
+                            ChangeValue::shielded(
+                                change_pool,
+                                if i == 0 {
+                                    // Add any remainder to the first output only
+                                    (*per_output_change.dividend() + *per_output_change.remainder())
+                                        .unwrap()
+                                } else {
+                                    // For any other output, the change value will just be the
+                                    // dividend.
+                                    *per_output_change.dividend()
+                                },
+                                change_memo.cloned(),
+                            )
+                        })
+                        .collect(),
                     fee_with_change,
                 )
             };
@@ -426,7 +447,7 @@ where
                 .dust_threshold()
                 .unwrap_or(cfg.default_dust_threshold);
 
-            if proposed_change < change_dust_threshold {
+            if per_output_change.dividend() < &change_dust_threshold {
                 match cfg.dust_output_policy.action() {
                     DustAction::Reject => {
                         // Always allow zero-valued change even for the `Reject` policy:
@@ -435,11 +456,11 @@ where
                         // * this case occurs in practice when sending all funds from an account;
                         // * zero-valued notes do not require witness tracking;
                         // * the effect on trial decryption overhead is small.
-                        if proposed_change.is_zero() {
+                        if per_output_change.dividend().is_zero() {
                             simple_case()
                         } else {
-                            let shortfall =
-                                (change_dust_threshold - proposed_change).ok_or_else(underflow)?;
+                            let shortfall = (change_dust_threshold - *per_output_change.dividend())
+                                .ok_or_else(underflow)?;
 
                             return Err(ChangeError::InsufficientFunds {
                                 available: total_in,