Make `ephemeral_parameters` and `EphemeralParameters::NONE` available

unconditionally.

This removes a bunch of `#[cfg(feature = "transparent-inputs")]`
conditionals, with negligible (if any, after compiler optimization)
overhead. It also requires callers of functions with an
`ephemeral_parameters` parameter to do the intended thing to work
with "transparent-inputs" either enabled or disabled.

Signed-off-by: Daira-Emma Hopwood <daira@jacaranda.org>

zcash_client_backend/CHANGELOG.md

@@ -67,11 +67,12 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
     return type of `ChangeValue::output_pool` has (unconditionally) changed
     from `ShieldedProtocol` to `zcash_protocol::PoolType`.
   - `ChangeStrategy::compute_balance`: this trait method has an additional
-    `&EphemeralParameters` parameter when the "transparent-inputs" feature is
+    `&EphemeralParameters` parameter. If the "transparent-inputs" feature is
-    enabled. This can be used to specify whether the change memo should be
+    enabled, this can be used to specify whether the change memo should be
     ignored, and the amounts of additional transparent P2PKH inputs and
     outputs. Passing `&EphemeralParameters::NONE` will retain the previous
-    behaviour.
+    behaviour (and is necessary when the "transparent-inputs" feature is
+    not enabled).
 - `zcash_client_backend::input_selection::GreedyInputSelectorError` has a
   new variant `UnsupportedTexAddress`.
 - `zcash_client_backend::proto::ProposalDecodingError` has a new variant

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -23,7 +23,7 @@ use zcash_primitives::{
 use crate::{
     address::{Address, UnifiedAddress},
     data_api::{InputSource, SimpleNoteRetention, SpendableNotes},
-    fees::{sapling, ChangeError, ChangeStrategy, DustOutputPolicy},
+    fees::{sapling, ChangeError, ChangeStrategy, DustOutputPolicy, EphemeralParameters},
     proposal::{Proposal, ProposalError, ShieldedInputs},
     wallet::WalletTransparentOutput,
     zip321::TransactionRequest,
@@ -33,7 +33,7 @@ use crate::{
 #[cfg(feature = "transparent-inputs")]
 use {
     crate::{
-        fees::{ChangeValue, EphemeralParameters},
+        fees::ChangeValue,
         proposal::{Step, StepOutput, StepOutputIndex},
         zip321::Payment,
     },
@@ -459,6 +459,9 @@ where
             }
         }
 
+        #[cfg(not(feature = "transparent-inputs"))]
+        let ephemeral_parameters = EphemeralParameters::NONE;
+
         #[cfg(feature = "transparent-inputs")]
         let (ephemeral_parameters, tr1_balance_opt) = {
             if tr1_transparent_outputs.is_empty() {
@@ -579,7 +582,6 @@ where
                     &orchard_outputs[..],
                 ),
                 &self.dust_output_policy,
-                #[cfg(feature = "transparent-inputs")]
                 &ephemeral_parameters,
             );
 
@@ -771,7 +773,6 @@ where
             #[cfg(feature = "orchard")]
             &orchard_fees::EmptyBundleView,
             &self.dust_output_policy,
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );
 

zcash_client_backend/src/fees.rs

@@ -332,8 +332,9 @@ impl Default for DustOutputPolicy {
 
 /// `EphemeralParameters` can be used to specify variations on how balance
 /// and fees are computed that are relevant to transactions using ephemeral
-/// transparent outputs.
+/// transparent outputs. These are only relevant when the "transparent-inputs"
-#[cfg(feature = "transparent-inputs")]
+/// feature is enabled, but to reduce feature-dependent boilerplate, the type
+/// and the `EphemeralParameters::NONE` constant are present unconditionally.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct EphemeralParameters {
     ignore_change_memo: bool,
@@ -341,7 +342,6 @@ pub struct EphemeralParameters {
     ephemeral_output_amount: Option<NonNegativeAmount>,
 }
 
-#[cfg(feature = "transparent-inputs")]
 impl EphemeralParameters {
     /// An `EphemeralParameters` indicating no use of ephemeral inputs
     /// or outputs. It has:
@@ -349,7 +349,11 @@ impl EphemeralParameters {
     /// * `ignore_change_memo: false`,
     /// * `ephemeral_input_amount: None`,
     /// * `ephemeral_output_amount: None`.
-    pub const NONE: Self = Self::new(false, None, None);
+    pub const NONE: Self = Self {
+        ignore_change_memo: false,
+        ephemeral_input_amount: None,
+        ephemeral_output_amount: None,
+    };
 
     /// Returns an `EphemeralParameters` with the following parameters:
     ///
@@ -360,6 +364,7 @@ impl EphemeralParameters {
     ///   additional P2PKH input of the given amount.
     /// * `ephemeral_output_amount`: specifies that there will be an
     ///   additional P2PKH output of the given amount.
+    #[cfg(feature = "transparent-inputs")]
     pub const fn new(
         ignore_change_memo: bool,
         ephemeral_input_amount: Option<NonNegativeAmount>,
@@ -414,6 +419,10 @@ pub trait ChangeStrategy {
                and fees are computed that are relevant to transactions using ephemeral
                transparent outputs; see [`EphemeralParameters::new`]."
     )]
+    #[cfg_attr(
+        not(feature = "transparent-inputs"),
+        doc = "`ephemeral_parameters` should be set to `&EphemeralParameters::NONE`."
+    )]
     #[allow(clippy::too_many_arguments)]
     fn compute_balance<P: consensus::Parameters, NoteRefT: Clone>(
         &self,
@@ -424,7 +433,7 @@ pub trait ChangeStrategy {
         sapling: &impl sapling::BundleView<NoteRefT>,
         #[cfg(feature = "orchard")] orchard: &impl orchard::BundleView<NoteRefT>,
         dust_output_policy: &DustOutputPolicy,
-        #[cfg(feature = "transparent-inputs")] ephemeral_parameters: &EphemeralParameters,
+        ephemeral_parameters: &EphemeralParameters,
     ) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>>;
 }
 

zcash_client_backend/src/fees/common.rs

@@ -5,22 +5,14 @@ use zcash_primitives::{
     memo::MemoBytes,
     transaction::{
         components::amount::{BalanceError, NonNegativeAmount},
-        fees::{transparent, zip317::MINIMUM_FEE, FeeRule},
+        fees::{transparent, zip317::MINIMUM_FEE, zip317::P2PKH_STANDARD_OUTPUT_SIZE, FeeRule},
     },
 };
 use zcash_protocol::ShieldedProtocol;
 
 use super::{
     sapling as sapling_fees, ChangeError, ChangeValue, DustAction, DustOutputPolicy,
-    TransactionBalance,
+    EphemeralParameters, TransactionBalance,
-};
-
-#[cfg(feature = "transparent-inputs")]
-use {
-    super::EphemeralParameters,
-    zcash_primitives::transaction::fees::{
-        transparent::InputSize, zip317::P2PKH_STANDARD_OUTPUT_SIZE,
-    },
 };
 
 #[cfg(feature = "orchard")]
@@ -57,17 +49,14 @@ pub(crate) fn calculate_net_flows<NoteRefT: Clone, F: FeeRule, E>(
     transparent_outputs: &[impl transparent::OutputView],
     sapling: &impl sapling_fees::BundleView<NoteRefT>,
     #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
-    #[cfg(feature = "transparent-inputs")] ephemeral_input_amount: Option<NonNegativeAmount>,
+    ephemeral_input_amount: Option<NonNegativeAmount>,
-    #[cfg(feature = "transparent-inputs")] ephemeral_output_amount: Option<NonNegativeAmount>,
+    ephemeral_output_amount: Option<NonNegativeAmount>,
 ) -> Result<NetFlows, ChangeError<E, NoteRefT>>
 where
     E: From<F::Error> + From<BalanceError>,
 {
     let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow));
 
-    #[cfg(not(feature = "transparent-inputs"))]
-    let (ephemeral_input_amount, ephemeral_output_amount) = (None, None);
-
     let t_in = transparent_inputs
         .iter()
         .map(|t_in| t_in.coin().value)
@@ -173,12 +162,11 @@ pub(crate) fn single_change_output_balance<
     fallback_change_pool: ShieldedProtocol,
     marginal_fee: NonNegativeAmount,
     grace_actions: usize,
-    #[cfg(feature = "transparent-inputs")] ephemeral_parameters: &EphemeralParameters,
+    ephemeral_parameters: &EphemeralParameters,
 ) -> Result<TransactionBalance, ChangeError<E, NoteRefT>>
 where
     E: From<F::Error> + From<BalanceError>,
 {
-    #[cfg(feature = "transparent-inputs")]
     let change_memo = change_memo.filter(|_| !ephemeral_parameters.ignore_change_memo());
 
     let overflow = || ChangeError::StrategyError(E::from(BalanceError::Overflow));
@@ -190,9 +178,7 @@ where
         sapling,
         #[cfg(feature = "orchard")]
         orchard,
-        #[cfg(feature = "transparent-inputs")]
         ephemeral_parameters.ephemeral_input_amount(),
-        #[cfg(feature = "transparent-inputs")]
         ephemeral_parameters.ephemeral_output_amount(),
     )?;
 
@@ -227,7 +213,6 @@ where
             marginal_fee,
             grace_actions,
             &possible_change[..],
-            #[cfg(feature = "transparent-inputs")]
             ephemeral_parameters,
         )?;
     }
