Allow ChangeValue::output_pool to reference the transparent pool by

changing its type from `ShieldedProtocol` to `PoolType`.

Also fix compilation errors when the "orchard" feature is used without
the "transparent-inputs" feature.

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

zcash_client_backend/CHANGELOG.md

@@ -10,6 +10,7 @@ and this library adheres to Rust's notion of
 ### Added
 - `zcash_client_backend::data_api`:
   - `chain::BlockCache` trait, behind the `sync` feature flag.
+- `zcash_client_backend::fees::ChangeValue::transparent`
 - `zcash_client_backend::scanning`:
   - `testing` module
 - `zcash_client_backend::sync` module, behind the `sync` feature flag.
@@ -26,6 +27,10 @@ and this library adheres to Rust's notion of
 - `zcash_client_backend::proto::proposal::Proposal::{from_standard_proposal, 
   try_into_standard_proposal}` each no longer require a `consensus::Parameters` 
   argument.
+- `zcash_client_backend::data_api::fees`
+  - The return type of `ChangeValue::output_pool`, and the type of the
+    `output_pool` argument to `ChangeValue::new`, have changed from
+    `ShieldedProtocol` to `zcash_protocol::PoolType`.
 - `zcash_client_backend::wallet::Recipient` variants have changed. Instead of
   wrapping protocol-address types, the `Recipient` type now wraps a
   `zcash_address::ZcashAddress`. This simplifies the process of tracking the

zcash_client_backend/src/data_api/wallet.rs

@@ -1051,8 +1051,9 @@ where
         let memo = change_value
             .memo()
             .map_or_else(MemoBytes::empty, |m| m.clone());
-        match change_value.output_pool() {
-            ShieldedProtocol::Sapling => {
+        let output_pool = change_value.output_pool();
+        match output_pool {
+            PoolType::Shielded(ShieldedProtocol::Sapling) => {
                 builder.add_sapling_output(
                     sapling_internal_ovk(),
                     sapling_dfvk.change_address().1,
@@ -1063,17 +1064,15 @@ where
                     Recipient::InternalAccount {
                         receiving_account: account,
                         external_address: None,
-                        note: PoolType::Shielded(ShieldedProtocol::Sapling),
+                        note: output_pool,
                     },
                     change_value.value(),
                     Some(memo),
                 ))
             }
-            ShieldedProtocol::Orchard => {
+            PoolType::Shielded(ShieldedProtocol::Orchard) => {
                 #[cfg(not(feature = "orchard"))]
-                return Err(Error::UnsupportedChangeType(PoolType::Shielded(
-                    ShieldedProtocol::Orchard,
-                )));
+                return Err(Error::UnsupportedChangeType(output_pool));
 
                 #[cfg(feature = "orchard")]
                 {
@@ -1087,13 +1086,16 @@ where
                         Recipient::InternalAccount {
                             receiving_account: account,
                             external_address: None,
-                            note: PoolType::Shielded(ShieldedProtocol::Orchard),
+                            note: output_pool,
                         },
                         change_value.value(),
                         Some(memo),
                     ))
                 }
             }
+            PoolType::Transparent => {
+                return Err(Error::UnsupportedChangeType(output_pool));
+            }
         }
     }
 

zcash_client_backend/src/fees.rs

@@ -11,8 +11,7 @@ use zcash_primitives::{
         fees::{transparent, FeeRule},
     },
 };
-
-use crate::ShieldedProtocol;
+use zcash_protocol::{PoolType, ShieldedProtocol};
 
 pub(crate) mod common;
 pub mod fixed;
@@ -25,29 +24,34 @@ pub mod zip317;
 /// A proposed change amount and output pool.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct ChangeValue {
