From 9b98f46bf6cbb3f48c7698fd3f4b33e7b5963e63 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sun, 14 Jan 2024 20:28:29 -0700 Subject: [PATCH 1/3] zcash_keys: Add `sapling` and `transparent-inputs` feature flags. Fixes #1160 --- zcash_client_backend/Cargo.toml | 2 +- zcash_client_backend/src/data_api/wallet.rs | 9 +- zcash_client_sqlite/Cargo.toml | 12 +- zcash_keys/Cargo.toml | 5 +- zcash_keys/src/address.rs | 191 +++++++----- zcash_keys/src/encoding.rs | 31 +- zcash_keys/src/keys.rs | 322 ++++++++++++-------- 7 files changed, 364 insertions(+), 208 deletions(-) diff --git a/zcash_client_backend/Cargo.toml b/zcash_client_backend/Cargo.toml index 61b2785c2..e15ecb228 100644 --- a/zcash_client_backend/Cargo.toml +++ b/zcash_client_backend/Cargo.toml @@ -27,7 +27,7 @@ rustdoc-args = ["--cfg", "docsrs"] [dependencies] zcash_address.workspace = true zcash_encoding.workspace = true -zcash_keys.workspace = true +zcash_keys = { workspace = true, features = ["sapling"] } zcash_note_encryption.workspace = true zcash_primitives.workspace = true zip32.workspace = true diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 5080e16bc..f941392fb 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -140,7 +140,7 @@ where /// }; /// use zcash_proofs::prover::LocalTxProver; /// use zcash_client_backend::{ -/// keys::UnifiedSpendingKey, +/// keys::{UnifiedSpendingKey, UnifiedAddressRequest}, /// data_api::{wallet::create_spend_to_address, error::Error, testing}, /// wallet::OvkPolicy, /// }; @@ -166,8 +166,13 @@ where /// }; /// /// let account = AccountId::from(0); +/// let req = UnifiedAddressRequest { +/// has_orchard: false, +/// has_sapling: true, +/// has_p2pkh: true +/// }; /// let usk = UnifiedSpendingKey::from_seed(&Network::TestNetwork, &[0; 32][..], account).unwrap(); -/// let to = usk.to_unified_full_viewing_key().default_address().0.into(); +/// let to = usk.to_unified_full_viewing_key().default_address(req).0.into(); /// /// let mut db_read = testing::MockWalletDb { /// network: Network::TestNetwork diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 33516475a..30d9e2fe0 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -22,7 +22,7 @@ rustdoc-args = ["--cfg", "docsrs"] zcash_address.workspace = true zcash_client_backend = { workspace = true, features = ["unstable-serialization", "unstable-spanning-tree"] } zcash_encoding.workspace = true -zcash_keys = { workspace = true, features = ["orchard"] } +zcash_keys = { workspace = true, features = ["sapling"] } zcash_primitives.workspace = true # Dependencies exposed in a public API: @@ -90,7 +90,7 @@ multicore = ["maybe-rayon/threads", "zcash_primitives/multicore"] ## Enables support for storing data related to the sending and receiving of ## Orchard funds. -orchard = ["dep:orchard", "zcash_client_backend/orchard"] +orchard = ["dep:orchard", "zcash_client_backend/orchard", "zcash_keys/orchard"] ## Exposes APIs that are useful for testing, such as `proptest` strategies. test-dependencies = [ @@ -100,8 +100,12 @@ test-dependencies = [ "incrementalmerkletree/test-dependencies", ] -## Enables receiving transparent funds and shielding them. -transparent-inputs = ["dep:hdwallet", "zcash_client_backend/transparent-inputs"] +## Enables receiving transparent funds and sending to transparent recipients +transparent-inputs = [ + "dep:hdwallet", + "zcash_keys/transparent-inputs", + "zcash_client_backend/transparent-inputs" +] #! ### Experimental features diff --git a/zcash_keys/Cargo.toml b/zcash_keys/Cargo.toml index 8a121d2b3..b62c8dbfe 100644 --- a/zcash_keys/Cargo.toml +++ b/zcash_keys/Cargo.toml @@ -48,7 +48,7 @@ subtle.workspace = true bls12_381.workspace = true group.workspace = true orchard = { workspace = true, optional = true } -sapling.workspace = true +sapling = { workspace = true, optional = true } # - Test dependencies proptest = { workspace = true, optional = true } @@ -75,6 +75,9 @@ transparent-inputs = ["dep:hdwallet", "zcash_primitives/transparent-inputs"] ## Enables use of Orchard key parts and addresses orchard = ["dep:orchard"] +## Enables use of Sapling key parts and addresses +sapling = ["dep:sapling"] + ## Exposes APIs that are useful for testing, such as `proptest` strategies. test-dependencies = [ "dep:proptest", diff --git a/zcash_keys/src/address.rs b/zcash_keys/src/address.rs index fd1397faf..b8c309510 100644 --- a/zcash_keys/src/address.rs +++ b/zcash_keys/src/address.rs @@ -1,17 +1,20 @@ //! Structs for handling supported address types. -use sapling::PaymentAddress; use zcash_address::{ unified::{self, Container, Encoding, Typecode}, ConversionError, Network, ToAddress, TryFromRawAddress, ZcashAddress, }; use zcash_primitives::{consensus, legacy::TransparentAddress}; +#[cfg(feature = "sapling")] +use sapling::PaymentAddress; + /// A Unified Address. #[derive(Clone, Debug, PartialEq, Eq)] pub struct UnifiedAddress { #[cfg(feature = "orchard")] orchard: Option, + #[cfg(feature = "sapling")] sapling: Option, transparent: Option, unknown: Vec<(u32, Vec)>, @@ -23,53 +26,62 @@ impl TryFrom for UnifiedAddress { fn try_from(ua: unified::Address) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; + #[cfg(feature = "sapling")] let mut sapling = None; let mut transparent = None; + let mut unknown: Vec<(u32, Vec)> = 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 = ua - .items_as_parsed() - .iter() - .filter_map(|receiver| match receiver { - #[cfg(feature = "orchard")] + for item in ua.items_as_parsed() { + match item { unified::Receiver::Orchard(data) => { - Option::from(orchard::Address::from_raw_address_bytes(data)) - .ok_or("Invalid Orchard receiver in Unified Address") - .map(|addr| { - orchard = Some(addr); - None - }) - .transpose() + #[cfg(feature = "orchard")] + { + orchard = Some( + Option::from(orchard::Address::from_raw_address_bytes(data)) + .ok_or("Invalid Orchard receiver in Unified Address")?, + ); + } + #[cfg(not(feature = "orchard"))] + { + unknown.push((unified::Typecode::Orchard.into(), data.to_vec())); + } } - #[cfg(not(feature = "orchard"))] - unified::Receiver::Orchard(data) => { - Some(Ok((unified::Typecode::Orchard.into(), data.to_vec()))) + + unified::Receiver::Sapling(data) => { + #[cfg(feature = "sapling")] + { + sapling = Some( + PaymentAddress::from_bytes(data) + .ok_or("Invalid Sapling receiver in Unified Address")?, + ); + } + #[cfg(not(feature = "sapling"))] + { + unknown.push((unified::Typecode::Sapling.into(), data.to_vec())); + } } - unified::Receiver::Sapling(data) => PaymentAddress::from_bytes(data) - .ok_or("Invalid Sapling receiver in Unified Address") - .map(|pa| { - sapling = Some(pa); - None - }) - .transpose(), + unified::Receiver::P2pkh(data) => { transparent = Some(TransparentAddress::PublicKeyHash(*data)); - None } + unified::Receiver::P2sh(data) => { transparent = Some(TransparentAddress::ScriptHash(*data)); - None } + unified::Receiver::Unknown { typecode, data } => { - Some(Ok((*typecode, data.clone()))) + unknown.push((*typecode, data.clone())); } - }) - .collect::>()?; + } + } Ok(Self { #[cfg(feature = "orchard")] orchard, + #[cfg(feature = "sapling")] sapling, transparent, unknown, @@ -84,18 +96,25 @@ impl UnifiedAddress { /// if no shielded receiver is provided). pub fn from_receivers( #[cfg(feature = "orchard")] orchard: Option, - sapling: Option, + #[cfg(feature = "sapling")] sapling: Option, transparent: Option, + // TODO: Add handling for address metadata items. ) -> Option { #[cfg(feature = "orchard")] let has_orchard = orchard.is_some(); #[cfg(not(feature = "orchard"))] let has_orchard = false; - if has_orchard || sapling.is_some() { + #[cfg(feature = "sapling")] + let has_sapling = sapling.is_some(); + #[cfg(not(feature = "sapling"))] + let has_sapling = false; + + if has_orchard || has_sapling { Some(Self { #[cfg(feature = "orchard")] orchard, + #[cfg(feature = "sapling")] sapling, transparent, unknown: vec![], @@ -128,6 +147,7 @@ impl UnifiedAddress { } /// Returns the Sapling receiver within this Unified Address, if any. + #[cfg(feature = "sapling")] pub fn sapling(&self) -> Option<&PaymentAddress> { self.sapling.as_ref() } @@ -148,36 +168,37 @@ impl UnifiedAddress { } fn to_address(&self, net: Network) -> ZcashAddress { - #[cfg(feature = "orchard")] - let orchard_receiver = self - .orchard - .as_ref() - .map(|addr| addr.to_raw_address_bytes()) - .map(unified::Receiver::Orchard); - #[cfg(not(feature = "orchard"))] - let orchard_receiver = None; + let items = self + .unknown + .iter() + .map(|(typecode, data)| unified::Receiver::Unknown { + typecode: *typecode, + data: data.clone(), + }); - let ua = unified::Address::try_from_items( - self.unknown - .iter() - .map(|(typecode, data)| unified::Receiver::Unknown { - typecode: *typecode, - data: data.clone(), - }) - .chain(self.transparent.as_ref().map(|taddr| match taddr { - TransparentAddress::PublicKeyHash(data) => unified::Receiver::P2pkh(*data), - TransparentAddress::ScriptHash(data) => unified::Receiver::P2sh(*data), - })) - .chain( - self.sapling - .as_ref() - .map(|pa| pa.to_bytes()) - .map(unified::Receiver::Sapling), - ) - .chain(orchard_receiver) - .collect(), - ) - .expect("UnifiedAddress should only be constructed safely"); + #[cfg(feature = "orchard")] + let items = items.chain( + self.orchard + .as_ref() + .map(|addr| addr.to_raw_address_bytes()) + .map(unified::Receiver::Orchard), + ); + + #[cfg(feature = "sapling")] + let items = items.chain( + self.sapling + .as_ref() + .map(|pa| pa.to_bytes()) + .map(unified::Receiver::Sapling), + ); + + let items = items.chain(self.transparent.as_ref().map(|taddr| match taddr { + TransparentAddress::PublicKeyHash(data) => unified::Receiver::P2pkh(*data), + TransparentAddress::ScriptHash(data) => unified::Receiver::P2sh(*data), + })); + + let ua = unified::Address::try_from_items(items.collect()) + .expect("UnifiedAddress should only be constructed safely"); ZcashAddress::from_unified(net, ua) } @@ -209,11 +230,13 @@ impl UnifiedAddress { /// An address that funds can be sent to. #[derive(Debug, PartialEq, Eq, Clone)] pub enum Address { + #[cfg(feature = "sapling")] Sapling(PaymentAddress), Transparent(TransparentAddress), Unified(UnifiedAddress), } +#[cfg(feature = "sapling")] impl From for Address { fn from(addr: PaymentAddress) -> Self { Address::Sapling(addr) @@ -235,6 +258,7 @@ impl From for Address { impl TryFromRawAddress for Address { type Error = &'static str; + #[cfg(feature = "sapling")] fn try_from_raw_sapling(data: [u8; 43]) -> Result> { let pa = PaymentAddress::from_bytes(&data).ok_or("Invalid Sapling payment address")?; Ok(pa.into()) @@ -270,6 +294,7 @@ impl Address { let net = params.address_network().expect("Unrecognized network"); match self { + #[cfg(feature = "sapling")] Address::Sapling(pa) => ZcashAddress::from_sapling(net, pa.to_bytes()), Address::Transparent(addr) => match addr { TransparentAddress::PublicKeyHash(data) => { @@ -288,13 +313,16 @@ impl Address { #[cfg(any(test, feature = "test-dependencies"))] pub mod testing { use proptest::prelude::*; - use sapling::testing::arb_payment_address; - use zcash_primitives::{consensus::Network, legacy::testing::arb_transparent_addr}; + use zcash_primitives::consensus::Network; use crate::keys::{testing::arb_unified_spending_key, UnifiedAddressRequest}; use super::{Address, UnifiedAddress}; + #[cfg(feature = "sapling")] + use sapling::testing::arb_payment_address; + use zcash_primitives::legacy::testing::arb_transparent_addr; + pub fn arb_unified_addr( params: Network, request: UnifiedAddressRequest, @@ -302,6 +330,7 @@ pub mod testing { arb_unified_spending_key(params).prop_map(move |k| k.default_address(request).0) } + #[cfg(feature = "sapling")] pub fn arb_addr(request: UnifiedAddressRequest) -> impl Strategy { prop_oneof![ arb_payment_address().prop_map(Address::Sapling), @@ -309,17 +338,34 @@ pub mod testing { arb_unified_addr(Network::TestNetwork, request).prop_map(Address::Unified), ] } + + #[cfg(not(feature = "sapling"))] + pub fn arb_addr(request: UnifiedAddressRequest) -> impl Strategy { + return prop_oneof![ + arb_transparent_addr().prop_map(Address::Transparent), + arb_unified_addr(Network::TestNetwork, request).prop_map(Address::Unified), + ]; + } } #[cfg(test)] mod tests { use zcash_address::test_vectors; - use zcash_primitives::{consensus::MAIN_NETWORK, zip32::AccountId}; + use zcash_primitives::consensus::MAIN_NETWORK; - use super::{Address, UnifiedAddress}; + use super::Address; + + #[cfg(feature = "sapling")] use crate::keys::sapling; + #[cfg(any(feature = "orchard", feature = "sapling"))] + use zcash_primitives::zip32::AccountId; + + #[cfg(any(feature = "orchard", feature = "sapling"))] + use super::UnifiedAddress; + #[test] + #[cfg(any(feature = "orchard", feature = "sapling"))] fn ua_round_trip() { #[cfg(feature = "orchard")] let orchard = { @@ -329,20 +375,24 @@ mod tests { Some(fvk.address_at(0u32, orchard::keys::Scope::External)) }; + #[cfg(feature = "sapling")] let sapling = { let extsk = sapling::spending_key(&[0; 32], 0, AccountId::ZERO); let dfvk = extsk.to_diversifiable_full_viewing_key(); Some(dfvk.default_address().1) }; - let transparent = { None }; + let transparent = None; - #[cfg(feature = "orchard")] + #[cfg(all(feature = "orchard", feature = "sapling"))] let ua = UnifiedAddress::from_receivers(orchard, sapling, transparent).unwrap(); - #[cfg(not(feature = "orchard"))] + #[cfg(all(not(feature = "orchard"), feature = "sapling"))] let ua = UnifiedAddress::from_receivers(sapling, transparent).unwrap(); + #[cfg(all(not(feature = "orchard"), not(feature = "sapling")))] + let ua = UnifiedAddress::from_receivers(transparent).unwrap(); + let addr = Address::Unified(ua); let addr_str = addr.encode(&MAIN_NETWORK); assert_eq!(Address::decode(&MAIN_NETWORK, &addr_str), Some(addr)); @@ -352,14 +402,15 @@ mod tests { fn ua_parsing() { for tv in test_vectors::UNIFIED { match Address::decode(&MAIN_NETWORK, tv.unified_addr) { - Some(Address::Unified(ua)) => { + Some(Address::Unified(_ua)) => { assert_eq!( - ua.transparent().is_some(), + _ua.transparent().is_some(), tv.p2pkh_bytes.is_some() || tv.p2sh_bytes.is_some() ); - assert_eq!(ua.sapling().is_some(), tv.sapling_raw_addr.is_some()); + #[cfg(feature = "sapling")] + assert_eq!(_ua.sapling().is_some(), tv.sapling_raw_addr.is_some()); #[cfg(feature = "orchard")] - assert_eq!(ua.orchard().is_some(), tv.orchard_raw_addr.is_some()); + assert_eq!(_ua.orchard().is_some(), tv.orchard_raw_addr.is_some()); } Some(_) => { panic!( diff --git a/zcash_keys/src/encoding.rs b/zcash_keys/src/encoding.rs index 0dad6bd62..7a280178c 100644 --- a/zcash_keys/src/encoding.rs +++ b/zcash_keys/src/encoding.rs @@ -4,15 +4,20 @@ //! [zcash_primitives::constants] module. use crate::address::UnifiedAddress; -use bech32::{self, Error, FromBase32, ToBase32, Variant}; use bs58::{self, decode::Error as Bs58Error}; use std::fmt; -use std::io::{self, Write}; -use sapling::zip32::{ExtendedFullViewingKey, ExtendedSpendingKey}; use zcash_address::unified::{self, Encoding}; use zcash_primitives::{consensus, legacy::TransparentAddress}; +#[cfg(feature = "sapling")] +use { + bech32::{self, Error, FromBase32, ToBase32, Variant}, + sapling::zip32::{ExtendedFullViewingKey, ExtendedSpendingKey}, + std::io::{self, Write}, +}; + +#[cfg(feature = "sapling")] fn bech32_encode(hrp: &str, write: F) -> String where F: Fn(&mut dyn Write) -> io::Result<()>, @@ -23,6 +28,7 @@ where } #[derive(Clone, Debug, PartialEq, Eq)] +#[cfg(feature = "sapling")] pub enum Bech32DecodeError { Bech32Error(Error), IncorrectVariant(Variant), @@ -30,12 +36,14 @@ pub enum Bech32DecodeError { HrpMismatch { expected: String, actual: String }, } +#[cfg(feature = "sapling")] impl From for Bech32DecodeError { fn from(err: Error) -> Self { Bech32DecodeError::Bech32Error(err) } } +#[cfg(feature = "sapling")] impl fmt::Display for Bech32DecodeError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match &self { @@ -57,6 +65,7 @@ impl fmt::Display for Bech32DecodeError { } } +#[cfg(feature = "sapling")] fn bech32_decode(hrp: &str, s: &str, read: F) -> Result where F: Fn(Vec) -> Option, @@ -140,6 +149,7 @@ impl AddressCodec