@@ -298,20 +283,22 @@ where
     // Note that using the `DustAction::AddDustToFee` policy inherently leaks
     // more information.
 
-    let transparent_input_sizes = transparent_inputs.iter().map(|i| i.serialized_size());
+    let transparent_input_sizes = transparent_inputs
-    #[cfg(feature = "transparent-inputs")]
+        .iter()
-    let transparent_input_sizes = transparent_input_sizes.chain(
+        .map(|i| i.serialized_size())
-        ephemeral_parameters
+        .chain(
-            .ephemeral_input_amount()
+            ephemeral_parameters
-            .map(|_| InputSize::STANDARD_P2PKH),
+                .ephemeral_input_amount()
-    );
+                .map(|_| transparent::InputSize::STANDARD_P2PKH),
-    let transparent_output_sizes = transparent_outputs.iter().map(|i| i.serialized_size());
+        );
-    #[cfg(feature = "transparent-inputs")]
+    let transparent_output_sizes = transparent_outputs
-    let transparent_output_sizes = transparent_output_sizes.chain(
+        .iter()
-        ephemeral_parameters
+        .map(|i| i.serialized_size())
-            .ephemeral_output_amount()
+        .chain(
-            .map(|_| P2PKH_STANDARD_OUTPUT_SIZE),
+            ephemeral_parameters
-    );
+                .ephemeral_output_amount()
+                .map(|_| P2PKH_STANDARD_OUTPUT_SIZE),
+        );
 
     let fee_without_change = fee_rule
         .fee_required(
@@ -469,7 +456,7 @@ pub(crate) fn check_for_uneconomic_inputs<NoteRefT: Clone, E>(
     marginal_fee: NonNegativeAmount,
     grace_actions: usize,
     possible_change: &[(usize, usize, usize)],
-    #[cfg(feature = "transparent-inputs")] ephemeral_parameters: &EphemeralParameters,
+    ephemeral_parameters: &EphemeralParameters,
 ) -> Result<(), ChangeError<E, NoteRefT>> {
     let mut t_dust: Vec<_> = transparent_inputs
         .iter()
@@ -516,11 +503,11 @@ pub(crate) fn check_for_uneconomic_inputs<NoteRefT: Clone, E>(
         return Ok(());
     }
 
-    let (t_inputs_len, t_outputs_len) = (transparent_inputs.len(), transparent_outputs.len());
-    #[cfg(feature = "transparent-inputs")]
     let (t_inputs_len, t_outputs_len) = (
-        t_inputs_len + usize::from(ephemeral_parameters.ephemeral_input_amount().is_some()),
+        transparent_inputs.len()
-        t_outputs_len + usize::from(ephemeral_parameters.ephemeral_output_amount().is_some()),
+            + usize::from(ephemeral_parameters.ephemeral_input_amount().is_some()),
+        transparent_outputs.len()
+            + usize::from(ephemeral_parameters.ephemeral_output_amount().is_some()),
     );
     let (s_inputs_len, s_outputs_len) = (sapling.inputs().len(), sapling.outputs().len());
     #[cfg(feature = "orchard")]

zcash_client_backend/src/fees/fixed.rs

@@ -13,15 +13,12 @@ use crate::ShieldedProtocol;
 
 use super::{
     common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy,
-    DustOutputPolicy, TransactionBalance,
+    DustOutputPolicy, EphemeralParameters, TransactionBalance,
 };
 
 #[cfg(feature = "orchard")]
 use super::orchard as orchard_fees;
 
-#[cfg(feature = "transparent-inputs")]
-use super::EphemeralParameters;
-
 /// A change strategy that proposes change as a single output to the most current supported
 /// shielded pool and delegates fee calculation to the provided fee rule.
 pub struct SingleOutputChangeStrategy {
@@ -66,7 +63,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
         sapling: &impl sapling_fees::BundleView<NoteRefT>,
         #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
         dust_output_policy: &DustOutputPolicy,
-        #[cfg(feature = "transparent-inputs")] ephemeral_parameters: &EphemeralParameters,
+        ephemeral_parameters: &EphemeralParameters,
     ) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
         single_change_output_balance(
             params,
@@ -83,7 +80,6 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
             self.fallback_change_pool,
             NonNegativeAmount::ZERO,
             0,
-            #[cfg(feature = "transparent-inputs")]
             ephemeral_parameters,
         )
     }
@@ -104,14 +100,11 @@ mod tests {
         data_api::wallet::input_selection::SaplingPayment,
         fees::{
             tests::{TestSaplingInput, TestTransparentInput},
-            ChangeError, ChangeStrategy, ChangeValue, DustOutputPolicy,
+            ChangeError, ChangeStrategy, ChangeValue, DustOutputPolicy, EphemeralParameters,
         },
         ShieldedProtocol,
     };
 
-    #[cfg(feature = "transparent-inputs")]
-    use crate::fees::EphemeralParameters;
-
     #[cfg(feature = "orchard")]
     use crate::fees::orchard as orchard_fees;
 
@@ -143,7 +136,6 @@ mod tests {
             #[cfg(feature = "orchard")]
             &orchard_fees::EmptyBundleView,
             &DustOutputPolicy::default(),
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );
 
@@ -190,7 +182,6 @@ mod tests {
             #[cfg(feature = "orchard")]
             &orchard_fees::EmptyBundleView,
             &DustOutputPolicy::default(),
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );
 

zcash_client_backend/src/fees/standard.rs

@@ -18,12 +18,9 @@ use crate::ShieldedProtocol;
 
 use super::{
     fixed, sapling as sapling_fees, zip317, ChangeError, ChangeStrategy, DustOutputPolicy,
-    TransactionBalance,
+    EphemeralParameters, TransactionBalance,
 };
 
-#[cfg(feature = "transparent-inputs")]
-use super::EphemeralParameters;
-
 #[cfg(feature = "orchard")]
 use super::orchard as orchard_fees;
 
@@ -71,7 +68,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
         sapling: &impl sapling_fees::BundleView<NoteRefT>,
         #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
         dust_output_policy: &DustOutputPolicy,
-        #[cfg(feature = "transparent-inputs")] ephemeral_parameters: &EphemeralParameters,
+        ephemeral_parameters: &EphemeralParameters,
     ) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
         #[allow(deprecated)]
         match self.fee_rule() {
@@ -89,7 +86,6 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
                 #[cfg(feature = "orchard")]
                 orchard,
                 dust_output_policy,
-                #[cfg(feature = "transparent-inputs")]
                 ephemeral_parameters,
             )
             .map_err(|e| e.map(Zip317FeeError::Balance)),
