zcash_keys: Box Address elements to avoid large variations in enum …

…variant sizes

zcash_client_backend/CHANGELOG.md

@@ -184,6 +184,8 @@ and this library adheres to Rust's notion of
   - The fields of `ReceivedSaplingNote` are now private. Use
     `ReceivedSaplingNote::from_parts` for construction instead. Accessor methods
     are provided for each previously public field.
+  - The address variants of `Recipient` now `Box` their contents to avoid large
+    discrepancies in enum variant sizing.
 - `zcash_client_backend::scanning::ScanError` has a new variant, `TreeSizeInvalid`.
 - The following fields now have type `NonNegativeAmount` instead of `Amount`:
   - `zcash_client_backend::data_api`:

zcash_client_backend/src/data_api/wallet.rs

@@ -703,16 +703,20 @@ where
                     .memo
                     .as_ref()
                     .map_or_else(MemoBytes::empty, |m| m.clone());
-                builder.add_sapling_output(external_ovk, *addr, payment.amount, memo.clone())?;
-                sapling_output_meta.push((Recipient::Sapling(*addr), payment.amount, Some(memo)));
+                builder.add_sapling_output(external_ovk, **addr, payment.amount, memo.clone())?;
+                sapling_output_meta.push((
+                    Recipient::Sapling(addr.clone()),
+                    payment.amount,
+                    Some(memo),
+                ));
             }
             Address::Transparent(to) => {
                 if payment.memo.is_some() {
                     return Err(Error::MemoForbidden);
                 } else {
                     builder.add_transparent_output(to, payment.amount)?;
                 }
-                transparent_output_meta.push((*to, payment.amount));
+                transparent_output_meta.push((to, payment.amount));
             }
         }
     }
