diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 1b82c0bb9..9873992fc 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -22,6 +22,7 @@ and this library adheres to Rust's notion of - `NoteRetention` - `ScannedBlock::orchard` - `ScannedBlockCommitments::orchard` + - `SeedRelevance` - `SentTransaction::new` - `SpendableNotes` - `ORCHARD_SHARD_HEIGHT` diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index c27d9b137..50841a87f 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -64,6 +64,7 @@ use std::{ }; use incrementalmerkletree::{frontier::Frontier, Retention}; +use nonempty::NonEmpty; use secrecy::SecretVec; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; use zip32::fingerprint::SeedFingerprint; @@ -742,10 +743,13 @@ pub trait WalletRead { /// Checks whether the given seed is relevant to any of the derived accounts (where /// [`Account::source`] is [`AccountSource::Derived`]) in the wallet. - fn is_seed_relevant_to_any_derived_accounts( + /// + /// This API does not check whether the seed is relevant to any imported account, + /// because that would require brute-forcing the ZIP 32 account index space. + fn seed_relevance_to_derived_accounts( &self, seed: &SecretVec, - ) -> Result; + ) -> Result, Self::Error>; /// Returns the account corresponding to a given [`UnifiedFullViewingKey`], if any. fn get_account_for_ufvk( @@ -907,6 +911,21 @@ pub trait WalletRead { } } +/// The relevance of a seed to a given wallet. +/// +/// This is the return type for [`WalletRead::seed_relevance_to_derived_accounts`]. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum SeedRelevance { + /// The seed is relevant to at least one derived account within the wallet. + Relevant { account_ids: NonEmpty }, + /// The seed is not relevant to any of the derived accounts within the wallet. + NotRelevant, + /// The wallet contains no derived accounts. + NoDerivedAccounts, + /// The wallet contains no accounts. + NoAccounts, +} + /// Metadata describing the sizes of the zcash note commitment trees as of a particular block. #[derive(Debug, Clone, Copy)] pub struct BlockMetadata { @@ -1632,8 +1651,8 @@ pub mod testing { chain::{ChainState, CommitmentTreeRoot}, scanning::ScanRange, AccountBirthday, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery, - ScannedBlock, SentTransaction, SpendableNotes, WalletCommitmentTrees, WalletRead, - WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, WalletCommitmentTrees, + WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }; #[cfg(feature = "transparent-inputs")] @@ -1726,11 +1745,11 @@ pub mod testing { Ok(false) } - fn is_seed_relevant_to_any_derived_accounts( + fn seed_relevance_to_derived_accounts( &self, _seed: &SecretVec, - ) -> Result { - Ok(false) + ) -> Result, Self::Error> { + Ok(SeedRelevance::NoAccounts) } fn get_account_for_ufvk( diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 2c247b393..45e475452 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -45,6 +45,7 @@ tracing.workspace = true # - Serialization byteorder.workspace = true +nonempty.workspace = true prost.workspace = true group.workspace = true jubjub.workspace = true @@ -82,7 +83,6 @@ bls12_381.workspace = true incrementalmerkletree = { workspace = true, features = ["test-dependencies"] } pasta_curves.workspace = true shardtree = { workspace = true, features = ["legacy-api", "test-dependencies"] } -nonempty.workspace = true orchard = { workspace = true, features = ["test-dependencies"] } proptest.workspace = true rand_chacha.workspace = true diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 6b8fd2982..bf822eeff 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -37,6 +37,7 @@ use maybe_rayon::{ prelude::{IndexedParallelIterator, ParallelIterator}, slice::ParallelSliceMut, }; +use nonempty::NonEmpty; use rusqlite::{self, Connection}; use secrecy::{ExposeSecret, SecretVec}; use shardtree::{error::ShardTreeError, ShardTree}; @@ -61,8 +62,8 @@ use zcash_client_backend::{ chain::{BlockSource, ChainState, CommitmentTreeRoot}, scanning::{ScanPriority, ScanRange}, Account, AccountBirthday, AccountSource, BlockMetadata, DecryptedTransaction, InputSource, - NullifierQuery, ScannedBlock, SentTransaction, SpendableNotes, WalletCommitmentTrees, - WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, + NullifierQuery, ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, + WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT, }, keys::{ AddressGenerationError, UnifiedAddressRequest, UnifiedFullViewingKey, UnifiedSpendingKey, @@ -337,11 +338,16 @@ impl, P: consensus::Parameters> WalletRead for W } } - fn is_seed_relevant_to_any_derived_accounts( + fn seed_relevance_to_derived_accounts( &self, seed: &SecretVec, - ) -> Result { + ) -> Result, Self::Error> { + let mut has_accounts = false; + let mut has_derived = false; + let mut relevant_account_ids = vec![]; + for account_id in self.get_account_ids()? { + has_accounts = true; let account = self.get_account(account_id)?.expect("account ID exists"); // If the account is imported, the seed _might_ be relevant, but the only @@ -353,6 +359,8 @@ impl, P: consensus::Parameters> WalletRead for W account_index, } = account.source() { + has_derived = true; + if wallet::seed_matches_derived_account( &self.params, seed, @@ -360,14 +368,23 @@ impl, P: consensus::Parameters> WalletRead for W account_index, &account.uivk(), )? { - // The seed is relevant to this account. No need to check any others. - return Ok(true); + // The seed is relevant to this account. + relevant_account_ids.push(account_id); } } } - // The seed was not relevant to any of the accounts in the wallet. - Ok(false) + Ok( + if let Some(account_ids) = NonEmpty::from_vec(relevant_account_ids) { + SeedRelevance::Relevant { account_ids } + } else if has_derived { + SeedRelevance::NotRelevant + } else if has_accounts { + SeedRelevance::NoDerivedAccounts + } else { + SeedRelevance::NoAccounts + }, + ) } fn get_account_for_ufvk( diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 20d01b08b..5772674ea 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -9,7 +9,10 @@ use secrecy::SecretVec; use shardtree::error::ShardTreeError; use uuid::Uuid; -use zcash_client_backend::{data_api::WalletRead, keys::AddressGenerationError}; +use zcash_client_backend::{ + data_api::{SeedRelevance, WalletRead}, + keys::AddressGenerationError, +}; use zcash_primitives::{consensus, transaction::components::amount::BalanceError}; use super::commitment_tree; @@ -134,6 +137,7 @@ fn sqlite_client_error_to_wallet_migration_error(e: SqliteClientError) -> Wallet SqliteClientError::Bech32DecodeError(e) => { WalletMigrationError::CorruptedData(e.to_string()) } + #[cfg(feature = "transparent-inputs")] SqliteClientError::HdwalletError(e) => WalletMigrationError::CorruptedData(e.to_string()), SqliteClientError::TransparentAddress(e) => { WalletMigrationError::CorruptedData(e.to_string()) @@ -155,10 +159,13 @@ fn sqlite_client_error_to_wallet_migration_error(e: SqliteClientError) -> Wallet | SqliteClientError::KeyDerivationError(_) | SqliteClientError::AccountIdDiscontinuity | SqliteClientError::AccountIdOutOfRange - | SqliteClientError::AddressNotRecognized(_) | SqliteClientError::CacheMiss(_) => { unreachable!("we only call WalletRead methods; mutations can't occur") } + #[cfg(feature = "transparent-inputs")] + SqliteClientError::AddressNotRecognized(_) => { + unreachable!("we only call WalletRead methods; mutations can't occur") + } SqliteClientError::AccountUnknown => { unreachable!("all accounts are known in migration context") } @@ -289,11 +296,18 @@ fn init_wallet_db_internal( // Now that the migration succeeded, check whether the seed is relevant to the wallet. if let Some(seed) = seed { - if !wdb - .is_seed_relevant_to_any_derived_accounts(&seed) + match wdb + .seed_relevance_to_derived_accounts(&seed) .map_err(sqlite_client_error_to_wallet_migration_error)? { - return Err(WalletMigrationError::SeedNotRelevant.into()); + SeedRelevance::Relevant { .. } => (), + // Every seed is relevant to a wallet with no accounts; this is most likely a + // new wallet database being initialized for the first time. + SeedRelevance::NoAccounts => (), + // No seed is relevant to a wallet that only has imported accounts. + SeedRelevance::NotRelevant | SeedRelevance::NoDerivedAccounts => { + return Err(WalletMigrationError::SeedNotRelevant.into()) + } } } @@ -1419,12 +1433,16 @@ mod tests { let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap(); assert_matches!(init_wallet_db(&mut db_data, None), Ok(_)); + // Prior to adding any accounts, every seed phrase is relevant to the wallet. let seed = test_vectors::UNIFIED[0].root_seed; + let other_seed = [7; 32]; assert_matches!( init_wallet_db(&mut db_data, Some(Secret::new(seed.to_vec()))), - Err(schemer::MigratorError::Adapter( - WalletMigrationError::SeedNotRelevant - )) + Ok(()) + ); + assert_matches!( + init_wallet_db(&mut db_data, Some(Secret::new(other_seed.to_vec()))), + Ok(()) ); let birthday = AccountBirthday::from_sapling_activation(&network); @@ -1439,6 +1457,18 @@ mod tests { ) ); + // After adding an account, only the real seed phrase is relevant to the wallet. + assert_matches!( + init_wallet_db(&mut db_data, Some(Secret::new(seed.to_vec()))), + Ok(()) + ); + assert_matches!( + init_wallet_db(&mut db_data, Some(Secret::new(other_seed.to_vec()))), + Err(schemer::MigratorError::Adapter( + WalletMigrationError::SeedNotRelevant + )) + ); + for tv in &test_vectors::UNIFIED[..3] { if let Some(Address::Unified(tvua)) = Address::decode(&Network::MainNetwork, tv.unified_addr)