for TransparentAddress { } } +#[cfg(feature = "sapling")] impl AddressCodec

for sapling::PaymentAddress { type Error = Bech32DecodeError; @@ -193,6 +203,7 @@ impl AddressCodec

for UnifiedAddress { /// let encoded = encode_extended_spending_key(HRP_SAPLING_EXTENDED_SPENDING_KEY, &extsk); /// ``` /// [`ExtendedSpendingKey`]: sapling::zip32::ExtendedSpendingKey +#[cfg(feature = "sapling")] pub fn encode_extended_spending_key(hrp: &str, extsk: &ExtendedSpendingKey) -> String { bech32_encode(hrp, |w| extsk.write(w)) } @@ -200,6 +211,7 @@ pub fn encode_extended_spending_key(hrp: &str, extsk: &ExtendedSpendingKey) -> S /// Decodes an [`ExtendedSpendingKey`] from a Bech32-encoded string. /// /// [`ExtendedSpendingKey`]: sapling::zip32::ExtendedSpendingKey +#[cfg(feature = "sapling")] pub fn decode_extended_spending_key( hrp: &str, s: &str, @@ -227,6 +239,7 @@ pub fn decode_extended_spending_key( /// let encoded = encode_extended_full_viewing_key(HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY, &extfvk); /// ``` /// [`ExtendedFullViewingKey`]: sapling::zip32::ExtendedFullViewingKey +#[cfg(feature = "sapling")] pub fn encode_extended_full_viewing_key(hrp: &str, extfvk: &ExtendedFullViewingKey) -> String { bech32_encode(hrp, |w| extfvk.write(w)) } @@ -234,6 +247,7 @@ pub fn encode_extended_full_viewing_key(hrp: &str, extfvk: &ExtendedFullViewingK /// Decodes an [`ExtendedFullViewingKey`] from a Bech32-encoded string. /// /// [`ExtendedFullViewingKey`]: sapling::zip32::ExtendedFullViewingKey +#[cfg(feature = "sapling")] pub fn decode_extended_full_viewing_key( hrp: &str, s: &str, @@ -269,6 +283,7 @@ pub fn decode_extended_full_viewing_key( /// ); /// ``` /// [`PaymentAddress`]: sapling::PaymentAddress +#[cfg(feature = "sapling")] pub fn encode_payment_address(hrp: &str, addr: &sapling::PaymentAddress) -> String { bech32_encode(hrp, |w| w.write_all(&addr.to_bytes())) } @@ -278,6 +293,7 @@ pub fn encode_payment_address(hrp: &str, addr: &sapling::PaymentAddress) -> Stri /// network parameters. /// /// [`PaymentAddress`]: sapling::PaymentAddress +#[cfg(feature = "sapling")] pub fn encode_payment_address_p( params: &P, addr: &sapling::PaymentAddress, @@ -316,6 +332,7 @@ pub fn encode_payment_address_p( /// ); /// ``` /// [`PaymentAddress`]: sapling::PaymentAddress +#[cfg(feature = "sapling")] pub fn decode_payment_address( hrp: &str, s: &str, @@ -454,15 +471,15 @@ pub fn decode_transparent_address( } #[cfg(test)] -mod tests { - use sapling::{zip32::ExtendedSpendingKey, PaymentAddress}; - use zcash_primitives::constants; - +#[cfg(feature = "sapling")] +mod tests_sapling { use super::{ decode_extended_full_viewing_key, decode_extended_spending_key, decode_payment_address, encode_extended_full_viewing_key, encode_extended_spending_key, encode_payment_address, Bech32DecodeError, }; + use sapling::{zip32::ExtendedSpendingKey, PaymentAddress}; + use zcash_primitives::constants; #[test] fn extended_spending_key() { diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 95337a24c..849a6bf0e 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -31,6 +31,7 @@ use { #[cfg(feature = "orchard")] use orchard::{self, keys::Scope}; +#[cfg(feature = "sapling")] pub mod sapling { pub use sapling::zip32::{ DiversifiableFullViewingKey, ExtendedFullViewingKey, ExtendedSpendingKey, @@ -167,6 +168,7 @@ impl Era { pub struct UnifiedSpendingKey { #[cfg(feature = "transparent-inputs")] transparent: legacy::AccountPrivKey, + #[cfg(feature = "sapling")] sapling: sapling::ExtendedSpendingKey, #[cfg(feature = "orchard")] orchard: orchard::keys::SpendingKey, @@ -175,29 +177,27 @@ pub struct UnifiedSpendingKey { #[doc(hidden)] impl UnifiedSpendingKey { pub fn from_seed( - params: &P, - seed: &[u8], - account: AccountId, + _params: &P, + _seed: &[u8], + _account: AccountId, ) -> Result { - if seed.len() < 32 { + if _seed.len() < 32 { panic!("ZIP 32 seeds MUST be at least 32 bytes"); } - #[cfg(feature = "orchard")] - let orchard = - orchard::keys::SpendingKey::from_zip32_seed(seed, params.coin_type(), account) - .map_err(DerivationError::Orchard)?; - - #[cfg(feature = "transparent-inputs")] - let transparent = legacy::AccountPrivKey::from_seed(params, seed, account) - .map_err(DerivationError::Transparent)?; - Ok(UnifiedSpendingKey { #[cfg(feature = "transparent-inputs")] - transparent, - sapling: sapling::spending_key(seed, params.coin_type(), account), + transparent: legacy::AccountPrivKey::from_seed(_params, _seed, _account) + .map_err(DerivationError::Transparent)?, + #[cfg(feature = "sapling")] + sapling: sapling::spending_key(_seed, _params.coin_type(), _account), #[cfg(feature = "orchard")] - orchard, + orchard: orchard::keys::SpendingKey::from_zip32_seed( + _seed, + _params.coin_type(), + _account, + ) + .map_err(DerivationError::Orchard)?, }) } @@ -205,6 +205,7 @@ impl UnifiedSpendingKey { UnifiedFullViewingKey { #[cfg(feature = "transparent-inputs")] transparent: Some(self.transparent.to_account_pubkey()), + #[cfg(feature = "sapling")] sapling: Some(self.sapling.to_diversifiable_full_viewing_key()), #[cfg(feature = "orchard")] orchard: Some((&self.orchard).into()), @@ -220,6 +221,7 @@ impl UnifiedSpendingKey { } /// Returns the Sapling extended spending key component of this unified spending key. + #[cfg(feature = "sapling")] pub fn sapling(&self) -> &sapling::ExtendedSpendingKey { &self.sapling } @@ -254,15 +256,16 @@ impl UnifiedSpendingKey { result.write_all(orchard_key_bytes).unwrap(); } - // sapling - let sapling_key = self.sapling(); - CompactSize::write(&mut result, usize::try_from(Typecode::Sapling).unwrap()).unwrap(); + #[cfg(feature = "sapling")] + { + let sapling_key = self.sapling(); + CompactSize::write(&mut result, usize::try_from(Typecode::Sapling).unwrap()).unwrap(); - let sapling_key_bytes = sapling_key.to_bytes(); - CompactSize::write(&mut result, sapling_key_bytes.len()).unwrap(); - result.write_all(&sapling_key_bytes).unwrap(); + let sapling_key_bytes = sapling_key.to_bytes(); + CompactSize::write(&mut result, sapling_key_bytes.len()).unwrap(); + result.write_all(&sapling_key_bytes).unwrap(); + } - // transparent #[cfg(feature = "transparent-inputs")] { let account_tkey = self.transparent(); @@ -294,6 +297,7 @@ impl UnifiedSpendingKey { #[cfg(feature = "orchard")] let mut orchard = None; + #[cfg(feature = "sapling")] let mut sapling = None; #[cfg(feature = "transparent-inputs")] let mut transparent = None; @@ -335,12 +339,15 @@ impl UnifiedSpendingKey { source .read_exact(&mut key) .map_err(|_| DecodingError::InsufficientData(Typecode::Sapling))?; - sapling = Some( - sapling::ExtendedSpendingKey::from_bytes(&key) - .map_err(|_| DecodingError::KeyDataInvalid(Typecode::Sapling))?, - ); + + #[cfg(feature = "sapling")] + { + sapling = Some( + sapling::ExtendedSpendingKey::from_bytes(&key) + .map_err(|_| DecodingError::KeyDataInvalid(Typecode::Sapling))?, + ); + } } - #[cfg(feature = "transparent-inputs")] Typecode::P2pkh => { if len != 64 { return Err(DecodingError::LengthMismatch(Typecode::P2pkh, len)); @@ -350,10 +357,14 @@ impl UnifiedSpendingKey { source .read_exact(&mut key) .map_err(|_| DecodingError::InsufficientData(Typecode::P2pkh))?; - transparent = Some( - legacy::AccountPrivKey::from_bytes(&key) - .ok_or(DecodingError::KeyDataInvalid(Typecode::P2pkh))?, - ); + + #[cfg(feature = "transparent-inputs")] + { + transparent = Some( + legacy::AccountPrivKey::from_bytes(&key) + .ok_or(DecodingError::KeyDataInvalid(Typecode::P2pkh))?, + ); + } } _ => { return Err(DecodingError::TypecodeInvalid); @@ -365,15 +376,21 @@ impl UnifiedSpendingKey { #[cfg(not(feature = "orchard"))] let has_orchard = true; + #[cfg(feature = "sapling")] + let has_sapling = sapling.is_some(); + #[cfg(not(feature = "sapling"))] + let has_sapling = true; + #[cfg(feature = "transparent-inputs")] let has_transparent = transparent.is_some(); #[cfg(not(feature = "transparent-inputs"))] let has_transparent = true; - if has_orchard && sapling.is_some() && has_transparent { + if has_orchard && has_sapling && has_transparent { return Ok(UnifiedSpendingKey { #[cfg(feature = "orchard")] orchard: orchard.unwrap(), + #[cfg(feature = "sapling")] sapling: sapling.unwrap(), #[cfg(feature = "transparent-inputs")] transparent: transparent.unwrap(), @@ -382,7 +399,7 @@ impl UnifiedSpendingKey { } } - #[cfg(feature = "test-dependencies")] + #[cfg(any(test, feature = "test-dependencies"))] pub fn default_address( &self, request: UnifiedAddressRequest, @@ -390,7 +407,10 @@ impl UnifiedSpendingKey { self.to_unified_full_viewing_key().default_address(request) } - #[cfg(all(feature = "test-dependencies", feature = "transparent-inputs"))] + #[cfg(all( + feature = "transparent-inputs", + any(test, feature = "test-dependencies") + ))] pub fn default_transparent_address(&self) -> (TransparentAddress, NonHardenedChildIndex) { self.transparent() .to_account_pubkey() @@ -405,8 +425,10 @@ impl UnifiedSpendingKey { pub enum AddressGenerationError { /// The requested diversifier index was outside the range of valid transparent /// child address indices. + #[cfg(feature = "transparent-inputs")] InvalidTransparentChildIndex(DiversifierIndex), /// The diversifier index could not be mapped to a valid Sapling diversifier. + #[cfg(feature = "sapling")] InvalidSaplingDiversifierIndex(DiversifierIndex), /// A requested address typecode was not recognized, so we are unable to generate the address /// as requested. @@ -422,13 +444,12 @@ pub enum AddressGenerationError { /// Specification for how a unified address should be generated from a unified viewing key. #[derive(Clone, Copy, Debug)] pub struct UnifiedAddressRequest { - _has_orchard: bool, + has_orchard: bool, has_sapling: bool, has_p2pkh: bool, } impl UnifiedAddressRequest { - /// Construct a new unified address request from its constituent parts pub fn new(has_orchard: bool, has_sapling: bool, has_p2pkh: bool) -> Option { let has_shielded_receiver = has_orchard || has_sapling; @@ -436,7 +457,7 @@ impl UnifiedAddressRequest { None } else { Some(Self { - _has_orchard: has_orchard, + has_orchard, has_sapling, has_p2pkh, }) @@ -452,7 +473,7 @@ impl UnifiedAddressRequest { } Self { - _has_orchard: has_orchard, + has_orchard, has_sapling, has_p2pkh, } @@ -465,6 +486,7 @@ impl UnifiedAddressRequest { pub struct UnifiedFullViewingKey { #[cfg(feature = "transparent-inputs")] transparent: Option, + #[cfg(feature = "sapling")] sapling: Option, #[cfg(feature = "orchard")] orchard: Option, @@ -476,22 +498,20 @@ impl UnifiedFullViewingKey { /// Construct a new unified full viewing key, if the required components are present. pub fn new( #[cfg(feature = "transparent-inputs")] transparent: Option, - sapling: Option, + #[cfg(feature = "sapling")] sapling: Option, #[cfg(feature = "orchard")] orchard: Option, - ) -> Option { - if sapling.is_none() { - None - } else { - Some(UnifiedFullViewingKey { - #[cfg(feature = "transparent-inputs")] - transparent, - 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![], - }) + // TODO: Implement construction of UFVKs with metadata items. + ) -> UnifiedFullViewingKey { + 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![], } } @@ -510,6 +530,7 @@ impl UnifiedFullViewingKey { #[cfg(feature = "orchard")] let mut orchard = None; + #[cfg(feature = "sapling")] let mut sapling = None; #[cfg(feature = "transparent-inputs")] let mut transparent = None; @@ -529,9 +550,11 @@ impl UnifiedFullViewingKey { }) .transpose(), #[cfg(not(feature = "orchard"))] - unified::Fvk::Orchard(data) => { - Some(Ok((unified::Typecode::Orchard.into(), data.to_vec()))) - } + unified::Fvk::Orchard(data) => Some(Ok::<_, &'static str>(( + 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") @@ -541,6 +564,11 @@ impl UnifiedFullViewingKey { }) .transpose() } + #[cfg(not(feature = "sapling"))] + unified::Fvk::Sapling(data) => Some(Ok::<_, &'static str>(( + 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") @@ -550,9 +578,10 @@ impl UnifiedFullViewingKey { }) .transpose(), #[cfg(not(feature = "transparent-inputs"))] - unified::Fvk::P2pkh(data) => { - Some(Ok((unified::Typecode::P2pkh.into(), data.to_vec()))) - } + unified::Fvk::P2pkh(data) => Some(Ok::<_, &'static str>(( + u32::from(unified::Typecode::P2pkh), + data.to_vec(), + ))), unified::Fvk::Unknown { typecode, data } => Some(Ok((*typecode, data.clone()))), }) .collect::>()?; @@ -560,6 +589,7 @@ impl UnifiedFullViewingKey { Ok(Self { #[cfg(feature = "transparent-inputs")] transparent, + #[cfg(feature = "sapling")] sapling, #[cfg(feature = "orchard")] orchard, @@ -569,21 +599,12 @@ impl UnifiedFullViewingKey { /// Returns the string encoding of this `UnifiedFullViewingKey` for the given network. pub fn encode(&self, params: &P) -> String { - let items = std::iter::empty() - .chain( - self.sapling - .as_ref() - .map(|dfvk| dfvk.to_bytes()) - .map(unified::Fvk::Sapling), - ) - .chain( - self.unknown - .iter() - .map(|(typecode, data)| unified::Fvk::Unknown { - typecode: *typecode, - data: data.clone(), - }), - ); + let items = std::iter::empty().chain(self.unknown.iter().map(|(typecode, data)| { + unified::Fvk::Unknown { + typecode: *typecode, + data: data.clone(), + } + })); #[cfg(feature = "orchard")] let items = items.chain( self.orchard @@ -591,6 +612,13 @@ impl UnifiedFullViewingKey { .map(|fvk| fvk.to_bytes()) .map(unified::Fvk::Orchard), ); + #[cfg(feature = "sapling")] + let items = items.chain( + self.sapling + .as_ref() + .map(|dfvk| dfvk.to_bytes()) + .map(unified::Fvk::Sapling), + ); #[cfg(feature = "transparent-inputs")] let items = items.chain( self.transparent @@ -612,6 +640,7 @@ impl UnifiedFullViewingKey { } /// Returns the Sapling diversifiable full viewing key component of this unified key. + #[cfg(feature = "sapling")] pub fn sapling(&self) -> Option<&sapling::DiversifiableFullViewingKey> { self.sapling.as_ref() } @@ -628,39 +657,52 @@ impl UnifiedFullViewingKey { /// Returns `None` if the specified index does not produce a valid diversifier. pub fn address( &self, - j: DiversifierIndex, - request: UnifiedAddressRequest, + _j: DiversifierIndex, + _request: UnifiedAddressRequest, ) -> Result { #[cfg(feature = "orchard")] - let orchard = if request._has_orchard { + let mut orchard = None; + if _request.has_orchard { + #[cfg(not(feature = "orchard"))] + return Err(AddressGenerationError::ReceiverTypeNotSupported( + Typecode::Orchard, + )); + + #[cfg(feature = "orchard")] if let Some(ofvk) = &self.orchard { - let orchard_j = orchard::keys::DiversifierIndex::from(*j.as_bytes()); - Some(ofvk.address_at(orchard_j, Scope::External)) + let orchard_j = orchard::keys::DiversifierIndex::from(*_j.as_bytes()); + orchard = Some(ofvk.address_at(orchard_j, Scope::External)) } else { return Err(AddressGenerationError::KeyNotAvailable(Typecode::Orchard)); } - } else { - None - }; + } - let sapling = if request.has_sapling { + #[cfg(feature = "sapling")] + let mut sapling = None; + if _request.has_sapling { + #[cfg(not(feature = "sapling"))] + return Err(AddressGenerationError::ReceiverTypeNotSupported( + Typecode::Sapling, + )); + + #[cfg(feature = "sapling")] if let Some(extfvk) = &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. - Some( + sapling = Some( extfvk - .address(j) - .ok_or(AddressGenerationError::InvalidSaplingDiversifierIndex(j))?, - ) + .address(_j) + .ok_or(AddressGenerationError::InvalidSaplingDiversifierIndex(_j))?, + ); } else { return Err(AddressGenerationError::KeyNotAvailable(Typecode::Sapling)); } - } else { - None - }; + } - let transparent = if request.has_p2pkh { + #[cfg(feature = "transparent-inputs")] + let mut transparent = None; + if _request.has_p2pkh { #[cfg(not(feature = "transparent-inputs"))] return Err(AddressGenerationError::ReceiverTypeNotSupported( Typecode::P2pkh, @@ -671,30 +713,25 @@ impl UnifiedFullViewingKey { // If a transparent receiver type is requested, we must be able to construct an // address; if we're unable to do so, then no Unified Address exists at this // diversifier. - match to_transparent_child_index(j) { - Some(transparent_j) => match tfvk - .derive_external_ivk() + let transparent_j = to_transparent_child_index(_j) + .ok_or(AddressGenerationError::InvalidTransparentChildIndex(_j))?; + + transparent = Some( + tfvk.derive_external_ivk() .and_then(|tivk| tivk.derive_address(transparent_j)) - { - Ok(taddr) => Some(taddr), - Err(_) => { - return Err(AddressGenerationError::InvalidTransparentChildIndex(j)) - } - }, - // Diversifier doesn't generate a valid transparent child index, so we eagerly - // return `None`. - None => return Err(AddressGenerationError::InvalidTransparentChildIndex(j)), - } + .map_err(|_| AddressGenerationError::InvalidTransparentChildIndex(_j))?, + ); } else { return Err(AddressGenerationError::KeyNotAvailable(Typecode::P2pkh)); } - } else { - None - }; + } + #[cfg(not(feature = "transparent-inputs"))] + let transparent = None; UnifiedAddress::from_receivers( #[cfg(feature = "orchard")] orchard, + #[cfg(feature = "sapling")] sapling, transparent, ) @@ -706,6 +743,7 @@ impl UnifiedFullViewingKey { /// diversifier along with the index at which the valid diversifier was found. /// /// Returns `None` if no valid diversifier exists + #[allow(unused_mut)] pub fn find_address( &self, mut j: DiversifierIndex, @@ -726,6 +764,7 @@ impl UnifiedFullViewingKey { Ok(ua) => { break Some((ua, j)); } + #[cfg(feature = "sapling")] Err(AddressGenerationError::InvalidSaplingDiversifierIndex(_)) => { if j.increment().is_err() { break None; @@ -776,10 +815,15 @@ pub mod testing { mod tests { use proptest::prelude::proptest; - use super::{sapling, UnifiedFullViewingKey}; + use super::UnifiedFullViewingKey; use zcash_primitives::consensus::MAIN_NETWORK; + + #[cfg(any(feature = "orchard", feature = "sapling"))] use zip32::AccountId; + #[cfg(feature = "sapling")] + use super::sapling; + #[cfg(feature = "transparent-inputs")] use { crate::{address::Address, encoding::AddressCodec}, @@ -808,6 +852,7 @@ mod tests { #[test] #[should_panic] + #[cfg(feature = "sapling")] fn spending_key_panics_on_short_seed() { let _ = sapling::spending_key(&[0; 31][..], 0, AccountId::ZERO); } @@ -831,8 +876,6 @@ mod tests { #[test] fn ufvk_round_trip() { - let account = AccountId::ZERO; - #[cfg(feature = "orchard")] let orchard = { let sk = @@ -840,25 +883,27 @@ mod tests { Some(orchard::keys::FullViewingKey::from(&sk)) }; + #[cfg(feature = "sapling")] let sapling = { - let extsk = sapling::spending_key(&[0; 32], 0, account); + let extsk = sapling::spending_key(&[0; 32], 0, AccountId::ZERO); Some(extsk.to_diversifiable_full_viewing_key()) }; #[cfg(feature = "transparent-inputs")] let transparent = { - let privkey = AccountPrivKey::from_seed(&MAIN_NETWORK, &[0; 32], account).unwrap(); + let privkey = + AccountPrivKey::from_seed(&MAIN_NETWORK, &[0; 32], AccountId::ZERO).unwrap(); Some(privkey.to_account_pubkey()) }; let ufvk = UnifiedFullViewingKey::new( #[cfg(feature = "transparent-inputs")] transparent, + #[cfg(feature = "sapling")] sapling, #[cfg(feature = "orchard")] orchard, - ) - .unwrap(); + ); let encoded = ufvk.encode(&MAIN_NETWORK); @@ -867,10 +912,10 @@ mod tests { let encoded_with_t = "uview1tg6rpjgju2s2j37gkgjq79qrh5lvzr6e0ed3n4sf4hu5qd35vmsh7avl80xa6mx7ryqce9hztwaqwrdthetpy4pc0kce25x453hwcmax02p80pg5savlg865sft9reat07c5vlactr6l2pxtlqtqunt2j9gmvr8spcuzf07af80h5qmut38h0gvcfa9k4rwujacwwca9vu8jev7wq6c725huv8qjmhss3hdj2vh8cfxhpqcm2qzc34msyrfxk5u6dqttt4vv2mr0aajreww5yufpk0gn4xkfm888467k7v6fmw7syqq6cceu078yw8xja502jxr0jgum43lhvpzmf7eu5dmnn6cr6f7p43yw8znzgxg598mllewnx076hljlvynhzwn5es94yrv65tdg3utuz2u3sras0wfcq4adxwdvlk387d22g3q98t5z74quw2fa4wed32escx8dwh4mw35t4jwf35xyfxnu83mk5s4kw2glkgsshmxk"; let _encoded_no_t = "uview12z384wdq76ceewlsu0esk7d97qnd23v2qnvhujxtcf2lsq8g4hwzpx44fwxssnm5tg8skyh4tnc8gydwxefnnm0hd0a6c6etmj0pp9jqkdsllkr70u8gpf7ndsfqcjlqn6dec3faumzqlqcmtjf8vp92h7kj38ph2786zx30hq2wru8ae3excdwc8w0z3t9fuw7mt7xy5sn6s4e45kwm0cjp70wytnensgdnev286t3vew3yuwt2hcz865y037k30e428dvgne37xvyeal2vu8yjnznphf9t2rw3gdp0hk5zwq00ws8f3l3j5n3qkqgsyzrwx4qzmgq0xwwk4vz2r6vtsykgz089jncvycmem3535zjwvvtvjw8v98y0d5ydwte575gjm7a7k"; - // We test the full roundtrip only with the `orchard` feature enabled, because we will - // not generate the `orchard` part of the encoding if the UFVK does not have an Orchard - // part. - #[cfg(feature = "orchard")] + // 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); @@ -887,6 +932,7 @@ mod tests { 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()), @@ -904,12 +950,42 @@ mod tests { ufvk.transparent.as_ref().map(|t| t.serialize()), ); - #[cfg(all(not(feature = "orchard"), not(feature = "transparent-inputs")))] + #[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); + #[cfg(all( + feature = "orchard", + not(feature = "sapling"), + not(feature = "transparent-inputs") + ))] assert_eq!(decoded_with_t.unknown.len(), 2); - #[cfg(all(feature = "transparent-inputs", not(feature = "orchard")))] - assert_eq!(decoded_with_t.unknown.len(), 1); - #[cfg(all(feature = "orchard", not(feature = "transparent-inputs")))] + #[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); + #[cfg(all( + not(feature = "orchard"), + not(feature = "sapling"), + not(feature = "transparent-inputs") + ))] + assert_eq!(decoded_with_t.unknown.len(), 3); } #[test] From 2a6330f2ea903897cd355c23eff9abb0f93cae14 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 23 Feb 2024 08:58:22 -0700 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: str4d --- zcash_client_backend/src/data_api/wallet.rs | 6 +- zcash_client_sqlite/Cargo.toml | 2 +- zcash_keys/src/address.rs | 11 +- zcash_keys/src/keys.rs | 212 +++++++++++--------- 4 files changed, 130 insertions(+), 101 deletions(-) diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index f941392fb..c8f383a16 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -166,11 +166,7 @@ where /// }; /// /// let account = AccountId::from(0); -/// let req = UnifiedAddressRequest { -/// has_orchard: false, -/// has_sapling: true, -/// has_p2pkh: true -/// }; +/// let req = UnifiedAddressRequest::new(false, true, true); /// let usk = UnifiedSpendingKey::from_seed(&Network::TestNetwork, &[0; 32][..], account).unwrap(); /// let to = usk.to_unified_full_viewing_key().default_address(req).0.into(); /// diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 30d9e2fe0..3a4c160e6 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -22,7 +22,7 @@ rustdoc-args = ["--cfg", "docsrs"] zcash_address.workspace = true zcash_client_backend = { workspace = true, features = ["unstable-serialization", "unstable-spanning-tree"] } zcash_encoding.workspace = true -zcash_keys = { workspace = true, features = ["sapling"] } +zcash_keys = { workspace = true, features = ["orchard", "sapling"] } zcash_primitives.workspace = true # Dependencies exposed in a public API: diff --git a/zcash_keys/src/address.rs b/zcash_keys/src/address.rs index b8c309510..6872e5133 100644 --- a/zcash_keys/src/address.rs +++ b/zcash_keys/src/address.rs @@ -390,14 +390,21 @@ mod tests { #[cfg(all(not(feature = "orchard"), feature = "sapling"))] let ua = UnifiedAddress::from_receivers(sapling, transparent).unwrap(); - #[cfg(all(not(feature = "orchard"), not(feature = "sapling")))] - let ua = UnifiedAddress::from_receivers(transparent).unwrap(); + #[cfg(all(feature = "orchard", not(feature = "sapling")))] + let ua = UnifiedAddress::from_receivers(orchard, transparent).unwrap(); let addr = Address::Unified(ua); let addr_str = addr.encode(&MAIN_NETWORK); assert_eq!(Address::decode(&MAIN_NETWORK, &addr_str), Some(addr)); } + #[test] + #[cfg(not(any(feature = "orchard", feature = "sapling")))] + fn ua_round_trip() { + let transparent = None; + assert_eq!(UnifiedAddress::from_receivers(transparent), None) + } + #[test] fn ua_parsing() { for tv in test_vectors::UNIFIED { diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 849a6bf0e..0beebe64a 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -178,22 +178,22 @@ pub struct UnifiedSpendingKey { impl UnifiedSpendingKey { pub fn from_seed( _params: &P, - _seed: &[u8], + seed: &[u8], _account: AccountId, ) -> Result { - if _seed.len() < 32 { + if seed.len() < 32 { panic!("ZIP 32 seeds MUST be at least 32 bytes"); } Ok(UnifiedSpendingKey { #[cfg(feature = "transparent-inputs")] - transparent: legacy::AccountPrivKey::from_seed(_params, _seed, _account) + transparent: legacy::AccountPrivKey::from_seed(_params, seed, _account) .map_err(DerivationError::Transparent)?, #[cfg(feature = "sapling")] - sapling: sapling::spending_key(_seed, _params.coin_type(), _account), + sapling: sapling::spending_key(seed, _params.coin_type(), _account), #[cfg(feature = "orchard")] orchard: orchard::keys::SpendingKey::from_zip32_seed( - _seed, + seed, _params.coin_type(), _account, ) @@ -501,17 +501,30 @@ impl UnifiedFullViewingKey { #[cfg(feature = "sapling")] sapling: Option, #[cfg(feature = "orchard")] orchard: Option, // TODO: Implement construction of UFVKs with metadata items. - ) -> UnifiedFullViewingKey { - 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![], + ) -> 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(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 } } @@ -905,87 +918,100 @@ mod tests { orchard, ); - let encoded = ufvk.encode(&MAIN_NETWORK); + #[cfg(not(any(feature = "orchard", feature = "sapling")))] + assert_eq!(ufvk, None); - // 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(any(feature = "orchard", feature = "sapling"))] { + 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!(encoded, encoded_with_t); - #[cfg(not(feature = "transparent-inputs"))] - assert_eq!(encoded, _encoded_no_t); + 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(), 2); + #[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); } - - 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()), - ); - - #[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); - #[cfg(all( - feature = "orchard", - not(feature = "sapling"), - not(feature = "transparent-inputs") - ))] - assert_eq!(decoded_with_t.unknown.len(), 2); - #[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); - #[cfg(all( - not(feature = "orchard"), - not(feature = "sapling"), - not(feature = "transparent-inputs") - ))] - assert_eq!(decoded_with_t.unknown.len(), 3); } #[test] From 635bc3158b1885ad5b6f9f75009d0fb9bb5ce281 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 26 Feb 2024 18:33:19 -0700 Subject: [PATCH 3/3] Fix an error in decoding tests. Co-authored-by: str4d --- zcash_keys/src/keys.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 0beebe64a..117e07812 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -990,7 +990,7 @@ mod tests { not(feature = "sapling"), feature = "transparent-inputs" ))] - assert_eq!(decoded_with_t.unknown.len(), 2); + assert_eq!(decoded_with_t.unknown.len(), 1); #[cfg(all( feature = "orchard", not(feature = "sapling"),