From 5d7a1857ee6edf9fdf3501583594a18515f60975 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 20 Dec 2024 13:49:51 -0700 Subject: [PATCH 1/9] zcash_protocol: Add unified address/fvk/ivk HRPs retrieval methods to `NetworkConstants` trait. --- .../zcash_address/src/kind/unified/address.rs | 8 +-- .../zcash_address/src/kind/unified/fvk.rs | 11 +++- .../zcash_address/src/kind/unified/ivk.rs | 7 ++- components/zcash_protocol/CHANGELOG.md | 6 ++ components/zcash_protocol/src/consensus.rs | 57 +++++++++++++++++++ .../zcash_protocol/src/constants/mainnet.rs | 21 +++++++ .../zcash_protocol/src/constants/regtest.rs | 13 +++++ .../zcash_protocol/src/constants/testnet.rs | 21 +++++++ 8 files changed, 134 insertions(+), 10 deletions(-) diff --git a/components/zcash_address/src/kind/unified/address.rs b/components/zcash_address/src/kind/unified/address.rs index 0a4423bf0c..41be7ab992 100644 --- a/components/zcash_address/src/kind/unified/address.rs +++ b/components/zcash_address/src/kind/unified/address.rs @@ -1,4 +1,4 @@ -use zcash_protocol::PoolType; +use zcash_protocol::{constants, PoolType}; use super::{private::SealedItem, ParseError, Typecode}; @@ -136,17 +136,17 @@ impl super::private::SealedContainer for Address { /// Defined in [ZIP 316][zip-0316]. /// /// [zip-0316]: https://zips.z.cash/zip-0316 - const MAINNET: &'static str = "u"; + const MAINNET: &'static str = constants::mainnet::HRP_UNIFIED_ADDRESS; /// The HRP for a Bech32m-encoded testnet Unified Address. /// /// Defined in [ZIP 316][zip-0316]. /// /// [zip-0316]: https://zips.z.cash/zip-0316 - const TESTNET: &'static str = "utest"; + const TESTNET: &'static str = constants::testnet::HRP_UNIFIED_ADDRESS; /// The HRP for a Bech32m-encoded regtest Unified Address. - const REGTEST: &'static str = "uregtest"; + const REGTEST: &'static str = constants::regtest::HRP_UNIFIED_ADDRESS; fn from_inner(receivers: Vec) -> Self { Self(receivers) diff --git a/components/zcash_address/src/kind/unified/fvk.rs b/components/zcash_address/src/kind/unified/fvk.rs index 52655dc703..534d6c7834 100644 --- a/components/zcash_address/src/kind/unified/fvk.rs +++ b/components/zcash_address/src/kind/unified/fvk.rs @@ -1,5 +1,6 @@ use alloc::vec::Vec; use core::convert::{TryFrom, TryInto}; +use zcash_protocol::constants; use super::{ private::{SealedContainer, SealedItem}, @@ -128,17 +129,21 @@ impl SealedContainer for Ufvk { /// Defined in [ZIP 316][zip-0316]. /// /// [zip-0316]: https://zips.z.cash/zip-0316 - const MAINNET: &'static str = "uview"; + const MAINNET: &'static str = constants::mainnet::HRP_UNIFIED_FVK; /// The HRP for a Bech32m-encoded testnet Unified FVK. /// /// Defined in [ZIP 316][zip-0316]. /// /// [zip-0316]: https://zips.z.cash/zip-0316 - const TESTNET: &'static str = "uviewtest"; + const TESTNET: &'static str = constants::testnet::HRP_UNIFIED_FVK; /// The HRP for a Bech32m-encoded regtest Unified FVK. - const REGTEST: &'static str = "uviewregtest"; + /// + /// Defined in [ZIP 316][zip-0316]. + /// + /// [zip-0316]: https://zips.z.cash/zip-0316 + const REGTEST: &'static str = constants::regtest::HRP_UNIFIED_FVK; fn from_inner(fvks: Vec) -> Self { Self(fvks) diff --git a/components/zcash_address/src/kind/unified/ivk.rs b/components/zcash_address/src/kind/unified/ivk.rs index 138c44fb09..fa613faf3e 100644 --- a/components/zcash_address/src/kind/unified/ivk.rs +++ b/components/zcash_address/src/kind/unified/ivk.rs @@ -1,4 +1,5 @@ use alloc::vec::Vec; +use zcash_protocol::constants; use core::convert::{TryFrom, TryInto}; use super::{ @@ -133,17 +134,17 @@ impl SealedContainer for Uivk { /// Defined in [ZIP 316][zip-0316]. /// /// [zip-0316]: https://zips.z.cash/zip-0316 - const MAINNET: &'static str = "uivk"; + const MAINNET: &'static str = constants::mainnet::HRP_UNIFIED_IVK; /// The HRP for a Bech32m-encoded testnet Unified IVK. /// /// Defined in [ZIP 316][zip-0316]. /// /// [zip-0316]: https://zips.z.cash/zip-0316 - const TESTNET: &'static str = "uivktest"; + const TESTNET: &'static str = constants::testnet::HRP_UNIFIED_IVK; /// The HRP for a Bech32m-encoded regtest Unified IVK. - const REGTEST: &'static str = "uivkregtest"; + const REGTEST: &'static str = constants::regtest::HRP_UNIFIED_IVK; fn from_inner(ivks: Vec) -> Self { Self(ivks) diff --git a/components/zcash_protocol/CHANGELOG.md b/components/zcash_protocol/CHANGELOG.md index 79c2cba44a..8ebbfd656e 100644 --- a/components/zcash_protocol/CHANGELOG.md +++ b/components/zcash_protocol/CHANGELOG.md @@ -7,6 +7,12 @@ and this library adheres to Rust's notion of ## [Unreleased] +### Changed +- `zcash_protocol::consensus::NetworkConstants` has added methods: + - `hrp_unified_address` + - `hrp_unified_fvk` + - `hrp_unified_ivk` + ## [0.4.3] - 2024-12-16 ### Added - `zcash_protocol::TxId` (moved from `zcash_primitives::transaction`). diff --git a/components/zcash_protocol/src/consensus.rs b/components/zcash_protocol/src/consensus.rs index 007fbe6c63..6648089e97 100644 --- a/components/zcash_protocol/src/consensus.rs +++ b/components/zcash_protocol/src/consensus.rs @@ -189,6 +189,27 @@ pub trait NetworkConstants: Clone { /// /// [ZIP 320]: https://zips.z.cash/zip-0320 fn hrp_tex_address(&self) -> &'static str; + + /// The HRP for a Bech32m-encoded mainnet Unified Address. + /// + /// Defined in [ZIP 316][zip-0316]. + /// + /// [zip-0316]: https://zips.z.cash/zip-0316 + fn hrp_unified_address(&self) -> &'static str; + + /// The HRP for a Bech32m-encoded mainnet Unified FVK. + /// + /// Defined in [ZIP 316][zip-0316]. + /// + /// [zip-0316]: https://zips.z.cash/zip-0316 + fn hrp_unified_fvk(&self) -> &'static str; + + /// The HRP for a Bech32m-encoded mainnet Unified IVK. + /// + /// Defined in [ZIP 316][zip-0316]. + /// + /// [zip-0316]: https://zips.z.cash/zip-0316 + fn hrp_unified_ivk(&self) -> &'static str; } /// The enumeration of known Zcash network types. @@ -272,6 +293,30 @@ impl NetworkConstants for NetworkType { NetworkType::Regtest => regtest::HRP_TEX_ADDRESS, } } + + fn hrp_unified_address(&self) -> &'static str { + match self { + NetworkType::Main => mainnet::HRP_UNIFIED_ADDRESS, + NetworkType::Test => testnet::HRP_UNIFIED_ADDRESS, + NetworkType::Regtest => regtest::HRP_UNIFIED_ADDRESS, + } + } + + fn hrp_unified_fvk(&self) -> &'static str { + match self { + NetworkType::Main => mainnet::HRP_UNIFIED_FVK, + NetworkType::Test => testnet::HRP_UNIFIED_FVK, + NetworkType::Regtest => regtest::HRP_UNIFIED_FVK, + } + } + + fn hrp_unified_ivk(&self) -> &'static str { + match self { + NetworkType::Main => mainnet::HRP_UNIFIED_IVK, + NetworkType::Test => testnet::HRP_UNIFIED_IVK, + NetworkType::Regtest => regtest::HRP_UNIFIED_IVK, + } + } } /// Zcash consensus parameters. @@ -322,6 +367,18 @@ impl NetworkConstants for P { fn hrp_tex_address(&self) -> &'static str { self.network_type().hrp_tex_address() } + + fn hrp_unified_address(&self) -> &'static str { + self.network_type().hrp_unified_address() + } + + fn hrp_unified_fvk(&self) -> &'static str { + self.network_type().hrp_unified_fvk() + } + + fn hrp_unified_ivk(&self) -> &'static str { + self.network_type().hrp_unified_ivk() + } } /// Marker struct for the production network. diff --git a/components/zcash_protocol/src/constants/mainnet.rs b/components/zcash_protocol/src/constants/mainnet.rs index 98c81caa25..ada1a42d16 100644 --- a/components/zcash_protocol/src/constants/mainnet.rs +++ b/components/zcash_protocol/src/constants/mainnet.rs @@ -50,3 +50,24 @@ pub const B58_SCRIPT_ADDRESS_PREFIX: [u8; 2] = [0x1c, 0xbd]; /// /// [ZIP 320]: https://zips.z.cash/zip-0320 pub const HRP_TEX_ADDRESS: &str = "tex"; + +/// The HRP for a Bech32m-encoded mainnet Unified Address. +/// +/// Defined in [ZIP 316][zip-0316]. +/// +/// [zip-0316]: https://zips.z.cash/zip-0316 +pub const HRP_UNIFIED_ADDRESS: &str = "u"; + +/// The HRP for a Bech32m-encoded mainnet Unified FVK. +/// +/// Defined in [ZIP 316][zip-0316]. +/// +/// [zip-0316]: https://zips.z.cash/zip-0316 +pub const HRP_UNIFIED_FVK: &str = "uview"; + +/// The HRP for a Bech32m-encoded mainnet Unified IVK. +/// +/// Defined in [ZIP 316][zip-0316]. +/// +/// [zip-0316]: https://zips.z.cash/zip-0316 +pub const HRP_UNIFIED_IVK: &str = "uivk"; diff --git a/components/zcash_protocol/src/constants/regtest.rs b/components/zcash_protocol/src/constants/regtest.rs index 001baa7ea4..c78f9a2950 100644 --- a/components/zcash_protocol/src/constants/regtest.rs +++ b/components/zcash_protocol/src/constants/regtest.rs @@ -57,3 +57,16 @@ pub const B58_SCRIPT_ADDRESS_PREFIX: [u8; 2] = [0x1c, 0xba]; /// /// [ZIP 320]: https://zips.z.cash/zip-0320 pub const HRP_TEX_ADDRESS: &str = "texregtest"; + +/// The HRP for a Bech32m-encoded regtest Unified Address. +/// +/// Defined in [ZIP 316][zip-0316]. +/// +/// [zip-0316]: https://zips.z.cash/zip-0316 +pub const HRP_UNIFIED_ADDRESS: &str = "uregtest"; + +/// The HRP for a Bech32m-encoded regtest Unified FVK. +pub const HRP_UNIFIED_FVK: &str = "uviewregtest"; + +/// The HRP for a Bech32m-encoded regtest Unified IVK. +pub const HRP_UNIFIED_IVK: &str = "uivkregtest"; diff --git a/components/zcash_protocol/src/constants/testnet.rs b/components/zcash_protocol/src/constants/testnet.rs index 023926546e..52e90a2561 100644 --- a/components/zcash_protocol/src/constants/testnet.rs +++ b/components/zcash_protocol/src/constants/testnet.rs @@ -50,3 +50,24 @@ pub const B58_SCRIPT_ADDRESS_PREFIX: [u8; 2] = [0x1c, 0xba]; /// /// [ZIP 320]: https://zips.z.cash/zip-0320 pub const HRP_TEX_ADDRESS: &str = "textest"; + +/// The HRP for a Bech32m-encoded testnet Unified Address. +/// +/// Defined in [ZIP 316][zip-0316]. +/// +/// [zip-0316]: https://zips.z.cash/zip-0316 +pub const HRP_UNIFIED_ADDRESS: &str = "utest"; + +/// The HRP for a Bech32m-encoded testnet Unified FVK. +/// +/// Defined in [ZIP 316][zip-0316]. +/// +/// [zip-0316]: https://zips.z.cash/zip-0316 +pub const HRP_UNIFIED_FVK: &str = "uviewtest"; + +/// The HRP for a Bech32m-encoded testnet Unified IVK. +/// +/// Defined in [ZIP 316][zip-0316]. +/// +/// [zip-0316]: https://zips.z.cash/zip-0316 +pub const HRP_UNIFIED_IVK: &str = "uivktest"; From da98f9f2b58d93ba6cc204b86229d6b9a87a0f93 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 23 Dec 2024 12:35:06 -0700 Subject: [PATCH 2/9] zcash_client_sqlite: factor out commonalities of account retrieval --- zcash_client_sqlite/src/wallet.rs | 127 ++++++++++++++---------------- 1 file changed, 57 insertions(+), 70 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index a3cd7d224c..e86ddb67ae 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -753,9 +753,9 @@ pub(crate) fn get_account_for_ufvk( let transparent_item: Option> = None; let mut stmt = conn.prepare( - "SELECT name, uuid, account_kind, - hd_seed_fingerprint, hd_account_index, key_source, - ufvk, has_spend_key + "SELECT name, uuid, account_kind, + hd_seed_fingerprint, hd_account_index, key_source, + ufvk, uivk, has_spend_key FROM accounts WHERE orchard_fvk_item_cache = :orchard_fvk_item_cache OR sapling_fvk_item_cache = :sapling_fvk_item_cache @@ -769,36 +769,7 @@ pub(crate) fn get_account_for_ufvk( ":sapling_fvk_item_cache": sapling_item, ":p2pkh_fvk_item_cache": transparent_item, ], - |row| { - let account_name = row.get("name")?; - let account_uuid = AccountUuid(row.get("uuid")?); - let kind = parse_account_source( - row.get("account_kind")?, - row.get("hd_seed_fingerprint")?, - row.get("hd_account_index")?, - row.get("has_spend_key")?, - row.get("key_source")?, - )?; - - // We looked up the account by FVK components, so the UFVK column must be - // non-null. - let ufvk_str: String = row.get("ufvk")?; - let viewing_key = ViewingKey::Full(Box::new( - UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { - SqliteClientError::CorruptedData(format!( - "Could not decode unified full viewing key for account {}: {}", - account_uuid.0, e - )) - })?, - )); - - Ok(Account { - name: account_name, - uuid: account_uuid, - kind, - viewing_key, - }) - }, + |row| parse_account_row(row, params), )? .collect::, _>>()?; @@ -811,6 +782,50 @@ pub(crate) fn get_account_for_ufvk( } } +fn parse_account_row( + row: &rusqlite::Row<'_>, + params: &P, +) -> Result { + let account_name = row.get("name")?; + let account_uuid = AccountUuid(row.get("uuid")?); + let kind = parse_account_source( + row.get("account_kind")?, + row.get("hd_seed_fingerprint")?, + row.get("hd_account_index")?, + row.get("has_spend_key")?, + row.get("key_source")?, + )?; + + let ufvk_str: Option = row.get("ufvk")?; + let viewing_key = if let Some(ufvk_str) = ufvk_str { + ViewingKey::Full(Box::new( + UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { + SqliteClientError::CorruptedData(format!( + "Could not decode unified full viewing key for account {}: {}", + account_uuid.0, e + )) + })?, + )) + } else { + let uivk_str: String = row.get("uivk")?; + ViewingKey::Incoming(Box::new( + UnifiedIncomingViewingKey::decode(params, &uivk_str).map_err(|e| { + SqliteClientError::CorruptedData(format!( + "Could not decode unified incoming viewing key for account {}: {}", + account_uuid.0, e + )) + })?, + )) + }; + + Ok(Account { + name: account_name, + uuid: account_uuid, + kind, + viewing_key, + }) +} + /// Returns the account id corresponding to a given [`SeedFingerprint`] /// and [`zip32::AccountId`], if any. pub(crate) fn get_derived_account( @@ -1919,50 +1934,22 @@ pub(crate) fn get_account( params: &P, account_uuid: AccountUuid, ) -> Result, SqliteClientError> { - let mut sql = conn.prepare_cached( + let mut stmt = conn.prepare_cached( r#" - SELECT name, account_kind, hd_seed_fingerprint, hd_account_index, key_source, ufvk, uivk, has_spend_key + SELECT name, uuid, account_kind, + hd_seed_fingerprint, hd_account_index, key_source, + ufvk, uivk, has_spend_key FROM accounts WHERE uuid = :account_uuid "#, )?; - let mut result = sql.query(named_params![":account_uuid": account_uuid.0])?; - let row = result.next()?; - match row { - Some(row) => { - let account_name = row.get("name")?; - let kind = parse_account_source( - row.get("account_kind")?, - row.get("hd_seed_fingerprint")?, - row.get("hd_account_index")?, - row.get("has_spend_key")?, - row.get("key_source")?, - )?; - - let ufvk_str: Option = row.get("ufvk")?; - let viewing_key = if let Some(ufvk_str) = ufvk_str { - ViewingKey::Full(Box::new( - UnifiedFullViewingKey::decode(params, &ufvk_str[..]) - .map_err(SqliteClientError::BadAccountData)?, - )) - } else { - let uivk_str: String = row.get("uivk")?; - ViewingKey::Incoming(Box::new( - UnifiedIncomingViewingKey::decode(params, &uivk_str[..]) - .map_err(SqliteClientError::BadAccountData)?, - )) - }; + let mut rows = stmt.query_and_then::<_, SqliteClientError, _, _>( + named_params![":account_uuid": account_uuid.0], + |row| parse_account_row(row, params), + )?; - Ok(Some(Account { - name: account_name, - uuid: account_uuid, - kind, - viewing_key, - })) - } - None => Ok(None), - } + rows.next().transpose() } /// Returns the minimum and maximum heights of blocks in the chain which may be scanned. From 736bfd555b1c72e92903b400539426d38f502815 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 23 Dec 2024 13:17:07 -0700 Subject: [PATCH 3/9] Move-only: group account retrieval methods together. --- .../zcash_address/src/kind/unified/ivk.rs | 2 +- zcash_client_sqlite/src/wallet.rs | 134 +++++++++--------- 2 files changed, 68 insertions(+), 68 deletions(-) diff --git a/components/zcash_address/src/kind/unified/ivk.rs b/components/zcash_address/src/kind/unified/ivk.rs index fa613faf3e..7b776b0008 100644 --- a/components/zcash_address/src/kind/unified/ivk.rs +++ b/components/zcash_address/src/kind/unified/ivk.rs @@ -1,6 +1,6 @@ use alloc::vec::Vec; -use zcash_protocol::constants; use core::convert::{TryFrom, TryInto}; +use zcash_protocol::constants; use super::{ private::{SealedContainer, SealedItem}, diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index e86ddb67ae..133dfa3fc2 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -733,6 +733,73 @@ pub(crate) fn get_unified_full_viewing_keys( Ok(res) } +fn parse_account_row( + row: &rusqlite::Row<'_>, + params: &P, +) -> Result { + let account_name = row.get("name")?; + let account_uuid = AccountUuid(row.get("uuid")?); + let kind = parse_account_source( + row.get("account_kind")?, + row.get("hd_seed_fingerprint")?, + row.get("hd_account_index")?, + row.get("has_spend_key")?, + row.get("key_source")?, + )?; + + let ufvk_str: Option = row.get("ufvk")?; + let viewing_key = if let Some(ufvk_str) = ufvk_str { + ViewingKey::Full(Box::new( + UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { + SqliteClientError::CorruptedData(format!( + "Could not decode unified full viewing key for account {}: {}", + account_uuid.0, e + )) + })?, + )) + } else { + let uivk_str: String = row.get("uivk")?; + ViewingKey::Incoming(Box::new( + UnifiedIncomingViewingKey::decode(params, &uivk_str).map_err(|e| { + SqliteClientError::CorruptedData(format!( + "Could not decode unified incoming viewing key for account {}: {}", + account_uuid.0, e + )) + })?, + )) + }; + + Ok(Account { + name: account_name, + uuid: account_uuid, + kind, + viewing_key, + }) +} + +pub(crate) fn get_account( + conn: &rusqlite::Connection, + params: &P, + account_uuid: AccountUuid, +) -> Result, SqliteClientError> { + let mut stmt = conn.prepare_cached( + r#" + SELECT name, uuid, account_kind, + hd_seed_fingerprint, hd_account_index, key_source, + ufvk, uivk, has_spend_key + FROM accounts + WHERE uuid = :account_uuid + "#, + )?; + + let mut rows = stmt.query_and_then::<_, SqliteClientError, _, _>( + named_params![":account_uuid": account_uuid.0], + |row| parse_account_row(row, params), + )?; + + rows.next().transpose() +} + /// Returns the account id corresponding to a given [`UnifiedFullViewingKey`], /// if any. pub(crate) fn get_account_for_ufvk( @@ -782,50 +849,6 @@ pub(crate) fn get_account_for_ufvk( } } -fn parse_account_row( - row: &rusqlite::Row<'_>, - params: &P, -) -> Result { - let account_name = row.get("name")?; - let account_uuid = AccountUuid(row.get("uuid")?); - let kind = parse_account_source( - row.get("account_kind")?, - row.get("hd_seed_fingerprint")?, - row.get("hd_account_index")?, - row.get("has_spend_key")?, - row.get("key_source")?, - )?; - - let ufvk_str: Option = row.get("ufvk")?; - let viewing_key = if let Some(ufvk_str) = ufvk_str { - ViewingKey::Full(Box::new( - UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { - SqliteClientError::CorruptedData(format!( - "Could not decode unified full viewing key for account {}: {}", - account_uuid.0, e - )) - })?, - )) - } else { - let uivk_str: String = row.get("uivk")?; - ViewingKey::Incoming(Box::new( - UnifiedIncomingViewingKey::decode(params, &uivk_str).map_err(|e| { - SqliteClientError::CorruptedData(format!( - "Could not decode unified incoming viewing key for account {}: {}", - account_uuid.0, e - )) - })?, - )) - }; - - Ok(Account { - name: account_name, - uuid: account_uuid, - kind, - viewing_key, - }) -} - /// Returns the account id corresponding to a given [`SeedFingerprint`] /// and [`zip32::AccountId`], if any. pub(crate) fn get_derived_account( @@ -1929,29 +1952,6 @@ pub(crate) fn get_account_uuid( .ok_or(SqliteClientError::AccountUnknown) } -pub(crate) fn get_account( - conn: &rusqlite::Connection, - params: &P, - account_uuid: AccountUuid, -) -> Result, SqliteClientError> { - let mut stmt = conn.prepare_cached( - r#" - SELECT name, uuid, account_kind, - hd_seed_fingerprint, hd_account_index, key_source, - ufvk, uivk, has_spend_key - FROM accounts - WHERE uuid = :account_uuid - "#, - )?; - - let mut rows = stmt.query_and_then::<_, SqliteClientError, _, _>( - named_params![":account_uuid": account_uuid.0], - |row| parse_account_row(row, params), - )?; - - rows.next().transpose() -} - /// Returns the minimum and maximum heights of blocks in the chain which may be scanned. pub(crate) fn chain_tip_height( conn: &rusqlite::Connection, From 5290d13942d4a3f149b86459ee3556d1dac196d8 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 23 Dec 2024 17:09:54 -0700 Subject: [PATCH 4/9] zcash_client_backend: Move `Recipient::EphemeralTransparent` behind the `transparent-inputs` feature flag. This variant should not have been a part of the public API unless `tranpsarent-inputs` was enabled, as it's necessary for the wallet to be able to spend a transparent input in order for a ZIP 320 transaction to be properly constructed and authorized. In addition, this simplifies the `Recipient` API by removing its type parameters in favor of concrete types, made possible by using a separate type for the build process. --- zcash_client_backend/CHANGELOG.md | 12 + zcash_client_backend/src/data_api.rs | 10 +- zcash_client_backend/src/data_api/wallet.rs | 239 +++++++++++++------- zcash_client_backend/src/wallet.rs | 105 ++------- zcash_client_sqlite/src/wallet.rs | 137 ++++++----- 5 files changed, 262 insertions(+), 241 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 21be939890..1d3860dcd5 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -9,6 +9,18 @@ and this library adheres to Rust's notion of ### Changed - Migrated to `nonempty 0.11` +- `zcash_client_backend::wallet::Recipient` has changed: + - The `Recipient::External` variant is now a structured variant. + - The `Recipient::EphemeralTransparent` variant is now only available if + `zcash_client_backend` is built using the `transparent-inputs` feature flag. + - The `N` and `O` type pararameters to this type have been replaced by + concrete uses of `Box` and `Outpoint` instead. The + `map_internal_account_note` and `map_ephemeral_transparent_outpoint` and + `internal_account_note_transpose_option` methods have consequently been + removed. +- `zcash_client_backend::data_api::WalletRead::get_known_ephemeral_addresses` + now takes a `Range` as its + argument instead of a `Range` ### Deprecated - `zcash_client_backend::address` (use `zcash_keys::address` instead) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index c10df8cdfe..045cee9b92 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -68,7 +68,6 @@ use std::{ use incrementalmerkletree::{frontier::Frontier, Retention}; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; -use ::transparent::bundle::OutPoint; use zcash_keys::{ address::UnifiedAddress, keys::{ @@ -99,7 +98,8 @@ use crate::{ #[cfg(feature = "transparent-inputs")] use { - crate::wallet::TransparentAddressMetadata, ::transparent::address::TransparentAddress, + crate::wallet::TransparentAddressMetadata, + ::transparent::{address::TransparentAddress, bundle::OutPoint}, std::ops::Range, }; @@ -1966,7 +1966,7 @@ impl<'a, AccountId> SentTransaction<'a, AccountId> { /// This type is capable of representing both shielded and transparent outputs. pub struct SentTransactionOutput { output_index: usize, - recipient: Recipient, + recipient: Recipient, value: Zatoshis, memo: Option, } @@ -1983,7 +1983,7 @@ impl SentTransactionOutput { /// * `memo` - the memo that was sent with this output pub fn from_parts( output_index: usize, - recipient: Recipient, + recipient: Recipient, value: Zatoshis, memo: Option, ) -> Self { @@ -2006,7 +2006,7 @@ impl SentTransactionOutput { } /// Returns the recipient address of the transaction, or the account id and /// resulting note/outpoint for wallet-internal outputs. - pub fn recipient(&self) -> &Recipient { + pub fn recipient(&self) -> &Recipient { &self.recipient } /// Returns the value of the newly created output. diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 054da0e9e5..a2b8deda26 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -35,18 +35,9 @@ to a wallet-internal shielded address, as described in [ZIP 316](https://zips.z. use nonempty::NonEmpty; use rand_core::OsRng; -use sapling::{ - note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey}, - prover::{OutputProver, SpendProver}, -}; -use shardtree::error::{QueryError, ShardTreeError}; use std::num::NonZeroU32; -use zcash_keys::{ - address::Address, - keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, -}; -use zcash_protocol::{PoolType, ShieldedProtocol}; -use zip321::Payment; + +use shardtree::error::{QueryError, ShardTreeError}; use super::InputSource; use crate::{ @@ -61,10 +52,18 @@ use crate::{ proposal::{Proposal, ProposalError, Step, StepOutputIndex}, wallet::{Note, OvkPolicy, Recipient}, }; - +use ::sapling::{ + note_encryption::{try_sapling_note_decryption, PreparedIncomingViewingKey}, + prover::{OutputProver, SpendProver}, +}; use ::transparent::{ address::TransparentAddress, builder::TransparentSigningSet, bundle::OutPoint, }; +use zcash_address::ZcashAddress; +use zcash_keys::{ + address::Address, + keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, +}; use zcash_primitives::transaction::{ builder::{BuildConfig, BuildResult, Builder}, components::sapling::zip212_enforcement, @@ -75,8 +74,10 @@ use zcash_protocol::{ consensus::{self, BlockHeight, NetworkUpgrade}, memo::MemoBytes, value::Zatoshis, + PoolType, ShieldedProtocol, }; use zip32::Scope; +use zip321::Payment; #[cfg(feature = "transparent-inputs")] use { @@ -100,7 +101,6 @@ use { }, sapling::note_encryption::SaplingDomain, serde::{Deserialize, Serialize}, - zcash_address::ZcashAddress, zcash_note_encryption::try_output_recovery_with_pkd_esk, zcash_protocol::{ consensus::NetworkConstants, @@ -133,25 +133,32 @@ struct ProposalInfo { #[derive(Serialize, Deserialize)] enum PcztRecipient { External, - EphemeralTransparent { receiving_account: AccountId }, - InternalAccount { receiving_account: AccountId }, + #[cfg(feature = "transparent-inputs")] + EphemeralTransparent { + receiving_account: AccountId, + }, + InternalAccount { + receiving_account: AccountId, + }, } #[cfg(feature = "pczt")] impl PcztRecipient { - fn from_recipient(recipient: Recipient) -> (Self, Option) { + fn from_recipient(recipient: BuildRecipient) -> (Self, Option) { match recipient { - Recipient::External(addr, _) => (PcztRecipient::External, Some(addr)), - Recipient::EphemeralTransparent { + BuildRecipient::External { + recipient_address, .. + } => (PcztRecipient::External, Some(recipient_address)), + #[cfg(feature = "transparent-inputs")] + BuildRecipient::EphemeralTransparent { receiving_account, .. } => ( PcztRecipient::EphemeralTransparent { receiving_account }, None, ), - Recipient::InternalAccount { + BuildRecipient::InternalAccount { receiving_account, external_address, - .. } => ( PcztRecipient::InternalAccount { receiving_account }, external_address, @@ -536,6 +543,72 @@ where Ok(NonEmpty::from_vec(txids).expect("proposal.steps is NonEmpty")) } +#[derive(Debug, Clone)] +enum BuildRecipient { + External { + recipient_address: ZcashAddress, + output_pool: PoolType, + }, + #[cfg(feature = "transparent-inputs")] + EphemeralTransparent { + receiving_account: AccountId, + ephemeral_address: TransparentAddress, + }, + InternalAccount { + receiving_account: AccountId, + external_address: Option, + }, +} + +impl BuildRecipient { + fn into_recipient_with_note(self, note: impl FnOnce() -> Note) -> Recipient { + match self { + BuildRecipient::External { + recipient_address, + output_pool, + } => Recipient::External { + recipient_address, + output_pool, + }, + #[cfg(feature = "transparent-inputs")] + BuildRecipient::EphemeralTransparent { .. } => unreachable!(), + BuildRecipient::InternalAccount { + receiving_account, + external_address, + } => Recipient::InternalAccount { + receiving_account, + external_address, + note: Box::new(note()), + }, + } + } + + fn into_recipient_with_outpoint( + self, + #[cfg(feature = "transparent-inputs")] outpoint: OutPoint, + ) -> Recipient { + match self { + BuildRecipient::External { + recipient_address, + output_pool, + } => Recipient::External { + recipient_address, + output_pool, + }, + #[cfg(feature = "transparent-inputs")] + BuildRecipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + } => Recipient::EphemeralTransparent { + receiving_account, + ephemeral_address, + outpoint, + }, + BuildRecipient::InternalAccount { .. } => unreachable!(), + } + } +} + #[allow(clippy::type_complexity)] struct BuildState<'a, P, AccountId> { #[cfg(feature = "transparent-inputs")] @@ -544,18 +617,10 @@ struct BuildState<'a, P, AccountId> { #[cfg(feature = "transparent-inputs")] transparent_input_addresses: HashMap, #[cfg(feature = "orchard")] - orchard_output_meta: Vec<( - Recipient, - Zatoshis, - Option, - )>, - sapling_output_meta: Vec<( - Recipient, - Zatoshis, - Option, - )>, + orchard_output_meta: Vec<(BuildRecipient, Zatoshis, Option)>, + sapling_output_meta: Vec<(BuildRecipient, Zatoshis, Option)>, transparent_output_meta: Vec<( - Recipient, + BuildRecipient, TransparentAddress, Zatoshis, StepOutputIndex, @@ -884,12 +949,10 @@ where }; #[cfg(feature = "orchard")] - let mut orchard_output_meta: Vec<(Recipient<_, PoolType, _>, Zatoshis, Option)> = - vec![]; - let mut sapling_output_meta: Vec<(Recipient<_, PoolType, _>, Zatoshis, Option)> = - vec![]; + let mut orchard_output_meta: Vec<(BuildRecipient<_>, Zatoshis, Option)> = vec![]; + let mut sapling_output_meta: Vec<(BuildRecipient<_>, Zatoshis, Option)> = vec![]; let mut transparent_output_meta: Vec<( - Recipient<_, _, ()>, + BuildRecipient<_>, TransparentAddress, Zatoshis, StepOutputIndex, @@ -915,7 +978,10 @@ where let memo = payment.memo().map_or_else(MemoBytes::empty, |m| m.clone()); builder.add_sapling_output(sapling_external_ovk, to, payment.amount(), memo.clone())?; sapling_output_meta.push(( - Recipient::External(recipient_address.clone(), PoolType::SAPLING), + BuildRecipient::External { + recipient_address: recipient_address.clone(), + output_pool: PoolType::SAPLING, + }, payment.amount(), Some(memo), )); @@ -936,7 +1002,10 @@ where memo.clone(), )?; orchard_output_meta.push(( - Recipient::External(recipient_address.clone(), PoolType::ORCHARD), + BuildRecipient::External { + recipient_address: recipient_address.clone(), + output_pool: PoolType::ORCHARD, + }, payment.amount(), Some(memo), )); @@ -962,7 +1031,10 @@ where } builder.add_transparent_output(&to, payment.amount())?; transparent_output_meta.push(( - Recipient::External(recipient_address.clone(), PoolType::TRANSPARENT), + BuildRecipient::External { + recipient_address: recipient_address.clone(), + output_pool: PoolType::TRANSPARENT, + }, to, payment.amount(), StepOutputIndex::Payment(payment_index), @@ -1031,10 +1103,9 @@ where memo.clone(), )?; sapling_output_meta.push(( - Recipient::InternalAccount { + BuildRecipient::InternalAccount { receiving_account: account_id, external_address: None, - note: output_pool, }, change_value.value(), Some(memo), @@ -1055,10 +1126,9 @@ where memo.clone(), )?; orchard_output_meta.push(( - Recipient::InternalAccount { + BuildRecipient::InternalAccount { receiving_account: account_id, external_address: None, - note: output_pool, }, change_value.value(), Some(memo), @@ -1100,10 +1170,9 @@ where // if a later step does not consume it. builder.add_transparent_output(&ephemeral_address, change_value.value())?; transparent_output_meta.push(( - Recipient::EphemeralTransparent { + BuildRecipient::EphemeralTransparent { receiving_account: account_id, ephemeral_address, - outpoint_metadata: (), }, ephemeral_address, change_value.value(), @@ -1208,20 +1277,17 @@ where .output_action_index(i) .expect("An action should exist in the transaction for each Orchard output."); - let recipient = recipient - .map_internal_account_note(|pool| { - assert!(pool == PoolType::ORCHARD); - build_result - .transaction() - .orchard_bundle() - .and_then(|bundle| { - bundle - .decrypt_output_with_key(output_index, &orchard_internal_ivk) - .map(|(note, _, _)| Note::Orchard(note)) - }) - }) - .internal_account_note_transpose_option() - .expect("Wallet-internal outputs must be decryptable with the wallet's IVK"); + let recipient = recipient.into_recipient_with_note(|| { + build_result + .transaction() + .orchard_bundle() + .and_then(|bundle| { + bundle + .decrypt_output_with_key(output_index, &orchard_internal_ivk) + .map(|(note, _, _)| Note::Orchard(note)) + }) + .expect("Wallet-internal outputs must be decryptable with the wallet's IVK") + }); SentTransactionOutput::from_parts(output_index, recipient, value, memo) }, @@ -1237,23 +1303,20 @@ where .output_index(i) .expect("An output should exist in the transaction for each Sapling payment."); - let recipient = recipient - .map_internal_account_note(|pool| { - assert!(pool == PoolType::SAPLING); - build_result - .transaction() - .sapling_bundle() - .and_then(|bundle| { - try_sapling_note_decryption( - &sapling_internal_ivk, - &bundle.shielded_outputs()[output_index], - zip212_enforcement(params, min_target_height), - ) - .map(|(note, _, _)| Note::Sapling(note)) - }) - }) - .internal_account_note_transpose_option() - .expect("Wallet-internal outputs must be decryptable with the wallet's IVK"); + let recipient = recipient.into_recipient_with_note(|| { + build_result + .transaction() + .sapling_bundle() + .and_then(|bundle| { + try_sapling_note_decryption( + &sapling_internal_ivk, + &bundle.shielded_outputs()[output_index], + zip212_enforcement(params, min_target_height), + ) + .map(|(note, _, _)| Note::Sapling(note)) + }) + .expect("Wallet-internal outputs must be decryptable with the wallet's IVK") + }); SentTransactionOutput::from_parts(output_index, recipient, value, memo) }, @@ -1280,7 +1343,11 @@ where // would not usefully improve privacy. let outpoint = OutPoint::new(txid, n as u32); - let recipient = recipient.map_ephemeral_transparent_outpoint(|()| outpoint.clone()); + let recipient = recipient.into_recipient_with_outpoint( + #[cfg(feature = "transparent-inputs")] + outpoint.clone(), + ); + #[cfg(feature = "transparent-inputs")] unused_transparent_outputs.insert( StepOutput::new(build_state.step_index, step_output_index), @@ -1819,12 +1886,14 @@ where let note_value = Zatoshis::try_from(note_value(¬e))?; let recipient = match (pczt_recipient, external_address) { - (PcztRecipient::External, Some(addr)) => { - Ok(Recipient::External(addr, PoolType::Shielded(output_pool))) - } + (PcztRecipient::External, Some(addr)) => Ok(Recipient::External { + recipient_address: addr, + output_pool: PoolType::Shielded(output_pool), + }), (PcztRecipient::External, None) => Err(PcztError::Invalid( "external recipient needs to have its user_address field set".into(), )), + #[cfg(feature = "transparent-inputs")] (PcztRecipient::EphemeralTransparent { .. }, _) => Err(PcztError::Invalid( "shielded output cannot be EphemeralTransparent".into(), )), @@ -1832,7 +1901,7 @@ where Ok(Recipient::InternalAccount { receiving_account, external_address, - note: wallet_note(note), + note: Box::new(wallet_note(note)), }) } }?; @@ -1927,11 +1996,15 @@ where let recipient = match (pczt_recipient, external_address) { (PcztRecipient::External, Some(addr)) => { - Ok(Recipient::External(addr, PoolType::Transparent)) + Ok(Recipient::External { + recipient_address: addr, + output_pool: PoolType::Transparent, + }) } (PcztRecipient::External, None) => Err(PcztError::Invalid( "external recipient needs to have its user_address field set".into(), )), + #[cfg(feature = "transparent-inputs")] (PcztRecipient::EphemeralTransparent { receiving_account }, _) => output .recipient_address() .ok_or(PcztError::Invalid( @@ -1941,7 +2014,7 @@ where .map(|ephemeral_address| Recipient::EphemeralTransparent { receiving_account, ephemeral_address, - outpoint_metadata: outpoint, + outpoint, }), ( PcztRecipient::InternalAccount { diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 1bb51954e2..f298b6fdb7 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -1,11 +1,12 @@ //! Structs representing transaction data scanned from the block chain by a wallet or //! light client. +use incrementalmerkletree::Position; + use ::transparent::{ address::TransparentAddress, bundle::{OutPoint, TxOut}, }; -use incrementalmerkletree::Position; use zcash_address::ZcashAddress; use zcash_note_encryption::EphemeralKeyBytes; use zcash_primitives::transaction::{fees::transparent as transparent_fees, TxId}; @@ -60,111 +61,31 @@ impl NoteId { } /// A type that represents the recipient of a transaction output: +/// /// * a recipient address; /// * for external unified addresses, the pool to which the payment is sent; -/// * for ephemeral transparent addresses, the internal account ID and metadata about the outpoint; /// * for wallet-internal outputs, the internal account ID and metadata about the note. +/// * if the `transparent-inputs` feature is enabled, for ephemeral transparent outputs, the +/// internal account ID and metadata about the outpoint; #[derive(Debug, Clone)] -pub enum Recipient { - External(ZcashAddress, PoolType), +pub enum Recipient { + External { + recipient_address: ZcashAddress, + output_pool: PoolType, + }, + #[cfg(feature = "transparent-inputs")] EphemeralTransparent { receiving_account: AccountId, ephemeral_address: TransparentAddress, - outpoint_metadata: O, + outpoint: OutPoint, }, InternalAccount { receiving_account: AccountId, external_address: Option, - note: N, + note: Box, }, } -impl Recipient { - /// Return a copy of this `Recipient` with `f` applied to the note metadata, if any. - pub fn map_internal_account_note B>( - self, - f: F, - ) -> Recipient { - match self { - Recipient::External(addr, pool) => Recipient::External(addr, pool), - Recipient::EphemeralTransparent { - receiving_account, - ephemeral_address, - outpoint_metadata, - } => Recipient::EphemeralTransparent { - receiving_account, - ephemeral_address, - outpoint_metadata, - }, - Recipient::InternalAccount { - receiving_account, - external_address, - note, - } => Recipient::InternalAccount { - receiving_account, - external_address, - note: f(note), - }, - } - } - - /// Return a copy of this `Recipient` with `f` applied to the output metadata, if any. - pub fn map_ephemeral_transparent_outpoint B>( - self, - f: F, - ) -> Recipient { - match self { - Recipient::External(addr, pool) => Recipient::External(addr, pool), - Recipient::EphemeralTransparent { - receiving_account, - ephemeral_address, - outpoint_metadata, - } => Recipient::EphemeralTransparent { - receiving_account, - ephemeral_address, - outpoint_metadata: f(outpoint_metadata), - }, - Recipient::InternalAccount { - receiving_account, - external_address, - note, - } => Recipient::InternalAccount { - receiving_account, - external_address, - note, - }, - } - } -} - -impl Recipient, O> { - /// Return a copy of this `Recipient` with optional note metadata transposed to - /// an optional result. - pub fn internal_account_note_transpose_option(self) -> Option> { - match self { - Recipient::External(addr, pool) => Some(Recipient::External(addr, pool)), - Recipient::EphemeralTransparent { - receiving_account, - ephemeral_address, - outpoint_metadata, - } => Some(Recipient::EphemeralTransparent { - receiving_account, - ephemeral_address, - outpoint_metadata, - }), - Recipient::InternalAccount { - receiving_account, - external_address, - note, - } => note.map(|n0| Recipient::InternalAccount { - receiving_account, - external_address, - note: n0, - }), - } - } -} - /// The shielded subset of a [`Transaction`]'s data that is relevant to a particular wallet. /// /// [`Transaction`]: zcash_primitives::transaction::Transaction diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 133dfa3fc2..c9c47562b6 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -84,7 +84,6 @@ use std::ops::RangeInclusive; use tracing::{debug, warn}; -use ::transparent::bundle::OutPoint; use zcash_address::ZcashAddress; use zcash_client_backend::{ data_api::{ @@ -114,8 +113,9 @@ use zcash_protocol::{ value::{ZatBalance, Zatoshis}, PoolType, ShieldedProtocol, }; -use zip32::{self, DiversifierIndex, Scope}; +use zip32::{DiversifierIndex, Scope}; +use self::scanning::{parse_priority_code, priority_code, replace_queue_entries}; use crate::{ error::SqliteClientError, wallet::commitment_tree::{get_max_checkpointed_height, SqliteShardStore}, @@ -125,9 +125,7 @@ use crate::{ use crate::{AccountUuid, TxRef, VERIFY_LOOKAHEAD}; #[cfg(feature = "transparent-inputs")] -use ::transparent::bundle::TxOut; - -use self::scanning::{parse_priority_code, priority_code, replace_queue_entries}; +use ::transparent::bundle::{OutPoint, TxOut}; #[cfg(feature = "orchard")] use {crate::ORCHARD_TABLES_PREFIX, zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT}; @@ -2269,55 +2267,53 @@ pub(crate) fn store_transaction_to_be_sent( match output.recipient() { Recipient::InternalAccount { receiving_account, - note: Note::Sapling(note), + note, .. - } => { - sapling::put_received_note( - wdb.conn.0, - &DecryptedOutput::new( - output.output_index(), - note.clone(), - *receiving_account, - output - .memo() - .map_or_else(MemoBytes::empty, |memo| memo.clone()), - TransferType::WalletInternal, - ), - tx_ref, - None, - )?; - } - #[cfg(feature = "orchard")] - Recipient::InternalAccount { - receiving_account, - note: Note::Orchard(note), - .. - } => { - orchard::put_received_note( - wdb.conn.0, - &DecryptedOutput::new( - output.output_index(), - *note, - *receiving_account, - output - .memo() - .map_or_else(MemoBytes::empty, |memo| memo.clone()), - TransferType::WalletInternal, - ), - tx_ref, - None, - )?; - } + } => match note.as_ref() { + Note::Sapling(note) => { + sapling::put_received_note( + wdb.conn.0, + &DecryptedOutput::new( + output.output_index(), + note.clone(), + *receiving_account, + output + .memo() + .map_or_else(MemoBytes::empty, |memo| memo.clone()), + TransferType::WalletInternal, + ), + tx_ref, + None, + )?; + } + #[cfg(feature = "orchard")] + Note::Orchard(note) => { + orchard::put_received_note( + wdb.conn.0, + &DecryptedOutput::new( + output.output_index(), + *note, + *receiving_account, + output + .memo() + .map_or_else(MemoBytes::empty, |memo| memo.clone()), + TransferType::WalletInternal, + ), + tx_ref, + None, + )?; + } + }, #[cfg(feature = "transparent-inputs")] Recipient::EphemeralTransparent { receiving_account, ephemeral_address, - outpoint_metadata, + outpoint, } => { transparent::put_transparent_output( wdb.conn.0, &wdb.params, - outpoint_metadata, + outpoint, &TxOut { value: output.value(), script_pubkey: ephemeral_address.script(), @@ -2733,11 +2729,14 @@ pub(crate) fn store_decrypted_tx( TransferType::Outgoing => { let recipient = { let receiver = Receiver::Sapling(output.note().recipient()); - let wallet_address = + let recipient_address = select_receiving_address(params, conn, *output.account(), &receiver)? .unwrap_or_else(|| receiver.to_zcash_address(params.network_type())); - Recipient::External(wallet_address, PoolType::SAPLING) + Recipient::External { + recipient_address, + output_pool: PoolType::SAPLING, + } }; put_sent_output( @@ -2757,7 +2756,7 @@ pub(crate) fn store_decrypted_tx( let recipient = Recipient::InternalAccount { receiving_account: *output.account(), external_address: None, - note: Note::Sapling(output.note().clone()), + note: Box::new(Note::Sapling(output.note().clone())), }; put_sent_output( @@ -2791,7 +2790,7 @@ pub(crate) fn store_decrypted_tx( }), ) }, - note: Note::Sapling(output.note().clone()), + note: Box::new(Note::Sapling(output.note().clone())), }; put_sent_output( @@ -2819,11 +2818,14 @@ pub(crate) fn store_decrypted_tx( TransferType::Outgoing => { let recipient = { let receiver = Receiver::Orchard(output.note().recipient()); - let wallet_address = + let recipient_address = select_receiving_address(params, conn, *output.account(), &receiver)? .unwrap_or_else(|| receiver.to_zcash_address(params.network_type())); - Recipient::External(wallet_address, PoolType::ORCHARD) + Recipient::External { + recipient_address, + output_pool: PoolType::ORCHARD, + } }; put_sent_output( @@ -2843,7 +2845,7 @@ pub(crate) fn store_decrypted_tx( let recipient = Recipient::InternalAccount { receiving_account: *output.account(), external_address: None, - note: Note::Orchard(*output.note()), + note: Box::new(Note::Orchard(*output.note())), }; put_sent_output( @@ -2878,7 +2880,7 @@ pub(crate) fn store_decrypted_tx( }), ) }, - note: Note::Orchard(*output.note()), + note: Box::new(Note::Orchard(*output.note())), }; put_sent_output( @@ -2989,14 +2991,17 @@ pub(crate) fn store_decrypted_tx( let receiver = Receiver::Transparent(address); #[cfg(feature = "transparent-inputs")] - let recipient_addr = + let recipient_address = select_receiving_address(params, conn, account_uuid, &receiver)? .unwrap_or_else(|| receiver.to_zcash_address(params.network_type())); #[cfg(not(feature = "transparent-inputs"))] - let recipient_addr = receiver.to_zcash_address(params.network_type()); + let recipient_address = receiver.to_zcash_address(params.network_type()); - let recipient = Recipient::External(recipient_addr, PoolType::TRANSPARENT); + let recipient = Recipient::External { + recipient_address, + output_pool: PoolType::TRANSPARENT, + }; put_sent_output( conn, @@ -3295,13 +3300,23 @@ pub(crate) fn notify_tx_retrieved( // and `put_sent_output` fn recipient_params( conn: &Connection, - params: &P, + _params: &P, from: AccountUuid, - to: &Recipient, + to: &Recipient, ) -> Result<(AccountRef, Option, Option, PoolType), SqliteClientError> { let from_account_id = get_account_ref(conn, from)?; match to { - Recipient::External(addr, pool) => Ok((from_account_id, Some(addr.encode()), None, *pool)), + Recipient::External { + recipient_address, + output_pool, + .. + } => Ok(( + from_account_id, + Some(recipient_address.encode()), + None, + *output_pool, + )), + #[cfg(feature = "transparent-inputs")] Recipient::EphemeralTransparent { receiving_account, ephemeral_address, @@ -3310,7 +3325,7 @@ fn recipient_params( let to_account = get_account_ref(conn, *receiving_account)?; Ok(( from_account_id, - Some(ephemeral_address.encode(params)), + Some(ephemeral_address.encode(_params)), Some(to_account), PoolType::TRANSPARENT, )) @@ -3414,7 +3429,7 @@ pub(crate) fn put_sent_output( from_account_uuid: AccountUuid, tx_ref: TxRef, output_index: usize, - recipient: &Recipient, + recipient: &Recipient, value: Zatoshis, memo: Option<&MemoBytes>, ) -> Result<(), SqliteClientError> { From 29d8f10cdc0d36dbb0a0e8fbcaee29d04033b206 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 23 Dec 2024 17:52:43 -0700 Subject: [PATCH 5/9] zcash_client_backend: Improve type safety of `get_known_ephemeral_addresses` --- zcash_client_backend/Cargo.toml | 1 + zcash_client_backend/src/data_api.rs | 11 ++--- zcash_client_backend/src/data_api/testing.rs | 47 ++++++++++--------- .../src/data_api/testing/pool.rs | 8 +++- zcash_client_sqlite/Cargo.toml | 1 + zcash_client_sqlite/src/lib.rs | 8 ++-- zcash_client_sqlite/src/testing/db.rs | 2 +- 7 files changed, 42 insertions(+), 36 deletions(-) diff --git a/zcash_client_backend/Cargo.toml b/zcash_client_backend/Cargo.toml index 110012c48e..5bdfdba9ab 100644 --- a/zcash_client_backend/Cargo.toml +++ b/zcash_client_backend/Cargo.toml @@ -166,6 +166,7 @@ lightwalletd-tonic-transport = ["lightwalletd-tonic", "tonic?/transport"] ## Enables receiving transparent funds and shielding them. transparent-inputs = [ "dep:bip32", + "transparent/transparent-inputs", "zcash_keys/transparent-inputs", "zcash_primitives/transparent-inputs", ] diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 045cee9b92..15f9dafbc8 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -74,15 +74,12 @@ use zcash_keys::{ UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedIncomingViewingKey, UnifiedSpendingKey, }, }; -use zcash_primitives::{ - block::BlockHash, - transaction::{Transaction, TxId}, -}; +use zcash_primitives::{block::BlockHash, transaction::Transaction}; use zcash_protocol::{ consensus::BlockHeight, memo::{Memo, MemoBytes}, value::{BalanceError, Zatoshis}, - ShieldedProtocol, + ShieldedProtocol, TxId, }; use zip32::fingerprint::SeedFingerprint; @@ -99,8 +96,8 @@ use crate::{ #[cfg(feature = "transparent-inputs")] use { crate::wallet::TransparentAddressMetadata, - ::transparent::{address::TransparentAddress, bundle::OutPoint}, std::ops::Range, + transparent::{address::TransparentAddress, bundle::OutPoint, keys::NonHardenedChildIndex}, }; #[cfg(feature = "test-dependencies")] @@ -1465,7 +1462,7 @@ pub trait WalletRead { fn get_known_ephemeral_addresses( &self, _account: Self::AccountId, - _index_range: Option>, + _index_range: Option>, ) -> Result, Self::Error> { Ok(vec![]) } diff --git a/zcash_client_backend/src/data_api/testing.rs b/zcash_client_backend/src/data_api/testing.rs index 27c845e0bf..4b88bb032a 100644 --- a/zcash_client_backend/src/data_api/testing.rs +++ b/zcash_client_backend/src/data_api/testing.rs @@ -8,11 +8,6 @@ use std::{ num::NonZeroU32, }; -use ::sapling::{ - note_encryption::{sapling_note_encryption, SaplingDomain}, - util::generate_random_rseed, - zip32::DiversifiableFullViewingKey, -}; use assert_matches::assert_matches; use group::ff::Field; use incrementalmerkletree::{Marking, Retention}; @@ -23,6 +18,11 @@ use secrecy::{ExposeSecret, Secret, SecretVec}; use shardtree::{error::ShardTreeError, store::memory::MemoryShardStore, ShardTree}; use subtle::ConditionallySelectable; +use ::sapling::{ + note_encryption::{sapling_note_encryption, SaplingDomain}, + util::generate_random_rseed, + zip32::DiversifiableFullViewingKey, +}; use zcash_address::ZcashAddress; use zcash_keys::{ address::{Address, UnifiedAddress}, @@ -44,20 +44,9 @@ use zcash_protocol::{ use zip32::{fingerprint::SeedFingerprint, DiversifierIndex}; use zip321::Payment; -use crate::{ - fees::{ - standard::{self, SingleOutputChangeStrategy}, - ChangeStrategy, DustOutputPolicy, StandardFeeRule, - }, - proposal::Proposal, - proto::compact_formats::{ - self, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, - }, - wallet::{Note, NoteId, OvkPolicy, ReceivedNote, WalletTransparentOutput}, -}; - use super::{ chain::{scan_cached_blocks, BlockSource, ChainState, CommitmentTreeRoot, ScanSummary}, + error::Error, scanning::ScanRange, wallet::{ create_proposed_transactions, @@ -65,17 +54,29 @@ use super::{ propose_standard_transfer_to_address, propose_transfer, }, Account, AccountBalance, AccountBirthday, AccountMeta, AccountPurpose, AccountSource, - BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SeedRelevance, - SentTransaction, SpendableNotes, TransactionDataRequest, TransactionStatus, + BlockMetadata, DecryptedTransaction, InputSource, NoteFilter, NullifierQuery, ScannedBlock, + SeedRelevance, SentTransaction, SpendableNotes, TransactionDataRequest, TransactionStatus, WalletCommitmentTrees, WalletRead, WalletSummary, WalletTest, WalletWrite, SAPLING_SHARD_HEIGHT, }; -use super::{error::Error, NoteFilter}; +use crate::{ + fees::{ + standard::{self, SingleOutputChangeStrategy}, + ChangeStrategy, DustOutputPolicy, StandardFeeRule, + }, + proposal::Proposal, + proto::compact_formats::{ + self, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, + }, + wallet::{Note, NoteId, OvkPolicy, ReceivedNote, WalletTransparentOutput}, +}; #[cfg(feature = "transparent-inputs")] use { - super::wallet::input_selection::ShieldingSelector, crate::wallet::TransparentAddressMetadata, - ::transparent::address::TransparentAddress, std::ops::Range, + super::wallet::input_selection::ShieldingSelector, + crate::wallet::TransparentAddressMetadata, + ::transparent::{address::TransparentAddress, keys::NonHardenedChildIndex}, + std::ops::Range, }; #[cfg(feature = "orchard")] @@ -2624,7 +2625,7 @@ impl WalletRead for MockWalletDb { fn get_known_ephemeral_addresses( &self, _account: Self::AccountId, - _index_range: Option>, + _index_range: Option>, ) -> Result, Self::Error> { Ok(vec![]) } diff --git a/zcash_client_backend/src/data_api/testing/pool.rs b/zcash_client_backend/src/data_api/testing/pool.rs index e74ce69a7c..a58e73b78f 100644 --- a/zcash_client_backend/src/data_api/testing/pool.rs +++ b/zcash_client_backend/src/data_api/testing/pool.rs @@ -827,7 +827,13 @@ pub fn send_multi_step_proposed_transfer( // the start of the gap to index 12. This also tests the `index_range` parameter. let newer_known_addrs = st .wallet() - .get_known_ephemeral_addresses(account_id, Some(5..100)) + .get_known_ephemeral_addresses( + account_id, + Some( + NonHardenedChildIndex::from_index(5).unwrap() + ..NonHardenedChildIndex::from_index(100).unwrap(), + ), + ) .unwrap(); assert_eq!(newer_known_addrs.len(), (GAP_LIMIT as usize) + 12 - 5); assert!(newer_known_addrs.starts_with(&new_known_addrs[5..])); diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 5c0a5339a9..53bc373a82 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -123,6 +123,7 @@ test-dependencies = [ ## Enables receiving transparent funds and sending to transparent recipients transparent-inputs = [ "dep:bip32", + "transparent/transparent-inputs", "zcash_keys/transparent-inputs", "zcash_client_backend/transparent-inputs" ] diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index a1f1a2692c..4af0c63aa9 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -92,9 +92,9 @@ use { #[cfg(feature = "transparent-inputs")] use { - ::transparent::{address::TransparentAddress, bundle::OutPoint}, + ::transparent::{address::TransparentAddress, bundle::OutPoint, keys::NonHardenedChildIndex}, zcash_client_backend::wallet::TransparentAddressMetadata, - zcash_keys::encoding::AddressCodec as _, + zcash_keys::encoding::AddressCodec, }; #[cfg(feature = "multicore")] @@ -659,14 +659,14 @@ impl, P: consensus::Parameters> WalletRead for W fn get_known_ephemeral_addresses( &self, account: Self::AccountId, - index_range: Option>, + index_range: Option>, ) -> Result, Self::Error> { let account_id = wallet::get_account_ref(self.conn.borrow(), account)?; wallet::transparent::ephemeral::get_known_ephemeral_addresses( self.conn.borrow(), &self.params, account_id, - index_range, + index_range.map(|i| i.start.index()..i.end.index()), ) } diff --git a/zcash_client_sqlite/src/testing/db.rs b/zcash_client_sqlite/src/testing/db.rs index 2675dff877..e7fa1d300e 100644 --- a/zcash_client_sqlite/src/testing/db.rs +++ b/zcash_client_sqlite/src/testing/db.rs @@ -42,7 +42,7 @@ use crate::{ #[cfg(feature = "transparent-inputs")] use { crate::TransparentAddressMetadata, - ::transparent::{address::TransparentAddress, bundle::OutPoint}, + ::transparent::{address::TransparentAddress, bundle::OutPoint, keys::NonHardenedChildIndex}, core::ops::Range, }; From f48f72b67b33957c71770c23f303d8d3496d8cdb Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 24 Dec 2024 13:44:15 -0700 Subject: [PATCH 6/9] zcash_client_sqlite: Move ephemeral address management test out of migration. This test is not specific to the migration; it's a more general test of ephemeral address rotation behavior and should evolve with the evolution of address rotation and gap limit handling, not be tied to the behavior of methods at the time that this migration was created. --- .../init/migrations/ephemeral_addresses.rs | 208 ------------------ zcash_client_sqlite/src/wallet/transparent.rs | 57 ++++- 2 files changed, 56 insertions(+), 209 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs index 802be4a7f9..109f307fff 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs @@ -85,216 +85,8 @@ impl RusqliteMigration for Migration

{ mod tests { use crate::wallet::init::migrations::tests::test_migrate; - #[cfg(feature = "transparent-inputs")] - use { - crate::{ - error::SqliteClientError, - wallet::{ - self, account_kind_code, init::init_wallet_db_internal, transparent::ephemeral, - }, - AccountRef, WalletDb, - }, - ::transparent::keys::NonHardenedChildIndex, - rusqlite::{named_params, Connection}, - secrecy::{ExposeSecret, Secret, SecretVec}, - tempfile::NamedTempFile, - zcash_client_backend::data_api::GAP_LIMIT, - zcash_client_backend::{ - data_api::{AccountBirthday, AccountSource}, - wallet::TransparentAddressMetadata, - }, - zcash_keys::keys::UnifiedSpendingKey, - zcash_primitives::block::BlockHash, - zcash_protocol::consensus::Network, - zip32::{fingerprint::SeedFingerprint, AccountId as Zip32AccountId}, - }; - - /// This is a minimized copy of [`wallet::create_account`] as of the time of the - /// creation of this migration. - #[cfg(feature = "transparent-inputs")] - fn create_account( - wdb: &mut WalletDb, - seed: &SecretVec, - birthday: &AccountBirthday, - ) -> Result<(AccountRef, UnifiedSpendingKey), SqliteClientError> { - wdb.transactionally(|wdb| { - let seed_fingerprint = - SeedFingerprint::from_seed(seed.expose_secret()).ok_or_else(|| { - SqliteClientError::BadAccountData( - "Seed must be between 32 and 252 bytes in length.".to_owned(), - ) - })?; - let account_index = wallet::max_zip32_account_index(wdb.conn.0, &seed_fingerprint)? - .map(|a| { - a.next() - .ok_or(SqliteClientError::Zip32AccountIndexOutOfRange) - }) - .transpose()? - .unwrap_or(zip32::AccountId::ZERO); - - let usk = - UnifiedSpendingKey::from_seed(&wdb.params, seed.expose_secret(), account_index) - .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?; - let ufvk = usk.to_unified_full_viewing_key(); - - #[cfg(feature = "orchard")] - let orchard_item = ufvk.orchard().map(|k| k.to_bytes()); - #[cfg(not(feature = "orchard"))] - let orchard_item: Option> = None; - - let sapling_item = ufvk.sapling().map(|k| k.to_bytes()); - - #[cfg(feature = "transparent-inputs")] - let transparent_item = ufvk.transparent().map(|k| k.serialize()); - #[cfg(not(feature = "transparent-inputs"))] - let transparent_item: Option> = None; - - let birthday_sapling_tree_size = Some(birthday.sapling_frontier().tree_size()); - #[cfg(feature = "orchard")] - let birthday_orchard_tree_size = Some(birthday.orchard_frontier().tree_size()); - #[cfg(not(feature = "orchard"))] - let birthday_orchard_tree_size: Option = None; - - let account_id: AccountRef = wdb.conn.0.query_row( - r#" - INSERT INTO accounts ( - account_kind, hd_seed_fingerprint, hd_account_index, - ufvk, uivk, - orchard_fvk_item_cache, sapling_fvk_item_cache, p2pkh_fvk_item_cache, - birthday_height, birthday_sapling_tree_size, birthday_orchard_tree_size, - recover_until_height - ) - VALUES ( - :account_kind, :hd_seed_fingerprint, :hd_account_index, - :ufvk, :uivk, - :orchard_fvk_item_cache, :sapling_fvk_item_cache, :p2pkh_fvk_item_cache, - :birthday_height, :birthday_sapling_tree_size, :birthday_orchard_tree_size, - :recover_until_height - ) - RETURNING id; - "#, - named_params![ - ":account_kind": 0, // 0 == Derived - ":hd_seed_fingerprint": seed_fingerprint.to_bytes(), - ":hd_account_index": u32::from(account_index), - ":ufvk": ufvk.encode(&wdb.params), - ":uivk": ufvk.to_unified_incoming_viewing_key().encode(&wdb.params), - ":orchard_fvk_item_cache": orchard_item, - ":sapling_fvk_item_cache": sapling_item, - ":p2pkh_fvk_item_cache": transparent_item, - ":birthday_height": u32::from(birthday.height()), - ":birthday_sapling_tree_size": birthday_sapling_tree_size, - ":birthday_orchard_tree_size": birthday_orchard_tree_size, - ":recover_until_height": birthday.recover_until().map(u32::from) - ], - |row| Ok(AccountRef(row.get(0)?)), - )?; - - // Initialize the `ephemeral_addresses` table. - #[cfg(feature = "transparent-inputs")] - wallet::transparent::ephemeral::init_account(wdb.conn.0, &wdb.params, account_id)?; - - Ok((account_id, usk)) - }) - } - #[test] fn migrate() { test_migrate(&[super::MIGRATION_ID]); } - - #[test] - #[cfg(feature = "transparent-inputs")] - fn initialize_table() { - use zcash_client_backend::data_api::Zip32Derivation; - - let network = Network::TestNetwork; - let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap(); - - let seed0 = vec![0x00; 32]; - init_wallet_db_internal( - &mut db_data, - Some(Secret::new(seed0.clone())), - super::DEPENDENCIES, - false, - ) - .unwrap(); - - let birthday = AccountBirthday::from_sapling_activation(&network, BlockHash([0; 32])); - - // Simulate creating an account prior to this migration. - let account0_index = Zip32AccountId::ZERO; - let account0_seed_fp = [0u8; 32]; - let account0_kind = account_kind_code(&AccountSource::Derived { - derivation: Zip32Derivation::new( - SeedFingerprint::from_seed(&account0_seed_fp).unwrap(), - account0_index, - ), - key_source: None, - }); - assert_eq!(u32::from(account0_index), 0); - let account0_id = AccountRef(0); - - let usk0 = UnifiedSpendingKey::from_seed(&network, &seed0, account0_index).unwrap(); - let ufvk0 = usk0.to_unified_full_viewing_key(); - let uivk0 = ufvk0.to_unified_incoming_viewing_key(); - - db_data - .conn - .execute( - "INSERT INTO accounts (id, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, birthday_height) - VALUES (:id, :account_kind, :hd_seed_fingerprint, :hd_account_index, :ufvk, :uivk, :birthday_height)", - named_params![ - ":id": account0_id.0, - ":account_kind": account0_kind, - ":hd_seed_fingerprint": account0_seed_fp, - ":hd_account_index": u32::from(account0_index), - ":ufvk": ufvk0.encode(&network), - ":uivk": uivk0.encode(&network), - ":birthday_height": u32::from(birthday.height()), - ], - ) - .unwrap(); - - // The `ephemeral_addresses` table is expected not to exist before migration. - assert_matches!( - ephemeral::first_unstored_index(&db_data.conn, account0_id), - Err(SqliteClientError::DbError(_)) - ); - - let check = |db: &WalletDb<_, _>, account_id| { - eprintln!("checking {account_id:?}"); - assert_matches!(ephemeral::first_unstored_index(&db.conn, account_id), Ok(addr_index) if addr_index == GAP_LIMIT); - assert_matches!(ephemeral::first_unreserved_index(&db.conn, account_id), Ok(addr_index) if addr_index == 0); - - let known_addrs = - ephemeral::get_known_ephemeral_addresses(&db.conn, &db.params, account_id, None) - .unwrap(); - - let expected_metadata: Vec = (0..GAP_LIMIT) - .map(|i| ephemeral::metadata(NonHardenedChildIndex::from_index(i).unwrap())) - .collect(); - let actual_metadata: Vec = - known_addrs.into_iter().map(|(_, meta)| meta).collect(); - assert_eq!(actual_metadata, expected_metadata); - }; - - // The migration should initialize `ephemeral_addresses`. - init_wallet_db_internal( - &mut db_data, - Some(Secret::new(seed0)), - &[super::MIGRATION_ID], - false, - ) - .unwrap(); - check(&db_data, account0_id); - - // Creating a new account should initialize `ephemeral_addresses` for that account. - let seed1 = vec![0x01; 32]; - let (account1_id, _usk) = - create_account(&mut db_data, &Secret::new(seed1), &birthday).unwrap(); - assert_ne!(account0_id, account1_id); - check(&db_data, account1_id); - } } diff --git a/zcash_client_sqlite/src/wallet/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs index 13f7dfe7c5..55844cb714 100644 --- a/zcash_client_sqlite/src/wallet/transparent.rs +++ b/zcash_client_sqlite/src/wallet/transparent.rs @@ -900,7 +900,19 @@ pub(crate) fn queue_transparent_spend_detection( #[cfg(test)] mod tests { - use crate::testing::{db::TestDbFactory, BlockCache}; + use secrecy::Secret; + use transparent::keys::NonHardenedChildIndex; + use zcash_client_backend::{ + data_api::{testing::TestBuilder, Account as _, WalletWrite, GAP_LIMIT}, + wallet::TransparentAddressMetadata, + }; + use zcash_primitives::block::BlockHash; + + use crate::{ + testing::{db::TestDbFactory, BlockCache}, + wallet::{get_account_ref, transparent::ephemeral}, + WalletDb, + }; #[test] fn put_received_transparent_utxo() { @@ -924,4 +936,47 @@ mod tests { BlockCache::new(), ); } + + #[test] + fn ephemeral_address_management() { + let mut st = TestBuilder::new() + .with_data_store_factory(TestDbFactory::default()) + .with_block_cache(BlockCache::new()) + .with_account_from_sapling_activation(BlockHash([0; 32])) + .build(); + + let birthday = st.test_account().unwrap().birthday().clone(); + let account0_uuid = st.test_account().unwrap().account().id(); + let account0_id = get_account_ref(&st.wallet().db().conn, account0_uuid).unwrap(); + + let check = |db: &WalletDb<_, _>, account_id| { + eprintln!("checking {account_id:?}"); + assert_matches!(ephemeral::first_unstored_index(&db.conn, account_id), Ok(addr_index) if addr_index == GAP_LIMIT); + assert_matches!(ephemeral::first_unreserved_index(&db.conn, account_id), Ok(addr_index) if addr_index == 0); + + let known_addrs = + ephemeral::get_known_ephemeral_addresses(&db.conn, &db.params, account_id, None) + .unwrap(); + + let expected_metadata: Vec = (0..GAP_LIMIT) + .map(|i| ephemeral::metadata(NonHardenedChildIndex::from_index(i).unwrap())) + .collect(); + let actual_metadata: Vec = + known_addrs.into_iter().map(|(_, meta)| meta).collect(); + assert_eq!(actual_metadata, expected_metadata); + }; + + check(st.wallet().db(), account0_id); + + // Creating a new account should initialize `ephemeral_addresses` for that account. + let seed1 = vec![0x01; 32]; + let (account1_uuid, _usk) = st + .wallet_mut() + .db_mut() + .create_account("test1", &Secret::new(seed1), &birthday, None) + .unwrap(); + let account1_id = get_account_ref(&st.wallet().db().conn, account1_uuid).unwrap(); + assert_ne!(account0_id, account1_id); + check(st.wallet().db(), account1_id); + } } From e4dac19eb6de06cf262ebbf7a45470c54d2811a2 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 26 Dec 2024 14:19:05 -0700 Subject: [PATCH 7/9] zcash_keys: Add `ReceiverRequirement` enum. This permits `UnifiedAddressRequest` values an additional dimension of flexibility, permitting generation of unified addresses having receivers for all recever types for which a key item exists and a diversifier index is valid. --- zcash_client_sqlite/src/lib.rs | 11 +- zcash_client_sqlite/src/wallet.rs | 5 +- zcash_client_sqlite/src/wallet/init.rs | 9 +- .../init/migrations/add_transaction_views.rs | 7 +- .../wallet/init/migrations/addresses_table.rs | 12 +- .../migrations/ensure_orchard_ua_receiver.rs | 22 +- .../wallet/init/migrations/ufvk_support.rs | 4 +- zcash_keys/CHANGELOG.md | 15 + zcash_keys/src/keys.rs | 268 +++++++++++------- 9 files changed, 223 insertions(+), 130 deletions(-) diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 4af0c63aa9..bcf2771974 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -62,7 +62,8 @@ use zcash_client_backend::{ use zcash_keys::{ address::UnifiedAddress, keys::{ - AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey, + AddressGenerationError, ReceiverRequirement, UnifiedAddressRequest, UnifiedFullViewingKey, + UnifiedSpendingKey, }, }; use zcash_primitives::{ @@ -154,14 +155,14 @@ pub(crate) const SAPLING_TABLES_PREFIX: &str = "sapling"; pub(crate) const ORCHARD_TABLES_PREFIX: &str = "orchard"; #[cfg(not(feature = "orchard"))] -pub(crate) const UA_ORCHARD: bool = false; +pub(crate) const UA_ORCHARD: ReceiverRequirement = ReceiverRequirement::Omit; #[cfg(feature = "orchard")] -pub(crate) const UA_ORCHARD: bool = true; +pub(crate) const UA_ORCHARD: ReceiverRequirement = ReceiverRequirement::Require; #[cfg(not(feature = "transparent-inputs"))] -pub(crate) const UA_TRANSPARENT: bool = false; +pub(crate) const UA_TRANSPARENT: ReceiverRequirement = ReceiverRequirement::Omit; #[cfg(feature = "transparent-inputs")] -pub(crate) const UA_TRANSPARENT: bool = true; +pub(crate) const UA_TRANSPARENT: ReceiverRequirement = ReceiverRequirement::Require; /// Unique identifier for a specific account tracked by a [`WalletDb`]. /// diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index c9c47562b6..99db16e477 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -292,10 +292,7 @@ pub(crate) fn seed_matches_derived_account( let usk = UnifiedSpendingKey::from_seed(params, &seed.expose_secret()[..], account_index) .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?; - let (seed_addr, _) = usk.to_unified_full_viewing_key().default_address(Some( - UnifiedAddressRequest::all().expect("At least one supported pool feature is enabled."), - ))?; - + let (seed_addr, _) = usk.to_unified_full_viewing_key().default_address(None)?; let (uivk_addr, _) = uivk.default_address(None)?; #[cfg(not(feature = "orchard"))] diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 80947101b7..d9d25c2b93 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -443,7 +443,10 @@ mod tests { use zcash_keys::{ address::Address, encoding::{encode_extended_full_viewing_key, encode_payment_address}, - keys::{sapling, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey}, + keys::{ + sapling, ReceiverRequirement::*, UnifiedAddressRequest, UnifiedFullViewingKey, + UnifiedSpendingKey, + }, }; use zcash_primitives::transaction::{TransactionData, TxVersion}; use zcash_protocol::consensus::{self, BlockHeight, BranchId, Network, NetworkConstants}; @@ -984,7 +987,7 @@ mod tests { // Unified addresses at the time of the addition of migrations did not contain an // Orchard component. - let ua_request = UnifiedAddressRequest::unsafe_new(false, true, UA_TRANSPARENT); + let ua_request = UnifiedAddressRequest::unsafe_new(Omit, Require, UA_TRANSPARENT); let address_str = Address::Unified( ufvk.default_address(Some(ua_request)) .expect("A valid default address exists for the UFVK") @@ -1111,7 +1114,7 @@ mod tests { assert_eq!(tv.unified_addr, ua.encode(&Network::MainNetwork)); // hardcoded with knowledge of what's coming next - let ua_request = UnifiedAddressRequest::unsafe_new(false, true, true); + let ua_request = UnifiedAddressRequest::unsafe_new(Omit, Require, Require); db_data .get_next_available_address(account_id, Some(ua_request)) .unwrap() diff --git a/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs b/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs index 5b092e9bc3..a7403c76d7 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs @@ -393,7 +393,8 @@ mod tests { #[cfg(feature = "transparent-inputs")] fn migrate_from_wm2() { use ::transparent::keys::NonHardenedChildIndex; - use zcash_keys::keys::UnifiedAddressRequest; + use zcash_client_backend::keys::UnifiedAddressRequest; + use zcash_keys::keys::ReceiverRequirement::*; use zcash_protocol::value::Zatoshis; use crate::UA_TRANSPARENT; @@ -441,8 +442,8 @@ mod tests { let ufvk = usk.to_unified_full_viewing_key(); let (ua, _) = ufvk .default_address(Some(UnifiedAddressRequest::unsafe_new( - false, - true, + Omit, + Require, UA_TRANSPARENT, ))) .expect("A valid default address exists for the UFVK"); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs b/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs index a1edabde5e..64b057a475 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/addresses_table.rs @@ -3,8 +3,12 @@ use std::collections::HashSet; use rusqlite::{named_params, Transaction}; use schemerz_rusqlite::RusqliteMigration; use uuid::Uuid; -use zcash_keys::{address::Address, keys::UnifiedFullViewingKey}; -use zcash_keys::{address::UnifiedAddress, encoding::AddressCodec, keys::UnifiedAddressRequest}; + +use zcash_keys::{ + address::{Address, UnifiedAddress}, + encoding::AddressCodec, + keys::{ReceiverRequirement::*, UnifiedAddressRequest, UnifiedFullViewingKey}, +}; use zcash_protocol::consensus; use zip32::{AccountId, DiversifierIndex}; @@ -87,7 +91,7 @@ impl RusqliteMigration for Migration

{ )); }; let (expected_address, idx) = ufvk.default_address(Some( - UnifiedAddressRequest::unsafe_new(false, true, UA_TRANSPARENT), + UnifiedAddressRequest::unsafe_new(Omit, Require, UA_TRANSPARENT), ))?; if decoded_address != expected_address { return Err(WalletMigrationError::CorruptedData(format!( @@ -159,7 +163,7 @@ impl RusqliteMigration for Migration

{ )?; let (address, d_idx) = ufvk.default_address(Some( - UnifiedAddressRequest::unsafe_new(false, true, UA_TRANSPARENT), + UnifiedAddressRequest::unsafe_new(Omit, Require, UA_TRANSPARENT), ))?; insert_address(transaction, &self.params, account, d_idx, &address)?; } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ensure_orchard_ua_receiver.rs b/zcash_client_sqlite/src/wallet/init/migrations/ensure_orchard_ua_receiver.rs index 66d6a5201c..633fc37aec 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ensure_orchard_ua_receiver.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ensure_orchard_ua_receiver.rs @@ -5,7 +5,9 @@ use rusqlite::named_params; use schemerz_rusqlite::RusqliteMigration; use uuid::Uuid; -use zcash_keys::keys::{UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedIncomingViewingKey}; +use zcash_keys::keys::{ + ReceiverRequirement::*, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedIncomingViewingKey, +}; use zcash_protocol::consensus; use super::orchard_received_notes; @@ -64,7 +66,7 @@ impl RusqliteMigration for Migration

{ }; let (default_addr, diversifier_index) = uivk.default_address(Some( - UnifiedAddressRequest::unsafe_new(UA_ORCHARD, true, UA_TRANSPARENT), + UnifiedAddressRequest::unsafe_new(UA_ORCHARD, Require, UA_TRANSPARENT), ))?; let mut di_be = *diversifier_index.as_bytes(); @@ -90,8 +92,10 @@ mod tests { use secrecy::SecretVec; use tempfile::NamedTempFile; - use zcash_keys::address::Address; - use zcash_keys::keys::{UnifiedAddressRequest, UnifiedSpendingKey}; + use zcash_keys::{ + address::Address, + keys::{ReceiverRequirement::*, UnifiedAddressRequest, UnifiedSpendingKey}, + }; use zcash_protocol::consensus::Network; use crate::{ @@ -139,8 +143,8 @@ mod tests { let (addr, diversifier_index) = ufvk .default_address(Some(UnifiedAddressRequest::unsafe_new( - false, - true, + Omit, + Require, UA_TRANSPARENT, ))) .unwrap(); @@ -168,7 +172,7 @@ mod tests { Ok(Address::Unified(ua)) => { assert!(!ua.has_orchard()); assert!(ua.has_sapling()); - assert_eq!(ua.has_transparent(), UA_TRANSPARENT); + assert_eq!(ua.has_transparent(), UA_TRANSPARENT == Require); } other => panic!("Unexpected result from address decoding: {:?}", other), } @@ -184,9 +188,9 @@ mod tests { Ok(Address::decode(&db_data.params, &row.get::<_, String>(0)?).unwrap()) }) { Ok(Address::Unified(ua)) => { - assert_eq!(ua.has_orchard(), UA_ORCHARD); + assert_eq!(ua.has_orchard(), UA_ORCHARD == Require); assert!(ua.has_sapling()); - assert_eq!(ua.has_transparent(), UA_TRANSPARENT); + assert_eq!(ua.has_transparent(), UA_TRANSPARENT == Require); } other => panic!("Unexpected result from address decoding: {:?}", other), } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs index 895f1b08bd..305b0f5eb6 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs @@ -8,7 +8,7 @@ use uuid::Uuid; use zcash_keys::{ address::Address, - keys::{UnifiedAddressRequest, UnifiedSpendingKey}, + keys::{ReceiverRequirement::*, UnifiedAddressRequest, UnifiedSpendingKey}, }; use zcash_protocol::{consensus, PoolType}; use zip32::AccountId; @@ -85,7 +85,7 @@ impl RusqliteMigration for Migration

{ // our second assumption above, and we report this as corrupted data. let mut seed_is_relevant = false; - let ua_request = UnifiedAddressRequest::unsafe_new(false, true, UA_TRANSPARENT); + let ua_request = UnifiedAddressRequest::unsafe_new(Omit, Require, UA_TRANSPARENT); let mut rows = stmt_fetch_accounts.query([])?; while let Some(row) = rows.next()? { // We only need to check for the presence of the seed if we have keys that diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 33b00bb7cd..8565b06c0a 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -9,9 +9,24 @@ and this library adheres to Rust's notion of ### Added - `no-std` compatibility (`alloc` is required). A default-enabled `std` feature flag has been added gating the `std::error::Error` usage. +- `zcash_keys::keys::ReceiverRequirement` ### Changed - Migrated to `nonempty 0.11` +- `zcash_keys::keys::UnifiedAddressRequest` has been substantially modified; + instead of a collection of boolean flags, it is now a collection of + `ReceiverRequirement` values that describe how addresses may be constructed + in the case that keys for a particular protocol are absent or it is not + possible to generate a specific receiver at a given diversifier index. + Behavior of methods that accept a `UnifiedAddressRequest` have been modified + accordingly. In addition, request construction methods that previously + returned `None` to indicate an attempt to generate an invalid request now + return `Err(())` + +### Removed +- `zcash_keys::keys::UnifiedAddressRequest::all` (use + `UnifiedAddressRequest::ALLOW_ALL` or + `UnifiedFullViewingKey::to_address_request` instead) ## [0.6.0] - 2024-12-16 diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 123dd20aab..dc6a04daa0 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -552,72 +552,107 @@ impl fmt::Display for AddressGenerationError { #[cfg(feature = "std")] impl std::error::Error for AddressGenerationError {} +/// An enumeration of the ways in which a receiver may be requested to be present in a generated +/// [`UnifiedAddress`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ReceiverRequirement { + /// A receiver of the associated type is required to be present in the generated + /// `[UnifiedAddress`], and if it is not possible to generate a receiver of this type, the + /// address generation method should return an error. When calling [`Self::intersect`], this + /// variant will be preferred over [`ReceiverRequirement::Allow`]. + Require, + /// The associated receiver should be included, if a corresponding item exists in the IVK from + /// which the address is being derived and derivation of the receiver succeeds at the given + /// diversifier index. + Allow, + /// No receiver of the associated type may be included in the generated [`UnifiedAddress`] + /// under any circumstances. When calling [`Self::intersect`], this variant will be preferred + /// over [`ReceiverRequirement::Allow`]. + Omit, +} + +impl ReceiverRequirement { + /// Return the intersection of two requirements that chooses the stronger requirement, if one + /// exists. [`ReceiverRequirement::Require`] and [`ReceiverRequirement::Omit`] are + /// incompatible; attempting an intersection between these will return an error. + pub fn intersect(self, other: Self) -> Result { + use ReceiverRequirement::*; + match (self, other) { + (Require, Omit) => Err(()), + (Require, Require) => Ok(Require), + (Require, Allow) => Ok(Require), + (Allow, Require) => Ok(Require), + (Allow, Allow) => Ok(Allow), + (Allow, Omit) => Ok(Omit), + (Omit, Require) => Err(()), + (Omit, Allow) => Ok(Omit), + (Omit, Omit) => Ok(Omit), + } + } +} + /// 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_sapling: bool, - has_p2pkh: bool, + orchard: ReceiverRequirement, + sapling: ReceiverRequirement, + p2pkh: ReceiverRequirement, } impl UnifiedAddressRequest { /// Construct a new unified address request from its constituent parts. /// - /// Returns `None` if the resulting unified address would not include at least one shielded receiver. - pub fn new(has_orchard: bool, has_sapling: bool, has_p2pkh: bool) -> Option { - let has_shielded_receiver = has_orchard || has_sapling; - - if !has_shielded_receiver { - None + /// Returns `Err(())` if the resulting unified address would not include at least one shielded receiver. + pub fn new( + orchard: ReceiverRequirement, + sapling: ReceiverRequirement, + p2pkh: ReceiverRequirement, + ) -> Result { + use ReceiverRequirement::*; + if orchard == Omit && sapling == Omit { + Err(()) } else { - Some(Self { - has_orchard, - has_sapling, - has_p2pkh, + Ok(Self { + orchard, + sapling, + p2pkh, }) } } - /// Constructs a new unified address request that includes a request for a receiver of each - /// type that is supported given the active feature flags. - pub fn all() -> Option { - let _has_orchard = false; - #[cfg(feature = "orchard")] - let _has_orchard = true; - - let _has_sapling = false; - #[cfg(feature = "sapling")] - let _has_sapling = true; - - let _has_p2pkh = false; - #[cfg(feature = "transparent-inputs")] - let _has_p2pkh = true; - - Self::new(_has_orchard, _has_sapling, _has_p2pkh) - } + /// Constructs a new unified address request that allows a receiver of each type. + pub const ALLOW_ALL: UnifiedAddressRequest = { + use ReceiverRequirement::*; + Self::unsafe_new(Allow, Allow, Allow) + }; - /// Constructs a new unified address request that includes only the receivers - /// that appear both in itself and a given other request. - pub fn intersect(&self, other: &UnifiedAddressRequest) -> Option { - Self::new( - self.has_orchard && other.has_orchard, - self.has_sapling && other.has_sapling, - self.has_p2pkh && other.has_p2pkh, - ) + /// Constructs a new unified address request that includes only the receivers that are allowed + /// both in itself and a given other request. Returns [`None`] if requirements are incompatible + /// or if no shielded receiver type is allowed. + pub fn intersect(&self, other: &UnifiedAddressRequest) -> Result { + let orchard = self.orchard.intersect(other.orchard)?; + let sapling = self.sapling.intersect(other.sapling)?; + let p2pkh = self.p2pkh.intersect(other.p2pkh)?; + Self::new(orchard, sapling, p2pkh) } /// Construct a new unified address request from its constituent parts. /// - /// Panics: at least one of `has_orchard` or `has_sapling` must be `true`. - pub const fn unsafe_new(has_orchard: bool, has_sapling: bool, has_p2pkh: bool) -> Self { - if !(has_orchard || has_sapling) { - panic!("At least one shielded receiver must be requested.") + /// Panics: at least one of `orchard` or `sapling` must be allowed. + pub const fn unsafe_new( + orchard: ReceiverRequirement, + sapling: ReceiverRequirement, + p2pkh: ReceiverRequirement, + ) -> Self { + use ReceiverRequirement::*; + if matches!(orchard, Omit) && matches!(sapling, Omit) { + panic!("At least one shielded receiver must be allowed.") } Self { - has_orchard, - has_sapling, - has_p2pkh, + orchard, + sapling, + p2pkh, } } } @@ -1120,78 +1155,93 @@ impl UnifiedIncomingViewingKey { } /// Attempts to derive the Unified Address for the given diversifier index and receiver types. - /// If `request` is None, the address should be derived to contain a receiver for each item in + /// If `request` is None, the address will be derived to contain a receiver for each item in /// this UFVK. /// - /// Returns `None` if the specified index does not produce a valid diversifier. + /// Returns an error if the this key does not produce a valid receiver for a required receiver + /// type at the given diversifier index. pub fn address( &self, _j: DiversifierIndex, request: Option, ) -> Result { + use ReceiverRequirement::*; + let request = request - .or(self.to_address_request()) + .or(self.to_address_request().ok()) .ok_or(AddressGenerationError::ShieldedReceiverRequired)?; + #[cfg(feature = "orchard")] let mut orchard = None; - if request.has_orchard { + if request.orchard != Omit { #[cfg(not(feature = "orchard"))] - return Err(AddressGenerationError::ReceiverTypeNotSupported( - Typecode::Orchard, - )); + if request.orchard == Require { + return Err(AddressGenerationError::ReceiverTypeNotSupported( + Typecode::Orchard, + )); + } #[cfg(feature = "orchard")] if let Some(oivk) = &self.orchard { let orchard_j = orchard::keys::DiversifierIndex::from(*_j.as_bytes()); orchard = Some(oivk.address_at(orchard_j)) - } else { + } else if request.orchard == Require { return Err(AddressGenerationError::KeyNotAvailable(Typecode::Orchard)); } } #[cfg(feature = "sapling")] let mut sapling = None; - if request.has_sapling { + if request.sapling != Omit { #[cfg(not(feature = "sapling"))] - return Err(AddressGenerationError::ReceiverTypeNotSupported( - Typecode::Sapling, - )); + if request.sapling == Require { + return Err(AddressGenerationError::ReceiverTypeNotSupported( + Typecode::Sapling, + )); + } #[cfg(feature = "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( - divk.address_at(_j) - .ok_or(AddressGenerationError::InvalidSaplingDiversifierIndex(_j))?, - ); - } else { + sapling = match (request.sapling, divk.address_at(_j)) { + (Require | Allow, Some(addr)) => Ok(Some(addr)), + (Require, None) => { + Err(AddressGenerationError::InvalidSaplingDiversifierIndex(_j)) + } + _ => Ok(None), + }?; + } else if request.sapling == Require { return Err(AddressGenerationError::KeyNotAvailable(Typecode::Sapling)); } } #[cfg(feature = "transparent-inputs")] let mut transparent = None; - if request.has_p2pkh { + if request.p2pkh != Omit { #[cfg(not(feature = "transparent-inputs"))] - return Err(AddressGenerationError::ReceiverTypeNotSupported( - Typecode::P2pkh, - )); + if request.p2pkh == Require { + return Err(AddressGenerationError::ReceiverTypeNotSupported( + Typecode::P2pkh, + )); + } #[cfg(feature = "transparent-inputs")] 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. - let transparent_j = to_transparent_child_index(_j) - .ok_or(AddressGenerationError::InvalidTransparentChildIndex(_j))?; - - transparent = Some( - tivk.derive_address(transparent_j) - .map_err(|_| AddressGenerationError::InvalidTransparentChildIndex(_j))?, - ); - } else { + let j = to_transparent_child_index(_j); + + transparent = match (request.p2pkh, j.and_then(|j| tivk.derive_address(j).ok())) { + (Require | Allow, Some(addr)) => Ok(Some(addr)), + (Require, None) => { + Err(AddressGenerationError::InvalidTransparentChildIndex(_j)) + } + _ => Ok(None), + }?; + } else if request.p2pkh == Require { return Err(AddressGenerationError::KeyNotAvailable(Typecode::P2pkh)); } } @@ -1208,23 +1258,33 @@ impl UnifiedIncomingViewingKey { .ok_or(AddressGenerationError::ShieldedReceiverRequired) } - /// 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. + /// Searches the diversifier space starting at diversifier index `j` for one which will produce + /// a valid address that conforms to the provided request, and returns that Unified Address + /// along with the index at which the valid diversifier was found. + /// + /// If [`None`] is specified for the `request` parameter, a default request that [`Require`]s a + /// receiver be present for each key item enabled by the feature flags in use will be used to + /// search the diversifier space. /// /// Returns an `Err(AddressGenerationError)` if no valid diversifier exists or if the features - /// required to satisfy the unified address request are not properly enabled. + /// required to satisfy the unified address request are not enabled. + /// + /// [`Require`]: ReceiverRequirement::Require #[allow(unused_mut)] pub fn find_address( &self, mut j: DiversifierIndex, request: Option, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { + let request = request + .or_else(|| self.to_address_request().ok()) + .ok_or(AddressGenerationError::ShieldedReceiverRequired)?; + // If we need to generate a transparent receiver, check that the user has not // specified an invalid transparent child index, from which we can never search to // find a valid index. #[cfg(feature = "transparent-inputs")] - if request.iter().any(|r| r.has_p2pkh) + if request.p2pkh == ReceiverRequirement::Require && self.transparent.is_some() && to_transparent_child_index(j).is_none() { @@ -1233,7 +1293,7 @@ impl UnifiedIncomingViewingKey { // Find a working diversifier and construct the associated address. loop { - let res = self.address(j, request); + let res = self.address(j, Some(request)); match res { Ok(ua) => { return Ok((ua, j)); @@ -1252,11 +1312,11 @@ impl UnifiedIncomingViewingKey { } /// Find the Unified Address corresponding to the smallest valid diversifier index, along with - /// that index. If `request` is None, the address should be derived to contain a receiver for - /// each item in this UFVK. + /// that index. If `request` is None, the address will be derived to contain a receiver for + /// each data item in this UFVK. /// - /// Returns an `Err(AddressGenerationError)` if no valid diversifier exists or if the features - /// required to satisfy the unified address request are not properly enabled. + /// Returns an error if the this key does not produce a valid receiver for a required receiver + /// type at any diversifier index. pub fn default_address( &self, request: Option, @@ -1264,24 +1324,32 @@ impl UnifiedIncomingViewingKey { self.find_address(DiversifierIndex::new(), request) } - /// Constructs a [`UnifiedAddressRequest`] that includes the components of this UIVK. - pub fn to_address_request(&self) -> Option { + /// Constructs a [`UnifiedAddressRequest`] that requires a receiver for each data item of this UIVK. + /// + /// Returns [`Err`] if the resulting request would not include a shielded receiver. + #[allow(unused_mut)] + pub fn to_address_request(&self) -> Result { + use ReceiverRequirement::*; + + let mut orchard = Omit; #[cfg(feature = "orchard")] - let has_orchard = self.orchard.is_some(); - #[cfg(not(feature = "orchard"))] - let has_orchard = false; + if self.orchard.is_some() { + orchard = Require; + } + let mut sapling = Omit; #[cfg(feature = "sapling")] - let has_sapling = self.sapling.is_some(); - #[cfg(not(feature = "sapling"))] - let has_sapling = false; + if self.sapling.is_some() { + sapling = Require; + } + let mut p2pkh = Omit; #[cfg(feature = "transparent-inputs")] - let has_p2pkh = self.transparent.is_some(); - #[cfg(not(feature = "transparent-inputs"))] - let has_p2pkh = false; + if self.transparent.is_some() { + p2pkh = Require; + } - UnifiedAddressRequest::new(has_orchard, has_sapling, has_p2pkh) + UnifiedAddressRequest::new(orchard, sapling, p2pkh) } } @@ -1499,7 +1567,7 @@ mod tests { fn ufvk_derivation() { use crate::keys::UnifiedAddressRequest; - use super::UnifiedSpendingKey; + use super::{ReceiverRequirement::*, UnifiedSpendingKey}; for tv in test_vectors::UNIFIED { let usk = UnifiedSpendingKey::from_seed( @@ -1522,7 +1590,7 @@ mod tests { let ua = ufvk .address( d_idx, - Some(UnifiedAddressRequest::unsafe_new(false, true, true)), + Some(UnifiedAddressRequest::unsafe_new(Omit, Require, Require)), ) .unwrap_or_else(|err| { panic!( @@ -1681,7 +1749,7 @@ mod tests { fn uivk_derivation() { use crate::keys::UnifiedAddressRequest; - use super::UnifiedSpendingKey; + use super::{ReceiverRequirement::*, UnifiedSpendingKey}; for tv in test_vectors::UNIFIED { let usk = UnifiedSpendingKey::from_seed( @@ -1706,7 +1774,7 @@ mod tests { let ua = uivk .address( d_idx, - Some(UnifiedAddressRequest::unsafe_new(false, true, true)), + Some(UnifiedAddressRequest::unsafe_new(Omit, Require, Require)), ) .unwrap_or_else(|err| { panic!( From c278405a6305eac079943d974c8944cf2d986c44 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 27 Dec 2024 21:07:27 -0700 Subject: [PATCH 8/9] zcash_keys: Add `Address::to_transparent_address` --- zcash_keys/CHANGELOG.md | 1 + zcash_keys/src/address.rs | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 8565b06c0a..d77cf19c02 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -10,6 +10,7 @@ and this library adheres to Rust's notion of - `no-std` compatibility (`alloc` is required). A default-enabled `std` feature flag has been added gating the `std::error::Error` usage. - `zcash_keys::keys::ReceiverRequirement` +- `zcash_keys::Address::to_transparent_address` ### Changed - Migrated to `nonempty 0.11` diff --git a/zcash_keys/src/address.rs b/zcash_keys/src/address.rs index 15b7fb8fd3..3a196ecf99 100644 --- a/zcash_keys/src/address.rs +++ b/zcash_keys/src/address.rs @@ -424,6 +424,18 @@ impl Address { }, } } + + /// Returns the transparent address corresponding to this address, if it is a transparent + /// address, a Unified address with a transparent receiver, or ZIP 320 (TEX) address. + pub fn to_transparent_address(&self) -> Option { + match self { + #[cfg(feature = "sapling")] + Address::Sapling(_) => None, + Address::Transparent(addr) => Some(*addr), + Address::Unified(ua) => ua.transparent().copied(), + Address::Tex(addr_bytes) => Some(TransparentAddress::PublicKeyHash(*addr_bytes)), + } + } } #[cfg(all( From e6b45f76f8d2a71149cc0646c8f906e8e78dd6b0 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 27 Dec 2024 08:26:21 -0700 Subject: [PATCH 9/9] zcash_client_sqlite: Remove duplicative migration test. This removes a `fix_bad_change_flagging` migration test duplicated from `zcash_client_backend::data_api::testing::pool::shiled_transparent`. It is impractical to maintain backwards compatibility to earlier database states for the entire test harness, which is more or less what retaining this test would require, and the desired outcome is already demonstrated by the `shield_transparent` test; demonstrating the fix directly is unnecessary. --- .../migrations/fix_bad_change_flagging.rs | 131 ------------------ 1 file changed, 131 deletions(-) diff --git a/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs b/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs index 056070fe09..38b1d49ae9 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/fix_bad_change_flagging.rs @@ -72,139 +72,8 @@ impl RusqliteMigration for Migration { mod tests { use crate::wallet::init::migrations::tests::test_migrate; - #[cfg(feature = "transparent-inputs")] - use { - crate::{ - testing::{db::TestDbFactory, BlockCache}, - wallet::init::init_wallet_db, - }, - ::transparent::bundle::{OutPoint, TxOut}, - zcash_client_backend::{ - data_api::{ - testing::{ - pool::ShieldedPoolTester, sapling::SaplingPoolTester, AddressType, TestBuilder, - }, - wallet::input_selection::GreedyInputSelector, - Account as _, WalletRead as _, WalletWrite as _, - }, - fees::{standard, DustOutputPolicy, StandardFeeRule}, - wallet::WalletTransparentOutput, - }, - zcash_primitives::block::BlockHash, - zcash_protocol::value::Zatoshis, - }; - #[test] fn migrate() { test_migrate(&[super::MIGRATION_ID]); } - - #[cfg(feature = "transparent-inputs")] - fn shield_transparent() { - let ds_factory = TestDbFactory::new( - super::DEPENDENCIES - .iter() - .copied() - // Pull in the account UUID migration so `TestBuilder::build` works. - .chain(Some(super::super::add_account_uuids::MIGRATION_ID)) - .collect(), - ); - let cache = BlockCache::new(); - let mut st = TestBuilder::new() - .with_data_store_factory(ds_factory) - .with_block_cache(cache) - .with_account_from_sapling_activation(BlockHash([0; 32])) - .build(); - - let account = st.test_account().cloned().unwrap(); - let dfvk = T::test_account_fvk(&st); - - let uaddr = st - .wallet() - .get_current_address(account.id()) - .unwrap() - .unwrap(); - let taddr = uaddr.transparent().unwrap(); - - // Ensure that the wallet has at least one block - let (h, _, _) = st.generate_next_block( - &dfvk, - AddressType::Internal, - Zatoshis::const_from_u64(50000), - ); - st.scan_cached_blocks(h, 1); - - let utxo = WalletTransparentOutput::from_parts( - OutPoint::fake(), - TxOut { - value: Zatoshis::const_from_u64(100000), - script_pubkey: taddr.script(), - }, - Some(h), - ) - .unwrap(); - - let res0 = st.wallet_mut().put_received_transparent_utxo(&utxo); - assert_matches!(res0, Ok(_)); - - let fee_rule = StandardFeeRule::Zip317; - - let input_selector = GreedyInputSelector::new(); - let change_strategy = standard::SingleOutputChangeStrategy::new( - fee_rule, - None, - T::SHIELDED_PROTOCOL, - DustOutputPolicy::default(), - ); - - let txids = st - .shield_transparent_funds( - &input_selector, - &change_strategy, - Zatoshis::from_u64(10000).unwrap(), - account.usk(), - &[*taddr], - account.id(), - 1, - ) - .unwrap(); - assert_eq!(txids.len(), 1); - - let tx = st.get_tx_from_history(*txids.first()).unwrap().unwrap(); - assert_eq!(tx.spent_note_count(), 1); - assert!(tx.has_change()); - assert_eq!(tx.received_note_count(), 0); - assert_eq!(tx.sent_note_count(), 0); - assert!(tx.is_shielding()); - - // Prior to the fix that removes the source of the error this migration is addressing, - // this scanning will result in a state where `tx.is_shielding() == false`. However, - // we can't validate that here, because after that fix, this test would fail. - let (h, _) = st.generate_next_block_including(*txids.first()); - st.scan_cached_blocks(h, 1); - - // Complete the migration to resolve the incorrect change flag value. - init_wallet_db(st.wallet_mut().db_mut(), None).unwrap(); - - let tx = st.get_tx_from_history(*txids.first()).unwrap().unwrap(); - assert_eq!(tx.spent_note_count(), 1); - assert!(tx.has_change()); - assert_eq!(tx.received_note_count(), 0); - assert_eq!(tx.sent_note_count(), 0); - assert!(tx.is_shielding()); - } - - #[test] - #[cfg(feature = "transparent-inputs")] - fn sapling_shield_transparent() { - shield_transparent::(); - } - - #[test] - #[cfg(all(feature = "orchard", feature = "transparent-inputs"))] - fn orchard_shield_transparent() { - use zcash_client_backend::data_api::testing::orchard::OrchardPoolTester; - - shield_transparent::(); - } }