From ff8104fa7569a1245919de2172410684329f5495 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 16 Aug 2023 11:15:10 -0600 Subject: [PATCH] zcash_client_sqlite: (testing) Initialize wallets using an `AccountBirthday` struct. --- zcash_client_backend/CHANGELOG.md | 1 + zcash_client_backend/src/data_api.rs | 30 +++++- zcash_client_sqlite/src/chain.rs | 25 ++--- zcash_client_sqlite/src/lib.rs | 21 +++-- zcash_client_sqlite/src/testing.rs | 103 ++++++--------------- zcash_client_sqlite/src/wallet.rs | 8 +- zcash_client_sqlite/src/wallet/sapling.rs | 7 +- zcash_client_sqlite/src/wallet/scanning.rs | 36 +++---- 8 files changed, 104 insertions(+), 127 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index bb0745557..8dfa7c49b 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -11,6 +11,7 @@ and this library adheres to Rust's notion of - `impl Eq for zcash_client_backend::zip321::{Payment, TransactionRequest}` - `impl Debug` for `zcash_client_backend::{data_api::wallet::input_selection::Proposal, wallet::ReceivedSaplingNote}` - `zcash_client_backend::data_api`: + - `AccountBirthday` - `BlockMetadata` - `NoteId` - `NullifierQuery` for use with `WalletRead::get_sapling_nullifiers` diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index d1711375f..10b428555 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use std::fmt::Debug; use std::num::NonZeroU32; -use incrementalmerkletree::Retention; +use incrementalmerkletree::{frontier::Frontier, Retention}; use secrecy::SecretVec; use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree}; use zcash_primitives::{ @@ -12,7 +12,7 @@ use zcash_primitives::{ consensus::BlockHeight, legacy::TransparentAddress, memo::{Memo, MemoBytes}, - sapling, + sapling::{self, Node, NOTE_COMMITMENT_TREE_DEPTH}, transaction::{ components::{amount::Amount, OutPoint}, Transaction, TxId, @@ -466,6 +466,32 @@ impl SentTransactionOutput { } } +#[derive(Clone, Debug)] +pub struct AccountBirthday { + height: BlockHeight, + sapling_frontier: Frontier, +} + +impl AccountBirthday { + pub fn from_parts( + height: BlockHeight, + sapling_frontier: Frontier, + ) -> Self { + Self { + height, + sapling_frontier, + } + } + + pub fn sapling_frontier(&self) -> &Frontier { + &self.sapling_frontier + } + + pub fn height(&self) -> BlockHeight { + self.height + } +} + /// This trait encapsulates the write capabilities required to update stored /// wallet data. pub trait WalletWrite: WalletRead { diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 519f891bb..c2b79467f 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -345,7 +345,7 @@ mod tests { }; use crate::{ - testing::{AddressType, TestBuilder}, + testing::{birthday_at_sapling_activation, AddressType, TestBuilder}, wallet::{get_balance, truncate_to_height}, AccountId, }; @@ -354,8 +354,7 @@ mod tests { fn valid_chain_states() { let mut st = TestBuilder::new() .with_block_cache() - .with_seed(Secret::new(vec![])) - .with_test_account() + .with_test_account(birthday_at_sapling_activation) .build(); let dfvk = st.test_account_sapling().unwrap(); @@ -388,8 +387,7 @@ mod tests { fn invalid_chain_cache_disconnected() { let mut st = TestBuilder::new() .with_block_cache() - .with_seed(Secret::new(vec![])) - .with_test_account() + .with_test_account(birthday_at_sapling_activation) .build(); let dfvk = st.test_account_sapling().unwrap(); @@ -440,8 +438,7 @@ mod tests { fn data_db_truncation() { let mut st = TestBuilder::new() .with_block_cache() - .with_seed(Secret::new(vec![])) - .with_test_account() + .with_test_account(birthday_at_sapling_activation) .build(); let dfvk = st.test_account_sapling().unwrap(); @@ -501,10 +498,7 @@ mod tests { #[test] fn scan_cached_blocks_allows_blocks_out_of_order() { - let mut st = TestBuilder::new() - .with_block_cache() - .with_seed(Secret::new(vec![])) - .build(); + let mut st = TestBuilder::new().with_block_cache().build(); // Add an account to the wallet let seed = Secret::new([0u8; 32].to_vec()); @@ -564,8 +558,7 @@ mod tests { fn scan_cached_blocks_finds_received_notes() { let mut st = TestBuilder::new() .with_block_cache() - .with_seed(Secret::new(vec![])) - .with_test_account() + .with_test_account(birthday_at_sapling_activation) .build(); let dfvk = st.test_account_sapling().unwrap(); @@ -607,8 +600,7 @@ mod tests { fn scan_cached_blocks_finds_change_notes() { let mut st = TestBuilder::new() .with_block_cache() - .with_seed(Secret::new(vec![])) - .with_test_account() + .with_test_account(birthday_at_sapling_activation) .build(); let dfvk = st.test_account_sapling().unwrap(); @@ -653,8 +645,7 @@ mod tests { fn scan_cached_blocks_detects_spends_out_of_order() { let mut st = TestBuilder::new() .with_block_cache() - .with_seed(Secret::new(vec![])) - .with_test_account() + .with_test_account(birthday_at_sapling_activation) .build(); let dfvk = st.test_account_sapling().unwrap(); diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index dde55aa63..93045c4c8 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -1087,7 +1087,10 @@ extern crate assert_matches; mod tests { use zcash_client_backend::data_api::{WalletRead, WalletWrite}; - use crate::{testing::TestBuilder, AccountId}; + use crate::{ + testing::{birthday_at_sapling_activation, TestBuilder}, + AccountId, + }; #[cfg(feature = "unstable")] use zcash_primitives::{consensus::Parameters, transaction::components::Amount}; @@ -1100,7 +1103,9 @@ mod tests { #[test] pub(crate) fn get_next_available_address() { - let mut st = TestBuilder::new().with_test_account().build(); + let mut st = TestBuilder::new() + .with_test_account(birthday_at_sapling_activation) + .build(); let account = AccountId::from(0); let current_addr = st.wallet().get_current_address(account).unwrap(); @@ -1117,17 +1122,15 @@ mod tests { #[cfg(feature = "transparent-inputs")] #[test] fn transparent_receivers() { - use secrecy::Secret; - + // Add an account to the wallet. let st = TestBuilder::new() .with_block_cache() - .with_seed(Secret::new(vec![])) - .with_test_account() + .with_test_account(birthday_at_sapling_activation) .build(); - // Add an account to the wallet. - let (ufvk, taddr) = st.test_account().unwrap(); - let taddr = taddr.unwrap(); + let (_, usk, _) = st.test_account().unwrap(); + let ufvk = usk.to_unified_full_viewing_key(); + let (taddr, _) = usk.default_transparent_address(); let receivers = st.wallet().get_transparent_receivers(0.into()).unwrap(); diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index c1ee2ec2f..e74bcd976 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -8,7 +8,7 @@ use std::fs::File; use prost::Message; use rand_core::{OsRng, RngCore}; use rusqlite::{params, Connection}; -use secrecy::SecretVec; +use secrecy::Secret; use tempfile::NamedTempFile; #[cfg(feature = "unstable")] @@ -25,8 +25,9 @@ use zcash_client_backend::{ input_selection::{GreedyInputSelectorError, InputSelector, Proposal}, propose_transfer, spend, }, + AccountBirthday, WalletWrite, }, - keys::{sapling, UnifiedFullViewingKey, UnifiedSpendingKey}, + keys::UnifiedSpendingKey, proto::compact_formats::{ self as compact, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, }, @@ -56,11 +57,7 @@ use zcash_primitives::{ use crate::{ chain::init::init_cache_database, error::SqliteClientError, - wallet::{ - commitment_tree, - init::{init_accounts_table, init_wallet_db}, - sapling::tests::test_prover, - }, + wallet::{commitment_tree, init::init_wallet_db, sapling::tests::test_prover}, AccountId, ReceivedNoteId, WalletDb, }; @@ -69,10 +66,7 @@ use super::BlockDb; #[cfg(feature = "transparent-inputs")] use { zcash_client_backend::data_api::wallet::{propose_shielding, shield_transparent_funds}, - zcash_primitives::{ - legacy::{self, keys::IncomingViewingKey}, - transaction::components::amount::NonNegativeAmount, - }, + zcash_primitives::transaction::components::amount::NonNegativeAmount, }; #[cfg(feature = "unstable")] @@ -85,8 +79,7 @@ use crate::{ pub(crate) struct TestBuilder { network: Network, cache: Cache, - seed: Option>, - with_test_account: bool, + test_account_birthday: Option, } impl TestBuilder<()> { @@ -95,8 +88,7 @@ impl TestBuilder<()> { TestBuilder { network: Network::TestNetwork, cache: (), - seed: None, - with_test_account: false, + test_account_birthday: None, } } @@ -105,8 +97,7 @@ impl TestBuilder<()> { TestBuilder { network: self.network, cache: BlockCache::new(), - seed: self.seed, - with_test_account: self.with_test_account, + test_account_birthday: self.test_account_birthday, } } @@ -116,22 +107,17 @@ impl TestBuilder<()> { TestBuilder { network: self.network, cache: FsBlockCache::new(), - seed: self.seed, - with_test_account: self.with_test_account, + test_account_birthday: self.test_account_birthday, } } } impl TestBuilder { - /// Gives the test knowledge of the wallet seed for initialization. - pub(crate) fn with_seed(mut self, seed: SecretVec) -> Self { - // TODO remove - self.seed = Some(seed); - self - } - - pub(crate) fn with_test_account(mut self) -> Self { - self.with_test_account = true; + pub(crate) fn with_test_account AccountBirthday>( + mut self, + birthday: F, + ) -> Self { + self.test_account_birthday = Some(birthday(&self.network)); self } @@ -139,11 +125,12 @@ impl TestBuilder { pub(crate) fn build(self) -> TestState { let data_file = NamedTempFile::new().unwrap(); let mut db_data = WalletDb::for_path(data_file.path(), self.network).unwrap(); - init_wallet_db(&mut db_data, self.seed).unwrap(); + init_wallet_db(&mut db_data, None).unwrap(); - let test_account = if self.with_test_account { - // Add an account to the wallet - Some(init_test_accounts_table_ufvk(&mut db_data)) + let test_account = if let Some(birthday) = self.test_account_birthday { + let seed = Secret::new(vec![0u8; 32]); + let (account, usk) = db_data.create_account(&seed).unwrap(); + Some((account, usk, birthday)) } else { None }; @@ -166,7 +153,7 @@ pub(crate) struct TestState { latest_cached_block: Option<(BlockHeight, BlockHash, u32)>, _data_file: NamedTempFile, db_data: WalletDb, - test_account: Option<(UnifiedFullViewingKey, Option)>, + test_account: Option<(AccountId, UnifiedSpendingKey, AccountBirthday)>, } impl TestState @@ -293,8 +280,7 @@ where /// Invokes [`scan_cached_blocks`] with the given arguments, expecting success. pub(crate) fn scan_cached_blocks(&mut self, from_height: BlockHeight, limit: usize) { - self.try_scan_cached_blocks(from_height, limit) - .expect("should succeed for this test"); + assert_matches!(self.try_scan_cached_blocks(from_height, limit), Ok(_)); } /// Invokes [`scan_cached_blocks`] with the given arguments. @@ -344,10 +330,7 @@ impl TestState { } /// Exposes the test account, if enabled via [`TestBuilder::with_test_account`]. - #[cfg(feature = "unstable")] - pub(crate) fn test_account( - &self, - ) -> Option<(UnifiedFullViewingKey, Option)> { + pub(crate) fn test_account(&self) -> Option<(AccountId, UnifiedSpendingKey, AccountBirthday)> { self.test_account.as_ref().cloned() } @@ -355,7 +338,7 @@ impl TestState { pub(crate) fn test_account_sapling(&self) -> Option { self.test_account .as_ref() - .map(|(ufvk, _)| ufvk.sapling().unwrap().clone()) + .and_then(|(_, usk, _)| usk.to_unified_full_viewing_key().sapling().cloned()) } /// Invokes [`create_spend_to_address`] with the given arguments. @@ -561,40 +544,14 @@ impl TestState { } #[cfg(test)] -pub(crate) fn init_test_accounts_table_ufvk( - db_data: &mut WalletDb, -) -> (UnifiedFullViewingKey, Option) { - use std::collections::HashMap; - - let seed = [0u8; 32]; - let account = AccountId::from(0); - let extsk = sapling::spending_key(&seed, db_data.params.coin_type(), account); - let dfvk = extsk.to_diversifiable_full_viewing_key(); - - #[cfg(feature = "transparent-inputs")] - let (tkey, taddr) = { - let tkey = legacy::keys::AccountPrivKey::from_seed(&db_data.params, &seed, account) - .unwrap() - .to_account_pubkey(); - let taddr = tkey.derive_external_ivk().unwrap().default_address().0; - (Some(tkey), Some(taddr)) - }; - - #[cfg(not(feature = "transparent-inputs"))] - let taddr = None; - - let ufvk = UnifiedFullViewingKey::new( - #[cfg(feature = "transparent-inputs")] - tkey, - Some(dfvk), - None, +pub(crate) fn birthday_at_sapling_activation( + params: &P, +) -> AccountBirthday { + use incrementalmerkletree::frontier::Frontier; + AccountBirthday::from_parts( + params.activation_height(NetworkUpgrade::Sapling).unwrap(), + Frontier::empty(), ) - .unwrap(); - - let ufvks = HashMap::from([(account, ufvk.clone())]); - init_accounts_table(db_data, &ufvks).unwrap(); - - (ufvk, taddr) } #[allow(dead_code)] diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 0434c2c6b..e1f8e19c3 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1577,7 +1577,10 @@ mod tests { use zcash_client_backend::data_api::WalletRead; - use crate::{testing::TestBuilder, AccountId}; + use crate::{ + testing::{birthday_at_sapling_activation, TestBuilder}, + AccountId, + }; use super::get_balance; @@ -1595,8 +1598,7 @@ mod tests { #[test] fn empty_database_has_no_balance() { let st = TestBuilder::new() - .with_seed(Secret::new(vec![])) - .with_test_account() + .with_test_account(birthday_at_sapling_activation) .build(); // The account should be empty diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 8dfd29645..db1d126c7 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -609,7 +609,7 @@ pub(crate) mod tests { #[test] fn create_to_address_fails_on_incorrect_usk() { - let mut st = TestBuilder::new().with_seed(Secret::new(vec![])).build(); + let mut st = TestBuilder::new().build(); // Add an account to the wallet let seed = Secret::new([0u8; 32].to_vec()); @@ -778,10 +778,7 @@ pub(crate) mod tests { #[test] fn create_to_address_fails_on_locked_notes() { - let mut st = TestBuilder::new() - .with_block_cache() - .with_seed(Secret::new(vec![])) - .build(); + let mut st = TestBuilder::new().with_block_cache().build(); // Add an account to the wallet let seed = Secret::new([0u8; 32].to_vec()); diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index 48ad6d4ae..ff1d91c85 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -739,6 +739,7 @@ mod tests { use incrementalmerkletree::{Hashable, Level}; use secrecy::Secret; + use zcash_client_backend::data_api::{ chain::CommitmentTreeRoot, scanning::{ScanPriority, ScanRange}, @@ -752,7 +753,7 @@ mod tests { }; use crate::{ - testing::{AddressType, TestBuilder}, + testing::{birthday_at_sapling_activation, AddressType, TestBuilder}, wallet::{init::init_blocks_table, scanning::suggest_scan_ranges}, }; @@ -1085,8 +1086,7 @@ mod tests { let mut st = TestBuilder::new() .with_block_cache() - .with_seed(Secret::new(vec![])) - .with_test_account() + .with_test_account(birthday_at_sapling_activation) .build(); let dfvk = st.test_account_sapling().unwrap(); @@ -1197,23 +1197,21 @@ mod tests { } #[test] - fn init_blocks_table_creates_ignored_range() { + fn create_account_creates_ignored_range() { use ScanPriority::*; - let mut st = TestBuilder::new().with_seed(Secret::new(vec![])).build(); + let mut st = TestBuilder::new().with_block_cache().build(); - let sap_active = st - .wallet() - .params - .activation_height(NetworkUpgrade::Sapling) - .unwrap(); - // Initialise the blocks table. We use Canopy activation as an arbitrary birthday height - // that's greater than Sapling activation. + let sap_active = st.sapling_activation_height(); + + // We use Canopy activation as an arbitrary birthday height that's greater than Sapling + // activation. let birthday_height = st - .wallet() - .params + .network() .activation_height(NetworkUpgrade::Canopy) .unwrap(); + + // call `init_blocks_table` to initialize the scan queue init_blocks_table( st.wallet_mut(), birthday_height, @@ -1223,6 +1221,10 @@ mod tests { ) .unwrap(); + let seed = Secret::new(vec![0u8; 32]); + let (_, usk) = st.wallet_mut().create_account(&seed).unwrap(); + let _dfvk = usk.to_unified_full_viewing_key().sapling().unwrap().clone(); + let expected = vec![ // The range up to and including the wallet's birthday height is ignored. scan_range( @@ -1290,9 +1292,7 @@ mod tests { ), ]; - assert_matches!( - suggest_scan_ranges(&st.wallet().conn, Ignored), - Ok(scan_ranges) if scan_ranges == expected - ); + let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap(); + assert_eq!(actual, expected); } }