diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 8962bb0ee9..5ba5f1052d 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/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`: diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 156f6ad6fb..afe095ecff 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -703,8 +703,12 @@ 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() { @@ -712,7 +716,7 @@ where } 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, diff --git a/zcash_client_backend/src/data_api/wallet/input_selection.rs b/zcash_client_backend/src/data_api/wallet/input_selection.rs index f33efefc0c..4eb8e2c56d 100644 --- a/zcash_client_backend/src/data_api/wallet/input_selection.rs +++ b/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()), )); } } diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 8cfb3a48d9..8059cd1700 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/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), + Sapling(Box), + Unified(Box, PoolType), InternalAccount(AccountId, PoolType), } diff --git a/zcash_client_backend/src/zip321.rs b/zcash_client_backend/src/zip321.rs index f7a8ccc014..4d80b6eb7f 100644 --- a/zcash_client_backend/src/zip321.rs +++ b/zcash_client_backend/src/zip321.rs @@ -773,9 +773,9 @@ pub mod testing { pub fn arb_addr() -> impl Strategy { 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, diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 52a0a7df57..fcaece6092 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/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, diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index b964efa8f3..ebd73eecd2 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -594,7 +594,7 @@ impl WalletWrite for WalletDb 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 WalletWrite for WalletDb *account_id, tx_ref, output_index, - &Recipient::Transparent(address), + &Recipient::Transparent(Box::new(address)), txout.value, None )?; diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 6972ee3544..9191f5b6cb 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -294,7 +294,7 @@ pub(crate) fn get_current_address( 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, diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index e3f470ce38..8cbbd9b3c6 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/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')", diff --git a/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs b/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs index 58b3490048..4d356fcd50 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs @@ -78,7 +78,7 @@ impl RusqliteMigration for Migration

{ )) })?; 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 RusqliteMigration for Migration

{ 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 RusqliteMigration for Migration

{ 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(), diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs index 30c84bf6c1..62e73830ea 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs @@ -92,11 +92,11 @@ impl RusqliteMigration for Migration

{ "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 RusqliteMigration for Migration

{ } 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))); } } diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 4e4c96f719..b886f02d8e 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/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, diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index a0aadde67d..df8cde22d7 100644 --- a/zcash_keys/CHANGELOG.md +++ b/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 diff --git a/zcash_keys/src/address.rs b/zcash_keys/src/address.rs index d9c1c8066e..a7d12a8c94 100644 --- a/zcash_keys/src/address.rs +++ b/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), + Transparent(Box), + Unified(Box), } impl From for Address { fn from(addr: PaymentAddress) -> Self { - Address::Sapling(addr) + Address::Sapling(Box::new(addr)) } } impl From for Address { fn from(addr: TransparentAddress) -> Self { - Address::Transparent(addr) + Address::Transparent(Box::new(addr)) } } impl From 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)); } diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 9551e7e005..e7093654f2 100644 --- a/zcash_keys/src/keys.rs +++ b/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