zcash_keys: Box `Address` elements to avoid large variations in enum variant sizes

zcash_client_backend/CHANGELOG.md

@@ -133,6 +133,9 @@ and this library adheres to Rust's notion of
   - Arguments to `ChangeStrategy::compute_balance` have changed.
   - `ChangeError::DustInputs` now has an `orchard` field behind the `orchard`
     feature flag.
+- `zcash_client_backend::wallet`:
+  - The address variants of `Recipient` now `Box` their contents to avoid large
+    discrepancies in enum variant sizing.
 - `zcash_client_backend::proto`:
   - `ProposalDecodingError` has a new variant `TransparentMemo`.
 - `zcash_client_backend::wallet::Recipient::InternalAccount` is now a structured

zcash_client_backend/src/data_api/wallet.rs

@@ -859,7 +859,7 @@ where
                         .convert_if_network(params.network_type())?;
 
                     let recipient_taddr = match recipient_address {
-                        Address::Transparent(t) => Some(t),
+                        Address::Transparent(t) => Some(t.as_ref()),
                         Address::Unified(uaddr) => uaddr.transparent(),
                         _ => None,
                     }
@@ -1022,7 +1022,7 @@ where
                 let memo = payment.memo().map_or_else(MemoBytes::empty, |m| m.clone());
                 builder.add_sapling_output(
                     sapling_external_ovk,
-                    addr,
+                    *addr,
                     payment.amount(),
                     memo.clone(),
                 )?;

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -8,7 +8,7 @@ use std::{
 };
 
 use nonempty::NonEmpty;
