Merge pull request #658 from nuttycom/wallet/builder_explicit_change

Update the transaction builder to make change outputs explicit

zcash_client_backend/CHANGELOG.md

@@ -57,6 +57,13 @@ and this library adheres to Rust's notion of
   - `KeyError`
   - `AddressCodec` implementations for `sapling::PaymentAddress` and
     `UnifiedAddress`
+- `zcash_client_backend::fees`
+  - `ChangeError`
+  - `ChangeStrategy`
+  - `ChangeValue`
+  - `TransactionBalance`
+  - `BasicFixedFeeChangeStrategy` - a `ChangeStrategy` implementation that
+    reproduces current wallet change behavior
 - New experimental APIs that should be considered unstable, and are
   likely to be modified and/or moved to a different module in a future
   release:
@@ -112,6 +119,8 @@ and this library adheres to Rust's notion of
 - `data_api::error::Error::Protobuf` now wraps `prost::DecodeError` instead of
   `protobuf::ProtobufError`.
 - `data_api::error::Error` has the following additional cases:
+  - `Error::BalanceError` in the case of amount addition overflow
+    or subtraction underflow.
   - `Error::MemoForbidden` to report the condition where a memo was
     specified to be sent to a transparent recipient.
   - `Error::TransparentInputsNotSupported` to represent the condition

zcash_client_backend/src/data_api/error.rs

@@ -6,7 +6,11 @@ use zcash_address::unified::Typecode;
 use zcash_primitives::{
     consensus::BlockHeight,
     sapling::Node,
-    transaction::{builder, components::amount::Amount, TxId},
+    transaction::{
+        builder,
+        components::amount::{Amount, BalanceError},
+        TxId,
+    },
     zip32::AccountId,
 };
 
