From ce40387511aaa3fcaeec6ec08efde261ba4716db Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Sun, 16 Jun 2024 21:58:39 -0600 Subject: [PATCH 01/11] Add `import_account_hd` and `import_account_ufvk` methods --- zcash_client_backend/CHANGELOG.md | 1 + zcash_client_backend/src/data_api.rs | 87 ++++++++++++++++++++++++++ zcash_client_sqlite/CHANGELOG.md | 2 + zcash_client_sqlite/src/error.rs | 5 ++ zcash_client_sqlite/src/lib.rs | 55 +++++++++++++++- zcash_client_sqlite/src/wallet.rs | 4 +- zcash_client_sqlite/src/wallet/init.rs | 1 + 7 files changed, 151 insertions(+), 4 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 9462f1694..83ef2c2dd 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -10,6 +10,7 @@ and this library adheres to Rust's notion of ### Added - `zcash_client_backend::data_api`: - `chain::BlockCache` trait, behind the `sync` feature flag. + - `WalletWrite` trait methods `import_account_hd` and `import_account_ufvk`. - `zcash_client_backend::scanning`: - `testing` module - `zcash_client_backend::sync` module, behind the `sync` feature flag. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 2721219ae..e85fdb63a 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1440,6 +1440,9 @@ pub trait WalletWrite: WalletRead { /// assumed equivalent to the ZIP 32 account index. It is an opaque identifier for a pool of /// funds or set of outputs controlled by a single spending authority. /// + /// The ZIP-32 account index may be obtained by calling [`WalletRead::get_account`] + /// with the returned account identifier. + /// /// If `birthday.height()` is below the current chain tip, this operation will /// trigger a re-scan of the blocks at and above the provided height. The birthday height is /// defined as the minimum block height that will be scanned for funds belonging to the wallet. @@ -1468,6 +1471,72 @@ pub trait WalletWrite: WalletRead { birthday: &AccountBirthday, ) -> Result<(Self::AccountId, UnifiedSpendingKey), Self::Error>; + /// Tells the wallet to track a specific account index for a given seed. + /// + /// Returns the account for the newly-created wallet database entry, along with the + /// associated [`UnifiedSpendingKey`]. Note that the unique account identifier should *not* be + /// assumed equivalent to the ZIP 32 account index. It is an opaque identifier for a pool of + /// funds or set of outputs controlled by a single spending authority. + /// + /// The ZIP-32 account index may be obtained by calling [`WalletRead::get_account`] + /// with the returned account identifier. + /// + /// If `birthday.height()` is below the current chain tip, this operation will + /// trigger a re-scan of the blocks at and above the provided height. The birthday height is + /// defined as the minimum block height that will be scanned for funds belonging to the wallet. + /// + /// For new wallets, callers should construct the [`AccountBirthday`] using + /// [`AccountBirthday::from_treestate`] for the block at height `chain_tip_height - 100`. + /// Setting the birthday height to a tree state below the pruning depth ensures that reorgs + /// cannot cause funds intended for the wallet to be missed; otherwise, if the chain tip height + /// were used for the wallet birthday, a transaction targeted at a height greater than the + /// chain tip could be mined at a height below that tip as part of a reorg. + /// + /// If `seed` was imported from a backup and this method is being used to restore a previous + /// wallet state, you should use this method to add all of the desired accounts before scanning + /// the chain from the seed's birthday height. + /// + /// By convention, wallets should only allow a new account to be generated after confirmed + /// funds have been received by the currently-available account (in order to enable automated + /// account recovery). + /// + /// Panics if the length of the seed is not between 32 and 252 bytes inclusive. + /// + /// [ZIP 316]: https://zips.z.cash/zip-0316 + fn import_account_hd( + &mut self, + seed: &SecretVec, + account_index: zip32::AccountId, + birthday: &AccountBirthday, + ) -> Result<(Self::Account, UnifiedSpendingKey), Self::Error>; + + /// Tells the wallet to track an account using a unified full viewing key. + /// + /// Returns the account for the newly-created wallet database entry. + /// + /// If `birthday.height()` is below the current chain tip, this operation will + /// trigger a re-scan of the blocks at and above the provided height. The birthday height is + /// defined as the minimum block height that will be scanned for funds belonging to the wallet. + /// + /// Certain optimizations are possible for accounts which will never be used to spend funds. + /// If `spending_key_available` is `false`, the wallet may choose to optimize for this case, + /// in which case any attempt to spend funds from the account will result in an error. + /// + /// For new wallets, callers should construct the [`AccountBirthday`] using + /// [`AccountBirthday::from_treestate`] for the block at height `chain_tip_height - 100`. + /// Setting the birthday height to a tree state below the pruning depth ensures that reorgs + /// cannot cause funds intended for the wallet to be missed; otherwise, if the chain tip height + /// were used for the wallet birthday, a transaction targeted at a height greater than the + /// chain tip could be mined at a height below that tip as part of a reorg. + /// + /// Panics if the length of the seed is not between 32 and 252 bytes inclusive. + fn import_account_ufvk( + &mut self, + unified_key: &UnifiedFullViewingKey, + birthday: &AccountBirthday, + spending_key_available: bool, + ) -> Result; + /// Generates and persists the next available diversified address, given the current /// addresses known to the wallet. /// @@ -1878,6 +1947,24 @@ pub mod testing { .map_err(|_| ()) } + fn import_account_hd( + &mut self, + _seed: &SecretVec, + _account_index: zip32::AccountId, + _birthday: &AccountBirthday, + ) -> Result<(Self::Account, UnifiedSpendingKey), Self::Error> { + todo!() + } + + fn import_account_ufvk( + &mut self, + _unified_key: &UnifiedFullViewingKey, + _birthday: &AccountBirthday, + _spending_key_available: bool, + ) -> Result { + todo!() + } + fn get_next_available_address( &mut self, _account: Self::AccountId, diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index d60043d39..7e5e7d45c 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -8,6 +8,8 @@ and this library adheres to Rust's notion of ## [Unreleased] ### Changed - MSRV is now 1.70.0. +- `SqliteClientError` enum + - Added `AccountCollision` variant. ## [0.10.3] - 2024-04-08 diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 2f961853c..4e1877d3e 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -11,6 +11,7 @@ use zcash_primitives::zip32; use zcash_primitives::{consensus::BlockHeight, transaction::components::amount::BalanceError}; use crate::wallet::commitment_tree; +use crate::AccountId; use crate::PRUNING_DEPTH; #[cfg(feature = "transparent-inputs")] @@ -72,6 +73,9 @@ pub enum SqliteClientError { /// The account for which information was requested does not belong to the wallet. AccountUnknown, + /// The account being added collides with an existing account in the wallet with the given ID. + AccountCollision(AccountId), + /// The account was imported, and ZIP-32 derivation information is not known for it. UnknownZip32Derivation, @@ -155,6 +159,7 @@ impl fmt::Display for SqliteClientError { SqliteClientError::BadAccountData(e) => write!(f, "Failed to add account: {}", e), SqliteClientError::AccountIdDiscontinuity => write!(f, "Wallet account identifiers must be sequential."), SqliteClientError::AccountIdOutOfRange => write!(f, "Wallet account identifiers must be less than 0x7FFFFFFF."), + SqliteClientError::AccountCollision(id) => write!(f, "An account with ID {} already exists in the wallet.", id.0), #[cfg(feature = "transparent-inputs")] SqliteClientError::AddressNotRecognized(_) => write!(f, "The address associated with a received txo is not identifiable as belonging to the wallet."), SqliteClientError::CommitmentTree(err) => write!(f, "An error occurred accessing or updating note commitment tree data: {}.", err), diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 6d539fd02..1388b8a5d 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -556,7 +556,7 @@ impl WalletWrite for WalletDb .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?; let ufvk = usk.to_unified_full_viewing_key(); - let account_id = wallet::add_account( + let account = wallet::add_account( wdb.conn.0, &wdb.params, AccountSource::Derived { @@ -567,7 +567,58 @@ impl WalletWrite for WalletDb birthday, )?; - Ok((account_id, usk)) + Ok((account.id(), usk)) + }) + } + + fn import_account_hd( + &mut self, + seed: &SecretVec, + account_index: zip32::AccountId, + birthday: &AccountBirthday, + ) -> Result<(Self::Account, UnifiedSpendingKey), Self::Error> { + self.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 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(); + + let account = wallet::add_account( + wdb.conn.0, + &wdb.params, + AccountSource::Derived { + seed_fingerprint, + account_index, + }, + wallet::ViewingKey::Full(Box::new(ufvk)), + birthday, + )?; + + Ok((account, usk)) + }) + } + + fn import_account_ufvk( + &mut self, + ufvk: &UnifiedFullViewingKey, + birthday: &AccountBirthday, + _spending_key_available: bool, + ) -> Result { + self.transactionally(|wdb| { + Ok(wallet::add_account( + wdb.conn.0, + &wdb.params, + AccountSource::Imported, + wallet::ViewingKey::Full(Box::new(ufvk.to_owned())), + birthday, + )?) }) } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 63ee3e163..9f15ffe50 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -358,7 +358,7 @@ pub(crate) fn add_account( kind: AccountSource, viewing_key: ViewingKey, birthday: &AccountBirthday, -) -> Result { +) -> Result { let (hd_seed_fingerprint, hd_account_index) = match kind { AccountSource::Derived { seed_fingerprint, @@ -524,7 +524,7 @@ pub(crate) fn add_account( let (address, d_idx) = account.default_address(DEFAULT_UA_REQUEST)?; insert_address(conn, params, account_id, d_idx, &address)?; - Ok(account_id) + Ok(account) } pub(crate) fn get_current_address( diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 9fff7e3a3..6b5043a53 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -158,6 +158,7 @@ fn sqlite_client_error_to_wallet_migration_error(e: SqliteClientError) -> Wallet | SqliteClientError::KeyDerivationError(_) | SqliteClientError::AccountIdDiscontinuity | SqliteClientError::AccountIdOutOfRange + | SqliteClientError::AccountCollision(_) | SqliteClientError::CacheMiss(_) => { unreachable!("we only call WalletRead methods; mutations can't occur") } From d27bf4fc125085eb830350a7ff8583fba9a7015b Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Mon, 17 Jun 2024 08:57:12 -0600 Subject: [PATCH 02/11] Fix importing of UFVKs with fewer keys than possible --- zcash_client_sqlite/src/wallet.rs | 17 ++++++++++++++--- zcash_keys/CHANGELOG.md | 3 +++ zcash_keys/src/keys.rs | 30 ++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 9f15ffe50..d9fcfafe2 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -213,9 +213,13 @@ impl Account { &self, request: UnifiedAddressRequest, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { + self.uivk().default_address(request) + } + + pub(crate) fn uivk(&self) -> UnifiedIncomingViewingKey { match &self.viewing_key { - ViewingKey::Full(ufvk) => ufvk.default_address(request), - ViewingKey::Incoming(uivk) => uivk.default_address(request), + ViewingKey::Full(ufvk) => ufvk.to_unified_incoming_viewing_key(), + ViewingKey::Incoming(uivk) => *uivk.clone(), } } } @@ -521,7 +525,14 @@ pub(crate) fn add_account( } // Always derive the default Unified Address for the account. - let (address, d_idx) = account.default_address(DEFAULT_UA_REQUEST)?; + let ua_request = account + .uivk() + .to_address_request() + .and_then(|ua_request| ua_request.intersect(&DEFAULT_UA_REQUEST)) + .ok_or_else(|| { + SqliteClientError::AddressGeneration(AddressGenerationError::ShieldedReceiverRequired) + })?; + let (address, d_idx) = account.default_address(ua_request)?; insert_address(conn, params, account_id, d_idx, &address)?; Ok(account) diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index 3e95ab948..d4faf91ae 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -8,6 +8,9 @@ and this library adheres to Rust's notion of ### Added - `zcash_keys::address::Address::try_from_zcash_address` - `zcash_keys::address::Receiver` +- `zcash_keys::keys::UnifiedAddressRequest` + - `intersect` + - `to_address_request` ### Changed - MSRV is now 1.70.0. diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 0115e1528..8b18b6462 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -587,6 +587,16 @@ impl UnifiedAddressRequest { Self::new(_has_orchard, _has_sapling, _has_p2pkh) } + /// 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, + ) + } + /// Construct a new unified address request from its constituent parts. /// /// Panics: at least one of `has_orchard` or `has_sapling` must be `true`. @@ -1201,6 +1211,26 @@ impl UnifiedIncomingViewingKey { ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { self.find_address(DiversifierIndex::new(), request) } + + /// Constructs a [`UnifiedAddressRequest`] that includes the components of this UIVK. + pub fn to_address_request(&self) -> Option { + #[cfg(feature = "orchard")] + let has_orchard = self.orchard.is_some(); + #[cfg(not(feature = "orchard"))] + let has_orchard = false; + + #[cfg(feature = "sapling")] + let has_sapling = self.sapling.is_some(); + #[cfg(not(feature = "sapling"))] + let has_sapling = false; + + #[cfg(feature = "transparent-inputs")] + let has_p2pkh = self.transparent.is_some(); + #[cfg(not(feature = "transparent-inputs"))] + let has_p2pkh = false; + + UnifiedAddressRequest::new(has_orchard, has_sapling, has_p2pkh) + } } #[cfg(any(test, feature = "test-dependencies"))] From b075636e86c4c192548d47f862d960a3b736f7c7 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Tue, 18 Jun 2024 11:12:05 -0600 Subject: [PATCH 03/11] Add several tests --- zcash_client_sqlite/src/lib.rs | 118 +++++++++++++++++++++++++++-- zcash_client_sqlite/src/testing.rs | 31 ++++++-- zcash_client_sqlite/src/wallet.rs | 91 +++++++++++++--------- 3 files changed, 196 insertions(+), 44 deletions(-) diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 1388b8a5d..ae4d7f086 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -612,13 +612,13 @@ impl WalletWrite for WalletDb _spending_key_available: bool, ) -> Result { self.transactionally(|wdb| { - Ok(wallet::add_account( + wallet::add_account( wdb.conn.0, &wdb.params, AccountSource::Imported, wallet::ViewingKey::Full(Box::new(ufvk.to_owned())), birthday, - )?) + ) }) } @@ -1909,11 +1909,14 @@ extern crate assert_matches; #[cfg(test)] mod tests { - use secrecy::SecretVec; - use zcash_client_backend::data_api::{WalletRead, WalletWrite}; + use secrecy::{Secret, SecretVec}; + use zcash_client_backend::data_api::{ + chain::ChainState, Account, AccountBirthday, AccountSource, WalletRead, WalletWrite, + }; + use zcash_keys::keys::UnifiedSpendingKey; use zcash_primitives::block::BlockHash; - use crate::{testing::TestBuilder, AccountId, DEFAULT_UA_REQUEST}; + use crate::{error::SqliteClientError, testing::TestBuilder, AccountId, DEFAULT_UA_REQUEST}; #[cfg(feature = "unstable")] use { @@ -1977,6 +1980,111 @@ mod tests { assert_eq!(addr2, addr2_cur); } + #[test] + pub(crate) fn import_account_hd_0() { + let st = TestBuilder::new() + .with_account_from_sapling_activation(BlockHash([0; 32])) + .with_account_having_index(zip32::AccountId::ZERO) + .build(); + assert_matches!( + st.test_account().unwrap().account().source(), + AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32::AccountId::ZERO); + } + + #[test] + pub(crate) fn import_account_hd_1_then_2() { + let mut st = TestBuilder::new().build(); + + let birthday = AccountBirthday::from_parts( + ChainState::empty(st.wallet().params.sapling.unwrap() - 1, BlockHash([0; 32])), + None, + ); + + let seed = Secret::new(vec![0u8; 32]); + let zip32_index = zip32::AccountId::ZERO.next().unwrap(); + + let first = st + .wallet_mut() + .import_account_hd(&seed, zip32_index, &birthday) + .unwrap(); + assert_matches!( + first.0.source(), + AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index); + + let second = st + .wallet_mut() + .import_account_hd(&seed, zip32_index.next().unwrap(), &birthday) + .unwrap(); + assert_matches!( + second.0.source(), + AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index.next().unwrap()); + } + + #[test] + pub(crate) fn import_account_hd_1_twice() { + let mut st = TestBuilder::new().build(); + + let birthday = AccountBirthday::from_parts( + ChainState::empty(st.wallet().params.sapling.unwrap() - 1, BlockHash([0; 32])), + None, + ); + + let seed = Secret::new(vec![0u8; 32]); + let zip32_index = zip32::AccountId::ZERO.next().unwrap(); + + let first = st + .wallet_mut() + .import_account_hd(&seed, zip32_index, &birthday) + .unwrap(); + + assert_matches!( + st.wallet_mut().import_account_hd(&seed, zip32_index, &birthday), + Err(SqliteClientError::AccountCollision(id)) if id == first.0.id()); + } + + #[test] + pub(crate) fn import_account_ufvk() { + let mut st = TestBuilder::new().build(); + + let birthday = AccountBirthday::from_parts( + ChainState::empty(st.wallet().params.sapling.unwrap() - 1, BlockHash([0; 32])), + None, + ); + + let seed = vec![0u8; 32]; + let usk = UnifiedSpendingKey::from_seed(&st.wallet().params, &seed, zip32::AccountId::ZERO) + .unwrap(); + let ufvk = usk.to_unified_full_viewing_key(); + + let account = st + .wallet_mut() + .import_account_ufvk(&ufvk, &birthday, true) + .unwrap(); + assert_eq!( + ufvk.encode(&st.wallet().params), + account.ufvk().unwrap().encode(&st.wallet().params) + ); + } + + #[test] + pub(crate) fn create_account_then_conflicting_import_account_ufvk() { + let mut st = TestBuilder::new().build(); + + let birthday = AccountBirthday::from_parts( + ChainState::empty(st.wallet().params.sapling.unwrap() - 1, BlockHash([0; 32])), + None, + ); + + let seed = Secret::new(vec![0u8; 32]); + let seed_based = st.wallet_mut().create_account(&seed, &birthday).unwrap(); + let seed_based_account = st.wallet().get_account(seed_based.0).unwrap().unwrap(); + let ufvk = seed_based_account.ufvk().unwrap(); + + assert_matches!( + st.wallet_mut().import_account_ufvk(ufvk, &birthday, true), + Err(SqliteClientError::AccountCollision(id)) if id == seed_based.0); + } + #[cfg(feature = "transparent-inputs")] #[test] fn transparent_receivers() { diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index 89bde533b..12e215d55 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -26,6 +26,7 @@ use sapling::{ zip32::DiversifiableFullViewingKey, Note, Nullifier, }; +use zcash_client_backend::data_api::Account as AccountTrait; #[allow(deprecated)] use zcash_client_backend::{ address::Address, @@ -74,7 +75,7 @@ use crate::{ error::SqliteClientError, wallet::{ commitment_tree, get_wallet_summary, init::init_wallet_db, sapling::tests::test_prover, - SubtreeScanProgress, + Account, SubtreeScanProgress, }, AccountId, ReceivedNoteId, WalletDb, }; @@ -117,6 +118,7 @@ pub(crate) struct TestBuilder { cache: Cache, initial_chain_state: Option, account_birthday: Option, + account_index: Option, } impl TestBuilder<()> { @@ -143,6 +145,7 @@ impl TestBuilder<()> { cache: (), initial_chain_state: None, account_birthday: None, + account_index: None, } } @@ -154,6 +157,7 @@ impl TestBuilder<()> { cache: BlockCache::new(), initial_chain_state: self.initial_chain_state, account_birthday: self.account_birthday, + account_index: self.account_index, } } @@ -166,6 +170,7 @@ impl TestBuilder<()> { cache: FsBlockCache::new(), initial_chain_state: self.initial_chain_state, account_birthday: self.account_birthday, + account_index: self.account_index, } } } @@ -227,6 +232,12 @@ impl TestBuilder { self } + pub(crate) fn with_account_having_index(mut self, index: zip32::AccountId) -> Self { + assert!(self.account_index.is_none()); + self.account_index = Some(index); + self + } + /// Builds the state for this test. pub(crate) fn build(self) -> TestState { let data_file = NamedTempFile::new().unwrap(); @@ -288,11 +299,17 @@ impl TestBuilder { let test_account = self.account_birthday.map(|birthday| { let seed = Secret::new(vec![0u8; 32]); - let (account_id, usk) = db_data.create_account(&seed, &birthday).unwrap(); + let (account, usk) = match self.account_index { + Some(index) => db_data.import_account_hd(&seed, index, &birthday).unwrap(), + None => { + let result = db_data.create_account(&seed, &birthday).unwrap(); + (db_data.get_account(result.0).unwrap().unwrap(), result.1) + } + }; ( seed, TestAccount { - account_id, + account, usk, birthday, }, @@ -394,14 +411,18 @@ impl CachedBlock { #[derive(Clone)] pub(crate) struct TestAccount { - account_id: AccountId, + account: Account, usk: UnifiedSpendingKey, birthday: AccountBirthday, } impl TestAccount { + pub(crate) fn account(&self) -> &Account { + &self.account + } + pub(crate) fn account_id(&self) -> AccountId { - self.account_id + self.account.id() } pub(crate) fn usk(&self) -> &UnifiedSpendingKey { diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index d9fcfafe2..e90fc857b 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -65,7 +65,7 @@ //! - `memo` the shielded memo associated with the output, if any. use incrementalmerkletree::Retention; -use rusqlite::{self, named_params, OptionalExtension}; +use rusqlite::{self, named_params, params, OptionalExtension}; use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; use zip32::fingerprint::SeedFingerprint; @@ -390,40 +390,63 @@ pub(crate) fn add_account( #[cfg(not(feature = "orchard"))] let birthday_orchard_tree_size: Option = None; - let account_id: AccountId = conn.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 + let ufvk_encoded = viewing_key.ufvk().map(|ufvk| ufvk.encode(params)); + let account_id: AccountId = conn + .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": account_kind_code(kind), + ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.to_bytes()), + ":hd_account_index": hd_account_index.map(u32::from), + ":ufvk": ufvk_encoded, + ":uivk": viewing_key.uivk().encode(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(AccountId(row.get(0)?)), ) - 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": account_kind_code(kind), - ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.to_bytes()), - ":hd_account_index": hd_account_index.map(u32::from), - ":ufvk": viewing_key.ufvk().map(|ufvk| ufvk.encode(params)), - ":uivk": viewing_key.uivk().encode(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(AccountId(row.get(0)?)), - )?; + .map_err(|e| match e { + rusqlite::Error::SqliteFailure(f, s) + if f.code == rusqlite::ErrorCode::ConstraintViolation => + { + // An account conflict occurred. + // Make a best effort to determine the AccountId of the pre-existing row + // and provide that to our caller. + if s.clone().is_some_and(|s| s.contains(".ufvk")) { + if let Ok(id) = conn.query_row( + "SELECT id FROM accounts WHERE ufvk = ?", + params![ufvk_encoded], + |row| Ok(AccountId(row.get(0)?)), + ) { + return SqliteClientError::AccountCollision(id); + } + } + + SqliteClientError::from(rusqlite::Error::SqliteFailure(f, s)) + } + _ => SqliteClientError::from(e), + })?; let account = Account { account_id, From 36b3fc866adde5038cf677062abd99476c88d909 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Thu, 18 Jul 2024 13:00:18 -0600 Subject: [PATCH 04/11] Drop the check on the error message --- zcash_client_sqlite/src/wallet.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 5f049bdaa..27876f062 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -422,14 +422,12 @@ pub(crate) fn add_account( // An account conflict occurred. // Make a best effort to determine the AccountId of the pre-existing row // and provide that to our caller. - if s.clone().is_some_and(|s| s.contains(".ufvk")) { - if let Ok(id) = conn.query_row( - "SELECT id FROM accounts WHERE ufvk = ?", - params![ufvk_encoded], - |row| Ok(AccountId(row.get(0)?)), - ) { - return SqliteClientError::AccountCollision(id); - } + if let Ok(id) = conn.query_row( + "SELECT id FROM accounts WHERE ufvk = ?", + params![ufvk_encoded], + |row| Ok(AccountId(row.get(0)?)), + ) { + return SqliteClientError::AccountCollision(id); } SqliteClientError::from(rusqlite::Error::SqliteFailure(f, s)) From fa5248def04a57baa6a5c67b5eb2bb4e6a774d3a Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Thu, 18 Jul 2024 18:10:04 -0600 Subject: [PATCH 05/11] Fix clippy error --- zcash_client_sqlite/src/error.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 882c64f25..b15a06451 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -16,7 +16,6 @@ use crate::PRUNING_DEPTH; #[cfg(feature = "transparent-inputs")] use { - crate::AccountId, zcash_client_backend::encoding::TransparentCodecError, zcash_primitives::{legacy::TransparentAddress, transaction::TxId}, }; From 664526f74a3f29630bd88e8403cdc2616f0c6ff0 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Tue, 23 Jul 2024 19:47:37 -0600 Subject: [PATCH 06/11] Fixes for nearly all PR comments --- zcash_client_backend/CHANGELOG.md | 2 +- zcash_client_backend/src/data_api.rs | 35 +++++++++++++++++++++------- zcash_client_sqlite/src/error.rs | 2 +- zcash_client_sqlite/src/lib.rs | 21 ++++++++++------- zcash_client_sqlite/src/testing.rs | 5 +++- zcash_client_sqlite/src/wallet.rs | 17 ++++---------- 6 files changed, 49 insertions(+), 33 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 3937e61c9..9f4cb9905 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -26,7 +26,6 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail ### Added - `zcash_client_backend::data_api`: - `chain::BlockCache` trait, behind the `sync` feature flag. - - `WalletWrite` trait methods `import_account_hd` and `import_account_ufvk`. - `WalletRead::get_spendable_transparent_outputs`. - `zcash_client_backend::fees`: - `EphemeralBalance` @@ -63,6 +62,7 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail methods when the "transparent-inputs" feature is enabled. - `WalletWrite` has a new `reserve_next_n_ephemeral_addresses` method when the "transparent-inputs" feature is enabled. + - `WalletWrite` has new methods `import_account_hd` and `import_account_ufvk`. - `error::Error` has new `Address` and (when the "transparent-inputs" feature is enabled) `PaysEphemeralTransparentAddress` variants. - `wallet::input_selection::InputSelectorError` has a new `Address` variant. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 7f5aaf4c2..5df58c767 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1565,6 +1565,13 @@ pub trait WalletWrite: WalletRead { /// Tells the wallet to track the next available account-level spend authority, given the /// current set of [ZIP 316] account identifiers known to the wallet database. /// + /// The "next available account" is defined as the first unused ZIP-32 account index (counting from 0) + /// among all accounts that share the given seed. + /// When [`Self::import_account_hd`] is used to import an account with a specific index, + /// account indexes *may* become fragmented instead of one contiguous range. + /// Where fragmentation occurs, the implementations *may* choose to find the first unused + /// account index or add 1 to the highest existing account index. + /// /// Returns the account identifier for the newly-created wallet database entry, along with the /// associated [`UnifiedSpendingKey`]. Note that the unique account identifier should *not* be /// assumed equivalent to the ZIP 32 account index. It is an opaque identifier for a pool of @@ -1605,13 +1612,11 @@ pub trait WalletWrite: WalletRead { /// Tells the wallet to track a specific account index for a given seed. /// - /// Returns the account for the newly-created wallet database entry, along with the - /// associated [`UnifiedSpendingKey`]. Note that the unique account identifier should *not* be - /// assumed equivalent to the ZIP 32 account index. It is an opaque identifier for a pool of - /// funds or set of outputs controlled by a single spending authority. - /// - /// The ZIP-32 account index may be obtained by calling [`WalletRead::get_account`] - /// with the returned account identifier. + /// Returns details about the imported account, including the unique account + /// identifier for the newly-created wallet database entry, along with the associated + /// [`UnifiedSpendingKey`]. Note that the unique account identifier should *not* be + /// assumed equivalent to the ZIP 32 account index. It is an opaque identifier for a + /// pool of funds or set of outputs controlled by a single spending authority. /// /// If `birthday.height()` is below the current chain tip, this operation will /// trigger a re-scan of the blocks at and above the provided height. The birthday height is @@ -1631,6 +1636,14 @@ pub trait WalletWrite: WalletRead { /// By convention, wallets should only allow a new account to be generated after confirmed /// funds have been received by the currently-available account (in order to enable automated /// account recovery). + /// The [`Self::create_occount`] function helps to conform with this convention by automatically + /// selecting the next unused account index, while *this* function should be used only + /// to import an existing account with a specific index. + /// + /// Avoid fragmenting the account indexes by importing accounts with indexes that are one greater + /// than the highest existing account index. + /// + /// # Panics /// /// Panics if the length of the seed is not between 32 and 252 bytes inclusive. /// @@ -1644,7 +1657,11 @@ pub trait WalletWrite: WalletRead { /// Tells the wallet to track an account using a unified full viewing key. /// - /// Returns the account for the newly-created wallet database entry. + /// Returns details about the imported account, including the unique account + /// identifier for the newly-created wallet database entry. Unlike the other account + /// creation APIs ([`Self::create_account`] and [`Self::import_account_hd`]), no + /// spending key is returned because the wallet has no information about how the UFVK + /// was derived. /// /// If `birthday.height()` is below the current chain tip, this operation will /// trigger a re-scan of the blocks at and above the provided height. The birthday height is @@ -1661,6 +1678,8 @@ pub trait WalletWrite: WalletRead { /// were used for the wallet birthday, a transaction targeted at a height greater than the /// chain tip could be mined at a height below that tip as part of a reorg. /// + /// # Panics + /// /// Panics if the length of the seed is not between 32 and 252 bytes inclusive. fn import_account_ufvk( &mut self, diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index b15a06451..b52f7bf3a 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -171,7 +171,7 @@ impl fmt::Display for SqliteClientError { SqliteClientError::BadAccountData(e) => write!(f, "Failed to add account: {}", e), SqliteClientError::AccountIdDiscontinuity => write!(f, "Wallet account identifiers must be sequential."), SqliteClientError::AccountIdOutOfRange => write!(f, "Wallet account identifiers must be less than 0x7FFFFFFF."), - SqliteClientError::AccountCollision(id) => write!(f, "An account with ID {} already exists in the wallet.", id.0), + SqliteClientError::AccountCollision(id) => write!(f, "An account corresponding to the data provided already exists in the wallet with internal identifier {}.", id.0), #[cfg(feature = "transparent-inputs")] SqliteClientError::AddressNotRecognized(_) => write!(f, "The address associated with a received txo is not identifiable as belonging to the wallet."), SqliteClientError::CommitmentTree(err) => write!(f, "An error occurred accessing or updating note commitment tree data: {}.", err), diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 1146880ac..977d25e6a 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -2078,7 +2078,7 @@ mod tests { pub(crate) fn import_account_hd_0() { let st = TestBuilder::new() .with_account_from_sapling_activation(BlockHash([0; 32])) - .with_account_having_index(zip32::AccountId::ZERO) + .set_account_index(zip32::AccountId::ZERO) .build(); assert_matches!( st.test_account().unwrap().account().source(), @@ -2095,23 +2095,24 @@ mod tests { ); let seed = Secret::new(vec![0u8; 32]); - let zip32_index = zip32::AccountId::ZERO.next().unwrap(); + let zip32_index_1 = zip32::AccountId::ZERO.next().unwrap(); let first = st .wallet_mut() - .import_account_hd(&seed, zip32_index, &birthday) + .import_account_hd(&seed, zip32_index_1, &birthday) .unwrap(); assert_matches!( first.0.source(), - AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index); + AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index_1); + let zip32_index_2 = zip32_index_1.next().unwrap(); let second = st .wallet_mut() - .import_account_hd(&seed, zip32_index.next().unwrap(), &birthday) + .import_account_hd(&seed, zip32_index_2, &birthday) .unwrap(); assert_matches!( second.0.source(), - AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index.next().unwrap()); + AccountSource::Derived { seed_fingerprint: _, account_index } if account_index == zip32_index_2); } #[test] @@ -2124,15 +2125,15 @@ mod tests { ); let seed = Secret::new(vec![0u8; 32]); - let zip32_index = zip32::AccountId::ZERO.next().unwrap(); + let zip32_index_1 = zip32::AccountId::ZERO.next().unwrap(); let first = st .wallet_mut() - .import_account_hd(&seed, zip32_index, &birthday) + .import_account_hd(&seed, zip32_index_1, &birthday) .unwrap(); assert_matches!( - st.wallet_mut().import_account_hd(&seed, zip32_index, &birthday), + st.wallet_mut().import_account_hd(&seed, zip32_index_1, &birthday), Err(SqliteClientError::AccountCollision(id)) if id == first.0.id()); } @@ -2158,6 +2159,8 @@ mod tests { ufvk.encode(&st.wallet().params), account.ufvk().unwrap().encode(&st.wallet().params) ); + + assert_matches!(account.source(), AccountSource::Imported); } #[test] diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index 2af48914a..517d0d47a 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -216,7 +216,10 @@ impl TestBuilder { self } - pub(crate) fn with_account_having_index(mut self, index: zip32::AccountId) -> Self { + /// Sets the [`account_index`] field for the test account + /// + /// Call either [`with_account_from_sapling_activation`] or [`with_account_having_current_birthday`] before calling this method. + pub(crate) fn set_account_index(mut self, index: zip32::AccountId) -> Self { assert!(self.account_index.is_none()); self.account_index = Some(index); self diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 27876f062..5f0d47d89 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -81,7 +81,7 @@ use zcash_address::ZcashAddress; use zcash_client_backend::{ data_api::{ scanning::{ScanPriority, ScanRange}, - AccountBalance, AccountBirthday, AccountSource, BlockMetadata, Ratio, + Account as _, AccountBalance, AccountBirthday, AccountSource, BlockMetadata, Ratio, SentTransactionOutput, WalletSummary, SAPLING_SHARD_HEIGHT, }, encoding::AddressCodec, @@ -204,13 +204,6 @@ impl Account { ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { self.uivk().default_address(request) } - - pub(crate) fn uivk(&self) -> UnifiedIncomingViewingKey { - match &self.viewing_key { - ViewingKey::Full(ufvk) => ufvk.to_unified_incoming_viewing_key(), - ViewingKey::Incoming(uivk) => *uivk.clone(), - } - } } impl zcash_client_backend::data_api::Account for Account { @@ -534,7 +527,9 @@ pub(crate) fn add_account( )?; } - // Always derive the default Unified Address for the account. + // Always derive the default Unified Address for the account. If the account's viewing + // key has fewer components than the wallet supports (most likely due to this being an + // imported viewing key), derive an address containing the common subset of receivers. let ua_request = account .uivk() .to_address_request() @@ -549,10 +544,6 @@ pub(crate) fn add_account( #[cfg(feature = "transparent-inputs")] transparent::ephemeral::init_account(conn, params, account_id)?; - // Initialize the `ephemeral_addresses` table. - #[cfg(feature = "transparent-inputs")] - transparent::ephemeral::init_account(conn, params, account_id)?; - Ok(account) } From cc39084de44fe4abf5fac408392028ea83f0ddae Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Tue, 23 Jul 2024 20:18:10 -0600 Subject: [PATCH 07/11] Improve docs around account creation --- zcash_client_backend/src/data_api.rs | 101 +++++++++++++-------------- 1 file changed, 50 insertions(+), 51 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 5df58c767..61ceb6c32 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1558,6 +1558,51 @@ impl AccountBirthday { /// This trait encapsulates the write capabilities required to update stored /// wallet data. +/// +/// # Creating a new wallet +/// +/// A wallet may be created by generating a new seed phrase and creating an account with that seed. +/// Account creation is typically done with the [`WalletWrite::create_account`] method. +/// +/// Callers should construct the [`AccountBirthday`] using +/// [`AccountBirthday::from_treestate`] for the block at height `chain_tip_height - 100`. +/// Setting the birthday height to a tree state below the pruning depth ensures that reorgs +/// cannot cause funds intended for the wallet to be missed; otherwise, if the chain tip height +/// were used for the wallet birthday, a transaction targeted at a height greater than the +/// chain tip could be mined at a height below that tip as part of a reorg. +/// +/// # Restoring from backup +/// +/// A wallet may restore existing accounts rather than create new ones. +/// This will typically be done by the user providing a seed phrase created in a previous wallet. +/// A seed phrase will allow generation of keys for any number of accounts. +/// +/// Knowing exactly how many accounts to activate based on the seed phrase is part of account recovery, +/// which is that we assume only one account may have been used to receive funds. +/// Once we find that it actually *has* received funds, we assume that the next account may have been used +/// and create the next account and start using its keys to scan subsequent blocks. +/// This process is repeated until we find an account that has not received funds. +/// Account recovery has *not* yet been implemented by this crate. So a wallet app that supports multiple accounts +/// is expected to implement it manually for now. +/// +/// If the number of accounts is known in advance, the wallet should create all accounts +/// before scanning the chain so that the scan can be done in a single pass for all accounts. +/// +/// # Account creation +/// +/// Any of several functions may be used to create the first or subsequent accounts, including: +/// - [`WalletWrite::create_account`] takes a seed phrase and creates a new account with the smallest unused ZIP-32 account index. +/// - [`WalletWrite::import_account_hd`] takes a seed phrase and creates an account with a specific ZIP-32 account index. +/// - [`WalletWrite::import_account_ufvk`] creates an account with a specific Unified Full Viewing Key. No assumption of ZIP-32 HD derivation is made. +/// +/// All of the account creation/import functions take a birthday height. +/// If `birthday.height()` is below the current chain tip, this operation will +/// trigger a re-scan of the blocks at and above the provided height. The birthday height is +/// defined as the minimum block height that will be scanned for funds belonging to the wallet. +/// +/// By convention, wallets should only allow a new account to be generated after confirmed +/// funds have been received by the newest existing account. This allows automated +/// account recovery to recover all funds. pub trait WalletWrite: WalletRead { /// The type of identifiers used to look up transparent UTXOs. type UtxoRef; @@ -1580,24 +1625,7 @@ pub trait WalletWrite: WalletRead { /// The ZIP-32 account index may be obtained by calling [`WalletRead::get_account`] /// with the returned account identifier. /// - /// If `birthday.height()` is below the current chain tip, this operation will - /// trigger a re-scan of the blocks at and above the provided height. The birthday height is - /// defined as the minimum block height that will be scanned for funds belonging to the wallet. - /// - /// For new wallets, callers should construct the [`AccountBirthday`] using - /// [`AccountBirthday::from_treestate`] for the block at height `chain_tip_height - 100`. - /// Setting the birthday height to a tree state below the pruning depth ensures that reorgs - /// cannot cause funds intended for the wallet to be missed; otherwise, if the chain tip height - /// were used for the wallet birthday, a transaction targeted at a height greater than the - /// chain tip could be mined at a height below that tip as part of a reorg. - /// - /// If `seed` was imported from a backup and this method is being used to restore a previous - /// wallet state, you should use this method to add all of the desired accounts before scanning - /// the chain from the seed's birthday height. - /// - /// By convention, wallets should only allow a new account to be generated after confirmed - /// funds have been received by the currently-available account (in order to enable automated - /// account recovery). + /// Learn more about account creation and import in the [`WalletWrite`] trait documentation. /// /// # Panics /// @@ -1618,30 +1646,10 @@ pub trait WalletWrite: WalletRead { /// assumed equivalent to the ZIP 32 account index. It is an opaque identifier for a /// pool of funds or set of outputs controlled by a single spending authority. /// - /// If `birthday.height()` is below the current chain tip, this operation will - /// trigger a re-scan of the blocks at and above the provided height. The birthday height is - /// defined as the minimum block height that will be scanned for funds belonging to the wallet. + /// Import accounts with indexes that are exactly one greater than the highest existing account index + /// to ensure account indexes are contiguous, thereby facilitating automated account recovery. /// - /// For new wallets, callers should construct the [`AccountBirthday`] using - /// [`AccountBirthday::from_treestate`] for the block at height `chain_tip_height - 100`. - /// Setting the birthday height to a tree state below the pruning depth ensures that reorgs - /// cannot cause funds intended for the wallet to be missed; otherwise, if the chain tip height - /// were used for the wallet birthday, a transaction targeted at a height greater than the - /// chain tip could be mined at a height below that tip as part of a reorg. - /// - /// If `seed` was imported from a backup and this method is being used to restore a previous - /// wallet state, you should use this method to add all of the desired accounts before scanning - /// the chain from the seed's birthday height. - /// - /// By convention, wallets should only allow a new account to be generated after confirmed - /// funds have been received by the currently-available account (in order to enable automated - /// account recovery). - /// The [`Self::create_occount`] function helps to conform with this convention by automatically - /// selecting the next unused account index, while *this* function should be used only - /// to import an existing account with a specific index. - /// - /// Avoid fragmenting the account indexes by importing accounts with indexes that are one greater - /// than the highest existing account index. + /// Learn more about account creation and import in the [`WalletWrite`] trait documentation. /// /// # Panics /// @@ -1663,20 +1671,11 @@ pub trait WalletWrite: WalletRead { /// spending key is returned because the wallet has no information about how the UFVK /// was derived. /// - /// If `birthday.height()` is below the current chain tip, this operation will - /// trigger a re-scan of the blocks at and above the provided height. The birthday height is - /// defined as the minimum block height that will be scanned for funds belonging to the wallet. - /// /// Certain optimizations are possible for accounts which will never be used to spend funds. /// If `spending_key_available` is `false`, the wallet may choose to optimize for this case, /// in which case any attempt to spend funds from the account will result in an error. /// - /// For new wallets, callers should construct the [`AccountBirthday`] using - /// [`AccountBirthday::from_treestate`] for the block at height `chain_tip_height - 100`. - /// Setting the birthday height to a tree state below the pruning depth ensures that reorgs - /// cannot cause funds intended for the wallet to be missed; otherwise, if the chain tip height - /// were used for the wallet birthday, a transaction targeted at a height greater than the - /// chain tip could be mined at a height below that tip as part of a reorg. + /// Learn more about account creation and import in the [`WalletWrite`] trait documentation. /// /// # Panics /// From c9ecc1196a21923a727d1f77490cb4bd6952a375 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Tue, 23 Jul 2024 20:36:32 -0600 Subject: [PATCH 08/11] Record whether imported accounts have a spending key --- zcash_client_sqlite/src/lib.rs | 7 ++- zcash_client_sqlite/src/wallet.rs | 10 +++- zcash_client_sqlite/src/wallet/db.rs | 1 + .../src/wallet/init/migrations.rs | 14 +++-- .../init/migrations/spend_key_available.rs | 58 +++++++++++++++++++ 5 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 zcash_client_sqlite/src/wallet/init/migrations/spend_key_available.rs diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 977d25e6a..a8c3e190d 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -600,6 +600,7 @@ impl WalletWrite for WalletDb .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?; let ufvk = usk.to_unified_full_viewing_key(); + let spending_key_available = true; let account = wallet::add_account( wdb.conn.0, &wdb.params, @@ -609,6 +610,7 @@ impl WalletWrite for WalletDb }, wallet::ViewingKey::Full(Box::new(ufvk)), birthday, + spending_key_available, )?; Ok((account.id(), usk)) @@ -634,6 +636,7 @@ impl WalletWrite for WalletDb .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?; let ufvk = usk.to_unified_full_viewing_key(); + let spending_key_available = true; let account = wallet::add_account( wdb.conn.0, &wdb.params, @@ -643,6 +646,7 @@ impl WalletWrite for WalletDb }, wallet::ViewingKey::Full(Box::new(ufvk)), birthday, + spending_key_available, )?; Ok((account, usk)) @@ -653,7 +657,7 @@ impl WalletWrite for WalletDb &mut self, ufvk: &UnifiedFullViewingKey, birthday: &AccountBirthday, - _spending_key_available: bool, + spending_key_available: bool, ) -> Result { self.transactionally(|wdb| { wallet::add_account( @@ -662,6 +666,7 @@ impl WalletWrite for WalletDb AccountSource::Imported, wallet::ViewingKey::Full(Box::new(ufvk.to_owned())), birthday, + spending_key_available, ) }) } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 5f0d47d89..807386c7b 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -344,6 +344,7 @@ pub(crate) fn add_account( kind: AccountSource, viewing_key: ViewingKey, birthday: &AccountBirthday, + spending_key_available: bool, ) -> Result { let (hd_seed_fingerprint, hd_account_index) = match kind { AccountSource::Derived { @@ -381,14 +382,16 @@ pub(crate) fn add_account( 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 + recover_until_height, + has_spend_key ) 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 + :recover_until_height, + :has_spend_key ) RETURNING id; "#, @@ -404,7 +407,8 @@ pub(crate) fn add_account( ":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) + ":recover_until_height": birthday.recover_until().map(u32::from), + ":has_spend_key": spending_key_available as i64, ], |row| Ok(AccountId(row.get(0)?)), ) diff --git a/zcash_client_sqlite/src/wallet/db.rs b/zcash_client_sqlite/src/wallet/db.rs index 65ce3b148..3b9ff8d15 100644 --- a/zcash_client_sqlite/src/wallet/db.rs +++ b/zcash_client_sqlite/src/wallet/db.rs @@ -36,6 +36,7 @@ CREATE TABLE "accounts" ( birthday_sapling_tree_size INTEGER, birthday_orchard_tree_size INTEGER, recover_until_height INTEGER, + has_spend_key INTEGER NOT NULL DEFAULT 1, CHECK ( ( account_kind = 0 diff --git a/zcash_client_sqlite/src/wallet/init/migrations.rs b/zcash_client_sqlite/src/wallet/init/migrations.rs index 3e6b056b9..ad533c025 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations.rs @@ -14,6 +14,7 @@ mod receiving_key_scopes; mod sapling_memo_consistency; mod sent_notes_to_internal; mod shardtree_support; +mod spend_key_available; mod ufvk_support; mod utxos_table; mod utxos_to_txos; @@ -63,12 +64,12 @@ pub(super) fn all_migrations( // \ \ | v_transactions_note_uniqueness // \ \ | / // -------------------- full_account_ids - // | - // orchard_received_notes - // / \ - // ensure_orchard_ua_receiver utxos_to_txos - // | - // ephemeral_addresses + // | \ + // orchard_received_notes spend_key_available + // / \ + // ensure_orchard_ua_receiver utxos_to_txos + // | + // ephemeral_addresses vec![ Box::new(initial_setup::Migration {}), Box::new(utxos_table::Migration {}), @@ -122,6 +123,7 @@ pub(super) fn all_migrations( Box::new(ephemeral_addresses::Migration { params: params.clone(), }), + Box::new(spend_key_available::Migration), ] } diff --git a/zcash_client_sqlite/src/wallet/init/migrations/spend_key_available.rs b/zcash_client_sqlite/src/wallet/init/migrations/spend_key_available.rs new file mode 100644 index 000000000..ac74e2bf6 --- /dev/null +++ b/zcash_client_sqlite/src/wallet/init/migrations/spend_key_available.rs @@ -0,0 +1,58 @@ +//! The migration that records ephemeral addresses for each account. +use std::collections::HashSet; + +use rusqlite; +use schemer; +use schemer_rusqlite::RusqliteMigration; +use uuid::Uuid; + +use crate::wallet::init::WalletMigrationError; + +use super::full_account_ids; + +pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0x07610aac_b0e3_4ba8_aaa6_cda606f0fd7b); + +const DEPENDENCIES: [Uuid; 1] = [full_account_ids::MIGRATION_ID]; + +#[allow(dead_code)] +pub(super) struct Migration; + +impl schemer::Migration for Migration { + fn id(&self) -> Uuid { + MIGRATION_ID + } + + fn dependencies(&self) -> HashSet { + DEPENDENCIES.into_iter().collect() + } + + fn description(&self) -> &'static str { + "Track which accounts have associated spending keys." + } +} + +impl RusqliteMigration for Migration { + type Error = WalletMigrationError; + + fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { + transaction.execute_batch( + "ALTER TABLE accounts ADD COLUMN has_spend_key INTEGER NOT NULL DEFAULT 1", + )?; + Ok(()) + } + + fn down(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { + transaction.execute_batch("ALTER TABLE accounts DROP COLUMN has_spend_key")?; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use crate::wallet::init::migrations::tests::test_migrate; + + #[test] + fn migrate() { + test_migrate(&[super::MIGRATION_ID]); + } +} From 5bee983e6c6f2fceedbd78c74ae85b5f57ec55fd Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 25 Jul 2024 16:52:07 -0600 Subject: [PATCH 09/11] zcash_client_sqlite: Make `ephemeral_addresses` migration forward-compatible --- .../init/migrations/ephemeral_addresses.rs | 100 +++++++++++++++++- 1 file changed, 95 insertions(+), 5 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 d9dffaf89..067234792 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs @@ -81,7 +81,98 @@ impl RusqliteMigration for Migration

