From a3196841e3a1f5408ae79752d372443709497eeb Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Thu, 7 Mar 2024 06:49:15 -0700 Subject: [PATCH] Drop new `Account` and associated APIs I moved them to an internal struct in the sqlite crate. I think I'd like to bring it back eventually, but with a more focused and intentional PR. --- zcash_client_backend/CHANGELOG.md | 4 - zcash_client_backend/src/data_api.rs | 14 +--- zcash_client_backend/src/data_api/wallet.rs | 85 -------------------- zcash_client_sqlite/src/lib.rs | 9 +-- zcash_client_sqlite/src/wallet.rs | 86 ++++++++++++++++++--- zcash_client_sqlite/src/wallet/init.rs | 6 +- 6 files changed, 82 insertions(+), 122 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index cade27ffe1..c20cead524 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -38,16 +38,12 @@ and this library adheres to Rust's notion of - `WalletOrchardSpend` - `WalletOrchardOutput` - `WalletTx::{orchard_spends, orchard_outputs}` - - `HdSeedAccount` - - `ImportedAccount` - - `Account` ### Changed - `zcash_client_backend::data_api`: - Arguments to `BlockMetadata::from_parts` have changed. - Arguments to `ScannedBlock::from_parts` have changed. - Changes to the `WalletRead` trait: - - Added `get_account` - Added `get_orchard_nullifiers` - `ShieldedProtocol` has a new `Orchard` variant. - `WalletCommitmentTrees` diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 991147c4e7..53a5be1597 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -67,7 +67,7 @@ use incrementalmerkletree::{frontier::Frontier, Retention}; use secrecy::SecretVec; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; -use self::{chain::CommitmentTreeRoot, scanning::ScanRange, wallet::Account}; +use self::{chain::CommitmentTreeRoot, scanning::ScanRange}; use crate::{ address::UnifiedAddress, decrypt::DecryptedOutput, @@ -492,11 +492,6 @@ pub trait WalletRead { /// will be interpreted as belonging to that account. type AccountId: Copy + Debug + Eq + Hash; - /// Gets some of the account details (e.g. seed fingerprint+index and/or uvk) for a given account id. - /// - /// Returns `Ok(None)` if no account by the given ID is known. - fn get_account(&self, account_id: Self::AccountId) -> Result, Self::Error>; - /// Verifies that the given seed corresponds to the viewing key for the specified account. /// /// Returns: @@ -1339,13 +1334,6 @@ pub mod testing { type Error = (); type AccountId = u32; - fn get_account( - &self, - _account_id: Self::AccountId, - ) -> Result, Self::Error> { - Ok(None) - } - fn validate_seed( &self, _account_id: Self::AccountId, diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 8d79e480c3..a8c9b30672 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -36,12 +36,6 @@ use sapling::{ }; use std::num::NonZeroU32; -use zcash_keys::{ - address::UnifiedAddress, - keys::{ - AddressGenerationError, HdSeedFingerprint, UnifiedAddressRequest, UnifiedFullViewingKey, - }, -}; use zcash_primitives::{ consensus::{self, BlockHeight, NetworkUpgrade}, memo::MemoBytes, @@ -56,8 +50,6 @@ use zcash_primitives::{ }, zip32::Scope, }; -use zip32::DiversifierIndex; - use super::InputSource; use crate::{ address::Address, @@ -88,83 +80,6 @@ use input_selection::{ GreedyInputSelector, GreedyInputSelectorError, InputSelector, InputSelectorError, }; -/// Describes the key inputs and UFVK for an account that was derived from a ZIP-32 HD seed and account index. -#[derive(Debug, Clone)] -pub struct HdSeedAccount(HdSeedFingerprint, zip32::AccountId, UnifiedFullViewingKey); - -impl HdSeedAccount { - pub fn new( - hd_seed_fingerprint: HdSeedFingerprint, - account_index: zip32::AccountId, - ufvk: UnifiedFullViewingKey, - ) -> Self { - Self(hd_seed_fingerprint, account_index, 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 enum ImportedAccount { - /// An account that was imported via its full viewing key. - Full(UnifiedFullViewingKey), -} - -/// Describes an account in terms of its UVK or ZIP-32 origins. -#[derive(Debug, Clone)] -pub enum Account { - /// Inputs for a ZIP-32 HD account. - Zip32(HdSeedAccount), - /// Inputs for an imported account. - Imported(ImportedAccount), -} - -impl Account { - /// Returns the default Unified Address for the account, - /// along with the diversifier index that generated it. - /// - /// 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( - &self, - request: UnifiedAddressRequest, - ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { - match self { - Account::Zip32(HdSeedAccount(_, _, ufvk)) => ufvk.default_address(request), - Account::Imported(ImportedAccount::Full(ufvk)) => ufvk.default_address(request), - } - } - - /// Gets the unified full viewing key for this account, if available. - /// - /// Accounts initialized with an incoming viewing key will not have a unified full viewing key. - pub fn ufvk(&self) -> Option<&UnifiedFullViewingKey> { - match self { - Account::Zip32(HdSeedAccount(_, _, ufvk)) => Some(ufvk), - Account::Imported(ImportedAccount::Full(ufvk)) => Some(ufvk), - } - } - - // TODO: When a UnifiedIncomingViewingKey is available, add a function that - // will return it (external scope only). Even if the Account was initialized with a UFVK, - // we can always derive a UIVK from that. -} - /// Scans a [`Transaction`] for any information that can be decrypted by the accounts in /// the wallet, and saves it to the wallet. pub fn decrypt_and_store_transaction( diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index c1bf3bec27..a094458889 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -63,7 +63,6 @@ use zcash_client_backend::{ self, chain::{BlockSource, CommitmentTreeRoot}, scanning::{ScanPriority, ScanRange}, - wallet::{Account, HdSeedAccount}, AccountBirthday, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, @@ -100,7 +99,7 @@ pub mod error; pub mod wallet; use wallet::{ commitment_tree::{self, put_shard_roots}, - SubtreeScanProgress, + Account, HdSeedAccount, SubtreeScanProgress, }; #[cfg(test)] @@ -266,7 +265,7 @@ impl, P: consensus::Parameters> WalletRead for W account_id: Self::AccountId, seed: &SecretVec, ) -> Result { - if let Some(account) = self.get_account(account_id)? { + if let Some(account) = wallet::get_account(self, account_id)? { if let Account::Zip32(hdaccount) = account { let usk = UnifiedSpendingKey::from_seed( &self.params, @@ -292,10 +291,6 @@ impl, P: consensus::Parameters> WalletRead for W } } - fn get_account(&self, account_id: Self::AccountId) -> Result, Self::Error> { - wallet::get_account(self.conn.borrow(), &self.params, account_id) - } - fn chain_height(&self) -> Result, Self::Error> { wallet::scan_queue_extrema(self.conn.borrow()) .map(|h| h.map(|range| *range.end())) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 119c711e58..bd7f46311a 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -67,19 +67,19 @@ 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}; use std::num::NonZeroU32; use std::ops::RangeInclusive; use tracing::debug; -use zcash_keys::keys::HdSeedFingerprint; +use zcash_keys::keys::{AddressGenerationError, HdSeedFingerprint, UnifiedAddressRequest}; use zcash_client_backend::{ address::{Address, UnifiedAddress}, data_api::{ scanning::{ScanPriority, ScanRange}, - wallet::{Account, HdSeedAccount, ImportedAccount}, AccountBalance, AccountBirthday, BlockMetadata, Ratio, SentTransactionOutput, WalletSummary, SAPLING_SHARD_HEIGHT, }, @@ -159,6 +159,69 @@ impl From for u32 { } } +/// Describes the key inputs and UFVK for an account that was derived from a ZIP-32 HD seed and account index. +#[derive(Debug, Clone)] +pub(crate) struct HdSeedAccount(HdSeedFingerprint, zip32::AccountId, UnifiedFullViewingKey); + +impl HdSeedAccount { + pub fn new( + hd_seed_fingerprint: HdSeedFingerprint, + account_index: zip32::AccountId, + ufvk: UnifiedFullViewingKey, + ) -> Self { + Self(hd_seed_fingerprint, account_index, 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. + Full(UnifiedFullViewingKey), +} + +/// Describes an account in terms of its UVK or ZIP-32 origins. +#[derive(Debug, Clone)] +pub(crate) enum Account { + /// Inputs for a ZIP-32 HD account. + Zip32(HdSeedAccount), + /// Inputs for an imported account. + Imported(ImportedAccount), +} + +impl Account { + /// Returns the default Unified Address for the account, + /// along with the diversifier index that generated it. + /// + /// 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( + &self, + request: UnifiedAddressRequest, + ) -> Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError> { + match self { + Account::Zip32(HdSeedAccount(_, _, ufvk)) => ufvk.default_address(request), + Account::Imported(ImportedAccount::Full(ufvk)) => ufvk.default_address(request), + } + } +} + pub(crate) fn pool_code(pool_type: PoolType) -> i64 { // These constants are *incidentally* shared with the typecodes // for unified addresses, but this is exclusively an internal @@ -1039,12 +1102,11 @@ pub(crate) fn block_height_extrema( }) } -pub(crate) fn get_account( - conn: &rusqlite::Connection, - params: &P, +pub(crate) fn get_account, P: Parameters>( + db: &WalletDb, account_id: AccountId, ) -> Result, SqliteClientError> { - let mut sql = conn.prepare_cached( + let mut sql = db.conn.borrow().prepare_cached( r#" SELECT account_type, uvk, hd_seed_fingerprint, hd_account_index FROM accounts @@ -1060,7 +1122,7 @@ pub(crate) fn get_account( SqliteClientError::CorruptedData("Unrecognized account_type".to_string()) })?; let uvk: String = row.get(1)?; - let ufvk = UnifiedFullViewingKey::decode(params, &uvk[..]) + let ufvk = UnifiedFullViewingKey::decode(&db.params, &uvk[..]) .map_err(SqliteClientError::CorruptedData)?; match account_type { @@ -1800,8 +1862,9 @@ pub(crate) fn put_received_transparent_utxo( } else { // If the UTXO is received at the legacy transparent address, there may be no entry in the // addresses table that can be used to tie the address to a particular account. In this - // case, we should look up the legacy address for account 0 and check whether it matches - // the address for the received UTXO, and if so then insert/update it directly. + // case, we should look up the legacy address for all ZIP-32 account index 0 accounts and + // check whether it matches the address for the received UTXO, and if so then insert/update + // it directly. let account = AccountId(0); get_legacy_transparent_address(params, conn, account).and_then(|legacy_taddr| { if legacy_taddr @@ -2155,11 +2218,12 @@ mod tests { use std::num::NonZeroU32; use sapling::zip32::ExtendedSpendingKey; - use zcash_client_backend::data_api::{wallet::Account, AccountBirthday, WalletRead}; + use zcash_client_backend::data_api::{AccountBirthday, WalletRead}; use zcash_primitives::{block::BlockHash, transaction::components::amount::NonNegativeAmount}; use crate::{ testing::{AddressType, BlockCache, TestBuilder, TestState}, + wallet::{get_account, Account}, AccountId, }; @@ -2307,7 +2371,7 @@ mod tests { .with_test_account(AccountBirthday::from_sapling_activation) .build(); let account_id = st.test_account().unwrap().0; - let account_parameters = st.wallet().get_account(account_id).unwrap().unwrap(); + let account_parameters = get_account(st.wallet(), 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 a12165f684..3cd2fa8ecd 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -1086,7 +1086,9 @@ mod tests { #[test] #[cfg(feature = "transparent-inputs")] fn account_produces_expected_ua_sequence() { - use zcash_client_backend::data_api::{wallet::Account, AccountBirthday, WalletRead}; + use zcash_client_backend::data_api::AccountBirthday; + + use crate::wallet::{get_account, Account}; let network = Network::MainNetwork; let data_file = NamedTempFile::new().unwrap(); @@ -1102,7 +1104,7 @@ mod tests { .create_account(&Secret::new(seed.to_vec()), birthday) .unwrap(); assert_matches!( - db_data.get_account(account_id), + get_account(&db_data, account_id), Ok(Some(Account::Zip32(hdaccount))) if hdaccount.account_index() == zip32::AccountId::ZERO );