From 7d603b8c598b0f9dc69d5f77b954181321317519 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 13 Mar 2024 12:52:47 +0000 Subject: [PATCH 1/8] zcash_client_backend: Add `proto::service::TreeState::to_chain_state` --- zcash_client_backend/CHANGELOG.md | 2 ++ zcash_client_backend/src/proto.rs | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 7aa804d5f..15c08646b 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -24,12 +24,14 @@ and this library adheres to Rust's notion of - `ORCHARD_SHARD_HEIGHT` - `BlockMetadata::orchard_tree_size` - `WalletSummary::next_orchard_subtree_index` + - `chain::ChainState` - `chain::ScanSummary::{spent_orchard_note_count, received_orchard_note_count}` - `zcash_client_backend::fees`: - `orchard` - `ChangeValue::orchard` - `zcash_client_backend::proto`: - `service::TreeState::orchard_tree` + - `service::TreeState::to_chain_state` - `impl TryFrom<&CompactOrchardAction> for CompactAction` - `CompactOrchardAction::{cmx, nf, ephemeral_key}` - `zcash_client_backend::scanning`: diff --git a/zcash_client_backend/src/proto.rs b/zcash_client_backend/src/proto.rs index f020e731d..5277d599c 100644 --- a/zcash_client_backend/src/proto.rs +++ b/zcash_client_backend/src/proto.rs @@ -20,7 +20,7 @@ use zcash_primitives::{ }; use crate::{ - data_api::InputSource, + data_api::{chain::ChainState, InputSource}, fees::{ChangeValue, TransactionBalance}, proposal::{Proposal, ProposalError, ShieldedInputs, Step, StepOutput, StepOutputIndex}, zip321::{TransactionRequest, Zip321Error}, @@ -290,6 +290,20 @@ impl service::TreeState { &orchard_tree_bytes[..], ) } + + /// Parses this tree state into a [`ChainState`] for use with [`scan_cached_blocks`]. + /// + /// [`scan_cached_blocks`]: crate::data_api::chain::scan_cached_blocks + pub fn to_chain_state(&self) -> io::Result { + Ok(ChainState::new( + self.height + .try_into() + .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "Invalid block height"))?, + self.sapling_tree()?.to_frontier(), + #[cfg(feature = "orchard")] + self.orchard_tree()?.to_frontier(), + )) + } } /// Constant for the V1 proposal serialization version. From 634ebf51ef6dae513ac65f1e992287c087625d70 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 13 Mar 2024 14:07:09 +0000 Subject: [PATCH 2/8] Reorder `WalletRead` and `WalletWrite` trait methods for clarity --- zcash_client_backend/src/data_api.rs | 246 +++++++++++++-------------- zcash_client_sqlite/src/lib.rs | 150 ++++++++-------- 2 files changed, 198 insertions(+), 198 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index be9177766..a1b5493ad 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -543,6 +543,17 @@ pub trait WalletRead { /// The concrete account type used by this wallet backend. type Account: Account; + /// Returns a vector with the IDs of all accounts known to this wallet. + fn get_account_ids(&self) -> Result, Self::Error>; + + /// Returns the account corresponding to a given [`HdSeedFingerprint`] and + /// [`zip32::AccountId`], if any. + fn get_seed_account( + &self, + seed: &HdSeedFingerprint, + account_id: zip32::AccountId, + ) -> Result, Self::Error>; + /// Verifies that the given seed corresponds to the viewing key for the specified account. /// /// Returns: @@ -558,12 +569,50 @@ pub trait WalletRead { seed: &SecretVec, ) -> Result; + /// Returns the account corresponding to a given [`UnifiedFullViewingKey`], if any. + fn get_account_for_ufvk( + &self, + ufvk: &UnifiedFullViewingKey, + ) -> Result, Self::Error>; + + /// Returns the most recently generated unified address for the specified account, if the + /// account identifier specified refers to a valid account for this wallet. + /// + /// This will return `Ok(None)` if the account identifier does not correspond to a known + /// account. + fn get_current_address( + &self, + account: Self::AccountId, + ) -> Result, Self::Error>; + + /// Returns the birthday height for the given account, or an error if the account is not known + /// to the wallet. + fn get_account_birthday(&self, account: Self::AccountId) -> Result; + + /// Returns the birthday height for the wallet. + /// + /// This returns the earliest birthday height among accounts maintained by this wallet, + /// or `Ok(None)` if the wallet has no initialized accounts. + fn get_wallet_birthday(&self) -> Result, Self::Error>; + + /// Returns the wallet balances and sync status for an account given the specified minimum + /// number of confirmations, or `Ok(None)` if the wallet has no balance data available. + fn get_wallet_summary( + &self, + min_confirmations: u32, + ) -> Result>, Self::Error>; + /// Returns the height of the chain as known to the wallet as of the most recent call to /// [`WalletWrite::update_chain_tip`]. /// /// This will return `Ok(None)` if the height of the current consensus chain tip is unknown. fn chain_height(&self) -> Result, Self::Error>; + /// Returns the block hash for the block at the given height, if the + /// associated block data is available. Returns `Ok(None)` if the hash + /// is not found in the database. + fn get_block_hash(&self, block_height: BlockHeight) -> Result, Self::Error>; + /// Returns the available block metadata for the block at the specified height, if any. fn block_metadata(&self, height: BlockHeight) -> Result, Self::Error>; @@ -576,6 +625,11 @@ pub trait WalletRead { /// block. fn block_fully_scanned(&self) -> Result, Self::Error>; + /// Returns the block height and hash for the block at the maximum scanned block height. + /// + /// This will return `Ok(None)` if no blocks have been scanned. + fn get_max_height_hash(&self) -> Result, Self::Error>; + /// Returns block metadata for the maximum height that the wallet has scanned. /// /// If the wallet is fully synced, this will be equivalent to `block_fully_scanned`; @@ -614,66 +668,15 @@ pub trait WalletRead { /// Returns the minimum block height corresponding to an unspent note in the wallet. fn get_min_unspent_height(&self) -> Result, Self::Error>; - /// Returns the block hash for the block at the given height, if the - /// associated block data is available. Returns `Ok(None)` if the hash - /// is not found in the database. - fn get_block_hash(&self, block_height: BlockHeight) -> Result, Self::Error>; - - /// Returns the block height and hash for the block at the maximum scanned block height. - /// - /// This will return `Ok(None)` if no blocks have been scanned. - fn get_max_height_hash(&self) -> Result, Self::Error>; - /// Returns the block height in which the specified transaction was mined, or `Ok(None)` if the /// transaction is not in the main chain. fn get_tx_height(&self, txid: TxId) -> Result, Self::Error>; - /// Returns the birthday height for the wallet. - /// - /// This returns the earliest birthday height among accounts maintained by this wallet, - /// or `Ok(None)` if the wallet has no initialized accounts. - fn get_wallet_birthday(&self) -> Result, Self::Error>; - - /// Returns the birthday height for the given account, or an error if the account is not known - /// to the wallet. - fn get_account_birthday(&self, account: Self::AccountId) -> Result; - - /// Returns the most recently generated unified address for the specified account, if the - /// account identifier specified refers to a valid account for this wallet. - /// - /// This will return `Ok(None)` if the account identifier does not correspond to a known - /// account. - fn get_current_address( - &self, - account: Self::AccountId, - ) -> Result, Self::Error>; - /// Returns all unified full viewing keys known to this wallet. fn get_unified_full_viewing_keys( &self, ) -> Result, Self::Error>; - /// Returns the account corresponding to a given [`UnifiedFullViewingKey`], if any. - fn get_account_for_ufvk( - &self, - ufvk: &UnifiedFullViewingKey, - ) -> Result, Self::Error>; - - /// Returns the account corresponding to a given [`HdSeedFingerprint`] and - /// [`zip32::AccountId`], if any. - fn get_seed_account( - &self, - seed: &HdSeedFingerprint, - account_id: zip32::AccountId, - ) -> Result, Self::Error>; - - /// Returns the wallet balances and sync status for an account given the specified minimum - /// number of confirmations, or `Ok(None)` if the wallet has no balance data available. - fn get_wallet_summary( - &self, - min_confirmations: u32, - ) -> Result>, Self::Error>; - /// Returns the memo for a note. /// /// Returns `Ok(None)` if the note is known to the wallet but memo data has not yet been @@ -724,9 +727,6 @@ pub trait WalletRead { ) -> Result, Self::Error> { Ok(HashMap::new()) } - - /// Returns a vector with the IDs of all accounts known to this wallet. - fn get_account_ids(&self) -> Result, Self::Error>; } /// Metadata describing the sizes of the zcash note commitment trees as of a particular block. @@ -1299,6 +1299,15 @@ pub trait WalletWrite: WalletRead { request: UnifiedAddressRequest, ) -> Result, Self::Error>; + /// Updates the wallet's view of the blockchain. + /// + /// This method is used to provide the wallet with information about the state of the + /// blockchain, and detect any previously scanned data that needs to be re-validated + /// before proceeding with scanning. It should be called at wallet startup prior to calling + /// [`WalletRead::suggest_scan_ranges`] in order to provide the wallet with the information it + /// needs to correctly prioritize scanning operations. + fn update_chain_tip(&mut self, tip_height: BlockHeight) -> Result<(), Self::Error>; + /// Updates the state of the wallet database by persisting the provided block information, /// along with the note commitments that were detected when scanning the block for transactions /// pertaining to this wallet. @@ -1313,14 +1322,11 @@ pub trait WalletWrite: WalletRead { blocks: Vec>, ) -> Result<(), Self::Error>; - /// Updates the wallet's view of the blockchain. - /// - /// This method is used to provide the wallet with information about the state of the - /// blockchain, and detect any previously scanned data that needs to be re-validated - /// before proceeding with scanning. It should be called at wallet startup prior to calling - /// [`WalletRead::suggest_scan_ranges`] in order to provide the wallet with the information it - /// needs to correctly prioritize scanning operations. - fn update_chain_tip(&mut self, tip_height: BlockHeight) -> Result<(), Self::Error>; + /// Adds a transparent UTXO received by the wallet to the data store. + fn put_received_transparent_utxo( + &mut self, + output: &WalletTransparentOutput, + ) -> Result; /// Caches a decrypted transaction in the persistent wallet store. fn store_decrypted_tx( @@ -1349,12 +1355,6 @@ pub trait WalletWrite: WalletRead { /// /// There may be restrictions on heights to which it is possible to truncate. fn truncate_to_height(&mut self, block_height: BlockHeight) -> Result<(), Self::Error>; - - /// Adds a transparent UTXO received by the wallet to the data store. - fn put_received_transparent_utxo( - &mut self, - output: &WalletTransparentOutput, - ) -> Result; } /// This trait describes a capability for manipulating wallet note commitment trees. @@ -1521,6 +1521,18 @@ pub mod testing { type AccountId = u32; type Account = (Self::AccountId, UnifiedFullViewingKey); + fn get_account_ids(&self) -> Result, Self::Error> { + Ok(Vec::new()) + } + + fn get_seed_account( + &self, + _seed: &HdSeedFingerprint, + _account_id: zip32::AccountId, + ) -> Result, Self::Error> { + Ok(None) + } + fn validate_seed( &self, _account_id: Self::AccountId, @@ -1529,10 +1541,49 @@ pub mod testing { Ok(false) } + fn get_account_for_ufvk( + &self, + _ufvk: &UnifiedFullViewingKey, + ) -> Result, Self::Error> { + Ok(None) + } + + fn get_current_address( + &self, + _account: Self::AccountId, + ) -> Result, Self::Error> { + Ok(None) + } + + fn get_account_birthday( + &self, + _account: Self::AccountId, + ) -> Result { + Err(()) + } + + fn get_wallet_birthday(&self) -> Result, Self::Error> { + Ok(None) + } + + fn get_wallet_summary( + &self, + _min_confirmations: u32, + ) -> Result>, Self::Error> { + Ok(None) + } + fn chain_height(&self) -> Result, Self::Error> { Ok(None) } + fn get_block_hash( + &self, + _block_height: BlockHeight, + ) -> Result, Self::Error> { + Ok(None) + } + fn block_metadata( &self, _height: BlockHeight, @@ -1544,6 +1595,10 @@ pub mod testing { Ok(None) } + fn get_max_height_hash(&self) -> Result, Self::Error> { + Ok(None) + } + fn block_max_scanned(&self) -> Result, Self::Error> { Ok(None) } @@ -1563,67 +1618,16 @@ pub mod testing { Ok(None) } - fn get_block_hash( - &self, - _block_height: BlockHeight, - ) -> Result, Self::Error> { - Ok(None) - } - - fn get_max_height_hash(&self) -> Result, Self::Error> { - Ok(None) - } - fn get_tx_height(&self, _txid: TxId) -> Result, Self::Error> { Ok(None) } - fn get_wallet_birthday(&self) -> Result, Self::Error> { - Ok(None) - } - - fn get_account_birthday( - &self, - _account: Self::AccountId, - ) -> Result { - Err(()) - } - - fn get_current_address( - &self, - _account: Self::AccountId, - ) -> Result, Self::Error> { - Ok(None) - } - fn get_unified_full_viewing_keys( &self, ) -> Result, Self::Error> { Ok(HashMap::new()) } - fn get_account_for_ufvk( - &self, - _ufvk: &UnifiedFullViewingKey, - ) -> Result, Self::Error> { - Ok(None) - } - - fn get_seed_account( - &self, - _seed: &HdSeedFingerprint, - _account_id: zip32::AccountId, - ) -> Result, Self::Error> { - Ok(None) - } - - fn get_wallet_summary( - &self, - _min_confirmations: u32, - ) -> Result>, Self::Error> { - Ok(None) - } - fn get_memo(&self, _id_note: NoteId) -> Result, Self::Error> { Ok(None) } @@ -1664,10 +1668,6 @@ pub mod testing { ) -> Result, Self::Error> { Ok(HashMap::new()) } - - fn get_account_ids(&self) -> Result, Self::Error> { - Ok(Vec::new()) - } } impl WalletWrite for MockWalletDb { diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 12c08f054..9e4c83252 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -289,6 +289,18 @@ impl, P: consensus::Parameters> WalletRead for W type AccountId = AccountId; type Account = (AccountId, Option); + fn get_account_ids(&self) -> Result, Self::Error> { + wallet::get_account_ids(self.conn.borrow()) + } + + fn get_seed_account( + &self, + seed: &HdSeedFingerprint, + account_id: zip32::AccountId, + ) -> Result, Self::Error> { + wallet::get_seed_account(self.conn.borrow(), &self.params, seed, account_id) + } + fn validate_seed( &self, account_id: Self::AccountId, @@ -333,12 +345,53 @@ impl, P: consensus::Parameters> WalletRead for W } } + fn get_account_for_ufvk( + &self, + ufvk: &UnifiedFullViewingKey, + ) -> Result, Self::Error> { + wallet::get_account_for_ufvk(self.conn.borrow(), &self.params, ufvk) + } + + fn get_current_address( + &self, + account: AccountId, + ) -> Result, Self::Error> { + wallet::get_current_address(self.conn.borrow(), &self.params, account) + .map(|res| res.map(|(addr, _)| addr)) + } + + fn get_account_birthday(&self, account: AccountId) -> Result { + wallet::account_birthday(self.conn.borrow(), account).map_err(SqliteClientError::from) + } + + fn get_wallet_birthday(&self) -> Result, Self::Error> { + wallet::wallet_birthday(self.conn.borrow()).map_err(SqliteClientError::from) + } + + fn get_wallet_summary( + &self, + min_confirmations: u32, + ) -> Result>, Self::Error> { + // This will return a runtime error if we call `get_wallet_summary` from two + // threads at the same time, as transactions cannot nest. + wallet::get_wallet_summary( + &self.conn.borrow().unchecked_transaction()?, + &self.params, + min_confirmations, + &SubtreeScanProgress, + ) + } + fn chain_height(&self) -> Result, Self::Error> { wallet::scan_queue_extrema(self.conn.borrow()) .map(|h| h.map(|range| *range.end())) .map_err(SqliteClientError::from) } + fn get_block_hash(&self, block_height: BlockHeight) -> Result, Self::Error> { + wallet::get_block_hash(self.conn.borrow(), block_height).map_err(SqliteClientError::from) + } + fn block_metadata(&self, height: BlockHeight) -> Result, Self::Error> { wallet::block_metadata(self.conn.borrow(), &self.params, height) } @@ -347,6 +400,10 @@ impl, P: consensus::Parameters> WalletRead for W wallet::block_fully_scanned(self.conn.borrow(), &self.params) } + fn get_max_height_hash(&self) -> Result, Self::Error> { + wallet::get_max_height_hash(self.conn.borrow()).map_err(SqliteClientError::from) + } + fn block_max_scanned(&self) -> Result, Self::Error> { wallet::block_max_scanned(self.conn.borrow(), &self.params) } @@ -368,69 +425,16 @@ impl, P: consensus::Parameters> WalletRead for W wallet::get_min_unspent_height(self.conn.borrow()).map_err(SqliteClientError::from) } - fn get_block_hash(&self, block_height: BlockHeight) -> Result, Self::Error> { - wallet::get_block_hash(self.conn.borrow(), block_height).map_err(SqliteClientError::from) - } - - fn get_max_height_hash(&self) -> Result, Self::Error> { - wallet::get_max_height_hash(self.conn.borrow()).map_err(SqliteClientError::from) - } - fn get_tx_height(&self, txid: TxId) -> Result, Self::Error> { wallet::get_tx_height(self.conn.borrow(), txid).map_err(SqliteClientError::from) } - fn get_wallet_birthday(&self) -> Result, Self::Error> { - wallet::wallet_birthday(self.conn.borrow()).map_err(SqliteClientError::from) - } - - fn get_account_birthday(&self, account: AccountId) -> Result { - wallet::account_birthday(self.conn.borrow(), account).map_err(SqliteClientError::from) - } - - fn get_current_address( - &self, - account: AccountId, - ) -> Result, Self::Error> { - wallet::get_current_address(self.conn.borrow(), &self.params, account) - .map(|res| res.map(|(addr, _)| addr)) - } - fn get_unified_full_viewing_keys( &self, ) -> Result, Self::Error> { wallet::get_unified_full_viewing_keys(self.conn.borrow(), &self.params) } - fn get_account_for_ufvk( - &self, - ufvk: &UnifiedFullViewingKey, - ) -> Result, Self::Error> { - wallet::get_account_for_ufvk(self.conn.borrow(), &self.params, ufvk) - } - - fn get_seed_account( - &self, - seed: &HdSeedFingerprint, - account_id: zip32::AccountId, - ) -> Result, Self::Error> { - wallet::get_seed_account(self.conn.borrow(), &self.params, seed, account_id) - } - - fn get_wallet_summary( - &self, - min_confirmations: u32, - ) -> Result>, Self::Error> { - // This will return a runtime error if we call `get_wallet_summary` from two - // threads at the same time, as transactions cannot nest. - wallet::get_wallet_summary( - &self.conn.borrow().unchecked_transaction()?, - &self.params, - min_confirmations, - &SubtreeScanProgress, - ) - } - fn get_memo(&self, note_id: NoteId) -> Result, Self::Error> { let sent_memo = wallet::get_sent_memo(self.conn.borrow(), note_id)?; if sent_memo.is_some() { @@ -475,10 +479,6 @@ impl, P: consensus::Parameters> WalletRead for W ) -> Result, Self::Error> { wallet::get_transparent_balances(self.conn.borrow(), &self.params, account, max_height) } - - fn get_account_ids(&self) -> Result, Self::Error> { - wallet::get_account_ids(self.conn.borrow()) - } } impl WalletWrite for WalletDb { @@ -544,6 +544,13 @@ impl WalletWrite for WalletDb ) } + fn update_chain_tip(&mut self, tip_height: BlockHeight) -> Result<(), Self::Error> { + let tx = self.conn.transaction()?; + wallet::scanning::update_chain_tip(&tx, &self.params, tip_height)?; + tx.commit()?; + Ok(()) + } + #[tracing::instrument(skip_all, fields(height = blocks.first().map(|b| u32::from(b.height()))))] #[allow(clippy::type_complexity)] fn put_blocks( @@ -900,11 +907,17 @@ impl WalletWrite for WalletDb }) } - fn update_chain_tip(&mut self, tip_height: BlockHeight) -> Result<(), Self::Error> { - let tx = self.conn.transaction()?; - wallet::scanning::update_chain_tip(&tx, &self.params, tip_height)?; - tx.commit()?; - Ok(()) + fn put_received_transparent_utxo( + &mut self, + _output: &WalletTransparentOutput, + ) -> Result { + #[cfg(feature = "transparent-inputs")] + return wallet::put_received_transparent_utxo(&self.conn, &self.params, _output); + + #[cfg(not(feature = "transparent-inputs"))] + panic!( + "The wallet must be compiled with the transparent-inputs feature to use this method." + ); } fn store_decrypted_tx( @@ -1162,19 +1175,6 @@ impl WalletWrite for WalletDb wallet::truncate_to_height(wdb.conn.0, &wdb.params, block_height) }) } - - fn put_received_transparent_utxo( - &mut self, - _output: &WalletTransparentOutput, - ) -> Result { - #[cfg(feature = "transparent-inputs")] - return wallet::put_received_transparent_utxo(&self.conn, &self.params, _output); - - #[cfg(not(feature = "transparent-inputs"))] - panic!( - "The wallet must be compiled with the transparent-inputs feature to use this method." - ); - } } impl WalletCommitmentTrees for WalletDb { From bc6aa955ffa69343f6e63ed6e0aae58041331dab Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 13 Mar 2024 18:33:28 +0000 Subject: [PATCH 3/8] zcash_client_sqlite: Refactor `wallet::Account` to be a struct --- zcash_client_sqlite/src/lib.rs | 33 +- zcash_client_sqlite/src/wallet.rs | 343 ++++++++---------- zcash_client_sqlite/src/wallet/init.rs | 7 +- .../init/migrations/full_account_ids.rs | 16 +- 4 files changed, 195 insertions(+), 204 deletions(-) diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 9e4c83252..1b91739b5 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -100,7 +100,7 @@ pub mod error; pub mod wallet; use wallet::{ commitment_tree::{self, put_shard_roots}, - Account, HdSeedAccount, SubtreeScanProgress, + AccountType, SubtreeScanProgress, }; #[cfg(test)] @@ -307,16 +307,19 @@ impl, P: consensus::Parameters> WalletRead for W seed: &SecretVec, ) -> Result { if let Some(account) = wallet::get_account(self, account_id)? { - if let Account::Zip32(hdaccount) = account { - let seed_fingerprint_match = - HdSeedFingerprint::from_seed(seed) == *hdaccount.hd_seed_fingerprint(); + if let AccountType::Derived { + seed_fingerprint, + account_index, + } = account.kind + { + let seed_fingerprint_match = HdSeedFingerprint::from_seed(seed) == seed_fingerprint; let usk = UnifiedSpendingKey::from_seed( &self.params, &seed.expose_secret()[..], - hdaccount.account_index(), + account_index, ) - .map_err(|_| SqliteClientError::KeyDerivationError(hdaccount.account_index()))?; + .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?; // Keys are not comparable with `Eq`, but addresses are, so we derive what should // be equivalent addresses for each key and use those to check for key equality. @@ -326,7 +329,7 @@ impl, P: consensus::Parameters> WalletRead for W Ok(usk .to_unified_full_viewing_key() .default_address(ua_request)? - == hdaccount.ufvk().default_address(ua_request)?) + == account.default_address(ua_request)?) }, )?; @@ -490,8 +493,8 @@ impl WalletWrite for WalletDb birthday: AccountBirthday, ) -> Result<(AccountId, UnifiedSpendingKey), Self::Error> { self.transactionally(|wdb| { - let seed_id = HdSeedFingerprint::from_seed(seed); - let account_index = wallet::max_zip32_account_index(wdb.conn.0, &seed_id)? + let seed_fingerprint = HdSeedFingerprint::from_seed(seed); + 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); @@ -501,8 +504,16 @@ impl WalletWrite for WalletDb .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?; let ufvk = usk.to_unified_full_viewing_key(); - let account = Account::Zip32(HdSeedAccount::new(seed_id, account_index, ufvk)); - let account_id = wallet::add_account(wdb.conn.0, &wdb.params, account, birthday)?; + let account_id = wallet::add_account( + wdb.conn.0, + &wdb.params, + AccountType::Derived { + seed_fingerprint, + account_index, + }, + wallet::ViewingKey::Full(Box::new(ufvk)), + birthday, + )?; Ok((account_id, usk)) }) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index e1fe9736c..3ab5e5f2d 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -140,82 +140,71 @@ pub(crate) const BLOCK_SAPLING_FRONTIER_ABSENT: &[u8] = &[0x0]; /// This tracks the allowed values of the `account_type` column of the `accounts` table /// and should not be made public. -enum AccountType { - Zip32, +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub(crate) enum AccountType { + /// An account derived from a known seed. + Derived { + seed_fingerprint: HdSeedFingerprint, + account_index: zip32::AccountId, + }, + + /// An account imported from a viewing key. Imported, } -impl TryFrom for AccountType { - type Error = (); - - fn try_from(value: u32) -> Result { - match value { - 0 => Ok(AccountType::Zip32), - 1 => Ok(AccountType::Imported), - _ => Err(()), - } +fn parse_account_kind( + account_type: u32, + hd_seed_fingerprint: Option<[u8; 32]>, + hd_account_index: Option, +) -> Result { + match (account_type, hd_seed_fingerprint, hd_account_index) { + (0, Some(seed_fp), Some(account_index)) => Ok(AccountType::Derived { + seed_fingerprint: HdSeedFingerprint::from_bytes(seed_fp), + account_index: zip32::AccountId::try_from(account_index).map_err(|_| { + SqliteClientError::CorruptedData( + "ZIP-32 account ID from wallet DB is out of range.".to_string(), + ) + })?, + }), + (1, None, None) => Ok(AccountType::Imported), + (0, None, None) | (1, Some(_), Some(_)) => Err(SqliteClientError::CorruptedData( + "Wallet DB account_type constraint violated".to_string(), + )), + (_, _, _) => Err(SqliteClientError::CorruptedData( + "Unrecognized account_type".to_string(), + )), } } -impl From for u32 { - fn from(value: AccountType) -> Self { - match value { - AccountType::Zip32 => 0, - AccountType::Imported => 1, - } +fn account_kind_code(value: AccountType) -> u32 { + match value { + AccountType::Derived { .. } => 0, + AccountType::Imported => 1, } } -/// Describes the key inputs and UFVK for an account that was derived from a ZIP-32 HD seed and account index. +/// The viewing key that an [`Account`] has available to it. #[derive(Debug, Clone)] -pub(crate) struct HdSeedAccount( - HdSeedFingerprint, - zip32::AccountId, - Box, -); - -impl HdSeedAccount { - pub fn new( - hd_seed_fingerprint: HdSeedFingerprint, - account_index: zip32::AccountId, - ufvk: UnifiedFullViewingKey, - ) -> Self { - Self(hd_seed_fingerprint, account_index, Box::new(ufvk)) - } - - /// Returns the HD seed fingerprint for this account. - pub fn hd_seed_fingerprint(&self) -> &HdSeedFingerprint { - &self.0 - } - - /// Returns the ZIP-32 account index for this account. - pub fn account_index(&self) -> zip32::AccountId { - self.1 - } - - /// Returns the Unified Full Viewing Key for this account. - pub fn ufvk(&self) -> &UnifiedFullViewingKey { - &self.2 - } -} - -/// Represents an arbitrary account for which the seed and ZIP-32 account ID are not known -/// and may not have been involved in creating this account. -#[derive(Debug, Clone)] -pub(crate) enum ImportedAccount { - /// An account that was imported via its full viewing key. +pub(crate) enum ViewingKey { + /// A full viewing key. + /// + /// This is available to derived accounts, as well as accounts directly imported as + /// full viewing keys. Full(Box), - /// An account that was imported via its incoming viewing key. + + /// An incoming viewing key. + /// + /// Accounts that have this kind of viewing key cannot be used in wallet contexts, + /// because they are unable to maintain an accurate balance. Incoming(Uivk), } -/// Describes an account in terms of its UVK or ZIP-32 origins. +/// An account stored in a `zcash_client_sqlite` database. #[derive(Debug, Clone)] -pub(crate) enum Account { - /// Inputs for a ZIP-32 HD account. - Zip32(HdSeedAccount), - /// Inputs for an imported account. - Imported(ImportedAccount), +pub(crate) struct Account { + account_id: AccountId, + pub(crate) kind: AccountType, + viewing_key: ViewingKey, } impl Account { @@ -228,10 +217,35 @@ impl Account { &self, request: UnifiedAddressRequest, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { + match &self.viewing_key { + ViewingKey::Full(ufvk) => ufvk.default_address(request), + ViewingKey::Incoming(_uivk) => todo!(), + } + } +} + +impl zcash_client_backend::data_api::Account for Account { + fn id(&self) -> AccountId { + self.account_id + } + + fn ufvk(&self) -> Option<&UnifiedFullViewingKey> { + self.viewing_key.ufvk() + } +} + +impl ViewingKey { + fn ufvk(&self) -> Option<&UnifiedFullViewingKey> { match self { - Account::Zip32(HdSeedAccount(_, _, ufvk)) => ufvk.default_address(request), - Account::Imported(ImportedAccount::Full(ufvk)) => ufvk.default_address(request), - Account::Imported(ImportedAccount::Incoming(_uivk)) => todo!(), + ViewingKey::Full(ufvk) => Some(ufvk), + ViewingKey::Incoming(_) => None, + } + } + + fn uivk_str(&self, params: &impl Parameters) -> Result { + match self { + ViewingKey::Full(ufvk) => ufvk_to_uivk(ufvk, params), + ViewingKey::Incoming(uivk) => Ok(uivk.encode(¶ms.network_type())), } } } @@ -291,44 +305,6 @@ pub(crate) fn max_zip32_account_index( ) } -struct AccountSqlValues<'a> { - account_type: u32, - hd_seed_fingerprint: Option<&'a [u8]>, - hd_account_index: Option, - ufvk: Option<&'a UnifiedFullViewingKey>, - uivk: String, -} - -/// Returns (account_type, hd_seed_fingerprint, hd_account_index, ufvk, uivk) for a given account. -fn get_sql_values_for_account_parameters<'a, P: consensus::Parameters>( - account: &'a Account, - params: &P, -) -> Result, SqliteClientError> { - Ok(match account { - Account::Zip32(hdaccount) => AccountSqlValues { - account_type: AccountType::Zip32.into(), - hd_seed_fingerprint: Some(hdaccount.hd_seed_fingerprint().as_bytes()), - hd_account_index: Some(hdaccount.account_index().into()), - ufvk: Some(hdaccount.ufvk()), - uivk: ufvk_to_uivk(hdaccount.ufvk(), params)?, - }, - Account::Imported(ImportedAccount::Full(ufvk)) => AccountSqlValues { - account_type: AccountType::Imported.into(), - hd_seed_fingerprint: None, - hd_account_index: None, - ufvk: Some(ufvk), - uivk: ufvk_to_uivk(ufvk, params)?, - }, - Account::Imported(ImportedAccount::Incoming(uivk)) => AccountSqlValues { - account_type: AccountType::Imported.into(), - hd_seed_fingerprint: None, - hd_account_index: None, - ufvk: None, - uivk: uivk.encode(¶ms.network_type()), - }, - }) -} - pub(crate) fn ufvk_to_uivk( ufvk: &UnifiedFullViewingKey, params: &P, @@ -357,20 +333,27 @@ pub(crate) fn ufvk_to_uivk( pub(crate) fn add_account( conn: &rusqlite::Transaction, params: &P, - account: Account, + kind: AccountType, + viewing_key: ViewingKey, birthday: AccountBirthday, ) -> Result { - let args = get_sql_values_for_account_parameters(&account, params)?; + let (hd_seed_fingerprint, hd_account_index) = match kind { + AccountType::Derived { + seed_fingerprint, + account_index, + } => (Some(seed_fingerprint), Some(account_index)), + AccountType::Imported => (None, None), + }; - let orchard_item = args - .ufvk + let orchard_item = viewing_key + .ufvk() .and_then(|ufvk| ufvk.orchard().map(|k| k.to_bytes())); - let sapling_item = args - .ufvk + let sapling_item = viewing_key + .ufvk() .and_then(|ufvk| ufvk.sapling().map(|k| k.to_bytes())); #[cfg(feature = "transparent-inputs")] - let transparent_item = args - .ufvk + let transparent_item = viewing_key + .ufvk() .and_then(|ufvk| ufvk.transparent().map(|k| k.serialize())); #[cfg(not(feature = "transparent-inputs"))] let transparent_item: Option> = None; @@ -392,11 +375,11 @@ pub(crate) fn add_account( RETURNING id; "#, named_params![ - ":account_type": args.account_type, - ":hd_seed_fingerprint": args.hd_seed_fingerprint, - ":hd_account_index": args.hd_account_index, - ":ufvk": args.ufvk.map(|ufvk| ufvk.encode(params)), - ":uivk": args.uivk, + ":account_type": account_kind_code(kind), + ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.as_bytes()), + ":hd_account_index": hd_account_index.map(u32::from), + ":ufvk": viewing_key.ufvk().map(|ufvk| ufvk.encode(params)), + ":uivk": viewing_key.uivk_str(params)?, ":orchard_fvk_item_cache": orchard_item, ":sapling_fvk_item_cache": sapling_item, ":p2pkh_fvk_item_cache": transparent_item, @@ -406,6 +389,12 @@ pub(crate) fn add_account( |row| Ok(AccountId(row.get(0)?)), )?; + let account = Account { + account_id, + kind, + viewing_key, + }; + // If a birthday frontier is available, insert it into the note commitment tree. If the // birthday frontier is the empty frontier, we don't need to do anything. if let Some(frontier) = birthday.sapling_frontier().value() { @@ -754,18 +743,18 @@ pub(crate) fn get_account_for_ufvk( ], |row| { let account_id = row.get::<_, u32>(0).map(AccountId)?; - Ok(( - account_id, - row.get::<_, Option>(1)? - .map(|ufvk_str| UnifiedFullViewingKey::decode(params, &ufvk_str)) - .transpose() - .map_err(|e| { - SqliteClientError::CorruptedData(format!( - "Could not decode unified full viewing key for account {:?}: {}", - account_id, e - )) - })?, - )) + + // We looked up the account by FVK components, so the UFVK column must be + // non-null. + let ufvk_str: String = row.get(1)?; + let ufvk = UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { + SqliteClientError::CorruptedData(format!( + "Could not decode unified full viewing key for account {:?}: {}", + account_id, e + )) + })?; + + Ok((account_id, Some(ufvk))) }, )? .collect::, _>>()?; @@ -785,7 +774,7 @@ pub(crate) fn get_seed_account( conn: &rusqlite::Connection, params: &P, seed: &HdSeedFingerprint, - account_id: zip32::AccountId, + account_index: zip32::AccountId, ) -> Result)>, SqliteClientError> { let mut stmt = conn.prepare( "SELECT id, ufvk @@ -797,22 +786,23 @@ pub(crate) fn get_seed_account( let mut accounts = stmt.query_and_then::<_, SqliteClientError, _, _>( named_params![ ":hd_seed_fingerprint": seed.as_bytes(), - ":hd_account_index": u32::from(account_id), + ":hd_account_index": u32::from(account_index), ], |row| { let account_id = row.get::<_, u32>(0).map(AccountId)?; - Ok(( - account_id, - row.get::<_, Option>(1)? - .map(|ufvk_str| UnifiedFullViewingKey::decode(params, &ufvk_str)) - .transpose() - .map_err(|e| { - SqliteClientError::CorruptedData(format!( - "Could not decode unified full viewing key for account {:?}: {}", - account_id, e - )) - })?, - )) + let ufvk = match row.get::<_, Option>(1)? { + None => Err(SqliteClientError::CorruptedData(format!( + "Missing unified full viewing key for derived account {:?}", + account_id, + ))), + Some(ufvk_str) => UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { + SqliteClientError::CorruptedData(format!( + "Could not decode unified full viewing key for account {:?}: {}", + account_id, e + )) + }), + }?; + Ok((account_id, Some(ufvk))) }, )?; @@ -1506,7 +1496,7 @@ pub(crate) fn get_account, P: Parameters>( ) -> Result, SqliteClientError> { let mut sql = db.conn.borrow().prepare_cached( r#" - SELECT account_type, ufvk, uivk, hd_seed_fingerprint, hd_account_index + SELECT account_type, hd_seed_fingerprint, hd_account_index, ufvk, uivk FROM accounts WHERE id = :account_id "#, @@ -1516,51 +1506,36 @@ pub(crate) fn get_account, P: Parameters>( let row = result.next()?; match row { Some(row) => { - let account_type: AccountType = - row.get::<_, u32>("account_type")?.try_into().map_err(|_| { - SqliteClientError::CorruptedData("Unrecognized account_type".to_string()) - })?; + let kind = parse_account_kind( + row.get("account_type")?, + row.get("hd_seed_fingerprint")?, + row.get("hd_account_index")?, + )?; + let ufvk_str: Option = row.get("ufvk")?; - let ufvk = if let Some(ufvk_str) = ufvk_str { - Some( + let viewing_key = if let Some(ufvk_str) = ufvk_str { + ViewingKey::Full(Box::new( UnifiedFullViewingKey::decode(&db.params, &ufvk_str[..]) .map_err(SqliteClientError::BadAccountData)?, - ) + )) } else { - None + let uivk_str: String = row.get("uivk")?; + let (network, uivk) = Uivk::decode(&uivk_str).map_err(|e| { + SqliteClientError::CorruptedData(format!("Failure to decode UIVK: {e}")) + })?; + if network != db.params.network_type() { + return Err(SqliteClientError::CorruptedData( + "UIVK network type does not match wallet network type".to_string(), + )); + } + ViewingKey::Incoming(uivk) }; - let uivk_str: String = row.get("uivk")?; - let (network, uivk) = Uivk::decode(&uivk_str).map_err(|e| { - SqliteClientError::CorruptedData(format!("Failure to decode UIVK: {e}")) - })?; - if network != db.params.network_type() { - return Err(SqliteClientError::CorruptedData( - "UIVK network type does not match wallet network type".to_string(), - )); - } - match account_type { - AccountType::Zip32 => Ok(Some(Account::Zip32(HdSeedAccount::new( - HdSeedFingerprint::from_bytes(row.get("hd_seed_fingerprint")?), - zip32::AccountId::try_from(row.get::<_, u32>("hd_account_index")?).map_err( - |_| { - SqliteClientError::CorruptedData( - "ZIP-32 account ID from db is out of range.".to_string(), - ) - }, - )?, - ufvk.ok_or_else(|| { - SqliteClientError::CorruptedData( - "ZIP-32 account is missing a full viewing key".to_string(), - ) - })?, - )))), - AccountType::Imported => Ok(Some(Account::Imported(if let Some(ufvk) = ufvk { - ImportedAccount::Full(Box::new(ufvk)) - } else { - ImportedAccount::Incoming(uivk) - }))), - } + Ok(Some(Account { + account_id, + kind, + viewing_key, + })) } None => Ok(None), } @@ -2725,7 +2700,7 @@ mod tests { use crate::{ testing::{AddressType, BlockCache, TestBuilder, TestState}, - wallet::{get_account, Account}, + wallet::{get_account, AccountType}, AccountId, }; @@ -2877,8 +2852,8 @@ mod tests { let expected_account_index = zip32::AccountId::try_from(0).unwrap(); assert_matches!( - account_parameters, - Account::Zip32(hdaccount) if hdaccount.account_index() == expected_account_index + account_parameters.kind, + AccountType::Derived{account_index, ..} if account_index == expected_account_index ); } diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 496cb4e0f..4881d2f24 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -1284,7 +1284,7 @@ mod tests { fn account_produces_expected_ua_sequence() { use zcash_client_backend::data_api::AccountBirthday; - use crate::wallet::{get_account, Account}; + use crate::wallet::{get_account, AccountType}; let network = Network::MainNetwork; let data_file = NamedTempFile::new().unwrap(); @@ -1301,7 +1301,10 @@ mod tests { .unwrap(); assert_matches!( get_account(&db_data, account_id), - Ok(Some(Account::Zip32(hdaccount))) if hdaccount.account_index() == zip32::AccountId::ZERO + Ok(Some(account)) if matches!( + account.kind, + AccountType::Derived{account_index, ..} if account_index == zip32::AccountId::ZERO, + ) ); for tv in &test_vectors::UNIFIED[..3] { diff --git a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs index d6391c78e..a8f663544 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs @@ -1,6 +1,6 @@ use std::{collections::HashSet, rc::Rc}; -use crate::wallet::{init::WalletMigrationError, ufvk_to_uivk, AccountType}; +use crate::wallet::{account_kind_code, init::WalletMigrationError, ufvk_to_uivk, AccountType}; use rusqlite::{named_params, Transaction}; use schemer_rusqlite::RusqliteMigration; use secrecy::{ExposeSecret, SecretVec}; @@ -44,8 +44,11 @@ impl RusqliteMigration for Migration