@@ -33,8 +37,8 @@ pub enum Error<NoteId> {
     /// No account with the given identifier was found in the wallet.
     AccountNotFound(AccountId),
 
-    /// The amount specified exceeds the allowed range.
+    /// Zcash amount computation encountered an overflow or underflow.
-    InvalidAmount,
+    BalanceError(BalanceError),
 
     /// Unable to create a new spend because the wallet balance is not sufficient.
     /// The first argument is the amount available, the second is the amount needed
@@ -113,9 +117,9 @@ impl<N: fmt::Display> fmt::Display for Error<N> {
             Error::AccountNotFound(account) => {
                 write!(f, "Wallet does not contain account {}", u32::from(*account))
             }
-            Error::InvalidAmount => write!(
+            Error::BalanceError(e) => write!(
                 f,
-                "The value lies outside the valid range of Zcash amounts."
+                "The value lies outside the valid range of Zcash amounts: {:?}.", e
             ),
             Error::InsufficientBalance(have, need) => write!(
                 f,

zcash_client_backend/src/data_api/wallet.rs

@@ -5,9 +5,10 @@ use zcash_primitives::{
     sapling::prover::TxProver,
     transaction::{
         builder::Builder,
-        components::{amount::DEFAULT_FEE, Amount},
+        components::amount::{Amount, BalanceError, DEFAULT_FEE},
         Transaction,
     },
+    zip32::Scope,
 };
 
 use crate::{
@@ -17,6 +18,7 @@ use crate::{
         SentTransactionOutput, WalletWrite,
     },
     decrypt_transaction,
+    fees::{BasicFixedFeeChangeStrategy, ChangeError, ChangeStrategy, ChangeValue},
     keys::UnifiedSpendingKey,
     wallet::OvkPolicy,
     zip321::{Payment, TransactionRequest},
@@ -92,19 +94,18 @@ where
 /// * `wallet_db`: A read/write reference to the wallet database
 /// * `params`: Consensus parameters
 /// * `prover`: The TxProver to use in constructing the shielded transaction.
-/// * `account`: The ZIP32 account identifier associated with the extended spending
+/// * `usk`: The unified spending key that controls the funds that will be spent
-///   key that controls the funds to be used in creating this transaction.  This
+///   in the resulting transaction. This procedure will return an error if the
-///   procedure will return an error if this does not correctly correspond to `extsk`.
+///   USK does not correspond to an account known to the wallet.
-/// * `extsk`: The extended spending key that controls the funds that will be spent
-///   in the resulting transaction.
-/// * `amount`: The amount to send.
 /// * `to`: The address to which `amount` will be paid.
+/// * `amount`: The amount to send.
 /// * `memo`: A memo to be included in the output to the recipient.
 /// * `ovk_policy`: The policy to use for constructing outgoing viewing keys that
 ///   can allow the sender to view the resulting notes on the blockchain.
 /// * `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.
+///
 /// # Examples
 ///
 /// ```
@@ -236,10 +237,8 @@ where
 /// * `params`: Consensus parameters
 /// * `prover`: The TxProver to use in constructing the shielded transaction.
 /// * `usk`: The unified spending key that controls the funds that will be spent
-///   in the resulting transaction.
+///   in the resulting transaction. This procedure will return an error if the
-/// * `account`: The ZIP32 account identifier associated with the extended spending
+///   USK does not correspond to an account known to the wallet.
-///   key that controls the funds to be used in creating this transaction.  This
-///   procedure will return an error if this does not correctly correspond to `extsk`.
 /// * `request`: The ZIP-321 payment request specifying the recipients and amounts
 ///   for the transaction.
 /// * `ovk_policy`: The policy to use for constructing outgoing viewing keys that
@@ -267,17 +266,17 @@ where
         .get_account_for_ufvk(&usk.to_unified_full_viewing_key())?
         .ok_or(Error::KeyNotRecognized)?;
 
-    let extfvk = usk.sapling().to_extended_full_viewing_key();
+    let dfvk = usk.sapling().to_diversifiable_full_viewing_key();
 
     // Apply the outgoing viewing key policy.
     let ovk = match ovk_policy {
-        OvkPolicy::Sender => Some(extfvk.fvk.ovk),
+        OvkPolicy::Sender => Some(dfvk.fvk().ovk),
         OvkPolicy::Custom(ovk) => Some(ovk),
         OvkPolicy::Discard => None,
     };
 
     // Target the next block, assuming we are up-to-date.
-    let (height, anchor_height) = wallet_db
+    let (target_height, anchor_height) = wallet_db
         .get_target_and_anchor_heights(min_confirmations)
         .and_then(|x| x.ok_or_else(|| Error::ScanRequired.into()))?;
 
@@ -286,8 +285,9 @@ where
         .iter()
         .map(|p| p.amount)
         .sum::<Option<Amount>>()
-        .ok_or_else(|| E::from(Error::InvalidAmount))?;
+        .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?;
-    let target_value = (value + DEFAULT_FEE).ok_or_else(|| E::from(Error::InvalidAmount))?;
+    let target_value = (value + DEFAULT_FEE)
+        .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?;
     let spendable_notes =
         wallet_db.select_spendable_sapling_notes(account, target_value, anchor_height)?;
 
@@ -296,7 +296,7 @@ where
         .iter()
         .map(|n| n.note_value)
         .sum::<Option<_>>()
-        .ok_or_else(|| E::from(Error::InvalidAmount))?;
+        .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?;
     if selected_value < target_value {
         return Err(E::from(Error::InsufficientBalance(
             selected_value,
@@ -305,10 +305,10 @@ where
     }
 
     // Create the transaction
-    let mut builder = Builder::new_with_fee(params.clone(), height, DEFAULT_FEE);
+    let mut builder = Builder::new(params.clone(), target_height);
     for selected in spendable_notes {
-        let from = extfvk
+        let from = dfvk
-            .fvk
+            .fvk()
             .vk
             .to_payment_address(selected.diversifier)
             .unwrap(); //DiversifyHash would have to unexpectedly return the zero point for this to be None
@@ -361,7 +361,42 @@ where
         }?
     }
 
-    let (tx, tx_metadata) = builder.build(&prover).map_err(Error::Builder)?;
+    let fee_strategy = BasicFixedFeeChangeStrategy::new(DEFAULT_FEE);
+    let balance = fee_strategy
+        .compute_balance(
+            params,
+            target_height,
+            builder.transparent_inputs(),
+            builder.transparent_outputs(),
+            builder.sapling_inputs(),
+            builder.sapling_outputs(),
+        )
+        .map_err(|e| match e {
+            ChangeError::InsufficientFunds {
+                available,
+                required,
+            } => Error::InsufficientBalance(available, required),
+            ChangeError::StrategyError(e) => Error::BalanceError(e),
+        })?;
+
+    for change_value in balance.proposed_change() {
+        match change_value {
+            ChangeValue::Sapling(amount) => {
+                builder
+                    .add_sapling_output(
+                        Some(dfvk.to_ovk(Scope::Internal)),
+                        dfvk.change_address().1,
+                        *amount,
+                        MemoBytes::empty(),
+                    )
+                    .map_err(Error::Builder)?;
+            }
+        }
+    }
+
+    let (tx, tx_metadata) = builder
+        .build(&prover, &fee_strategy.fee_rule())
+        .map_err(Error::Builder)?;
 
     let sent_outputs = request.payments().iter().enumerate().map(|(i, payment)| {
         let (output_index, recipient) = match &payment.recipient_address {
@@ -405,7 +440,7 @@ where
         created: time::OffsetDateTime::now_utc(),
         account,
         outputs: sent_outputs,
-        fee_amount: DEFAULT_FEE,
+        fee_amount: balance.fee_required(),
         #[cfg(feature = "transparent-inputs")]
         utxos_spent: vec![],
     })
@@ -423,12 +458,12 @@ where
 /// * `wallet_db`: A read/write reference to the wallet database
 /// * `params`: Consensus parameters
 /// * `prover`: The TxProver to use in constructing the shielded transaction.
-/// * `sk`: The secp256k1 secret key that will be used to detect and spend transparent
+/// * `usk`: The unified spending key that will be used to detect and spend transparent UTXOs,
-///   UTXOs.
+///   and that will provide the shielded address to which funds will be sent. Funds will be
-/// * `account`: The ZIP32 account identifier for the account to which funds will
+///   shielded to the internal (change) address associated with the most preferred shielded
-///   be shielded. Funds will be shielded to the internal (change) address associated with the
+///   receiver corresponding to this account, or if no shielded receiver can be used for this
-///   most preferred shielded receiver corresponding to this account, or if no shielded
+///   account, this function will return an error. This procedure will return an error if the
-///   receiver can be used for this account, this function will return an error.
+///   USK does not correspond to an account known to the wallet.
 /// * `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
@@ -462,7 +497,7 @@ where
         .to_diversifiable_full_viewing_key()
         .change_address()
         .1;
-    let (latest_scanned_height, latest_anchor) = wallet_db
+    let (target_height, latest_anchor) = wallet_db
         .get_target_and_anchor_heights(min_confirmations)
         .and_then(|x| x.ok_or_else(|| Error::ScanRequired.into()))?;
 
@@ -476,21 +511,15 @@ where
         utxos.append(&mut outputs);
     }
 
-    let total_amount = utxos
+    let _total_amount = utxos
         .iter()
         .map(|utxo| utxo.txout().value)
         .sum::<Option<Amount>>()
-        .ok_or_else(|| E::from(Error::InvalidAmount))?;
+        .ok_or_else(|| E::from(Error::BalanceError(BalanceError::Overflow)))?;
-
-    let fee = DEFAULT_FEE;
-    if fee >= total_amount {
-        return Err(E::from(Error::InsufficientBalance(total_amount, fee)));
-    }
-
-    let amount_to_shield = (total_amount - fee).ok_or_else(|| E::from(Error::InvalidAmount))?;
 
     let addr_metadata = wallet_db.get_transparent_receivers(account)?;
-    let mut builder = Builder::new_with_fee(params.clone(), latest_scanned_height, fee);
+    let mut builder = Builder::new(params.clone(), target_height);
+
     for utxo in &utxos {
         let diversifier_index = addr_metadata
             .get(utxo.recipient_address())
@@ -510,15 +539,44 @@ where
             .map_err(Error::Builder)?;
     }
 
-    // there are no sapling notes so we set the change manually
+    // Compute the balance of the transaction. We have only added inputs, so the total change
-    builder.send_change_to(ovk, shielding_address.clone());
+    // amount required will be the total of the UTXOs minus fees.
+    let fee_strategy = BasicFixedFeeChangeStrategy::new(DEFAULT_FEE);
+    let balance = fee_strategy
+        .compute_balance(
+            params,
+            target_height,
+            builder.transparent_inputs(),
+            builder.transparent_outputs(),
+            builder.sapling_inputs(),
+            builder.sapling_outputs(),
+        )
+        .map_err(|e| match e {
+            ChangeError::InsufficientFunds {
+                available,
+                required,
+            } => Error::InsufficientBalance(available, required),
+            ChangeError::StrategyError(e) => Error::BalanceError(e),
+        })?;
 
-    // add the sapling output to shield the funds
+    let fee = balance.fee_required();
+    let mut total_out = Amount::zero();
+    for change_value in balance.proposed_change() {
+        total_out = (total_out + change_value.value())
+            .ok_or(Error::BalanceError(BalanceError::Overflow))?;
+        match change_value {
+            ChangeValue::Sapling(amount) => {
                 builder
-        .add_sapling_output(Some(ovk), shielding_address, amount_to_shield, memo.clone())
+                    .add_sapling_output(Some(ovk), shielding_address.clone(), *amount, memo.clone())
                     .map_err(Error::Builder)?;
+            }
+        }
+    }
 
-    let (tx, tx_metadata) = builder.build(&prover).map_err(Error::Builder)?;
+    // The transaction build process will check that the inputs and outputs balance
+    let (tx, tx_metadata) = builder
+        .build(&prover, &fee_strategy.fee_rule())
+        .map_err(Error::Builder)?;
     let output_index = tx_metadata.output_index(0).expect(
         "No sapling note was created in autoshielding transaction. This is a programming error.",
     );
@@ -529,8 +587,8 @@ where
         account,
         outputs: vec![SentTransactionOutput {
             output_index,
-            value: amount_to_shield,
             recipient: Recipient::InternalAccount(account, PoolType::Sapling),
+            value: total_out,
             memo: Some(memo.clone()),
         }],
         fee_amount: fee,

zcash_client_backend/src/fees.rs

@@ -0,0 +1,163 @@
+use zcash_primitives::{
+    consensus::{self, BlockHeight},
+    transaction::{
+        components::{
+            amount::{Amount, BalanceError},
+            sapling::fees as sapling,
+            transparent::fees as transparent,
+        },
+        fees::{FeeRule, FixedFeeRule},
+    },
+};
+
+/// A proposed change amount and output pool.
+pub enum ChangeValue {
+    Sapling(Amount),
+}
+
+impl ChangeValue {
+    pub fn value(&self) -> Amount {
+        match self {
+            ChangeValue::Sapling(value) => *value,
+        }
+    }
+}
+
+/// The amount of change and fees required to make a transaction's inputs and
+/// outputs balance under a specific fee rule, as computed by a particular
+/// [`ChangeStrategy`] that is aware of that rule.
+pub struct TransactionBalance {
+    proposed_change: Vec<ChangeValue>,
+    fee_required: Amount,
+}
+
+impl TransactionBalance {
+    /// Constructs a new balance from its constituent parts.
+    pub fn new(proposed_change: Vec<ChangeValue>, fee_required: Amount) -> Self {
+        TransactionBalance {
+            proposed_change,
+            fee_required,
+        }
+    }
+
+    /// The change values proposed by the [`ChangeStrategy`] that computed this balance.  
+    pub fn proposed_change(&self) -> &[ChangeValue] {
+        &self.proposed_change
+    }
+
+    /// Returns the fee computed for the transaction, assuming that the suggested
+    /// change outputs are added to the transaction.
+    pub fn fee_required(&self) -> Amount {
+        self.fee_required
+    }
+}
+
+/// Errors that can occur in computing suggested change and/or fees.
+pub enum ChangeError<E> {
+    InsufficientFunds { available: Amount, required: Amount },
+    StrategyError(E),
+}
+
+/// A trait that represents the ability to compute the suggested change and fees that must be paid
+/// by a transaction having a specified set of inputs and outputs.
+pub trait ChangeStrategy {
+    type FeeRule: FeeRule;
+    type Error;
+
+    /// Returns the fee rule that this change strategy will respect when performing
+    /// balance computations.
+    fn fee_rule(&self) -> Self::FeeRule;
+
+    /// Computes the totals of inputs, suggested change amounts, and fees given the
+    /// provided inputs and outputs being used to construct a transaction.
+    ///
+    /// The fee computed as part of this operation should take into account the prospective
+    /// change outputs recommended by this operation. If insufficient funds are available to
+    /// supply the requested outputs and required fees, implementations should return
+    /// [`ChangeError::InsufficientFunds`].
+    fn compute_balance<P: consensus::Parameters>(
+        &self,
+        params: &P,
+        target_height: BlockHeight,
+        transparent_inputs: &[impl transparent::InputView],
+        transparent_outputs: &[impl transparent::OutputView],
+        sapling_inputs: &[impl sapling::InputView],
+        sapling_outputs: &[impl sapling::OutputView],
+    ) -> Result<TransactionBalance, ChangeError<Self::Error>>;
+}
+
+/// A change strategy that uses a fixed fee amount and proposes change as a single output
+/// to the most current supported pool.
+pub struct BasicFixedFeeChangeStrategy {
+    fixed_fee: Amount,
+}
+
+impl BasicFixedFeeChangeStrategy {
+    // Constructs a new [`BasicFixedFeeChangeStrategy`] with the specified fixed fee
+    // amount.
+    pub fn new(fixed_fee: Amount) -> Self {
+        Self { fixed_fee }
+    }
+}
+
+impl ChangeStrategy for BasicFixedFeeChangeStrategy {
+    type FeeRule = FixedFeeRule;
+    type Error = BalanceError;
+
+    fn fee_rule(&self) -> Self::FeeRule {
+        FixedFeeRule::new(self.fixed_fee)
+    }
+
+    fn compute_balance<P: consensus::Parameters>(
+        &self,
+        _params: &P,
+        _target_height: BlockHeight,
+        transparent_inputs: &[impl transparent::InputView],
+        transparent_outputs: &[impl transparent::OutputView],
+        sapling_inputs: &[impl sapling::InputView],
+        sapling_outputs: &[impl sapling::OutputView],
+    ) -> Result<TransactionBalance, ChangeError<Self::Error>> {
+        let overflow = || ChangeError::StrategyError(BalanceError::Overflow);
+        let underflow = || ChangeError::StrategyError(BalanceError::Underflow);
+
+        let t_in = transparent_inputs
+            .iter()
+            .map(|t_in| t_in.coin().value)
+            .sum::<Option<_>>()
+            .ok_or_else(overflow)?;
+        let t_out = transparent_outputs
+            .iter()
+            .map(|t_out| t_out.value())
+            .sum::<Option<_>>()
+            .ok_or_else(overflow)?;
+        let sapling_in = sapling_inputs
+            .iter()
+            .map(|s_in| s_in.value())
+            .sum::<Option<_>>()
+            .ok_or_else(overflow)?;
+        let sapling_out = sapling_outputs
+            .iter()
+            .map(|s_out| s_out.value())
+            .sum::<Option<_>>()
+            .ok_or_else(overflow)?;
+
+        let total_in = (t_in + sapling_in).ok_or_else(overflow)?;
+        let total_out = [t_out, sapling_out, self.fixed_fee]
+            .iter()
+            .sum::<Option<Amount>>()
+            .ok_or_else(overflow)?;
+
+        let proposed_change = (total_in - total_out).ok_or_else(underflow)?;
+        if proposed_change < Amount::zero() {
+            Err(ChangeError::InsufficientFunds {
+                available: total_in,
+                required: total_out,
+            })
+        } else {
+            Ok(TransactionBalance::new(
+                vec![ChangeValue::Sapling(proposed_change)],
+                self.fixed_fee,
+            ))
+        }
+    }
+}

zcash_client_backend/src/lib.rs

@@ -12,6 +12,7 @@ pub mod address;
 pub mod data_api;
 mod decrypt;
 pub mod encoding;
+pub mod fees;
 pub mod keys;
 pub mod proto;
 pub mod scan;

zcash_extensions/src/transparent/demo.rs

@@ -495,6 +495,7 @@ mod tests {
                 amount::{Amount, DEFAULT_FEE},
                 tze::{Authorized, Bundle, OutPoint, TzeIn, TzeOut},
             },
+            fees::FixedFeeRule,
             Transaction, TransactionData, TxVersion,
         },
         zip32::ExtendedSpendingKey,
@@ -808,12 +809,13 @@ mod tests {
         //
 
         let mut rng = OsRng;
+        let fee_rule = FixedFeeRule::new(DEFAULT_FEE);
 
         // create some inputs to spend
         let extsk = ExtendedSpendingKey::master(&[]);
         let to = extsk.default_address().1;
         let note1 = to
-            .create_note(110000, Rseed::BeforeZip212(jubjub::Fr::random(&mut rng)))
+            .create_note(101000, Rseed::BeforeZip212(jubjub::Fr::random(&mut rng)))
             .unwrap();
         let cm1 = Node::new(note1.cmu().to_repr());
         let mut tree = CommitmentTree::empty();
@@ -835,7 +837,7 @@ mod tests {
             .unwrap();
         let (tx_a, _) = builder_a
             .txn_builder
-            .build(&prover)
+            .build_zfuture(&prover, &fee_rule)
             .map_err(|e| format!("build failure: {:?}", e))
             .unwrap();
         let tze_a = tx_a.tze_bundle().unwrap();
@@ -853,7 +855,7 @@ mod tests {
             .unwrap();
         let (tx_b, _) = builder_b
             .txn_builder
-            .build(&prover)
+            .build_zfuture(&prover, &fee_rule)
             .map_err(|e| format!("build failure: {:?}", e))
             .unwrap();
         let tze_b = tx_b.tze_bundle().unwrap();
@@ -878,7 +880,7 @@ mod tests {
 
         let (tx_c, _) = builder_c
             .txn_builder
-            .build(&prover)
+            .build_zfuture(&prover, &fee_rule)
             .map_err(|e| format!("build failure: {:?}", e))
             .unwrap();
         let tze_c = tx_c.tze_bundle().unwrap();

zcash_primitives/CHANGELOG.md

@@ -10,6 +10,46 @@ and this library adheres to Rust's notion of
 ### Added
 - Added in `zcash_primitives::zip32`
   - An implementation of `TryFrom<DiversifierIndex>` for `u32`
+- Added to `zcash_primitives::transaction::builder`
+  - `Error::InsufficientFunds`
+  - `Error::ChangeRequired`
+  - `Builder` state accessor methods:
+    - `Builder::params()`
+    - `Builder::target_height()`
+    - `Builder::transparent_inputs()`
+    - `Builder::transparent_outputs()`
+    - `Builder::sapling_inputs()`
+    - `Builder::sapling_outputs()`
+- `zcash_primitives::transaction::fees` a new module containing abstractions
+  and types related to fee calculations.
+  - `FeeRule` a trait that describes how to compute the fee required for a
+    transaction given inputs and outputs to the transaction.
+- Added to `zcash_primitives::transaction::components::sapling::builder`
+  - `SaplingBuilder::{inputs, outputs}`: accessors for Sapling builder state.
+- `zcash_primitives::transaction::components::sapling::fees`
+- Added to `zcash_primitives::transaction::components::transparent::builder`
+  - `TransparentBuilder::{inputs, outputs}`: accessors for transparent builder state.
+- `zcash_primitives::transaction::components::transparent::fees`
+
+### Changed
+- `zcash_primitives::transaction::builder::Builder::build` now takes a `FeeRule`
+  argument which is used to compute the fee for the transaction as part of the
+  build process.
+- `zcash_primitives::transaction::builder::Builder::{new, new_with_rng}` no
+  longer fixes the fee for transactions to 0.00001 ZEC; the builder instead
+  computes the fee using a `FeeRule` implementation at build time.
+
+### Removed
+- Removed from `zcash_primitives::transaction::builder::Builder`
+  - `Builder::{new_with_fee, new_with_rng_and_fee`} (use `Builder::{new, new_with_rng}` 
+    instead along with a `FeeRule` implementation passed to `Builder::build`.)
+  - `Builder::send_change_to` has been removed. Change outputs must be added to the
+    builder by the caller, just like any other output.
+- Removed from `zcash_primitives::transaction::builder::Error`
+  - `Error::ChangeIsNegative`
+  - `Error::NoChangeAddress`
+- `zcash_primitives::transaction::components::sapling::builder::SaplingBuilder::get_candidate_change_address`
+   has been removed; change outputs must now be added by the caller.
 
 ## [0.8.1] - 2022-10-19
 ### Added

zcash_primitives/src/transaction/builder.rs

@@ -1,5 +1,7 @@
 //! Structs for building transactions.
 
+use std::cmp::Ordering;
+use std::convert::Infallible;
 use std::error;
 use std::fmt;
 use std::sync::mpsc::Sender;
@@ -18,13 +20,14 @@ use crate::{
     sapling::{prover::TxProver, Diversifier, Node, Note, PaymentAddress},
     transaction::{
         components::{
-            amount::{Amount, DEFAULT_FEE},
+            amount::Amount,
             sapling::{
                 self,
                 builder::{SaplingBuilder, SaplingMetadata},
             },
             transparent::{self, builder::TransparentBuilder},
         },
+        fees::FeeRule,
         sighash::{signature_hash, SignableInput},
         txid::TxIdDigester,
         Transaction, TransactionData, TxVersion, Unauthorized,
@@ -38,24 +41,33 @@ use crate::transaction::components::transparent::TxOut;
 #[cfg(feature = "zfuture")]
 use crate::{
     extensions::transparent::{ExtensionTxBuilder, ToPayload},
-    transaction::components::{
+    transaction::{
+        components::{
             tze::builder::TzeBuilder,
             tze::{self, TzeOut},
         },
+        fees::FutureFeeRule,
+    },
 };
 
-#[cfg(any(test, feature = "test-dependencies"))]
-use crate::sapling::prover::mock::MockTxProver;
-
 const DEFAULT_TX_EXPIRY_DELTA: u32 = 20;
 
+/// Errors that can occur during transaction construction.
 #[derive(Debug, PartialEq, Eq)]
 pub enum Error {
-    ChangeIsNegative(Amount),
+    /// Insufficient funds were provided to the transaction builder; the given
+    /// additional amount is required in order to construct the transaction.
+    InsufficientFunds(Amount),
+    /// The transaction has inputs in excess of outputs and fees; the user must
+    /// add a change output.
+    ChangeRequired(Amount),
+    /// An overflow or underflow occurred when computing value balances
     InvalidAmount,
-    NoChangeAddress,
+    /// An error occurred in constructing the transparent parts of a transaction.
     TransparentBuild(transparent::builder::Error),
+    /// An error occurred in constructing the Sapling parts of a transaction.
     SaplingBuild(sapling::builder::Error),
+    /// An error occurred in constructing the TZE parts of a transaction.
     #[cfg(feature = "zfuture")]
     TzeBuild(tze::builder::Error),
 }
@@ -63,11 +75,17 @@ pub enum Error {
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            Error::ChangeIsNegative(amount) => {
+            Error::InsufficientFunds(amount) => write!(
-                write!(f, "Change is negative ({:?} zatoshis)", amount)
+                f,
-            }
+                "Insufficient funds for transaction construction; need an additional {:?} zatoshis",
-            Error::InvalidAmount => write!(f, "Invalid amount"),
+                amount
-            Error::NoChangeAddress => write!(f, "No change address specified or discoverable"),
+            ),
+            Error::ChangeRequired(amount) => write!(
+                f,
+                "The transaction requires an additional change output of {:?} zatoshis",
+                amount
+            ),
+            Error::InvalidAmount => write!(f, "Invalid amount (overflow or underflow)"),
             Error::TransparentBuild(err) => err.fmt(f),
             Error::SaplingBuild(err) => err.fmt(f),
             #[cfg(feature = "zfuture")]
@@ -78,6 +96,12 @@ impl fmt::Display for Error {
 
 impl error::Error for Error {}
 
+impl From<Infallible> for Error {
+    fn from(_: Infallible) -> Error {
+        unreachable!()
+    }
+}
+
 /// Reports on the progress made by the builder towards building a transaction.
 pub struct Progress {
     /// The number of steps completed.
@@ -107,20 +131,14 @@ impl Progress {
     }
 }
 
-enum ChangeAddress {
-    SaplingChangeAddress(OutgoingViewingKey, PaymentAddress),
-}
-
 /// Generates a [`Transaction`] from its inputs and outputs.
 pub struct Builder<'a, P, R> {
     params: P,
     rng: R,
     target_height: BlockHeight,
     expiry_height: BlockHeight,
-    fee: Amount,
     transparent_builder: TransparentBuilder,
     sapling_builder: SaplingBuilder<P>,
-    change_address: Option<ChangeAddress>,
     #[cfg(feature = "zfuture")]
     tze_builder: TzeBuilder<'a, TransactionData<Unauthorized>>,
     #[cfg(not(feature = "zfuture"))]
@@ -128,6 +146,42 @@ pub struct Builder<'a, P, R> {
     progress_notifier: Option<Sender<Progress>>,
 }
 
+impl<'a, P, R> Builder<'a, P, R> {
+    /// Returns the network parameters that the builder has been configured for.
+    pub fn params(&self) -> &P {
+        &self.params
+    }
+
+    /// Returns the target height of the transaction under construction.
+    pub fn target_height(&self) -> BlockHeight {
+        self.target_height
+    }
+
+    /// Returns the set of transparent inputs currently committed to be consumed
+    /// by the transaction.
+    pub fn transparent_inputs(&self) -> &[impl transparent::fees::InputView] {
+        self.transparent_builder.inputs()
+    }
+
+    /// Returns the set of transparent outputs currently set to be produced by
+    /// the transaction.
+    pub fn transparent_outputs(&self) -> &[impl transparent::fees::OutputView] {
+        self.transparent_builder.outputs()
+    }
+
+    /// Returns the set of Sapling inputs currently committed to be consumed
+    /// by the transaction.
+    pub fn sapling_inputs(&self) -> &[impl sapling::fees::InputView] {
+        self.sapling_builder.inputs()
+    }
+
+    /// Returns the set of Sapling outputs currently set to be produced by
+    /// the transaction.
+    pub fn sapling_outputs(&self) -> &[impl sapling::fees::OutputView] {
+        self.sapling_builder.outputs()
+    }
+}
+
 impl<'a, P: consensus::Parameters> Builder<'a, P, OsRng> {
     /// Creates a new `Builder` targeted for inclusion in the block with the given height,
     /// using default values for general transaction fields and the default OS random.
@@ -136,23 +190,9 @@ impl<'a, P: consensus::Parameters> Builder<'a, P, OsRng> {
     ///
     /// The expiry height will be set to the given height plus the default transaction
     /// expiry delta (20 blocks).
-    ///
-    /// The fee will be set to the default fee (0.0001 ZEC).
     pub fn new(params: P, target_height: BlockHeight) -> Self {
         Builder::new_with_rng(params, target_height, OsRng)
     }
-
-    /// Creates a new `Builder` targeted for inclusion in the block with the given height, using
-    /// the specified fee, and otherwise default values for general transaction fields and the
-    /// default OS random.
-    ///
-    /// # Default values
-    ///
-    /// The expiry height will be set to the given height plus the default transaction
-    /// expiry delta (20 blocks).
-    pub fn new_with_fee(params: P, target_height: BlockHeight, fee: Amount) -> Self {
-        Builder::new_with_rng_and_fee(params, OsRng, target_height, fee)
-    }
 }
 
 impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
@@ -163,27 +203,8 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> Builder<'a, P, R> {
     ///
     /// The expiry height will be set to the given height plus the default transaction
     /// expiry delta (20 blocks).
-    ///
-    /// The fee will be set to the default fee (0.0001 ZEC).
     pub fn new_with_rng(params: P, target_height: BlockHeight, rng: R) -> Builder<'a, P, R> {
-        Self::new_internal(params, rng, target_height, DEFAULT_FEE)
+        Self::new_internal(params, rng, target_height)
-    }
-
-    /// Creates a new `Builder` targeted for inclusion in the block with the given height, and
-    /// randomness source, using the specified fee, and otherwise default values for general
-    /// transaction fields and the default OS random.
-    ///
-    /// # Default values
-    ///
-    /// The expiry height will be set to the given height plus the default transaction
-    /// expiry delta (20 blocks).
-    pub fn new_with_rng_and_fee(
-        params: P,
-        rng: R,
-        target_height: BlockHeight,
-        fee: Amount,
-    ) -> Builder<'a, P, R> {
-        Self::new_internal(params, rng, target_height, fee)
     }
 }
 
@@ -192,21 +213,14 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> {
     ///
     /// WARNING: THIS MUST REMAIN PRIVATE AS IT ALLOWS CONSTRUCTION
     /// OF BUILDERS WITH NON-CryptoRng RNGs
-    fn new_internal(
+    fn new_internal(params: P, rng: R, target_height: BlockHeight) -> Builder<'a, P, R> {
-        params: P,
-        rng: R,
-        target_height: BlockHeight,
-        fee: Amount,
-    ) -> Builder<'a, P, R> {
         Builder {
             params: params.clone(),
             rng,
             target_height,
             expiry_height: target_height + DEFAULT_TX_EXPIRY_DELTA,
-            fee,
             transparent_builder: TransparentBuilder::empty(),
             sapling_builder: SaplingBuilder::new(params, target_height),
-            change_address: None,
             #[cfg(feature = "zfuture")]
             tze_builder: TzeBuilder::empty(),
             #[cfg(not(feature = "zfuture"))]
@@ -269,14 +283,6 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> {
             .map_err(Error::TransparentBuild)
     }
 
-    /// Sets the Sapling address to which any change will be sent.
-    ///
-    /// By default, change is sent to the Sapling address corresponding to the first note
-    /// being spent (i.e. the first call to [`Builder::add_sapling_spend`]).
-    pub fn send_change_to(&mut self, ovk: OutgoingViewingKey, to: PaymentAddress) {
-        self.change_address = Some(ChangeAddress::SaplingChangeAddress(ovk, to))
-    }
-
     /// Sets the notifier channel, where progress of building the transaction is sent.
     ///
     /// An update is sent after every Spend or Output is computed, and the `u32` sent
@@ -310,9 +316,55 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> {
     ///
     /// Upon success, returns a tuple containing the final transaction, and the
     /// [`SaplingMetadata`] generated during the build process.
-    pub fn build(
+    pub fn build<FR: FeeRule>(
-        mut self,
+        self,
         prover: &impl TxProver,
+        fee_rule: &FR,
+    ) -> Result<(Transaction, SaplingMetadata), Error>
+    where
+        Error: From<FR::Error>,
+    {
+        let fee = fee_rule.fee_required(
+            &self.params,
+            self.target_height,
+            self.transparent_builder.inputs(),
+            self.transparent_builder.outputs(),
+            self.sapling_builder.inputs(),
+            self.sapling_builder.outputs(),
+        )?;
+        self.build_internal(prover, fee)
+    }
+
+    /// Builds a transaction from the configured spends and outputs.
+    ///
+    /// Upon success, returns a tuple containing the final transaction, and the
+    /// [`SaplingMetadata`] generated during the build process.
+    #[cfg(feature = "zfuture")]
+    pub fn build_zfuture<FR: FutureFeeRule>(
+        self,
+        prover: &impl TxProver,
+        fee_rule: &FR,
+    ) -> Result<(Transaction, SaplingMetadata), Error>
+    where
+        Error: From<FR::Error>,
+    {
+        let fee = fee_rule.fee_required_zfuture(
+            &self.params,
+            self.target_height,
+            self.transparent_builder.inputs(),
+            self.transparent_builder.outputs(),
+            self.sapling_builder.inputs(),
+            self.sapling_builder.outputs(),
+            self.tze_builder.inputs(),
+            self.tze_builder.outputs(),
+        )?;
+        self.build_internal(prover, fee)
+    }
+
+    fn build_internal(
+        self,
+        prover: &impl TxProver,
+        fee: Amount,
     ) -> Result<(Transaction, SaplingMetadata), Error> {
         let consensus_branch_id = BranchId::for_height(&self.params, self.target_height);
 
@@ -323,33 +375,18 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> {
         // Consistency checks
         //
 
-        // Valid change
+        // After fees are accounted for, the value balance of the transaction must be zero.
-        let change = (self.value_balance()? - self.fee).ok_or(Error::InvalidAmount)?;
+        let balance_after_fees = (self.value_balance()? - fee).ok_or(Error::InvalidAmount)?;
 
-        if change.is_negative() {
+        match balance_after_fees.cmp(&Amount::zero()) {
-            return Err(Error::ChangeIsNegative(change));
+            Ordering::Less => {
-        }
+                return Err(Error::InsufficientFunds(-balance_after_fees));
-
-        //
-        // Change output
-        //
-
-        if change.is_positive() {
-            // Send change to the specified change address. If no change address
-            // was set, send change to the first Sapling address given as input.
-            match self.change_address.take() {
-                Some(ChangeAddress::SaplingChangeAddress(ovk, addr)) => {
-                    self.add_sapling_output(Some(ovk), addr, change, MemoBytes::empty())?;
-                }
-                None => {
-                    let (ovk, addr) = self
-                        .sapling_builder
-                        .get_candidate_change_address()
-                        .ok_or(Error::NoChangeAddress)?;
-                    self.add_sapling_output(Some(ovk), addr, change, MemoBytes::empty())?;
-                }
             }
+            Ordering::Greater => {
+                return Err(Error::ChangeRequired(balance_after_fees));
             }
+            Ordering::Equal => (),
+        };
 
         let transparent_bundle = self.transparent_builder.build();
 
@@ -475,6 +512,16 @@ impl<'a, P: consensus::Parameters, R: RngCore + CryptoRng> ExtensionTxBuilder<'a
 }
 
 #[cfg(any(test, feature = "test-dependencies"))]
+mod testing {
+    use rand::RngCore;
+
+    use super::{Builder, Error, SaplingMetadata};
+    use crate::{
+        consensus::{self, BlockHeight},
+        sapling::prover::mock::MockTxProver,
+        transaction::{components::amount::DEFAULT_FEE, fees::FixedFeeRule, Transaction},
+    };
+
     impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> {
         /// Creates a new `Builder` targeted for inclusion in the block with the given height
         /// and randomness source, using default values for general transaction fields.
@@ -484,15 +531,14 @@ impl<'a, P: consensus::Parameters, R: RngCore> Builder<'a, P, R> {
         /// The expiry height will be set to the given height plus the default transaction
         /// expiry delta (20 blocks).
         ///
-    /// The fee will be set to the default fee (0.0001 ZEC).
-    ///
         /// WARNING: DO NOT USE IN PRODUCTION
         pub fn test_only_new_with_rng(params: P, height: BlockHeight, rng: R) -> Builder<'a, P, R> {
-        Self::new_internal(params, rng, height, DEFAULT_FEE)
+            Self::new_internal(params, rng, height)
         }
 
         pub fn mock_build(self) -> Result<(Transaction, SaplingMetadata), Error> {
-        self.build(&MockTxProver)
+            self.build(&MockTxProver, &FixedFeeRule::new(DEFAULT_FEE))
+        }
     }
 }
 
@@ -506,7 +552,7 @@ mod tests {
         legacy::TransparentAddress,
         memo::MemoBytes,
         merkle_tree::{CommitmentTree, IncrementalWitness},
-        sapling::{prover::mock::MockTxProver, Node, Rseed},
+        sapling::{Node, Rseed},
         transaction::components::{
             amount::{Amount, DEFAULT_FEE},
             sapling::builder::{self as build_s},
@@ -515,14 +561,25 @@ mod tests {
         zip32::{ExtendedFullViewingKey, ExtendedSpendingKey},
     };
 
-    use super::{Builder, Error, SaplingBuilder, DEFAULT_TX_EXPIRY_DELTA};
+    use super::{Builder, Error};
 
     #[cfg(feature = "zfuture")]
+    #[cfg(feature = "transparent-inputs")]
     use super::TzeBuilder;
 
     #[cfg(not(feature = "zfuture"))]
     use std::marker::PhantomData;
 
+    #[cfg(feature = "transparent-inputs")]
+    use crate::{
+        legacy::keys::{AccountPrivKey, IncomingViewingKey},
+        transaction::{
+            builder::{SaplingBuilder, DEFAULT_TX_EXPIRY_DELTA},
+            OutPoint, TxOut,
+        },
+        zip32::AccountId,
+    };
+
     #[test]
     fn fails_on_negative_output() {
         let extsk = ExtendedSpendingKey::master(&[]);
@@ -546,7 +603,10 @@ mod tests {
         );
     }
 
+    // This test only works with the transparent_inputs feature because we have to
+    // be able to create a tx with a valid balance, without using Sapling inputs.
     #[test]
+    #[cfg(feature = "transparent-inputs")]
     fn binding_sig_absent_if_no_shielded_spend_or_output() {
         use crate::consensus::NetworkUpgrade;
         use crate::transaction::builder::{self, TransparentBuilder};
@@ -561,10 +621,8 @@ mod tests {
             rng: OsRng,
             target_height: sapling_activation_height,
             expiry_height: sapling_activation_height + DEFAULT_TX_EXPIRY_DELTA,
-            fee: Amount::zero(),
             transparent_builder: TransparentBuilder::empty(),
             sapling_builder: SaplingBuilder::new(TEST_NETWORK, sapling_activation_height),
-            change_address: None,
             #[cfg(feature = "zfuture")]
             tze_builder: TzeBuilder::empty(),
             #[cfg(not(feature = "zfuture"))]
@@ -572,12 +630,34 @@ mod tests {
             progress_notifier: None,
         };
 
-        // Create a tx with only t output. No binding_sig should be present
+        let tsk = AccountPrivKey::from_seed(&TEST_NETWORK, &[0u8; 32], AccountId::from(0)).unwrap();
+        let prev_coin = TxOut {
+            value: Amount::from_u64(50000).unwrap(),
+            script_pubkey: tsk
+                .to_account_pubkey()
+                .derive_external_ivk()
+                .unwrap()
+                .derive_address(0)
+                .unwrap()
+                .script(),
+        };
         builder
-            .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount::zero())
+            .add_transparent_input(
+                tsk.derive_external_secret_key(0).unwrap(),
+                OutPoint::new([0u8; 32], 1),
+                prev_coin,
+            )
             .unwrap();
 
-        let (tx, _) = builder.build(&MockTxProver).unwrap();
+        // Create a tx with only t output. No binding_sig should be present
+        builder
+            .add_transparent_output(
+                &TransparentAddress::PublicKey([0; 20]),
+                Amount::from_u64(49000).unwrap(),
+            )
+            .unwrap();
+
+        let (tx, _) = builder.mock_build().unwrap();
         // No binding signature, because only t input and outputs
         assert!(tx.sapling_bundle.is_none());
     }
@@ -609,13 +689,16 @@ mod tests {
             .unwrap();
 
         builder
-            .add_transparent_output(&TransparentAddress::PublicKey([0; 20]), Amount::zero())
+            .add_transparent_output(
+                &TransparentAddress::PublicKey([0; 20]),
+                Amount::from_u64(49000).unwrap(),
+            )
             .unwrap();
 
         // Expect a binding signature error, because our inputs aren't valid, but this shows
         // that a binding signature was attempted
         assert_eq!(
-            builder.build(&MockTxProver),
+            builder.mock_build(),
             Err(Error::SaplingBuild(build_s::Error::BindingSig))
         );
     }
@@ -650,10 +733,8 @@ mod tests {
         {
             let builder = Builder::new(TEST_NETWORK, tx_height);
             assert_eq!(
-                builder.build(&MockTxProver),
+                builder.mock_build(),
-                Err(Error::ChangeIsNegative(
+                Err(Error::InsufficientFunds(DEFAULT_FEE))
-                    (Amount::zero() - DEFAULT_FEE).unwrap()
-                ))
             );
         }
 
@@ -674,9 +755,9 @@ mod tests {
                 )
                 .unwrap();
             assert_eq!(
-                builder.build(&MockTxProver),
+                builder.mock_build(),
-                Err(Error::ChangeIsNegative(
+                Err(Error::InsufficientFunds(
-                    (Amount::from_i64(-50000).unwrap() - DEFAULT_FEE).unwrap()
+                    (Amount::from_i64(50000).unwrap() + DEFAULT_FEE).unwrap()
                 ))
             );
         }
@@ -692,9 +773,9 @@ mod tests {
                 )
                 .unwrap();
             assert_eq!(
-                builder.build(&MockTxProver),
+                builder.mock_build(),
-                Err(Error::ChangeIsNegative(
+                Err(Error::InsufficientFunds(
-                    (Amount::from_i64(-50000).unwrap() - DEFAULT_FEE).unwrap()
+                    (Amount::from_i64(50000).unwrap() + DEFAULT_FEE).unwrap()
                 ))
             );
         }
@@ -734,8 +815,8 @@ mod tests {
                 )
                 .unwrap();
             assert_eq!(
-                builder.build(&MockTxProver),
+                builder.mock_build(),
-                Err(Error::ChangeIsNegative(Amount::from_i64(-1).unwrap()))
+                Err(Error::InsufficientFunds(Amount::from_i64(1).unwrap()))
             );
         }
 
@@ -780,7 +861,7 @@ mod tests {
                 )
                 .unwrap();
             assert_eq!(
-                builder.build(&MockTxProver),
+                builder.mock_build(),
                 Err(Error::SaplingBuild(build_s::Error::BindingSig))
             )
         }

zcash_primitives/src/transaction/components/sapling.rs

@@ -24,6 +24,7 @@ use super::{amount::Amount, GROTH_PROOF_SIZE};
 pub type GrothProofBytes = [u8; GROTH_PROOF_SIZE];
 
 pub mod builder;
+pub mod fees;
 
 pub trait Authorization: Debug {
     type Proof: Clone + Debug;

zcash_primitives/src/transaction/components/sapling/builder.rs

@@ -25,7 +25,7 @@ use crate::{
         components::{
             amount::Amount,
             sapling::{
-                Authorization, Authorized, Bundle, GrothProofBytes, OutputDescription,
+                fees, Authorization, Authorized, Bundle, GrothProofBytes, OutputDescription,
                 SpendDescription,
             },
         },
@@ -69,8 +69,18 @@ pub struct SpendDescriptionInfo {
     merkle_path: MerklePath<Node>,
 }
 
+impl fees::InputView for SpendDescriptionInfo {
+    fn value(&self) -> Amount {
+        // An existing note to be spent must have a valid
+        // amount value.
+        Amount::from_u64(self.note.value).unwrap()
+    }
+}
+
+/// A struct containing the information required in order to construct a
+/// Sapling output to a transaction.
 #[derive(Clone)]
-struct SaplingOutput {
+struct SaplingOutputInfo {
     /// `None` represents the `ovk = ⊥` case.
     ovk: Option<OutgoingViewingKey>,
     to: PaymentAddress,
@@ -78,7 +88,7 @@ struct SaplingOutput {
     memo: MemoBytes,
 }
 
-impl SaplingOutput {
+impl SaplingOutputInfo {
     fn new_internal<P: consensus::Parameters, R: RngCore>(
         params: &P,
         rng: &mut R,
@@ -102,7 +112,7 @@ impl SaplingOutput {
             rseed,
         };
 
-        Ok(SaplingOutput {
+        Ok(SaplingOutputInfo {
             ovk,
             to,
             note,
@@ -150,6 +160,12 @@ impl SaplingOutput {
     }
 }
 
+impl fees::OutputView for SaplingOutputInfo {
+    fn value(&self) -> Amount {
+        Amount::from_u64(self.note.value).expect("Note values should be checked at construction.")
+    }
+}
+
 /// Metadata about a transaction created by a [`SaplingBuilder`].
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct SaplingMetadata {
@@ -194,7 +210,7 @@ pub struct SaplingBuilder<P> {
     target_height: BlockHeight,
     value_balance: Amount,
     spends: Vec<SpendDescriptionInfo>,
-    outputs: Vec<SaplingOutput>,
+    outputs: Vec<SaplingOutputInfo>,
 }
 
 #[derive(Clone)]
@@ -213,7 +229,7 @@ impl Authorization for Unauthorized {
     type AuthSig = SpendDescriptionInfo;
 }
 
-impl<P: consensus::Parameters> SaplingBuilder<P> {
+impl<P> SaplingBuilder<P> {
     pub fn new(params: P, target_height: BlockHeight) -> Self {
         SaplingBuilder {
             params,
@@ -225,11 +241,24 @@ impl<P: consensus::Parameters> SaplingBuilder<P> {
         }
     }
 
+    /// Returns the list of Sapling inputs that will be consumed by the transaction being
+    /// constructed.
+    pub fn inputs(&self) -> &[impl fees::InputView] {
+        &self.spends
+    }
+
+    /// Returns the Sapling outputs that will be produced by the transaction being constructed
+    pub fn outputs(&self) -> &[impl fees::OutputView] {
+        &self.outputs
+    }
+
     /// Returns the net value represented by the spends and outputs added to this builder.
     pub fn value_balance(&self) -> Amount {
         self.value_balance
     }
+}
 
+impl<P: consensus::Parameters> SaplingBuilder<P> {
     /// Adds a Sapling note to be spent in this transaction.
     ///
     /// Returns an error if the given Merkle path does not have the same anchor as the
@@ -278,7 +307,7 @@ impl<P: consensus::Parameters> SaplingBuilder<P> {
         value: Amount,
         memo: MemoBytes,
     ) -> Result<(), Error> {
-        let output = SaplingOutput::new_internal(
+        let output = SaplingOutputInfo::new_internal(
             &self.params,
             &mut rng,
             self.target_height,
@@ -295,15 +324,6 @@ impl<P: consensus::Parameters> SaplingBuilder<P> {
         Ok(())
     }
 
-    /// Send change to the specified change address. If no change address
-    /// was set, send change to the first Sapling address given as input.
-    pub fn get_candidate_change_address(&self) -> Option<(OutgoingViewingKey, PaymentAddress)> {
-        self.spends.first().and_then(|spend| {
-            PaymentAddress::from_parts(spend.diversifier, spend.note.pk_d)
-                .map(|addr| (spend.extsk.expsk.ovk, addr))
-        })
-    }
-
     pub fn build<Pr: TxProver, R: RngCore>(
         self,
         prover: &Pr,

zcash_primitives/src/transaction/components/sapling/fees.rs

@@ -0,0 +1,18 @@
+//! Types related to computation of fees and change related to the Sapling components
+//! of a transaction.
+
+use crate::transaction::components::amount::Amount;
+
+/// A trait that provides a minimized view of a Sapling input suitable for use in
+/// fee and change calculation.
+pub trait InputView {
+    /// The value of the input being spent.
+    fn value(&self) -> Amount;
+}
+
+/// A trait that provides a minimized view of a Sapling output suitable for use in
+/// fee and change calculation.
+pub trait OutputView {
+    /// The value of the output being produced.
+    fn value(&self) -> Amount;
+}

zcash_primitives/src/transaction/components/transparent.rs

@@ -10,6 +10,7 @@ use crate::legacy::{Script, TransparentAddress};
 use super::amount::{Amount, BalanceError};
 
 pub mod builder;
+pub mod fees;
 
 pub trait Authorization: Debug {
     type ScriptSig: Debug + Clone + PartialEq;

zcash_primitives/src/transaction/components/transparent/builder.rs

@@ -7,9 +7,10 @@ use crate::{
     transaction::{
         components::{
             amount::Amount,
-            transparent::{self, Authorization, Authorized, Bundle, TxIn, TxOut},
+            transparent::{self, fees, Authorization, Authorized, Bundle, TxIn, TxOut},
         },
         sighash::TransparentAuthorizingContext,
+        OutPoint,
     },
 };
 
@@ -17,7 +18,6 @@ use crate::{
 use {
     crate::transaction::{
         self as tx,
-        components::OutPoint,
         sighash::{signature_hash, SignableInput, SIGHASH_ALL},
         TransactionData, TxDigests,
     },
@@ -40,6 +40,21 @@ impl fmt::Display for Error {
     }
 }
 
+/// An uninhabited type that allows the type of [`TransparentBuilder::inputs`]
+/// to resolve when the transparent-inputs feature is not turned on.
+#[cfg(not(feature = "transparent-inputs"))]
+enum InvalidTransparentInput {}
+
+#[cfg(not(feature = "transparent-inputs"))]
+impl fees::InputView for InvalidTransparentInput {
+    fn outpoint(&self) -> &OutPoint {
+        panic!("transparent-inputs feature flag is not enabled.");
+    }
+    fn coin(&self) -> &TxOut {
+        panic!("transparent-inputs feature flag is not enabled.");
+    }
+}
+
 #[cfg(feature = "transparent-inputs")]
 #[derive(Debug, Clone)]
 struct TransparentInputInfo {
@@ -49,6 +64,17 @@ struct TransparentInputInfo {
     coin: TxOut,
 }
 
+#[cfg(feature = "transparent-inputs")]
+impl fees::InputView for TransparentInputInfo {
+    fn outpoint(&self) -> &OutPoint {
+        &self.utxo
+    }
+
+    fn coin(&self) -> &TxOut {
+        &self.coin
+    }
+}
+
 pub struct TransparentBuilder {
     #[cfg(feature = "transparent-inputs")]
     secp: secp256k1::Secp256k1<secp256k1::SignOnly>,
@@ -70,6 +96,7 @@ impl Authorization for Unauthorized {
 }
 
 impl TransparentBuilder {
+    /// Constructs a new TransparentBuilder
     pub fn empty() -> Self {
         TransparentBuilder {
             #[cfg(feature = "transparent-inputs")]
@@ -80,6 +107,25 @@ impl TransparentBuilder {
         }
     }
 
+    /// Returns the list of transparent inputs that will be consumed by the transaction being
+    /// constructed.
+    pub fn inputs(&self) -> &[impl fees::InputView] {
+        #[cfg(feature = "transparent-inputs")]
+        return &self.inputs;
+
+        #[cfg(not(feature = "transparent-inputs"))]
+        {
+            let invalid: &[InvalidTransparentInput] = &[];
+            return invalid;
+        }
+    }
+
+    /// Returns the transparent outputs that will be produced by the transaction being constructed.
+    pub fn outputs(&self) -> &[impl fees::OutputView] {
+        &self.vout
+    }
+
+    /// Adds a coin (the output of a previous transaction) to be spent to the transaction.
     #[cfg(feature = "transparent-inputs")]
     pub fn add_input(
         &mut self,

zcash_primitives/src/transaction/components/transparent/fees.rs

@@ -0,0 +1,36 @@
+//! Types related to computation of fees and change related to the transparent components
+//! of a transaction.
+
+use super::TxOut;
+use crate::{
+    legacy::Script,
+    transaction::{components::amount::Amount, OutPoint},
+};
+
+/// This trait provides a minimized view of a transparent input suitable for use in
+/// fee and change computation.
+pub trait InputView {
+    /// The outpoint to which the input refers.
+    fn outpoint(&self) -> &OutPoint;
+    /// The previous output being spent.
+    fn coin(&self) -> &TxOut;
+}
+
+/// This trait provides a minimized view of a transparent output suitable for use in
+/// fee and change computation.
+pub trait OutputView {
+    /// Returns the value of the output being created.
+    fn value(&self) -> Amount;
+    /// Returns the script corresponding to the newly created output.
+    fn script_pubkey(&self) -> &Script;
+}
+
+impl OutputView for TxOut {
+    fn value(&self) -> Amount {
+        self.value
+    }
+
+    fn script_pubkey(&self) -> &Script {
+        &self.script_pubkey
+    }
+}

zcash_primitives/src/transaction/components/tze.rs

@@ -12,6 +12,7 @@ use super::amount::Amount;
 use crate::{extensions::transparent as tze, transaction::TxId};
 
 pub mod builder;
+pub mod fees;
 
 fn to_io_error(_: std::num::TryFromIntError) -> io::Error {
     io::Error::new(io::ErrorKind::InvalidData, "value out of range")

zcash_primitives/src/transaction/components/tze/builder.rs

@@ -9,7 +9,7 @@ use crate::{
         self as tx,
         components::{
             amount::Amount,
-            tze::{Authorization, Authorized, Bundle, OutPoint, TzeIn, TzeOut},
+            tze::{fees, Authorization, Authorized, Bundle, OutPoint, TzeIn, TzeOut},
         },
     },
 };
@@ -32,13 +32,27 @@ impl fmt::Display for Error {
 
 #[allow(clippy::type_complexity)]
 pub struct TzeSigner<'a, BuildCtx> {
-    prevout: TzeOut,
     builder: Box<dyn FnOnce(&BuildCtx) -> Result<(u32, Vec<u8>), Error> + 'a>,
 }
 
+#[derive(Clone)]
+struct TzeBuildInput {
+    tzein: TzeIn<()>,
+    coin: TzeOut,
+}
+
+impl fees::InputView for TzeBuildInput {
+    fn outpoint(&self) -> &OutPoint {
+        &self.tzein.prevout
+    }
+    fn coin(&self) -> &TzeOut {
+        &self.coin
+    }
+}
+
 pub struct TzeBuilder<'a, BuildCtx> {
     signers: Vec<TzeSigner<'a, BuildCtx>>,
-    vin: Vec<TzeIn<()>>,
+    vin: Vec<TzeBuildInput>,
     vout: Vec<TzeOut>,
 }
 
@@ -58,18 +72,28 @@ impl<'a, BuildCtx> TzeBuilder<'a, BuildCtx> {
         }
     }
 
+    pub fn inputs(&self) -> &[impl fees::InputView] {
+        &self.vin
+    }
+
+    pub fn outputs(&self) -> &[impl fees::OutputView] {
+        &self.vout
+    }
+
     pub fn add_input<WBuilder, W: ToPayload>(
         &mut self,
         extension_id: u32,
         mode: u32,
-        (outpoint, prevout): (OutPoint, TzeOut),
+        (outpoint, coin): (OutPoint, TzeOut),
         witness_builder: WBuilder,
     ) where
         WBuilder: 'a + FnOnce(&BuildCtx) -> Result<W, Error>,
     {
-        self.vin.push(TzeIn::new(outpoint, extension_id, mode));
+        self.vin.push(TzeBuildInput {
+            tzein: TzeIn::new(outpoint, extension_id, mode),
+            coin,
+        });
         self.signers.push(TzeSigner {
-            prevout,
             builder: Box::new(move |ctx| witness_builder(ctx).map(|x| x.to_payload())),
         });
     }
@@ -98,9 +122,9 @@ impl<'a, BuildCtx> TzeBuilder<'a, BuildCtx> {
     }
 
     pub fn value_balance(&self) -> Option<Amount> {
-        self.signers
+        self.vin
             .iter()
-            .map(|s| s.prevout.value)
+            .map(|tzi| tzi.coin.value)
             .sum::<Option<Amount>>()?
             - self
                 .vout
@@ -115,7 +139,7 @@ impl<'a, BuildCtx> TzeBuilder<'a, BuildCtx> {
         } else {
             (
                 Some(Bundle {
-                    vin: self.vin.clone(),
+                    vin: self.vin.iter().map(|vin| vin.tzein.clone()).collect(),
                     vout: self.vout.clone(),
                     authorization: Unauthorized,
                 }),

zcash_primitives/src/transaction/components/tze/fees.rs

@@ -0,0 +1,37 @@
+//! Abstractions and types related to fee calculations for TZE components of a transaction.
+
+use crate::{
+    extensions::transparent::{self as tze},
+    transaction::components::{
+        amount::Amount,
+        tze::{OutPoint, TzeOut},
+    },
+};
+
+/// This trait provides a minimized view of a TZE input suitable for use in
+/// fee computation.
+pub trait InputView {
+    /// The outpoint to which the input refers.
+    fn outpoint(&self) -> &OutPoint;
+    /// The previous output being consumed.
+    fn coin(&self) -> &TzeOut;
+}
+
+/// This trait provides a minimized view of a TZE output suitable for use in
+/// fee computation.
+pub trait OutputView {
+    /// The value of the newly created output
+    fn value(&self) -> Amount;
+    /// The precondition that must be satisfied in order to spend this output.
+    fn precondition(&self) -> &tze::Precondition;
+}
+
+impl OutputView for TzeOut {
+    fn value(&self) -> Amount {
+        self.value
+    }
+
+    fn precondition(&self) -> &tze::Precondition {
+        &self.precondition
+    }
+}

zcash_primitives/src/transaction/fees.rs

@@ -0,0 +1,101 @@
+//! Abstractions and types related to fee calculations.
+
+use crate::{
+    consensus::{self, BlockHeight},
+    transaction::components::{
+        amount::Amount, sapling::fees as sapling, transparent::fees as transparent,
+    },
+};
+
+#[cfg(feature = "zfuture")]
+use crate::transaction::components::tze::fees as tze;
+
+/// A trait that represents the ability to compute the fees that must be paid
+/// by a transaction having a specified set of inputs and outputs.
+pub trait FeeRule {
+    type Error;
+
+    /// Computes the total fee required for a transaction given the provided inputs and outputs.
+    ///
+    /// Implementations of this method should compute the fee amount given exactly the inputs and
+    /// outputs specified, and should NOT compute speculative fees given any additional change
+    /// outputs that may need to be created in order for inputs and outputs to balance.
+    fn fee_required<P: consensus::Parameters>(
+        &self,
+        params: &P,
+        target_height: BlockHeight,
+        transparent_inputs: &[impl transparent::InputView],
+        transparent_outputs: &[impl transparent::OutputView],
+        sapling_inputs: &[impl sapling::InputView],
+        sapling_outputs: &[impl sapling::OutputView],
+    ) -> Result<Amount, Self::Error>;
+}
+
+/// A trait that represents the ability to compute the fees that must be paid by a transaction
+/// having a specified set of inputs and outputs, for use when experimenting with the TZE feature.
+#[cfg(feature = "zfuture")]
+pub trait FutureFeeRule: FeeRule {
+    /// Computes the total fee required for a transaction given the provided inputs and outputs.
+    ///
+    /// Implementations of this method should compute the fee amount given exactly the inputs and
+    /// outputs specified, and should NOT compute speculative fees given any additional change
+    /// outputs that may need to be created in order for inputs and outputs to balance.
+    #[allow(clippy::too_many_arguments)]
+    fn fee_required_zfuture<P: consensus::Parameters>(
+        &self,
+        params: &P,
+        target_height: BlockHeight,
+        transparent_inputs: &[impl transparent::InputView],
+        transparent_outputs: &[impl transparent::OutputView],
+        sapling_inputs: &[impl sapling::InputView],
+        sapling_outputs: &[impl sapling::OutputView],
+        tze_inputs: &[impl tze::InputView],
+        tze_outputs: &[impl tze::OutputView],
+    ) -> Result<Amount, Self::Error>;
+}
+
+/// A fee rule that always returns a fixed fee, irrespective of the structure of
+/// the transaction being constructed.
+pub struct FixedFeeRule {
+    fixed_fee: Amount,
+}
+
+impl FixedFeeRule {
+    /// Creates a new fixed fee rule with the specified fixed fee.
+    pub fn new(fixed_fee: Amount) -> Self {
+        Self { fixed_fee }
+    }
+}
+
+impl FeeRule for FixedFeeRule {
+    type Error = std::convert::Infallible;
+
+    fn fee_required<P: consensus::Parameters>(
+        &self,
+        _params: &P,
+        _target_height: BlockHeight,
+        _transparent_inputs: &[impl transparent::InputView],
+        _transparent_outputs: &[impl transparent::OutputView],
+        _sapling_inputs: &[impl sapling::InputView],
+        _sapling_outputs: &[impl sapling::OutputView],
+    ) -> Result<Amount, Self::Error> {
+        Ok(self.fixed_fee)
+    }
+}
+
+#[cfg(feature = "zfuture")]
+impl FutureFeeRule for FixedFeeRule {
+    fn fee_required_zfuture<P: consensus::Parameters>(
+        &self,
+        _params: &P,
+        _target_height: BlockHeight,
+        _transparent_inputs: &[impl transparent::InputView],
+        _transparent_outputs: &[impl transparent::OutputView],
+        _sapling_inputs: &[impl sapling::InputView],
+        _sapling_outputs: &[impl sapling::OutputView],
+        _tze_inputs: &[impl tze::InputView],
+        _tze_outputs: &[impl tze::OutputView],
+    ) -> Result<Amount, Self::Error> {
+        Ok(self.fixed_fee)
+    }
+}

zcash_primitives/src/transaction/mod.rs

@@ -1,6 +1,7 @@
 //! Structs and methods for handling Zcash transactions.
 pub mod builder;
 pub mod components;
+pub mod fees;
 pub mod sighash;
 pub mod sighash_v4;
 pub mod sighash_v5;