zcash_keys: Replace the UnifiedAddressRequest struct with an enum.

This change clarifies APIs that take `UnifiedAddressRequest` values by
making it possible to explicitly request that addresses be created using
all available key items. Previously, this was indicated by providing a
`None` value for an `Option<UnifiedAddressRequest>`, but in some
circumstances the meaning of this `None` could potentially be unclear.
Replacing `UnifiedAddressRequest` with an enum that can express the
caller's intent directly simplifies and clarifies a fair amount of code.

zcash_client_backend/CHANGELOG.md

@@ -26,7 +26,14 @@ and this library adheres to Rust's notion of
 - `zcash_client_backend::data_api::WalletWrite`:
   - has added method `get_address_for_index`
   - `get_next_available_address` now returns the diversifier index at which the
-    address was generated in addition to the address.
+    address was generated in addition to the address. In addition, the
+    `UnifiedAddressRequest` argument is now non-optional; use
+    `UnifiedAddressRequest::AllAvailableKeys` to indicate that all available
+    keys should be used to generate receivers instead of `None`.
+  - Arguments to `get_address_for_index` have changed: the
+    `UnifiedAddressRequest` argument is now non-optional; use
+    `UnifiedAddressRequest::AllAvailableKeys` to indicate that all available
+    keys should be used to generate receivers instead of `None`.
 
 ### Removed
 - `zcash_client_backend::data_api::GAP_LIMIT` gap limits are now configured

zcash_client_backend/src/data_api.rs

@@ -1226,18 +1226,15 @@ pub trait WalletRead {
     ) -> Result<Option<Self::Account>, Self::Error>;
 
     /// Returns the most recently generated unified address for the specified account that conforms
-    /// to the specified address request, if the account identifier specified refers to a valid
+    /// to the specified address filter, if the account identifier specified refers to a valid
     /// account for this wallet.
     ///
-    /// If the `request` parameter is `None`, this will be interpreted as a request for an address
-    /// having a receiver corresponding to each data item in the account's UIVK.
-    ///
     /// This will return `Ok(None)` if no previously generated address conforms to the specified
     /// request.
     fn get_last_generated_address(
         &self,
         account: Self::AccountId,
-        request: Option<UnifiedAddressRequest>,
+        address_filter: UnifiedAddressRequest,
     ) -> Result<Option<UnifiedAddress>, Self::Error>;
 
     /// Returns the birthday height for the given account, or an error if the account is not known
@@ -2375,21 +2372,18 @@ pub trait WalletWrite: WalletRead {
     ) -> Result<Self::Account, Self::Error>;
 
     /// Generates, persists, and marks as exposed 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.
+    /// specified account, given the current addresses known to the wallet.
     ///
     /// 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: Option<UnifiedAddressRequest>,
+        request: UnifiedAddressRequest,
     ) -> Result<Option<(UnifiedAddress, DiversifierIndex)>, Self::Error>;
 
     /// Generates, persists, and marks as exposed a diversified address for the specified account
-    /// at the provided diversifier index. If the `request` parameter is `None`, an address should
-    /// be generated using all of the available receivers for the account's UFVK.
+    /// at the provided diversifier index.
     ///
     /// Returns `Ok(None)` in the case that it is not possible to generate an address conforming
     /// to the provided request at the specified diversifier index. Such a result might arise from
@@ -2406,7 +2400,7 @@ pub trait WalletWrite: WalletRead {
         &mut self,
         account: Self::AccountId,
         diversifier_index: DiversifierIndex,
-        request: Option<UnifiedAddressRequest>,
+        request: UnifiedAddressRequest,
     ) -> Result<Option<UnifiedAddress>, Self::Error>;
 
     /// Updates the wallet's view of the blockchain.

zcash_client_backend/src/data_api/testing.rs