-    output_pool: ShieldedProtocol,
+    output_pool: PoolType,
     value: NonNegativeAmount,
     memo: Option<MemoBytes>,
 }
 
 impl ChangeValue {
     /// Constructs a new change value from its constituent parts.
-    pub fn new(
-        output_pool: ShieldedProtocol,
-        value: NonNegativeAmount,
-        memo: Option<MemoBytes>,
-    ) -> Self {
+    pub fn new(output_pool: PoolType, value: NonNegativeAmount, memo: Option<MemoBytes>) -> Self {
         Self {
             output_pool,
             value,
             memo,
         }
     }
 
+    /// Constructs a new change value that will be created as a transparent output.
+    pub fn transparent(value: NonNegativeAmount) -> Self {
+        Self {
+            output_pool: PoolType::Transparent,
+            value,
+            memo: None,
+        }
+    }
+
     /// Constructs a new change value that will be created as a Sapling output.
     pub fn sapling(value: NonNegativeAmount, memo: Option<MemoBytes>) -> Self {
         Self {
-            output_pool: ShieldedProtocol::Sapling,
+            output_pool: PoolType::Shielded(ShieldedProtocol::Sapling),
             value,
             memo,
         }
@@ -57,14 +61,14 @@ impl ChangeValue {
     #[cfg(feature = "orchard")]
     pub fn orchard(value: NonNegativeAmount, memo: Option<MemoBytes>) -> Self {
         Self {
-            output_pool: ShieldedProtocol::Orchard,
+            output_pool: PoolType::Shielded(ShieldedProtocol::Orchard),
             value,
             memo,
         }
     }
 
     /// Returns the pool to which the change output should be sent.
-    pub fn output_pool(&self) -> ShieldedProtocol {
+    pub fn output_pool(&self) -> PoolType {
         self.output_pool
     }
 

zcash_client_backend/src/fees/common.rs

@@ -6,8 +6,7 @@ use zcash_primitives::{
         fees::{transparent, FeeRule},
     },
 };
-
-use crate::ShieldedProtocol;
+use zcash_protocol::{PoolType, ShieldedProtocol};
 
 use super::{
     sapling as sapling_fees, ChangeError, ChangeValue, DustAction, DustOutputPolicy,
@@ -220,7 +219,11 @@ where
                     })
                 }
                 DustAction::AllowDustChange => TransactionBalance::new(
-                    vec![ChangeValue::new(change_pool, proposed_change, change_memo)],
+                    vec![ChangeValue::new(
+                        PoolType::Shielded(change_pool),
+                        proposed_change,
+                        change_memo,
+                    )],
                     fee_amount,
                 )
                 .map_err(|_| overflow()),
@@ -232,7 +235,11 @@ where
             }
         } else {
             TransactionBalance::new(
-                vec![ChangeValue::new(change_pool, proposed_change, change_memo)],
+                vec![ChangeValue::new(
+                    PoolType::Shielded(change_pool),
+                    proposed_change,
+                    change_memo,
+                )],
                 fee_amount,
             )
             .map_err(|_| overflow())

zcash_client_backend/src/proposal.rs

@@ -490,57 +490,37 @@ impl<NoteRef> Step<NoteRef> {
         self.is_shielding
     }
 
-    /// Returns whether or not this proposal requires interaction with the specified pool
+    /// Returns whether or not this proposal requires interaction with the specified pool.
     pub fn involves(&self, pool_type: PoolType) -> bool {
-        match pool_type {
-            PoolType::Transparent => {
-                self.is_shielding
-                    || !self.transparent_inputs.is_empty()
-                    || self
-                        .payment_pools()
-                        .values()
-                        .any(|pool| matches!(pool, PoolType::Transparent))
-            }
+        let input_in_this_pool = || match pool_type {
+            PoolType::Transparent => self.is_shielding || !self.transparent_inputs.is_empty(),
             PoolType::Shielded(ShieldedProtocol::Sapling) => {
-                let sapling_in = self.shielded_inputs.iter().any(|s_in| {
+                self.shielded_inputs.iter().any(|s_in| {
                     s_in.notes()
                         .iter()
                         .any(|note| matches!(note.note(), Note::Sapling(_)))
-                });
-                let sapling_out = self
-                    .payment_pools()
-                    .values()
-                    .any(|pool| matches!(pool, PoolType::Shielded(ShieldedProtocol::Sapling)));
-                let sapling_change = self
-                    .balance
-                    .proposed_change()
-                    .iter()
-                    .any(|c| c.output_pool() == ShieldedProtocol::Sapling);
-
-                sapling_in || sapling_out || sapling_change
+                })
             }
+            #[cfg(feature = "orchard")]
             PoolType::Shielded(ShieldedProtocol::Orchard) => {
-                #[cfg(not(feature = "orchard"))]
-                let orchard_in = false;
-                #[cfg(feature = "orchard")]
-                let orchard_in = self.shielded_inputs.iter().any(|s_in| {
+                self.shielded_inputs.iter().any(|s_in| {
                     s_in.notes()
                         .iter()
                         .any(|note| matches!(note.note(), Note::Orchard(_)))
-                });
-                let orchard_out = self
-                    .payment_pools()
-                    .values()
-                    .any(|pool| matches!(pool, PoolType::Shielded(ShieldedProtocol::Orchard)));
-                let orchard_change = self
-                    .balance
-                    .proposed_change()
-                    .iter()
-                    .any(|c| c.output_pool() == ShieldedProtocol::Orchard);
-
-                orchard_in || orchard_out || orchard_change
+                })
             }
-        }
+            #[cfg(not(feature = "orchard"))]
+            PoolType::Shielded(ShieldedProtocol::Orchard) => false,
+        };
+        let output_in_this_pool = || self.payment_pools().values().any(|pool| *pool == pool_type);
+        let change_in_this_pool = || {
+            self.balance
+                .proposed_change()
+                .iter()
+                .any(|c| c.output_pool() == pool_type)
+        };
+
+        input_in_this_pool() || output_in_this_pool() || change_in_this_pool()
     }
 }
 

