diff --git a/devtools/src/bin/inspect/keys.rs b/devtools/src/bin/inspect/keys.rs index d735cfadd3..a45486cab0 100644 --- a/devtools/src/bin/inspect/keys.rs +++ b/devtools/src/bin/inspect/keys.rs @@ -7,7 +7,7 @@ use zcash_address::{ unified::{self, Encoding}, ToAddress, ZcashAddress, }; -use zcash_keys::keys::{UnifiedAddressRequest, UnifiedFullViewingKey}; +use zcash_keys::keys::UnifiedFullViewingKey; use zcash_primitives::{ legacy::{ keys::{AccountPrivKey, IncomingViewingKey}, @@ -220,9 +220,7 @@ pub(crate) fn inspect_sapling_extsk( }; eprintln!("- UFVK: {encoded_ufvk}"); - let (default_ua, _) = ufvk - .default_address(UnifiedAddressRequest::unsafe_new(false, true, false)) - .expect("should exist"); + let (default_ua, _) = ufvk.default_address(None).expect("should exist"); let encoded_ua = match network { NetworkType::Main => default_ua.encode(&Network::MainNetwork), NetworkType::Test => default_ua.encode(&Network::TestNetwork), diff --git a/devtools/src/bin/inspect/keys/view.rs b/devtools/src/bin/inspect/keys/view.rs index 7824b1d137..037aff8dde 100644 --- a/devtools/src/bin/inspect/keys/view.rs +++ b/devtools/src/bin/inspect/keys/view.rs @@ -1,6 +1,6 @@ use bech32::{FromBase32, ToBase32}; use zcash_address::unified::{self, Container, Encoding}; -use zcash_keys::keys::{UnifiedAddressRequest, UnifiedFullViewingKey}; +use zcash_keys::keys::UnifiedFullViewingKey; use zcash_protocol::{ consensus::{Network, NetworkConstants, NetworkType}, local_consensus::LocalNetwork, @@ -138,9 +138,7 @@ pub(crate) fn inspect_sapling_extfvk( }; eprintln!("- Equivalent UFVK: {encoded_ufvk}"); - let (default_ua, _) = ufvk - .default_address(UnifiedAddressRequest::unsafe_new(false, true, false)) - .expect("should exist"); + let (default_ua, _) = ufvk.default_address(None).expect("should exist"); let encoded_ua = match network { NetworkType::Main => default_ua.encode(&Network::MainNetwork), NetworkType::Test => default_ua.encode(&Network::TestNetwork), diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 7090c15ca4..020efc208d 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -26,6 +26,7 @@ and this library adheres to Rust's notion of additional `key_source` field that is used to convey application-specific key source metadata. - The `Copy` impl for this type has been removed. + - The `request` argument to `WalletRead::get_next_available_address` is now optional. - `zcash_client_backend::data_api::Account` has an additional `name` method that returns the human-readable name of the account, if any. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 81bda75e0f..715624c19e 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -2375,15 +2375,16 @@ pub trait WalletWrite: WalletRead { key_source: Option<&str>, ) -> Result; - /// Generates and persists the next available diversified address, given the current - /// addresses known to the wallet. + /// Generates and persists the next available diversified address for the specified account, + /// given the current addresses known to the wallet. If the `request` parameter is `None`, + /// an address should be generated using all of the available receivers for the account's UFVK. /// /// Returns `Ok(None)` if the account identifier does not correspond to a known /// account. fn get_next_available_address( &mut self, account: Self::AccountId, - request: UnifiedAddressRequest, + request: Option, ) -> Result, Self::Error>; /// Updates the wallet's view of the blockchain. diff --git a/zcash_client_backend/src/data_api/testing.rs b/zcash_client_backend/src/data_api/testing.rs index 4e37ae9687..d80a9d4fb6 100644 --- a/zcash_client_backend/src/data_api/testing.rs +++ b/zcash_client_backend/src/data_api/testing.rs @@ -2638,7 +2638,7 @@ impl WalletWrite for MockWalletDb { fn get_next_available_address( &mut self, _account: Self::AccountId, - _request: UnifiedAddressRequest, + _request: Option, ) -> Result, Self::Error> { Ok(None) } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 9fe3fe45eb..1e27c54986 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -159,9 +159,6 @@ pub(crate) const UA_TRANSPARENT: bool = false; #[cfg(feature = "transparent-inputs")] pub(crate) const UA_TRANSPARENT: bool = true; -pub(crate) const DEFAULT_UA_REQUEST: UnifiedAddressRequest = - UnifiedAddressRequest::unsafe_new(UA_ORCHARD, true, UA_TRANSPARENT); - /// Unique identifier for a specific account tracked by a [`WalletDb`]. /// /// Account identifiers are "one-way stable": a given identifier always points to a @@ -937,7 +934,7 @@ impl WalletWrite for WalletDb fn get_next_available_address( &mut self, account_uuid: Self::AccountId, - request: UnifiedAddressRequest, + request: Option, ) -> Result, Self::Error> { self.transactionally( |wdb| match wdb.get_unified_full_viewing_keys()?.get(&account_uuid) { @@ -1940,9 +1937,7 @@ mod tests { use zcash_primitives::block::BlockHash; use zcash_protocol::consensus; - use crate::{ - error::SqliteClientError, testing::db::TestDbFactory, AccountUuid, DEFAULT_UA_REQUEST, - }; + use crate::{error::SqliteClientError, testing::db::TestDbFactory, AccountUuid}; #[cfg(feature = "unstable")] use { @@ -1993,7 +1988,7 @@ mod tests { let addr2 = st .wallet_mut() - .get_next_available_address(account.id(), DEFAULT_UA_REQUEST) + .get_next_available_address(account.id(), None) .unwrap(); assert!(addr2.is_some()); assert_ne!(current_addr, addr2); @@ -2244,7 +2239,7 @@ mod tests { // The receiver for the default UA should be in the set. assert!(receivers.contains_key( - ufvk.default_address(DEFAULT_UA_REQUEST) + ufvk.default_address(None) .expect("A valid default address exists for the UFVK") .0 .transparent() diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index e2678e139e..650964a2fe 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -119,8 +119,8 @@ use zip32::{self, DiversifierIndex, Scope}; use crate::{ error::SqliteClientError, wallet::commitment_tree::{get_max_checkpointed_height, SqliteShardStore}, - AccountRef, SqlTransaction, TransferType, WalletCommitmentTrees, WalletDb, DEFAULT_UA_REQUEST, - PRUNING_DEPTH, SAPLING_TABLES_PREFIX, + AccountRef, SqlTransaction, TransferType, WalletCommitmentTrees, WalletDb, PRUNING_DEPTH, + SAPLING_TABLES_PREFIX, }; use crate::{AccountUuid, TxRef, VERIFY_LOOKAHEAD}; @@ -220,14 +220,14 @@ pub struct Account { } impl Account { - /// Returns the default Unified Address for the account, - /// along with the diversifier index that generated it. + /// Returns the default Unified Address for the account, along with the diversifier index that + /// generated it. /// /// The diversifier index may be non-zero if the Unified Address includes a Sapling /// receiver, and there was no valid Sapling receiver at diversifier index zero. pub(crate) fn default_address( &self, - request: UnifiedAddressRequest, + request: Option, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { self.uivk().default_address(request) } @@ -287,24 +287,42 @@ pub(crate) fn seed_matches_derived_account( ) })? == seed_fingerprint; - // Keys are not comparable with `Eq`, but addresses are, so we derive what should - // be equivalent addresses for each key and use those to check for key equality. - let uivk_match = - match UnifiedSpendingKey::from_seed(params, &seed.expose_secret()[..], account_index) { - // If we can't derive a USK from the given seed with the account's ZIP 32 - // account index, then we immediately know the UIVK won't match because wallet - // accounts are required to have a known UIVK. - Err(_) => false, - Ok(usk) => UnifiedAddressRequest::all().map_or( - Ok::<_, SqliteClientError>(false), - |ua_request| { - Ok(usk - .to_unified_full_viewing_key() - .default_address(ua_request)? - == uivk.default_address(ua_request)?) - }, - )?, - }; + // `UnifiedIncomingViewingKey`s are not comparable with `Eq`, but Unified Address + // components are, so we derive corresponding addresses for each key and use + // those to check whether any components match. + let uivk_match = { + let usk = UnifiedSpendingKey::from_seed(params, &seed.expose_secret()[..], account_index) + .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?; + + let (seed_addr, _) = usk.to_unified_full_viewing_key().default_address(Some( + UnifiedAddressRequest::all().expect("At least one supported pool feature is enabled."), + ))?; + + let (uivk_addr, _) = uivk.default_address(None)?; + + #[cfg(not(feature = "orchard"))] + let orchard_match = false; + #[cfg(feature = "orchard")] + let orchard_match = seed_addr + .orchard() + .zip(uivk_addr.orchard()) + .map(|(a, b)| a == b) + == Some(true); + + let sapling_match = seed_addr + .sapling() + .zip(uivk_addr.sapling()) + .map(|(a, b)| a == b) + == Some(true); + + let p2pkh_match = seed_addr + .transparent() + .zip(uivk_addr.transparent()) + .map(|(a, b)| a == b) + == Some(true); + + orchard_match || sapling_match || p2pkh_match + }; if seed_fingerprint_match != uivk_match { // If these mismatch, it suggests database corruption. @@ -597,14 +615,7 @@ pub(crate) fn add_account( // Always derive the default Unified Address for the account. If the account's viewing // key has fewer components than the wallet supports (most likely due to this being an // imported viewing key), derive an address containing the common subset of receivers. - let ua_request = account - .uivk() - .to_address_request() - .and_then(|ua_request| ua_request.intersect(&DEFAULT_UA_REQUEST)) - .ok_or_else(|| { - SqliteClientError::AddressGeneration(AddressGenerationError::ShieldedReceiverRequired) - })?; - let (address, d_idx) = account.default_address(ua_request)?; + let (address, d_idx) = account.default_address(None)?; insert_address(conn, params, account_id, d_idx, &address)?; // Initialize the `ephemeral_addresses` table. diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 82a44d1a3e..3b1dacf8f9 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -991,7 +991,7 @@ mod tests { // Orchard component. let ua_request = UnifiedAddressRequest::unsafe_new(false, true, UA_TRANSPARENT); let address_str = Address::Unified( - ufvk.default_address(ua_request) + ufvk.default_address(Some(ua_request)) .expect("A valid default address exists for the UFVK") .0, ) @@ -1011,7 +1011,7 @@ mod tests { { let taddr = Address::Transparent( *ufvk - .default_address(ua_request) + .default_address(Some(ua_request)) .expect("A valid default address exists for the UFVK") .0 .transparent() @@ -1118,7 +1118,7 @@ mod tests { // hardcoded with knowledge of what's coming next let ua_request = UnifiedAddressRequest::unsafe_new(false, true, true); db_data - .get_next_available_address(account_id, ua_request) + .get_next_available_address(account_id, Some(ua_request)) .unwrap() .expect("get_next_available_address generated an address"); } else { diff --git a/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs b/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs index 242a285e32..99c8681575 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs @@ -443,11 +443,11 @@ mod tests { let usk = UnifiedSpendingKey::from_seed(&network, &[0u8; 32][..], AccountId::ZERO).unwrap(); let ufvk = usk.to_unified_full_viewing_key(); let (ua, _) = ufvk - .default_address(UnifiedAddressRequest::unsafe_new( + .default_address(Some(UnifiedAddressRequest::unsafe_new( false, true, UA_TRANSPARENT, - )) + ))) .expect("A valid default address exists for the UFVK"); let taddr = ufvk .transparent() 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 a6fc1483e7..25e0cf5c63 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs @@ -86,9 +86,9 @@ impl RusqliteMigration for Migration

{ "Address in accounts table was not a Unified Address.".to_string(), )); }; - let (expected_address, idx) = ufvk.default_address( + let (expected_address, idx) = ufvk.default_address(Some( UnifiedAddressRequest::unsafe_new(false, true, UA_TRANSPARENT), - )?; + ))?; if decoded_address != expected_address { return Err(WalletMigrationError::CorruptedData(format!( "Decoded UA {} does not match the UFVK's default address {} at {:?}.", @@ -158,10 +158,8 @@ impl RusqliteMigration for Migration

{ ], )?; - let (address, d_idx) = ufvk.default_address(UnifiedAddressRequest::unsafe_new( - false, - true, - UA_TRANSPARENT, + let (address, d_idx) = ufvk.default_address(Some( + UnifiedAddressRequest::unsafe_new(false, true, UA_TRANSPARENT), ))?; insert_address(transaction, &self.params, account, d_idx, &address)?; } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ensure_orchard_ua_receiver.rs b/zcash_client_sqlite/src/wallet/init/migrations/ensure_orchard_ua_receiver.rs index 8bca372acd..982f8c67e2 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ensure_orchard_ua_receiver.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ensure_orchard_ua_receiver.rs @@ -65,9 +65,9 @@ impl RusqliteMigration for Migration

{ })? }; - let (default_addr, diversifier_index) = uivk.default_address( + let (default_addr, diversifier_index) = uivk.default_address(Some( UnifiedAddressRequest::unsafe_new(UA_ORCHARD, true, UA_TRANSPARENT), - )?; + ))?; let mut di_be = *diversifier_index.as_bytes(); di_be.reverse(); @@ -140,11 +140,11 @@ mod tests { .unwrap(); let (addr, diversifier_index) = ufvk - .default_address(UnifiedAddressRequest::unsafe_new( + .default_address(Some(UnifiedAddressRequest::unsafe_new( false, true, UA_TRANSPARENT, - )) + ))) .unwrap(); let mut di_be = *diversifier_index.as_bytes(); di_be.reverse(); 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 17054ee55e..2881c9410c 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs @@ -135,7 +135,7 @@ impl RusqliteMigration for Migration

{ "Address field value decoded to a transparent address; should have been Sapling or unified.".to_string())); } Address::Unified(decoded_address) => { - let (expected_address, idx) = ufvk.default_address(ua_request)?; + let (expected_address, idx) = ufvk.default_address(Some(ua_request))?; if decoded_address != expected_address { return Err(if seed_is_relevant { WalletMigrationError::CorruptedData( @@ -154,7 +154,10 @@ impl RusqliteMigration for Migration

{ seed_is_relevant = true; let ufvk_str: String = ufvk.encode(&self.params); - let address_str: String = ufvk.default_address(ua_request)?.0.encode(&self.params); + let address_str: String = ufvk + .default_address(Some(ua_request))? + .0 + .encode(&self.params); // This migration, and the wallet behaviour before it, stored the default // transparent address in the `accounts` table. This does not necessarily diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 54a42f5849..42259e7b7d 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -6,6 +6,16 @@ and this library adheres to Rust's notion of ## [Unreleased] +### Changed +- The `UnifiedAddressRequest` argument to the following methods is now optional: + - `zcash_keys::keys::UnifiedSpendingKey::address` + - `zcash_keys::keys::UnifiedSpendingKey::default_address` + - `zcash_keys::keys::UnifiedFullViewingKey::find_address` + - `zcash_keys::keys::UnifiedFullViewingKey::default_address` + - `zcash_keys::keys::UnifiedIncomingViewingKey::address` + - `zcash_keys::keys::UnifiedIncomingViewingKey::find_address` + - `zcash_keys::keys::UnifiedIncomingViewingKey::default_address` + ## [0.5.0] - 2024-11-14 ### Changed diff --git a/zcash_keys/src/address.rs b/zcash_keys/src/address.rs index 665c03a34e..265280ee6a 100644 --- a/zcash_keys/src/address.rs +++ b/zcash_keys/src/address.rs @@ -447,7 +447,7 @@ pub mod testing { params: Network, request: UnifiedAddressRequest, ) -> impl Strategy { - arb_unified_spending_key(params).prop_map(move |k| k.default_address(request).0) + arb_unified_spending_key(params).prop_map(move |k| k.default_address(Some(request)).0) } #[cfg(feature = "sapling")] diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index fc78facfe7..698294a5fd 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -459,7 +459,7 @@ impl UnifiedSpendingKey { #[cfg(any(test, feature = "test-dependencies"))] pub fn default_address( &self, - request: UnifiedAddressRequest, + request: Option, ) -> (UnifiedAddress, DiversifierIndex) { self.to_unified_full_viewing_key() .default_address(request) @@ -890,42 +890,44 @@ impl UnifiedFullViewingKey { self.orchard.as_ref() } - /// Attempts to derive the Unified Address for the given diversifier index and - /// receiver types. + /// Attempts to derive the Unified Address for the given diversifier index and receiver types. + /// If `request` is None, the address should be derived to contain a receiver for each item in + /// this UFVK. /// /// Returns `None` if the specified index does not produce a valid diversifier. pub fn address( &self, j: DiversifierIndex, - request: UnifiedAddressRequest, + request: Option, ) -> Result { self.to_unified_incoming_viewing_key().address(j, request) } - /// Searches the diversifier space starting at diversifier index `j` for one which will - /// produce a valid diversifier, and return the Unified Address constructed using that - /// diversifier along with the index at which the valid diversifier was found. + /// Searches the diversifier space starting at diversifier index `j` for one which will produce + /// a valid diversifier, and return the Unified Address constructed using that diversifier + /// along with the index at which the valid diversifier was found. If `request` is None, the + /// address should be derived to contain a receiver for each item in this UFVK. /// /// Returns an `Err(AddressGenerationError)` if no valid diversifier exists or if the features /// required to satisfy the unified address request are not properly enabled. - #[allow(unused_mut)] pub fn find_address( &self, - mut j: DiversifierIndex, - request: UnifiedAddressRequest, + j: DiversifierIndex, + request: Option, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { self.to_unified_incoming_viewing_key() .find_address(j, request) } /// Find the Unified Address corresponding to the smallest valid diversifier index, along with - /// that index. + /// that index. If `request` is None, the address should be derived to contain a receiver for + /// each item in this UFVK. /// /// Returns an `Err(AddressGenerationError)` if no valid diversifier exists or if the features /// required to satisfy the unified address request are not properly enabled. pub fn default_address( &self, - request: UnifiedAddressRequest, + request: Option, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { self.find_address(DiversifierIndex::new(), request) } @@ -1113,15 +1115,19 @@ impl UnifiedIncomingViewingKey { &self.orchard } - /// Attempts to derive the Unified Address for the given diversifier index and - /// receiver types. + /// Attempts to derive the Unified Address for the given diversifier index and receiver types. + /// If `request` is None, the address should be derived to contain a receiver for each item in + /// this UFVK. /// /// Returns `None` if the specified index does not produce a valid diversifier. pub fn address( &self, _j: DiversifierIndex, - request: UnifiedAddressRequest, + request: Option, ) -> Result { + let request = request + .or(self.to_address_request()) + .ok_or(AddressGenerationError::ShieldedReceiverRequired)?; #[cfg(feature = "orchard")] let mut orchard = None; if request.has_orchard { @@ -1208,13 +1214,13 @@ impl UnifiedIncomingViewingKey { pub fn find_address( &self, mut j: DiversifierIndex, - request: UnifiedAddressRequest, + request: Option, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { // If we need to generate a transparent receiver, check that the user has not // specified an invalid transparent child index, from which we can never search to // find a valid index. #[cfg(feature = "transparent-inputs")] - if request.has_p2pkh + if request.iter().any(|r| r.has_p2pkh) && self.transparent.is_some() && to_transparent_child_index(j).is_none() { @@ -1242,13 +1248,14 @@ impl UnifiedIncomingViewingKey { } /// Find the Unified Address corresponding to the smallest valid diversifier index, along with - /// that index. + /// that index. If `request` is None, the address should be derived to contain a receiver for + /// each item in this UFVK. /// /// Returns an `Err(AddressGenerationError)` if no valid diversifier exists or if the features /// required to satisfy the unified address request are not properly enabled. pub fn default_address( &self, - request: UnifiedAddressRequest, + request: Option, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { self.find_address(DiversifierIndex::new(), request) } @@ -1506,7 +1513,10 @@ mod tests { } let ua = ufvk - .address(d_idx, UnifiedAddressRequest::unsafe_new(false, true, true)) + .address( + d_idx, + Some(UnifiedAddressRequest::unsafe_new(false, true, true)), + ) .unwrap_or_else(|err| { panic!( "unified address generation failed for account {}: {:?}", @@ -1687,7 +1697,10 @@ mod tests { } let ua = uivk - .address(d_idx, UnifiedAddressRequest::unsafe_new(false, true, true)) + .address( + d_idx, + Some(UnifiedAddressRequest::unsafe_new(false, true, true)), + ) .unwrap_or_else(|err| { panic!( "unified address generation failed for account {}: {:?}",