From ac7cbf9a415b5700f0241d44b4364d9324c43aa6 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 9 Aug 2024 14:41:51 -0600 Subject: [PATCH] zcash_client_backend: Add a `purpose` modifier for imported accounts. This moves the tracking of whether or not a spending key is expected to be available for an imported account into the `AccountSource::Imported` variant. --- zcash_client_backend/CHANGELOG.md | 4 ++ zcash_client_backend/src/data_api.rs | 41 +++++++++++++++---- zcash_client_sqlite/src/lib.rs | 31 +++++++------- zcash_client_sqlite/src/wallet.rs | 36 ++++++++++------ .../init/migrations/full_account_ids.rs | 11 ++++- .../src/wallet/transparent/ephemeral.rs | 31 ++++++++++---- 6 files changed, 108 insertions(+), 46 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 92dc478fe..ac906639e 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -30,6 +30,7 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail - `DecryptedTransaction::mined_height` - `TransactionDataRequest` - `TransactionStatus` + - `AccountType` - `zcash_client_backend::fees`: - `EphemeralBalance` - `ChangeValue::shielded, is_ephemeral` @@ -88,6 +89,9 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail references to slices, with a corresponding change to `SentTransaction::new`. - `SentTransaction` takes an additional `target_height` argument, which is used to record the target height used in transaction generation. + - `AccountSource::Imported` is now a struct variant with a `purpose` field. + - The `Account` trait now defines a new `purpose` method with a default + implementation (which need not be overridden.) - `zcash_client_backend::data_api::fees` - When the "transparent-inputs" feature is enabled, `ChangeValue` can also represent an ephemeral transparent output in a proposal. Accordingly, the diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 0ca7083e2..388ccb10b 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -321,6 +321,17 @@ impl AccountBalance { } } +/// An enumeration used to control what information is tracked by the wallet for +/// notes received by a given account. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub enum AccountPurpose { + /// For spending accounts, the wallet will track information needed to spend + /// received notes. + Spending, + /// For view-only accounts, the wallet will not track spend information. + ViewOnly, +} + /// The kinds of accounts supported by `zcash_client_backend`. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum AccountSource { @@ -331,7 +342,7 @@ pub enum AccountSource { }, /// An account imported from a viewing key. - Imported, + Imported { purpose: AccountPurpose }, } /// A set of capabilities that a client account must provide. @@ -343,6 +354,14 @@ pub trait Account { /// if applicable. fn source(&self) -> AccountSource; + /// Returns whether the account is a spending account or a view-only account. + fn purpose(&self) -> AccountPurpose { + match self.source() { + AccountSource::Derived { .. } => AccountPurpose::Spending, + AccountSource::Imported { purpose } => purpose, + } + } + /// 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 @@ -364,7 +383,9 @@ impl Account for (A, UnifiedFullViewingKey) { } fn source(&self) -> AccountSource { - AccountSource::Imported + AccountSource::Imported { + purpose: AccountPurpose::ViewOnly, + } } fn ufvk(&self) -> Option<&UnifiedFullViewingKey> { @@ -383,7 +404,9 @@ impl Account for (A, UnifiedIncomingViewingKey) { } fn source(&self) -> AccountSource { - AccountSource::Imported + AccountSource::Imported { + purpose: AccountPurpose::ViewOnly, + } } fn ufvk(&self) -> Option<&UnifiedFullViewingKey> { @@ -1816,7 +1839,7 @@ pub trait WalletWrite: WalletRead { &mut self, unified_key: &UnifiedFullViewingKey, birthday: &AccountBirthday, - spending_key_available: bool, + purpose: AccountPurpose, ) -> Result; /// Generates and persists the next available diversified address, given the current @@ -2027,10 +2050,10 @@ pub mod testing { use super::{ chain::{ChainState, CommitmentTreeRoot}, scanning::ScanRange, - AccountBirthday, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, - ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, TransactionDataRequest, - TransactionStatus, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, - SAPLING_SHARD_HEIGHT, + AccountBirthday, AccountPurpose, BlockMetadata, DecryptedTransaction, InputSource, + NullifierQuery, ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, + TransactionDataRequest, TransactionStatus, WalletCommitmentTrees, WalletRead, + WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }; #[cfg(feature = "transparent-inputs")] @@ -2319,7 +2342,7 @@ pub mod testing { &mut self, _unified_key: &UnifiedFullViewingKey, _birthday: &AccountBirthday, - _spending_key_available: bool, + _purpose: AccountPurpose, ) -> Result { todo!() } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index e4728625d..c8811a80d 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -50,10 +50,10 @@ use zcash_client_backend::{ self, chain::{BlockSource, ChainState, CommitmentTreeRoot}, scanning::{ScanPriority, ScanRange}, - Account, AccountBirthday, AccountSource, BlockMetadata, DecryptedTransaction, InputSource, - NullifierQuery, ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, - TransactionDataRequest, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, - SAPLING_SHARD_HEIGHT, + Account, AccountBirthday, AccountPurpose, AccountSource, BlockMetadata, + DecryptedTransaction, InputSource, NullifierQuery, ScannedBlock, SeedRelevance, + SentTransaction, SpendableNotes, TransactionDataRequest, WalletCommitmentTrees, WalletRead, + WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }, keys::{ AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey, @@ -630,7 +630,6 @@ impl WalletWrite for WalletDb .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?; let ufvk = usk.to_unified_full_viewing_key(); - let spending_key_available = true; let account = wallet::add_account( wdb.conn.0, &wdb.params, @@ -640,7 +639,6 @@ impl WalletWrite for WalletDb }, wallet::ViewingKey::Full(Box::new(ufvk)), birthday, - spending_key_available, )?; Ok((account.id(), usk)) @@ -666,7 +664,6 @@ impl WalletWrite for WalletDb .map_err(|_| SqliteClientError::KeyDerivationError(account_index))?; let ufvk = usk.to_unified_full_viewing_key(); - let spending_key_available = true; let account = wallet::add_account( wdb.conn.0, &wdb.params, @@ -676,7 +673,6 @@ impl WalletWrite for WalletDb }, wallet::ViewingKey::Full(Box::new(ufvk)), birthday, - spending_key_available, )?; Ok((account, usk)) @@ -687,16 +683,15 @@ impl WalletWrite for WalletDb &mut self, ufvk: &UnifiedFullViewingKey, birthday: &AccountBirthday, - spending_key_available: bool, + purpose: AccountPurpose, ) -> Result { self.transactionally(|wdb| { wallet::add_account( wdb.conn.0, &wdb.params, - AccountSource::Imported, + AccountSource::Imported { purpose }, wallet::ViewingKey::Full(Box::new(ufvk.to_owned())), birthday, - spending_key_available, ) }) } @@ -2029,7 +2024,8 @@ extern crate assert_matches; mod tests { use secrecy::{Secret, SecretVec}; use zcash_client_backend::data_api::{ - chain::ChainState, Account, AccountBirthday, AccountSource, WalletRead, WalletWrite, + chain::ChainState, Account, AccountBirthday, AccountPurpose, AccountSource, WalletRead, + WalletWrite, }; use zcash_keys::keys::UnifiedSpendingKey; use zcash_primitives::block::BlockHash; @@ -2177,14 +2173,19 @@ mod tests { let account = st .wallet_mut() - .import_account_ufvk(&ufvk, &birthday, true) + .import_account_ufvk(&ufvk, &birthday, AccountPurpose::Spending) .unwrap(); assert_eq!( ufvk.encode(&st.wallet().params), account.ufvk().unwrap().encode(&st.wallet().params) ); - assert_matches!(account.source(), AccountSource::Imported); + assert_matches!( + account.source(), + AccountSource::Imported { + purpose: AccountPurpose::Spending + } + ); } #[test] @@ -2202,7 +2203,7 @@ mod tests { let ufvk = seed_based_account.ufvk().unwrap(); assert_matches!( - st.wallet_mut().import_account_ufvk(ufvk, &birthday, true), + st.wallet_mut().import_account_ufvk(ufvk, &birthday, AccountPurpose::Spending), Err(SqliteClientError::AccountCollision(id)) if id == seed_based.0); } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index f3260863c..683fb77d3 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -68,7 +68,7 @@ use incrementalmerkletree::{Marking, Retention}; use rusqlite::{self, named_params, params, OptionalExtension}; use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; -use zcash_client_backend::data_api::{TransactionDataRequest, TransactionStatus}; +use zcash_client_backend::data_api::{AccountPurpose, TransactionDataRequest, TransactionStatus}; use zip32::fingerprint::SeedFingerprint; use std::collections::{HashMap, HashSet}; @@ -146,6 +146,7 @@ fn parse_account_source( account_kind: u32, hd_seed_fingerprint: Option<[u8; 32]>, hd_account_index: Option, + spending_key_available: bool, ) -> Result { match (account_kind, hd_seed_fingerprint, hd_account_index) { (0, Some(seed_fp), Some(account_index)) => Ok(AccountSource::Derived { @@ -156,7 +157,13 @@ fn parse_account_source( ) })?, }), - (1, None, None) => Ok(AccountSource::Imported), + (1, None, None) => Ok(AccountSource::Imported { + purpose: if spending_key_available { + AccountPurpose::Spending + } else { + AccountPurpose::ViewOnly + }, + }), (0, None, None) | (1, Some(_), Some(_)) => Err(SqliteClientError::CorruptedData( "Wallet DB account_kind constraint violated".to_string(), )), @@ -169,7 +176,7 @@ fn parse_account_source( fn account_kind_code(value: AccountSource) -> u32 { match value { AccountSource::Derived { .. } => 0, - AccountSource::Imported => 1, + AccountSource::Imported { .. } => 1, } } @@ -349,14 +356,13 @@ pub(crate) fn add_account( kind: AccountSource, viewing_key: ViewingKey, birthday: &AccountBirthday, - spending_key_available: bool, ) -> Result { - let (hd_seed_fingerprint, hd_account_index) = match kind { + let (hd_seed_fingerprint, hd_account_index, spending_key_available) = match kind { AccountSource::Derived { seed_fingerprint, account_index, - } => (Some(seed_fingerprint), Some(account_index)), - AccountSource::Imported => (None, None), + } => (Some(seed_fingerprint), Some(account_index), true), + AccountSource::Imported { purpose } => (None, None, purpose == AccountPurpose::Spending), }; let orchard_item = viewing_key @@ -676,7 +682,7 @@ pub(crate) fn get_account_for_ufvk( let transparent_item: Option> = None; let mut stmt = conn.prepare( - "SELECT id, account_kind, hd_seed_fingerprint, hd_account_index, ufvk + "SELECT id, account_kind, hd_seed_fingerprint, hd_account_index, ufvk, has_spend_key FROM accounts WHERE orchard_fvk_item_cache = :orchard_fvk_item_cache OR sapling_fvk_item_cache = :sapling_fvk_item_cache @@ -691,12 +697,17 @@ pub(crate) fn get_account_for_ufvk( ":p2pkh_fvk_item_cache": transparent_item, ], |row| { - let account_id = row.get::<_, u32>(0).map(AccountId)?; - let kind = parse_account_source(row.get(1)?, row.get(2)?, row.get(3)?)?; + let account_id = row.get::<_, u32>("id").map(AccountId)?; + let kind = parse_account_source( + row.get("account_kind")?, + row.get("hd_seed_fingerprint")?, + row.get("hd_account_index")?, + row.get("has_spend_key")?, + )?; // We looked up the account by FVK components, so the UFVK column must be // non-null. - let ufvk_str: String = row.get(4)?; + let ufvk_str: String = row.get("ufvk")?; let viewing_key = ViewingKey::Full(Box::new( UnifiedFullViewingKey::decode(params, &ufvk_str).map_err(|e| { SqliteClientError::CorruptedData(format!( @@ -1501,7 +1512,7 @@ pub(crate) fn get_account( ) -> Result, SqliteClientError> { let mut sql = conn.prepare_cached( r#" - SELECT account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk + SELECT account_kind, hd_seed_fingerprint, hd_account_index, ufvk, uivk, has_spend_key FROM accounts WHERE id = :account_id "#, @@ -1515,6 +1526,7 @@ pub(crate) fn get_account( row.get("account_kind")?, row.get("hd_seed_fingerprint")?, row.get("hd_account_index")?, + row.get("has_spend_key")?, )?; let ufvk_str: Option = row.get("ufvk")?; 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 273808282..cd03ed5d4 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 @@ -5,7 +5,10 @@ use rusqlite::{named_params, OptionalExtension, Transaction}; use schemer_rusqlite::RusqliteMigration; use secrecy::{ExposeSecret, SecretVec}; use uuid::Uuid; -use zcash_client_backend::{data_api::AccountSource, keys::UnifiedSpendingKey}; +use zcash_client_backend::{ + data_api::{AccountPurpose, AccountSource}, + keys::UnifiedSpendingKey, +}; use zcash_keys::keys::UnifiedFullViewingKey; use zcash_primitives::consensus; use zip32::fingerprint::SeedFingerprint; @@ -53,7 +56,11 @@ impl RusqliteMigration for Migration

