zcash_keys: Make address generation use all items of the source key b…

…y default.

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),

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),

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.
 

zcash_client_backend/src/data_api.rs

@@ -2375,15 +2375,16 @@ pub trait WalletWrite: WalletRead {
         key_source: Option<&str>,
     ) -> Result<Self::Account, Self::Error>;
 
-    /// 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<UnifiedAddressRequest>,
     ) -> Result<Option<UnifiedAddress>, Self::Error>;
 
     /// Updates the wallet's view of the blockchain.

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<UnifiedAddressRequest>,
     ) -> Result<Option<UnifiedAddress>, Self::Error> {
         Ok(None)
     }

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<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
     fn get_next_available_address(
         &mut self,
         account_uuid: Self::AccountId,
-        request: UnifiedAddressRequest,
+        request: Option<UnifiedAddressRequest>,
     ) -> Result<Option<UnifiedAddress>, 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()

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<UnifiedAddressRequest>,
     ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> {
         self.uivk().default_address(request)
     }
@@ -289,22 +289,39 @@ pub(crate) fn seed_matches_derived_account<P: consensus::Parameters>(
 
     // 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)?)
-                },
-            )?,
-        };
+    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 ie 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 +614,7 @@ pub(crate) fn add_account<P: consensus::Parameters>(
     // 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.

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 {

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()

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

@@ -86,9 +86,9 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                     "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<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                 ],
             )?;
 
-            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)?;
         }

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

@@ -65,9 +65,9 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                 })?
             };
 
-            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();

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

@@ -135,7 +135,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                             "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<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                 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

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

zcash_keys/src/address.rs

@@ -447,7 +447,7 @@ pub mod testing {
         params: Network,
         request: UnifiedAddressRequest,
     ) -> impl Strategy<Value = UnifiedAddress> {
-        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")]