diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 607207685..c8fdf3635 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -63,6 +63,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 9c8e5a932..87631aa69 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -1556,8 +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. +/// +/// # 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. +/// +/// [`ZIP 32`]: https://zips.z.cash/zip-0032 pub trait WalletWrite: WalletRead { /// The type of identifiers used to look up transparent UTXOs. type UtxoRef; @@ -1565,29 +1614,22 @@ 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 /// 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. + /// The ZIP-32 account index may be obtained by calling [`WalletRead::get_account`] + /// with the returned account identifier. /// - /// 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 /// @@ -1600,6 +1642,55 @@ 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 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. + /// + /// Learn more about account creation and import in the [`WalletWrite`] trait documentation. + /// + /// # Panics + /// + /// 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 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. + /// + /// Learn more about account creation and import in the [`WalletWrite`] trait documentation. + /// + /// # Panics + /// + /// 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. /// @@ -2062,6 +2153,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 615e22aef..9fa681ba2 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -31,6 +31,7 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail - MSRV is now 1.70.0. - `zcash_client_sqlite::error::SqliteClientError` has changed variants: - Removed `HdwalletError`. + - Added `AccountCollision`. - Added `TransparentDerivation`. - The `block` column of the `v_transactions` view has been renamed to `mined_height`. diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 3e9ce5721..b52f7bf3a 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -11,11 +11,11 @@ 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")] use { - crate::AccountId, zcash_client_backend::encoding::TransparentCodecError, zcash_primitives::{legacy::TransparentAddress, transaction::TxId}, }; @@ -73,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, @@ -168,6 +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 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 afb3d170b..cb5f43aa8 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -614,7 +614,8 @@ 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 spending_key_available = true; + let account = wallet::add_account( wdb.conn.0, &wdb.params, AccountSource::Derived { @@ -623,9 +624,64 @@ impl WalletWrite for WalletDb }, wallet::ViewingKey::Full(Box::new(ufvk)), birthday, + spending_key_available, )?; - 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 spending_key_available = true; + let account = wallet::add_account( + wdb.conn.0, + &wdb.params, + AccountSource::Derived { + seed_fingerprint, + account_index, + }, + wallet::ViewingKey::Full(Box::new(ufvk)), + birthday, + spending_key_available, + )?; + + Ok((account, usk)) + }) + } + + fn import_account_ufvk( + &mut self, + ufvk: &UnifiedFullViewingKey, + birthday: &AccountBirthday, + spending_key_available: bool, + ) -> Result { + self.transactionally(|wdb| { + wallet::add_account( + wdb.conn.0, + &wdb.params, + AccountSource::Imported, + wallet::ViewingKey::Full(Box::new(ufvk.to_owned())), + birthday, + spending_key_available, + ) }) } @@ -1966,11 +2022,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 { @@ -2034,6 +2093,114 @@ 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])) + .set_account_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_1 = zip32::AccountId::ZERO.next().unwrap(); + + let first = st + .wallet_mut() + .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_1); + + let zip32_index_2 = zip32_index_1.next().unwrap(); + let second = st + .wallet_mut() + .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_2); + } + + #[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_1 = zip32::AccountId::ZERO.next().unwrap(); + + let first = st + .wallet_mut() + .import_account_hd(&seed, zip32_index_1, &birthday) + .unwrap(); + + assert_matches!( + st.wallet_mut().import_account_hd(&seed, zip32_index_1, &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) + ); + + assert_matches!(account.source(), AccountSource::Imported); + } + + #[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 2019723b0..fbc2c625b 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, } } } @@ -210,6 +215,15 @@ impl TestBuilder { 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 + } + /// Builds the state for this test. pub(crate) fn build(self) -> TestState { let data_file = NamedTempFile::new().unwrap(); @@ -271,11 +285,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, }, @@ -377,14 +397,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 b7d0b9230..807386c7b 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::{Marking, 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; @@ -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, @@ -202,10 +202,7 @@ impl Account { &self, request: UnifiedAddressRequest, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { - match &self.viewing_key { - ViewingKey::Full(ufvk) => ufvk.default_address(request), - ViewingKey::Incoming(uivk) => uivk.default_address(request), - } + self.uivk().default_address(request) } } @@ -347,7 +344,8 @@ pub(crate) fn add_account( kind: AccountSource, viewing_key: ViewingKey, birthday: &AccountBirthday, -) -> Result { + spending_key_available: bool, +) -> Result { let (hd_seed_fingerprint, hd_account_index) = match kind { AccountSource::Derived { seed_fingerprint, @@ -375,40 +373,64 @@ 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, + 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, + :has_spend_key + ) + 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), + ":has_spend_key": spending_key_available as i64, + ], + |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 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, @@ -509,15 +531,24 @@ pub(crate) fn add_account( )?; } - // Always derive the default Unified Address for the account. - let (address, d_idx) = account.default_address(DEFAULT_UA_REQUEST)?; + // 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() + .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)?; // Initialize the `ephemeral_addresses` table. #[cfg(feature = "transparent-inputs")] transparent::ephemeral::init_account(conn, params, account_id)?; - Ok(account_id) + Ok(account) } pub(crate) fn get_current_address( diff --git a/zcash_client_sqlite/src/wallet/db.rs b/zcash_client_sqlite/src/wallet/db.rs index b57079041..2f7f01435 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.rs b/zcash_client_sqlite/src/wallet/init.rs index 7c5a27f73..ea6c935c2 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -160,6 +160,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") } 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/ephemeral_addresses.rs b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs index d9dffaf89..02df28563 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(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); } 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]); + } +} diff --git a/zcash_keys/CHANGELOG.md b/zcash_keys/CHANGELOG.md index accb894fb..38b70e9fb 100644 --- a/zcash_keys/CHANGELOG.md +++ b/zcash_keys/CHANGELOG.md @@ -12,6 +12,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 - `zcash_keys::Address` has a new variant `Tex`. diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index 151b9ca5b..51a39fec7 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"))]