From 3be55ae96443f0d376f64510c5c788738a4421d6 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 16 Aug 2023 11:15:10 -0600 Subject: [PATCH 1/4] zcash_client_backend: Add test-only convenience methods for default addresses. --- zcash_client_backend/CHANGELOG.md | 20 +++++++++++--------- zcash_client_backend/src/keys.rs | 17 +++++++++++++++++ zcash_client_sqlite/Cargo.toml | 1 + 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 6980d35ef..bb0745557 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -28,15 +28,17 @@ and this library adheres to Rust's notion of - `ScanError` - `impl ScanningKey for &K` - `impl ScanningKey for (zip32::Scope, sapling::SaplingIvk, sapling::NullifierDerivingKey)` +- Test utility functions `zcash_client_backend::keys::UnifiedSpendingKey::{default_address, + default_transparent_address}` are now available under the `test-dependencies` feature flag. ### Changed - MSRV is now 1.65.0. - Bumped dependencies to `hdwallet 0.4`, `zcash_primitives 0.12`, `zcash_note_encryption 0.4`, `incrementalmerkletree 0.4`, `orchard 0.5`, `bs58 0.5` - `zcash_client_backend::data_api`: - - `WalletRead::TxRef` has been removed in favor of consistently using `TxId` instead. + - `WalletRead::TxRef` has been removed in favor of consistently using `TxId` instead. - `WalletRead::get_transaction` now takes a `TxId` as its argument. - - `WalletWrite::{store_decrypted_tx, store_sent_tx}` now return `Result<(), Self::Error>` + - `WalletWrite::{store_decrypted_tx, store_sent_tx}` now return `Result<(), Self::Error>` as the `WalletRead::TxRef` associated type has been removed. Use `WalletRead::get_transaction` with the transaction's `TxId` instead. - `WalletRead::get_memo` now takes a `NoteId` as its argument instead of `Self::NoteRef` @@ -56,22 +58,22 @@ and this library adheres to Rust's notion of - `wallet::{create_spend_to_address, create_proposed_transaction, shield_transparent_funds}` all now require that `WalletCommitmentTrees` be implemented for the type passed to them for the `wallet_db` parameter. - - `wallet::create_proposed_transaction` now takes an additional + - `wallet::create_proposed_transaction` now takes an additional `min_confirmations` argument. - - `wallet::{spend, create_spend_to_address, shield_transparent_funds, + - `wallet::{spend, create_spend_to_address, shield_transparent_funds, propose_transfer, propose_shielding, create_proposed_transaction}` now take their respective `min_confirmations` arguments as `NonZeroU32` - `wallet::input_selection::InputSelector::{propose_transaction, propose_shielding}` now take their respective `min_confirmations` arguments as `NonZeroU32` - A new `Scan` variant has been added to `data_api::chain::error::Error`. - A new `SyncRequired` variant has been added to `data_api::wallet::input_selection::InputSelectorError`. - - The variants of the `PoolType` enum have changed; the `PoolType::Sapling` variant has been + - The variants of the `PoolType` enum have changed; the `PoolType::Sapling` variant has been removed in favor of a `PoolType::Shielded` variant that wraps a `ShieldedProtocol` value. - `zcash_client_backend::wallet`: - `SpendableNote` has been renamed to `ReceivedSaplingNote`. - Arguments to `WalletSaplingOutput::from_parts` have changed. - `zcash_client_backend::data_api::wallet::input_selection::InputSelector`: - - Arguments to `{propose_transaction, propose_shielding}` have changed. + - Arguments to `{propose_transaction, propose_shielding}` have changed. - `zcash_client_backend::data_api::wallet::{create_spend_to_address, spend, create_proposed_transaction, shield_transparent_funds}` now return the `TxId` for the newly created transaction instead an internal database identifier. @@ -85,7 +87,7 @@ and this library adheres to Rust's notion of tree and incremental witnesses for each previously-known note. In addition, the return type has now been updated to return a `Result`. - `proto/service.proto` has been updated to include the new GRPC endpoints - supported by lightwalletd v0.4.15 + supported by lightwalletd v0.4.15 ### Removed - `zcash_client_backend::data_api`: @@ -99,12 +101,12 @@ and this library adheres to Rust's notion of - `WalletWrite::advance_by_block` (use `WalletWrite::put_blocks` instead). - `PrunedBlock` has been replaced by `ScannedBlock` - `testing::MockWalletDb`, which is available under the `test-dependencies` - feature flag, has been modified by the addition of a `sapling_tree` property. + feature flag, has been modified by the addition of a `sapling_tree` property. - `wallet::input_selection`: - `Proposal::target_height` (use `Proposal::min_target_height` instead). - `zcash_client_backend::data_api::chain::validate_chain` (logic merged into `chain::scan_cached_blocks`. -- `zcash_client_backend::data_api::chain::error::{ChainError, Cause}` have been +- `zcash_client_backend::data_api::chain::error::{ChainError, Cause}` have been replaced by `zcash_client_backend::scanning::ScanError` - `zcash_client_backend::wallet::WalletSaplingOutput::{witness, witness_mut}` have been removed as individual incremental witnesses are no longer tracked on a diff --git a/zcash_client_backend/src/keys.rs b/zcash_client_backend/src/keys.rs index 285e65bb1..a850b618d 100644 --- a/zcash_client_backend/src/keys.rs +++ b/zcash_client_backend/src/keys.rs @@ -14,6 +14,9 @@ use { zcash_primitives::legacy::keys::{self as legacy, IncomingViewingKey}, }; +#[cfg(all(feature = "test-dependencies", feature = "transparent-inputs"))] +use zcash_primitives::legacy::TransparentAddress; + #[cfg(feature = "unstable")] use { byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}, @@ -336,6 +339,20 @@ impl UnifiedSpendingKey { } } } + + #[cfg(feature = "test-dependencies")] + pub fn default_address(&self) -> (UnifiedAddress, DiversifierIndex) { + self.to_unified_full_viewing_key().default_address() + } + + #[cfg(all(feature = "test-dependencies", feature = "transparent-inputs"))] + pub fn default_transparent_address(&self) -> (TransparentAddress, u32) { + self.transparent() + .to_account_pubkey() + .derive_external_ivk() + .unwrap() + .default_address() + } } /// A [ZIP 316](https://zips.z.cash/zip-0316) unified full viewing key. diff --git a/zcash_client_sqlite/Cargo.toml b/zcash_client_sqlite/Cargo.toml index 0d409717a..3494b1996 100644 --- a/zcash_client_sqlite/Cargo.toml +++ b/zcash_client_sqlite/Cargo.toml @@ -60,6 +60,7 @@ tempfile = "3.5.0" zcash_note_encryption = "0.4" zcash_proofs = { version = "0.12", path = "../zcash_proofs" } zcash_primitives = { version = "0.12", path = "../zcash_primitives", features = ["test-dependencies"] } +zcash_client_backend = { version = "0.9", path = "../zcash_client_backend", features = ["test-dependencies"] } zcash_address = { version = "0.3", path = "../components/zcash_address", features = ["test-dependencies"] } [features] From a238007e14627caf2bb04c6e517f59df7680bfec Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 16 Aug 2023 11:15:10 -0600 Subject: [PATCH 2/4] zcash_client_sqlite: Remove `testing::network` global accessor function. --- zcash_client_sqlite/src/testing.rs | 112 ++++++++++-------- .../src/wallet/commitment_tree.rs | 8 +- zcash_client_sqlite/src/wallet/init.rs | 85 +++++++------ .../init/migrations/add_transaction_views.rs | 21 ++-- .../migrations/received_notes_nullable_nf.rs | 9 +- .../init/migrations/v_transactions_net.rs | 13 +- zcash_client_sqlite/src/wallet/sapling.rs | 14 +-- zcash_client_sqlite/src/wallet/scanning.rs | 17 +-- 8 files changed, 145 insertions(+), 134 deletions(-) diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index 84eb383ed..c1ee2ec2f 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -1,6 +1,6 @@ use std::convert::Infallible; use std::fmt; -use std::{collections::HashMap, num::NonZeroU32}; +use std::num::NonZeroU32; #[cfg(feature = "unstable")] use std::fs::File; @@ -15,14 +15,13 @@ use tempfile::NamedTempFile; use tempfile::TempDir; #[allow(deprecated)] -use zcash_client_backend::data_api::wallet::create_spend_to_address; use zcash_client_backend::{ address::RecipientAddress, data_api::{ self, chain::{scan_cached_blocks, BlockSource}, wallet::{ - create_proposed_transaction, + create_proposed_transaction, create_spend_to_address, input_selection::{GreedyInputSelectorError, InputSelector, Proposal}, propose_transfer, spend, }, @@ -37,7 +36,7 @@ use zcash_client_backend::{ use zcash_note_encryption::Domain; use zcash_primitives::{ block::BlockHash, - consensus::{BlockHeight, Network, NetworkUpgrade, Parameters}, + consensus::{self, BlockHeight, Network, NetworkUpgrade, Parameters}, legacy::TransparentAddress, memo::MemoBytes, sapling::{ @@ -54,13 +53,6 @@ use zcash_primitives::{ zip32::{sapling::DiversifiableFullViewingKey, DiversifierIndex}, }; -#[cfg(feature = "transparent-inputs")] -use zcash_client_backend::data_api::wallet::{propose_shielding, shield_transparent_funds}; -#[cfg(feature = "transparent-inputs")] -use zcash_primitives::{ - legacy, legacy::keys::IncomingViewingKey, transaction::components::amount::NonNegativeAmount, -}; - use crate::{ chain::init::init_cache_database, error::SqliteClientError, @@ -74,13 +66,24 @@ use crate::{ 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, + }, +}; + #[cfg(feature = "unstable")] use crate::{ chain::{init::init_blockmeta_db, BlockMeta}, FsBlockDb, }; + /// A builder for a `zcash_client_sqlite` test. pub(crate) struct TestBuilder { + network: Network, cache: Cache, seed: Option>, with_test_account: bool, @@ -90,6 +93,7 @@ impl TestBuilder<()> { /// Constructs a new test. pub(crate) fn new() -> Self { TestBuilder { + network: Network::TestNetwork, cache: (), seed: None, with_test_account: false, @@ -99,6 +103,7 @@ impl TestBuilder<()> { /// Adds a [`BlockDb`] cache to the test. pub(crate) fn with_block_cache(self) -> TestBuilder { TestBuilder { + network: self.network, cache: BlockCache::new(), seed: self.seed, with_test_account: self.with_test_account, @@ -109,6 +114,7 @@ impl TestBuilder<()> { #[cfg(feature = "unstable")] pub(crate) fn with_fs_block_cache(self) -> TestBuilder { TestBuilder { + network: self.network, cache: FsBlockCache::new(), seed: self.seed, with_test_account: self.with_test_account, @@ -131,10 +137,8 @@ impl TestBuilder { /// Builds the state for this test. pub(crate) fn build(self) -> TestState { - let params = network(); - let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), params).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), self.network).unwrap(); init_wallet_db(&mut db_data, self.seed).unwrap(); let test_account = if self.with_test_account { @@ -145,7 +149,7 @@ impl TestBuilder { }; TestState { - params, + params: self.network, cache: self.cache, latest_cached_block: None, _data_file: data_file, @@ -186,7 +190,15 @@ where let (height, prev_hash, initial_sapling_tree_size) = self .latest_cached_block .map(|(prev_height, prev_hash, end_size)| (prev_height + 1, prev_hash, end_size)) - .unwrap_or_else(|| (sapling_activation_height(), BlockHash([0; 32]), 0)); + .unwrap_or_else(|| { + ( + self.params + .activation_height(NetworkUpgrade::Sapling) + .unwrap(), + BlockHash([0; 32]), + 0, + ) + }); let (res, nf) = self.generate_block_at( height, @@ -215,6 +227,7 @@ where initial_sapling_tree_size: u32, ) -> (Cache::InsertResult, Nullifier) { let (cb, nf) = fake_compact_block( + &self.params, height, prev_hash, dfvk, @@ -246,9 +259,18 @@ where let (height, prev_hash, initial_sapling_tree_size) = self .latest_cached_block .map(|(prev_height, prev_hash, end_size)| (prev_height + 1, prev_hash, end_size)) - .unwrap_or_else(|| (sapling_activation_height(), BlockHash([0; 32]), 0)); + .unwrap_or_else(|| { + ( + self.params + .activation_height(NetworkUpgrade::Sapling) + .unwrap(), + BlockHash([0; 32]), + 0, + ) + }); let cb = fake_compact_block_spending( + &self.params, height, prev_hash, note, @@ -308,6 +330,19 @@ impl TestState { &mut self.db_data } + /// Exposes an immutable reference to the network in use. + pub(crate) fn network(&self) -> &Network { + &self.db_data.params + } + + /// Convenience method for obtaining the Sapling activation height for the network under test. + pub(crate) fn sapling_activation_height(&self) -> BlockHeight { + self.db_data + .params + .activation_height(NetworkUpgrade::Sapling) + .expect("Sapling activation height must be known.") + } + /// Exposes the test account, if enabled via [`TestBuilder::with_test_account`]. #[cfg(feature = "unstable")] pub(crate) fn test_account( @@ -525,42 +560,20 @@ impl TestState { } } -#[cfg(feature = "mainnet")] -pub(crate) fn network() -> Network { - Network::MainNetwork -} - -#[cfg(not(feature = "mainnet"))] -pub(crate) fn network() -> Network { - Network::TestNetwork -} - -#[cfg(feature = "mainnet")] -pub(crate) fn sapling_activation_height() -> BlockHeight { - Network::MainNetwork - .activation_height(NetworkUpgrade::Sapling) - .unwrap() -} - -#[cfg(not(feature = "mainnet"))] -pub(crate) fn sapling_activation_height() -> BlockHeight { - Network::TestNetwork - .activation_height(NetworkUpgrade::Sapling) - .unwrap() -} - #[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, network().coin_type(), account); + 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(&network(), &seed, account) + 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; @@ -593,7 +606,8 @@ pub(crate) enum AddressType { /// Create a fake CompactBlock at the given height, containing a single output paying /// an address. Returns the CompactBlock and the nullifier for the new note. -pub(crate) fn fake_compact_block( +pub(crate) fn fake_compact_block( + params: &P, height: BlockHeight, prev_hash: BlockHash, dfvk: &DiversifiableFullViewingKey, @@ -609,7 +623,7 @@ pub(crate) fn fake_compact_block( // Create a fake Note for the account let mut rng = OsRng; - let rseed = generate_random_rseed(&network(), height, &mut rng); + let rseed = generate_random_rseed(params, height, &mut rng); let note = Note::from_parts(to, NoteValue::from_raw(value.into()), rseed); let encryptor = sapling_note_encryption::<_, Network>( Some(dfvk.fvk().ovk), @@ -655,7 +669,9 @@ pub(crate) fn fake_compact_block( /// Create a fake CompactBlock at the given height, spending a single note from the /// given address. -pub(crate) fn fake_compact_block_spending( +#[allow(clippy::too_many_arguments)] +pub(crate) fn fake_compact_block_spending( + params: &P, height: BlockHeight, prev_hash: BlockHash, (nf, in_value): (Nullifier, Amount), @@ -665,7 +681,7 @@ pub(crate) fn fake_compact_block_spending( initial_sapling_tree_size: u32, ) -> CompactBlock { let mut rng = OsRng; - let rseed = generate_random_rseed(&network(), height, &mut rng); + let rseed = generate_random_rseed(params, height, &mut rng); // Create a fake CompactBlock containing the note let cspend = CompactSaplingSpend { nf: nf.to_vec() }; @@ -700,7 +716,7 @@ pub(crate) fn fake_compact_block_spending( // Create a fake Note for the change ctx.outputs.push({ let change_addr = dfvk.default_address().1; - let rseed = generate_random_rseed(&network(), height, &mut rng); + let rseed = generate_random_rseed(params, height, &mut rng); let note = Note::from_parts( change_addr, NoteValue::from_raw((in_value - value).unwrap().into()), diff --git a/zcash_client_sqlite/src/wallet/commitment_tree.rs b/zcash_client_sqlite/src/wallet/commitment_tree.rs index 324208071..01302dca0 100644 --- a/zcash_client_sqlite/src/wallet/commitment_tree.rs +++ b/zcash_client_sqlite/src/wallet/commitment_tree.rs @@ -985,14 +985,14 @@ mod tests { }; use shardtree::ShardTree; use zcash_client_backend::data_api::chain::CommitmentTreeRoot; - use zcash_primitives::consensus::BlockHeight; + use zcash_primitives::consensus::{BlockHeight, Network}; use super::SqliteShardStore; - use crate::{testing, wallet::init::init_wallet_db, WalletDb, SAPLING_TABLES_PREFIX}; + use crate::{wallet::init::init_wallet_db, WalletDb, SAPLING_TABLES_PREFIX}; fn new_tree(m: usize) -> ShardTree, 4, 3> { let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); data_file.keep().unwrap(); init_wallet_db(&mut db_data, None).unwrap(); @@ -1040,7 +1040,7 @@ mod tests { #[test] fn put_shard_roots() { let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); data_file.keep().unwrap(); init_wallet_db(&mut db_data, None).unwrap(); diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 40913504c..354016fe4 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -386,36 +386,27 @@ mod tests { use zcash_primitives::{ block::BlockHash, - consensus::{BlockHeight, BranchId, NetworkUpgrade, Parameters}, + consensus::{self, BlockHeight, BranchId, Network, NetworkUpgrade, Parameters}, transaction::{TransactionData, TxVersion}, zip32::sapling::ExtendedFullViewingKey, }; - use crate::{ - error::SqliteClientError, - testing::{self, network}, - wallet::scanning::priority_code, - AccountId, WalletDb, - }; + use crate::{error::SqliteClientError, wallet::scanning::priority_code, AccountId, WalletDb}; use super::{init_accounts_table, init_blocks_table, init_wallet_db}; #[cfg(feature = "transparent-inputs")] use { - crate::{ - wallet::{self, pool_code, PoolType}, - WalletWrite, - }, + crate::wallet::{self, pool_code, PoolType}, zcash_address::test_vectors, - zcash_primitives::{ - consensus::Network, legacy::keys as transparent, zip32::DiversifierIndex, - }, + zcash_client_backend::data_api::WalletWrite, + zcash_primitives::{legacy::keys as transparent, zip32::DiversifierIndex}, }; #[test] fn verify_schema() { let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); init_wallet_db(&mut db_data, None).unwrap(); use regex::Regex; @@ -609,7 +600,7 @@ mod tests { AND (scan_queue.block_range_end - 1) >= shard.subtree_end_height ) WHERE scan_queue.priority > {}", - u32::from(testing::network().activation_height(NetworkUpgrade::Sapling).unwrap()), + u32::from(db_data.params.activation_height(NetworkUpgrade::Sapling).unwrap()), priority_code(&ScanPriority::Scanned), ), // v_transactions @@ -768,7 +759,7 @@ mod tests { #[test] fn init_migrate_from_0_3_0() { - fn init_0_3_0