{ #[cfg(test)] mod tests { - use crate::wallet::init::migrations::tests::test_migrate; + use rusqlite::{named_params, Connection}; + use secrecy::{ExposeSecret, SecretVec}; + use zcash_client_backend::data_api::AccountBirthday; + use zcash_keys::keys::UnifiedSpendingKey; + use zcash_protocol::consensus::Network; + use zip32::fingerprint::SeedFingerprint; + + use crate::{ + error::SqliteClientError, + wallet::{self, init::migrations::tests::test_migrate, transparent}, + AccountId, WalletDb, + }; + + /// This is a minimized copy of [`wallet::create_account`] as of the time of the + /// creation of this migration. + fn create_account( + wdb: &mut WalletDb, + seed: &SecretVec, + birthday: &AccountBirthday, + ) -> Result<(AccountId, 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::AccountIdOutOfRange)) + .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(); + + let orchard_item = ufvk.orchard().map(|k| k.to_bytes()); + 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: AccountId = 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(AccountId(row.get(0)?)), + )?; + + // Initialize the `ephemeral_addresses` table. + #[cfg(feature = "transparent-inputs")] + transparent::ephemeral::init_account(&mut wdb.conn.0, &wdb.params, account_id)?; + + Ok((account_id, usk)) + }) + } #[test] fn migrate() { @@ -95,7 +186,7 @@ mod tests { use secrecy::Secret; use tempfile::NamedTempFile; use zcash_client_backend::{ - data_api::{AccountBirthday, AccountSource, WalletWrite}, + data_api::{AccountBirthday, AccountSource}, wallet::TransparentAddressMetadata, }; use zcash_keys::keys::UnifiedSpendingKey; @@ -192,9 +283,8 @@ mod tests { // Creating a new account should initialize `ephemeral_addresses` for that account. let seed1 = vec![0x01; 32]; - let (account1_id, _usk) = db_data - .create_account(&Secret::new(seed1), &birthday) - .unwrap(); + 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); } From 202137762628713a8aaf902b8c9742036e1cb963 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 25 Jul 2024 20:05:01 -0600 Subject: [PATCH 10/11] Minor documentation formatting. --- zcash_client_backend/src/data_api.rs | 106 ++++++++++++++------------- 1 file changed, 55 insertions(+), 51 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 61ceb6c32..87631aa69 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1556,53 +1556,57 @@ impl AccountBirthday { } } -/// This trait encapsulates the write capabilities required to update stored -/// wallet data. +/// This trait encapsulates the write capabilities required to update stored wallet data. /// /// # Creating a new wallet /// /// A wallet may be created by generating a new seed phrase and creating an account with that seed. /// Account creation is typically done with the [`WalletWrite::create_account`] method. /// -/// Callers should construct the [`AccountBirthday`] using -/// [`AccountBirthday::from_treestate`] for the block at height `chain_tip_height - 100`. -/// Setting the birthday height to a tree state below the pruning depth ensures that reorgs -/// cannot cause funds intended for the wallet to be missed; otherwise, if the chain tip height -/// were used for the wallet birthday, a transaction targeted at a height greater than the -/// chain tip could be mined at a height below that tip as part of a reorg. +/// Callers should construct the [`AccountBirthday`] using [`AccountBirthday::from_treestate`] for +/// the block at height `chain_tip_height - 100`. Setting the birthday height to a tree state below +/// the pruning depth ensures that reorgs cannot cause funds intended for the wallet to be missed; +/// otherwise, if the chain tip height were used for the wallet birthday, a transaction targeted at +/// a height greater than the chain tip could be mined at a height below that tip as part of a +/// reorg. /// /// # Restoring from backup /// -/// A wallet may restore existing accounts rather than create new ones. -/// This will typically be done by the user providing a seed phrase created in a previous wallet. -/// A seed phrase will allow generation of keys for any number of accounts. +/// A wallet may restore existing accounts rather than create new ones. This will typically be done +/// by the user providing a seed phrase created in a previous wallet. A seed phrase will allow +/// generation of keys for any number of accounts. /// -/// Knowing exactly how many accounts to activate based on the seed phrase is part of account recovery, -/// which is that we assume only one account may have been used to receive funds. -/// Once we find that it actually *has* received funds, we assume that the next account may have been used -/// and create the next account and start using its keys to scan subsequent blocks. -/// This process is repeated until we find an account that has not received funds. -/// Account recovery has *not* yet been implemented by this crate. So a wallet app that supports multiple accounts -/// is expected to implement it manually for now. +/// Knowing exactly how many accounts to activate based on the seed phrase is part of account +/// recovery, which is that we assume only one account may have been used to receive funds. Once we +/// find that it actually *has* received funds, we assume that the next account may have been used +/// and create the next account and start using its keys to scan subsequent blocks. This process is +/// repeated until we find an account that has not received funds. Account recovery has *not* yet +/// been implemented by this crate. So a wallet app that supports multiple accounts is expected to +/// implement it manually for now. /// -/// If the number of accounts is known in advance, the wallet should create all accounts -/// before scanning the chain so that the scan can be done in a single pass for all accounts. +/// If the number of accounts is known in advance, the wallet should create all accounts before +/// scanning the chain so that the scan can be done in a single pass for all accounts. /// /// # Account creation /// /// Any of several functions may be used to create the first or subsequent accounts, including: -/// - [`WalletWrite::create_account`] takes a seed phrase and creates a new account with the smallest unused ZIP-32 account index. -/// - [`WalletWrite::import_account_hd`] takes a seed phrase and creates an account with a specific ZIP-32 account index. -/// - [`WalletWrite::import_account_ufvk`] creates an account with a specific Unified Full Viewing Key. No assumption of ZIP-32 HD derivation is made. +/// - [`WalletWrite::create_account`] takes a seed phrase and creates a new account with the +/// smallest unused [`ZIP 32`] account index. +/// - [`WalletWrite::import_account_hd`] takes a seed phrase and creates an account with a specific +/// [`ZIP 32`] account index. +/// - [`WalletWrite::import_account_ufvk`] creates an account with a specific Unified Full Viewing +/// Key. No assumption of [`ZIP 32`] HD derivation is made. /// -/// All of the account creation/import functions take a birthday height. -/// If `birthday.height()` is below the current chain tip, this operation will -/// trigger a re-scan of the blocks at and above the provided height. The birthday height is -/// defined as the minimum block height that will be scanned for funds belonging to the wallet. +/// All of the account creation/import functions take a birthday height. If `birthday.height()` is +/// below the current chain tip, this operation will trigger a re-scan of the blocks at and above +/// the provided height. The birthday height is defined as the minimum block height that will be +/// scanned for funds belonging to the wallet. /// -/// By convention, wallets should only allow a new account to be generated after confirmed -/// funds have been received by the newest existing account. This allows automated -/// account recovery to recover all funds. +/// By convention, wallets should only allow a new account to be generated after confirmed funds +/// have been received by the newest existing account. This allows automated account recovery to +/// recover all funds. +/// +/// [`ZIP 32`]: https://zips.z.cash/zip-0032 pub trait WalletWrite: WalletRead { /// The type of identifiers used to look up transparent UTXOs. type UtxoRef; @@ -1610,12 +1614,12 @@ pub trait WalletWrite: WalletRead { /// Tells the wallet to track the next available account-level spend authority, given the /// current set of [ZIP 316] account identifiers known to the wallet database. /// - /// The "next available account" is defined as the first unused ZIP-32 account index (counting from 0) - /// among all accounts that share the given seed. - /// When [`Self::import_account_hd`] is used to import an account with a specific index, - /// account indexes *may* become fragmented instead of one contiguous range. - /// Where fragmentation occurs, the implementations *may* choose to find the first unused - /// account index or add 1 to the highest existing account index. + /// The "next available account" is defined as the first unused ZIP-32 account index (counting + /// from 0) among all accounts that share the given seed. When [`Self::import_account_hd`] is + /// used to import an account with a specific index, account indexes *may* become fragmented + /// instead of one contiguous range. Where fragmentation occurs, the implementations *may* + /// choose to find the first unused account index or add 1 to the highest existing account + /// index. /// /// Returns the account identifier for the newly-created wallet database entry, along with the /// associated [`UnifiedSpendingKey`]. Note that the unique account identifier should *not* be @@ -1640,14 +1644,15 @@ pub trait WalletWrite: WalletRead { /// Tells the wallet to track a specific account index for a given seed. /// - /// Returns details about the imported account, including the unique account - /// identifier for the newly-created wallet database entry, along with the associated - /// [`UnifiedSpendingKey`]. Note that the unique account identifier should *not* be - /// assumed equivalent to the ZIP 32 account index. It is an opaque identifier for a - /// pool of funds or set of outputs controlled by a single spending authority. + /// Returns details about the imported account, including the unique account identifier for + /// the newly-created wallet database entry, along with the associated [`UnifiedSpendingKey`]. + /// Note that the unique account identifier should *not* be assumed equivalent to the ZIP 32 + /// account index. It is an opaque identifier for a pool of funds or set of outputs controlled + /// by a single spending authority. /// - /// Import accounts with indexes that are exactly one greater than the highest existing account index - /// to ensure account indexes are contiguous, thereby facilitating automated account recovery. + /// Import accounts with indexes that are exactly one greater than the highest existing account + /// index to ensure account indexes are contiguous, thereby facilitating automated account + /// recovery. /// /// Learn more about account creation and import in the [`WalletWrite`] trait documentation. /// @@ -1665,15 +1670,14 @@ pub trait WalletWrite: WalletRead { /// Tells the wallet to track an account using a unified full viewing key. /// - /// Returns details about the imported account, including the unique account - /// identifier for the newly-created wallet database entry. Unlike the other account - /// creation APIs ([`Self::create_account`] and [`Self::import_account_hd`]), no - /// spending key is returned because the wallet has no information about how the UFVK - /// was derived. + /// Returns details about the imported account, including the unique account identifier for + /// the newly-created wallet database entry. Unlike the other account creation APIs + /// ([`Self::create_account`] and [`Self::import_account_hd`]), no spending key is returned + /// because the wallet has no information about how the UFVK was derived. /// - /// Certain optimizations are possible for accounts which will never be used to spend funds. - /// If `spending_key_available` is `false`, the wallet may choose to optimize for this case, - /// in which case any attempt to spend funds from the account will result in an error. + /// Certain optimizations are possible for accounts which will never be used to spend funds. If + /// `spending_key_available` is `false`, the wallet may choose to optimize for this case, in + /// which case any attempt to spend funds from the account will result in an error. /// /// Learn more about account creation and import in the [`WalletWrite`] trait documentation. /// From 31f8e644837d643c1823a8adb82c08abae5bc36d Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 26 Jul 2024 08:55:16 -0600 Subject: [PATCH 11/11] Fix clippy lint. --- .../src/wallet/init/migrations/ephemeral_addresses.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 067234792..02df28563 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs @@ -168,7 +168,7 @@ mod tests { // Initialize the `ephemeral_addresses` table. #[cfg(feature = "transparent-inputs")] - transparent::ephemeral::init_account(&mut wdb.conn.0, &wdb.params, account_id)?; + transparent::ephemeral::init_account(wdb.conn.0, &wdb.params, account_id)?; Ok((account_id, usk)) })