zcash_client_sqlite/src/testing/pool.rs

@@ -63,11 +63,14 @@ use crate::{
 #[cfg(feature = "transparent-inputs")]
 use {
     zcash_client_backend::{
-        fees::TransactionBalance, proposal::Step, wallet::WalletTransparentOutput, PoolType,
+        fees::TransactionBalance, proposal::Step, wallet::WalletTransparentOutput,
     },
     zcash_primitives::transaction::components::{OutPoint, TxOut},
 };
 
+#[cfg(any(feature = "transparent-inputs", feature = "orchard"))]
+use zcash_client_backend::PoolType;
+
 pub(crate) type OutputRecoveryError = Error<
     SqliteClientError,
     commitment_tree::Error,
@@ -1485,7 +1488,10 @@ pub(crate) fn pool_crossing_required<P0: ShieldedPoolTester, P1: ShieldedPoolTes
     // Since this is a cross-pool transfer, change will be sent to the preferred pool.
     assert_eq!(
         change_output.output_pool(),
-        std::cmp::max(ShieldedProtocol::Sapling, ShieldedProtocol::Orchard)
+        PoolType::Shielded(std::cmp::max(
+            ShieldedProtocol::Sapling,
+            ShieldedProtocol::Orchard
+        ))
     );
     assert_eq!(change_output.value(), expected_change);
 
@@ -1574,7 +1580,10 @@ pub(crate) fn fully_funded_fully_private<P0: ShieldedPoolTester, P1: ShieldedPoo
     let change_output = proposed_change.get(0).unwrap();
     // Since there are sufficient funds in either pool, change is kept in the same pool as
     // the source note (the target pool), and does not necessarily follow preference order.
-    assert_eq!(change_output.output_pool(), P1::SHIELDED_PROTOCOL);
+    assert_eq!(
+        change_output.output_pool(),
+        PoolType::Shielded(P1::SHIELDED_PROTOCOL)
+    );
     assert_eq!(change_output.value(), expected_change);
 
     let create_proposed_result = st.create_proposed_transactions::<Infallible, _>(
@@ -1661,7 +1670,10 @@ pub(crate) fn fully_funded_send_to_t<P0: ShieldedPoolTester, P1: ShieldedPoolTes
     // Since there are sufficient funds in either pool, change is kept in the same pool as
     // the source note (the target pool), and does not necessarily follow preference order.
     // The source note will always be sapling, as we spend Sapling funds preferentially.
-    assert_eq!(change_output.output_pool(), ShieldedProtocol::Sapling);
+    assert_eq!(
+        change_output.output_pool(),
+        PoolType::Shielded(ShieldedProtocol::Sapling)
+    );
     assert_eq!(change_output.value(), expected_change);
 
     let create_proposed_result = st.create_proposed_transactions::<Infallible, _>(