From 071d7c51d777a207f66a287c249ee4dbec7e77a8 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Fri, 8 Mar 2024 23:05:37 -0700 Subject: [PATCH 1/8] Add `UnifiedIncomingViewingKey` struct Also update sqlite to utilize the new struct --- zcash_client_sqlite/src/wallet.rs | 52 +- .../init/migrations/full_account_ids.rs | 10 +- zcash_keys/CHANGELOG.md | 9 + zcash_keys/src/keys.rs | 554 +++++++++++++++++- 4 files changed, 564 insertions(+), 61 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 0a8a02a4e..7d2f8b793 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -75,7 +75,9 @@ use std::num::NonZeroU32; use std::ops::RangeInclusive; use tracing::debug; use zcash_address::unified::{Encoding, Ivk, Uivk}; -use zcash_keys::keys::{AddressGenerationError, HdSeedFingerprint, UnifiedAddressRequest}; +use zcash_keys::keys::{ + AddressGenerationError, HdSeedFingerprint, UnifiedAddressRequest, UnifiedIncomingViewingKey, +}; use zcash_client_backend::{ address::{Address, UnifiedAddress}, @@ -200,7 +202,7 @@ pub(crate) enum ImportedAccount { /// An account that was imported via its full viewing key. Full(Box), /// An account that was imported via its incoming viewing key. - Incoming(Uivk), + Incoming(Box), } /// Describes an account in terms of its UVK or ZIP-32 origins. @@ -225,7 +227,7 @@ impl Account { match self { Account::Zip32(HdSeedAccount(_, _, ufvk)) => ufvk.default_address(request), Account::Imported(ImportedAccount::Full(ufvk)) => ufvk.default_address(request), - Account::Imported(ImportedAccount::Incoming(_uivk)) => todo!(), + Account::Imported(ImportedAccount::Incoming(uivk)) => uivk.default_address(request), } } } @@ -304,50 +306,34 @@ fn get_sql_values_for_account_parameters<'a, P: consensus::Parameters>( hd_seed_fingerprint: Some(hdaccount.hd_seed_fingerprint().as_bytes()), hd_account_index: Some(hdaccount.account_index().into()), ufvk: Some(hdaccount.ufvk().encode(params)), - uivk: ufvk_to_uivk(hdaccount.ufvk(), params)?, + uivk: hdaccount + .ufvk() + .to_unified_incoming_viewing_key() + .map_err(|e| SqliteClientError::CorruptedData(e.to_string()))? + .to_uivk() + .encode(¶ms.network_type()), }, Account::Imported(ImportedAccount::Full(ufvk)) => AccountSqlValues { account_type: AccountType::Imported.into(), hd_seed_fingerprint: None, hd_account_index: None, ufvk: Some(ufvk.encode(params)), - uivk: ufvk_to_uivk(ufvk, params)?, + uivk: ufvk + .to_unified_incoming_viewing_key() + .map_err(|e| SqliteClientError::CorruptedData(e.to_string()))? + .to_uivk() + .encode(¶ms.network_type()), }, Account::Imported(ImportedAccount::Incoming(uivk)) => AccountSqlValues { account_type: AccountType::Imported.into(), hd_seed_fingerprint: None, hd_account_index: None, ufvk: None, - uivk: uivk.encode(¶ms.network_type()), + uivk: uivk.to_uivk().encode(¶ms.network_type()), }, }) } -pub(crate) fn ufvk_to_uivk( - ufvk: &UnifiedFullViewingKey, - params: &P, -) -> Result { - let mut ivks: Vec = Vec::new(); - if let Some(orchard) = ufvk.orchard() { - ivks.push(Ivk::Orchard(orchard.to_ivk(Scope::External).to_bytes())); - } - if let Some(sapling) = ufvk.sapling() { - let ivk = sapling.to_external_ivk(); - ivks.push(Ivk::Sapling(ivk.to_bytes())); - } - #[cfg(feature = "transparent-inputs")] - if let Some(tfvk) = ufvk.transparent() { - let tivk = tfvk.derive_external_ivk()?; - ivks.push(Ivk::P2pkh(tivk.serialize().try_into().map_err(|_| { - SqliteClientError::BadAccountData("Unable to serialize transparent IVK.".to_string()) - })?)); - } - - let uivk = zcash_address::unified::Uivk::try_from_items(ivks) - .map_err(|e| SqliteClientError::BadAccountData(format!("Unable to derive UIVK: {}", e)))?; - Ok(uivk.encode(¶ms.network_type())) -} - pub(crate) fn add_account( conn: &rusqlite::Transaction, params: &P, @@ -1202,6 +1188,8 @@ pub(crate) fn get_account, P: Parameters>( "UIVK network type does not match wallet network type".to_string(), )); } + let uivk = UnifiedIncomingViewingKey::from_uivk(&uivk) + .map_err(|e| SqliteClientError::CorruptedData(e.to_string()))?; match account_type { AccountType::Zip32 => Ok(Some(Account::Zip32(HdSeedAccount::new( @@ -1222,7 +1210,7 @@ pub(crate) fn get_account, P: Parameters>( AccountType::Imported => Ok(Some(Account::Imported(if let Some(ufvk) = ufvk { ImportedAccount::Full(Box::new(ufvk)) } else { - ImportedAccount::Incoming(uivk) + ImportedAccount::Incoming(Box::new(uivk)) }))), } } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs index fae568533..ec1913b23 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs @@ -1,10 +1,11 @@ use std::{collections::HashSet, rc::Rc}; -use crate::wallet::{init::WalletMigrationError, ufvk_to_uivk, AccountType}; +use crate::wallet::{init::WalletMigrationError, AccountType}; use rusqlite::{named_params, Transaction}; use schemer_rusqlite::RusqliteMigration; use secrecy::{ExposeSecret, SecretVec}; use uuid::Uuid; +use zcash_address::unified::Encoding; use zcash_client_backend::keys::UnifiedSpendingKey; use zcash_keys::keys::{HdSeedFingerprint, UnifiedFullViewingKey}; use zcash_primitives::consensus; @@ -113,8 +114,11 @@ impl RusqliteMigration for Migration

{ )); } - let uivk = ufvk_to_uivk(&ufvk_parsed, &self.params) - .map_err(|e| WalletMigrationError::CorruptedData(e.to_string()))?; + let uivk = ufvk_parsed + .to_unified_incoming_viewing_key() + .map_err(|e| WalletMigrationError::CorruptedData(e.to_string()))? + .to_uivk() + .encode(&self.params.network_type()); transaction.execute(r#" INSERT INTO accounts_new (id, account_type, hd_seed_fingerprint, hd_account_index, ufvk, uivk, birthday_height, recover_until_height) diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 79654d4c9..08a1f9f57 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -11,6 +11,11 @@ and this library adheres to Rust's notion of - `zcash_keys::address::Address::has_receiver` - `impl Display for zcash_keys::keys::AddressGenerationError` - `impl std::error::Error for zcash_keys::keys::AddressGenerationError` +- `zcash_keys::keys::UnifiedError` +- `zcash_keys::keys::UnifiedFullViewingKey::from_ufvk` +- `zcash_keys::keys::UnifiedFullViewingKey::to_ufvk` +- `zcash_keys::keys::UnifiedFullViewingKey::to_unified_incoming_viewing_key` +- `zcash_keys::keys::UnifiedIncomingViewingKey` ### Changed - `zcash_keys::keys::AddressGenerationError` has a new variant @@ -18,6 +23,10 @@ and this library adheres to Rust's notion of - `zcash_keys::keys::UnifiedFullViewingKey::{find_address, default_address}` now return `Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError>` (instead of `Option<(UnifiedAddress, DiversifierIndex)>` for `find_address`). +- `zcash_keys::keys::AddressGenerationError` + - Dropped `Clone` trait + - Added `UnifiedError` variant. + - Added `HDWalletError` variant. ### Fixed - `UnifiedFullViewingKey::find_address` can now find an address for a diversifier diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 27dfb8393..c48e12a1e 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -3,8 +3,8 @@ use blake2b_simd::Params as blake2bParams; use secrecy::{ExposeSecret, SecretVec}; use std::{error, fmt}; -use zcash_address::unified::{self, Container, Encoding, Typecode}; -use zcash_protocol::consensus::{self, NetworkConstants}; +use zcash_address::unified::{self, Container, Encoding, Typecode, Ufvk, Uivk}; +use zcash_protocol::{consensus, ShieldedProtocol}; use zip32::{AccountId, DiversifierIndex}; use crate::address::UnifiedAddress; @@ -15,6 +15,10 @@ use { zcash_primitives::legacy::keys::{self as legacy, IncomingViewingKey, NonHardenedChildIndex}, }; +#[cfg(any(feature = "sapling", feature = "orchard"))] +// Your code here +use zcash_protocol::consensus::NetworkConstants; + #[cfg(all( feature = "transparent-inputs", any(test, feature = "test-dependencies") @@ -472,7 +476,7 @@ impl UnifiedSpendingKey { } /// Errors that can occur in the generation of unified addresses. -#[derive(Clone, Debug)] +#[derive(Debug)] pub enum AddressGenerationError { /// The requested diversifier index was outside the range of valid transparent /// child address indices. @@ -492,6 +496,26 @@ pub enum AddressGenerationError { /// A Unified address cannot be generated without at least one shielded receiver being /// included. ShieldedReceiverRequired, + + // An error occurred while deriving a key or address from an HD wallet. + UnifiedError(UnifiedError), + + // An error occurred while deriving a transparent key or address from an HD wallet. + #[cfg(feature = "transparent-inputs")] + HDWalletError(hdwallet::error::Error), +} + +impl From for AddressGenerationError { + fn from(e: UnifiedError) -> Self { + AddressGenerationError::UnifiedError(e) + } +} + +#[cfg(feature = "transparent-inputs")] +impl From for AddressGenerationError { + fn from(e: hdwallet::error::Error) -> Self { + AddressGenerationError::HDWalletError(e) + } } impl fmt::Display for AddressGenerationError { @@ -505,6 +529,7 @@ impl fmt::Display for AddressGenerationError { i ) } + #[cfg(feature = "sapling")] AddressGenerationError::InvalidSaplingDiversifierIndex(i) => { write!( f, @@ -535,6 +560,13 @@ impl fmt::Display for AddressGenerationError { AddressGenerationError::ShieldedReceiverRequired => { write!(f, "A Unified Address requires at least one shielded (Sapling or Orchard) receiver.") } + AddressGenerationError::UnifiedError(e) => { + write!(f, "UnifiedError: {}", e) + } + #[cfg(feature = "transparent-inputs")] + AddressGenerationError::HDWalletError(e) => { + write!(f, "HDWalletError: {}", e) + } } } } @@ -601,9 +633,43 @@ impl UnifiedAddressRequest { } } +/// Errors that may be returned from the [UnifiedFullViewingKey] and [UnifiedIncomingViewingKey] types. +#[derive(Debug)] +pub enum UnifiedError { + InvalidShieldedKey(ShieldedProtocol), + #[cfg(feature = "transparent-inputs")] + InvalidTransparentKey(hdwallet::error::Error), + #[cfg(feature = "transparent-inputs")] + HDWallet(hdwallet::error::Error), +} + +#[cfg(feature = "transparent-inputs")] +impl From for UnifiedError { + fn from(e: hdwallet::error::Error) -> Self { + UnifiedError::HDWallet(e) + } +} + +impl fmt::Display for UnifiedError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match &self { + UnifiedError::InvalidShieldedKey(p) => { + write!(f, "Invalid key for shielded protocol: {:?}", p) + } + #[cfg(feature = "transparent-inputs")] + UnifiedError::InvalidTransparentKey(e) => { + write!(f, "Invalid key for transparent protocol: {:?}", e) + } + #[cfg(feature = "transparent-inputs")] + UnifiedError::HDWallet(e) => { + write!(f, "HDWallet error: {}", e) + } + } + } +} + /// A [ZIP 316](https://zips.z.cash/zip-0316) unified full viewing key. #[derive(Clone, Debug)] -#[doc(hidden)] pub struct UnifiedFullViewingKey { #[cfg(feature = "transparent-inputs")] transparent: Option, @@ -662,6 +728,13 @@ impl UnifiedFullViewingKey { )); } + Self::from_ufvk(&ufvk).map_err(|e| e.to_string()) + } + + /// Parses a `UnifiedFullViewingKey` from its [ZIP 316] string encoding. + /// + /// [ZIP 316]: https://zips.z.cash/zip-0316 + pub fn from_ufvk(ufvk: &Ufvk) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; #[cfg(feature = "sapling")] @@ -677,21 +750,21 @@ impl UnifiedFullViewingKey { .filter_map(|receiver| match receiver { #[cfg(feature = "orchard")] unified::Fvk::Orchard(data) => orchard::keys::FullViewingKey::from_bytes(data) - .ok_or("Invalid Orchard FVK in Unified FVK") + .ok_or(UnifiedError::InvalidShieldedKey(ShieldedProtocol::Orchard)) .map(|addr| { orchard = Some(addr); None }) .transpose(), #[cfg(not(feature = "orchard"))] - unified::Fvk::Orchard(data) => Some(Ok::<_, &'static str>(( + unified::Fvk::Orchard(data) => Some(Ok::<_, UnifiedError>(( u32::from(unified::Typecode::Orchard), data.to_vec(), ))), #[cfg(feature = "sapling")] unified::Fvk::Sapling(data) => { sapling::DiversifiableFullViewingKey::from_bytes(data) - .ok_or("Invalid Sapling FVK in Unified FVK") + .ok_or(UnifiedError::InvalidShieldedKey(ShieldedProtocol::Sapling)) .map(|pa| { sapling = Some(pa); None @@ -699,20 +772,20 @@ impl UnifiedFullViewingKey { .transpose() } #[cfg(not(feature = "sapling"))] - unified::Fvk::Sapling(data) => Some(Ok::<_, &'static str>(( + unified::Fvk::Sapling(data) => Some(Ok::<_, UnifiedError>(( u32::from(unified::Typecode::Sapling), data.to_vec(), ))), #[cfg(feature = "transparent-inputs")] unified::Fvk::P2pkh(data) => legacy::AccountPubKey::deserialize(data) - .map_err(|_| "Invalid transparent FVK in Unified FVK") + .map_err(UnifiedError::InvalidTransparentKey) .map(|tfvk| { transparent = Some(tfvk); None }) .transpose(), #[cfg(not(feature = "transparent-inputs"))] - unified::Fvk::P2pkh(data) => Some(Ok::<_, &'static str>(( + unified::Fvk::P2pkh(data) => Some(Ok::<_, UnifiedError>(( u32::from(unified::Typecode::P2pkh), data.to_vec(), ))), @@ -733,6 +806,11 @@ impl UnifiedFullViewingKey { /// Returns the string encoding of this `UnifiedFullViewingKey` for the given network. pub fn encode(&self, params: &P) -> String { + self.to_ufvk().encode(¶ms.network_type()) + } + + /// Returns the string encoding of this `UnifiedFullViewingKey` for the given network. + pub fn to_ufvk(&self) -> Ufvk { let items = std::iter::empty().chain(self.unknown.iter().map(|(typecode, data)| { unified::Fvk::Unknown { typecode: *typecode, @@ -761,9 +839,27 @@ impl UnifiedFullViewingKey { .map(unified::Fvk::P2pkh), ); - let ufvk = unified::Ufvk::try_from_items(items.collect()) - .expect("UnifiedFullViewingKey should only be constructed safely"); - ufvk.encode(¶ms.network_type()) + unified::Ufvk::try_from_items(items.collect()) + .expect("UnifiedFullViewingKey should only be constructed safely") + } + + /// Derives a Unified Incoming Viewing Key from this Unified Full Viewing Key. + pub fn to_unified_incoming_viewing_key( + &self, + ) -> Result { + Ok(UnifiedIncomingViewingKey { + #[cfg(feature = "transparent-inputs")] + transparent: self + .transparent + .as_ref() + .map(|t| t.derive_external_ivk()) + .transpose()?, + #[cfg(feature = "sapling")] + sapling: self.sapling.as_ref().map(|s| s.to_external_ivk()), + #[cfg(feature = "orchard")] + orchard: self.orchard.as_ref().map(|o| o.to_ivk(Scope::External)), + unknown: Vec::new(), + }) } /// Returns the transparent component of the unified key at the @@ -785,6 +881,221 @@ impl UnifiedFullViewingKey { self.orchard.as_ref() } + /// 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. + pub fn address( + &self, + j: DiversifierIndex, + request: UnifiedAddressRequest, + ) -> Result { + self.to_unified_incoming_viewing_key()?.address(j, request) + } + + /// Searches the diversifier space starting at diversifier index `j` for one which will + /// produce a valid diversifier, and return the Unified Address constructed using that + /// diversifier along with the index at which the valid diversifier was found. + /// + /// Returns an `Err(AddressGenerationError)` if no valid diversifier exists or if the features + /// required to satisfy the unified address request are not properly enabled. + #[allow(unused_mut)] + pub fn find_address( + &self, + mut j: DiversifierIndex, + request: UnifiedAddressRequest, + ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { + self.to_unified_incoming_viewing_key()? + .find_address(j, request) + } + + /// Find the Unified Address corresponding to the smallest valid diversifier index, along with + /// that index. + /// + /// Returns an `Err(AddressGenerationError)` if no valid diversifier exists or if the features + /// required to satisfy the unified address request are not properly enabled. + pub fn default_address( + &self, + request: UnifiedAddressRequest, + ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { + self.find_address(DiversifierIndex::new(), request) + } +} + +/// A [ZIP 316](https://zips.z.cash/zip-0316) unified incoming viewing key. +#[derive(Clone, Debug)] +pub struct UnifiedIncomingViewingKey { + #[cfg(feature = "transparent-inputs")] + transparent: Option, + #[cfg(feature = "sapling")] + sapling: Option<::sapling::zip32::IncomingViewingKey>, + #[cfg(feature = "orchard")] + orchard: Option, + /// Stores the unrecognized elements of the unified encoding. + unknown: Vec<(u32, Vec)>, +} + +impl UnifiedIncomingViewingKey { + /// Construct a new unified incoming viewing key, if the required components are present. + pub fn new( + #[cfg(feature = "transparent-inputs")] transparent: Option< + zcash_primitives::legacy::keys::ExternalIvk, + >, + #[cfg(feature = "sapling")] sapling: Option<::sapling::zip32::IncomingViewingKey>, + #[cfg(feature = "orchard")] orchard: Option, + // TODO: Implement construction of UIVKs with metadata items. + ) -> Option { + #[cfg(feature = "orchard")] + let has_orchard = orchard.is_some(); + #[cfg(not(feature = "orchard"))] + let has_orchard = false; + #[cfg(feature = "sapling")] + let has_sapling = sapling.is_some(); + #[cfg(not(feature = "sapling"))] + let has_sapling = false; + + if has_orchard || has_sapling { + Some(UnifiedIncomingViewingKey { + #[cfg(feature = "transparent-inputs")] + transparent, + #[cfg(feature = "sapling")] + sapling, + #[cfg(feature = "orchard")] + orchard, + // We don't allow constructing new UFVKs with unknown items, but we store + // this to allow parsing such UFVKs. + unknown: vec![], + }) + } else { + None + } + } + + /// Constructs a unified incoming viewing key from a parsed unified encoding. + pub fn from_uivk(uivk: &Uivk) -> Result { + #[cfg(feature = "orchard")] + let mut orchard = None; + #[cfg(feature = "sapling")] + let mut sapling = None; + #[cfg(feature = "transparent-inputs")] + let mut transparent = None; + + // We can use as-parsed order here for efficiency, because we're breaking out the + // receivers we support from the unknown receivers. + let unknown = uivk + .items_as_parsed() + .iter() + .filter_map(|receiver| match receiver { + #[cfg(feature = "orchard")] + unified::Ivk::Orchard(data) => { + Option::from(orchard::keys::IncomingViewingKey::from_bytes(data)) + .ok_or(UnifiedError::InvalidShieldedKey(ShieldedProtocol::Orchard)) + .map(|addr| { + orchard = Some(addr); + None + }) + .transpose() + } + #[cfg(not(feature = "orchard"))] + unified::Ivk::Orchard(data) => Some(Ok::<_, UnifiedError>(( + u32::from(unified::Typecode::Orchard), + data.to_vec(), + ))), + #[cfg(feature = "sapling")] + unified::Ivk::Sapling(data) => { + Option::from(::sapling::zip32::IncomingViewingKey::from_bytes(data)) + .ok_or(UnifiedError::InvalidShieldedKey(ShieldedProtocol::Sapling)) + .map(|pa| { + sapling = Some(pa); + None + }) + .transpose() + } + #[cfg(not(feature = "sapling"))] + unified::Ivk::Sapling(data) => Some(Ok::<_, UnifiedError>(( + u32::from(unified::Typecode::Sapling), + data.to_vec(), + ))), + #[cfg(feature = "transparent-inputs")] + unified::Ivk::P2pkh(data) => legacy::ExternalIvk::deserialize(data) + .map_err(UnifiedError::InvalidTransparentKey) + .map(|tivk| { + transparent = Some(tivk); + None + }) + .transpose(), + #[cfg(not(feature = "transparent-inputs"))] + unified::Ivk::P2pkh(data) => Some(Ok::<_, UnifiedError>(( + u32::from(unified::Typecode::P2pkh), + data.to_vec(), + ))), + unified::Ivk::Unknown { typecode, data } => Some(Ok((*typecode, data.clone()))), + }) + .collect::>()?; + + Ok(Self { + #[cfg(feature = "transparent-inputs")] + transparent, + #[cfg(feature = "sapling")] + sapling, + #[cfg(feature = "orchard")] + orchard, + unknown, + }) + } + + /// Converts this unified incoming viewing key to a unified encoding. + pub fn to_uivk(&self) -> Uivk { + let items = std::iter::empty().chain(self.unknown.iter().map(|(typecode, data)| { + unified::Ivk::Unknown { + typecode: *typecode, + data: data.clone(), + } + })); + #[cfg(feature = "orchard")] + let items = items.chain( + self.orchard + .as_ref() + .map(|ivk| ivk.to_bytes()) + .map(unified::Ivk::Orchard), + ); + #[cfg(feature = "sapling")] + let items = items.chain( + self.sapling + .as_ref() + .map(|divk| divk.to_bytes()) + .map(unified::Ivk::Sapling), + ); + #[cfg(feature = "transparent-inputs")] + let items = items.chain( + self.transparent + .as_ref() + .map(|tivk| tivk.serialize().try_into().unwrap()) + .map(unified::Ivk::P2pkh), + ); + + unified::Uivk::try_from_items(items.collect()) + .expect("UnifiedIncomingViewingKey should only be constructed safely.") + } + + /// Returns the Transparent external IVK, if present. + #[cfg(feature = "transparent-inputs")] + pub fn transparent(&self) -> &Option { + &self.transparent + } + + /// Returns the Sapling IVK, if present. + #[cfg(feature = "sapling")] + pub fn sapling(&self) -> &Option<::sapling::zip32::IncomingViewingKey> { + &self.sapling + } + + /// Returns the Orchard IVK, if present. + #[cfg(feature = "orchard")] + pub fn orchard(&self) -> &Option { + &self.orchard + } + /// Attempts to derive the Unified Address for the given diversifier index and /// receiver types. /// @@ -792,20 +1103,20 @@ impl UnifiedFullViewingKey { pub fn address( &self, _j: DiversifierIndex, - _request: UnifiedAddressRequest, + request: UnifiedAddressRequest, ) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; - if _request.has_orchard { + if request.has_orchard { #[cfg(not(feature = "orchard"))] return Err(AddressGenerationError::ReceiverTypeNotSupported( Typecode::Orchard, )); #[cfg(feature = "orchard")] - if let Some(ofvk) = &self.orchard { + if let Some(oivk) = &self.orchard { let orchard_j = orchard::keys::DiversifierIndex::from(*_j.as_bytes()); - orchard = Some(ofvk.address_at(orchard_j, Scope::External)) + orchard = Some(oivk.address_at(orchard_j)) } else { return Err(AddressGenerationError::KeyNotAvailable(Typecode::Orchard)); } @@ -813,20 +1124,19 @@ impl UnifiedFullViewingKey { #[cfg(feature = "sapling")] let mut sapling = None; - if _request.has_sapling { + if request.has_sapling { #[cfg(not(feature = "sapling"))] return Err(AddressGenerationError::ReceiverTypeNotSupported( Typecode::Sapling, )); #[cfg(feature = "sapling")] - if let Some(extfvk) = &self.sapling { + if let Some(divk) = &self.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. sapling = Some( - extfvk - .address(_j) + divk.address_at(_j) .ok_or(AddressGenerationError::InvalidSaplingDiversifierIndex(_j))?, ); } else { @@ -836,14 +1146,14 @@ impl UnifiedFullViewingKey { #[cfg(feature = "transparent-inputs")] let mut transparent = None; - if _request.has_p2pkh { + if request.has_p2pkh { #[cfg(not(feature = "transparent-inputs"))] return Err(AddressGenerationError::ReceiverTypeNotSupported( Typecode::P2pkh, )); #[cfg(feature = "transparent-inputs")] - if let Some(tfvk) = self.transparent.as_ref() { + if let Some(tivk) = self.transparent.as_ref() { // 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. @@ -851,8 +1161,7 @@ impl UnifiedFullViewingKey { .ok_or(AddressGenerationError::InvalidTransparentChildIndex(_j))?; transparent = Some( - tfvk.derive_external_ivk() - .and_then(|tivk| tivk.derive_address(transparent_j)) + tivk.derive_address(transparent_j) .map_err(|_| AddressGenerationError::InvalidTransparentChildIndex(_j))?, ); } else { @@ -953,8 +1262,13 @@ pub mod testing { #[cfg(test)] mod tests { + use crate::keys::UnifiedIncomingViewingKey; + use super::UnifiedFullViewingKey; use proptest::prelude::proptest; + use zcash_address::unified::{Encoding, Uivk}; + #[cfg(feature = "orchard")] + use zip32::Scope; #[cfg(any( feature = "orchard", @@ -1199,10 +1513,198 @@ mod tests { } } + #[test] + fn uivk_round_trip() { + #[cfg(feature = "orchard")] + let orchard = { + let sk = + orchard::keys::SpendingKey::from_zip32_seed(&[0; 32], 0, AccountId::ZERO).unwrap(); + Some(orchard::keys::FullViewingKey::from(&sk).to_ivk(Scope::External)) + }; + + #[cfg(feature = "sapling")] + let sapling = { + let extsk = sapling::spending_key(&[0; 32], 0, AccountId::ZERO); + Some(extsk.to_diversifiable_full_viewing_key().to_external_ivk()) + }; + + #[cfg(feature = "transparent-inputs")] + let transparent = { + let privkey = + AccountPrivKey::from_seed(&MAIN_NETWORK, &[0; 32], AccountId::ZERO).unwrap(); + Some(privkey.to_account_pubkey().derive_external_ivk().unwrap()) + }; + + let uivk = UnifiedIncomingViewingKey::new( + #[cfg(feature = "transparent-inputs")] + transparent, + #[cfg(feature = "sapling")] + sapling, + #[cfg(feature = "orchard")] + orchard, + ); + + #[cfg(not(any(feature = "orchard", feature = "sapling")))] + assert!(uivk.is_none()); + + #[cfg(any(feature = "orchard", feature = "sapling"))] + { + let uivk = uivk.expect("Orchard or Sapling ivk is present."); + let encoded = uivk.to_uivk().encode(&Network::Main); + + // Test encoded form against known values; these test vectors contain Orchard receivers + // that will be treated as unknown if the `orchard` feature is not enabled. + let encoded_with_t = "uivk1z28yg638vjwusmf0zc9ad2j0mpv6s42wc5kqt004aaqfu5xxxgu7mdcydn9qf723fnryt34s6jyxyw0jt7spq04c3v9ze6qe9gjjc5aglz8zv5pqtw58czd0actynww5n85z3052kzgy6cu0fyjafyp4sr4kppyrrwhwev2rr0awq6m8d66esvk6fgacggqnswg5g9gkv6t6fj9ajhyd0gmel4yzscprpzduncc0e2lywufup6fvzf6y8cefez2r99pgge5yyfuus0r60khgu895pln5e7nn77q6s9kh2uwf6lrfu06ma2kd7r05jjvl4hn6nupge8fajh0cazd7mkmz23t79w"; + let _encoded_no_t = "uivk1020vq9j5zeqxh303sxa0zv2hn9wm9fev8x0p8yqxdwyzde9r4c90fcglc63usj0ycl2scy8zxuhtser0qrq356xfy8x3vyuxu7f6gas75svl9v9m3ctuazsu0ar8e8crtx7x6zgh4kw8xm3q4rlkpm9er2wefxhhf9pn547gpuz9vw27gsdp6c03nwlrxgzhr2g6xek0x8l5avrx9ue9lf032tr7kmhqf3nfdxg7ldfgx6yf09g"; + + // We test the full roundtrip only with the `sapling` and `orchard` features enabled, + // because we will not generate these parts of the encoding if the UIVK does not have an + // these parts. + #[cfg(all(feature = "sapling", feature = "orchard"))] + { + #[cfg(feature = "transparent-inputs")] + assert_eq!(encoded, encoded_with_t); + #[cfg(not(feature = "transparent-inputs"))] + assert_eq!(encoded, _encoded_no_t); + } + + let decoded = + UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(&encoded).unwrap().1).unwrap(); + let reencoded = decoded.to_uivk().encode(&Network::Main); + assert_eq!(encoded, reencoded); + + #[cfg(feature = "transparent-inputs")] + assert_eq!( + decoded.transparent.map(|t| t.serialize()), + uivk.transparent.as_ref().map(|t| t.serialize()), + ); + #[cfg(feature = "sapling")] + assert_eq!( + decoded.sapling.map(|s| s.to_bytes()), + uivk.sapling.map(|s| s.to_bytes()), + ); + #[cfg(feature = "orchard")] + assert_eq!( + decoded.orchard.map(|o| o.to_bytes()), + uivk.orchard.map(|o| o.to_bytes()), + ); + + let decoded_with_t = + UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(encoded_with_t).unwrap().1) + .unwrap(); + #[cfg(feature = "transparent-inputs")] + assert_eq!( + decoded_with_t.transparent.map(|t| t.serialize()), + uivk.transparent.as_ref().map(|t| t.serialize()), + ); + + // Both Orchard and Sapling enabled + #[cfg(all( + feature = "orchard", + feature = "sapling", + feature = "transparent-inputs" + ))] + assert_eq!(decoded_with_t.unknown.len(), 0); + #[cfg(all( + feature = "orchard", + feature = "sapling", + not(feature = "transparent-inputs") + ))] + assert_eq!(decoded_with_t.unknown.len(), 1); + + // Orchard enabled + #[cfg(all( + feature = "orchard", + not(feature = "sapling"), + feature = "transparent-inputs" + ))] + assert_eq!(decoded_with_t.unknown.len(), 1); + #[cfg(all( + feature = "orchard", + not(feature = "sapling"), + not(feature = "transparent-inputs") + ))] + assert_eq!(decoded_with_t.unknown.len(), 2); + + // Sapling enabled + #[cfg(all( + not(feature = "orchard"), + feature = "sapling", + feature = "transparent-inputs" + ))] + assert_eq!(decoded_with_t.unknown.len(), 1); + #[cfg(all( + not(feature = "orchard"), + feature = "sapling", + not(feature = "transparent-inputs") + ))] + assert_eq!(decoded_with_t.unknown.len(), 2); + } + } + + #[test] + #[cfg(feature = "transparent-inputs")] + fn uivk_derivation() { + use crate::keys::UnifiedAddressRequest; + + use super::UnifiedSpendingKey; + + for tv in test_vectors::UNIFIED { + let usk = UnifiedSpendingKey::from_seed( + &MAIN_NETWORK, + &tv.root_seed, + AccountId::try_from(tv.account).unwrap(), + ) + .expect("seed produced a valid unified spending key"); + + let d_idx = DiversifierIndex::from(tv.diversifier_index); + let uivk = usk + .to_unified_full_viewing_key() + .to_unified_incoming_viewing_key() + .unwrap(); + + // The test vectors contain some diversifier indices that do not generate + // valid Sapling addresses, so skip those. + #[cfg(feature = "sapling")] + if uivk.sapling().as_ref().unwrap().address_at(d_idx).is_none() { + continue; + } + + let ua = uivk + .address(d_idx, UnifiedAddressRequest::unsafe_new(false, true, true)) + .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)) => { + // We always derive transparent and Sapling receivers, but not + // every value in the test vectors has these present. + if tvua.transparent().is_some() { + assert_eq!(tvua.transparent(), ua.transparent()); + } + #[cfg(feature = "sapling")] + if tvua.sapling().is_some() { + assert_eq!(tvua.sapling(), ua.sapling()); + } + } + _other => { + panic!( + "{} did not decode to a valid unified address", + tv.unified_addr + ); + } + } + } + } + proptest! { #[test] #[cfg(feature = "unstable")] - fn prop_usk_roundtrip(usk in arb_unified_spending_key(Network::MainNetwork)) { + fn prop_usk_roundtrip(usk in arb_unified_spending_key(zcash_protocol::consensus::Network::MainNetwork)) { let encoded = usk.to_bytes(Era::Orchard); let encoded_len = { From 2b3060d138bcac207f7af2cac0948c4fad65a4ba Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Sat, 9 Mar 2024 21:22:25 -0700 Subject: [PATCH 2/8] Apply updated style for key discovery Co-authored-by: Kris Nuttycombe --- zcash_keys/src/keys.rs | 91 +++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index c48e12a1e..8eefd34f0 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -980,58 +980,57 @@ impl UnifiedIncomingViewingKey { #[cfg(feature = "transparent-inputs")] let mut transparent = None; + let mut unknown = vec![]; + // We can use as-parsed order here for efficiency, because we're breaking out the // receivers we support from the unknown receivers. - let unknown = uivk - .items_as_parsed() - .iter() - .filter_map(|receiver| match receiver { - #[cfg(feature = "orchard")] + for receiver in uivk.items_as_parsed() { + match receiver { unified::Ivk::Orchard(data) => { - Option::from(orchard::keys::IncomingViewingKey::from_bytes(data)) - .ok_or(UnifiedError::InvalidShieldedKey(ShieldedProtocol::Orchard)) - .map(|addr| { - orchard = Some(addr); - None - }) - .transpose() + #[cfg(feature = "orchard")] + { + orchard = Some( + Option::from(orchard::keys::IncomingViewingKey::from_bytes(data)) + .ok_or(UnifiedError::InvalidShieldedKey( + ShieldedProtocol::Orchard, + ))?, + ); + } + + #[cfg(not(feature = "orchard"))] + unknown.push((u32::from(unified::Typecode::Orchard), data.to_vec())); } - #[cfg(not(feature = "orchard"))] - unified::Ivk::Orchard(data) => Some(Ok::<_, UnifiedError>(( - u32::from(unified::Typecode::Orchard), - data.to_vec(), - ))), - #[cfg(feature = "sapling")] unified::Ivk::Sapling(data) => { - Option::from(::sapling::zip32::IncomingViewingKey::from_bytes(data)) - .ok_or(UnifiedError::InvalidShieldedKey(ShieldedProtocol::Sapling)) - .map(|pa| { - sapling = Some(pa); - None - }) - .transpose() + #[cfg(feature = "sapling")] + { + sapling = Some( + Option::from(::sapling::zip32::IncomingViewingKey::from_bytes(data)) + .ok_or(UnifiedError::InvalidShieldedKey( + ShieldedProtocol::Sapling, + ))?, + ); + } + + #[cfg(not(feature = "sapling"))] + unknown.push((u32::from(unified::Typecode::Sapling), data.to_vec())); } - #[cfg(not(feature = "sapling"))] - unified::Ivk::Sapling(data) => Some(Ok::<_, UnifiedError>(( - u32::from(unified::Typecode::Sapling), - data.to_vec(), - ))), - #[cfg(feature = "transparent-inputs")] - unified::Ivk::P2pkh(data) => legacy::ExternalIvk::deserialize(data) - .map_err(UnifiedError::InvalidTransparentKey) - .map(|tivk| { - transparent = Some(tivk); - None - }) - .transpose(), - #[cfg(not(feature = "transparent-inputs"))] - unified::Ivk::P2pkh(data) => Some(Ok::<_, UnifiedError>(( - u32::from(unified::Typecode::P2pkh), - data.to_vec(), - ))), - unified::Ivk::Unknown { typecode, data } => Some(Ok((*typecode, data.clone()))), - }) - .collect::>()?; + unified::Ivk::P2pkh(data) => { + #[cfg(feature = "transparent-inputs")] + { + transparent = Some( + legacy::ExternalIvk::deserialize(data) + .map_err(UnifiedError::InvalidTransparentKey)?, + ); + } + + #[cfg(not(feature = "transparent-inputs"))] + unknown.push((u32::from(unified::Typecode::P2pkh), data.to_vec())); + } + unified::Ivk::Unknown { typecode, data } => { + unknown.push((*typecode, data.clone())); + } + } + } Ok(Self { #[cfg(feature = "transparent-inputs")] From c99338a7a134483dcff735a3d6a8c7bfc66d9b6d Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Wed, 13 Mar 2024 18:59:37 -0600 Subject: [PATCH 3/8] Merge new error type into existing one --- zcash_client_sqlite/src/wallet.rs | 23 +++---- zcash_keys/CHANGELOG.md | 5 +- zcash_keys/src/keys.rs | 105 ++++++++++++------------------ 3 files changed, 54 insertions(+), 79 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index c52a47b39..22ba9c066 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -298,14 +298,13 @@ struct AccountSqlValues<'a> { hd_seed_fingerprint: Option<&'a [u8]>, hd_account_index: Option, ufvk: Option<&'a UnifiedFullViewingKey>, - uivk: String, + uivk: UnifiedIncomingViewingKey, } /// Returns (account_type, hd_seed_fingerprint, hd_account_index, ufvk, uivk) for a given account. -fn get_sql_values_for_account_parameters<'a, P: consensus::Parameters>( - account: &'a Account, - params: &P, -) -> Result, SqliteClientError> { +fn get_sql_values_for_account_parameters( + account: &Account, +) -> Result { Ok(match account { Account::Zip32(hdaccount) => AccountSqlValues { account_type: AccountType::Zip32.into(), @@ -315,9 +314,7 @@ fn get_sql_values_for_account_parameters<'a, P: consensus::Parameters>( uivk: hdaccount .ufvk() .to_unified_incoming_viewing_key() - .map_err(|e| SqliteClientError::CorruptedData(e.to_string()))? - .to_uivk() - .encode(¶ms.network_type()), + .map_err(|e| SqliteClientError::CorruptedData(e.to_string()))?, }, Account::Imported(ImportedAccount::Full(ufvk)) => AccountSqlValues { account_type: AccountType::Imported.into(), @@ -326,16 +323,14 @@ fn get_sql_values_for_account_parameters<'a, P: consensus::Parameters>( ufvk: Some(ufvk), uivk: ufvk .to_unified_incoming_viewing_key() - .map_err(|e| SqliteClientError::CorruptedData(e.to_string()))? - .to_uivk() - .encode(¶ms.network_type()), + .map_err(|e| SqliteClientError::CorruptedData(e.to_string()))?, }, Account::Imported(ImportedAccount::Incoming(uivk)) => AccountSqlValues { account_type: AccountType::Imported.into(), hd_seed_fingerprint: None, hd_account_index: None, ufvk: None, - uivk: uivk.to_uivk().encode(¶ms.network_type()), + uivk: (**uivk).clone(), }, }) } @@ -346,7 +341,7 @@ pub(crate) fn add_account( account: Account, birthday: AccountBirthday, ) -> Result { - let args = get_sql_values_for_account_parameters(&account, params)?; + let args = get_sql_values_for_account_parameters(&account)?; let orchard_item = args .ufvk @@ -382,7 +377,7 @@ pub(crate) fn add_account( ":hd_seed_fingerprint": args.hd_seed_fingerprint, ":hd_account_index": args.hd_account_index, ":ufvk": args.ufvk.map(|ufvk| ufvk.encode(params)), - ":uivk": args.uivk, + ":uivk": args.uivk.to_uivk().encode(¶ms.network_type()), ":orchard_fvk_item_cache": orchard_item, ":sapling_fvk_item_cache": sapling_item, ":p2pkh_fvk_item_cache": transparent_item, diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 08a1f9f57..65763612d 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -11,7 +11,6 @@ and this library adheres to Rust's notion of - `zcash_keys::address::Address::has_receiver` - `impl Display for zcash_keys::keys::AddressGenerationError` - `impl std::error::Error for zcash_keys::keys::AddressGenerationError` -- `zcash_keys::keys::UnifiedError` - `zcash_keys::keys::UnifiedFullViewingKey::from_ufvk` - `zcash_keys::keys::UnifiedFullViewingKey::to_ufvk` - `zcash_keys::keys::UnifiedFullViewingKey::to_unified_incoming_viewing_key` @@ -25,8 +24,8 @@ and this library adheres to Rust's notion of (instead of `Option<(UnifiedAddress, DiversifierIndex)>` for `find_address`). - `zcash_keys::keys::AddressGenerationError` - Dropped `Clone` trait - - Added `UnifiedError` variant. - - Added `HDWalletError` variant. +- `zcash_keys::keys::DerivationError` + - Added `InvalidShieldedKey` variant. ### Fixed - `UnifiedFullViewingKey::find_address` can now find an address for a diversifier diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 9bffc6be0..c6d389c37 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -1,7 +1,10 @@ //! Helper functions for managing light client key material. use blake2b_simd::Params as blake2bParams; use secrecy::{ExposeSecret, SecretVec}; -use std::{error, fmt}; +use std::{ + error, + fmt::{self, Display}, +}; use zcash_address::unified::{self, Container, Encoding, Typecode, Ufvk, Uivk}; use zcash_protocol::{consensus, ShieldedProtocol}; @@ -138,12 +141,27 @@ fn to_transparent_child_index(j: DiversifierIndex) -> Option) -> fmt::Result { + match self { + DerivationError::InvalidShieldedKey(protocol) => { + write!(f, "Invalid shielded key for protocol {:?}", protocol) + } + #[cfg(feature = "orchard")] + DerivationError::Orchard(e) => write!(f, "Orchard error: {}", e), + #[cfg(feature = "transparent-inputs")] + DerivationError::Transparent(e) => write!(f, "Transparent error: {}", e), + } + } +} + /// A version identifier for the encoding of unified spending keys. /// /// Each era corresponds to a range of block heights. During an era, the unified spending key @@ -498,23 +516,19 @@ pub enum AddressGenerationError { ShieldedReceiverRequired, // An error occurred while deriving a key or address from an HD wallet. - UnifiedError(UnifiedError), - - // An error occurred while deriving a transparent key or address from an HD wallet. - #[cfg(feature = "transparent-inputs")] - HDWalletError(hdwallet::error::Error), -} - -impl From for AddressGenerationError { - fn from(e: UnifiedError) -> Self { - AddressGenerationError::UnifiedError(e) - } + Derivation(DerivationError), } #[cfg(feature = "transparent-inputs")] impl From for AddressGenerationError { fn from(e: hdwallet::error::Error) -> Self { - AddressGenerationError::HDWalletError(e) + AddressGenerationError::Derivation(DerivationError::Transparent(e)) + } +} + +impl From for AddressGenerationError { + fn from(e: DerivationError) -> Self { + AddressGenerationError::Derivation(e) } } @@ -560,13 +574,7 @@ impl fmt::Display for AddressGenerationError { AddressGenerationError::ShieldedReceiverRequired => { write!(f, "A Unified Address requires at least one shielded (Sapling or Orchard) receiver.") } - AddressGenerationError::UnifiedError(e) => { - write!(f, "UnifiedError: {}", e) - } - #[cfg(feature = "transparent-inputs")] - AddressGenerationError::HDWalletError(e) => { - write!(f, "HDWalletError: {}", e) - } + AddressGenerationError::Derivation(e) => write!(f, "Error deriving address: {}", e), } } } @@ -633,38 +641,10 @@ impl UnifiedAddressRequest { } } -/// Errors that may be returned from the [UnifiedFullViewingKey] and [UnifiedIncomingViewingKey] types. -#[derive(Debug)] -pub enum UnifiedError { - InvalidShieldedKey(ShieldedProtocol), - #[cfg(feature = "transparent-inputs")] - InvalidTransparentKey(hdwallet::error::Error), - #[cfg(feature = "transparent-inputs")] - HDWallet(hdwallet::error::Error), -} - #[cfg(feature = "transparent-inputs")] -impl From for UnifiedError { +impl From for DerivationError { fn from(e: hdwallet::error::Error) -> Self { - UnifiedError::HDWallet(e) - } -} - -impl fmt::Display for UnifiedError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match &self { - UnifiedError::InvalidShieldedKey(p) => { - write!(f, "Invalid key for shielded protocol: {:?}", p) - } - #[cfg(feature = "transparent-inputs")] - UnifiedError::InvalidTransparentKey(e) => { - write!(f, "Invalid key for transparent protocol: {:?}", e) - } - #[cfg(feature = "transparent-inputs")] - UnifiedError::HDWallet(e) => { - write!(f, "HDWallet error: {}", e) - } - } + DerivationError::Transparent(e) } } @@ -734,7 +714,7 @@ impl UnifiedFullViewingKey { /// Parses a `UnifiedFullViewingKey` from its [ZIP 316] string encoding. /// /// [ZIP 316]: https://zips.z.cash/zip-0316 - pub fn from_ufvk(ufvk: &Ufvk) -> Result { + pub fn from_ufvk(ufvk: &Ufvk) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; #[cfg(feature = "sapling")] @@ -750,7 +730,9 @@ impl UnifiedFullViewingKey { .filter_map(|receiver| match receiver { #[cfg(feature = "orchard")] unified::Fvk::Orchard(data) => orchard::keys::FullViewingKey::from_bytes(data) - .ok_or(UnifiedError::InvalidShieldedKey(ShieldedProtocol::Orchard)) + .ok_or(DerivationError::InvalidShieldedKey( + ShieldedProtocol::Orchard, + )) .map(|addr| { orchard = Some(addr); None @@ -764,7 +746,9 @@ impl UnifiedFullViewingKey { #[cfg(feature = "sapling")] unified::Fvk::Sapling(data) => { sapling::DiversifiableFullViewingKey::from_bytes(data) - .ok_or(UnifiedError::InvalidShieldedKey(ShieldedProtocol::Sapling)) + .ok_or(DerivationError::InvalidShieldedKey( + ShieldedProtocol::Sapling, + )) .map(|pa| { sapling = Some(pa); None @@ -778,7 +762,7 @@ impl UnifiedFullViewingKey { ))), #[cfg(feature = "transparent-inputs")] unified::Fvk::P2pkh(data) => legacy::AccountPubKey::deserialize(data) - .map_err(UnifiedError::InvalidTransparentKey) + .map_err(DerivationError::Transparent) .map(|tfvk| { transparent = Some(tfvk); None @@ -846,7 +830,7 @@ impl UnifiedFullViewingKey { /// Derives a Unified Incoming Viewing Key from this Unified Full Viewing Key. pub fn to_unified_incoming_viewing_key( &self, - ) -> Result { + ) -> Result { Ok(UnifiedIncomingViewingKey { #[cfg(feature = "transparent-inputs")] transparent: self @@ -972,7 +956,7 @@ impl UnifiedIncomingViewingKey { } /// Constructs a unified incoming viewing key from a parsed unified encoding. - pub fn from_uivk(uivk: &Uivk) -> Result { + pub fn from_uivk(uivk: &Uivk) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; #[cfg(feature = "sapling")] @@ -991,7 +975,7 @@ impl UnifiedIncomingViewingKey { { orchard = Some( Option::from(orchard::keys::IncomingViewingKey::from_bytes(data)) - .ok_or(UnifiedError::InvalidShieldedKey( + .ok_or(DerivationError::InvalidShieldedKey( ShieldedProtocol::Orchard, ))?, ); @@ -1005,7 +989,7 @@ impl UnifiedIncomingViewingKey { { sapling = Some( Option::from(::sapling::zip32::IncomingViewingKey::from_bytes(data)) - .ok_or(UnifiedError::InvalidShieldedKey( + .ok_or(DerivationError::InvalidShieldedKey( ShieldedProtocol::Sapling, ))?, ); @@ -1017,10 +1001,7 @@ impl UnifiedIncomingViewingKey { unified::Ivk::P2pkh(data) => { #[cfg(feature = "transparent-inputs")] { - transparent = Some( - legacy::ExternalIvk::deserialize(data) - .map_err(UnifiedError::InvalidTransparentKey)?, - ); + transparent = Some(legacy::ExternalIvk::deserialize(data)?); } #[cfg(not(feature = "transparent-inputs"))] From 9d6a8b69417a15a7339cf70b9e448318a82f8053 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 13 Mar 2024 20:14:43 -0600 Subject: [PATCH 4/8] zcash_keys: Use `DecodingError` instead of `DerivationError` for key parsing. --- zcash_client_sqlite/src/wallet.rs | 3 +- zcash_keys/CHANGELOG.md | 7 ++- zcash_keys/src/keys.rs | 81 ++++++++++++++++++------------- 3 files changed, 52 insertions(+), 39 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index bdcdfddb8..5f11ebb9b 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -74,7 +74,7 @@ use std::io::{self, Cursor}; use std::num::NonZeroU32; use std::ops::RangeInclusive; use tracing::debug; -use zcash_address::unified::{Encoding, Ivk, Uivk}; +use zcash_address::unified::{Encoding, Uivk}; use zcash_keys::keys::{ AddressGenerationError, HdSeedFingerprint, UnifiedAddressRequest, UnifiedIncomingViewingKey, }; @@ -120,6 +120,7 @@ use { crate::UtxoId, rusqlite::Row, std::collections::BTreeSet, + zcash_address::unified::Ivk, zcash_client_backend::wallet::{TransparentAddressMetadata, WalletTransparentOutput}, zcash_primitives::{ legacy::{ diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 65763612d..e29d0fe90 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -11,21 +11,20 @@ and this library adheres to Rust's notion of - `zcash_keys::address::Address::has_receiver` - `impl Display for zcash_keys::keys::AddressGenerationError` - `impl std::error::Error for zcash_keys::keys::AddressGenerationError` +- `zcash_keys::keys::DecodingError` - `zcash_keys::keys::UnifiedFullViewingKey::from_ufvk` - `zcash_keys::keys::UnifiedFullViewingKey::to_ufvk` - `zcash_keys::keys::UnifiedFullViewingKey::to_unified_incoming_viewing_key` - `zcash_keys::keys::UnifiedIncomingViewingKey` ### 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)>` for `find_address`). - `zcash_keys::keys::AddressGenerationError` - Dropped `Clone` trait -- `zcash_keys::keys::DerivationError` - - Added `InvalidShieldedKey` variant. + - Added `KeyDecoding` variant. + - Added `DiversifierSpaceExhausted` variant. ### Fixed - `UnifiedFullViewingKey::find_address` can now find an address for a diversifier diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 350ae858d..0120d6167 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -7,7 +7,7 @@ use std::{ }; use zcash_address::unified::{self, Container, Encoding, Typecode, Ufvk, Uivk}; -use zcash_protocol::{consensus, ShieldedProtocol}; +use zcash_protocol::consensus; use zip32::{AccountId, DiversifierIndex}; use crate::address::UnifiedAddress; @@ -139,9 +139,7 @@ fn to_transparent_child_index(j: DiversifierIndex) -> Option) -> fmt::Result { match self { - DerivationError::InvalidShieldedKey(protocol) => { - write!(f, "Invalid shielded key for protocol {:?}", protocol) - } #[cfg(feature = "orchard")] DerivationError::Orchard(e) => write!(f, "Orchard error: {}", e), #[cfg(feature = "transparent-inputs")] @@ -177,28 +172,40 @@ pub enum Era { } /// A type for errors that can occur when decoding keys from their serialized representations. -#[cfg(feature = "unstable")] #[derive(Debug, PartialEq, Eq)] pub enum DecodingError { + #[cfg(feature = "unstable")] ReadError(&'static str), + #[cfg(feature = "unstable")] EraInvalid, + #[cfg(feature = "unstable")] EraMismatch(Era), + #[cfg(feature = "unstable")] TypecodeInvalid, + #[cfg(feature = "unstable")] LengthInvalid, + #[cfg(feature = "unstable")] LengthMismatch(Typecode, u32), + #[cfg(feature = "unstable")] InsufficientData(Typecode), + /// The key data could not be decoded from its string representation to a valid key. KeyDataInvalid(Typecode), } -#[cfg(feature = "unstable")] impl std::fmt::Display for DecodingError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { + #[cfg(feature = "unstable")] DecodingError::ReadError(s) => write!(f, "Read error: {}", s), + #[cfg(feature = "unstable")] DecodingError::EraInvalid => write!(f, "Invalid era"), + #[cfg(feature = "unstable")] DecodingError::EraMismatch(e) => write!(f, "Era mismatch: actual {:?}", e), + #[cfg(feature = "unstable")] DecodingError::TypecodeInvalid => write!(f, "Invalid typecode"), + #[cfg(feature = "unstable")] DecodingError::LengthInvalid => write!(f, "Invalid length"), + #[cfg(feature = "unstable")] DecodingError::LengthMismatch(t, l) => { write!( f, @@ -206,10 +213,11 @@ impl std::fmt::Display for DecodingError { l, t ) } + #[cfg(feature = "unstable")] DecodingError::InsufficientData(t) => { write!(f, "Insufficient data for typecode {:?}", t) } - DecodingError::KeyDataInvalid(t) => write!(f, "Invalid key data for typecode {:?}", t), + DecodingError::KeyDataInvalid(t) => write!(f, "Invalid key data for key type {:?}", t), } } } @@ -514,8 +522,10 @@ pub enum AddressGenerationError { /// A Unified address cannot be generated without at least one shielded receiver being /// included. ShieldedReceiverRequired, - // An error occurred while deriving a key or address from an HD wallet. + /// An error occurred while deriving a key or address from an HD wallet. Derivation(DerivationError), + /// An error occurred while decoding a key from its encoded representation. + KeyDecoding(DecodingError), } #[cfg(feature = "transparent-inputs")] @@ -531,6 +541,12 @@ impl From for AddressGenerationError { } } +impl From for AddressGenerationError { + fn from(e: DecodingError) -> Self { + AddressGenerationError::KeyDecoding(e) + } +} + impl fmt::Display for AddressGenerationError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match &self { @@ -574,6 +590,11 @@ impl fmt::Display for AddressGenerationError { write!(f, "A Unified Address requires at least one shielded (Sapling or Orchard) receiver.") } AddressGenerationError::Derivation(e) => write!(f, "Error deriving address: {}", e), + AddressGenerationError::KeyDecoding(e) => write!( + f, + "Error decoding key from its serialized representation: {}.", + e + ), } } } @@ -713,7 +734,7 @@ impl UnifiedFullViewingKey { /// Parses a `UnifiedFullViewingKey` from its [ZIP 316] string encoding. /// /// [ZIP 316]: https://zips.z.cash/zip-0316 - pub fn from_ufvk(ufvk: &Ufvk) -> Result { + pub fn from_ufvk(ufvk: &Ufvk) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; #[cfg(feature = "sapling")] @@ -729,25 +750,21 @@ impl UnifiedFullViewingKey { .filter_map(|receiver| match receiver { #[cfg(feature = "orchard")] unified::Fvk::Orchard(data) => orchard::keys::FullViewingKey::from_bytes(data) - .ok_or(DerivationError::InvalidShieldedKey( - ShieldedProtocol::Orchard, - )) + .ok_or(DecodingError::KeyDataInvalid(Typecode::Orchard)) .map(|addr| { orchard = Some(addr); None }) .transpose(), #[cfg(not(feature = "orchard"))] - unified::Fvk::Orchard(data) => Some(Ok::<_, DerivationError>(( + unified::Fvk::Orchard(data) => Some(Ok::<_, DecodingError>(( u32::from(unified::Typecode::Orchard), data.to_vec(), ))), #[cfg(feature = "sapling")] unified::Fvk::Sapling(data) => { sapling::DiversifiableFullViewingKey::from_bytes(data) - .ok_or(DerivationError::InvalidShieldedKey( - ShieldedProtocol::Sapling, - )) + .ok_or(DecodingError::KeyDataInvalid(Typecode::Sapling)) .map(|pa| { sapling = Some(pa); None @@ -755,20 +772,20 @@ impl UnifiedFullViewingKey { .transpose() } #[cfg(not(feature = "sapling"))] - unified::Fvk::Sapling(data) => Some(Ok::<_, DerivationError>(( + unified::Fvk::Sapling(data) => Some(Ok::<_, DecodingError>(( u32::from(unified::Typecode::Sapling), data.to_vec(), ))), #[cfg(feature = "transparent-inputs")] unified::Fvk::P2pkh(data) => legacy::AccountPubKey::deserialize(data) - .map_err(DerivationError::Transparent) + .map_err(|_| DecodingError::KeyDataInvalid(Typecode::P2pkh)) .map(|tfvk| { transparent = Some(tfvk); None }) .transpose(), #[cfg(not(feature = "transparent-inputs"))] - unified::Fvk::P2pkh(data) => Some(Ok::<_, DerivationError>(( + unified::Fvk::P2pkh(data) => Some(Ok::<_, DecodingError>(( u32::from(unified::Typecode::P2pkh), data.to_vec(), ))), @@ -955,7 +972,7 @@ impl UnifiedIncomingViewingKey { } /// Constructs a unified incoming viewing key from a parsed unified encoding. - pub fn from_uivk(uivk: &Uivk) -> Result { + pub fn from_uivk(uivk: &Uivk) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; #[cfg(feature = "sapling")] @@ -974,9 +991,7 @@ impl UnifiedIncomingViewingKey { { orchard = Some( Option::from(orchard::keys::IncomingViewingKey::from_bytes(data)) - .ok_or(DerivationError::InvalidShieldedKey( - ShieldedProtocol::Orchard, - ))?, + .ok_or(DecodingError::KeyDataInvalid(Typecode::Orchard))?, ); } @@ -988,9 +1003,7 @@ impl UnifiedIncomingViewingKey { { sapling = Some( Option::from(::sapling::zip32::IncomingViewingKey::from_bytes(data)) - .ok_or(DerivationError::InvalidShieldedKey( - ShieldedProtocol::Sapling, - ))?, + .ok_or(DecodingError::KeyDataInvalid(Typecode::Sapling))?, ); } @@ -1000,7 +1013,10 @@ impl UnifiedIncomingViewingKey { unified::Ivk::P2pkh(data) => { #[cfg(feature = "transparent-inputs")] { - transparent = Some(legacy::ExternalIvk::deserialize(data)?); + transparent = Some( + legacy::ExternalIvk::deserialize(data) + .map_err(|_| DecodingError::KeyDataInvalid(Typecode::P2pkh))?, + ); } #[cfg(not(feature = "transparent-inputs"))] @@ -1271,10 +1287,7 @@ mod tests { }; #[cfg(feature = "unstable")] - use { - super::{testing::arb_unified_spending_key, Era, UnifiedSpendingKey}, - zcash_primitives::consensus::Network, - }; + use super::{testing::arb_unified_spending_key, Era, UnifiedSpendingKey}; #[cfg(all(feature = "orchard", feature = "unstable"))] use subtle::ConstantTimeEq; @@ -1551,7 +1564,7 @@ mod tests { let decoded = UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(&encoded).unwrap().1).unwrap(); - let reencoded = decoded.to_uivk().encode(&Network::Main); + let reencoded = decoded.to_uivk().encode(&NetworkType::Main); assert_eq!(encoded, reencoded); #[cfg(feature = "transparent-inputs")] From 4d9927b993b10ae407952efd1bcbc423eb532b22 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 14 Mar 2024 08:27:49 -0600 Subject: [PATCH 5/8] zcash_keys: Verify the ability to derive addresses at USK and UFVK construction. --- zcash_client_sqlite/src/wallet.rs | 10 +- .../init/migrations/full_account_ids.rs | 1 - zcash_keys/CHANGELOG.md | 11 +- zcash_keys/src/keys.rs | 229 +++++++++--------- 4 files changed, 122 insertions(+), 129 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 5f11ebb9b..e73d6dd71 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -235,12 +235,10 @@ impl ViewingKey { } } - fn uivk(&self) -> Result { + fn uivk(&self) -> UnifiedIncomingViewingKey { match self { - ViewingKey::Full(ufvk) => Ok(ufvk - .to_unified_incoming_viewing_key() - .map_err(|e| SqliteClientError::CorruptedData(e.to_string()))?), - ViewingKey::Incoming(uivk) => Ok((**uivk).to_owned()), + ViewingKey::Full(ufvk) => ufvk.as_ref().to_unified_incoming_viewing_key(), + ViewingKey::Incoming(uivk) => uivk.as_ref().clone(), } } } @@ -349,7 +347,7 @@ pub(crate) fn add_account( ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.as_bytes()), ":hd_account_index": hd_account_index.map(u32::from), ":ufvk": viewing_key.ufvk().map(|ufvk| ufvk.encode(params)), - ":uivk": viewing_key.uivk()?.to_uivk().encode(¶ms.network_type()), + ":uivk": viewing_key.uivk().to_uivk().encode(¶ms.network_type()), ":orchard_fvk_item_cache": orchard_item, ":sapling_fvk_item_cache": sapling_item, ":p2pkh_fvk_item_cache": transparent_item, diff --git a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs index 089f799e9..07689421d 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs @@ -122,7 +122,6 @@ impl RusqliteMigration for Migration

{ let uivk = ufvk_parsed .to_unified_incoming_viewing_key() - .map_err(|e| WalletMigrationError::CorruptedData(e.to_string()))? .to_uivk() .encode(&self.params.network_type()); diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index e29d0fe90..93c627aa6 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -18,14 +18,17 @@ and this library adheres to Rust's notion of - `zcash_keys::keys::UnifiedIncomingViewingKey` ### Changed -- `zcash_keys::keys::UnifiedFullViewingKey::{find_address, default_address}` +- `zcash_keys::keys::UnifiedFullViewingKey::{find_address, default_address}` now return `Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError>` (instead of `Option<(UnifiedAddress, DiversifierIndex)>` for `find_address`). - `zcash_keys::keys::AddressGenerationError` - - Dropped `Clone` trait - - Added `KeyDecoding` variant. - Added `DiversifierSpaceExhausted` variant. +### Removed +- `UnifiedFullViewingKey::new` has been placed behind the `test-dependencies` + feature flag. UFVKs should only be produced by derivation from the USK, or + parsed from their string representation. + ### Fixed - `UnifiedFullViewingKey::find_address` can now find an address for a diversifier index outside the valid transparent range if you aren't requesting a @@ -37,7 +40,7 @@ and this library adheres to Rust's notion of - `zcash_keys::keys::UnifiedAddressRequest::all` ### Fixed -- A missing application of the `sapling` feature flag was remedied; +- A missing application of the `sapling` feature flag was remedied; prior to this fix it was not possible to use this crate without the `sapling` feature enabled. diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 0120d6167..a5dbbe368 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -7,7 +7,7 @@ use std::{ }; use zcash_address::unified::{self, Container, Encoding, Typecode, Ufvk, Uivk}; -use zcash_protocol::consensus; +use zcash_protocol::consensus::{self, NetworkConstants}; use zip32::{AccountId, DiversifierIndex}; use crate::address::UnifiedAddress; @@ -18,10 +18,6 @@ use { zcash_primitives::legacy::keys::{self as legacy, IncomingViewingKey, NonHardenedChildIndex}, }; -#[cfg(any(feature = "sapling", feature = "orchard"))] -// Your code here -use zcash_protocol::consensus::NetworkConstants; - #[cfg(all( feature = "transparent-inputs", any(test, feature = "test-dependencies") @@ -264,19 +260,37 @@ impl UnifiedSpendingKey { panic!("ZIP 32 seeds MUST be at least 32 bytes"); } - Ok(UnifiedSpendingKey { + UnifiedSpendingKey::from_checked_parts( #[cfg(feature = "transparent-inputs")] - transparent: legacy::AccountPrivKey::from_seed(_params, seed, _account) + legacy::AccountPrivKey::from_seed(_params, seed, _account) .map_err(DerivationError::Transparent)?, #[cfg(feature = "sapling")] - sapling: sapling::spending_key(seed, _params.coin_type(), _account), + sapling::spending_key(seed, _params.coin_type(), _account), #[cfg(feature = "orchard")] - orchard: orchard::keys::SpendingKey::from_zip32_seed( - seed, - _params.coin_type(), - _account, - ) - .map_err(DerivationError::Orchard)?, + orchard::keys::SpendingKey::from_zip32_seed(seed, _params.coin_type(), _account) + .map_err(DerivationError::Orchard)?, + ) + } + + /// Construct a USK from its constituent parts, after verifying that UIVK derivation can + /// succeed. + fn from_checked_parts( + #[cfg(feature = "transparent-inputs")] transparent: legacy::AccountPrivKey, + #[cfg(feature = "sapling")] sapling: sapling::ExtendedSpendingKey, + #[cfg(feature = "orchard")] orchard: orchard::keys::SpendingKey, + ) -> Result { + // Verify that FVK and IVK derivation succeed; we don't want to construct a USK + // that can't derive transparent addresses. + #[cfg(feature = "transparent-inputs")] + let _ = transparent.to_account_pubkey().derive_external_ivk()?; + + Ok(UnifiedSpendingKey { + #[cfg(feature = "transparent-inputs")] + transparent, + #[cfg(feature = "sapling")] + sapling, + #[cfg(feature = "orchard")] + orchard, }) } @@ -466,14 +480,15 @@ impl UnifiedSpendingKey { let has_transparent = true; if has_orchard && has_sapling && has_transparent { - return Ok(UnifiedSpendingKey { - #[cfg(feature = "orchard")] - orchard: orchard.unwrap(), - #[cfg(feature = "sapling")] - sapling: sapling.unwrap(), + return UnifiedSpendingKey::from_checked_parts( #[cfg(feature = "transparent-inputs")] - transparent: transparent.unwrap(), - }); + transparent.unwrap(), + #[cfg(feature = "sapling")] + sapling.unwrap(), + #[cfg(feature = "orchard")] + orchard.unwrap(), + ) + .map_err(|_| DecodingError::KeyDataInvalid(Typecode::P2pkh)); } } } @@ -502,7 +517,7 @@ impl UnifiedSpendingKey { } /// Errors that can occur in the generation of unified addresses. -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum AddressGenerationError { /// The requested diversifier index was outside the range of valid transparent /// child address indices. @@ -522,29 +537,6 @@ pub enum AddressGenerationError { /// A Unified address cannot be generated without at least one shielded receiver being /// included. ShieldedReceiverRequired, - /// An error occurred while deriving a key or address from an HD wallet. - Derivation(DerivationError), - /// An error occurred while decoding a key from its encoded representation. - KeyDecoding(DecodingError), -} - -#[cfg(feature = "transparent-inputs")] -impl From for AddressGenerationError { - fn from(e: hdwallet::error::Error) -> Self { - AddressGenerationError::Derivation(DerivationError::Transparent(e)) - } -} - -impl From for AddressGenerationError { - fn from(e: DerivationError) -> Self { - AddressGenerationError::Derivation(e) - } -} - -impl From for AddressGenerationError { - fn from(e: DecodingError) -> Self { - AddressGenerationError::KeyDecoding(e) - } } impl fmt::Display for AddressGenerationError { @@ -589,12 +581,6 @@ impl fmt::Display for AddressGenerationError { AddressGenerationError::ShieldedReceiverRequired => { write!(f, "A Unified Address requires at least one shielded (Sapling or Orchard) receiver.") } - AddressGenerationError::Derivation(e) => write!(f, "Error deriving address: {}", e), - AddressGenerationError::KeyDecoding(e) => write!( - f, - "Error decoding key from its serialized representation: {}.", - e - ), } } } @@ -682,37 +668,56 @@ pub struct UnifiedFullViewingKey { #[doc(hidden)] impl UnifiedFullViewingKey { - /// Construct a new unified full viewing key, if the required components are present. + /// Construct a new unified full viewing key. + /// + /// This method is only available when the `test-dependencies` feature is enabled, + /// as derivation from the USK or deserialization from the serialized form should + /// be used instead. + #[cfg(any(test, feature = "test-dependencies"))] pub fn new( #[cfg(feature = "transparent-inputs")] transparent: Option, #[cfg(feature = "sapling")] sapling: Option, #[cfg(feature = "orchard")] orchard: Option, // TODO: Implement construction of UFVKs with metadata items. - ) -> Option { - #[cfg(feature = "orchard")] - let has_orchard = orchard.is_some(); - #[cfg(not(feature = "orchard"))] - let has_orchard = false; - #[cfg(feature = "sapling")] - let has_sapling = sapling.is_some(); - #[cfg(not(feature = "sapling"))] - let has_sapling = false; + ) -> Result { + Self::from_checked_parts( + #[cfg(feature = "transparent-inputs")] + transparent, + #[cfg(feature = "sapling")] + sapling, + #[cfg(feature = "orchard")] + orchard, + // We don't currently allow constructing new UFVKs with unknown items, but we store + // this to allow parsing such UFVKs. + vec![], + ) + } - if has_orchard || has_sapling { - Some(UnifiedFullViewingKey { - #[cfg(feature = "transparent-inputs")] - transparent, - #[cfg(feature = "sapling")] - sapling, - #[cfg(feature = "orchard")] - orchard, - // We don't allow constructing new UFVKs with unknown items, but we store - // this to allow parsing such UFVKs. - unknown: vec![], - }) - } else { - None - } + /// Construct a UFVK from its constituent parts, after verifying that UIVK derivation can + /// succeed. + fn from_checked_parts( + #[cfg(feature = "transparent-inputs")] transparent: Option, + #[cfg(feature = "sapling")] sapling: Option, + #[cfg(feature = "orchard")] orchard: Option, + unknown: Vec<(u32, Vec)>, + ) -> Result { + // Verify that IVK derivation succeeds; we don't want to construct a UFVK + // that can't derive transparent addresses. + #[cfg(feature = "transparent-inputs")] + let _ = transparent + .as_ref() + .map(|t| t.derive_external_ivk()) + .transpose()?; + + Ok(UnifiedFullViewingKey { + #[cfg(feature = "transparent-inputs")] + transparent, + #[cfg(feature = "sapling")] + sapling, + #[cfg(feature = "orchard")] + orchard, + unknown, + }) } /// Parses a `UnifiedFullViewingKey` from its [ZIP 316] string encoding. @@ -793,7 +798,7 @@ impl UnifiedFullViewingKey { }) .collect::>()?; - Ok(Self { + Self::from_checked_parts( #[cfg(feature = "transparent-inputs")] transparent, #[cfg(feature = "sapling")] @@ -801,7 +806,8 @@ impl UnifiedFullViewingKey { #[cfg(feature = "orchard")] orchard, unknown, - }) + ) + .map_err(|_| DecodingError::KeyDataInvalid(Typecode::P2pkh)) } /// Returns the string encoding of this `UnifiedFullViewingKey` for the given network. @@ -844,22 +850,19 @@ impl UnifiedFullViewingKey { } /// Derives a Unified Incoming Viewing Key from this Unified Full Viewing Key. - pub fn to_unified_incoming_viewing_key( - &self, - ) -> Result { - Ok(UnifiedIncomingViewingKey { + pub fn to_unified_incoming_viewing_key(&self) -> UnifiedIncomingViewingKey { + UnifiedIncomingViewingKey { #[cfg(feature = "transparent-inputs")] - transparent: self - .transparent - .as_ref() - .map(|t| t.derive_external_ivk()) - .transpose()?, + transparent: self.transparent.as_ref().map(|t| { + t.derive_external_ivk() + .expect("Transparent IVK derivation was checked at construction.") + }), #[cfg(feature = "sapling")] sapling: self.sapling.as_ref().map(|s| s.to_external_ivk()), #[cfg(feature = "orchard")] orchard: self.orchard.as_ref().map(|o| o.to_ivk(Scope::External)), unknown: Vec::new(), - }) + } } /// Returns the transparent component of the unified key at the @@ -890,7 +893,7 @@ impl UnifiedFullViewingKey { j: DiversifierIndex, request: UnifiedAddressRequest, ) -> Result { - self.to_unified_incoming_viewing_key()?.address(j, request) + self.to_unified_incoming_viewing_key().address(j, request) } /// Searches the diversifier space starting at diversifier index `j` for one which will @@ -905,7 +908,7 @@ impl UnifiedFullViewingKey { mut j: DiversifierIndex, request: UnifiedAddressRequest, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { - self.to_unified_incoming_viewing_key()? + self.to_unified_incoming_viewing_key() .find_address(j, request) } @@ -936,7 +939,12 @@ pub struct UnifiedIncomingViewingKey { } impl UnifiedIncomingViewingKey { - /// Construct a new unified incoming viewing key, if the required components are present. + /// Construct a new unified incoming viewing key. + /// + /// This method is only available when the `test-dependencies` feature is enabled, + /// as derivation from the UFVK or deserialization from the serialized form should + /// be used instead. + #[cfg(any(test, feature = "test-dependencies"))] pub fn new( #[cfg(feature = "transparent-inputs")] transparent: Option< zcash_primitives::legacy::keys::ExternalIvk, @@ -944,30 +952,17 @@ impl UnifiedIncomingViewingKey { #[cfg(feature = "sapling")] sapling: Option<::sapling::zip32::IncomingViewingKey>, #[cfg(feature = "orchard")] orchard: Option, // TODO: Implement construction of UIVKs with metadata items. - ) -> Option { - #[cfg(feature = "orchard")] - let has_orchard = orchard.is_some(); - #[cfg(not(feature = "orchard"))] - let has_orchard = false; - #[cfg(feature = "sapling")] - let has_sapling = sapling.is_some(); - #[cfg(not(feature = "sapling"))] - let has_sapling = false; - - if has_orchard || has_sapling { - Some(UnifiedIncomingViewingKey { - #[cfg(feature = "transparent-inputs")] - transparent, - #[cfg(feature = "sapling")] - sapling, - #[cfg(feature = "orchard")] - orchard, - // We don't allow constructing new UFVKs with unknown items, but we store - // this to allow parsing such UFVKs. - unknown: vec![], - }) - } else { - None + ) -> UnifiedIncomingViewingKey { + UnifiedIncomingViewingKey { + #[cfg(feature = "transparent-inputs")] + transparent, + #[cfg(feature = "sapling")] + sapling, + #[cfg(feature = "orchard")] + orchard, + // We don't allow constructing new UFVKs with unknown items, but we store + // this to allow parsing such UFVKs. + unknown: vec![], } } @@ -1543,7 +1538,6 @@ mod tests { #[cfg(any(feature = "orchard", feature = "sapling"))] { - let uivk = uivk.expect("Orchard or Sapling ivk is present."); let encoded = uivk.to_uivk().encode(&NetworkType::Main); // Test encoded form against known values; these test vectors contain Orchard receivers @@ -1654,8 +1648,7 @@ mod tests { let d_idx = DiversifierIndex::from(tv.diversifier_index); let uivk = usk .to_unified_full_viewing_key() - .to_unified_incoming_viewing_key() - .unwrap(); + .to_unified_incoming_viewing_key(); // The test vectors contain some diversifier indices that do not generate // valid Sapling addresses, so skip those. From b4171caaf43514b5a51d94c4b6b764a70bbec805 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 14 Mar 2024 11:59:58 -0600 Subject: [PATCH 6/8] zcash_keys: Fix no-default-features compilation. --- zcash_keys/CHANGELOG.md | 2 + zcash_keys/src/address.rs | 9 +- zcash_keys/src/keys.rs | 406 +++++++++++++++++++------------------- zcash_keys/src/lib.rs | 6 + 4 files changed, 214 insertions(+), 209 deletions(-) diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 93c627aa6..500318415 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -23,6 +23,8 @@ and this library adheres to Rust's notion of (instead of `Option<(UnifiedAddress, DiversifierIndex)>` for `find_address`). - `zcash_keys::keys::AddressGenerationError` - Added `DiversifierSpaceExhausted` variant. +- At least one of the `orchard`, `sapling`, or `transparent-inputs` features + must be enabled for the `keys` module to be accessible. ### Removed - `UnifiedFullViewingKey::new` has been placed behind the `test-dependencies` diff --git a/zcash_keys/src/address.rs b/zcash_keys/src/address.rs index dd1fb9ac4..c26c64a84 100644 --- a/zcash_keys/src/address.rs +++ b/zcash_keys/src/address.rs @@ -342,7 +342,14 @@ impl Address { } } -#[cfg(any(test, feature = "test-dependencies"))] +#[cfg(all( + any( + feature = "orchard", + feature = "sapling", + feature = "transparent-inputs" + ), + any(test, feature = "test-dependencies") +))] pub mod testing { use proptest::prelude::*; use zcash_primitives::consensus::Network; diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index a5dbbe368..aac28cda3 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -7,11 +7,14 @@ use std::{ }; use zcash_address::unified::{self, Container, Encoding, Typecode, Ufvk, Uivk}; -use zcash_protocol::consensus::{self, NetworkConstants}; +use zcash_protocol::consensus; use zip32::{AccountId, DiversifierIndex}; use crate::address::UnifiedAddress; +#[cfg(any(feature = "sapling", feature = "orchard"))] +use zcash_protocol::consensus::NetworkConstants; + #[cfg(feature = "transparent-inputs")] use { std::convert::TryInto, @@ -143,12 +146,16 @@ pub enum DerivationError { } impl Display for DerivationError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fn fmt(&self, _f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { #[cfg(feature = "orchard")] - DerivationError::Orchard(e) => write!(f, "Orchard error: {}", e), + DerivationError::Orchard(e) => write!(_f, "Orchard error: {}", e), #[cfg(feature = "transparent-inputs")] - DerivationError::Transparent(e) => write!(f, "Transparent error: {}", e), + DerivationError::Transparent(e) => write!(_f, "Transparent error: {}", e), + #[cfg(not(any(feature = "orchard", feature = "transparent-inputs")))] + other => { + unreachable!("Unhandled DerivationError variant {:?}", other) + } } } } @@ -1252,21 +1259,20 @@ pub mod testing { #[cfg(test)] mod tests { - use crate::keys::UnifiedIncomingViewingKey; - use super::UnifiedFullViewingKey; use proptest::prelude::proptest; - use zcash_address::unified::{Encoding, Uivk}; + + use {zcash_primitives::consensus::MAIN_NETWORK, zip32::AccountId}; + + #[cfg(any(feature = "sapling", feature = "orchard"))] + use { + super::{UnifiedFullViewingKey, UnifiedIncomingViewingKey}, + zcash_address::unified::{Encoding, Uivk}, + }; + #[cfg(feature = "orchard")] use zip32::Scope; - #[cfg(any( - feature = "orchard", - feature = "sapling", - feature = "transparent-inputs" - ))] - use {zcash_primitives::consensus::MAIN_NETWORK, zip32::AccountId}; - #[cfg(feature = "sapling")] use super::sapling; @@ -1274,10 +1280,7 @@ mod tests { use { crate::{address::Address, encoding::AddressCodec}, zcash_address::test_vectors, - zcash_primitives::legacy::{ - self, - keys::{AccountPrivKey, IncomingViewingKey}, - }, + zcash_primitives::legacy::keys::{AccountPrivKey, IncomingViewingKey}, zip32::DiversifierIndex, }; @@ -1305,19 +1308,19 @@ mod tests { fn pk_to_taddr() { use zcash_primitives::legacy::keys::NonHardenedChildIndex; - let taddr = - legacy::keys::AccountPrivKey::from_seed(&MAIN_NETWORK, &seed(), AccountId::ZERO) - .unwrap() - .to_account_pubkey() - .derive_external_ivk() - .unwrap() - .derive_address(NonHardenedChildIndex::ZERO) - .unwrap() - .encode(&MAIN_NETWORK); + let taddr = AccountPrivKey::from_seed(&MAIN_NETWORK, &seed(), AccountId::ZERO) + .unwrap() + .to_account_pubkey() + .derive_external_ivk() + .unwrap() + .derive_address(NonHardenedChildIndex::ZERO) + .unwrap() + .encode(&MAIN_NETWORK); assert_eq!(taddr, "t1PKtYdJJHhc3Pxowmznkg7vdTwnhEsCvR4".to_string()); } #[test] + #[cfg(any(feature = "orchard", feature = "sapling"))] fn ufvk_round_trip() { #[cfg(feature = "orchard")] let orchard = { @@ -1348,100 +1351,93 @@ mod tests { orchard, ); - #[cfg(not(any(feature = "orchard", feature = "sapling")))] - assert!(ufvk.is_none()); + let ufvk = ufvk.expect("Orchard or Sapling fvk is present."); + let encoded = ufvk.encode(&MAIN_NETWORK); - #[cfg(any(feature = "orchard", feature = "sapling"))] + // Test encoded form against known values; these test vectors contain Orchard receivers + // that will be treated as unknown if the `orchard` feature is not enabled. + let encoded_with_t = "uview1tg6rpjgju2s2j37gkgjq79qrh5lvzr6e0ed3n4sf4hu5qd35vmsh7avl80xa6mx7ryqce9hztwaqwrdthetpy4pc0kce25x453hwcmax02p80pg5savlg865sft9reat07c5vlactr6l2pxtlqtqunt2j9gmvr8spcuzf07af80h5qmut38h0gvcfa9k4rwujacwwca9vu8jev7wq6c725huv8qjmhss3hdj2vh8cfxhpqcm2qzc34msyrfxk5u6dqttt4vv2mr0aajreww5yufpk0gn4xkfm888467k7v6fmw7syqq6cceu078yw8xja502jxr0jgum43lhvpzmf7eu5dmnn6cr6f7p43yw8znzgxg598mllewnx076hljlvynhzwn5es94yrv65tdg3utuz2u3sras0wfcq4adxwdvlk387d22g3q98t5z74quw2fa4wed32escx8dwh4mw35t4jwf35xyfxnu83mk5s4kw2glkgsshmxk"; + let _encoded_no_t = "uview12z384wdq76ceewlsu0esk7d97qnd23v2qnvhujxtcf2lsq8g4hwzpx44fwxssnm5tg8skyh4tnc8gydwxefnnm0hd0a6c6etmj0pp9jqkdsllkr70u8gpf7ndsfqcjlqn6dec3faumzqlqcmtjf8vp92h7kj38ph2786zx30hq2wru8ae3excdwc8w0z3t9fuw7mt7xy5sn6s4e45kwm0cjp70wytnensgdnev286t3vew3yuwt2hcz865y037k30e428dvgne37xvyeal2vu8yjnznphf9t2rw3gdp0hk5zwq00ws8f3l3j5n3qkqgsyzrwx4qzmgq0xwwk4vz2r6vtsykgz089jncvycmem3535zjwvvtvjw8v98y0d5ydwte575gjm7a7k"; + + // We test the full roundtrip only with the `sapling` and `orchard` features enabled, + // because we will not generate these parts of the encoding if the UFVK does not have an + // these parts. + #[cfg(all(feature = "sapling", feature = "orchard"))] { - let ufvk = ufvk.expect("Orchard or Sapling fvk is present."); - let encoded = ufvk.encode(&MAIN_NETWORK); - - // Test encoded form against known values; these test vectors contain Orchard receivers - // that will be treated as unknown if the `orchard` feature is not enabled. - let encoded_with_t = "uview1tg6rpjgju2s2j37gkgjq79qrh5lvzr6e0ed3n4sf4hu5qd35vmsh7avl80xa6mx7ryqce9hztwaqwrdthetpy4pc0kce25x453hwcmax02p80pg5savlg865sft9reat07c5vlactr6l2pxtlqtqunt2j9gmvr8spcuzf07af80h5qmut38h0gvcfa9k4rwujacwwca9vu8jev7wq6c725huv8qjmhss3hdj2vh8cfxhpqcm2qzc34msyrfxk5u6dqttt4vv2mr0aajreww5yufpk0gn4xkfm888467k7v6fmw7syqq6cceu078yw8xja502jxr0jgum43lhvpzmf7eu5dmnn6cr6f7p43yw8znzgxg598mllewnx076hljlvynhzwn5es94yrv65tdg3utuz2u3sras0wfcq4adxwdvlk387d22g3q98t5z74quw2fa4wed32escx8dwh4mw35t4jwf35xyfxnu83mk5s4kw2glkgsshmxk"; - let _encoded_no_t = "uview12z384wdq76ceewlsu0esk7d97qnd23v2qnvhujxtcf2lsq8g4hwzpx44fwxssnm5tg8skyh4tnc8gydwxefnnm0hd0a6c6etmj0pp9jqkdsllkr70u8gpf7ndsfqcjlqn6dec3faumzqlqcmtjf8vp92h7kj38ph2786zx30hq2wru8ae3excdwc8w0z3t9fuw7mt7xy5sn6s4e45kwm0cjp70wytnensgdnev286t3vew3yuwt2hcz865y037k30e428dvgne37xvyeal2vu8yjnznphf9t2rw3gdp0hk5zwq00ws8f3l3j5n3qkqgsyzrwx4qzmgq0xwwk4vz2r6vtsykgz089jncvycmem3535zjwvvtvjw8v98y0d5ydwte575gjm7a7k"; - - // We test the full roundtrip only with the `sapling` and `orchard` features enabled, - // because we will not generate these parts of the encoding if the UFVK does not have an - // these parts. - #[cfg(all(feature = "sapling", feature = "orchard"))] - { - #[cfg(feature = "transparent-inputs")] - assert_eq!(encoded, encoded_with_t); - #[cfg(not(feature = "transparent-inputs"))] - assert_eq!(encoded, _encoded_no_t); - } - - let decoded = UnifiedFullViewingKey::decode(&MAIN_NETWORK, &encoded).unwrap(); - let reencoded = decoded.encode(&MAIN_NETWORK); - assert_eq!(encoded, reencoded); - #[cfg(feature = "transparent-inputs")] - assert_eq!( - decoded.transparent.map(|t| t.serialize()), - ufvk.transparent.as_ref().map(|t| t.serialize()), - ); - #[cfg(feature = "sapling")] - assert_eq!( - decoded.sapling.map(|s| s.to_bytes()), - ufvk.sapling.map(|s| s.to_bytes()), - ); - #[cfg(feature = "orchard")] - assert_eq!( - decoded.orchard.map(|o| o.to_bytes()), - ufvk.orchard.map(|o| o.to_bytes()), - ); - - let decoded_with_t = - UnifiedFullViewingKey::decode(&MAIN_NETWORK, encoded_with_t).unwrap(); - #[cfg(feature = "transparent-inputs")] - assert_eq!( - decoded_with_t.transparent.map(|t| t.serialize()), - ufvk.transparent.as_ref().map(|t| t.serialize()), - ); - - // Both Orchard and Sapling enabled - #[cfg(all( - feature = "orchard", - feature = "sapling", - feature = "transparent-inputs" - ))] - assert_eq!(decoded_with_t.unknown.len(), 0); - #[cfg(all( - feature = "orchard", - feature = "sapling", - not(feature = "transparent-inputs") - ))] - assert_eq!(decoded_with_t.unknown.len(), 1); - - // Orchard enabled - #[cfg(all( - feature = "orchard", - not(feature = "sapling"), - feature = "transparent-inputs" - ))] - assert_eq!(decoded_with_t.unknown.len(), 1); - #[cfg(all( - feature = "orchard", - not(feature = "sapling"), - not(feature = "transparent-inputs") - ))] - assert_eq!(decoded_with_t.unknown.len(), 2); - - // Sapling enabled - #[cfg(all( - not(feature = "orchard"), - feature = "sapling", - feature = "transparent-inputs" - ))] - assert_eq!(decoded_with_t.unknown.len(), 1); - #[cfg(all( - not(feature = "orchard"), - feature = "sapling", - not(feature = "transparent-inputs") - ))] - assert_eq!(decoded_with_t.unknown.len(), 2); + assert_eq!(encoded, encoded_with_t); + #[cfg(not(feature = "transparent-inputs"))] + assert_eq!(encoded, _encoded_no_t); } + + let decoded = UnifiedFullViewingKey::decode(&MAIN_NETWORK, &encoded).unwrap(); + let reencoded = decoded.encode(&MAIN_NETWORK); + assert_eq!(encoded, reencoded); + + #[cfg(feature = "transparent-inputs")] + assert_eq!( + decoded.transparent.map(|t| t.serialize()), + ufvk.transparent.as_ref().map(|t| t.serialize()), + ); + #[cfg(feature = "sapling")] + assert_eq!( + decoded.sapling.map(|s| s.to_bytes()), + ufvk.sapling.map(|s| s.to_bytes()), + ); + #[cfg(feature = "orchard")] + assert_eq!( + decoded.orchard.map(|o| o.to_bytes()), + ufvk.orchard.map(|o| o.to_bytes()), + ); + + let decoded_with_t = UnifiedFullViewingKey::decode(&MAIN_NETWORK, encoded_with_t).unwrap(); + #[cfg(feature = "transparent-inputs")] + assert_eq!( + decoded_with_t.transparent.map(|t| t.serialize()), + ufvk.transparent.as_ref().map(|t| t.serialize()), + ); + + // Both Orchard and Sapling enabled + #[cfg(all( + feature = "orchard", + feature = "sapling", + feature = "transparent-inputs" + ))] + assert_eq!(decoded_with_t.unknown.len(), 0); + #[cfg(all( + feature = "orchard", + feature = "sapling", + not(feature = "transparent-inputs") + ))] + assert_eq!(decoded_with_t.unknown.len(), 1); + + // Orchard enabled + #[cfg(all( + feature = "orchard", + not(feature = "sapling"), + feature = "transparent-inputs" + ))] + assert_eq!(decoded_with_t.unknown.len(), 1); + #[cfg(all( + feature = "orchard", + not(feature = "sapling"), + not(feature = "transparent-inputs") + ))] + assert_eq!(decoded_with_t.unknown.len(), 2); + + // Sapling enabled + #[cfg(all( + not(feature = "orchard"), + feature = "sapling", + feature = "transparent-inputs" + ))] + assert_eq!(decoded_with_t.unknown.len(), 1); + #[cfg(all( + not(feature = "orchard"), + feature = "sapling", + not(feature = "transparent-inputs") + ))] + assert_eq!(decoded_with_t.unknown.len(), 2); } #[test] @@ -1501,6 +1497,7 @@ mod tests { } #[test] + #[cfg(any(feature = "orchard", feature = "sapling"))] fn uivk_round_trip() { use zcash_primitives::consensus::NetworkType; @@ -1533,101 +1530,94 @@ mod tests { orchard, ); - #[cfg(not(any(feature = "orchard", feature = "sapling")))] - assert!(uivk.is_none()); + let encoded = uivk.to_uivk().encode(&NetworkType::Main); - #[cfg(any(feature = "orchard", feature = "sapling"))] + // Test encoded form against known values; these test vectors contain Orchard receivers + // that will be treated as unknown if the `orchard` feature is not enabled. + let encoded_with_t = "uivk1z28yg638vjwusmf0zc9ad2j0mpv6s42wc5kqt004aaqfu5xxxgu7mdcydn9qf723fnryt34s6jyxyw0jt7spq04c3v9ze6qe9gjjc5aglz8zv5pqtw58czd0actynww5n85z3052kzgy6cu0fyjafyp4sr4kppyrrwhwev2rr0awq6m8d66esvk6fgacggqnswg5g9gkv6t6fj9ajhyd0gmel4yzscprpzduncc0e2lywufup6fvzf6y8cefez2r99pgge5yyfuus0r60khgu895pln5e7nn77q6s9kh2uwf6lrfu06ma2kd7r05jjvl4hn6nupge8fajh0cazd7mkmz23t79w"; + let _encoded_no_t = "uivk1020vq9j5zeqxh303sxa0zv2hn9wm9fev8x0p8yqxdwyzde9r4c90fcglc63usj0ycl2scy8zxuhtser0qrq356xfy8x3vyuxu7f6gas75svl9v9m3ctuazsu0ar8e8crtx7x6zgh4kw8xm3q4rlkpm9er2wefxhhf9pn547gpuz9vw27gsdp6c03nwlrxgzhr2g6xek0x8l5avrx9ue9lf032tr7kmhqf3nfdxg7ldfgx6yf09g"; + + // We test the full roundtrip only with the `sapling` and `orchard` features enabled, + // because we will not generate these parts of the encoding if the UIVK does not have an + // these parts. + #[cfg(all(feature = "sapling", feature = "orchard"))] { - let encoded = uivk.to_uivk().encode(&NetworkType::Main); - - // Test encoded form against known values; these test vectors contain Orchard receivers - // that will be treated as unknown if the `orchard` feature is not enabled. - let encoded_with_t = "uivk1z28yg638vjwusmf0zc9ad2j0mpv6s42wc5kqt004aaqfu5xxxgu7mdcydn9qf723fnryt34s6jyxyw0jt7spq04c3v9ze6qe9gjjc5aglz8zv5pqtw58czd0actynww5n85z3052kzgy6cu0fyjafyp4sr4kppyrrwhwev2rr0awq6m8d66esvk6fgacggqnswg5g9gkv6t6fj9ajhyd0gmel4yzscprpzduncc0e2lywufup6fvzf6y8cefez2r99pgge5yyfuus0r60khgu895pln5e7nn77q6s9kh2uwf6lrfu06ma2kd7r05jjvl4hn6nupge8fajh0cazd7mkmz23t79w"; - let _encoded_no_t = "uivk1020vq9j5zeqxh303sxa0zv2hn9wm9fev8x0p8yqxdwyzde9r4c90fcglc63usj0ycl2scy8zxuhtser0qrq356xfy8x3vyuxu7f6gas75svl9v9m3ctuazsu0ar8e8crtx7x6zgh4kw8xm3q4rlkpm9er2wefxhhf9pn547gpuz9vw27gsdp6c03nwlrxgzhr2g6xek0x8l5avrx9ue9lf032tr7kmhqf3nfdxg7ldfgx6yf09g"; - - // We test the full roundtrip only with the `sapling` and `orchard` features enabled, - // because we will not generate these parts of the encoding if the UIVK does not have an - // these parts. - #[cfg(all(feature = "sapling", feature = "orchard"))] - { - #[cfg(feature = "transparent-inputs")] - assert_eq!(encoded, encoded_with_t); - #[cfg(not(feature = "transparent-inputs"))] - assert_eq!(encoded, _encoded_no_t); - } - - let decoded = - UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(&encoded).unwrap().1).unwrap(); - let reencoded = decoded.to_uivk().encode(&NetworkType::Main); - assert_eq!(encoded, reencoded); - #[cfg(feature = "transparent-inputs")] - assert_eq!( - decoded.transparent.map(|t| t.serialize()), - uivk.transparent.as_ref().map(|t| t.serialize()), - ); - #[cfg(feature = "sapling")] - assert_eq!( - decoded.sapling.map(|s| s.to_bytes()), - uivk.sapling.map(|s| s.to_bytes()), - ); - #[cfg(feature = "orchard")] - assert_eq!( - decoded.orchard.map(|o| o.to_bytes()), - uivk.orchard.map(|o| o.to_bytes()), - ); - - let decoded_with_t = - UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(encoded_with_t).unwrap().1) - .unwrap(); - #[cfg(feature = "transparent-inputs")] - assert_eq!( - decoded_with_t.transparent.map(|t| t.serialize()), - uivk.transparent.as_ref().map(|t| t.serialize()), - ); - - // Both Orchard and Sapling enabled - #[cfg(all( - feature = "orchard", - feature = "sapling", - feature = "transparent-inputs" - ))] - assert_eq!(decoded_with_t.unknown.len(), 0); - #[cfg(all( - feature = "orchard", - feature = "sapling", - not(feature = "transparent-inputs") - ))] - assert_eq!(decoded_with_t.unknown.len(), 1); - - // Orchard enabled - #[cfg(all( - feature = "orchard", - not(feature = "sapling"), - feature = "transparent-inputs" - ))] - assert_eq!(decoded_with_t.unknown.len(), 1); - #[cfg(all( - feature = "orchard", - not(feature = "sapling"), - not(feature = "transparent-inputs") - ))] - assert_eq!(decoded_with_t.unknown.len(), 2); - - // Sapling enabled - #[cfg(all( - not(feature = "orchard"), - feature = "sapling", - feature = "transparent-inputs" - ))] - assert_eq!(decoded_with_t.unknown.len(), 1); - #[cfg(all( - not(feature = "orchard"), - feature = "sapling", - not(feature = "transparent-inputs") - ))] - assert_eq!(decoded_with_t.unknown.len(), 2); + assert_eq!(encoded, encoded_with_t); + #[cfg(not(feature = "transparent-inputs"))] + assert_eq!(encoded, _encoded_no_t); } + + let decoded = + UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(&encoded).unwrap().1).unwrap(); + let reencoded = decoded.to_uivk().encode(&NetworkType::Main); + assert_eq!(encoded, reencoded); + + #[cfg(feature = "transparent-inputs")] + assert_eq!( + decoded.transparent.map(|t| t.serialize()), + uivk.transparent.as_ref().map(|t| t.serialize()), + ); + #[cfg(feature = "sapling")] + assert_eq!( + decoded.sapling.map(|s| s.to_bytes()), + uivk.sapling.map(|s| s.to_bytes()), + ); + #[cfg(feature = "orchard")] + assert_eq!( + decoded.orchard.map(|o| o.to_bytes()), + uivk.orchard.map(|o| o.to_bytes()), + ); + + let decoded_with_t = + UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(encoded_with_t).unwrap().1).unwrap(); + #[cfg(feature = "transparent-inputs")] + assert_eq!( + decoded_with_t.transparent.map(|t| t.serialize()), + uivk.transparent.as_ref().map(|t| t.serialize()), + ); + + // Both Orchard and Sapling enabled + #[cfg(all( + feature = "orchard", + feature = "sapling", + feature = "transparent-inputs" + ))] + assert_eq!(decoded_with_t.unknown.len(), 0); + #[cfg(all( + feature = "orchard", + feature = "sapling", + not(feature = "transparent-inputs") + ))] + assert_eq!(decoded_with_t.unknown.len(), 1); + + // Orchard enabled + #[cfg(all( + feature = "orchard", + not(feature = "sapling"), + feature = "transparent-inputs" + ))] + assert_eq!(decoded_with_t.unknown.len(), 1); + #[cfg(all( + feature = "orchard", + not(feature = "sapling"), + not(feature = "transparent-inputs") + ))] + assert_eq!(decoded_with_t.unknown.len(), 2); + + // Sapling enabled + #[cfg(all( + not(feature = "orchard"), + feature = "sapling", + feature = "transparent-inputs" + ))] + assert_eq!(decoded_with_t.unknown.len(), 1); + #[cfg(all( + not(feature = "orchard"), + feature = "sapling", + not(feature = "transparent-inputs") + ))] + assert_eq!(decoded_with_t.unknown.len(), 2); } #[test] diff --git a/zcash_keys/src/lib.rs b/zcash_keys/src/lib.rs index 992170ab1..eb881a80f 100644 --- a/zcash_keys/src/lib.rs +++ b/zcash_keys/src/lib.rs @@ -16,4 +16,10 @@ pub mod address; pub mod encoding; + +#[cfg(any( + feature = "orchard", + feature = "sapling", + feature = "transparent-inputs" +))] pub mod keys; From 9ddbf1e3e95b2d3033d1cff0258f244640349b74 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Thu, 14 Mar 2024 15:21:46 -0600 Subject: [PATCH 7/8] Implement todo! line --- zcash_client_sqlite/src/wallet.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index e73d6dd71..90d2616fc 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -208,7 +208,7 @@ impl Account { ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { match &self.viewing_key { ViewingKey::Full(ufvk) => ufvk.default_address(request), - ViewingKey::Incoming(_uivk) => todo!(), + ViewingKey::Incoming(uivk) => uivk.default_address(request), } } } From 9e1a4327c3b9d7632925add0ef87821c118aa036 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 14 Mar 2024 15:23:40 -0600 Subject: [PATCH 8/8] zcash_keys: Keep the Ufvk and Uivk encodings private. --- zcash_client_sqlite/src/wallet.rs | 18 ++------- .../init/migrations/full_account_ids.rs | 4 +- zcash_keys/CHANGELOG.md | 2 - zcash_keys/src/keys.rs | 40 ++++++++++++++----- 4 files changed, 35 insertions(+), 29 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 90d2616fc..90dc840ae 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -74,7 +74,6 @@ use std::io::{self, Cursor}; use std::num::NonZeroU32; use std::ops::RangeInclusive; use tracing::debug; -use zcash_address::unified::{Encoding, Uivk}; use zcash_keys::keys::{ AddressGenerationError, HdSeedFingerprint, UnifiedAddressRequest, UnifiedIncomingViewingKey, }; @@ -120,7 +119,7 @@ use { crate::UtxoId, rusqlite::Row, std::collections::BTreeSet, - zcash_address::unified::Ivk, + zcash_address::unified::{Encoding, Ivk, Uivk}, zcash_client_backend::wallet::{TransparentAddressMetadata, WalletTransparentOutput}, zcash_primitives::{ legacy::{ @@ -347,7 +346,7 @@ pub(crate) fn add_account( ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.as_bytes()), ":hd_account_index": hd_account_index.map(u32::from), ":ufvk": viewing_key.ufvk().map(|ufvk| ufvk.encode(params)), - ":uivk": viewing_key.uivk().to_uivk().encode(¶ms.network_type()), + ":uivk": viewing_key.uivk().encode(params), ":orchard_fvk_item_cache": orchard_item, ":sapling_fvk_item_cache": sapling_item, ":p2pkh_fvk_item_cache": transparent_item, @@ -1503,18 +1502,9 @@ pub(crate) fn get_account( )) } else { let uivk_str: String = row.get("uivk")?; - let (network, uivk) = Uivk::decode(&uivk_str).map_err(|e| { - SqliteClientError::CorruptedData(format!("Failure to decode UIVK: {e}")) - })?; - if network != params.network_type() { - return Err(SqliteClientError::CorruptedData( - "UIVK network type does not match wallet network type".to_string(), - )); - } ViewingKey::Incoming(Box::new( - UnifiedIncomingViewingKey::from_uivk(&uivk).map_err(|e| { - SqliteClientError::CorruptedData(format!("Failure to decode UIVK: {e}")) - })?, + UnifiedIncomingViewingKey::decode(params, &uivk_str[..]) + .map_err(SqliteClientError::BadAccountData)?, )) }; diff --git a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs index 07689421d..fa019be21 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs @@ -5,7 +5,6 @@ use rusqlite::{named_params, Transaction}; use schemer_rusqlite::RusqliteMigration; use secrecy::{ExposeSecret, SecretVec}; use uuid::Uuid; -use zcash_address::unified::Encoding; use zcash_client_backend::{data_api::AccountKind, keys::UnifiedSpendingKey}; use zcash_keys::keys::{HdSeedFingerprint, UnifiedFullViewingKey}; use zcash_primitives::consensus; @@ -122,8 +121,7 @@ impl RusqliteMigration for Migration

{ let uivk = ufvk_parsed .to_unified_incoming_viewing_key() - .to_uivk() - .encode(&self.params.network_type()); + .encode(&self.params); #[cfg(feature = "transparent-inputs")] let transparent_item = ufvk_parsed.transparent().map(|k| k.serialize()); diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 500318415..8f2627d98 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -12,8 +12,6 @@ and this library adheres to Rust's notion of - `impl Display for zcash_keys::keys::AddressGenerationError` - `impl std::error::Error for zcash_keys::keys::AddressGenerationError` - `zcash_keys::keys::DecodingError` -- `zcash_keys::keys::UnifiedFullViewingKey::from_ufvk` -- `zcash_keys::keys::UnifiedFullViewingKey::to_ufvk` - `zcash_keys::keys::UnifiedFullViewingKey::to_unified_incoming_viewing_key` - `zcash_keys::keys::UnifiedIncomingViewingKey` diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index aac28cda3..d03b5852c 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -740,13 +740,13 @@ impl UnifiedFullViewingKey { )); } - Self::from_ufvk(&ufvk).map_err(|e| e.to_string()) + Self::parse(&ufvk).map_err(|e| e.to_string()) } /// Parses a `UnifiedFullViewingKey` from its [ZIP 316] string encoding. /// /// [ZIP 316]: https://zips.z.cash/zip-0316 - pub fn from_ufvk(ufvk: &Ufvk) -> Result { + pub fn parse(ufvk: &Ufvk) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; #[cfg(feature = "sapling")] @@ -823,7 +823,7 @@ impl UnifiedFullViewingKey { } /// Returns the string encoding of this `UnifiedFullViewingKey` for the given network. - pub fn to_ufvk(&self) -> Ufvk { + fn to_ufvk(&self) -> Ufvk { let items = std::iter::empty().chain(self.unknown.iter().map(|(typecode, data)| { unified::Fvk::Unknown { typecode: *typecode, @@ -973,8 +973,24 @@ impl UnifiedIncomingViewingKey { } } + /// Parses a `UnifiedFullViewingKey` from its [ZIP 316] string encoding. + /// + /// [ZIP 316]: https://zips.z.cash/zip-0316 + pub fn decode(params: &P, encoding: &str) -> Result { + let (net, ufvk) = unified::Uivk::decode(encoding).map_err(|e| e.to_string())?; + let expected_net = params.network_type(); + if net != expected_net { + return Err(format!( + "UIVK is for network {:?} but we expected {:?}", + net, expected_net, + )); + } + + Self::parse(&ufvk).map_err(|e| e.to_string()) + } + /// Constructs a unified incoming viewing key from a parsed unified encoding. - pub fn from_uivk(uivk: &Uivk) -> Result { + fn parse(uivk: &Uivk) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; #[cfg(feature = "sapling")] @@ -1041,8 +1057,13 @@ impl UnifiedIncomingViewingKey { }) } + /// Returns the string encoding of this `UnifiedFullViewingKey` for the given network. + pub fn encode(&self, params: &P) -> String { + self.render().encode(¶ms.network_type()) + } + /// Converts this unified incoming viewing key to a unified encoding. - pub fn to_uivk(&self) -> Uivk { + fn render(&self) -> Uivk { let items = std::iter::empty().chain(self.unknown.iter().map(|(typecode, data)| { unified::Ivk::Unknown { typecode: *typecode, @@ -1530,7 +1551,7 @@ mod tests { orchard, ); - let encoded = uivk.to_uivk().encode(&NetworkType::Main); + let encoded = uivk.render().encode(&NetworkType::Main); // Test encoded form against known values; these test vectors contain Orchard receivers // that will be treated as unknown if the `orchard` feature is not enabled. @@ -1548,9 +1569,8 @@ mod tests { assert_eq!(encoded, _encoded_no_t); } - let decoded = - UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(&encoded).unwrap().1).unwrap(); - let reencoded = decoded.to_uivk().encode(&NetworkType::Main); + let decoded = UnifiedIncomingViewingKey::parse(&Uivk::decode(&encoded).unwrap().1).unwrap(); + let reencoded = decoded.render().encode(&NetworkType::Main); assert_eq!(encoded, reencoded); #[cfg(feature = "transparent-inputs")] @@ -1570,7 +1590,7 @@ mod tests { ); let decoded_with_t = - UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(encoded_with_t).unwrap().1).unwrap(); + UnifiedIncomingViewingKey::parse(&Uivk::decode(encoded_with_t).unwrap().1).unwrap(); #[cfg(feature = "transparent-inputs")] assert_eq!( decoded_with_t.transparent.map(|t| t.serialize()),