Address comments from code review.

zcash_client_backend/src/fees.rs

@@ -52,7 +52,7 @@ impl TransactionBalance {
     }
 }
 
-/// Errors that can occur in balance
+/// Errors that can occur in computing suggested change and/or fees.
 pub enum ChangeError<E> {
     InsufficientFunds { available: Amount, required: Amount },
     StrategyError(E),

zcash_primitives/CHANGELOG.md

@@ -21,14 +21,9 @@ and this library adheres to Rust's notion of
     - `Builder::sapling_inputs()`
     - `Builder::sapling_outputs()`
 - `zcash_primitives::transaction::fees` a new module containing abstractions
-  and types related to fee calculations.
-  - `TransactionBalance` A type representing the unspent balance of transaction inputs,
-    allocated separately to change and fee portions.
-  - `FeeRule` a function that computes the fee required for a transaction given
-    inputs and outputs to the transaction.
-  - `Infalliable` an uninhabited error type for conditions where no error can occur.
-  - `FixedFeeRule` A `FeeRule` implementation that always computes the same fixed
-    fee irrespective of the inputs and outputs to the transaction being constructed.
+  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`
   - `SaplingInput` a trait that provides a minimized view of a Sapling input suitable
     for use in fee computation.
@@ -39,26 +34,27 @@ and this library adheres to Rust's notion of
 - Added to `zcash_primitives::transaction::components::transparent::builder`
   - `TransparentInput` a trait that provides a minimized view of a transparent input suitable
     for use in fee computation.
-  - `TransparentBuilder::inputs` and `TransparentBuilder::outputs`: accessors for Sapling 
-    builder state.
+  - `TransparentBuilder::{inputs, outputs}`: accessors for transparent builder state.
 
 ### 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` and `Builder::new_with_rng_and_fee` have been removed;
-    The transaction builder no longer fixes the fee for transactions to 0.00001 ZEC,
-    but instead computes the fee using a `FeeRule` implementation at build time.
+  - `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`
-- Removed from `zcash_primitives::transaction::components::sapling::builder::SaplingBuilder`
-  - `get_candidate_change_address` change outputs must now be added by the caller.
+- `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,6 +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;
@@ -29,7 +30,7 @@ use crate::{
                 builder::{TransparentBuilder, TransparentInput},
             },
         },
-        fees::{FeeRule, Infallible},
+        fees::FeeRule,
         sighash::{signature_hash, SignableInput},
         txid::TxIdDigester,
         Transaction, TransactionData, TxOut, TxVersion, Unauthorized,

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

@@ -61,7 +61,7 @@ impl fmt::Display for Error {
 }
 
 /// A trait that provides a minimized view of a Sapling input suitable for use in
-/// fee calculation.
+/// fee and change calculation.
 pub trait SaplingInput {
     /// The value of the input being spent.
     fn value(&self) -> Amount;

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

@@ -57,10 +57,10 @@ enum InvalidTransparentInput {}
 #[cfg(not(feature = "transparent-inputs"))]
 impl TransparentInput for InvalidTransparentInput {
     fn outpoint(&self) -> &OutPoint {
-        panic!("InvalidTransparentInput is uninhabited.");
+        panic!("transparent-inputs feature flag is not enabled.");
     }
     fn coin(&self) -> &TxOut {
-        panic!("InvalidTransparentInput is uninhabited.");
+        panic!("transparent-inputs feature flag is not enabled.");
     }
 }
 
@@ -129,7 +129,7 @@ impl TransparentBuilder {
         }
     }
 
-    /// Returns the transparent outputs that will be produced by the transaction being constructed
+    /// Returns the transparent outputs that will be produced by the transaction being constructed.
     pub fn outputs(&self) -> &[TxOut] {
         &self.vout
     }

zcash_primitives/src/transaction/fees.rs

@@ -1,3 +1,5 @@
+//! Abstractions and types related to fee calculations.
+
 use crate::{
     consensus::{self, BlockHeight},
     transaction::components::{
@@ -15,13 +17,11 @@ use crate::transaction::components::tze::{TzeInput, TzeOut};
 pub trait FeeRule {
     type Error;
 
-    /// Computes the totals of inputs, required change amount, and fees given the
-    /// provided inputs and outputs being used to construct a transaction.
+    /// 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.
+    /// 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,
@@ -33,23 +33,15 @@ pub trait FeeRule {
     ) -> 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.
-///
-/// 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.
+/// 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 totals of inputs, required change amount, and fees given the
-    /// provided inputs and outputs being used to construct a transaction.
+    /// 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.
+    /// 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,
@@ -64,10 +56,6 @@ pub trait FutureFeeRule: FeeRule {
     ) -> Result<Amount, Self::Error>;
 }
 
-/// An uninhabited error type used to indicate when an operation
-/// that returns a `Result` cannot fail.
-pub enum Infallible {}
-
 /// A fee rule that always returns a fixed fee, irrespective of the structure of
 /// the transaction being constructed.
 pub struct FixedFeeRule {
@@ -82,7 +70,7 @@ impl FixedFeeRule {
 }
 
 impl FeeRule for FixedFeeRule {
-    type Error = Infallible;
+    type Error = std::convert::Infallible;
 
     fn fee_required<P: consensus::Parameters>(
         &self,