Merge pull request zcash#829 from nuttycom/wallet/select_ua_receivers

Add receiver type selection to unified address derivation.

zcash_client_backend/CHANGELOG.md

@@ -29,6 +29,9 @@ and this library adheres to Rust's notion of
     functionality and move it behind the `transparent-inputs` feature flag.
 - `zcash_client_backend::fees::{standard, orchard, sapling}`
 - `zcash_client_backend::fees::ChangeValue::{new, orchard}`
+- `zcash_client_backend::keys`:
+  - `AddressGenerationError`
+  - `UnifiedAddressRequest`
 - `zcash_client_backend::wallet`:
   - `Note`
   - `ReceivedNote`
@@ -81,6 +84,8 @@ and this library adheres to Rust's notion of
   - Fields of `Balance` and `AccountBalance` have been made private and the values
     of these fields have been made available via methods having the same names
     as the previously-public fields.
+  - `WalletWrite::get_next_available_address` now takes an additional 
+    `UnifiedAddressRequest` argument.
   - `chain::scan_cached_blocks` now returns a `ScanSummary` containing metadata
     about the scanned blocks on success.
   - `error::Error` enum changes:
@@ -199,8 +204,15 @@ and this library adheres to Rust's notion of
 - `zcash_client_backend::keys`:
   - `DerivationError::Orchard` is now only available when the `orchard` feature
     is enabled.
+  - `UnifiedSpendingKey::address` now takes an argument that specifies the
+    receivers to be generated in the resulting address. Also, it now returns
+    `Result<UnifiedAddress, AddressGenerationError>` instead of
+    `Option<UnifiedAddress>` so that we may better report to the user how
+    address generation has failed.
   - `UnifiedSpendingKey::orchard` is now only available when the `orchard`
     feature is enabled.
+  - `UnifiedSpendingKey::transparent` is now only available when the
+    `transparent-inputs` feature is enabled.
   - `UnifiedFullViewingKey::new` no longer takes an Orchard full viewing key
     argument unless the `orchard` feature is enabled.
 

zcash_client_backend/Cargo.toml

@@ -29,6 +29,7 @@ zcash_address.workspace = true
 zcash_encoding.workspace = true
 zcash_note_encryption.workspace = true
 zcash_primitives.workspace = true
+zip32.workspace = true
 
 # Dependencies exposed in a public API:
 # (Breaking upgrades to these require a breaking upgrade to this crate.)

zcash_client_backend/src/data_api.rs

