From 2da2b3e9fef5948daf49234dcb4027f4a864311d Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 4 Dec 2024 22:57:21 -0700 Subject: [PATCH 1/2] zcash_client_backend: Add optional derivation metadata when importing UFVKs with spending purpose. --- zcash_client_backend/src/data_api.rs | 40 ++++++++++-- zcash_client_sqlite/src/lib.rs | 52 +++++++--------- zcash_client_sqlite/src/wallet.rs | 61 +++++++++++-------- zcash_client_sqlite/src/wallet/db.rs | 17 +++--- zcash_client_sqlite/src/wallet/init.rs | 4 +- .../init/migrations/add_account_uuids.rs | 14 +++-- .../init/migrations/ephemeral_addresses.rs | 8 ++- .../init/migrations/full_account_ids.rs | 8 ++- 8 files changed, 120 insertions(+), 84 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 6eaca5cb7..2512c409d 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -331,13 +331,40 @@ impl AccountBalance { } } +/// Source metadata for a ZIP 32-derived key. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct Zip32Derivation { + seed_fingerprint: SeedFingerprint, + account_index: zip32::AccountId, +} + +impl Zip32Derivation { + /// Constructs new derivation metadata from its constituent parts. + pub fn new(seed_fingerprint: SeedFingerprint, account_index: zip32::AccountId) -> Self { + Self { + seed_fingerprint, + account_index, + } + } + + /// Returns the seed fingerprint. + pub fn seed_fingerprint(&self) -> &SeedFingerprint { + &self.seed_fingerprint + } + + /// Returns the account-level index in the ZIP 32 derivation path. + pub fn account_index(&self) -> zip32::AccountId { + self.account_index + } +} + /// 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)] +#[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum AccountPurpose { /// For spending accounts, the wallet will track information needed to spend /// received notes. - Spending, + Spending { derivation: Option }, /// For view-only accounts, the wallet will not track spend information. ViewOnly, } @@ -347,8 +374,7 @@ pub enum AccountPurpose { pub enum AccountSource { /// An account derived from a known seed. Derived { - seed_fingerprint: SeedFingerprint, - account_index: zip32::AccountId, + derivation: Zip32Derivation, key_source: Option, }, @@ -376,8 +402,10 @@ pub trait Account { /// 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, + AccountSource::Derived { derivation, .. } => AccountPurpose::Spending { + derivation: Some(derivation.clone()), + }, + AccountSource::Imported { purpose, .. } => purpose.clone(), } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 7d9bacca9..9fe3fe45e 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -54,7 +54,7 @@ use zcash_client_backend::{ Account, AccountBirthday, AccountMeta, AccountPurpose, AccountSource, BlockMetadata, DecryptedTransaction, InputSource, NoteFilter, NullifierQuery, ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, TransactionDataRequest, WalletCommitmentTrees, WalletRead, - WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + WalletSummary, WalletWrite, Zip32Derivation, SAPLING_SHARD_HEIGHT, }, keys::{ AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey, @@ -442,17 +442,12 @@ impl, P: consensus::Parameters> WalletRead for W seed: &SecretVec, ) -> Result { if let Some(account) = self.get_account(account_id)? { - if let AccountSource::Derived { - seed_fingerprint, - account_index, - .. - } = account.source() - { + if let AccountSource::Derived { derivation, .. } = account.source() { wallet::seed_matches_derived_account( &self.params, seed, - seed_fingerprint, - *account_index, + derivation.seed_fingerprint(), + derivation.account_index(), &account.uivk(), ) } else { @@ -480,19 +475,14 @@ impl, P: consensus::Parameters> WalletRead for W // way we could determine that is by brute-forcing the ZIP 32 account // index space, which we're not going to do. The method name indicates to // the caller that we only check derived accounts. - if let AccountSource::Derived { - seed_fingerprint, - account_index, - .. - } = account.source() - { + if let AccountSource::Derived { derivation, .. } = account.source() { has_derived = true; if wallet::seed_matches_derived_account( &self.params, seed, - seed_fingerprint, - *account_index, + derivation.seed_fingerprint(), + derivation.account_index(), &account.uivk(), )? { // The seed is relevant to this account. @@ -873,8 +863,7 @@ impl WalletWrite for WalletDb &wdb.params, account_name, &AccountSource::Derived { - seed_fingerprint, - account_index: zip32_account_index, + derivation: Zip32Derivation::new(seed_fingerprint, zip32_account_index), key_source: key_source.map(|s| s.to_owned()), }, wallet::ViewingKey::Full(Box::new(ufvk)), @@ -911,8 +900,7 @@ impl WalletWrite for WalletDb &wdb.params, account_name, &AccountSource::Derived { - seed_fingerprint, - account_index, + derivation: Zip32Derivation::new(seed_fingerprint, account_index), key_source: key_source.map(|s| s.to_owned()), }, wallet::ViewingKey::Full(Box::new(ufvk)), @@ -2023,7 +2011,7 @@ mod tests { .build(); assert_matches!( st.test_account().unwrap().account().source(), - AccountSource::Derived { account_index, .. } if *account_index == zip32::AccountId::ZERO); + AccountSource::Derived { derivation, .. } if derivation.account_index() == zip32::AccountId::ZERO); } #[test] @@ -2046,7 +2034,7 @@ mod tests { .unwrap(); assert_matches!( first.0.source(), - AccountSource::Derived { account_index, .. } if *account_index == zip32_index_1); + AccountSource::Derived { derivation, .. } if derivation.account_index() == zip32_index_1); let zip32_index_2 = zip32_index_1.next().unwrap(); let second = st @@ -2055,7 +2043,7 @@ mod tests { .unwrap(); assert_matches!( second.0.source(), - AccountSource::Derived { account_index, .. } if *account_index == zip32_index_2); + AccountSource::Derived { derivation, .. } if derivation.account_index() == zip32_index_2); } fn check_collisions( @@ -2068,7 +2056,7 @@ mod tests { { assert_matches!( st.wallet_mut() - .import_account_ufvk("", ufvk, birthday, AccountPurpose::Spending, None), + .import_account_ufvk("", ufvk, birthday, AccountPurpose::Spending { derivation: None }, None), Err(e) if is_account_collision(&e) ); @@ -2089,7 +2077,7 @@ mod tests { "", &subset_ufvk, birthday, - AccountPurpose::Spending, + AccountPurpose::Spending { derivation: None }, None, ), Err(e) if is_account_collision(&e) @@ -2113,7 +2101,7 @@ mod tests { "", &subset_ufvk, birthday, - AccountPurpose::Spending, + AccountPurpose::Spending { derivation: None }, None, ), Err(e) if is_account_collision(&e) @@ -2172,7 +2160,13 @@ mod tests { let account = st .wallet_mut() - .import_account_ufvk("", &ufvk, &birthday, AccountPurpose::Spending, None) + .import_account_ufvk( + "", + &ufvk, + &birthday, + AccountPurpose::Spending { derivation: None }, + None, + ) .unwrap(); assert_eq!( ufvk.encode(st.network()), @@ -2182,7 +2176,7 @@ mod tests { assert_matches!( account.source(), AccountSource::Imported { - purpose: AccountPurpose::Spending, + purpose: AccountPurpose::Spending { .. }, .. } ); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index ff98461f3..8ae5fbd04 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -72,7 +72,9 @@ use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; use uuid::Uuid; use zcash_client_backend::data_api::{ AccountPurpose, DecryptedTransaction, Progress, TransactionDataRequest, TransactionStatus, + Zip32Derivation, }; + use zip32::fingerprint::SeedFingerprint; use std::collections::{HashMap, HashSet}; @@ -150,28 +152,36 @@ fn parse_account_source( spending_key_available: bool, key_source: Option, ) -> Result { - match (account_kind, hd_seed_fingerprint, hd_account_index) { - (0, Some(seed_fp), Some(account_index)) => Ok(AccountSource::Derived { - seed_fingerprint: SeedFingerprint::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(), - ) - })?, + let derivation = hd_seed_fingerprint + .zip(hd_account_index) + .map(|(seed_fp, idx)| { + zip32::AccountId::try_from(idx) + .map_err(|_| { + SqliteClientError::CorruptedData( + "ZIP-32 account ID from wallet DB is out of range.".to_string(), + ) + }) + .map(|idx| Zip32Derivation::new(SeedFingerprint::from_bytes(seed_fp), idx)) + }) + .transpose()?; + + match (account_kind, derivation) { + (0, Some(derivation)) => Ok(AccountSource::Derived { + derivation, key_source, }), - (1, None, None) => Ok(AccountSource::Imported { + (1, derivation) => Ok(AccountSource::Imported { purpose: if spending_key_available { - AccountPurpose::Spending + AccountPurpose::Spending { derivation } } else { AccountPurpose::ViewOnly }, key_source, }), - (0, None, None) | (1, Some(_), Some(_)) => Err(SqliteClientError::CorruptedData( + (0, None) => Err(SqliteClientError::CorruptedData( "Wallet DB account_kind constraint violated".to_string(), )), - (_, _, _) => Err(SqliteClientError::CorruptedData( + (_, _) => Err(SqliteClientError::CorruptedData( "Unrecognized account_kind".to_string(), )), } @@ -378,21 +388,19 @@ pub(crate) fn add_account( let account_uuid = AccountUuid(Uuid::new_v4()); - let (hd_seed_fingerprint, hd_account_index, spending_key_available, key_source) = match kind { + let (derivation, spending_key_available, key_source) = match kind { AccountSource::Derived { - seed_fingerprint, - account_index, + derivation, key_source, - } => ( - Some(seed_fingerprint), - Some(account_index), - true, + } => (Some(derivation), true, key_source), + AccountSource::Imported { + purpose: AccountPurpose::Spending { derivation }, key_source, - ), + } => (derivation.as_ref(), true, key_source), AccountSource::Imported { - purpose, + purpose: AccountPurpose::ViewOnly, key_source, - } => (None, None, *purpose == AccountPurpose::Spending, key_source), + } => (None, false, key_source), }; #[cfg(feature = "orchard")] @@ -449,8 +457,8 @@ pub(crate) fn add_account( ":account_name": account_name, ":uuid": account_uuid.0, ":account_kind": account_kind_code(kind), - ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.to_bytes()), - ":hd_account_index": hd_account_index.map(|i| u32::from(*i)), + ":hd_seed_fingerprint": derivation.map(|d| d.seed_fingerprint().to_bytes()), + ":hd_account_index": derivation.map(|d| u32::from(d.account_index())), ":key_source": key_source, ":ufvk": ufvk_encoded, ":uivk": viewing_key.uivk().encode(params), @@ -832,8 +840,7 @@ pub(crate) fn get_derived_account( name: account_name, uuid: account_uuid, kind: AccountSource::Derived { - seed_fingerprint: *seed_fp, - account_index, + derivation: Zip32Derivation::new(*seed_fp, account_index), key_source, }, viewing_key: ViewingKey::Full(Box::new(ufvk)), @@ -3761,7 +3768,7 @@ mod tests { let expected_account_index = zip32::AccountId::try_from(0).unwrap(); assert_matches!( account_parameters.kind, - AccountSource::Derived{account_index, ..} if account_index == expected_account_index + AccountSource::Derived{derivation, ..} if derivation.account_index() == expected_account_index ); } diff --git a/zcash_client_sqlite/src/wallet/db.rs b/zcash_client_sqlite/src/wallet/db.rs index 4b7e4c9e0..a2f920afb 100644 --- a/zcash_client_sqlite/src/wallet/db.rs +++ b/zcash_client_sqlite/src/wallet/db.rs @@ -41,18 +41,17 @@ CREATE TABLE "accounts" ( recover_until_height INTEGER, has_spend_key INTEGER NOT NULL DEFAULT 1, CHECK ( - ( + ( account_kind = 0 AND hd_seed_fingerprint IS NOT NULL AND hd_account_index IS NOT NULL AND ufvk IS NOT NULL - ) - OR - ( + ) + OR + ( account_kind = 1 - AND hd_seed_fingerprint IS NULL - AND hd_account_index IS NULL - ) + AND (hd_seed_fingerprint IS NULL) = (hd_account_index IS NULL) + ) ) )"#; pub(super) const INDEX_ACCOUNTS_UUID: &str = @@ -384,8 +383,8 @@ CREATE TABLE transparent_spend_map ( prevout_txid BLOB NOT NULL, prevout_output_index INTEGER NOT NULL, FOREIGN KEY (spending_transaction_id) REFERENCES transactions(id_tx) - -- NOTE: We can't create a unique constraint on just (prevout_txid, prevout_output_index) - -- because the same output may be attempted to be spent in multiple transactions, even + -- NOTE: We can't create a unique constraint on just (prevout_txid, prevout_output_index) + -- because the same output may be attempted to be spent in multiple transactions, even -- though only one will ever be mined. CONSTRAINT transparent_spend_map_unique UNIQUE ( spending_transaction_id, prevout_txid, prevout_output_index diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index d6b0e8288..82a44d1a3 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -1084,8 +1084,8 @@ mod tests { assert_matches!( db_data.get_account(account_id), Ok(Some(account)) if matches!( - account.kind, - AccountSource::Derived{account_index, ..} if account_index == zip32::AccountId::ZERO, + &account.kind, + AccountSource::Derived{derivation, ..} if derivation.account_index() == zip32::AccountId::ZERO, ) ); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/add_account_uuids.rs b/zcash_client_sqlite/src/wallet/init/migrations/add_account_uuids.rs index e2934a8f2..aa38cd12c 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/add_account_uuids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/add_account_uuids.rs @@ -1,11 +1,12 @@ -//! This migration adds a UUID to each account record. +//! This migration adds a UUID to each account record, and adds `name` and `key_source` columns. In +//! addition, imported account records are now permitted to include key derivation metadata. use std::collections::HashSet; use rusqlite::named_params; use schemerz_rusqlite::RusqliteMigration; use uuid::Uuid; -use zcash_client_backend::data_api::{AccountPurpose, AccountSource}; +use zcash_client_backend::data_api::{AccountPurpose, AccountSource, Zip32Derivation}; use zip32::fingerprint::SeedFingerprint; use crate::wallet::{account_kind_code, init::WalletMigrationError}; @@ -37,8 +38,10 @@ impl RusqliteMigration for Migration { fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), Self::Error> { let account_kind_derived = account_kind_code(&AccountSource::Derived { - seed_fingerprint: SeedFingerprint::from_bytes([0; 32]), - account_index: zip32::AccountId::ZERO, + derivation: Zip32Derivation::new( + SeedFingerprint::from_bytes([0; 32]), + zip32::AccountId::ZERO, + ), key_source: None, }); let account_kind_imported = account_kind_code(&AccountSource::Imported { @@ -77,8 +80,7 @@ impl RusqliteMigration for Migration { OR ( account_kind = {account_kind_imported} - AND hd_seed_fingerprint IS NULL - AND hd_account_index IS NULL + AND (hd_seed_fingerprint IS NULL) = (hd_account_index IS NULL) ) ) ); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs index 225cac2ab..196c922ec 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/ephemeral_addresses.rs @@ -209,6 +209,8 @@ mod tests { #[test] #[cfg(feature = "transparent-inputs")] fn initialize_table() { + use zcash_client_backend::data_api::Zip32Derivation; + let network = Network::TestNetwork; let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap(); @@ -228,8 +230,10 @@ mod tests { let account0_index = Zip32AccountId::ZERO; let account0_seed_fp = [0u8; 32]; let account0_kind = account_kind_code(&AccountSource::Derived { - seed_fingerprint: SeedFingerprint::from_seed(&account0_seed_fp).unwrap(), - account_index: account0_index, + derivation: Zip32Derivation::new( + SeedFingerprint::from_seed(&account0_seed_fp).unwrap(), + account0_index, + ), key_source: None, }); assert_eq!(u32::from(account0_index), 0); 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 ef1ad071a..e8df8b216 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 @@ -6,7 +6,7 @@ use schemerz_rusqlite::RusqliteMigration; use secrecy::{ExposeSecret, SecretVec}; use uuid::Uuid; use zcash_client_backend::{ - data_api::{AccountPurpose, AccountSource}, + data_api::{AccountPurpose, AccountSource, Zip32Derivation}, keys::UnifiedSpendingKey, }; use zcash_keys::keys::UnifiedFullViewingKey; @@ -53,8 +53,10 @@ impl RusqliteMigration for Migration

