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

zcash_client_sqlite: Always check for seed relevance in `init_wallet_db` 

Closes zcash/librustzcash#1283.
This commit is contained in:
Jack Grigg 2024-03-18 22:42:01 +00:00
parent e6bc21b461
commit 4fa0547b84
4 changed files with 127 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1,6 +1,7 @@
//! Functions for initializing the various databases.
use std::fmt;
use std::rc::Rc;
use schemer::{Migrator, MigratorError};
use schemer_rusqlite::RusqliteAdapter;
@ -8,11 +9,11 @@ use secrecy::SecretVec;
use shardtree::error::ShardTreeError;
use uuid::Uuid;
use zcash_client_backend::keys::AddressGenerationError;
use zcash_client_backend::{data_api::WalletRead, keys::AddressGenerationError};
use zcash_primitives::{consensus, transaction::components::amount::BalanceError};
use super::commitment_tree;
use crate::WalletDb;
use crate::{error::SqliteClientError, WalletDb};
mod migrations;
@ -118,6 +119,58 @@ impl std::error::Error for WalletMigrationError {
}
}
/// Helper to enable calling regular `WalletDb` methods inside the migration code.
///
/// In this context we can know the full set of errors that are generated by any call we
/// make, so we mark errors as unreachable instead of adding new `WalletMigrationError`
/// variants.
fn sqlite_client_error_to_wallet_migration_error(e: SqliteClientError) -> WalletMigrationError {
match e {
SqliteClientError::CorruptedData(e) => WalletMigrationError::CorruptedData(e),
SqliteClientError::Protobuf(e) => WalletMigrationError::CorruptedData(e.to_string()),
SqliteClientError::InvalidNote => {
WalletMigrationError::CorruptedData("invalid note".into())
}
SqliteClientError::Bech32DecodeError(e) => {
WalletMigrationError::CorruptedData(e.to_string())
}
SqliteClientError::HdwalletError(e) => WalletMigrationError::CorruptedData(e.to_string()),
SqliteClientError::TransparentAddress(e) => {
WalletMigrationError::CorruptedData(e.to_string())
}
SqliteClientError::DbError(e) => WalletMigrationError::DbError(e),
SqliteClientError::Io(e) => WalletMigrationError::CorruptedData(e.to_string()),
SqliteClientError::InvalidMemo(e) => WalletMigrationError::CorruptedData(e.to_string()),
SqliteClientError::AddressGeneration(e) => WalletMigrationError::AddressGeneration(e),
SqliteClientError::BadAccountData(e) => WalletMigrationError::CorruptedData(e),
SqliteClientError::CommitmentTree(e) => WalletMigrationError::CommitmentTree(e),
SqliteClientError::UnsupportedPoolType(pool) => WalletMigrationError::CorruptedData(
format!("Wallet DB contains unsupported pool type {}", pool),
),
SqliteClientError::BalanceError(e) => WalletMigrationError::BalanceError(e),
SqliteClientError::TableNotEmpty => unreachable!("wallet already initialized"),
SqliteClientError::BlockConflict(_)
| SqliteClientError::NonSequentialBlocks
| SqliteClientError::RequestedRewindInvalid(_, _)
| SqliteClientError::KeyDerivationError(_)
| SqliteClientError::AccountIdDiscontinuity
| SqliteClientError::AccountIdOutOfRange
| SqliteClientError::AddressNotRecognized(_)
| SqliteClientError::CacheMiss(_) => {
unreachable!("we only call WalletRead methods; mutations can't occur")
}
SqliteClientError::AccountUnknown => {
unreachable!("all accounts are known in migration context")
}
SqliteClientError::UnknownZip32Derivation => {
unreachable!("we don't call methods that require operating on imported accounts")
}
SqliteClientError::ChainHeightUnknown => {
unreachable!("we don't call methods that require a known chain height")
}
}
}
/// Sets up the internal structure of the data database.
///
/// This procedure will automatically perform migration operations to update the wallet database to
@ -126,26 +179,67 @@ impl std::error::Error for WalletMigrationError {
/// operation of this procedure is idempotent, so it is safe (though not required) to invoke this
/// operation every time the wallet is opened.
///
/// In order to correctly apply migrations to accounts derived from a seed, sometimes the
/// optional `seed` argument is required. This function should first be invoked with
/// `seed` set to `None`; if a pending migration requires the seed, the function returns
/// `Err(schemer::MigratorError::Migration { error: WalletMigrationError::SeedRequired, .. })`.
/// The caller can then re-call this function with the necessary seed.
///
/// > Note that currently only one seed can be provided; as such, wallets containing
/// > accounts derived from several different seeds are unsupported, and will result in an
/// > error. Support for multi-seed wallets is being tracked in [zcash/librustzcash#1284].
///
/// When the `seed` argument is provided, the seed is checked against the database for
/// _relevance_: if any account in the wallet for which [`Account::source`] is
/// [`AccountSource::Derived`] can be derived from the given seed, the seed is relevant to
/// the wallet. If the given seed is not relevant, the function returns
/// `Err(schemer::MigratorError::Migration { error: WalletMigrationError::SeedNotRelevant, .. })`
/// or `Err(schemer::MigratorError::Adapter(WalletMigrationError::SeedNotRelevant))`.
///
/// We do not check whether the seed is relevant to any imported account, because that
/// would require brute-forcing the ZIP 32 account index space. Consequentially, imported
/// accounts are not migrated.
///
/// It is safe to use a wallet database previously created without the ability to create
/// transparent spends with a build that enables transparent spends (via use of the
/// `transparent-inputs` feature flag.) The reverse is unsafe, as wallet balance calculations would
/// ignore the transparent UTXOs already controlled by the wallet.
///
/// [zcash/librustzcash#1284]: https://github.com/zcash/librustzcash/issues/1284
/// [`Account::source`]: zcash_client_backend::data_api::Account::source
/// [`AccountSource::Derived`]: zcash_client_backend::data_api::AccountSource::Derived
///
/// # Examples
///
/// ```
/// use secrecy::Secret;
/// use tempfile::NamedTempFile;
/// # use std::error::Error;
/// # use secrecy::SecretVec;
/// # use tempfile::NamedTempFile;
/// use zcash_primitives::consensus::Network;
/// use zcash_client_sqlite::{
/// WalletDb,
/// wallet::init::init_wallet_db,
/// wallet::init::{WalletMigrationError, init_wallet_db},
/// };
///
/// let data_file = NamedTempFile::new().unwrap();
/// let mut db = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap();
/// init_wallet_db(&mut db, Some(Secret::new(vec![]))).unwrap();
/// # fn main() -> Result<(), Box<dyn Error>> {
/// # let data_file = NamedTempFile::new().unwrap();
/// # let get_data_db_path = || data_file.path();
/// # let load_seed = || -> Result<_, String> { Ok(SecretVec::new(vec![])) };
/// let mut db = WalletDb::for_path(get_data_db_path(), Network::TestNetwork)?;
/// match init_wallet_db(&mut db, None) {
/// Err(e)
/// if matches!(
/// e.source().and_then(|e| e.downcast_ref()),
/// Some(&WalletMigrationError::SeedRequired)
/// ) =>
/// {
/// let seed = load_seed()?;
/// init_wallet_db(&mut db, Some(seed))
/// }
/// res => res,
/// }?;
/// # Ok(())
/// # }
/// ```
// TODO: It would be possible to make the transition from providing transparent support to no
// longer providing transparent support safe, by including a migration that verifies that no
@ -165,6 +259,8 @@ fn init_wallet_db_internal<P: consensus::Parameters + 'static>(
seed: Option<SecretVec<u8>>,
target_migrations: &[Uuid],
) -> Result<(), MigratorError<WalletMigrationError>> {
let seed = seed.map(Rc::new);
// Turn off foreign keys, and ensure that table replacement/modification
// does not break views
wdb.conn
@ -178,7 +274,7 @@ fn init_wallet_db_internal<P: consensus::Parameters + 'static>(
let mut migrator = Migrator::new(adapter);
migrator
.register_multiple(migrations::all_migrations(&wdb.params, seed))
.register_multiple(migrations::all_migrations(&wdb.params, seed.clone()))
.expect("Wallet migration registration should have been successful.");
if target_migrations.is_empty() {
migrator.up(None)?;
@ -190,6 +286,17 @@ fn init_wallet_db_internal<P: consensus::Parameters + 'static>(
wdb.conn
.execute("PRAGMA foreign_keys = ON", [])
.map_err(|e| MigratorError::Adapter(WalletMigrationError::from(e)))?;
// 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)
.map_err(sqlite_client_error_to_wallet_migration_error)?
{
return Err(WalletMigrationError::SeedNotRelevant.into());
}
}
Ok(())
}
@ -221,7 +328,7 @@ mod tests {
testing::TestBuilder, wallet::scanning::priority_code, WalletDb, DEFAULT_UA_REQUEST,
};
use super::init_wallet_db;
use super::{init_wallet_db, WalletMigrationError};
#[cfg(feature = "transparent-inputs")]
use {
@ -1310,10 +1417,14 @@ mod tests {
let network = Network::MainNetwork;
let data_file = NamedTempFile::new().unwrap();
let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap();
assert_matches!(init_wallet_db(&mut db_data, None), Ok(_));
let seed = test_vectors::UNIFIED[0].root_seed;
assert_matches!(
init_wallet_db(&mut db_data, Some(Secret::new(seed.to_vec()))),
Ok(_)
Err(schemer::MigratorError::Adapter(
WalletMigrationError::SeedNotRelevant
))
);
let birthday = AccountBirthday::from_sapling_activation(&network);

3
zcash_client_sqlite/src/wallet/init/migrations.rs
View File

@ -32,9 +32,8 @@ use super::WalletMigrationError;
pub(super) fn all_migrations<P: consensus::Parameters + 'static>(
params: &P,
seed: Option<SecretVec<u8>>,
seed: Option<Rc<SecretVec<u8>>>,
) -> Vec<Box<dyn RusqliteMigration<Error = WalletMigrationError>>> {
let seed = Rc::new(seed);
// initial_setup
// / \
// utxos_table ufvk_support

4
zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs
View File

@ -17,7 +17,7 @@ use super::{add_account_birthdays, receiving_key_scopes, v_transactions_note_uni
pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0x1b104345_f27e_42da_a9e3_1de22694da43);
pub(crate) struct Migration<P: consensus::Parameters> {
pub(super) seed: Rc<Option<SecretVec<u8>>>,
pub(super) seed: Option<Rc<SecretVec<u8>>>,
pub(super) params: P,
}
@ -83,7 +83,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
if transaction.query_row("SELECT COUNT(*) FROM accounts", [], |row| {
Ok(row.get::<_, u32>(0)? > 0)
})? {
if let Some(seed) = &self.seed.as_ref() {
if let Some(seed) = &self.seed {
let seed_id = SeedFingerprint::from_seed(seed.expose_secret())
.expect("Seed is between 32 and 252 bytes in length.");

4
zcash_client_sqlite/src/wallet/init/migrations/ufvk_support.rs
View File

@ -31,7 +31,7 @@ pub(super) const MIGRATION_ID: Uuid = Uuid::from_u128(0xbe57ef3b_388e_42ea_97e2_
pub(super) struct Migration<P> {
pub(super) params: P,
pub(super) seed: Rc<Option<SecretVec<u8>>>,
pub(super) seed: Option<Rc<SecretVec<u8>>>,
}
impl<P> schemer::Migration for Migration<P> {
@ -89,7 +89,7 @@ impl<P: consensus::Parameters> RusqliteMigration for Migration<P> {
// We only need to check for the presence of the seed if we have keys that
// need to be migrated; otherwise, it's fine to not supply the seed if this
// migration is being used to initialize an empty database.
if let Some(seed) = &self.seed.as_ref() {
if let Some(seed) = &self.seed {
let account: u32 = row.get(0)?;
let account = AccountId::try_from(account).map_err(|_| {
WalletMigrationError::CorruptedData("Account ID is invalid".to_owned())