@@ -807,7 +811,7 @@ where
 
         SentTransactionOutput::from_parts(
             output_index,
-            Recipient::Transparent(addr),
+            Recipient::Transparent(addr.clone()),
             value,
             None,
             None,

zcash_client_backend/src/data_api/wallet/input_selection.rs

@@ -542,7 +542,7 @@ where
 
             match &payment.recipient_address {
                 Address::Transparent(addr) => {
-                    push_transparent(*addr);
+                    push_transparent(**addr);
                 }
                 Address::Sapling(_) => {
                     push_sapling();
@@ -562,7 +562,7 @@ where
                         push_transparent(*addr);
                     } else {
                         return Err(InputSelectorError::Selection(
-                            GreedyInputSelectorError::UnsupportedAddress(Box::new(addr.clone())),
+                            GreedyInputSelectorError::UnsupportedAddress(addr.clone()),
                         ));
                     }
                 }

zcash_client_backend/src/wallet.rs

@@ -63,9 +63,9 @@ impl NoteId {
 /// output.
 #[derive(Debug, Clone)]
 pub enum Recipient {
-    Transparent(TransparentAddress),
-    Sapling(sapling::PaymentAddress),
-    Unified(UnifiedAddress, PoolType),
+    Transparent(Box<TransparentAddress>),
+    Sapling(Box<sapling::PaymentAddress>),
+    Unified(Box<UnifiedAddress>, PoolType),
     InternalAccount(AccountId, PoolType),
 }
 

zcash_client_backend/src/zip321.rs

@@ -773,9 +773,9 @@ pub mod testing {
 
     pub fn arb_addr() -> impl Strategy<Value = Address> {
         prop_oneof![
-            arb_payment_address().prop_map(Address::Sapling),
-            arb_transparent_addr().prop_map(Address::Transparent),
-            arb_unified_addr().prop_map(Address::Unified),
+            arb_payment_address().prop_map(Address::from),
+            arb_transparent_addr().prop_map(Address::from),
+            arb_unified_addr().prop_map(Address::from),
         ]
     }
 
@@ -904,7 +904,7 @@ mod tests {
         let expected = TransactionRequest {
             payments: vec![
                 Payment {
-                    recipient_address: Address::Sapling(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap()),
+                    recipient_address: Address::from(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap()),
                     amount: NonNegativeAmount::const_from_u64(376876902796286),
                     memo: None,
                     label: None,
@@ -925,7 +925,7 @@ mod tests {
         let expected = TransactionRequest {
             payments: vec![
                 Payment {
-                    recipient_address: Address::Sapling(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap()),
+                    recipient_address: Address::from(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap()),
                     amount: NonNegativeAmount::ZERO,
                     memo: None,
                     label: None,
@@ -943,7 +943,7 @@ mod tests {
         let req = TransactionRequest {
             payments: vec![
                 Payment {
-                    recipient_address: Address::Sapling(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap()),
+                    recipient_address: Address::from(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap()),
                     amount: NonNegativeAmount::ZERO,
                     memo: None,
                     label: None,

zcash_client_sqlite/src/chain.rs

@@ -520,7 +520,7 @@ mod tests {
 
         // We can spend the received notes
         let req = TransactionRequest::new(vec![Payment {
-            recipient_address: Address::Sapling(dfvk.default_address().1),
+            recipient_address: Address::from(dfvk.default_address().1),
             amount: NonNegativeAmount::const_from_u64(110_000),
             memo: None,
             label: None,

zcash_client_sqlite/src/lib.rs

@@ -594,7 +594,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                 match output.transfer_type {
                     TransferType::Outgoing | TransferType::WalletInternal => {
                         let recipient = if output.transfer_type == TransferType::Outgoing {
-                            Recipient::Sapling(output.note.recipient())
+                            Recipient::Sapling(Box::new(output.note.recipient()))
                         } else {
                             Recipient::InternalAccount(
                                 output.account,
@@ -663,7 +663,7 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                                 *account_id,
                                 tx_ref,
                                 output_index,
-                                &Recipient::Transparent(address),
+                                &Recipient::Transparent(Box::new(address)),
                                 txout.value,
                                 None
                             )?;

zcash_client_sqlite/src/wallet.rs

@@ -294,7 +294,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,

zcash_client_sqlite/src/wallet/init.rs

@@ -1005,7 +1005,7 @@ mod tests {
             )?;
 
             let ufvk_str = ufvk.encode(&wdb.params);
-            let address_str = Address::Unified(ufvk.default_address().0).encode(&wdb.params);
+            let address_str = Address::from(ufvk.default_address().0).encode(&wdb.params);
             wdb.conn.execute(
                 "INSERT INTO accounts (account, ufvk, address, transparent_address)
                 VALUES (?, ?, ?, '')",
@@ -1019,7 +1019,7 @@ mod tests {
             // add a transparent "sent note"
             #[cfg(feature = "transparent-inputs")]
             {
-                let taddr = Address::Transparent(*ufvk.default_address().0.transparent().unwrap())
+                let taddr = Address::from(*ufvk.default_address().0.transparent().unwrap())
                     .encode(&wdb.params);
                 wdb.conn.execute(
                     "INSERT INTO blocks (height, hash, time, sapling_tree) VALUES (0, 0, 0, x'000000')",

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

@@ -78,7 +78,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(),
@@ -89,7 +89,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,
                 )));
             }
@@ -107,7 +107,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

@@ -92,11 +92,11 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                             "Derivation should have produced a UFVK containing a Sapling component.",
                         );
                         let (idx, expected_address) = dfvk.default_address();
-                        if decoded_address != expected_address {
+                        if *decoded_address != expected_address {
                             return Err(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)));
                         }
                     }
@@ -106,11 +106,11 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                     }
                     Address::Unified(decoded_address) => {
                         let (expected_address, idx) = ufvk.default_address();
-                        if decoded_address != expected_address {
+                        if *decoded_address != expected_address {
                             return Err(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)));
                         }
                     }

zcash_client_sqlite/src/wallet/sapling.rs

@@ -1276,7 +1276,7 @@ pub(crate) mod tests {
         let req = TransactionRequest::new(vec![
             // payment to an external recipient
             Payment {
-                recipient_address: Address::Sapling(addr2),
+                recipient_address: Address::from(addr2),
                 amount: amount_sent,
                 memo: None,
                 label: None,
@@ -1285,7 +1285,7 @@ pub(crate) mod tests {
             },
             // payment back to the originating wallet, simulating legacy change
             Payment {
-                recipient_address: Address::Sapling(addr),
+                recipient_address: Address::from(addr),
                 amount: amount_legacy_change,
                 memo: None,
                 label: None,
@@ -1399,7 +1399,7 @@ pub(crate) mod tests {
 
         // This first request will fail due to insufficient non-dust funds
         let req = TransactionRequest::new(vec![Payment {
-            recipient_address: Address::Sapling(dfvk.default_address().1),
+            recipient_address: Address::from(dfvk.default_address().1),
             amount: NonNegativeAmount::const_from_u64(50000),
             memo: None,
             label: None,
@@ -1424,7 +1424,7 @@ pub(crate) mod tests {
         // This request will succeed, spending a single dust input to pay the 10000
         // ZAT fee in addition to the 41000 ZAT output to the recipient
         let req = TransactionRequest::new(vec![Payment {
-            recipient_address: Address::Sapling(dfvk.default_address().1),
+            recipient_address: Address::from(dfvk.default_address().1),
             amount: NonNegativeAmount::const_from_u64(41000),
             memo: None,
             label: None,

zcash_keys/CHANGELOG.md

@@ -24,7 +24,8 @@ The entries below are relative to the `zcash_client_backend` crate as of
 
 ### Changed
 - `zcash_keys::address`:
-  - `RecipientAddress` has been renamed to `Address`
+  - `RecipientAddress` has been renamed to `Address`. Also, each variant now
+    `Box`es its contents to avoid large discrepancies in enum variant sizing.
   - `Address::Shielded` has been renamed to `Address::Sapling`
   - `UnifiedAddress::from_receivers` has been renamed to `UnifiedAddress::new`.
     It no longer takes an Orchard receiver argument unless the `orchard` feature

zcash_keys/src/address.rs

@@ -234,10 +234,7 @@ impl UnifiedAddress {
                     self.expiry_height
                         .map(|h| unified::MetadataItem::ExpiryHeight(u32::from(h))),
                 )
-                .chain(
-                    self.expiry_time
-                        .map(|t| unified::MetadataItem::ExpiryTime(t)),
-                )
+                .chain(self.expiry_time.map(unified::MetadataItem::ExpiryTime))
                 .map(Item::Metadata);
 
             data_items.chain(meta_items).collect()
@@ -250,26 +247,26 @@ impl UnifiedAddress {
 /// An address that funds can be sent to.
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum Address {
-    Sapling(PaymentAddress),
-    Transparent(TransparentAddress),
-    Unified(UnifiedAddress),
+    Sapling(Box<PaymentAddress>),
+    Transparent(Box<TransparentAddress>),
+    Unified(Box<UnifiedAddress>),
 }
 
 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))
     }
 }
 
@@ -312,11 +309,11 @@ impl Address {
 
         match self {
             Address::Sapling(pa) => ZcashAddress::from_sapling(net, pa.to_bytes()),
-            Address::Transparent(addr) => match addr {
+            Address::Transparent(addr) => match **addr {
                 TransparentAddress::PublicKey(data) => {
-                    ZcashAddress::from_transparent_p2pkh(net, *data)
+                    ZcashAddress::from_transparent_p2pkh(net, data)
                 }
-                TransparentAddress::Script(data) => ZcashAddress::from_transparent_p2sh(net, *data),
+                TransparentAddress::Script(data) => ZcashAddress::from_transparent_p2sh(net, data),
             },
             Address::Unified(ua) => ua.to_address(net),
         }
@@ -355,7 +352,7 @@ mod tests {
         #[cfg(not(feature = "orchard"))]
         let ua = UnifiedAddress::new(sapling, transparent, None, None).unwrap();
 
-        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));
     }

zcash_keys/src/keys.rs

@@ -634,10 +634,7 @@ impl UnifiedFullViewingKey {
                 self.expiry_height
                     .map(|h| unified::MetadataItem::ExpiryHeight(u32::from(h))),
             )
-            .chain(
-                self.expiry_time
-                    .map(|t| unified::MetadataItem::ExpiryTime(t)),
-            );
+            .chain(self.expiry_time.map(unified::MetadataItem::ExpiryTime));
 
         let ufvk = unified::Ufvk::try_from_items(
             data_items