@@ -26,7 +26,7 @@ use zcash_primitives::{
 use crate::{
     address::{AddressMetadata, UnifiedAddress},
     decrypt::DecryptedOutput,
-    keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
+    keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
     proto::service::TreeState,
     wallet::{Note, NoteId, ReceivedNote, Recipient, WalletTransparentOutput, WalletTx},
     ShieldedProtocol,
@@ -1004,6 +1004,7 @@ pub trait WalletWrite: WalletRead {
     fn get_next_available_address(
         &mut self,
         account: AccountId,
+        request: UnifiedAddressRequest,
     ) -> Result<Option<UnifiedAddress>, Self::Error>;
 
     /// Updates the state of the wallet database by persisting the provided block information,
@@ -1104,7 +1105,7 @@ pub mod testing {
 
     use crate::{
         address::{AddressMetadata, UnifiedAddress},
-        keys::{UnifiedFullViewingKey, UnifiedSpendingKey},
+        keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey},
         wallet::{Note, NoteId, ReceivedNote, WalletTransparentOutput},
         ShieldedProtocol,
     };
@@ -1301,6 +1302,7 @@ pub mod testing {
         fn get_next_available_address(
             &mut self,
             _account: AccountId,
+            _request: UnifiedAddressRequest,
         ) -> Result<Option<UnifiedAddress>, Self::Error> {
             Ok(None)
         }

zcash_client_backend/src/fees/common.rs

@@ -86,6 +86,7 @@ where
 
     // TODO: implement a less naive strategy for selecting the pool to which change will be sent.
     #[cfg(feature = "orchard")]
+    #[allow(clippy::if_same_then_else)]
     let (change_pool, sapling_change, orchard_change) =
         if orchard_in.is_positive() || orchard_out.is_positive() {
             // Send change to Orchard if we're spending any Orchard inputs or creating any Orchard outputs

zcash_client_backend/src/keys.rs

@@ -1,5 +1,5 @@
 //! Helper functions for managing light client key material.
-use zcash_address::unified::{self, Container, Encoding};
+use zcash_address::unified::{self, Container, Encoding, Typecode};
 use zcash_primitives::{
     consensus,
     zip32::{AccountId, DiversifierIndex},
@@ -21,16 +21,18 @@ use {
     byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt},
     std::convert::TryFrom,
     std::io::{Read, Write},
-    zcash_address::unified::Typecode,
     zcash_encoding::CompactSize,
     zcash_primitives::consensus::BranchId,
 };
 
+#[cfg(feature = "orchard")]
+use orchard::{self, keys::Scope};
+
 pub mod sapling {
     pub use sapling::zip32::{
         DiversifiableFullViewingKey, ExtendedFullViewingKey, ExtendedSpendingKey,
     };
-    use zcash_primitives::zip32::{AccountId, ChildIndex};
+    use zip32::{AccountId, ChildIndex};
 
     /// Derives the ZIP 32 [`ExtendedSpendingKey`] for a given coin type and account from the
     /// given seed.
@@ -44,11 +46,11 @@ pub mod sapling {
     /// ```
     /// use zcash_primitives::{
     ///     constants::testnet::COIN_TYPE,
-    ///     zip32::AccountId,
     /// };
     /// use zcash_client_backend::{
     ///     keys::sapling,
     /// };
+    /// use zip32::AccountId;
     ///
     /// let extsk = sapling::spending_key(&[0; 32][..], COIN_TYPE, AccountId::ZERO);
     /// ```
@@ -160,8 +162,7 @@ impl Era {
     }
 }
 
-/// A set of spending keys that are all associated with a single
-/// ZIP-0032 account identifier.
+/// A set of spending keys that are all associated with a single ZIP-0032 account identifier.
 #[derive(Clone, Debug)]
 #[doc(hidden)]
 pub struct UnifiedSpendingKey {
@@ -397,6 +398,65 @@ impl UnifiedSpendingKey {
     }
 }
 
+/// Errors that can occur in the generation of unified addresses.
+#[derive(Clone, Debug)]
+pub enum AddressGenerationError {
+    /// The requested diversifier index was outside the range of valid transparent
+    /// child address indices.
+    InvalidTransparentChildIndex(DiversifierIndex),
+    /// The diversifier index could not be mapped to a valid Sapling diversifier.
+    InvalidSaplingDiversifierIndex(DiversifierIndex),
+    /// A requested address typecode was not recognized, so we are unable to generate the address
+    /// as requested.
+    ReceiverTypeNotSupported(Typecode),
+    /// A Unified address cannot be generated without at least one shielded receiver being
+    /// included.
+    ShieldedReceiverRequired,
+}
+
+/// Specification for how a unified address should be generated from a unified viewing key.
+#[derive(Clone, Copy, Debug)]
+pub struct UnifiedAddressRequest {
+    #[cfg(feature = "orchard")]
+    has_orchard: bool,
+    has_sapling: bool,
+    #[cfg(feature = "transparent-inputs")]
+    has_p2pkh: bool,
+}
+
+impl UnifiedAddressRequest {
+    pub const DEFAULT: UnifiedAddressRequest = Self {
+        #[cfg(feature = "orchard")]
+        has_orchard: false, // FIXME: Always request Orchard receivers once we can receive Orchard funds
+        has_sapling: true,
+        #[cfg(feature = "transparent-inputs")]
+        has_p2pkh: true,
+    };
+
+    pub fn new(
+        #[cfg(feature = "orchard")] has_orchard: bool,
+        has_sapling: bool,
+        #[cfg(feature = "transparent-inputs")] has_p2pkh: bool,
+    ) -> Option<Self> {
+        #[cfg(feature = "orchard")]
+        let has_shielded_receiver = has_orchard || has_sapling;
+        #[cfg(not(feature = "orchard"))]
+        let has_shielded_receiver = has_sapling;
+
+        if !has_shielded_receiver {
+            None
+        } else {
+            Some(Self {
+                #[cfg(feature = "orchard")]
+                has_orchard,
+                has_sapling,
+                #[cfg(feature = "transparent-inputs")]
+                has_p2pkh,
+            })
+        }
+    }
+}
+
 /// A [ZIP 316](https://zips.z.cash/zip-0316) unified full viewing key.
 #[derive(Clone, Debug)]
 #[doc(hidden)]
@@ -560,29 +620,55 @@ impl UnifiedFullViewingKey {
         self.orchard.as_ref()
     }
 
-    /// Attempts to derive the Unified Address for the given diversifier index.
+    /// Attempts to derive the Unified Address for the given diversifier index and
+    /// receiver types.
     ///
     /// Returns `None` if the specified index does not produce a valid diversifier.
-    // TODO: Allow filtering down by receiver types?
-    pub fn address(&self, j: DiversifierIndex) -> Option<UnifiedAddress> {
-        let sapling = if let Some(extfvk) = self.sapling.as_ref() {
-            Some(extfvk.address(j)?)
+    pub fn address(
+        &self,
+        j: DiversifierIndex,
+        request: UnifiedAddressRequest,
+    ) -> Result<UnifiedAddress, AddressGenerationError> {
+        #[cfg(feature = "orchard")]
+        let orchard = {
+            let orchard_j = orchard::keys::DiversifierIndex::from(*j.as_bytes());
+            self.orchard
+                .as_ref()
+                .filter(|_| request.has_orchard)
+                .map(|ofvk| ofvk.address_at(orchard_j, Scope::External))
+        };
+
+        let sapling = if let Some(extfvk) = self.sapling.as_ref().filter(|_| request.has_sapling) {
+            // If a Sapling receiver type is requested, we must be able to construct an
+            // address; if we're unable to do so, then no Unified Address exists at this
+            // diversifier and we use `?` to early-return from this method.
+            Some(
+                extfvk
+                    .address(j)
+                    .ok_or(AddressGenerationError::InvalidSaplingDiversifierIndex(j))?,
+            )
         } else {
             None
         };
 
         #[cfg(feature = "transparent-inputs")]
-        let transparent = if let Some(tfvk) = self.transparent.as_ref() {
+        let transparent = if let Some(tfvk) =
+            self.transparent.as_ref().filter(|_| request.has_p2pkh)
+        {
+            // If a transparent receiver type is requested, we must be able to construct an
+            // address; if we're unable to do so, then no Unified Address exists at this
+            // diversifier.
             match to_transparent_child_index(j) {
                 Some(transparent_j) => match tfvk
                     .derive_external_ivk()
                     .and_then(|tivk| tivk.derive_address(transparent_j))
                 {
                     Ok(taddr) => Some(taddr),
-                    Err(_) => return None,
+                    Err(_) => return Err(AddressGenerationError::InvalidTransparentChildIndex(j)),
                 },
-                // Diversifier doesn't generate a valid transparent child index.
-                None => return None,
+                // Diversifier doesn't generate a valid transparent child index, so we eagerly
+                // return `None`.
+                None => return Err(AddressGenerationError::InvalidTransparentChildIndex(j)),
             }
         } else {
             None
@@ -592,10 +678,11 @@ impl UnifiedFullViewingKey {
 
         UnifiedAddress::from_receivers(
             #[cfg(feature = "orchard")]
-            None,
+            orchard,
             sapling,
             transparent,
         )
+        .ok_or(AddressGenerationError::ShieldedReceiverRequired)
     }
 
     /// Searches the diversifier space starting at diversifier index `j` for one which will
@@ -606,6 +693,7 @@ impl UnifiedFullViewingKey {
     pub fn find_address(
         &self,
         mut j: DiversifierIndex,
+        request: UnifiedAddressRequest,
     ) -> Option<(UnifiedAddress, DiversifierIndex)> {
         // 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
@@ -617,20 +705,28 @@ impl UnifiedFullViewingKey {
 
         // Find a working diversifier and construct the associated address.
         loop {
-            let res = self.address(j);
-            if let Some(ua) = res {
-                break Some((ua, j));
-            }
-            if j.increment().is_err() {
-                break None;
+            let res = self.address(j, request);
+            match res {
+                Ok(ua) => {
+                    break Some((ua, j));
+                }
+                Err(AddressGenerationError::InvalidSaplingDiversifierIndex(_)) => {
+                    if j.increment().is_err() {
+                        break None;
+                    }
+                }
+                Err(_) => {
+                    break None;
+                }
             }
         }
     }
 
     /// Returns the Unified Address corresponding to the smallest valid diversifier index,
     /// along with that index.
     pub fn default_address(&self) -> (UnifiedAddress, DiversifierIndex) {
-        self.find_address(DiversifierIndex::new())
+        // FIXME: Enable Orchard keys
+        self.find_address(DiversifierIndex::new(), UnifiedAddressRequest::DEFAULT)
             .expect("UFVK should have at least one valid diversifier")
     }
 }
@@ -663,19 +759,18 @@ mod tests {
     use proptest::prelude::proptest;
 
     use super::{sapling, UnifiedFullViewingKey};
-    use zcash_primitives::{consensus::MAIN_NETWORK, zip32::AccountId};
+    use zcash_primitives::consensus::MAIN_NETWORK;
+    use zip32::AccountId;
 
     #[cfg(feature = "transparent-inputs")]
     use {
         crate::{address::Address, encoding::AddressCodec},
         zcash_address::test_vectors,
-        zcash_primitives::{
-            legacy::{
-                self,
-                keys::{AccountPrivKey, IncomingViewingKey},
-            },
-            zip32::DiversifierIndex,
+        zcash_primitives::legacy::{
+            self,
+            keys::{AccountPrivKey, IncomingViewingKey},
         },
+        zip32::DiversifierIndex,
     };
 
     #[cfg(feature = "unstable")]
@@ -797,6 +892,8 @@ mod tests {
     #[test]
     #[cfg(feature = "transparent-inputs")]
     fn ufvk_derivation() {
+        use crate::keys::UnifiedAddressRequest;
+
         use super::UnifiedSpendingKey;
 
         for tv in test_vectors::UNIFIED {
@@ -816,8 +913,14 @@ mod tests {
                 continue;
             }
 
-            let ua = ufvk.address(d_idx).unwrap_or_else(|| panic!("diversifier index {} should have produced a valid unified address for account {}",
-                tv.diversifier_index, tv.account));
+            let ua = ufvk
+                .address(d_idx, UnifiedAddressRequest::DEFAULT)
+                .unwrap_or_else(|err| {
+                    panic!(
+                        "unified address generation failed for account {}: {:?}",
+                        tv.account, err
+                    )
+                });
 
             match Address::decode(&MAIN_NETWORK, tv.unified_addr) {
                 Some(Address::Unified(tvua)) => {

zcash_client_backend/src/zip321.rs

@@ -794,8 +794,7 @@ pub mod testing {
             label in option::of(any::<String>()),
             // prevent duplicates by generating a set rather than a vec
             other_params in btree_map(VALID_PARAMNAME, any::<String>(), 0..3),
-            ) -> Payment {
-
+        ) -> Payment {
             let is_shielded = match recipient_address {
                 Address::Transparent(_) => false,
                 Address::Sapling(_) | Address::Unified(_) => true,