@@ -107,7 +103,6 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
                 #[cfg(feature = "orchard")]
                 orchard,
                 dust_output_policy,
-                #[cfg(feature = "transparent-inputs")]
                 ephemeral_parameters,
             )
             .map_err(|e| e.map(Zip317FeeError::Balance)),
@@ -125,7 +120,6 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
                 #[cfg(feature = "orchard")]
                 orchard,
                 dust_output_policy,
-                #[cfg(feature = "transparent-inputs")]
                 ephemeral_parameters,
             ),
         }

zcash_client_backend/src/fees/zip317.rs

@@ -17,12 +17,9 @@ use crate::ShieldedProtocol;
 
 use super::{
     common::single_change_output_balance, sapling as sapling_fees, ChangeError, ChangeStrategy,
-    DustOutputPolicy, TransactionBalance,
+    DustOutputPolicy, EphemeralParameters, TransactionBalance,
 };
 
-#[cfg(feature = "transparent-inputs")]
-use super::EphemeralParameters;
-
 #[cfg(feature = "orchard")]
 use super::orchard as orchard_fees;
 
@@ -70,7 +67,7 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
         sapling: &impl sapling_fees::BundleView<NoteRefT>,
         #[cfg(feature = "orchard")] orchard: &impl orchard_fees::BundleView<NoteRefT>,
         dust_output_policy: &DustOutputPolicy,