{ fn up(&self, transaction: &Transaction) -> Result<(), WalletMigrationError> { let account_kind_derived = account_kind_code(&AccountSource::Derived { - seed_fingerprint: SeedFingerprint::from_bytes([0; 32]), - account_index: zip32::AccountId::ZERO, + derivation: Zip32Derivation::new( + SeedFingerprint::from_bytes([0; 32]), + zip32::AccountId::ZERO, + ), key_source: None, }); let account_kind_imported = account_kind_code(&AccountSource::Imported { From cc2dfbf7bf1f40a4d9d1aeb731537568bab61164 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 5 Dec 2024 15:46:58 -0700 Subject: [PATCH 2/2] zcash_client_backend: Add AccountSource::key_derivation --- zcash_client_backend/CHANGELOG.md | 3 +++ zcash_client_backend/src/data_api.rs | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index e55b14989..e0ad6c724 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -7,6 +7,9 @@ and this library adheres to Rust's notion of ## [Unreleased] +### Added +- `zcash_client_backend::data_api::AccountSource::key_derivation` + ### Changed - `zcash_client_backend::data_api::WalletRead`: - The `create_account`, `import_account_hd`, and `import_account_ufvk` diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 2512c409d..3b3ab351b 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -385,6 +385,28 @@ pub enum AccountSource { }, } +impl AccountSource { + /// Returns the key derivation metadata for the account source, if any is available. + pub fn key_derivation(&self) -> Option<&Zip32Derivation> { + match self { + AccountSource::Derived { derivation, .. } => Some(derivation), + AccountSource::Imported { + purpose: AccountPurpose::Spending { derivation }, + .. + } => derivation.as_ref(), + _ => None, + } + } + + /// Returns the application-level key source identifier. + pub fn key_source(&self) -> Option<&str> { + match self { + AccountSource::Derived { key_source, .. } => key_source.as_ref().map(|s| s.as_str()), + AccountSource::Imported { key_source, .. } => key_source.as_ref().map(|s| s.as_str()), + } + } +} + /// A set of capabilities that a client account must provide. pub trait Account { type AccountId: Copy;