@@ -2526,7 +2526,7 @@ impl WalletRead for MockWalletDb {
     fn get_last_generated_address(
         &self,
         _account: Self::AccountId,
-        _request: Option<UnifiedAddressRequest>,
+        _request: UnifiedAddressRequest,
     ) -> Result<Option<UnifiedAddress>, Self::Error> {
         Ok(None)
     }
@@ -2709,7 +2709,7 @@ impl WalletWrite for MockWalletDb {
     fn get_next_available_address(
         &mut self,
         _account: Self::AccountId,
-        _request: Option<UnifiedAddressRequest>,
+        _request: UnifiedAddressRequest,
     ) -> Result<Option<(UnifiedAddress, DiversifierIndex)>, Self::Error> {
         Ok(None)
     }
@@ -2718,7 +2718,7 @@ impl WalletWrite for MockWalletDb {
         &mut self,
         _account: Self::AccountId,
         _diversifier_index: DiversifierIndex,
-        _request: Option<UnifiedAddressRequest>,
+        _request: UnifiedAddressRequest,
     ) -> Result<Option<UnifiedAddress>, Self::Error> {
         Ok(None)
     }

zcash_client_backend/src/data_api/testing/pool.rs

@@ -1755,6 +1755,8 @@ where
     DSF: DataStoreFactory,
     <<DSF as DataStoreFactory>::DataStore as WalletWrite>::UtxoRef: std::fmt::Debug,
 {
+    use zcash_keys::keys::UnifiedAddressRequest;
+
     let mut st = TestBuilder::new()
         .with_data_store_factory(ds_factory)
         .with_block_cache(cache)
@@ -1766,7 +1768,7 @@ where
 
     let uaddr = st
         .wallet()
-        .get_last_generated_address(account.id(), None)
+        .get_last_generated_address(account.id(), UnifiedAddressRequest::AllAvailableKeys)
         .unwrap()
         .unwrap();
     let taddr = uaddr.transparent().unwrap();

zcash_client_backend/src/data_api/testing/transparent.rs

@@ -18,7 +18,7 @@ use ::transparent::{
     bundle::{OutPoint, TxOut},
 };
 use sapling::zip32::ExtendedSpendingKey;
-use zcash_keys::address::Address;
+use zcash_keys::{address::Address, keys::UnifiedAddressRequest};
 use zcash_primitives::block::BlockHash;
 use zcash_protocol::{local_consensus::LocalNetwork, value::Zatoshis};
 
@@ -102,7 +102,7 @@ where
     let account_id = st.test_account().unwrap().id();
     let uaddr = st
         .wallet()
-        .get_last_generated_address(account_id, None)
+        .get_last_generated_address(account_id, UnifiedAddressRequest::AllAvailableKeys)
         .unwrap()
         .unwrap();
     let taddr = uaddr.transparent().unwrap();
@@ -193,7 +193,7 @@ where
     let account = st.test_account().cloned().unwrap();
     let uaddr = st
         .wallet()
-        .get_last_generated_address(account.id(), None)
+        .get_last_generated_address(account.id(), UnifiedAddressRequest::AllAvailableKeys)
         .unwrap()
         .unwrap();
     let taddr = uaddr.transparent().unwrap();
@@ -301,7 +301,7 @@ where
     let account = st.test_account().cloned().unwrap();
     let uaddr = st
         .wallet()
-        .get_last_generated_address(account.id(), None)
+        .get_last_generated_address(account.id(), UnifiedAddressRequest::AllAvailableKeys)
         .unwrap()
         .unwrap();
     let taddr = uaddr.transparent().unwrap();

zcash_client_sqlite/src/lib.rs

@@ -682,7 +682,7 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters, CL> WalletRead
     fn get_last_generated_address(
         &self,
         account: Self::AccountId,
-        request: Option<UnifiedAddressRequest>,
+        request: UnifiedAddressRequest,
     ) -> Result<Option<UnifiedAddress>, Self::Error> {
         wallet::get_last_generated_address(self.conn.borrow(), &self.params, account, request)
             .map(|res| res.map(|(addr, _)| addr))
@@ -1138,7 +1138,7 @@ impl<C: BorrowMut<rusqlite::Connection>, P: consensus::Parameters, CL: Clock> Wa
     fn get_next_available_address(
         &mut self,
         account_uuid: Self::AccountId,
-        request: Option<UnifiedAddressRequest>,
+        request: UnifiedAddressRequest,
     ) -> Result<Option<(UnifiedAddress, DiversifierIndex)>, Self::Error> {
         self.transactionally(|wdb| {
             wallet::get_next_available_address(
@@ -1157,7 +1157,7 @@ impl<C: BorrowMut<rusqlite::Connection>, P: consensus::Parameters, CL: Clock> Wa
         &mut self,
         account: Self::AccountId,
         diversifier_index: DiversifierIndex,
-        request: Option<UnifiedAddressRequest>,
+        request: UnifiedAddressRequest,
     ) -> Result<Option<UnifiedAddress>, Self::Error> {
         if let Some(account) = self.get_account(account)? {
             if let Ok(address) = account.uivk().address(diversifier_index, request) {
@@ -1391,13 +1391,14 @@ impl<C: BorrowMut<rusqlite::Connection>, P: consensus::Parameters, CL: Clock> Wa
 
             #[cfg(feature = "transparent-inputs")]
             for (account_id, key_scope) in wallet::involved_accounts(wdb.conn.0, tx_refs)? {
+                use ReceiverRequirement::*;
                 wallet::transparent::generate_gap_addresses(
                     wdb.conn.0,
                     &wdb.params,
                     account_id,
                     key_scope,
                     &wdb.gap_limits,
-                    None,
+                    UnifiedAddressRequest::unsafe_custom(Allow, Allow, Require),
                 )?;
             }
 
@@ -1663,13 +1664,14 @@ impl<C: BorrowMut<rusqlite::Connection>, P: consensus::Parameters, CL: Clock> Wa
                     _output,
                 )?;
 
+            use ReceiverRequirement::*;
             wallet::transparent::generate_gap_addresses(
                 wdb.conn.0,
                 &wdb.params,
                 account_id,
                 key_scope,
                 &wdb.gap_limits,
-                None,
+                UnifiedAddressRequest::unsafe_custom(Allow, Allow, Require),
             )?;
 
             Ok(utxo_id)
@@ -2280,35 +2282,35 @@ mod tests {
 
         let current_addr = st
             .wallet()
-            .get_last_generated_address(account.id(), None)
+            .get_last_generated_address(account.id(), UnifiedAddressRequest::AllAvailableKeys)
             .unwrap();
         assert!(current_addr.is_some());
 
         let addr2 = st
             .wallet_mut()
-            .get_next_available_address(account.id(), None)
+            .get_next_available_address(account.id(), UnifiedAddressRequest::AllAvailableKeys)
             .unwrap()
             .map(|(a, _)| a);
         assert!(addr2.is_some());
         assert_ne!(current_addr, addr2);
 
         let addr2_cur = st
             .wallet()
-            .get_last_generated_address(account.id(), None)
+            .get_last_generated_address(account.id(), UnifiedAddressRequest::AllAvailableKeys)
             .unwrap();
         assert_eq!(addr2, addr2_cur);
 
         // Perform similar tests for shielded-only addresses. These should be timestamp-based; we
         // will tick the clock between each generation.
         use zcash_keys::keys::ReceiverRequirement::*;
         #[cfg(feature = "orchard")]
-        let shielded_only_request = UnifiedAddressRequest::unsafe_new(Require, Require, Omit);
+        let shielded_only_request = UnifiedAddressRequest::unsafe_custom(Require, Require, Omit);
         #[cfg(not(feature = "orchard"))]
-        let shielded_only_request = UnifiedAddressRequest::unsafe_new(Omit, Require, Omit);
+        let shielded_only_request = UnifiedAddressRequest::unsafe_custom(Omit, Require, Omit);
 
         let cur_shielded_only = st
             .wallet()
-            .get_last_generated_address(account.id(), Some(shielded_only_request))
+            .get_last_generated_address(account.id(), shielded_only_request)
             .unwrap();
         assert!(cur_shielded_only.is_none());
 
@@ -2324,7 +2326,7 @@ mod tests {
 
         let (shielded_only, di) = st
             .wallet_mut()
-            .get_next_available_address(account.id(), Some(shielded_only_request))
+            .get_next_available_address(account.id(), shielded_only_request)
             .unwrap()
             .expect("generated a shielded-only address");
 
@@ -2334,7 +2336,7 @@ mod tests {
 
         let cur_shielded_only = st
             .wallet()
-            .get_last_generated_address(account.id(), Some(shielded_only_request))
+            .get_last_generated_address(account.id(), shielded_only_request)
             .unwrap()
             .expect("retrieved the last-generated shielded-only address");
         assert_eq!(cur_shielded_only, shielded_only);
@@ -2350,7 +2352,7 @@ mod tests {
 
         let (shielded_only_2, di_2) = st
             .wallet_mut()
-            .get_next_available_address(account.id(), Some(shielded_only_request))
+            .get_next_available_address(account.id(), shielded_only_request)
             .unwrap()
             .expect("generated a shielded-only address");
         assert_ne!(shielded_only_2, shielded_only);
@@ -2602,7 +2604,7 @@ mod tests {
 
         // The receiver for the default UA should be in the set.
         assert!(receivers.contains_key(
-            ufvk.default_address(None)
+            ufvk.default_address(UnifiedAddressRequest::AllAvailableKeys)
                 .expect("A valid default address exists for the UFVK")
                 .0
                 .transparent()