{ type Error = WalletMigrationError; fn up(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { - let account_type_zip32 = u32::from(AccountType::Zip32); - let account_type_imported = u32::from(AccountType::Imported); + let account_type_derived = account_kind_code(AccountType::Derived { + seed_fingerprint: HdSeedFingerprint::from_bytes([0; 32]), + account_index: zip32::AccountId::ZERO, + }); + let account_type_imported = account_kind_code(AccountType::Imported); transaction.execute_batch( &format!(r#" PRAGMA foreign_keys = OFF; @@ -53,7 +56,7 @@ impl RusqliteMigration for Migration

{ CREATE TABLE accounts_new ( id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, - account_type INTEGER NOT NULL DEFAULT {account_type_zip32}, + account_type INTEGER NOT NULL DEFAULT {account_type_derived}, hd_seed_fingerprint BLOB, hd_account_index INTEGER, ufvk TEXT, @@ -64,7 +67,7 @@ impl RusqliteMigration for Migration

{ birthday_height INTEGER NOT NULL, recover_until_height INTEGER, CHECK ( - (account_type = {account_type_zip32} AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL AND ufvk IS NOT NULL) + (account_type = {account_type_derived} AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL AND ufvk IS NOT NULL) OR (account_type = {account_type_imported} AND hd_seed_fingerprint IS NULL AND hd_account_index IS NULL) ) @@ -85,7 +88,6 @@ impl RusqliteMigration for Migration

{ let mut rows = q.query([])?; while let Some(row) = rows.next()? { let account_index: u32 = row.get("account")?; - let account_type = u32::from(AccountType::Zip32); let birthday_height: u32 = row.get("birthday_height")?; let recover_until_height: Option = row.get("recover_until_height")?; @@ -142,7 +144,7 @@ impl RusqliteMigration for Migration

{ "#, named_params![ ":account_id": account_id, - ":account_type": account_type, + ":account_type": account_type_derived, ":seed_id": seed_id.as_bytes(), ":account_index": account_index, ":ufvk": ufvk, From 65093487c349f9fea1eb68fa744f0b733ae2f2c7 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 13 Mar 2024 19:04:45 +0000 Subject: [PATCH 4/8] zcash_client_backend: Expose the kind of an `Account` --- zcash_client_backend/CHANGELOG.md | 1 + zcash_client_backend/src/data_api.rs | 28 +++++++++++ zcash_client_sqlite/src/lib.rs | 14 +++--- zcash_client_sqlite/src/wallet.rs | 46 ++++++++----------- zcash_client_sqlite/src/wallet/init.rs | 6 +-- .../init/migrations/full_account_ids.rs | 8 ++-- 6 files changed, 61 insertions(+), 42 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 15c08646b..a5ca64fcf 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -16,6 +16,7 @@ and this library adheres to Rust's notion of - `Account` - `AccountBalance::with_orchard_balance_mut` - `AccountBirthday::orchard_frontier` + - `AccountKind` - `BlockMetadata::orchard_tree_size` - `DecryptedTransaction::{new, tx(), orchard_outputs()}` - `ScannedBlock::orchard` diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index a1b5493ad..80b1bef83 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -315,12 +315,32 @@ impl AccountBalance { } } +/// The kinds of accounts supported by `zcash_client_backend`. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub enum AccountKind { + /// An account derived from a known seed. + Derived { + seed_fingerprint: HdSeedFingerprint, + account_index: zip32::AccountId, + }, + + /// An account imported from a viewing key. + Imported, +} + /// A set of capabilities that a client account must provide. pub trait Account { /// Returns the unique identifier for the account. fn id(&self) -> AccountId; + /// Returns whether this account is derived or imported, and the derivation parameters + /// if applicable. + fn kind(&self) -> AccountKind; + /// Returns the UFVK that the wallet backend has stored for the account, if any. + /// + /// Accounts for which this returns `None` cannot be used in wallet contexts, because + /// they are unable to maintain an accurate balance. fn ufvk(&self) -> Option<&UnifiedFullViewingKey>; } @@ -329,6 +349,10 @@ impl Account for (A, UnifiedFullViewingKey) { self.0 } + fn kind(&self) -> AccountKind { + AccountKind::Imported + } + fn ufvk(&self) -> Option<&UnifiedFullViewingKey> { Some(&self.1) } @@ -339,6 +363,10 @@ impl Account for (A, Option) { self.0 } + fn kind(&self) -> AccountKind { + AccountKind::Imported + } + fn ufvk(&self) -> Option<&UnifiedFullViewingKey> { self.1.as_ref() } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 1b91739b5..e78f72d27 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -60,9 +60,9 @@ use zcash_client_backend::{ self, chain::{BlockSource, ChainState, CommitmentTreeRoot}, scanning::{ScanPriority, ScanRange}, - AccountBirthday, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, - ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary, - WalletWrite, SAPLING_SHARD_HEIGHT, + Account, AccountBirthday, AccountKind, BlockMetadata, DecryptedTransaction, InputSource, + NullifierQuery, ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, + WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }, keys::{ AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey, @@ -100,7 +100,7 @@ pub mod error; pub mod wallet; use wallet::{ commitment_tree::{self, put_shard_roots}, - AccountType, SubtreeScanProgress, + SubtreeScanProgress, }; #[cfg(test)] @@ -307,10 +307,10 @@ impl, P: consensus::Parameters> WalletRead for W seed: &SecretVec, ) -> Result { if let Some(account) = wallet::get_account(self, account_id)? { - if let AccountType::Derived { + if let AccountKind::Derived { seed_fingerprint, account_index, - } = account.kind + } = account.kind() { let seed_fingerprint_match = HdSeedFingerprint::from_seed(seed) == seed_fingerprint; @@ -507,7 +507,7 @@ impl WalletWrite for WalletDb let account_id = wallet::add_account( wdb.conn.0, &wdb.params, - AccountType::Derived { + AccountKind::Derived { seed_fingerprint, account_index, }, diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 3ab5e5f2d..2ab96454d 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -81,7 +81,7 @@ use zcash_client_backend::{ address::{Address, UnifiedAddress}, data_api::{ scanning::{ScanPriority, ScanRange}, - AccountBalance, AccountBirthday, BlockMetadata, Ratio, SentTransactionOutput, + AccountBalance, AccountBirthday, AccountKind, BlockMetadata, Ratio, SentTransactionOutput, WalletSummary, SAPLING_SHARD_HEIGHT, }, encoding::AddressCodec, @@ -138,27 +138,13 @@ pub(crate) mod scanning; pub(crate) const BLOCK_SAPLING_FRONTIER_ABSENT: &[u8] = &[0x0]; -/// This tracks the allowed values of the `account_type` column of the `accounts` table -/// and should not be made public. -#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] -pub(crate) enum AccountType { - /// An account derived from a known seed. - Derived { - seed_fingerprint: HdSeedFingerprint, - account_index: zip32::AccountId, - }, - - /// An account imported from a viewing key. - Imported, -} - fn parse_account_kind( account_type: u32, hd_seed_fingerprint: Option<[u8; 32]>, hd_account_index: Option, -) -> Result { +) -> Result { match (account_type, hd_seed_fingerprint, hd_account_index) { - (0, Some(seed_fp), Some(account_index)) => Ok(AccountType::Derived { + (0, Some(seed_fp), Some(account_index)) => Ok(AccountKind::Derived { seed_fingerprint: HdSeedFingerprint::from_bytes(seed_fp), account_index: zip32::AccountId::try_from(account_index).map_err(|_| { SqliteClientError::CorruptedData( @@ -166,7 +152,7 @@ fn parse_account_kind( ) })?, }), - (1, None, None) => Ok(AccountType::Imported), + (1, None, None) => Ok(AccountKind::Imported), (0, None, None) | (1, Some(_), Some(_)) => Err(SqliteClientError::CorruptedData( "Wallet DB account_type constraint violated".to_string(), )), @@ -176,10 +162,10 @@ fn parse_account_kind( } } -fn account_kind_code(value: AccountType) -> u32 { +fn account_kind_code(value: AccountKind) -> u32 { match value { - AccountType::Derived { .. } => 0, - AccountType::Imported => 1, + AccountKind::Derived { .. } => 0, + AccountKind::Imported => 1, } } @@ -203,7 +189,7 @@ pub(crate) enum ViewingKey { #[derive(Debug, Clone)] pub(crate) struct Account { account_id: AccountId, - pub(crate) kind: AccountType, + kind: AccountKind, viewing_key: ViewingKey, } @@ -229,6 +215,10 @@ impl zcash_client_backend::data_api::Account for Account { self.account_id } + fn kind(&self) -> AccountKind { + self.kind + } + fn ufvk(&self) -> Option<&UnifiedFullViewingKey> { self.viewing_key.ufvk() } @@ -333,16 +323,16 @@ pub(crate) fn ufvk_to_uivk( pub(crate) fn add_account( conn: &rusqlite::Transaction, params: &P, - kind: AccountType, + kind: AccountKind, viewing_key: ViewingKey, birthday: AccountBirthday, ) -> Result { let (hd_seed_fingerprint, hd_account_index) = match kind { - AccountType::Derived { + AccountKind::Derived { seed_fingerprint, account_index, } => (Some(seed_fingerprint), Some(account_index)), - AccountType::Imported => (None, None), + AccountKind::Imported => (None, None), }; let orchard_item = viewing_key @@ -2695,12 +2685,12 @@ mod tests { use std::num::NonZeroU32; use sapling::zip32::ExtendedSpendingKey; - use zcash_client_backend::data_api::{AccountBirthday, WalletRead}; + use zcash_client_backend::data_api::{AccountBirthday, AccountKind, WalletRead}; use zcash_primitives::{block::BlockHash, transaction::components::amount::NonNegativeAmount}; use crate::{ testing::{AddressType, BlockCache, TestBuilder, TestState}, - wallet::{get_account, AccountType}, + wallet::get_account, AccountId, }; @@ -2853,7 +2843,7 @@ mod tests { let expected_account_index = zip32::AccountId::try_from(0).unwrap(); assert_matches!( account_parameters.kind, - AccountType::Derived{account_index, ..} if account_index == expected_account_index + AccountKind::Derived{account_index, ..} if account_index == expected_account_index ); } diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 4881d2f24..3f93447c8 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -1282,9 +1282,9 @@ mod tests { #[test] #[cfg(feature = "transparent-inputs")] fn account_produces_expected_ua_sequence() { - use zcash_client_backend::data_api::AccountBirthday; + use zcash_client_backend::data_api::{AccountBirthday, AccountKind}; - use crate::wallet::{get_account, AccountType}; + use crate::wallet::get_account; let network = Network::MainNetwork; let data_file = NamedTempFile::new().unwrap(); @@ -1303,7 +1303,7 @@ mod tests { get_account(&db_data, account_id), Ok(Some(account)) if matches!( account.kind, - AccountType::Derived{account_index, ..} if account_index == zip32::AccountId::ZERO, + AccountKind::Derived{account_index, ..} if account_index == zip32::AccountId::ZERO, ) ); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs index a8f663544..08ca4b9c7 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs @@ -1,11 +1,11 @@ use std::{collections::HashSet, rc::Rc}; -use crate::wallet::{account_kind_code, init::WalletMigrationError, ufvk_to_uivk, AccountType}; +use crate::wallet::{account_kind_code, init::WalletMigrationError, ufvk_to_uivk}; use rusqlite::{named_params, Transaction}; use schemer_rusqlite::RusqliteMigration; use secrecy::{ExposeSecret, SecretVec}; use uuid::Uuid; -use zcash_client_backend::keys::UnifiedSpendingKey; +use zcash_client_backend::{data_api::AccountKind, keys::UnifiedSpendingKey}; use zcash_keys::keys::{HdSeedFingerprint, UnifiedFullViewingKey}; use zcash_primitives::consensus; @@ -44,11 +44,11 @@ impl RusqliteMigration for Migration

{ type Error = WalletMigrationError; fn up(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { - let account_type_derived = account_kind_code(AccountType::Derived { + let account_type_derived = account_kind_code(AccountKind::Derived { seed_fingerprint: HdSeedFingerprint::from_bytes([0; 32]), account_index: zip32::AccountId::ZERO, }); - let account_type_imported = account_kind_code(AccountType::Imported); + let account_type_imported = account_kind_code(AccountKind::Imported); transaction.execute_batch( &format!(r#" PRAGMA foreign_keys = OFF; From 5f3d5e9f4af5abd71414373ea9bb78866368f924 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 13 Mar 2024 20:11:01 +0000 Subject: [PATCH 5/8] zcash_client_sqlite: Use `wallet::Account` for `WalletRead::Account` --- zcash_client_backend/CHANGELOG.md | 6 ++--- zcash_client_sqlite/CHANGELOG.md | 3 ++- zcash_client_sqlite/src/lib.rs | 2 +- zcash_client_sqlite/src/wallet.rs | 42 ++++++++++++++++++++----------- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index a5ca64fcf..e5f7a291a 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -54,10 +54,10 @@ and this library adheres to Rust's notion of - Arguments to `ScannedBlock::from_parts` have changed. - Changes to the `WalletRead` trait: - Added `Account` associated type. - - Added `get_orchard_nullifiers` method. - - `get_account_for_ufvk` now returns an `Self::Account` instead of a bare - `AccountId` + - `get_account_for_ufvk` now returns `Self::Account` instead of a bare + `AccountId`. - Added `get_seed_account` method. + - Added `get_orchard_nullifiers` method. - Changes to the `InputSource` trait: - `select_spendable_notes` now takes its `target_value` argument as a `NonNegativeAmount`. Also, the values of the returned map are also diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 1490aeb22..d4242f89d 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -10,7 +10,8 @@ and this library adheres to Rust's notion of ### Added - A new `orchard` feature flag has been added to make it possible to build client code without `orchard` dependendencies. -- `zcash_client_sqlite::AccountId` +- `zcash_client_sqlite::AccountId` +- `zcash_client_sqlite::wallet::Account` - `impl From for SqliteClientError` ### Changed diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index e78f72d27..e30dbd15c 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -287,7 +287,7 @@ impl, P: consensus::Parameters> InputSource for impl, P: consensus::Parameters> WalletRead for WalletDb { type Error = SqliteClientError; type AccountId = AccountId; - type Account = (AccountId, Option); + type Account = wallet::Account; fn get_account_ids(&self) -> Result, Self::Error> { wallet::get_account_ids(self.conn.borrow()) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 2ab96454d..eb6cebf05 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -187,7 +187,7 @@ pub(crate) enum ViewingKey { /// An account stored in a `zcash_client_sqlite` database. #[derive(Debug, Clone)] -pub(crate) struct Account { +pub struct Account { account_id: AccountId, kind: AccountKind, viewing_key: ViewingKey, @@ -199,7 +199,7 @@ impl Account { /// /// The diversifier index may be non-zero if the Unified Address includes a Sapling /// receiver, and there was no valid Sapling receiver at diversifier index zero. - pub fn default_address( + pub(crate) fn default_address( &self, request: UnifiedAddressRequest, ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { @@ -710,14 +710,14 @@ pub(crate) fn get_account_for_ufvk( conn: &rusqlite::Connection, params: &P, ufvk: &UnifiedFullViewingKey, -) -> Result)>, SqliteClientError> { +) -> Result, SqliteClientError> { #[cfg(feature = "transparent-inputs")] let transparent_item = ufvk.transparent().map(|k| k.serialize()); #[cfg(not(feature = "transparent-inputs"))] let transparent_item: Option> = None; let mut stmt = conn.prepare( - "SELECT id, ufvk + "SELECT id, account_type, hd_seed_fingerprint, hd_account_index, ufvk FROM accounts WHERE orchard_fvk_item_cache = :orchard_fvk_item_cache OR sapling_fvk_item_cache = :sapling_fvk_item_cache @@ -733,18 +733,25 @@ pub(crate) fn get_account_for_ufvk( ], |row| { let account_id = row.get::<_, u32>(0).map(AccountId)?; + let kind = parse_account_kind(row.get(1)?, row.get(2)?, row.get(3)?)?; // We looked up the account by FVK components, so the UFVK column must be // non-null. - let ufvk_str: String = row.get(1)?; - let ufvk = UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { - SqliteClientError::CorruptedData(format!( - "Could not decode unified full viewing key for account {:?}: {}", - account_id, e - )) - })?; + let ufvk_str: String = row.get(4)?; + let viewing_key = ViewingKey::Full(Box::new( + UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { + SqliteClientError::CorruptedData(format!( + "Could not decode unified full viewing key for account {:?}: {}", + account_id, e + )) + })?, + )); - Ok((account_id, Some(ufvk))) + Ok(Account { + account_id, + kind, + viewing_key, + }) }, )? .collect::, _>>()?; @@ -765,7 +772,7 @@ pub(crate) fn get_seed_account( params: &P, seed: &HdSeedFingerprint, account_index: zip32::AccountId, -) -> Result)>, SqliteClientError> { +) -> Result, SqliteClientError> { let mut stmt = conn.prepare( "SELECT id, ufvk FROM accounts @@ -792,7 +799,14 @@ pub(crate) fn get_seed_account( )) }), }?; - Ok((account_id, Some(ufvk))) + Ok(Account { + account_id, + kind: AccountKind::Derived { + seed_fingerprint: *seed, + account_index, + }, + viewing_key: ViewingKey::Full(Box::new(ufvk)), + }) }, )?; From 64aabdc54af153d1370e4ab71569e00423056e01 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 13 Mar 2024 20:15:45 +0000 Subject: [PATCH 6/8] Rename `WalletRead::get_seed_account` to `WalletRead::get_derived_account` --- zcash_client_backend/CHANGELOG.md | 2 +- zcash_client_backend/src/data_api.rs | 4 ++-- zcash_client_sqlite/src/lib.rs | 4 ++-- zcash_client_sqlite/src/wallet.rs | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index e5f7a291a..25c58a929 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -54,9 +54,9 @@ and this library adheres to Rust's notion of - Arguments to `ScannedBlock::from_parts` have changed. - Changes to the `WalletRead` trait: - Added `Account` associated type. + - Added `get_derived_account` method. - `get_account_for_ufvk` now returns `Self::Account` instead of a bare `AccountId`. - - Added `get_seed_account` method. - Added `get_orchard_nullifiers` method. - Changes to the `InputSource` trait: - `select_spendable_notes` now takes its `target_value` argument as a diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 80b1bef83..d20ffe131 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -576,7 +576,7 @@ pub trait WalletRead { /// Returns the account corresponding to a given [`HdSeedFingerprint`] and /// [`zip32::AccountId`], if any. - fn get_seed_account( + fn get_derived_account( &self, seed: &HdSeedFingerprint, account_id: zip32::AccountId, @@ -1553,7 +1553,7 @@ pub mod testing { Ok(Vec::new()) } - fn get_seed_account( + fn get_derived_account( &self, _seed: &HdSeedFingerprint, _account_id: zip32::AccountId, diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index e30dbd15c..2286291b5 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -293,12 +293,12 @@ impl, P: consensus::Parameters> WalletRead for W wallet::get_account_ids(self.conn.borrow()) } - fn get_seed_account( + fn get_derived_account( &self, seed: &HdSeedFingerprint, account_id: zip32::AccountId, ) -> Result, Self::Error> { - wallet::get_seed_account(self.conn.borrow(), &self.params, seed, account_id) + wallet::get_derived_account(self.conn.borrow(), &self.params, seed, account_id) } fn validate_seed( diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index eb6cebf05..fb3962bd7 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -767,7 +767,7 @@ pub(crate) fn get_account_for_ufvk( /// Returns the account id corresponding to a given [`HdSeedFingerprint`] /// and [`zip32::AccountId`], if any. -pub(crate) fn get_seed_account( +pub(crate) fn get_derived_account( conn: &rusqlite::Connection, params: &P, seed: &HdSeedFingerprint, From bbb7f36e553974d4f81de782b16e0c615c48dbf7 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 13 Mar 2024 20:21:33 +0000 Subject: [PATCH 7/8] zcash_client_backend: Add `WalletRead::get_account` --- zcash_client_backend/CHANGELOG.md | 1 + zcash_client_backend/src/data_api.rs | 13 +++++++++++++ zcash_client_sqlite/src/lib.rs | 9 ++++++++- zcash_client_sqlite/src/wallet.rs | 16 ++++++++-------- zcash_client_sqlite/src/wallet/init.rs | 6 ++---- 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 25c58a929..be1ba5e8e 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -54,6 +54,7 @@ and this library adheres to Rust's notion of - Arguments to `ScannedBlock::from_parts` have changed. - Changes to the `WalletRead` trait: - Added `Account` associated type. + - Added `get_account` method. - Added `get_derived_account` method. - `get_account_for_ufvk` now returns `Self::Account` instead of a bare `AccountId`. diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index d20ffe131..1c0ff1745 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -574,6 +574,12 @@ pub trait WalletRead { /// Returns a vector with the IDs of all accounts known to this wallet. fn get_account_ids(&self) -> Result, Self::Error>; + /// Returns the account corresponding to the given ID, if any. + fn get_account( + &self, + account_id: Self::AccountId, + ) -> Result, Self::Error>; + /// Returns the account corresponding to a given [`HdSeedFingerprint`] and /// [`zip32::AccountId`], if any. fn get_derived_account( @@ -1553,6 +1559,13 @@ pub mod testing { Ok(Vec::new()) } + fn get_account( + &self, + _account_id: Self::AccountId, + ) -> Result, Self::Error> { + Ok(None) + } + fn get_derived_account( &self, _seed: &HdSeedFingerprint, diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 2286291b5..f4137d8be 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -293,6 +293,13 @@ impl, P: consensus::Parameters> WalletRead for W wallet::get_account_ids(self.conn.borrow()) } + fn get_account( + &self, + account_id: Self::AccountId, + ) -> Result, Self::Error> { + wallet::get_account(self.conn.borrow(), &self.params, account_id) + } + fn get_derived_account( &self, seed: &HdSeedFingerprint, @@ -306,7 +313,7 @@ impl, P: consensus::Parameters> WalletRead for W account_id: Self::AccountId, seed: &SecretVec, ) -> Result { - if let Some(account) = wallet::get_account(self, account_id)? { + if let Some(account) = self.get_account(account_id)? { if let AccountKind::Derived { seed_fingerprint, account_index, diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index fb3962bd7..dbb8cc841 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -67,7 +67,7 @@ use incrementalmerkletree::Retention; use rusqlite::{self, named_params, params, OptionalExtension}; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; -use std::borrow::Borrow; + use std::collections::HashMap; use std::convert::TryFrom; use std::io::{self, Cursor}; @@ -1494,11 +1494,12 @@ pub(crate) fn block_height_extrema( }) } -pub(crate) fn get_account, P: Parameters>( - db: &WalletDb, +pub(crate) fn get_account( + conn: &rusqlite::Connection, + params: &P, account_id: AccountId, ) -> Result, SqliteClientError> { - let mut sql = db.conn.borrow().prepare_cached( + let mut sql = conn.prepare_cached( r#" SELECT account_type, hd_seed_fingerprint, hd_account_index, ufvk, uivk FROM accounts @@ -1519,7 +1520,7 @@ pub(crate) fn get_account, P: Parameters>( let ufvk_str: Option = row.get("ufvk")?; let viewing_key = if let Some(ufvk_str) = ufvk_str { ViewingKey::Full(Box::new( - UnifiedFullViewingKey::decode(&db.params, &ufvk_str[..]) + UnifiedFullViewingKey::decode(params, &ufvk_str[..]) .map_err(SqliteClientError::BadAccountData)?, )) } else { @@ -1527,7 +1528,7 @@ pub(crate) fn get_account, P: Parameters>( let (network, uivk) = Uivk::decode(&uivk_str).map_err(|e| { SqliteClientError::CorruptedData(format!("Failure to decode UIVK: {e}")) })?; - if network != db.params.network_type() { + if network != params.network_type() { return Err(SqliteClientError::CorruptedData( "UIVK network type does not match wallet network type".to_string(), )); @@ -2704,7 +2705,6 @@ mod tests { use crate::{ testing::{AddressType, BlockCache, TestBuilder, TestState}, - wallet::get_account, AccountId, }; @@ -2852,7 +2852,7 @@ mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); let account_id = st.test_account().unwrap().0; - let account_parameters = get_account(st.wallet(), account_id).unwrap().unwrap(); + let account_parameters = st.wallet().get_account(account_id).unwrap().unwrap(); let expected_account_index = zip32::AccountId::try_from(0).unwrap(); assert_matches!( diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 3f93447c8..e74259b56 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -1282,9 +1282,7 @@ mod tests { #[test] #[cfg(feature = "transparent-inputs")] fn account_produces_expected_ua_sequence() { - use zcash_client_backend::data_api::{AccountBirthday, AccountKind}; - - use crate::wallet::get_account; + use zcash_client_backend::data_api::{AccountBirthday, AccountKind, WalletRead}; let network = Network::MainNetwork; let data_file = NamedTempFile::new().unwrap(); @@ -1300,7 +1298,7 @@ mod tests { .create_account(&Secret::new(seed.to_vec()), birthday) .unwrap(); assert_matches!( - get_account(&db_data, account_id), + db_data.get_account(account_id), Ok(Some(account)) if matches!( account.kind, AccountKind::Derived{account_index, ..} if account_index == zip32::AccountId::ZERO, From b161472cc0a551d7dfacf33517d4c5d7a0a6521b Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 13 Mar 2024 21:06:30 +0000 Subject: [PATCH 8/8] zcash_client_sqlite: Rename `account_type` column to `account_kind` --- zcash_client_sqlite/src/wallet.rs | 20 +++++++++---------- zcash_client_sqlite/src/wallet/init.rs | 4 ++-- .../init/migrations/full_account_ids.rs | 16 +++++++-------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index dbb8cc841..95a7e631f 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -139,11 +139,11 @@ pub(crate) mod scanning; pub(crate) const BLOCK_SAPLING_FRONTIER_ABSENT: &[u8] = &[0x0]; fn parse_account_kind( - account_type: u32, + account_kind: u32, hd_seed_fingerprint: Option<[u8; 32]>, hd_account_index: Option, ) -> Result { - match (account_type, hd_seed_fingerprint, hd_account_index) { + match (account_kind, hd_seed_fingerprint, hd_account_index) { (0, Some(seed_fp), Some(account_index)) => Ok(AccountKind::Derived { seed_fingerprint: HdSeedFingerprint::from_bytes(seed_fp), account_index: zip32::AccountId::try_from(account_index).map_err(|_| { @@ -154,10 +154,10 @@ fn parse_account_kind( }), (1, None, None) => Ok(AccountKind::Imported), (0, None, None) | (1, Some(_), Some(_)) => Err(SqliteClientError::CorruptedData( - "Wallet DB account_type constraint violated".to_string(), + "Wallet DB account_kind constraint violated".to_string(), )), (_, _, _) => Err(SqliteClientError::CorruptedData( - "Unrecognized account_type".to_string(), + "Unrecognized account_kind".to_string(), )), } } @@ -351,13 +351,13 @@ pub(crate) fn add_account( let account_id: AccountId = conn.query_row( r#" INSERT INTO accounts ( - account_type, hd_seed_fingerprint, hd_account_index, + account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, orchard_fvk_item_cache, sapling_fvk_item_cache, p2pkh_fvk_item_cache, birthday_height, recover_until_height ) VALUES ( - :account_type, :hd_seed_fingerprint, :hd_account_index, + :account_kind, :hd_seed_fingerprint, :hd_account_index, :ufvk, :uivk, :orchard_fvk_item_cache, :sapling_fvk_item_cache, :p2pkh_fvk_item_cache, :birthday_height, :recover_until_height @@ -365,7 +365,7 @@ pub(crate) fn add_account( RETURNING id; "#, named_params![ - ":account_type": account_kind_code(kind), + ":account_kind": account_kind_code(kind), ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.as_bytes()), ":hd_account_index": hd_account_index.map(u32::from), ":ufvk": viewing_key.ufvk().map(|ufvk| ufvk.encode(params)), @@ -717,7 +717,7 @@ pub(crate) fn get_account_for_ufvk( let transparent_item: Option> = None; let mut stmt = conn.prepare( - "SELECT id, account_type, hd_seed_fingerprint, hd_account_index, ufvk + "SELECT id, account_kind, hd_seed_fingerprint, hd_account_index, ufvk FROM accounts WHERE orchard_fvk_item_cache = :orchard_fvk_item_cache OR sapling_fvk_item_cache = :sapling_fvk_item_cache @@ -1501,7 +1501,7 @@ pub(crate) fn get_account( ) -> Result, SqliteClientError> { let mut sql = conn.prepare_cached( r#" - SELECT account_type, hd_seed_fingerprint, hd_account_index, ufvk, uivk + SELECT account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk FROM accounts WHERE id = :account_id "#, @@ -1512,7 +1512,7 @@ pub(crate) fn get_account( match row { Some(row) => { let kind = parse_account_kind( - row.get("account_type")?, + row.get("account_kind")?, row.get("hd_seed_fingerprint")?, row.get("hd_account_index")?, )?; diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index e74259b56..00147527d 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -224,7 +224,7 @@ mod tests { let expected_tables = vec![ r#"CREATE TABLE "accounts" ( id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, - account_type INTEGER NOT NULL DEFAULT 0, + account_kind INTEGER NOT NULL DEFAULT 0, hd_seed_fingerprint BLOB, hd_account_index INTEGER, ufvk TEXT, @@ -234,7 +234,7 @@ mod tests { p2pkh_fvk_item_cache BLOB, birthday_height INTEGER NOT NULL, recover_until_height INTEGER, - CHECK ( (account_type = 0 AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL AND ufvk IS NOT NULL) OR (account_type = 1 AND hd_seed_fingerprint IS NULL AND hd_account_index IS NULL) ) + CHECK ( (account_kind = 0 AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL AND ufvk IS NOT NULL) OR (account_kind = 1 AND hd_seed_fingerprint IS NULL AND hd_account_index IS NULL) ) )"#, r#"CREATE TABLE "addresses" ( account_id INTEGER NOT NULL, diff --git a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs index 08ca4b9c7..60f9b924a 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs @@ -44,11 +44,11 @@ impl RusqliteMigration for Migration

{ type Error = WalletMigrationError; fn up(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { - let account_type_derived = account_kind_code(AccountKind::Derived { + let account_kind_derived = account_kind_code(AccountKind::Derived { seed_fingerprint: HdSeedFingerprint::from_bytes([0; 32]), account_index: zip32::AccountId::ZERO, }); - let account_type_imported = account_kind_code(AccountKind::Imported); + let account_kind_imported = account_kind_code(AccountKind::Imported); transaction.execute_batch( &format!(r#" PRAGMA foreign_keys = OFF; @@ -56,7 +56,7 @@ impl RusqliteMigration for Migration

{ CREATE TABLE accounts_new ( id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, - account_type INTEGER NOT NULL DEFAULT {account_type_derived}, + account_kind INTEGER NOT NULL DEFAULT {account_kind_derived}, hd_seed_fingerprint BLOB, hd_account_index INTEGER, ufvk TEXT, @@ -67,9 +67,9 @@ impl RusqliteMigration for Migration

{ birthday_height INTEGER NOT NULL, recover_until_height INTEGER, CHECK ( - (account_type = {account_type_derived} AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL AND ufvk IS NOT NULL) + (account_kind = {account_kind_derived} AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL AND ufvk IS NOT NULL) OR - (account_type = {account_type_imported} AND hd_seed_fingerprint IS NULL AND hd_account_index IS NULL) + (account_kind = {account_kind_imported} AND hd_seed_fingerprint IS NULL AND hd_account_index IS NULL) ) ); CREATE UNIQUE INDEX hd_account ON accounts_new (hd_seed_fingerprint, hd_account_index); @@ -130,13 +130,13 @@ impl RusqliteMigration for Migration

{ transaction.execute( r#" INSERT INTO accounts_new ( - id, account_type, hd_seed_fingerprint, hd_account_index, + id, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, orchard_fvk_item_cache, sapling_fvk_item_cache, p2pkh_fvk_item_cache, birthday_height, recover_until_height ) VALUES ( - :account_id, :account_type, :seed_id, :account_index, + :account_id, :account_kind, :seed_id, :account_index, :ufvk, :uivk, :orchard_fvk_item_cache, :sapling_fvk_item_cache, :p2pkh_fvk_item_cache, :birthday_height, :recover_until_height @@ -144,7 +144,7 @@ impl RusqliteMigration for Migration

{ "#, named_params![ ":account_id": account_id, - ":account_type": account_type_derived, + ":account_kind": account_kind_derived, ":seed_id": seed_id.as_bytes(), ":account_index": account_index, ":ufvk": ufvk,