From 02562187afcfca9eb0efa1d06c5b85a0f0495bcf Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sun, 10 Mar 2024 18:47:59 -0600 Subject: [PATCH 1/3] zcash_client_sqlite: Make scan_cached_blocks_detects_spends_out_of_order a common single-pool test --- zcash_client_sqlite/src/chain.rs | 37 --------------------- zcash_client_sqlite/src/testing/pool.rs | 39 +++++++++++++++++++++++ zcash_client_sqlite/src/wallet/orchard.rs | 5 +++ zcash_client_sqlite/src/wallet/sapling.rs | 5 +++ 4 files changed, 49 insertions(+), 37 deletions(-) diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 38b32d3c1..284bc11de 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -613,41 +613,4 @@ mod tests { // Account balance should equal the change assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap()); } - - #[test] - fn scan_cached_blocks_detects_spends_out_of_order() { - let mut st = TestBuilder::new() - .with_block_cache() - .with_test_account(AccountBirthday::from_sapling_activation) - .build(); - let account = st.test_account().unwrap(); - - let dfvk = st.test_account_sapling().unwrap(); - - // Wallet summary is not yet available - assert_eq!(st.get_wallet_summary(0), None); - - // Create a fake CompactBlock sending value to the address - let value = NonNegativeAmount::const_from_u64(5); - let (received_height, _, nf) = - st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); - - // Create a second fake CompactBlock spending value from the address - let extsk2 = ExtendedSpendingKey::master(&[0]); - let to2 = extsk2.default_address().1; - let value2 = NonNegativeAmount::const_from_u64(2); - let (spent_height, _) = st.generate_next_block_spending(&dfvk, (nf, value), to2, value2); - - // Scan the spending block first. - st.scan_cached_blocks(spent_height, 1); - - // Account balance should equal the change - assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap()); - - // Now scan the block in which we received the note that was spent. - st.scan_cached_blocks(received_height, 1); - - // Account balance should be the same. - assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap()); - } } diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 16150165f..309840e08 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -1396,3 +1396,42 @@ pub(crate) fn checkpoint_gaps() { Ok(_) ); } + +pub(crate) fn scan_cached_blocks_detects_spends_out_of_order() { + let mut st = TestBuilder::new() + .with_block_cache() + .with_test_account(AccountBirthday::from_sapling_activation) + .build(); + + let account = st.test_account().unwrap(); + let dfvk = T::test_account_fvk(&st); + + // Wallet summary is not yet available + assert_eq!(st.get_wallet_summary(0), None); + + // Create a fake CompactBlock sending value to the address + let value = NonNegativeAmount::const_from_u64(5); + let (received_height, _, nf) = st.generate_next_block( + &dfvk, + AddressType::DefaultExternal, + value + ); + + // Create a second fake CompactBlock spending value from the address + let not_our_key = T::sk_to_fvk(&T::sk(&[0xf5; 32])); + let to2 = T::fvk_default_address(¬_our_key); + let value2 = NonNegativeAmount::const_from_u64(2); + let (spent_height, _) = st.generate_next_block_spending(&dfvk, (nf, value), to2, value2); + + // Scan the spending block first. + st.scan_cached_blocks(spent_height, 1); + + // Account balance should equal the change + assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap()); + + // Now scan the block in which we received the note that was spent. + st.scan_cached_blocks(received_height, 1); + + // Account balance should be the same. + assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap()); +} diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index e9251ef01..4af9cc1d0 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -235,4 +235,9 @@ pub(crate) mod tests { fn checkpoint_gaps() { testing::pool::checkpoint_gaps::() } + + #[test] + fn scan_cached_blocks_detects_spends_out_of_order() { + testing::pool::scan_cached_blocks_detects_spends_out_of_order::() + } } diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index fa318c772..e5ba32910 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -701,4 +701,9 @@ pub(crate) mod tests { fn checkpoint_gaps() { testing::pool::checkpoint_gaps::() } + + #[test] + fn scan_cached_blocks_detects_spends_out_of_order() { + testing::pool::scan_cached_blocks_detects_spends_out_of_order::() + } } From 99b3d3c9975eef235eacd5e4215fc99d729f79d6 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sun, 10 Mar 2024 18:47:59 -0600 Subject: [PATCH 2/3] zcash_client_sqlite: Generalize chain tests. --- zcash_client_sqlite/src/chain.rs | 320 ++++------------------ zcash_client_sqlite/src/testing/pool.rs | 281 ++++++++++++++++++- zcash_client_sqlite/src/wallet/orchard.rs | 6 + zcash_client_sqlite/src/wallet/sapling.rs | 6 + 4 files changed, 341 insertions(+), 272 deletions(-) diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 284bc11de..c0d5a8f1b 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -322,295 +322,85 @@ where #[cfg(test)] #[allow(deprecated)] mod tests { - use std::num::NonZeroU32; + use crate::{testing, wallet::sapling::tests::SaplingPoolTester}; - use sapling::zip32::ExtendedSpendingKey; - use zcash_primitives::{ - block::BlockHash, - transaction::{components::amount::NonNegativeAmount, fees::zip317::FeeRule}, - }; - - use zcash_client_backend::{ - address::Address, - data_api::{ - chain::error::Error, wallet::input_selection::GreedyInputSelector, AccountBirthday, - WalletRead, - }, - fees::{zip317::SingleOutputChangeStrategy, DustOutputPolicy}, - scanning::ScanError, - wallet::OvkPolicy, - zip321::{Payment, TransactionRequest}, - ShieldedProtocol, - }; - - use crate::{ - testing::{AddressType, TestBuilder}, - wallet::truncate_to_height, - }; + #[cfg(feature = "orchard")] + use crate::wallet::orchard::tests::OrchardPoolTester; #[test] - fn valid_chain_states() { - let mut st = TestBuilder::new() - .with_block_cache() - .with_test_account(AccountBirthday::from_sapling_activation) - .build(); - - let dfvk = st.test_account_sapling().unwrap(); - - // Empty chain should return None - assert_matches!(st.wallet().chain_height(), Ok(None)); - - // Create a fake CompactBlock sending value to the address - let (h1, _, _) = st.generate_next_block( - &dfvk, - AddressType::DefaultExternal, - NonNegativeAmount::const_from_u64(5), - ); - - // Scan the cache - st.scan_cached_blocks(h1, 1); - - // Create a second fake CompactBlock sending more value to the address - let (h2, _, _) = st.generate_next_block( - &dfvk, - AddressType::DefaultExternal, - NonNegativeAmount::const_from_u64(7), - ); - - // Scanning should detect no inconsistencies - st.scan_cached_blocks(h2, 1); + fn valid_chain_states_sapling() { + testing::pool::valid_chain_states::() } #[test] - fn invalid_chain_cache_disconnected() { - let mut st = TestBuilder::new() - .with_block_cache() - .with_test_account(AccountBirthday::from_sapling_activation) - .build(); - - let dfvk = st.test_account_sapling().unwrap(); - - // Create some fake CompactBlocks - let (h, _, _) = st.generate_next_block( - &dfvk, - AddressType::DefaultExternal, - NonNegativeAmount::const_from_u64(5), - ); - let (last_contiguous_height, _, _) = st.generate_next_block( - &dfvk, - AddressType::DefaultExternal, - NonNegativeAmount::const_from_u64(7), - ); - - // Scanning the cache should find no inconsistencies - st.scan_cached_blocks(h, 2); - - // Create more fake CompactBlocks that don't connect to the scanned ones - let disconnect_height = last_contiguous_height + 1; - st.generate_block_at( - disconnect_height, - BlockHash([1; 32]), - &dfvk, - AddressType::DefaultExternal, - NonNegativeAmount::const_from_u64(8), - 2, - 2, - ); - st.generate_next_block( - &dfvk, - AddressType::DefaultExternal, - NonNegativeAmount::const_from_u64(3), - ); - - // Data+cache chain should be invalid at the data/cache boundary - assert_matches!( - st.try_scan_cached_blocks( - disconnect_height, - 2 - ), - Err(Error::Scan(ScanError::PrevHashMismatch { at_height })) - if at_height == disconnect_height - ); + #[cfg(feature = "orchard")] + fn valid_chain_states_orchard() { + testing::pool::valid_chain_states::() } #[test] - fn data_db_truncation() { - let mut st = TestBuilder::new() - .with_block_cache() - .with_test_account(AccountBirthday::from_sapling_activation) - .build(); - let account = st.test_account().unwrap(); - - let dfvk = st.test_account_sapling().unwrap(); - - // Wallet summary is not yet available - assert_eq!(st.get_wallet_summary(0), None); - - // Create fake CompactBlocks sending value to the address - let value = NonNegativeAmount::const_from_u64(5); - let value2 = NonNegativeAmount::const_from_u64(7); - let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); - st.generate_next_block(&dfvk, AddressType::DefaultExternal, value2); - - // Scan the cache - st.scan_cached_blocks(h, 2); - - // Account balance should reflect both received notes - assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap()); - - // "Rewind" to height of last scanned block - st.wallet_mut() - .transactionally(|wdb| truncate_to_height(wdb.conn.0, &wdb.params, h + 1)) - .unwrap(); - - // Account balance should be unaltered - assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap()); - - // Rewind so that one block is dropped - st.wallet_mut() - .transactionally(|wdb| truncate_to_height(wdb.conn.0, &wdb.params, h)) - .unwrap(); - - // Account balance should only contain the first received note - assert_eq!(st.get_total_balance(account.0), value); - - // Scan the cache again - st.scan_cached_blocks(h, 2); - - // Account balance should again reflect both received notes - assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap()); + fn invalid_chain_cache_disconnected_sapling() { + testing::pool::invalid_chain_cache_disconnected::() } #[test] - fn scan_cached_blocks_allows_blocks_out_of_order() { - let mut st = TestBuilder::new() - .with_block_cache() - .with_test_account(AccountBirthday::from_sapling_activation) - .build(); - let account = st.test_account().unwrap(); - - let (_, usk, _) = st.test_account().unwrap(); - let dfvk = st.test_account_sapling().unwrap(); - - // Create a block with height SAPLING_ACTIVATION_HEIGHT - let value = NonNegativeAmount::const_from_u64(50000); - let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); - st.scan_cached_blocks(h1, 1); - assert_eq!(st.get_total_balance(account.0), value); - - // Create blocks to reach SAPLING_ACTIVATION_HEIGHT + 2 - let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); - let (h3, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); - - // Scan the later block first - st.scan_cached_blocks(h3, 1); - - // Now scan the block of height SAPLING_ACTIVATION_HEIGHT + 1 - st.scan_cached_blocks(h2, 1); - assert_eq!( - st.get_total_balance(account.0), - NonNegativeAmount::const_from_u64(150_000) - ); - - // We can spend the received notes - let req = TransactionRequest::new(vec![Payment { - recipient_address: Address::Sapling(dfvk.default_address().1), - amount: NonNegativeAmount::const_from_u64(110_000), - memo: None, - label: None, - message: None, - other_params: vec![], - }]) - .unwrap(); - let input_selector = GreedyInputSelector::new( - SingleOutputChangeStrategy::new(FeeRule::standard(), None, ShieldedProtocol::Sapling), - DustOutputPolicy::default(), - ); - assert_matches!( - st.spend( - &input_selector, - &usk, - req, - OvkPolicy::Sender, - NonZeroU32::new(1).unwrap(), - ), - Ok(_) - ); + fn invalid_chain_cache_disconnected_orchard() { + #[cfg(feature = "orchard")] + testing::pool::invalid_chain_cache_disconnected::() } #[test] - fn scan_cached_blocks_finds_received_notes() { - let mut st = TestBuilder::new() - .with_block_cache() - .with_test_account(AccountBirthday::from_sapling_activation) - .build(); - let account = st.test_account().unwrap(); - - let dfvk = st.test_account_sapling().unwrap(); - - // Wallet summary is not yet available - assert_eq!(st.get_wallet_summary(0), None); - - // Create a fake CompactBlock sending value to the address - let value = NonNegativeAmount::const_from_u64(5); - let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); - - // Scan the cache - let summary = st.scan_cached_blocks(h1, 1); - assert_eq!(summary.scanned_range().start, h1); - assert_eq!(summary.scanned_range().end, h1 + 1); - assert_eq!(summary.received_sapling_note_count(), 1); - - // Account balance should reflect the received note - assert_eq!(st.get_total_balance(account.0), value); - - // Create a second fake CompactBlock sending more value to the address - let value2 = NonNegativeAmount::const_from_u64(7); - let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value2); - - // Scan the cache again - let summary = st.scan_cached_blocks(h2, 1); - assert_eq!(summary.scanned_range().start, h2); - assert_eq!(summary.scanned_range().end, h2 + 1); - assert_eq!(summary.received_sapling_note_count(), 1); - - // Account balance should reflect both received notes - assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap()); + fn data_db_truncation_sapling() { + testing::pool::data_db_truncation::() } #[test] - fn scan_cached_blocks_finds_change_notes() { - let mut st = TestBuilder::new() - .with_block_cache() - .with_test_account(AccountBirthday::from_sapling_activation) - .build(); - let account = st.test_account().unwrap(); - let dfvk = st.test_account_sapling().unwrap(); + #[cfg(feature = "orchard")] + fn data_db_truncation_orchard() { + testing::pool::data_db_truncation::() + } - // Wallet summary is not yet available - assert_eq!(st.get_wallet_summary(0), None); + #[test] + fn scan_cached_blocks_allows_blocks_out_of_order_sapling() { + testing::pool::scan_cached_blocks_allows_blocks_out_of_order::() + } - // Create a fake CompactBlock sending value to the address - let value = NonNegativeAmount::const_from_u64(5); - let (received_height, _, nf) = - st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + #[test] + #[cfg(feature = "orchard")] + fn scan_cached_blocks_allows_blocks_out_of_order_orchard() { + testing::pool::scan_cached_blocks_allows_blocks_out_of_order::() + } - // Scan the cache - st.scan_cached_blocks(received_height, 1); + #[test] + fn scan_cached_blocks_finds_received_notes_sapling() { + testing::pool::scan_cached_blocks_finds_received_notes::() + } - // Account balance should reflect the received note - assert_eq!(st.get_total_balance(account.0), value); + #[test] + #[cfg(feature = "orchard")] + fn scan_cached_blocks_finds_received_notes_orchard() { + testing::pool::scan_cached_blocks_finds_received_notes::() + } - // Create a second fake CompactBlock spending value from the address - let extsk2 = ExtendedSpendingKey::master(&[0]); - let to2 = extsk2.default_address().1; - let value2 = NonNegativeAmount::const_from_u64(2); - let (spent_height, _) = st.generate_next_block_spending(&dfvk, (nf, value), to2, value2); + #[test] + fn scan_cached_blocks_finds_change_notes_sapling() { + testing::pool::scan_cached_blocks_finds_change_notes::() + } - // Scan the cache again - st.scan_cached_blocks(spent_height, 1); + #[test] + #[cfg(feature = "orchard")] + fn scan_cached_blocks_finds_change_notes_orchard() { + testing::pool::scan_cached_blocks_finds_change_notes::() + } - // Account balance should equal the change - assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap()); + #[test] + fn scan_cached_blocks_detects_spends_out_of_order_sapling() { + testing::pool::scan_cached_blocks_detects_spends_out_of_order::() + } + + #[test] + #[cfg(feature = "orchard")] + fn scan_cached_blocks_detects_spends_out_of_order_orchard() { + testing::pool::scan_cached_blocks_detects_spends_out_of_order::() } } diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 309840e08..84884ba86 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -27,7 +27,7 @@ use zcash_client_backend::{ address::Address, data_api::{ self, - chain::CommitmentTreeRoot, + chain::{self, CommitmentTreeRoot, ScanSummary}, error::Error, wallet::input_selection::{GreedyInputSelector, GreedyInputSelectorError}, AccountBirthday, DecryptedTransaction, Ratio, WalletRead, WalletSummary, WalletWrite, @@ -35,6 +35,7 @@ use zcash_client_backend::{ decrypt_transaction, fees::{fixed, standard, DustOutputPolicy}, keys::UnifiedSpendingKey, + scanning::ScanError, wallet::{Note, OvkPolicy, ReceivedNote}, zip321::{self, Payment, TransactionRequest}, ShieldedProtocol, @@ -47,7 +48,7 @@ use crate::{ testing::{input_selector, AddressType, BlockCache, TestBuilder, TestState}, wallet::{ block_max_scanned, commitment_tree, parse_scope, - scanning::tests::test_with_nu5_birthday_offset, + scanning::tests::test_with_nu5_birthday_offset, truncate_to_height, }, AccountId, NoteId, ReceivedNoteId, }; @@ -120,6 +121,8 @@ pub(crate) trait ShieldedPoolTester { tx: &Transaction, fvk: &Self::Fvk, ) -> Result, OutputRecoveryError>; + + fn received_note_count(summary: &ScanSummary) -> usize; } pub(crate) fn send_single_step_proposed_transfer() { @@ -1397,6 +1400,273 @@ pub(crate) fn checkpoint_gaps() { ); } +pub(crate) fn valid_chain_states() { + let mut st = TestBuilder::new() + .with_block_cache() + .with_test_account(AccountBirthday::from_sapling_activation) + .build(); + + let dfvk = T::test_account_fvk(&st); + + // Empty chain should return None + assert_matches!(st.wallet().chain_height(), Ok(None)); + + // Create a fake CompactBlock sending value to the address + let (h1, _, _) = st.generate_next_block( + &dfvk, + AddressType::DefaultExternal, + NonNegativeAmount::const_from_u64(5), + ); + + // Scan the cache + st.scan_cached_blocks(h1, 1); + + // Create a second fake CompactBlock sending more value to the address + let (h2, _, _) = st.generate_next_block( + &dfvk, + AddressType::DefaultExternal, + NonNegativeAmount::const_from_u64(7), + ); + + // Scanning should detect no inconsistencies + st.scan_cached_blocks(h2, 1); +} + +pub(crate) fn invalid_chain_cache_disconnected() { + let mut st = TestBuilder::new() + .with_block_cache() + .with_test_account(AccountBirthday::from_sapling_activation) + .build(); + + let dfvk = T::test_account_fvk(&st); + + // Create some fake CompactBlocks + let (h, _, _) = st.generate_next_block( + &dfvk, + AddressType::DefaultExternal, + NonNegativeAmount::const_from_u64(5), + ); + let (last_contiguous_height, _, _) = st.generate_next_block( + &dfvk, + AddressType::DefaultExternal, + NonNegativeAmount::const_from_u64(7), + ); + + // Scanning the cache should find no inconsistencies + st.scan_cached_blocks(h, 2); + + // Create more fake CompactBlocks that don't connect to the scanned ones + let disconnect_height = last_contiguous_height + 1; + st.generate_block_at( + disconnect_height, + BlockHash([1; 32]), + &dfvk, + AddressType::DefaultExternal, + NonNegativeAmount::const_from_u64(8), + 2, + 2, + ); + st.generate_next_block( + &dfvk, + AddressType::DefaultExternal, + NonNegativeAmount::const_from_u64(3), + ); + + // Data+cache chain should be invalid at the data/cache boundary + assert_matches!( + st.try_scan_cached_blocks( + disconnect_height, + 2 + ), + Err(chain::error::Error::Scan(ScanError::PrevHashMismatch { at_height })) + if at_height == disconnect_height + ); +} + +pub(crate) fn data_db_truncation() { + let mut st = TestBuilder::new() + .with_block_cache() + .with_test_account(AccountBirthday::from_sapling_activation) + .build(); + + let account = st.test_account().unwrap(); + let dfvk = T::test_account_fvk(&st); + + // Wallet summary is not yet available + assert_eq!(st.get_wallet_summary(0), None); + + // Create fake CompactBlocks sending value to the address + let value = NonNegativeAmount::const_from_u64(5); + let value2 = NonNegativeAmount::const_from_u64(7); + let (h, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + st.generate_next_block(&dfvk, AddressType::DefaultExternal, value2); + + // Scan the cache + st.scan_cached_blocks(h, 2); + + // Account balance should reflect both received notes + assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap()); + + // "Rewind" to height of last scanned block + st.wallet_mut() + .transactionally(|wdb| truncate_to_height(wdb.conn.0, &wdb.params, h + 1)) + .unwrap(); + + // Account balance should be unaltered + assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap()); + + // Rewind so that one block is dropped + st.wallet_mut() + .transactionally(|wdb| truncate_to_height(wdb.conn.0, &wdb.params, h)) + .unwrap(); + + // Account balance should only contain the first received note + assert_eq!(st.get_total_balance(account.0), value); + + // Scan the cache again + st.scan_cached_blocks(h, 2); + + // Account balance should again reflect both received notes + assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap()); +} + +pub(crate) fn scan_cached_blocks_allows_blocks_out_of_order() { + let mut st = TestBuilder::new() + .with_block_cache() + .with_test_account(AccountBirthday::from_sapling_activation) + .build(); + + let account = st.test_account().unwrap(); + let (_, usk, _) = st.test_account().unwrap(); + let dfvk = T::test_account_fvk(&st); + + let value = NonNegativeAmount::const_from_u64(50000); + let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + st.scan_cached_blocks(h1, 1); + assert_eq!(st.get_total_balance(account.0), value); + + // Create blocks to reach height + 2 + let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + let (h3, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + + // Scan the later block first + st.scan_cached_blocks(h3, 1); + + // Now scan the block of height height + 1 + st.scan_cached_blocks(h2, 1); + assert_eq!( + st.get_total_balance(account.0), + NonNegativeAmount::const_from_u64(150_000) + ); + + // We can spend the received notes + let req = TransactionRequest::new(vec![Payment { + recipient_address: T::fvk_default_address(&dfvk), + amount: NonNegativeAmount::const_from_u64(110_000), + memo: None, + label: None, + message: None, + other_params: vec![], + }]) + .unwrap(); + + #[allow(deprecated)] + let input_selector = GreedyInputSelector::new( + standard::SingleOutputChangeStrategy::new( + StandardFeeRule::Zip317, + None, + T::SHIELDED_PROTOCOL, + ), + DustOutputPolicy::default(), + ); + assert_matches!( + st.spend( + &input_selector, + &usk, + req, + OvkPolicy::Sender, + NonZeroU32::new(1).unwrap(), + ), + Ok(_) + ); +} + +pub(crate) fn scan_cached_blocks_finds_received_notes() { + let mut st = TestBuilder::new() + .with_block_cache() + .with_test_account(AccountBirthday::from_sapling_activation) + .build(); + + let account = st.test_account().unwrap(); + let dfvk = T::test_account_fvk(&st); + + // Wallet summary is not yet available + assert_eq!(st.get_wallet_summary(0), None); + + // Create a fake CompactBlock sending value to the address + let value = NonNegativeAmount::const_from_u64(5); + let (h1, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + + // Scan the cache + let summary = st.scan_cached_blocks(h1, 1); + assert_eq!(summary.scanned_range().start, h1); + assert_eq!(summary.scanned_range().end, h1 + 1); + assert_eq!(T::received_note_count(&summary), 1); + + // Account balance should reflect the received note + assert_eq!(st.get_total_balance(account.0), value); + + // Create a second fake CompactBlock sending more value to the address + let value2 = NonNegativeAmount::const_from_u64(7); + let (h2, _, _) = st.generate_next_block(&dfvk, AddressType::DefaultExternal, value2); + + // Scan the cache again + let summary = st.scan_cached_blocks(h2, 1); + assert_eq!(summary.scanned_range().start, h2); + assert_eq!(summary.scanned_range().end, h2 + 1); + assert_eq!(T::received_note_count(&summary), 1); + + // Account balance should reflect both received notes + assert_eq!(st.get_total_balance(account.0), (value + value2).unwrap()); +} + +// TODO: This test can probably be entirely removed, as the following test duplicates it entirely. +pub(crate) fn scan_cached_blocks_finds_change_notes() { + let mut st = TestBuilder::new() + .with_block_cache() + .with_test_account(AccountBirthday::from_sapling_activation) + .build(); + + let account = st.test_account().unwrap(); + let dfvk = T::test_account_fvk(&st); + + // Wallet summary is not yet available + assert_eq!(st.get_wallet_summary(0), None); + + // Create a fake CompactBlock sending value to the address + let value = NonNegativeAmount::const_from_u64(5); + let (received_height, _, nf) = + st.generate_next_block(&dfvk, AddressType::DefaultExternal, value); + + // Scan the cache + st.scan_cached_blocks(received_height, 1); + + // Account balance should reflect the received note + assert_eq!(st.get_total_balance(account.0), value); + + // Create a second fake CompactBlock spending value from the address + let not_our_key = T::sk_to_fvk(&T::sk(&[0xf5; 32])); + let to2 = T::fvk_default_address(¬_our_key); + let value2 = NonNegativeAmount::const_from_u64(2); + let (spent_height, _) = st.generate_next_block_spending(&dfvk, (nf, value), to2, value2); + + // Scan the cache again + st.scan_cached_blocks(spent_height, 1); + + // Account balance should equal the change + assert_eq!(st.get_total_balance(account.0), (value - value2).unwrap()); +} + pub(crate) fn scan_cached_blocks_detects_spends_out_of_order() { let mut st = TestBuilder::new() .with_block_cache() @@ -1411,11 +1681,8 @@ pub(crate) fn scan_cached_blocks_detects_spends_out_of_order usize { + summary.received_orchard_note_count() + } } #[test] diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index e5ba32910..cff74aa43 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -620,6 +620,12 @@ pub(crate) mod tests { Ok(None) } + + fn received_note_count( + summary: &zcash_client_backend::data_api::chain::ScanSummary, + ) -> usize { + summary.received_sapling_note_count() + } } pub(crate) fn test_prover() -> impl SpendProver + OutputProver { From de58b5a5b147c466aba5cbeb1a9ad81d3327ca7e Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sun, 10 Mar 2024 22:13:39 -0600 Subject: [PATCH 3/3] zcash_client_sqlite: Add a failing test of cross-pool transfer --- zcash_client_backend/src/data_api.rs | 33 ++++++--- zcash_client_sqlite/src/testing/pool.rs | 87 +++++++++++++++++++++++ zcash_client_sqlite/src/wallet/orchard.rs | 7 ++ zcash_client_sqlite/src/wallet/sapling.rs | 8 +++ 4 files changed, 126 insertions(+), 9 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 362c2bf11..9c32c6480 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -92,6 +92,9 @@ use { zcash_primitives::{legacy::TransparentAddress, transaction::components::OutPoint}, }; +#[cfg(feature = "test-dependencies")] +use zcash_primitives::consensus::NetworkUpgrade; + pub mod chain; pub mod error; pub mod scanning; @@ -1153,6 +1156,26 @@ impl AccountBirthday { self.recover_until } + #[cfg(feature = "test-dependencies")] + /// Constructs a new [`AccountBirthday`] at the given network upgrade's activation, + /// with no "recover until" height. + /// + /// # Panics + /// + /// Panics if the activation height for the given network upgrade is not set. + pub fn from_activation( + params: &P, + network_upgrade: NetworkUpgrade, + ) -> AccountBirthday { + AccountBirthday::from_parts( + params.activation_height(network_upgrade).unwrap(), + Frontier::empty(), + #[cfg(feature = "orchard")] + Frontier::empty(), + None, + ) + } + #[cfg(feature = "test-dependencies")] /// Constructs a new [`AccountBirthday`] at Sapling activation, with no /// "recover until" height. @@ -1163,15 +1186,7 @@ impl AccountBirthday { pub fn from_sapling_activation( params: &P, ) -> AccountBirthday { - use zcash_primitives::consensus::NetworkUpgrade; - - AccountBirthday::from_parts( - params.activation_height(NetworkUpgrade::Sapling).unwrap(), - Frontier::empty(), - #[cfg(feature = "orchard")] - Frontier::empty(), - None, - ) + Self::from_activation(params, NetworkUpgrade::Sapling) } } diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 84884ba86..9cdb440b4 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -53,6 +53,9 @@ use crate::{ AccountId, NoteId, ReceivedNoteId, }; +#[cfg(feature = "orchard")] +use zcash_primitives::consensus::NetworkUpgrade; + #[cfg(feature = "transparent-inputs")] use { zcash_client_backend::{ @@ -1400,6 +1403,90 @@ pub(crate) fn checkpoint_gaps() { ); } +#[cfg(feature = "orchard")] +pub(crate) fn cross_pool_exchange() { + let mut st = TestBuilder::new() + .with_block_cache() + .with_test_account(|params| AccountBirthday::from_activation(params, NetworkUpgrade::Nu5)) + .build(); + + let (account, usk, birthday) = st.test_account().unwrap(); + + let p0_fvk = P0::test_account_fvk(&st); + + let p1_fvk = P1::test_account_fvk(&st); + let p1_to = P1::fvk_default_address(&p1_fvk); + + let note_value = NonNegativeAmount::const_from_u64(300000); + st.generate_next_block(&p0_fvk, AddressType::DefaultExternal, note_value); + st.generate_next_block(&p1_fvk, AddressType::DefaultExternal, note_value); + st.scan_cached_blocks(birthday.height(), 2); + + let initial_balance = (note_value * 2).unwrap(); + assert_eq!(st.get_total_balance(account), initial_balance); + assert_eq!(st.get_spendable_balance(account, 1), initial_balance); + + let transfer_amount = NonNegativeAmount::const_from_u64(200000); + let p0_to_p1 = zip321::TransactionRequest::new(vec![Payment { + recipient_address: p1_to, + amount: transfer_amount, + memo: None, + label: None, + message: None, + other_params: vec![], + }]) + .unwrap(); + + let fee_rule = StandardFeeRule::Zip317; + let input_selector = GreedyInputSelector::new( + standard::SingleOutputChangeStrategy::new(fee_rule, None, P1::SHIELDED_PROTOCOL), + DustOutputPolicy::default(), + ); + let proposal0 = st + .propose_transfer( + account, + &input_selector, + p0_to_p1, + NonZeroU32::new(1).unwrap(), + ) + .unwrap(); + + let _min_target_height = proposal0.min_target_height(); + assert_eq!(proposal0.steps().len(), 1); + let step0 = &proposal0.steps().head; + + // We expect 4 logical actions, two per pool (due to padding). + let expected_fee = NonNegativeAmount::const_from_u64(20000); + assert_eq!(step0.balance().fee_required(), expected_fee); + + let expected_change = (note_value - transfer_amount - expected_fee).unwrap(); + let proposed_change = step0.balance().proposed_change(); + assert_eq!(proposed_change.len(), 1); + let change_output = proposed_change.get(0).unwrap(); + // Since this is a cross-pool transfer, change will be sent to the preferred pool. + assert_eq!( + change_output.output_pool(), + std::cmp::max(ShieldedProtocol::Sapling, ShieldedProtocol::Orchard) + ); + assert_eq!(change_output.value(), expected_change); + + let create_proposed_result = + st.create_proposed_transactions::(&usk, OvkPolicy::Sender, &proposal0); + assert_matches!(&create_proposed_result, Ok(txids) if txids.len() == 1); + + let (h, _) = st.generate_next_block_including(create_proposed_result.unwrap()[0]); + st.scan_cached_blocks(h, 1); + + assert_eq!( + st.get_total_balance(account), + (initial_balance - expected_fee).unwrap() + ); + assert_eq!( + st.get_spendable_balance(account, 1), + (initial_balance - expected_fee).unwrap() + ); +} + pub(crate) fn valid_chain_states() { let mut st = TestBuilder::new() .with_block_cache() diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 870ab25ba..2de55071d 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -246,4 +246,11 @@ pub(crate) mod tests { fn scan_cached_blocks_detects_spends_out_of_order() { testing::pool::scan_cached_blocks_detects_spends_out_of_order::() } + + #[test] + fn cross_pool_exchange() { + use crate::wallet::sapling::tests::SaplingPoolTester; + + testing::pool::cross_pool_exchange::() + } } diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index cff74aa43..ed748b48b 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -712,4 +712,12 @@ pub(crate) mod tests { fn scan_cached_blocks_detects_spends_out_of_order() { testing::pool::scan_cached_blocks_detects_spends_out_of_order::() } + + #[test] + #[cfg(feature = "orchard")] + fn cross_pool_exchange() { + use crate::wallet::orchard::tests::OrchardPoolTester; + + testing::pool::cross_pool_exchange::() + } }