zcash_client_backend: Update address rotation to take gap limits into…

… account.

Cargo.toml

@@ -94,6 +94,7 @@ document-features = "0.2"
 # Encodings
 base64 = "0.22"
 bech32 = { version = "0.11", default-features = false, features = ["alloc"] }
+bitflags = "2"
 bs58 = { version = "0.5", default-features = false, features = ["alloc", "check"] }
 byteorder = "1"
 hex = { version = "0.4", default-features = false, features = ["alloc"] }

zcash_client_backend/CHANGELOG.md

@@ -14,15 +14,19 @@ and this library adheres to Rust's notion of
 
 ### Changed
 - `zcash_client_backend::data_api::WalletRead`:
-    - `get_transparent_receivers` now takes additional `include_change` and
-      `include_ephemeral` arguments.
-    - `get_known_ephemeral_addresses` now takes a
-      `Range<zcash_transparent::keys::NonHardenedChildIndex>` as its argument
-      instead of a `Range<u32>`
-    - Has added method `utxo_query_height` when the `transparent-inputs` feature
-      flag is active.
-- `zcash_client_backend::data_api::WalletWrite` has an added method
-  `get_address_for_index`
+  - `get_transparent_receivers` now takes additional `include_change` and
+    `include_ephemeral` arguments.
+  - `get_known_ephemeral_addresses` now takes a
+    `Range<zcash_transparent::keys::NonHardenedChildIndex>` as its argument
+    instead of a `Range<u32>`
+  - Has added method `utxo_query_height` when the `transparent-inputs` feature
+    flag is active.
+  - has removed method `get_current_address`. It has been replaced by
+    added method `WalletWrite::get_last_generated_address`
+- `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.
 
 ### Removed
 - `zcash_client_backend::data_api::GAP_LIMIT` gap limits are now configured

zcash_client_backend/src/data_api.rs

@@ -1225,14 +1225,19 @@ pub trait WalletRead {
         ufvk: &UnifiedFullViewingKey,
     ) -> Result<Option<Self::Account>, Self::Error>;
 
-    /// Returns the most recently generated unified address for the specified account, if the
-    /// account identifier specified refers to a valid account for this wallet.
+    /// 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
+    /// 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 the account identifier does not correspond to a known
-    /// account.
-    fn get_current_address(
+    /// account, or if no previously generated address conforms to the specified request.
+    fn get_last_generated_address(
         &self,
         account: Self::AccountId,
+        request: Option<UnifiedAddressRequest>,
     ) -> Result<Option<UnifiedAddress>, Self::Error>;
 
     /// Returns the birthday height for the given account, or an error if the account is not known
@@ -2380,7 +2385,7 @@ pub trait WalletWrite: WalletRead {
         &mut self,
         account: Self::AccountId,
         request: Option<UnifiedAddressRequest>,
-    ) -> Result<Option<UnifiedAddress>, Self::Error>;
+    ) -> 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
@@ -2392,7 +2397,9 @@ pub trait WalletWrite: WalletRead {
     /// [`ReceiverRequirement::Require`]'ed receiver.
     ///
     /// Address generation should fail if a transparent receiver would be generated that violates
-    /// the backend's internally configured gap limit for HD-seed-based recovery.
+    /// the backend's internally configured gap limit for HD-seed-based recovery, or if an address
+    /// has already been exposed for the given diversifier index and the given request produced an
+    /// address having different receivers than what was originally exposed.
     ///
     /// [`ReceiverRequirement::Require`]: zcash_keys::keys::ReceiverRequirement::Require
     fn get_address_for_index(

zcash_client_backend/src/data_api/testing.rs

@@ -2523,9 +2523,10 @@ impl WalletRead for MockWalletDb {
         Ok(None)
     }
 
-    fn get_current_address(
+    fn get_last_generated_address(
         &self,
         _account: Self::AccountId,
+        _request: Option<UnifiedAddressRequest>,
     ) -> Result<Option<UnifiedAddress>, Self::Error> {
         Ok(None)
     }
@@ -2709,7 +2710,7 @@ impl WalletWrite for MockWalletDb {
         &mut self,
         _account: Self::AccountId,
         _request: Option<UnifiedAddressRequest>,
-    ) -> Result<Option<UnifiedAddress>, Self::Error> {
+    ) -> Result<Option<(UnifiedAddress, DiversifierIndex)>, Self::Error> {
         Ok(None)
     }
 

zcash_client_backend/src/data_api/testing/pool.rs

@@ -1766,7 +1766,7 @@ where
 
     let uaddr = st
         .wallet()
-        .get_current_address(account.id())
+        .get_last_generated_address(account.id(), None)
         .unwrap()
         .unwrap();
     let taddr = uaddr.transparent().unwrap();

zcash_client_backend/src/data_api/testing/transparent.rs

@@ -102,7 +102,7 @@ where
     let account_id = st.test_account().unwrap().id();
     let uaddr = st
         .wallet()
-        .get_current_address(account_id)
+        .get_last_generated_address(account_id, None)
         .unwrap()
         .unwrap();
     let taddr = uaddr.transparent().unwrap();
@@ -193,7 +193,7 @@ where
     let account = st.test_account().cloned().unwrap();
     let uaddr = st
         .wallet()
-        .get_current_address(account.id())
+        .get_last_generated_address(account.id(), None)
         .unwrap()
         .unwrap();
     let taddr = uaddr.transparent().unwrap();
@@ -301,7 +301,7 @@ where
     let account = st.test_account().cloned().unwrap();
     let uaddr = st
         .wallet()
-        .get_current_address(account.id())
+        .get_last_generated_address(account.id(), None)
         .unwrap()
         .unwrap();
     let taddr = uaddr.transparent().unwrap();

zcash_client_sqlite/CHANGELOG.md

@@ -27,6 +27,7 @@ and this library adheres to Rust's notion of
     for the account that reached the limit in its payload. In addition
     to the transparent address index, it also contains the key scope
     involved when the error was encountered.
+  - A new `DiversifierIndexReuse` variant has been added.
 - Each row returned from the `v_received_outputs` view now exposes an
   internal identifier for the address that received that output. This should
   be ignored by external consumers of this view.

zcash_client_sqlite/Cargo.toml

@@ -45,6 +45,7 @@ bs58.workspace = true
 tracing.workspace = true
 
 # - Serialization
+bitflags.workspace = true
 byteorder.workspace = true
 nonempty.workspace = true
 prost.workspace = true

zcash_client_sqlite/src/error.rs

@@ -8,8 +8,10 @@ use shardtree::error::ShardTreeError;
 
 use zcash_address::ParseError;
 use zcash_client_backend::data_api::NoteFilter;
+use zcash_keys::address::UnifiedAddress;
 use zcash_keys::keys::AddressGenerationError;
 use zcash_protocol::{consensus::BlockHeight, value::BalanceError, PoolType, TxId};
+use zip32::DiversifierIndex;
 
 use crate::{wallet::commitment_tree, AccountUuid};
 
@@ -126,6 +128,11 @@ pub enum SqliteClientError {
     #[cfg(feature = "transparent-inputs")]
     ReachedGapLimit(KeyScope, u32),
 
+    /// The backend encountered an attempt to reuse a diversifier index to generate an address
+    /// having different receivers from an address that had previously been exposed for that
+    /// diversifier index. Returns the previously exposed address.
+    DiversifierIndexReuse(DiversifierIndex, Box<UnifiedAddress>),
+
     /// The wallet attempted to create a transaction that would use of one of the wallet's
     /// previously-used addresses, potentially creating a problem with on-chain transaction
     /// linkability. The returned value contains the string encoding of the address and the txid(s)
@@ -196,6 +203,13 @@ impl fmt::Display for SqliteClientError {
                      KeyScope::Ephemeral => "ephemeral transparent",
                  }
             ),
+            SqliteClientError::DiversifierIndexReuse(i, _) => {
+                write!(
+                    f,
+                    "An address has already been exposed for diversifier index {}",
+                    u128::from(*i)
+                )
+            }
             SqliteClientError::AddressReuse(address_str, txids) => {
                 write!(f, "The address {address_str} previously used in txid(s) {:?} would be reused.", txids)
             }

zcash_client_sqlite/src/lib.rs

@@ -67,10 +67,7 @@ use zcash_client_backend::{
 };
 use zcash_keys::{
     address::UnifiedAddress,
-    keys::{
-        AddressGenerationError, ReceiverRequirement, UnifiedAddressRequest, UnifiedFullViewingKey,
-        UnifiedSpendingKey,
-    },
+    keys::{ReceiverRequirement, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
 };
 use zcash_primitives::{
     block::BlockHash,
@@ -86,7 +83,6 @@ use zip32::{fingerprint::SeedFingerprint, DiversifierIndex};
 
 use crate::{error::SqliteClientError, wallet::commitment_tree::SqliteShardStore};
 use wallet::{
-    chain_tip_height,
     commitment_tree::{self, put_shard_roots},
     common::spendable_notes_meta,
     upsert_address, SubtreeProgressEstimator,
@@ -670,11 +666,12 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters, CL> WalletRead
         wallet::get_account_for_ufvk(self.conn.borrow(), &self.params, ufvk)
     }
 
-    fn get_current_address(
+    fn get_last_generated_address(
         &self,
         account: Self::AccountId,
+        request: Option<UnifiedAddressRequest>,
     ) -> Result<Option<UnifiedAddress>, Self::Error> {
-        wallet::get_current_address(self.conn.borrow(), &self.params, account)
+        wallet::get_last_generated_address(self.conn.borrow(), &self.params, account, request)
             .map(|res| res.map(|(addr, _)| addr))
     }
 
@@ -1129,39 +1126,18 @@ impl<C: BorrowMut<rusqlite::Connection>, P: consensus::Parameters, CL: Clock> Wa
         &mut self,
         account_uuid: Self::AccountId,
         request: Option<UnifiedAddressRequest>,
-    ) -> Result<Option<UnifiedAddress>, Self::Error> {
-        self.transactionally(
-            |wdb| match wdb.get_unified_full_viewing_keys()?.get(&account_uuid) {
-                Some(ufvk) => {
-                    let search_from =
-                        match wallet::get_current_address(wdb.conn.0, &wdb.params, account_uuid)? {
-                            Some((_, mut last_diversifier_index)) => {
-                                last_diversifier_index.increment().map_err(|_| {
-                                    AddressGenerationError::DiversifierSpaceExhausted
-                                })?;
-                                last_diversifier_index
-                            }
-                            None => DiversifierIndex::default(),
-                        };
-
-                    let (addr, diversifier_index) = ufvk.find_address(search_from, request)?;
-                    let account_id = wallet::get_account_ref(wdb.conn.0, account_uuid)?;
-                    let chain_tip_height = chain_tip_height(wdb.conn.0)?
-                        .ok_or(SqliteClientError::ChainHeightUnknown)?;
-                    wallet::upsert_address(
-                        wdb.conn.0,
-                        &wdb.params,
-                        account_id,
-                        diversifier_index,
-                        &addr,
-                        Some(chain_tip_height),
-                    )?;
-
-                    Ok(Some(addr))
-                }
-                None => Ok(None),
-            },
-        )
+    ) -> Result<Option<(UnifiedAddress, DiversifierIndex)>, Self::Error> {
+        self.transactionally(|wdb| {
+            wallet::get_next_available_address(
+                wdb.conn.0,
+                &wdb.params,
+                &wdb.clock, // TODO: make an abstract Clock a field of `WalletDb`
+                account_uuid,
+                request,
+                #[cfg(feature = "transparent-inputs")]
+                &wdb.gap_limits,
+            )
+        })
     }
 
     fn get_address_for_index(
@@ -1180,6 +1156,7 @@ impl<C: BorrowMut<rusqlite::Connection>, P: consensus::Parameters, CL: Clock> Wa
                     diversifier_index,
                     &address,
                     Some(chain_tip_height.unwrap_or(account.birthday())),
+                    true,
                 )?;
 
                 Ok(Some(address))
@@ -2283,17 +2260,24 @@ mod tests {
             .update_chain_tip(account.birthday().height())
             .unwrap();
 
-        let current_addr = st.wallet().get_current_address(account.id()).unwrap();
+        let current_addr = st
+            .wallet()
+            .get_last_generated_address(account.id(), None)
+            .unwrap();
         assert!(current_addr.is_some());
 
         let addr2 = st
             .wallet_mut()
             .get_next_available_address(account.id(), None)
-            .unwrap();
+            .unwrap()
+            .map(|(a, _)| a);
         assert!(addr2.is_some());
         assert_ne!(current_addr, addr2);
 
-        let addr2_cur = st.wallet().get_current_address(account.id()).unwrap();
+        let addr2_cur = st
+            .wallet()
+            .get_last_generated_address(account.id(), None)
+            .unwrap();
         assert_eq!(addr2, addr2_cur);
     }
 

zcash_client_sqlite/src/util.rs

@@ -51,7 +51,7 @@ pub mod testing {
         /// that instant.
         pub fn tick(&self, delta: Duration) {
             let mut w = self.now.write().unwrap();
-            *w = *w + delta;
+            *w += delta;
         }
     }