Expose address generation errors when constructing default addresses

zcash_client_backend/src/zip321.rs

@@ -306,8 +306,8 @@ impl TransactionRequest {
     /// Parse the provided URI to a payment request value.
     pub fn from_uri<P: consensus::Parameters>(params: &P, uri: &str) -> Result<Self, Zip321Error> {
         // Parse the leading zcash:<address>
-        let (rest, primary_addr_param) =
-            parse::lead_addr(params)(uri).map_err(|e| Zip321Error::ParseError(e.to_string()))?;
+        let (rest, primary_addr_param) = parse::lead_addr(params)(uri)
+            .map_err(|e| Zip321Error::ParseError(format!("Error parsing lead address: {}", e)))?;
 
         // Parse the remaining parameters as an undifferentiated list
         let (_, xs) = if rest.is_empty() {
@@ -317,7 +317,9 @@ impl TransactionRequest {
                 char('?'),
                 separated_list0(char('&'), parse::zcashparam(params)),
             ))(rest)
-            .map_err(|e| Zip321Error::ParseError(e.to_string()))?
+            .map_err(|e| {
+                Zip321Error::ParseError(format!("Error parsing query parameters: {}", e))
+            })?
         };
 
         // Construct sets of payment parameters, keyed by the payment index.

zcash_client_sqlite/CHANGELOG.md

@@ -10,6 +10,12 @@ and this library adheres to Rust's notion of
 ### Added
 - A new `orchard` feature flag has been added to make it possible to
   build client code without `orchard` dependendencies.
+- `impl From<zcash_keys::keys::AddressGenerationError> for SqliteClientError`
+
+### Changed
+- `zcash_client_sqlite::error::SqliteClientError` has changed variants:
+  - Added `AddressGeneration`
+  - Removed `DiversifierIndexOutOfRange`
 
 ## [0.9.0] - 2024-03-01
 

zcash_client_sqlite/src/error.rs

@@ -8,6 +8,7 @@ use zcash_client_backend::{
     encoding::{Bech32DecodeError, TransparentCodecError},
     PoolType,
 };
+use zcash_keys::keys::AddressGenerationError;
 use zcash_primitives::{
     consensus::BlockHeight, transaction::components::amount::BalanceError, zip32::AccountId,
 };
@@ -68,8 +69,8 @@ pub enum SqliteClientError {
     /// this error is (safe rewind height, requested height).
     RequestedRewindInvalid(BlockHeight, BlockHeight),
 
-    /// The space of allocatable diversifier indices has been exhausted for the given account.
-    DiversifierIndexOutOfRange,
+    /// An error occurred in generating a Zcash address.
+    AddressGeneration(AddressGenerationError),
 
     /// The account for which information was requested does not belong to the wallet.
     AccountUnknown(AccountId),
@@ -119,6 +120,7 @@ impl error::Error for SqliteClientError {
             SqliteClientError::DbError(e) => Some(e),
             SqliteClientError::Io(e) => Some(e),
             SqliteClientError::BalanceError(e) => Some(e),
+            SqliteClientError::AddressGeneration(e) => Some(e),
             _ => None,
         }
     }
@@ -146,7 +148,7 @@ impl fmt::Display for SqliteClientError {
             SqliteClientError::InvalidMemo(e) => write!(f, "{}", e),
             SqliteClientError::BlockConflict(h) => write!(f, "A block hash conflict occurred at height {}; rewind required.", u32::from(*h)),
             SqliteClientError::NonSequentialBlocks => write!(f, "`put_blocks` requires that the provided block range be sequential"),
-            SqliteClientError::DiversifierIndexOutOfRange => write!(f, "The space of available diversifier indices is exhausted"),
+            SqliteClientError::AddressGeneration(e) => write!(f, "{}", e),
             SqliteClientError::AccountUnknown(acct_id) => write!(f, "Account {} does not belong to this wallet.", u32::from(*acct_id)),
 
             SqliteClientError::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {}", u32::from(*acct_id)),
@@ -217,3 +219,9 @@ impl From<BalanceError> for SqliteClientError {
         SqliteClientError::BalanceError(e)
     }
 }
+
+impl From<AddressGenerationError> for SqliteClientError {
+    fn from(e: AddressGenerationError) -> Self {
+        SqliteClientError::AddressGeneration(e)
+    }
+}

zcash_client_sqlite/src/lib.rs

@@ -66,7 +66,9 @@ use zcash_client_backend::{
         ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary,
         WalletWrite, SAPLING_SHARD_HEIGHT,
     },
-    keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
+    keys::{
+        AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey,
+    },
     proto::compact_formats::CompactBlock,
     wallet::{Note, NoteId, ReceivedNote, Recipient, WalletTransparentOutput},
     DecryptedOutput, ShieldedProtocol, TransferType,
@@ -262,10 +264,15 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
                 // 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.
                 UnifiedAddressRequest::all().map_or(Ok(false), |ua_request| {
-                    Ok(usk
-                        .to_unified_full_viewing_key()
-                        .default_address(ua_request)
-                        == ufvk.default_address(ua_request))
+                    match (
+                        usk.to_unified_full_viewing_key()
+                            .default_address(ua_request),
+                        ufvk.default_address(ua_request),
+                    ) {
+                        (Ok(a), Ok(b)) => Ok(a == b),
+                        (Err(e), _) => Err(e.into()),
+                        (_, Err(e)) => Err(e.into()),
+                    }
                 })
             })
         })