-use zcash_address::ConversionError;
+use zcash_address::{ConversionError, ZcashAddress};
 use zcash_primitives::{
     consensus::{self, BlockHeight},
     transaction::{
@@ -21,7 +21,7 @@ use zcash_primitives::{
 };
 
 use crate::{
-    address::{Address, UnifiedAddress},
+    address::Address,
     data_api::{InputSource, SimpleNoteRetention, SpendableNotes},
     fees::{sapling, ChangeError, ChangeStrategy, DustOutputPolicy},
     proposal::{Proposal, ProposalError, ShieldedInputs},
@@ -221,7 +221,7 @@ pub enum GreedyInputSelectorError<ChangeStrategyErrT, NoteRefT> {
     /// An intermediate value overflowed or underflowed the valid monetary range.
     Balance(BalanceError),
     /// A unified address did not contain a supported receiver.
-    UnsupportedAddress(Box<UnifiedAddress>),
+    UnsupportedAddress(ZcashAddress),
     /// An error was encountered in change selection.
     Change(ChangeError<ChangeStrategyErrT, NoteRefT>),
 }
@@ -234,10 +234,12 @@ impl<CE: fmt::Display, N: fmt::Display> fmt::Display for GreedyInputSelectorErro
                 "A balance calculation violated amount validity bounds: {:?}.",
                 e
             ),
-            GreedyInputSelectorError::UnsupportedAddress(_) => {
-                // we can't encode the UA to its string representation because we
-                // don't have network parameters here
-                write!(f, "Unified address contains no supported receivers.")
+            GreedyInputSelectorError::UnsupportedAddress(addr) => {
+                write!(
+                    f,
+                    "Unified address {} contains no supported receivers.",
+                    addr.encode()
+                )
             }
             GreedyInputSelectorError::Change(err) => {
                 write!(f, "An error occurred computing change and fees: {}", err)
@@ -401,7 +403,9 @@ where
                     }
 
                     return Err(InputSelectorError::Selection(
-                        GreedyInputSelectorError::UnsupportedAddress(Box::new(addr)),
+                        GreedyInputSelectorError::UnsupportedAddress(
+                            payment.recipient_address().clone(),
+                        ),
                     ));
                 }
             }

zcash_client_sqlite/src/testing.rs

@@ -1682,7 +1682,7 @@ fn fake_compact_block_spending<P: consensus::Parameters, Fvk: TestFvk>(
             compact_sapling_output(
                 params,
                 height,
-                recipient,
+                *recipient,
                 value,
                 fvk.sapling_ovk(),
                 &mut rng,

zcash_client_sqlite/src/testing/pool.rs

@@ -332,7 +332,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
     // spends the first step's output.
 
     // The first step will deshield to the wallet's default transparent address
-    let to0 = Address::Transparent(account.usk().default_transparent_address().0);
+    let to0 = Address::from(account.usk().default_transparent_address().0);
     let request0 = zip321::TransactionRequest::new(vec![Payment::without_memo(
         to0.to_zcash_address(&st.network()),
         NonNegativeAmount::const_from_u64(50000),
@@ -364,7 +364,7 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
 
     // We'll use an internal transparent address that hasn't been added to the wallet
     // to simulate an external transparent recipient.
-    let to1 = Address::Transparent(
+    let to1 = Address::from(
         account
             .usk()
             .transparent()
@@ -1620,7 +1620,7 @@ pub(crate) fn fully_funded_send_to_t<P0: ShieldedPoolTester, P1: ShieldedPoolTes
 
     let transfer_amount = NonNegativeAmount::const_from_u64(200000);
     let p0_to_p1 = zip321::TransactionRequest::new(vec![Payment::without_memo(
-        Address::Transparent(p1_to).to_zcash_address(&st.network()),
+        Address::Transparent(Box::new(p1_to)).to_zcash_address(&st.network()),
         transfer_amount,
     )])
     .unwrap();

zcash_client_sqlite/src/wallet.rs

@@ -555,7 +555,7 @@ pub(crate) fn get_current_address<P: consensus::Parameters>(
                 SqliteClientError::CorruptedData("Not a valid Zcash recipient address".to_owned())
             })
             .and_then(|addr| match addr {
-                Address::Unified(ua) => Ok(ua),
+                Address::Unified(ua) => Ok(*ua),
                 _ => Err(SqliteClientError::CorruptedData(format!(
                     "Addresses table contains {} which is not a unified address",
                     addr_str,
@@ -2386,7 +2386,7 @@ pub(crate) fn select_receiving_address<P: consensus::Parameters>(
                  FROM addresses
                  WHERE cached_transparent_receiver_address = :taddr",
                 named_params! {
-                    ":taddr": Address::Transparent(*taddr).encode(_params)
+                    ":taddr": Address::Transparent(Box::new(*taddr)).encode(_params)
                 },
                 |row| row.get::<_, String>(0),
             )

zcash_client_sqlite/src/wallet/init.rs

@@ -1420,7 +1420,7 @@ mod tests {
             // Orchard component.
             let ua_request =
                 UnifiedAddressRequest::unsafe_new_without_expiry(false, true, UA_TRANSPARENT);
-            let address_str = Address::Unified(
+            let address_str = Address::from(
                 ufvk.default_address(ua_request)
                     .expect("A valid default address exists for the UFVK")
                     .0,
@@ -1439,7 +1439,7 @@ mod tests {
             // add a transparent "sent note"
             #[cfg(feature = "transparent-inputs")]
             {
-                let taddr = Address::Transparent(
+                let taddr = Address::from(
                     *ufvk
                         .default_address(ua_request)
                         .expect("A valid default address exists for the UFVK")

zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs

@@ -79,7 +79,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                 ))
             })?;
             let decoded_address = if let Address::Unified(ua) = decoded {
-                ua
+                *ua
             } else {
                 return Err(WalletMigrationError::CorruptedData(
                     "Address in accounts table was not a Unified Address.".to_string(),
@@ -92,7 +92,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                 return Err(WalletMigrationError::CorruptedData(format!(
                     "Decoded UA {} does not match the UFVK's default address {} at {:?}.",
                     address,
-                    Address::Unified(expected_address).encode(&self.params),
+                    Address::from(expected_address).encode(&self.params),
                     idx,
                 )));
             }
@@ -110,7 +110,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                 let decoded_transparent_address = if let Address::Transparent(addr) =
                     decoded_transparent
                 {
-                    addr
+                    *addr
                 } else {
                     return Err(WalletMigrationError::CorruptedData(
                         "Address in transparent_address column of accounts table was not a transparent address.".to_string(),

zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs

@@ -120,12 +120,12 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                         let dfvk = ufvk.sapling().ok_or_else(||
                             WalletMigrationError::CorruptedData("Derivation should have produced a UFVK containing a Sapling component.".to_owned()))?;
                         let (idx, expected_address) = dfvk.default_address();
-                        if decoded_address != expected_address {
+                        if *decoded_address != expected_address {
                             return Err(if seed_is_relevant {
                                 WalletMigrationError::CorruptedData(
                                 format!("Decoded Sapling address {} does not match the ufvk's Sapling address {} at {:?}.",
                                     address,
-                                    Address::Sapling(expected_address).encode(&self.params),
+                                    Address::from(expected_address).encode(&self.params),
                                     idx))
                             } else {
                                 WalletMigrationError::SeedNotRelevant
@@ -138,12 +138,12 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                     }
                     Address::Unified(decoded_address) => {
                         let (expected_address, idx) = ufvk.default_address(ua_request)?;
-                        if decoded_address != expected_address {
+                        if *decoded_address != expected_address {
                             return Err(if seed_is_relevant {
                                 WalletMigrationError::CorruptedData(
                                 format!("Decoded unified address {} does not match the ufvk's default address {} at {:?}.",
                                     address,
-                                    Address::Unified(expected_address).encode(&self.params),
+                                    Address::from(expected_address).encode(&self.params),
                                     idx))
                             } else {
                                 WalletMigrationError::SeedNotRelevant

zcash_keys/CHANGELOG.md

@@ -36,6 +36,10 @@ and this library adheres to Rust's notion of
   must be enabled for the `keys` module to be accessible.
 - Updated to `zcash_primitives-0.15.0`
 
+### Changed
+- `zcash_keys::address::Address` variants now `Box` their contents to
+  avoid large discrepancies in enum variant sizing.
+
 ### Removed
 - `UnifiedFullViewingKey::new` has been placed behind the `test-dependencies`
   feature flag. UFVKs should only be produced by derivation from the USK, or

zcash_keys/src/address.rs

@@ -374,27 +374,27 @@ impl Receiver {
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum Address {
     #[cfg(feature = "sapling")]
-    Sapling(PaymentAddress),
-    Transparent(TransparentAddress),
-    Unified(UnifiedAddress),
+    Sapling(Box<PaymentAddress>),
+    Transparent(Box<TransparentAddress>),
+    Unified(Box<UnifiedAddress>),
 }
 
 #[cfg(feature = "sapling")]
 impl From<PaymentAddress> for Address {
     fn from(addr: PaymentAddress) -> Self {
-        Address::Sapling(addr)
+        Address::Sapling(Box::new(addr))
     }
 }
 
 impl From<TransparentAddress> for Address {
     fn from(addr: TransparentAddress) -> Self {
-        Address::Transparent(addr)
+        Address::Transparent(Box::new(addr))
     }
 }
 
 impl From<UnifiedAddress> for Address {
     fn from(addr: UnifiedAddress) -> Self {
-        Address::Unified(addr)
+        Address::Unified(Box::new(addr))
     }
 }
 
@@ -450,12 +450,12 @@ impl Address {
         match self {
             #[cfg(feature = "sapling")]
             Address::Sapling(pa) => ZcashAddress::from_sapling(net, pa.to_bytes()),
-            Address::Transparent(addr) => match addr {
+            Address::Transparent(addr) => match **addr {
                 TransparentAddress::PublicKeyHash(data) => {
-                    ZcashAddress::from_transparent_p2pkh(net, *data)
+                    ZcashAddress::from_transparent_p2pkh(net, data)
                 }
                 TransparentAddress::ScriptHash(data) => {
-                    ZcashAddress::from_transparent_p2sh(net, *data)
+                    ZcashAddress::from_transparent_p2sh(net, data)
                 }
             },
             Address::Unified(ua) => ua.to_address(net),
@@ -526,17 +526,17 @@ pub mod testing {
     #[cfg(feature = "sapling")]
     pub fn arb_addr(request: UnifiedAddressRequest) -> impl Strategy<Value = Address> {
         prop_oneof![
-            arb_payment_address().prop_map(Address::Sapling),
-            arb_transparent_addr().prop_map(Address::Transparent),
-            arb_unified_addr(Network::TestNetwork, request).prop_map(Address::Unified),
+            arb_payment_address().prop_map(Address::from),
+            arb_transparent_addr().prop_map(Address::from),
+            arb_unified_addr(Network::TestNetwork, request).prop_map(Address::from),
         ]
     }
 
     #[cfg(not(feature = "sapling"))]
     pub fn arb_addr(request: UnifiedAddressRequest) -> impl Strategy<Value = Address> {
         return prop_oneof![
-            arb_transparent_addr().prop_map(Address::Transparent),
-            arb_unified_addr(Network::TestNetwork, request).prop_map(Address::Unified),
+            arb_transparent_addr().prop_map(Address::from),
+            arb_unified_addr(Network::TestNetwork, request).prop_map(Address::from),
         ];
     }
 }
@@ -583,7 +583,7 @@ mod tests {
         #[cfg(all(feature = "orchard", not(feature = "sapling")))]
         let ua = UnifiedAddress::new_internal(orchard, transparent, None, None);
 
-        let addr = Address::Unified(ua);
+        let addr = Address::from(ua);
         let addr_str = addr.encode(&MAIN_NETWORK);
         assert_eq!(Address::decode(&MAIN_NETWORK, &addr_str), Some(addr));
     }