zec
/
librustzcash
mirror of https://github.com/zcash/librustzcash.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #1287 from zcash/zcs-distinguish-seed-relevance-with-no-accounts 

Distinguish seed relevance when no derived accounts are present
This commit is contained in:
Kris Nuttycombe 2024-03-19 12:14:50 -06:00 committed by GitHub
parent 5b3ebca351 3c1e82a0c8
commit 35a60abc30
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 91 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
zcash_client_backend/CHANGELOG.md
View File

@ -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`

33
zcash_client_backend/src/data_api.rs
View File

@ -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<u8>,
) -> Result<bool, Self::Error>;
) -> Result<SeedRelevance<Self::AccountId>, 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<A: Copy> {
/// The seed is relevant to at least one derived account within the wallet.
Relevant { account_ids: NonEmpty<A> },
/// 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<u8>,
) -> Result<bool, Self::Error> {
Ok(false)
) -> Result<SeedRelevance<Self::AccountId>, Self::Error> {
Ok(SeedRelevance::NoAccounts)
}
fn get_account_for_ufvk(

2
zcash_client_sqlite/Cargo.toml
View File

@ -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

33
zcash_client_sqlite/src/lib.rs
View File

@ -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<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletRead for W
}
}
fn is_seed_relevant_to_any_derived_accounts(
fn seed_relevance_to_derived_accounts(
&self,
seed: &SecretVec<u8>,
) -> Result<bool, Self::Error> {
) -> Result<SeedRelevance<Self::AccountId>, 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<C: Borrow<rusqlite::Connection>, 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<C: Borrow<rusqlite::Connection>, 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(

46
zcash_client_sqlite/src/wallet/init.rs
View File

@ -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<P: consensus::Parameters + 'static>(
// 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)