( + fn init_0_3_0( wdb: &mut WalletDb, extfvk: &ExtendedFullViewingKey, account: AccountId, @@ -852,11 +843,11 @@ mod tests { )?; let address = encode_payment_address( - testing::network().hrp_sapling_payment_address(), + wdb.params.hrp_sapling_payment_address(), &extfvk.default_address().1, ); let extfvk = encode_extended_full_viewing_key( - testing::network().hrp_sapling_extended_full_viewing_key(), + wdb.params.hrp_sapling_extended_full_viewing_key(), extfvk, ); wdb.conn.execute( @@ -872,19 +863,21 @@ mod tests { Ok(()) } + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); + let seed = [0xab; 32]; let account = AccountId::from(0); - let secret_key = sapling::spending_key(&seed, testing::network().coin_type(), account); + let secret_key = sapling::spending_key(&seed, db_data.params.coin_type(), account); let extfvk = secret_key.to_extended_full_viewing_key(); - let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + init_0_3_0(&mut db_data, &extfvk, account).unwrap(); init_wallet_db(&mut db_data, Some(Secret::new(seed.to_vec()))).unwrap(); } #[test] fn init_migrate_from_autoshielding_poc() { - fn init_autoshielding

( + fn init_autoshielding( wdb: &mut WalletDb, extfvk: &ExtendedFullViewingKey, account: AccountId, @@ -984,11 +977,11 @@ mod tests { )?; let address = encode_payment_address( - testing::network().hrp_sapling_payment_address(), + wdb.params.hrp_sapling_payment_address(), &extfvk.default_address().1, ); let extfvk = encode_extended_full_viewing_key( - testing::network().hrp_sapling_extended_full_viewing_key(), + wdb.params.hrp_sapling_extended_full_viewing_key(), extfvk, ); wdb.conn.execute( @@ -1038,19 +1031,21 @@ mod tests { Ok(()) } + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); + let seed = [0xab; 32]; let account = AccountId::from(0); - let secret_key = sapling::spending_key(&seed, testing::network().coin_type(), account); + let secret_key = sapling::spending_key(&seed, db_data.params.coin_type(), account); let extfvk = secret_key.to_extended_full_viewing_key(); - let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + init_autoshielding(&mut db_data, &extfvk, account).unwrap(); init_wallet_db(&mut db_data, Some(Secret::new(seed.to_vec()))).unwrap(); } #[test] fn init_migrate_from_main_pre_migrations() { - fn init_main

( + fn init_main( wdb: &mut WalletDb, ufvk: &UnifiedFullViewingKey, account: AccountId, @@ -1150,9 +1145,9 @@ mod tests { [], )?; - let ufvk_str = ufvk.encode(&testing::network()); + let ufvk_str = ufvk.encode(&wdb.params); let address_str = - RecipientAddress::Unified(ufvk.default_address().0).encode(&testing::network()); + RecipientAddress::Unified(ufvk.default_address().0).encode(&wdb.params); wdb.conn.execute( "INSERT INTO accounts (account, ufvk, address, transparent_address) VALUES (?, ?, ?, '')", @@ -1168,7 +1163,7 @@ mod tests { { let taddr = RecipientAddress::Transparent(*ufvk.default_address().0.transparent().unwrap()) - .encode(&testing::network()); + .encode(&wdb.params); wdb.conn.execute( "INSERT INTO blocks (height, hash, time, sapling_tree) VALUES (0, 0, 0, x'000000')", [], @@ -1186,12 +1181,13 @@ mod tests { Ok(()) } + let data_file = NamedTempFile::new().unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); + let seed = [0xab; 32]; let account = AccountId::from(0); - let secret_key = - UnifiedSpendingKey::from_seed(&testing::network(), &seed, account).unwrap(); - let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let secret_key = UnifiedSpendingKey::from_seed(&db_data.params, &seed, account).unwrap(); + init_main( &mut db_data, &secret_key.to_unified_full_viewing_key(), @@ -1204,7 +1200,7 @@ mod tests { #[test] fn init_accounts_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // We can call the function as many times as we want with no data @@ -1215,13 +1211,13 @@ mod tests { let account = AccountId::from(0); // First call with data should initialise the accounts table - let extsk = sapling::spending_key(&seed, network().coin_type(), account); + 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 ufvk = UnifiedFullViewingKey::new( Some( - transparent::AccountPrivKey::from_seed(&network(), &seed, account) + transparent::AccountPrivKey::from_seed(&db_data.params, &seed, account) .unwrap() .to_account_pubkey(), ), @@ -1243,8 +1239,9 @@ mod tests { #[test] fn init_accounts_table_allows_no_gaps() { + let params = Network::TestNetwork; let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), params).unwrap(); init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // allow sequential initialization @@ -1253,7 +1250,7 @@ mod tests { ids.iter() .map(|a| { let account = AccountId::from(*a); - UnifiedSpendingKey::from_seed(&network(), &seed, account) + UnifiedSpendingKey::from_seed(¶ms, &seed, account) .map(|k| (account, k.to_unified_full_viewing_key())) .unwrap() }) @@ -1273,7 +1270,7 @@ mod tests { #[test] fn init_blocks_table_only_works_once() { let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); // First call with data should initialise the blocks table @@ -1300,14 +1297,14 @@ mod tests { #[test] fn init_accounts_table_stores_correct_address() { let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); init_wallet_db(&mut db_data, None).unwrap(); let seed = [0u8; 32]; // Add an account to the wallet let account_id = AccountId::from(0); - let usk = UnifiedSpendingKey::from_seed(&testing::network(), &seed, account_id).unwrap(); + let usk = UnifiedSpendingKey::from_seed(&db_data.params, &seed, account_id).unwrap(); let ufvk = usk.to_unified_full_viewing_key(); let expected_address = ufvk.sapling().unwrap().default_address().1; let ufvks = HashMap::from([(account_id, ufvk)]); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs b/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs index 0d217c545..05ab54aed 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/add_transaction_views.rs @@ -283,10 +283,9 @@ mod tests { use tempfile::NamedTempFile; use zcash_client_backend::keys::UnifiedSpendingKey; - use zcash_primitives::zip32::AccountId; + use zcash_primitives::{consensus::Network, zip32::AccountId}; use crate::{ - testing, wallet::init::{init_wallet_db_internal, migrations::addresses_table}, WalletDb, }; @@ -310,19 +309,19 @@ mod tests { #[test] fn transaction_views() { + let network = Network::TestNetwork; let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap(); init_wallet_db_internal(&mut db_data, None, &[addresses_table::MIGRATION_ID]).unwrap(); let usk = - UnifiedSpendingKey::from_seed(&testing::network(), &[0u8; 32][..], AccountId::from(0)) - .unwrap(); + UnifiedSpendingKey::from_seed(&network, &[0u8; 32][..], AccountId::from(0)).unwrap(); let ufvk = usk.to_unified_full_viewing_key(); db_data .conn .execute( "INSERT INTO accounts (account, ufvk) VALUES (0, ?)", - params![ufvk.encode(&testing::network())], + params![ufvk.encode(&network)], ) .unwrap(); @@ -402,8 +401,9 @@ mod tests { #[test] #[cfg(feature = "transparent-inputs")] fn migrate_from_wm2() { + let network = Network::TestNetwork; let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), network).unwrap(); init_wallet_db_internal( &mut db_data, None, @@ -440,8 +440,7 @@ mod tests { tx.write(&mut tx_bytes).unwrap(); let usk = - UnifiedSpendingKey::from_seed(&testing::network(), &[0u8; 32][..], AccountId::from(0)) - .unwrap(); + UnifiedSpendingKey::from_seed(&network, &[0u8; 32][..], AccountId::from(0)).unwrap(); let ufvk = usk.to_unified_full_viewing_key(); let (ua, _) = ufvk.default_address(); let taddr = ufvk @@ -451,11 +450,11 @@ mod tests { .ok() .map(|k| k.derive_address(0).unwrap()) }) - .map(|a| a.encode(&testing::network())); + .map(|a| a.encode(&network)); db_data.conn.execute( "INSERT INTO accounts (account, ufvk, address, transparent_address) VALUES (0, ?, ?, ?)", - params![ufvk.encode(&testing::network()), ua.encode(&testing::network()), &taddr] + params![ufvk.encode(&network), ua.encode(&network), &taddr] ).unwrap(); db_data .conn diff --git a/zcash_client_sqlite/src/wallet/init/migrations/received_notes_nullable_nf.rs b/zcash_client_sqlite/src/wallet/init/migrations/received_notes_nullable_nf.rs index 2c946f3aa..5a856dc3d 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/received_notes_nullable_nf.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/received_notes_nullable_nf.rs @@ -233,10 +233,9 @@ mod tests { use tempfile::NamedTempFile; use zcash_client_backend::keys::UnifiedSpendingKey; - use zcash_primitives::zip32::AccountId; + use zcash_primitives::{consensus::Network, zip32::AccountId}; use crate::{ - testing, wallet::init::{init_wallet_db_internal, migrations::v_transactions_net}, WalletDb, }; @@ -244,19 +243,19 @@ mod tests { #[test] fn received_notes_nullable_migration() { let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); init_wallet_db_internal(&mut db_data, None, &[v_transactions_net::MIGRATION_ID]).unwrap(); // Create an account in the wallet let usk0 = - UnifiedSpendingKey::from_seed(&testing::network(), &[0u8; 32][..], AccountId::from(0)) + UnifiedSpendingKey::from_seed(&db_data.params, &[0u8; 32][..], AccountId::from(0)) .unwrap(); let ufvk0 = usk0.to_unified_full_viewing_key(); db_data .conn .execute( "INSERT INTO accounts (account, ufvk) VALUES (0, ?)", - params![ufvk0.encode(&testing::network())], + params![ufvk0.encode(&db_data.params)], ) .unwrap(); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs index b6bfe4a19..14cd830b1 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_net.rs @@ -212,10 +212,9 @@ mod tests { use tempfile::NamedTempFile; use zcash_client_backend::keys::UnifiedSpendingKey; - use zcash_primitives::zip32::AccountId; + use zcash_primitives::{consensus::Network, zip32::AccountId}; use crate::{ - testing, wallet::init::{init_wallet_db_internal, migrations::add_transaction_views}, WalletDb, }; @@ -223,32 +222,32 @@ mod tests { #[test] fn v_transactions_net() { let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), testing::network()).unwrap(); + let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); init_wallet_db_internal(&mut db_data, None, &[add_transaction_views::MIGRATION_ID]) .unwrap(); // Create two accounts in the wallet. let usk0 = - UnifiedSpendingKey::from_seed(&testing::network(), &[0u8; 32][..], AccountId::from(0)) + UnifiedSpendingKey::from_seed(&db_data.params, &[0u8; 32][..], AccountId::from(0)) .unwrap(); let ufvk0 = usk0.to_unified_full_viewing_key(); db_data .conn .execute( "INSERT INTO accounts (account, ufvk) VALUES (0, ?)", - params![ufvk0.encode(&testing::network())], + params![ufvk0.encode(&db_data.params)], ) .unwrap(); let usk1 = - UnifiedSpendingKey::from_seed(&testing::network(), &[1u8; 32][..], AccountId::from(1)) + UnifiedSpendingKey::from_seed(&db_data.params, &[1u8; 32][..], AccountId::from(1)) .unwrap(); let ufvk1 = usk1.to_unified_full_viewing_key(); db_data .conn .execute( "INSERT INTO accounts (account, ufvk) VALUES (1, ?)", - params![ufvk1.encode(&testing::network())], + params![ufvk1.encode(&db_data.params)], ) .unwrap(); diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index edf4ac205..8dfd29645 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -439,7 +439,7 @@ pub(crate) mod tests { use crate::{ error::SqliteClientError, - testing::{self, network, AddressType, BlockCache, TestBuilder, TestState}, + testing::{AddressType, BlockCache, TestBuilder, TestState}, wallet::{commitment_tree, get_balance, get_balance_at}, AccountId, NoteId, ReceivedNoteId, }; @@ -532,7 +532,7 @@ pub(crate) mod tests { let ufvks = [(account, usk.to_unified_full_viewing_key())] .into_iter() .collect(); - let decrypted_outputs = decrypt_transaction(&testing::network(), h + 1, &tx, &ufvks); + let decrypted_outputs = decrypt_transaction(st.network(), h + 1, &tx, &ufvks); assert_eq!(decrypted_outputs.len(), 2); let mut found_tx_change_memo = false; @@ -619,7 +619,7 @@ pub(crate) mod tests { // Create a USK that doesn't exist in the wallet let acct1 = AccountId::from(1); - let usk1 = UnifiedSpendingKey::from_seed(&network(), &[1u8; 32], acct1).unwrap(); + let usk1 = UnifiedSpendingKey::from_seed(st.network(), &[1u8; 32], acct1).unwrap(); // Attempting to spend with a USK that is not in the wallet results in an error assert_matches!( @@ -900,7 +900,7 @@ pub(crate) mod tests { let to = addr2.into(); #[allow(clippy::type_complexity)] - let send_and_recover_with_policy = |test: &mut TestState, + let send_and_recover_with_policy = |st: &mut TestState, ovk_policy| -> Result< Option<(Note, PaymentAddress, MemoBytes)>, @@ -912,7 +912,7 @@ pub(crate) mod tests { ReceivedNoteId, >, > { - let txid = test.create_spend_to_address( + let txid = st.create_spend_to_address( &usk, &to, Amount::from_u64(15000).unwrap(), @@ -922,7 +922,7 @@ pub(crate) mod tests { )?; // Fetch the transaction from the database - let raw_tx: Vec<_> = test + let raw_tx: Vec<_> = st .wallet() .conn .query_row( @@ -937,7 +937,7 @@ pub(crate) mod tests { for output in tx.sapling_bundle().unwrap().shielded_outputs() { // Find the output that decrypts with the external OVK let result = try_sapling_output_recovery( - &network(), + st.network(), h1, &dfvk.to_ovk(Scope::External), output, diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index 25a1be909..48ad6d4ae 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -752,7 +752,7 @@ mod tests { }; use crate::{ - testing::{sapling_activation_height, AddressType, TestBuilder}, + testing::{AddressType, TestBuilder}, wallet::{init::init_blocks_table, scanning::suggest_scan_ranges}, }; @@ -1090,6 +1090,7 @@ mod tests { .build(); let dfvk = st.test_account_sapling().unwrap(); + let sapling_activation_height = st.sapling_activation_height(); assert_matches!( // In the following, we don't care what the root hashes are, they just need to be @@ -1098,15 +1099,15 @@ mod tests { 0, &[ CommitmentTreeRoot::from_parts( - sapling_activation_height() + 100, + sapling_activation_height + 100, Node::empty_root(Level::from(0)) ), CommitmentTreeRoot::from_parts( - sapling_activation_height() + 200, + sapling_activation_height + 200, Node::empty_root(Level::from(1)) ), CommitmentTreeRoot::from_parts( - sapling_activation_height() + 300, + sapling_activation_height + 300, Node::empty_root(Level::from(2)) ), ] @@ -1118,7 +1119,7 @@ mod tests { // of 10 blocks. After `scan_cached_blocks`, the scan queue should have a requested scan // range of 300..310 with `FoundNote` priority, 310..320 with `Scanned` priority. let initial_sapling_tree_size = (0x1 << 16) * 3 + 5; - let initial_height = sapling_activation_height() + 310; + let initial_height = sapling_activation_height + 310; let value = Amount::from_u64(50000).unwrap(); st.generate_block_at( @@ -1141,7 +1142,7 @@ mod tests { st.scan_cached_blocks(initial_height, 10); // Verify the that adjacent range needed to make the note spendable has been prioritized. - let sap_active = u32::from(sapling_activation_height()); + let sap_active = u32::from(sapling_activation_height); assert_matches!( st.wallet().suggest_scan_ranges(), Ok(scan_ranges) if scan_ranges == vec![ @@ -1162,7 +1163,7 @@ mod tests { // future. assert_matches!( st.wallet_mut() - .update_chain_tip(sapling_activation_height() + 340), + .update_chain_tip(sapling_activation_height + 340), Ok(()) ); @@ -1179,7 +1180,7 @@ mod tests { // Now simulate a jump ahead more than 100 blocks. assert_matches!( st.wallet_mut() - .update_chain_tip(sapling_activation_height() + 450), + .update_chain_tip(sapling_activation_height + 450), Ok(()) ); From ff8104fa7569a1245919de2172410684329f5495 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 16 Aug 2023 11:15:10 -0600 Subject: [PATCH 3/4] 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); } } From 22b6ddd7544f9a8da8d5069b28feb7901f7d6fde Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Tue, 29 Aug 2023 16:32:18 -0600 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: str4d --- zcash_client_backend/src/data_api.rs | 11 +++++ zcash_client_sqlite/src/testing.rs | 52 +++++++++------------- zcash_client_sqlite/src/wallet/init.rs | 17 ++++--- zcash_client_sqlite/src/wallet/sapling.rs | 6 +-- zcash_client_sqlite/src/wallet/scanning.rs | 2 +- 5 files changed, 45 insertions(+), 43 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 10b428555..cda26813b 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -466,6 +466,8 @@ impl SentTransactionOutput { } } +/// A data structure used to set the birthday height for an account, and ensure that the initial +/// note commitment tree state is recorded at that height. #[derive(Clone, Debug)] pub struct AccountBirthday { height: BlockHeight, @@ -473,6 +475,12 @@ pub struct AccountBirthday { } impl AccountBirthday { + /// Constructs a new [`AccountBirthday`] from its constituent parts. + /// + /// * `height`: The birthday height of the account. This is defined as the height of the last + /// block that is known to contain no transactions sent to addresses belonging to the account. + /// * `sapling_frontier`: The Sapling note commitment tree frontier as of the end of the block + /// at `height`. pub fn from_parts( height: BlockHeight, sapling_frontier: Frontier, @@ -483,10 +491,13 @@ impl AccountBirthday { } } + /// Returns the Sapling note commitment tree frontier as of the end of the block at + /// [`Self::height`]. pub fn sapling_frontier(&self) -> &Frontier { &self.sapling_frontier } + /// Returns the birthday height of the account. pub fn height(&self) -> BlockHeight { self.height } diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index e74bcd976..b98e546ec 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -136,7 +136,6 @@ impl TestBuilder { }; TestState { - params: self.network, cache: self.cache, latest_cached_block: None, _data_file: data_file, @@ -148,7 +147,6 @@ impl TestBuilder { /// The state for a `zcash_client_sqlite` test. pub(crate) struct TestState { - params: Network, cache: Cache, latest_cached_block: Option<(BlockHeight, BlockHash, u32)>, _data_file: NamedTempFile, @@ -177,15 +175,7 @@ where let (height, prev_hash, initial_sapling_tree_size) = self .latest_cached_block .map(|(prev_height, prev_hash, end_size)| (prev_height + 1, prev_hash, end_size)) - .unwrap_or_else(|| { - ( - self.params - .activation_height(NetworkUpgrade::Sapling) - .unwrap(), - BlockHash([0; 32]), - 0, - ) - }); + .unwrap_or_else(|| (self.sapling_activation_height(), BlockHash([0; 32]), 0)); let (res, nf) = self.generate_block_at( height, @@ -214,7 +204,7 @@ where initial_sapling_tree_size: u32, ) -> (Cache::InsertResult, Nullifier) { let (cb, nf) = fake_compact_block( - &self.params, + &self.network(), height, prev_hash, dfvk, @@ -246,18 +236,10 @@ where let (height, prev_hash, initial_sapling_tree_size) = self .latest_cached_block .map(|(prev_height, prev_hash, end_size)| (prev_height + 1, prev_hash, end_size)) - .unwrap_or_else(|| { - ( - self.params - .activation_height(NetworkUpgrade::Sapling) - .unwrap(), - BlockHash([0; 32]), - 0, - ) - }); + .unwrap_or_else(|| (self.sapling_activation_height(), BlockHash([0; 32]), 0)); let cb = fake_compact_block_spending( - &self.params, + &self.network(), height, prev_hash, note, @@ -296,7 +278,7 @@ where >, > { scan_cached_blocks( - &self.params, + &self.network(), self.cache.block_source(), &mut self.db_data, from_height, @@ -316,9 +298,9 @@ impl TestState { &mut self.db_data } - /// Exposes an immutable reference to the network in use. - pub(crate) fn network(&self) -> &Network { - &self.db_data.params + /// Exposes the network in use. + pub(crate) fn network(&self) -> Network { + self.db_data.params } /// Convenience method for obtaining the Sapling activation height for the network under test. @@ -362,9 +344,10 @@ impl TestState { ReceivedNoteId, >, > { + let params = self.network(); create_spend_to_address( &mut self.db_data, - &self.params, + ¶ms, test_prover(), usk, to, @@ -397,9 +380,10 @@ impl TestState { where InputsT: InputSelector>, { + let params = self.network(); spend( &mut self.db_data, - &self.params, + ¶ms, test_prover(), input_selector, usk, @@ -430,9 +414,10 @@ impl TestState { where InputsT: InputSelector>, { + let params = self.network(); propose_transfer::<_, _, _, Infallible>( &mut self.db_data, - &self.params, + ¶ms, spend_from_account, input_selector, request, @@ -462,9 +447,10 @@ impl TestState { where InputsT: InputSelector>, { + let params = self.network(); propose_shielding::<_, _, _, Infallible>( &mut self.db_data, - &self.params, + ¶ms, input_selector, shielding_threshold, from_addrs, @@ -493,9 +479,10 @@ impl TestState { where FeeRuleT: FeeRule, { + let params = self.network(); create_proposed_transaction::<_, _, Infallible, _>( &mut self.db_data, - &self.params, + ¶ms, test_prover(), usk, ovk_policy, @@ -529,9 +516,10 @@ impl TestState { where InputsT: InputSelector>, { + let params = self.network(); shield_transparent_funds( &mut self.db_data, - &self.params, + ¶ms, test_prover(), input_selector, shielding_threshold, diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index 354016fe4..5856970ae 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -391,7 +391,10 @@ mod tests { zip32::sapling::ExtendedFullViewingKey, }; - use crate::{error::SqliteClientError, wallet::scanning::priority_code, AccountId, WalletDb}; + use crate::{ + error::SqliteClientError, testing::TestBuilder, wallet::scanning::priority_code, AccountId, + WalletDb, + }; use super::{init_accounts_table, init_blocks_table, init_wallet_db}; @@ -405,9 +408,7 @@ mod tests { #[test] fn verify_schema() { - let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), Network::TestNetwork).unwrap(); - init_wallet_db(&mut db_data, None).unwrap(); + let st = TestBuilder::new().build(); use regex::Regex; let re = Regex::new(r"\s+").unwrap(); @@ -560,7 +561,8 @@ mod tests { )", ]; - let mut tables_query = db_data + let mut tables_query = st + .wallet() .conn .prepare("SELECT sql FROM sqlite_schema WHERE type = 'table' ORDER BY tbl_name") .unwrap(); @@ -600,7 +602,7 @@ mod tests { AND (scan_queue.block_range_end - 1) >= shard.subtree_end_height ) WHERE scan_queue.priority > {}", - u32::from(db_data.params.activation_height(NetworkUpgrade::Sapling).unwrap()), + u32::from(st.network().activation_height(NetworkUpgrade::Sapling).unwrap()), priority_code(&ScanPriority::Scanned), ), // v_transactions @@ -741,7 +743,8 @@ mod tests { OR sapling_received_notes.is_change = 0".to_owned(), ]; - let mut views_query = db_data + let mut views_query = st + .wallet() .conn .prepare("SELECT sql FROM sqlite_schema WHERE type = 'view' ORDER BY tbl_name") .unwrap(); diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index db1d126c7..9c254c789 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -532,7 +532,7 @@ pub(crate) mod tests { let ufvks = [(account, usk.to_unified_full_viewing_key())] .into_iter() .collect(); - let decrypted_outputs = decrypt_transaction(st.network(), h + 1, &tx, &ufvks); + let decrypted_outputs = decrypt_transaction(&st.network(), h + 1, &tx, &ufvks); assert_eq!(decrypted_outputs.len(), 2); let mut found_tx_change_memo = false; @@ -619,7 +619,7 @@ pub(crate) mod tests { // Create a USK that doesn't exist in the wallet let acct1 = AccountId::from(1); - let usk1 = UnifiedSpendingKey::from_seed(st.network(), &[1u8; 32], acct1).unwrap(); + let usk1 = UnifiedSpendingKey::from_seed(&st.network(), &[1u8; 32], acct1).unwrap(); // Attempting to spend with a USK that is not in the wallet results in an error assert_matches!( @@ -934,7 +934,7 @@ pub(crate) mod tests { for output in tx.sapling_bundle().unwrap().shielded_outputs() { // Find the output that decrypts with the external OVK let result = try_sapling_output_recovery( - st.network(), + &st.network(), h1, &dfvk.to_ovk(Scope::External), output, diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index ff1d91c85..e7d8c7668 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -1200,7 +1200,7 @@ mod tests { fn create_account_creates_ignored_range() { use ScanPriority::*; - let mut st = TestBuilder::new().with_block_cache().build(); + let mut st = TestBuilder::new().build(); let sap_active = st.sapling_activation_height();