From 6cbdd494cf92ed9223164efa33d14acfa777fc59 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 28 Apr 2023 11:40:16 -0600 Subject: [PATCH] zcash_client_backend: Add receiver type selection to unified address derivation. --- Cargo.lock | 1 + zcash_client_backend/CHANGELOG.md | 12 ++ zcash_client_backend/Cargo.toml | 1 + zcash_client_backend/src/data_api.rs | 6 +- zcash_client_backend/src/fees/common.rs | 1 + zcash_client_backend/src/keys.rs | 167 +++++++++++++++++++----- zcash_client_backend/src/zip321.rs | 3 +- zcash_client_sqlite/Cargo.toml | 1 + zcash_client_sqlite/src/lib.rs | 16 ++- zcash_client_sqlite/src/wallet/init.rs | 4 +- 10 files changed, 170 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7622809e3..66e18a749 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3041,6 +3041,7 @@ dependencies = [ "zcash_note_encryption", "zcash_primitives", "zcash_proofs", + "zip32", ] [[package]] diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 0a15aca9c..2ec395ed6 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/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` instead of + `Option` 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. diff --git a/zcash_client_backend/Cargo.toml b/zcash_client_backend/Cargo.toml index ad8c872e6..486810e3d 100644 --- a/zcash_client_backend/Cargo.toml +++ b/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.) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index b00433632..b2980a139 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/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, 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, Self::Error> { Ok(None) } diff --git a/zcash_client_backend/src/fees/common.rs b/zcash_client_backend/src/fees/common.rs index 2bef424cb..aba4fe3d8 100644 --- a/zcash_client_backend/src/fees/common.rs +++ b/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 diff --git a/zcash_client_backend/src/keys.rs b/zcash_client_backend/src/keys.rs index 1e2be9303..e2aec528a 100644 --- a/zcash_client_backend/src/keys.rs +++ b/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 { + #[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 { - let sapling = if let Some(extfvk) = self.sapling.as_ref() { - Some(extfvk.address(j)?) + pub fn address( + &self, + j: DiversifierIndex, + request: UnifiedAddressRequest, + ) -> Result { + #[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,12 +705,19 @@ 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; + } } } } @@ -630,7 +725,8 @@ impl UnifiedFullViewingKey { /// 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)) => { diff --git a/zcash_client_backend/src/zip321.rs b/zcash_client_backend/src/zip321.rs index 6cf104d5e..9eb043d5c 100644 --- a/zcash_client_backend/src/zip321.rs +++ b/zcash_client_backend/src/zip321.rs @@ -794,8 +794,7 @@ pub mod testing { label in option::of(any::()), // prevent duplicates by generating a set rather than a vec other_params in btree_map(VALID_PARAMNAME, any::(), 0..3), - ) -> Payment { - + ) -> Payment { let is_shielded = match recipient_address { Address::Transparent(_) => false, Address::Sapling(_) | Address::Unified(_) => true, diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 69422c42f..95ea752df 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -19,6 +19,7 @@ all-features = true rustdoc-args = ["--cfg", "docsrs"] [dependencies] +zcash_address.workspace = true zcash_client_backend = { workspace = true, features = ["unstable-serialization", "unstable-spanning-tree"] } zcash_encoding.workspace = true zcash_primitives.workspace = true diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 40164475c..db432183e 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -67,7 +67,7 @@ use zcash_client_backend::{ ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }, - keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, + keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, wallet::{Note, NoteId, ReceivedNote, Recipient, WalletTransparentOutput}, DecryptedOutput, PoolType, ShieldedProtocol, TransferType, @@ -401,6 +401,7 @@ impl WalletWrite for WalletDb fn get_next_available_address( &mut self, account: AccountId, + request: UnifiedAddressRequest, ) -> Result, Self::Error> { self.transactionally( |wdb| match wdb.get_unified_full_viewing_keys()?.get(&account) { @@ -417,7 +418,7 @@ impl WalletWrite for WalletDb }; let (addr, diversifier_index) = ufvk - .find_address(search_from) + .find_address(search_from, request) .ok_or(SqliteClientError::DiversifierIndexOutOfRange)?; wallet::insert_address( @@ -1109,7 +1110,10 @@ extern crate assert_matches; #[cfg(test)] mod tests { - use zcash_client_backend::data_api::{AccountBirthday, WalletRead, WalletWrite}; + use zcash_client_backend::{ + data_api::{AccountBirthday, WalletRead, WalletWrite}, + keys::UnifiedAddressRequest, + }; use crate::{testing::TestBuilder, AccountId}; @@ -1132,7 +1136,11 @@ mod tests { let current_addr = st.wallet().get_current_address(account).unwrap(); assert!(current_addr.is_some()); - let addr2 = st.wallet_mut().get_next_available_address(account).unwrap(); + // TODO: Add Orchard + let addr2 = st + .wallet_mut() + .get_next_available_address(account, UnifiedAddressRequest::DEFAULT) + .unwrap(); assert!(addr2.is_some()); assert_ne!(current_addr, addr2); diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 464df6124..e3f470ce3 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -1060,7 +1060,7 @@ mod tests { #[test] #[cfg(feature = "transparent-inputs")] fn account_produces_expected_ua_sequence() { - use zcash_client_backend::data_api::AccountBirthday; + use zcash_client_backend::{data_api::AccountBirthday, keys::UnifiedAddressRequest}; let network = Network::MainNetwork; let data_file = NamedTempFile::new().unwrap(); @@ -1090,7 +1090,7 @@ mod tests { assert_eq!(tv.unified_addr, ua.encode(&Network::MainNetwork)); db_data - .get_next_available_address(account) + .get_next_available_address(account, UnifiedAddressRequest::DEFAULT) .unwrap() .expect("get_next_available_address generated an address"); } else {