Refactor FeeRule::fee_required to take the sizes of transparent

inputs and outputs.

Co-authored-by: Kris Nuttycombe <kris@nutty.land>
Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>

zcash_client_backend/src/fees/common.rs

@@ -183,8 +183,8 @@ where
         .fee_required(
             params,
             target_height,
-            transparent_inputs,
-            transparent_outputs,
+            transparent_inputs.iter().map(|i| i.serialized_size()),
+            transparent_outputs.iter().map(|i| i.serialized_size()),
             sapling_input_count,
             sapling_output_count,
             orchard_action_count,

zcash_primitives/CHANGELOG.md

@@ -30,6 +30,11 @@ and this library adheres to Rust's notion of
     - `AccountPubKey::derive_internal_ivk`
     - `AccountPubKey::deserialize`
     - `IncomingViewingKey::derive_address`
+- `zcash_primitives::transaction::fees::FeeRule::fee_required`: the types
+  of parameters relating to transparent inputs and outputs have changed.
+  This method now requires their `tx_in` and `tx_out` serialized sizes
+  (expressed as iterators of `InputSize` for inputs and `usize` for outputs)
+  rather than a slice of `InputView` or `OutputView`.
 
 ### Removed
 - The `zcash_primitives::zip339` module, which reexported parts of the API of

zcash_primitives/src/transaction/builder.rs

@@ -22,7 +22,10 @@ use crate::{
             amount::{Amount, BalanceError},
             transparent::{self, builder::TransparentBuilder, TxOut},
         },
-        fees::FeeRule,
+        fees::{
+            transparent::{InputView, OutputView},
+            FeeRule,
+        },
         sighash::{signature_hash, SignableInput},
         txid::TxIdDigester,
         Transaction, TransactionData, TxVersion, Unauthorized,
@@ -554,8 +557,11 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<
             .fee_required(
                 &self.params,
                 self.target_height,
-                transparent_inputs,
-                self.transparent_builder.outputs(),
+                transparent_inputs.iter().map(|i| i.serialized_size()),
+                self.transparent_builder
+                    .outputs()
+                    .iter()
+                    .map(|i| i.serialized_size()),
                 sapling_spends,
                 self.sapling_builder
                     .as_ref()
@@ -597,8 +603,11 @@ impl<'a, P: consensus::Parameters, U: sapling::builder::ProverProgress> Builder<
             .fee_required_zfuture(
                 &self.params,
                 self.target_height,
-                transparent_inputs,
-                self.transparent_builder.outputs(),
+                transparent_inputs.iter().map(|i| i.serialized_size()),
+                self.transparent_builder
+                    .outputs()
+                    .iter()
+                    .map(|i| i.serialized_size()),
                 sapling_spends,
                 self.sapling_builder
                     .as_ref()

zcash_primitives/src/transaction/fees.rs

@@ -2,7 +2,7 @@
 
 use crate::{
     consensus::{self, BlockHeight},
-    transaction::components::amount::NonNegativeAmount,
+    transaction::{components::amount::NonNegativeAmount, fees::transparent::InputSize},
 };
 
 pub mod fixed;
@@ -27,8 +27,8 @@ pub trait FeeRule {
         &self,
         params: &P,
         target_height: BlockHeight,
-        transparent_inputs: &[impl transparent::InputView],
-        transparent_outputs: &[impl transparent::OutputView],
+        transparent_input_sizes: impl IntoIterator<Item = InputSize>,
+        transparent_output_sizes: impl IntoIterator<Item = usize>,
         sapling_input_count: usize,
         sapling_output_count: usize,
         orchard_action_count: usize,
@@ -49,8 +49,8 @@ pub trait FutureFeeRule: FeeRule {
         &self,
         params: &P,
         target_height: BlockHeight,
-        transparent_inputs: &[impl transparent::InputView],
-        transparent_outputs: &[impl transparent::OutputView],
+        transparent_input_sizes: impl IntoIterator<Item = InputSize>,
+        transparent_output_sizes: impl IntoIterator<Item = usize>,
         sapling_input_count: usize,
         sapling_output_count: usize,
         orchard_action_count: usize,
@@ -80,8 +80,8 @@ impl FeeRule for StandardFeeRule {
         &self,
         params: &P,
         target_height: BlockHeight,
-        transparent_inputs: &[impl transparent::InputView],
-        transparent_outputs: &[impl transparent::OutputView],
+        transparent_input_sizes: impl IntoIterator<Item = InputSize>,
+        transparent_output_sizes: impl IntoIterator<Item = usize>,
         sapling_input_count: usize,
         sapling_output_count: usize,
         orchard_action_count: usize,
@@ -93,8 +93,8 @@ impl FeeRule for StandardFeeRule {
             Self::Zip317 => zip317::FeeRule::standard().fee_required(
                 params,
                 target_height,
-                transparent_inputs,
-                transparent_outputs,
+                transparent_input_sizes,
+                transparent_output_sizes,
                 sapling_input_count,
                 sapling_output_count,
                 orchard_action_count,

zcash_primitives/src/transaction/fees/fixed.rs

@@ -52,8 +52,8 @@ impl super::FeeRule for FeeRule {
         &self,
         _params: &P,
         _target_height: BlockHeight,
-        _transparent_inputs: &[impl transparent::InputView],
-        _transparent_outputs: &[impl transparent::OutputView],
+        _transparent_input_sizes: impl IntoIterator<Item = transparent::InputSize>,
+        _transparent_output_sizes: impl IntoIterator<Item = usize>,
         _sapling_input_count: usize,
         _sapling_output_count: usize,
         _orchard_action_count: usize,
@@ -68,8 +68,8 @@ impl super::FutureFeeRule for FeeRule {
         &self,
         _params: &P,
         _target_height: BlockHeight,
-        _transparent_inputs: &[impl transparent::InputView],
-        _transparent_outputs: &[impl transparent::OutputView],
+        _transparent_input_sizes: impl IntoIterator<Item = transparent::InputSize>,
+        _transparent_output_sizes: impl IntoIterator<Item = usize>,
         _sapling_input_count: usize,
         _sapling_output_count: usize,
         _orchard_action_count: usize,

zcash_primitives/src/transaction/fees/transparent.rs

@@ -4,20 +4,46 @@
 use std::convert::Infallible;
 
 use crate::{
-    legacy::Script,
-    transaction::components::{amount::NonNegativeAmount, transparent::TxOut, OutPoint},
+    legacy::{Script, TransparentAddress},
+    transaction::{
+        components::{amount::NonNegativeAmount, transparent::TxOut, OutPoint},
+        fees::zip317::P2PKH_STANDARD_INPUT_SIZE,
+    },
 };
 
 #[cfg(feature = "transparent-inputs")]
 use crate::transaction::components::transparent::builder::TransparentInputInfo;
 
+/// The size of a transparent input, or the outpoint corresponding to the input
+/// if the size of the script required to spend that input is unknown.
+pub enum InputSize {
+    /// The txin size is known.
+    Known(usize),
+    /// The size of the script required to spend this input (and therefore the txin size)
+    /// is unknown.
+    Unknown(OutPoint),
+}
+
+impl InputSize {
+    pub const STANDARD_P2PKH: InputSize = InputSize::Known(P2PKH_STANDARD_INPUT_SIZE);
+}
+
 /// This trait provides a minimized view of a transparent input suitable for use in
 /// fee and change computation.
 pub trait InputView: std::fmt::Debug {
     /// The outpoint to which the input refers.
     fn outpoint(&self) -> &OutPoint;
+
     /// The previous output being spent.
     fn coin(&self) -> &TxOut;
+
+    /// The size of the transparent script required to spend this input.
+    fn serialized_size(&self) -> InputSize {
+        match self.coin().script_pubkey.address() {
+            Some(TransparentAddress::PublicKeyHash(_)) => InputSize::STANDARD_P2PKH,
+            _ => InputSize::Unknown(self.outpoint().clone()),
+        }
+    }
 }
 
 #[cfg(feature = "transparent-inputs")]
@@ -45,8 +71,19 @@ impl InputView for Infallible {
 pub trait OutputView: std::fmt::Debug {
     /// Returns the value of the output being created.
     fn value(&self) -> NonNegativeAmount;
+
     /// Returns the script corresponding to the newly created output.
     fn script_pubkey(&self) -> &Script;
+
+    /// Returns the serialized size of the txout.
+    fn serialized_size(&self) -> usize {
+        let mut buf: Vec<u8> = vec![];
+        self.script_pubkey()
+            .write(&mut buf)
+            .expect("script does not exceed available memory");
+        // The length of a transparent TxOut is the length of an amount plus the length of the serialized script pubkey.
+        8 + buf.len()
+    }
 }
 
 impl OutputView for TxOut {

zcash_primitives/src/transaction/fees/zip317.rs

@@ -6,7 +6,6 @@ use core::cmp::max;
 
 use crate::{
     consensus::{self, BlockHeight},
-    legacy::TransparentAddress,
     transaction::{
         components::{
             amount::{BalanceError, NonNegativeAmount},
@@ -147,27 +146,30 @@ impl super::FeeRule for FeeRule {
         &self,
         _params: &P,
         _target_height: BlockHeight,
-        transparent_inputs: &[impl transparent::InputView],
-        transparent_outputs: &[impl transparent::OutputView],
+        transparent_input_sizes: impl IntoIterator<Item = transparent::InputSize>,
+        transparent_output_sizes: impl IntoIterator<Item = usize>,
         sapling_input_count: usize,
         sapling_output_count: usize,
         orchard_action_count: usize,
     ) -> Result<NonNegativeAmount, Self::Error> {
-        let non_p2pkh_inputs: Vec<_> = transparent_inputs
-            .iter()
-            .filter_map(|t_in| match t_in.coin().script_pubkey.address() {
-                Some(TransparentAddress::PublicKeyHash(_)) => None,
-                _ => Some(t_in.outpoint()),
-            })
-            .cloned()
-            .collect();
+        let mut t_in_total_size: usize = 0;
+        let mut non_p2pkh_outpoints = vec![];
+        for sz in transparent_input_sizes.into_iter() {
+            match sz {
+                transparent::InputSize::Known(s) => {
+                    t_in_total_size += s;
+                }
+                transparent::InputSize::Unknown(outpoint) => {
+                    non_p2pkh_outpoints.push(outpoint.clone());
+                }
+            }
+        }
 
-        if !non_p2pkh_inputs.is_empty() {
-            return Err(FeeError::NonP2pkhInputs(non_p2pkh_inputs));
+        if !non_p2pkh_outpoints.is_empty() {
+            return Err(FeeError::NonP2pkhInputs(non_p2pkh_outpoints));
         }
 
-        let t_in_total_size = transparent_inputs.len() * 150;
-        let t_out_total_size = transparent_outputs.len() * 34;
+        let t_out_total_size = transparent_output_sizes.into_iter().sum();
 
         let ceildiv = |num: usize, den: usize| (num + den - 1) / den;