@@ -450,17 +457,15 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                     let search_from =
                         match wallet::get_current_address(wdb.conn.0, &wdb.params, account)? {
                             Some((_, mut last_diversifier_index)) => {
-                                last_diversifier_index
-                                    .increment()
-                                    .map_err(|_| SqliteClientError::DiversifierIndexOutOfRange)?;
+                                last_diversifier_index.increment().map_err(|_| {
+                                    AddressGenerationError::DiversifierSpaceExhausted
+                                })?;
                                 last_diversifier_index
                             }
                             None => DiversifierIndex::default(),
                         };
 
-                    let (addr, diversifier_index) = ufvk
-                        .find_address(search_from, request)
-                        .ok_or(SqliteClientError::DiversifierIndexOutOfRange)?;
+                    let (addr, diversifier_index) = ufvk.find_address(search_from, request)?;
 
                     wallet::insert_address(
                         wdb.conn.0,
@@ -1318,6 +1323,7 @@ mod tests {
         // The receiver for the default UA should be in the set.
         assert!(receivers.contains_key(
             ufvk.default_address(DEFAULT_UA_REQUEST)
+                .expect("A valid default address exists for the UFVK")
                 .0
                 .transparent()
                 .unwrap()

zcash_client_sqlite/src/wallet.rs

@@ -263,7 +263,9 @@ pub(crate) fn add_account<P: consensus::Parameters>(
     }
 
     // Always derive the default Unified Address for the account.
-    let (address, d_idx) = key.default_address(DEFAULT_UA_REQUEST);
+    let (address, d_idx) = key
+        .default_address(DEFAULT_UA_REQUEST)
+        .expect("A valid default address exists for the UFVK");
     insert_address(conn, params, account, d_idx, &address)?;
 
     Ok(())

zcash_client_sqlite/src/wallet/init.rs

@@ -1015,8 +1015,12 @@ mod tests {
             )?;
 
             let ufvk_str = ufvk.encode(&wdb.params);
-            let address_str =
-                Address::Unified(ufvk.default_address(DEFAULT_UA_REQUEST).0).encode(&wdb.params);
+            let address_str = Address::Unified(
+                ufvk.default_address(DEFAULT_UA_REQUEST)
+                    .expect("A valid default address exists for the UFVK")
+                    .0,
+            )
+            .encode(&wdb.params);
             wdb.conn.execute(
                 "INSERT INTO accounts (account, ufvk, address, transparent_address)
                 VALUES (?, ?, ?, '')",
@@ -1033,6 +1037,7 @@ mod tests {
                 let taddr = Address::Transparent(
                     *ufvk
                         .default_address(DEFAULT_UA_REQUEST)
+                        .expect("A valid default address exists for the UFVK")
                         .0
                         .transparent()
                         .unwrap(),

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

@@ -441,11 +441,13 @@ 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(
-            false,
-            true,
-            UA_TRANSPARENT,
-        ));
+        let (ua, _) = ufvk
+            .default_address(UnifiedAddressRequest::unsafe_new(
+                false,
+                true,
+                UA_TRANSPARENT,
+            ))
+            .expect("A valid default address exists for the UFVK");
         let taddr = ufvk
             .transparent()
             .and_then(|k| {

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

@@ -88,11 +88,13 @@ 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(UnifiedAddressRequest::unsafe_new(
-                false,
-                true,
-                UA_TRANSPARENT,
-            ));
+            let (expected_address, idx) = ufvk
+                .default_address(UnifiedAddressRequest::unsafe_new(
+                    false,
+                    true,
+                    UA_TRANSPARENT,
+                ))
+                .expect("A valid default address exists for the UFVK");
             if decoded_address != expected_address {
                 return Err(WalletMigrationError::CorruptedData(format!(
                     "Decoded UA {} does not match the UFVK's default address {} at {:?}.",
@@ -162,11 +164,13 @@ 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(UnifiedAddressRequest::unsafe_new(
+                    false,
+                    true,
+                    UA_TRANSPARENT,
+                ))
+                .expect("A valid default address exists for the UFVK");
             insert_address(transaction, &self.params, account, d_idx, &address)?;
         }
 

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

@@ -111,7 +111,9 @@ 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(ua_request)
+                            .expect("A valid default address exists for the UFVK");
                         if decoded_address != expected_address {
                             return Err(WalletMigrationError::CorruptedData(
                                 format!("Decoded unified address {} does not match the ufvk's default address {} at {:?}.",
@@ -123,7 +125,11 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
                 }
 
                 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(ua_request)
+                    .expect("A valid default address exists for the UFVK")
+                    .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

@@ -8,6 +8,15 @@ and this library adheres to Rust's notion of
 
 ### Added
 - `zcash_keys::address::Address::has_receiver`
+- `impl Display for zcash_keys::keys::AddressGenerationError`
+- `impl std::error::Error for zcash_keys::keys::AddressGenerationError`
+
+### Changed
+- `zcash_keys::keys::AddressGenerationError` has a new variant
+  `DiversifierSpaceExhausted`
+- `zcash_keys::keys::UnifiedFullViewingKey::{find_address, default_address}` 
+  now return `Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError>`
+  instead of `Option<(UnifiedAddress, DiversifierIndex)>`
 
 ## [0.1.1] - 2024-03-04
 

zcash_keys/src/keys.rs

@@ -1,4 +1,6 @@
 //! Helper functions for managing light client key material.
+use std::{error, fmt};
+
 use zcash_address::unified::{self, Container, Encoding, Typecode};
 use zcash_protocol::consensus::{self, NetworkConstants};
 use zip32::{AccountId, DiversifierIndex};
@@ -402,7 +404,9 @@ impl UnifiedSpendingKey {
         &self,
         request: UnifiedAddressRequest,
     ) -> (UnifiedAddress, DiversifierIndex) {
-        self.to_unified_full_viewing_key().default_address(request)
+        self.to_unified_full_viewing_key()
+            .default_address(request)
+            .unwrap()
     }
 
     #[cfg(all(
@@ -428,6 +432,8 @@ pub enum AddressGenerationError {
     /// The diversifier index could not be mapped to a valid Sapling diversifier.
     #[cfg(feature = "sapling")]
     InvalidSaplingDiversifierIndex(DiversifierIndex),
+    /// The space of available diversifier indices has been exhausted.
+    DiversifierSpaceExhausted,
     /// A requested address typecode was not recognized, so we are unable to generate the address
     /// as requested.
     ReceiverTypeNotSupported(Typecode),
@@ -439,6 +445,52 @@ pub enum AddressGenerationError {
     ShieldedReceiverRequired,
 }
 
+impl fmt::Display for AddressGenerationError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match &self {
+            AddressGenerationError::InvalidTransparentChildIndex(i) => {
+                write!(
+                    f,
+                    "Child index {:?} does not generate a valid transparent receiver",
+                    i
+                )
+            }
+            AddressGenerationError::InvalidSaplingDiversifierIndex(i) => {
+                write!(
+                    f,
+                    "Child index {:?} does not generate a valid Sapling receiver",
+                    i
+                )
+            }
+            AddressGenerationError::DiversifierSpaceExhausted => {
+                write!(
+                    f,
+                    "Exhausted the space of diversifier indices without finding an address."
+                )
+            }
+            AddressGenerationError::ReceiverTypeNotSupported(t) => {
+                write!(
+                    f,
+                    "Unified Address generation does not yet support receivers of type {:?}.",
+                    t
+                )
+            }
+            AddressGenerationError::KeyNotAvailable(t) => {
+                write!(
+                    f,
+                    "The Unified Viewing Key does not contain a key for typecode {:?}.",
+                    t
+                )
+            }
+            AddressGenerationError::ShieldedReceiverRequired => {
+                write!(f, "A Unified Address requires at least one shielded (Sapling or Orchard) receiver.")
+            }
+        }
+    }
+}
+
+impl error::Error for AddressGenerationError {}
+
 /// Specification for how a unified address should be generated from a unified viewing key.
 #[derive(Clone, Copy, Debug)]
 pub struct UnifiedAddressRequest {
@@ -780,13 +832,16 @@ impl UnifiedFullViewingKey {
         &self,
         mut j: DiversifierIndex,
         request: UnifiedAddressRequest,
-    ) -> Option<(UnifiedAddress, DiversifierIndex)> {
+    ) -> 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 self.transparent.is_some() && to_transparent_child_index(j).is_none() {
-            return None;
+        if request.has_p2pkh
+            && self.transparent.is_some()
+            && to_transparent_child_index(j).is_none()
+        {
+            return Err(AddressGenerationError::InvalidTransparentChildIndex(j));
         }
 
         // Find a working diversifier and construct the associated address.
@@ -794,16 +849,16 @@ impl UnifiedFullViewingKey {
             let res = self.address(j, request);
             match res {
                 Ok(ua) => {
-                    break Some((ua, j));
+                    return Ok((ua, j));
                 }
                 #[cfg(feature = "sapling")]
                 Err(AddressGenerationError::InvalidSaplingDiversifierIndex(_)) => {
                     if j.increment().is_err() {
-                        break None;
+                        return Err(AddressGenerationError::DiversifierSpaceExhausted);
                     }
                 }
-                Err(_) => {
-                    break None;
+                Err(other) => {
+                    return Err(other);
                 }
             }
         }
@@ -814,9 +869,8 @@ impl UnifiedFullViewingKey {
     pub fn default_address(
         &self,
         request: UnifiedAddressRequest,
-    ) -> (UnifiedAddress, DiversifierIndex) {
+    ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> {
         self.find_address(DiversifierIndex::new(), request)
-            .expect("UFVK should have at least one valid diversifier")
     }
 }