{ seed_fingerprint: SeedFingerprint::from_bytes([0; 32]), account_index: zip32::AccountId::ZERO, }); - let account_kind_imported = account_kind_code(AccountSource::Imported); + let account_kind_imported = account_kind_code(AccountSource::Imported { + // the purpose here is irrelevant; we just use it to get the correct code + // for the account kind + purpose: AccountPurpose::ViewOnly, + }); transaction.execute_batch(&format!( r#" CREATE TABLE accounts_new ( diff --git a/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs index f796d931a..5be7c675d 100644 --- a/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs +++ b/zcash_client_sqlite/src/wallet/transparent/ephemeral.rs @@ -4,7 +4,8 @@ use std::ops::Range; use rusqlite::{named_params, OptionalExtension}; -use zcash_client_backend::{data_api::Account, wallet::TransparentAddressMetadata}; +use zcash_client_backend::wallet::TransparentAddressMetadata; +use zcash_keys::keys::UnifiedFullViewingKey; use zcash_keys::{encoding::AddressCodec, keys::AddressGenerationError}; use zcash_primitives::{ legacy::{ @@ -15,11 +16,8 @@ use zcash_primitives::{ }; use zcash_protocol::consensus; -use crate::TxRef; use crate::{ - error::SqliteClientError, - wallet::{get_account, GAP_LIMIT}, - AccountId, SqlTransaction, WalletDb, + error::SqliteClientError, wallet::GAP_LIMIT, AccountId, SqlTransaction, TxRef, WalletDb, }; // Returns `TransparentAddressMetadata` in the ephemeral scope for the @@ -118,12 +116,29 @@ pub(crate) fn get_ephemeral_ivk( params: &P, account_id: AccountId, ) -> Result { - Ok(get_account(conn, params, account_id)? + let ufvk = conn + .query_row( + "SELECT ufvk FROM accounts WHERE id = :account_id", + named_params![":account_id": account_id.0], + |row| { + let ufvk_str: Option = row.get("ufvk")?; + Ok(ufvk_str.map(|s| { + UnifiedFullViewingKey::decode(params, &s[..]) + .map_err(SqliteClientError::BadAccountData) + })) + }, + ) + .optional()? .ok_or(SqliteClientError::AccountUnknown)? - .ufvk() + .transpose()?; + + let eivk = ufvk + .as_ref() .and_then(|ufvk| ufvk.transparent()) .ok_or(SqliteClientError::UnknownZip32Derivation)? - .derive_ephemeral_ivk()?) + .derive_ephemeral_ivk()?; + + Ok(eivk) } /// Returns a vector of ephemeral transparent addresses associated with the given