-        #[cfg(feature = "transparent-inputs")] ephemeral_parameters: &EphemeralParameters,
+        ephemeral_parameters: &EphemeralParameters,
     ) -> Result<TransactionBalance, ChangeError<Self::Error, NoteRefT>> {
         single_change_output_balance(
             params,
@@ -87,7 +84,6 @@ impl ChangeStrategy for SingleOutputChangeStrategy {
             self.fallback_change_pool,
             self.fee_rule.marginal_fee(),
             self.fee_rule.grace_actions(),
-            #[cfg(feature = "transparent-inputs")]
             ephemeral_parameters,
         )
     }
@@ -112,13 +108,11 @@ mod tests {
         fees::{
             tests::{TestSaplingInput, TestTransparentInput},
             ChangeError, ChangeStrategy, ChangeValue, DustAction, DustOutputPolicy,
+            EphemeralParameters,
         },
         ShieldedProtocol,
     };
 
-    #[cfg(feature = "transparent-inputs")]
-    use crate::fees::EphemeralParameters;
-
     #[cfg(feature = "orchard")]
     use {
         crate::data_api::wallet::input_selection::OrchardPayment,
@@ -154,7 +148,6 @@ mod tests {
             #[cfg(feature = "orchard")]
             &orchard_fees::EmptyBundleView,
             &DustOutputPolicy::default(),
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );
 
@@ -199,7 +192,6 @@ mod tests {
                 ))][..],
             ),
             &DustOutputPolicy::default(),
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );
 
@@ -253,7 +245,6 @@ mod tests {
             #[cfg(feature = "orchard")]
             &orchard_fees::EmptyBundleView,
             dust_output_policy,
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );
 
@@ -298,7 +289,6 @@ mod tests {
             #[cfg(feature = "orchard")]
             &orchard_fees::EmptyBundleView,
             &DustOutputPolicy::default(),
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );
 
@@ -343,7 +333,6 @@ mod tests {
             #[cfg(feature = "orchard")]
             &orchard_fees::EmptyBundleView,
             &DustOutputPolicy::default(),
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );
 
@@ -394,7 +383,6 @@ mod tests {
                 DustAction::AllowDustChange,
                 Some(NonNegativeAmount::const_from_u64(1000)),
             ),
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );
 
@@ -456,7 +444,6 @@ mod tests {
             #[cfg(feature = "orchard")]
             &orchard_fees::EmptyBundleView,
             dust_output_policy,
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );
 
@@ -508,7 +495,6 @@ mod tests {
             #[cfg(feature = "orchard")]
             &orchard_fees::EmptyBundleView,
             &DustOutputPolicy::default(),
-            #[cfg(feature = "transparent-inputs")]
             &EphemeralParameters::NONE,
         );