From 4f7c5bd722f6592a2fcd111adaf443bbdd10cd08 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 18 Mar 2024 15:29:16 -0600 Subject: [PATCH] zcash_client_sqlite: Fix `scan_complete` tests. --- Cargo.lock | 11 +-- Cargo.toml | 7 +- zcash_client_backend/CHANGELOG.md | 1 + zcash_client_backend/src/data_api/chain.rs | 17 +++- zcash_client_backend/src/proto.rs | 12 +++ zcash_client_sqlite/src/testing.rs | 97 ++++++++++++++++++---- zcash_client_sqlite/src/wallet/orchard.rs | 1 + zcash_client_sqlite/src/wallet/sapling.rs | 1 + zcash_client_sqlite/src/wallet/scanning.rs | 92 ++++++++++++-------- 9 files changed, 175 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 820c7cf97..33f697f20 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1087,10 +1087,12 @@ dependencies = [ [[package]] name = "incrementalmerkletree" version = "0.5.0" -source = "git+https://github.com/nuttycom/incrementalmerkletree?rev=fa147c89c6c98a03bba745538f4e68d4eaed5146#fa147c89c6c98a03bba745538f4e68d4eaed5146" +source = "git+https://github.com/zcash/incrementalmerkletree?rev=e1a7a80212c22e5a8912d05860f7eb6899c56a7c#e1a7a80212c22e5a8912d05860f7eb6899c56a7c" dependencies = [ "either", "proptest", + "rand", + "rand_core", ] [[package]] @@ -1475,7 +1477,7 @@ checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5" [[package]] name = "orchard" version = "0.7.1" -source = "git+https://github.com/zcash/orchard?rev=e74879dd0ad0918f4ffe0826e03905cd819981bd#e74879dd0ad0918f4ffe0826e03905cd819981bd" +source = "git+https://github.com/zcash/orchard?rev=33474bdbfd7268e1f84718078d47f63d01a879d5#33474bdbfd7268e1f84718078d47f63d01a879d5" dependencies = [ "aes", "bitvec", @@ -2103,8 +2105,7 @@ dependencies = [ [[package]] name = "sapling-crypto" version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0db258736b34dfc6bec50fab2afb94c1845d90370b38309ebf9bb166cc951251" +source = "git+https://github.com/zcash/sapling-crypto?rev=22412ae07644813253feb064d1692b0823242853#22412ae07644813253feb064d1692b0823242853" dependencies = [ "aes", "bellman", @@ -2244,7 +2245,7 @@ dependencies = [ [[package]] name = "shardtree" version = "0.2.0" -source = "git+https://github.com/nuttycom/incrementalmerkletree?rev=fa147c89c6c98a03bba745538f4e68d4eaed5146#fa147c89c6c98a03bba745538f4e68d4eaed5146" +source = "git+https://github.com/zcash/incrementalmerkletree?rev=e1a7a80212c22e5a8912d05860f7eb6899c56a7c#e1a7a80212c22e5a8912d05860f7eb6899c56a7c" dependencies = [ "assert_matches", "bitflags 2.4.1", diff --git a/Cargo.toml b/Cargo.toml index ab50918ed..2e904303f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -122,6 +122,7 @@ panic = 'abort' codegen-units = 1 [patch.crates-io] -orchard = { git = "https://github.com/zcash/orchard", rev = "e74879dd0ad0918f4ffe0826e03905cd819981bd" } -incrementalmerkletree = { git = "https://github.com/nuttycom/incrementalmerkletree", rev = "fa147c89c6c98a03bba745538f4e68d4eaed5146" } -shardtree = { git = "https://github.com/nuttycom/incrementalmerkletree", rev = "fa147c89c6c98a03bba745538f4e68d4eaed5146" } +orchard = { git = "https://github.com/zcash/orchard", rev = "33474bdbfd7268e1f84718078d47f63d01a879d5" } +incrementalmerkletree = { git = "https://github.com/zcash/incrementalmerkletree", rev = "e1a7a80212c22e5a8912d05860f7eb6899c56a7c" } +sapling = { git = "https://github.com/zcash/sapling-crypto", package = "sapling-crypto", rev = "22412ae07644813253feb064d1692b0823242853" } +shardtree = { git = "https://github.com/zcash/incrementalmerkletree", rev = "e1a7a80212c22e5a8912d05860f7eb6899c56a7c" } diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 9873992fc..2211d07e6 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -30,6 +30,7 @@ and this library adheres to Rust's notion of - `WalletSummary::next_orchard_subtree_index` - `chain::ChainState` - `chain::ScanSummary::{spent_orchard_note_count, received_orchard_note_count}` + - `impl Debug for chain::CommitmentTreeRoot` - `zcash_client_backend::fees`: - `orchard` - `ChangeValue::orchard` diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index cb4e14e40..21e136d71 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -155,7 +155,10 @@ use std::ops::Range; use incrementalmerkletree::frontier::Frontier; use subtle::ConditionallySelectable; -use zcash_primitives::consensus::{self, BlockHeight}; +use zcash_primitives::{ + block::BlockHash, + consensus::{self, BlockHeight}, +}; use crate::{ data_api::{NullifierQuery, WalletWrite}, @@ -172,6 +175,7 @@ use super::WalletRead; /// /// This stores the block height at which the leaf that completed the subtree was /// added, and the root hash of the complete subtree. +#[derive(Debug)] pub struct CommitmentTreeRoot { subtree_end_height: BlockHeight, root_hash: H, @@ -291,6 +295,7 @@ impl ScanSummary { #[derive(Debug, Clone)] pub struct ChainState { block_height: BlockHeight, + block_hash: BlockHash, final_sapling_tree: Frontier, #[cfg(feature = "orchard")] final_orchard_tree: @@ -299,9 +304,10 @@ pub struct ChainState { impl ChainState { /// Construct a new empty chain state. - pub fn empty(block_height: BlockHeight) -> Self { + pub fn empty(block_height: BlockHeight, block_hash: BlockHash) -> Self { Self { block_height, + block_hash, final_sapling_tree: Frontier::empty(), #[cfg(feature = "orchard")] final_orchard_tree: Frontier::empty(), @@ -311,6 +317,7 @@ impl ChainState { /// Construct a new [`ChainState`] from its constituent parts. pub fn new( block_height: BlockHeight, + block_hash: BlockHash, final_sapling_tree: Frontier, #[cfg(feature = "orchard")] final_orchard_tree: Frontier< orchard::tree::MerkleHashOrchard, @@ -319,6 +326,7 @@ impl ChainState { ) -> Self { Self { block_height, + block_hash, final_sapling_tree, #[cfg(feature = "orchard")] final_orchard_tree, @@ -330,6 +338,11 @@ impl ChainState { self.block_height } + /// Return the hash of the block. + pub fn block_hash(&self) -> BlockHash { + self.block_hash + } + /// Returns the frontier of the Sapling note commitment tree as of the end of the block at /// [`Self::block_height`]. pub fn final_sapling_tree( diff --git a/zcash_client_backend/src/proto.rs b/zcash_client_backend/src/proto.rs index 69d89070c..3e58bc309 100644 --- a/zcash_client_backend/src/proto.rs +++ b/zcash_client_backend/src/proto.rs @@ -295,10 +295,22 @@ impl service::TreeState { /// /// [`scan_cached_blocks`]: crate::data_api::chain::scan_cached_blocks pub fn to_chain_state(&self) -> io::Result { + let mut hash_bytes = hex::decode(&self.hash).map_err(|e| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("Block hash is not valid hex: {:?}", e), + ) + })?; + // Zcashd hex strings for block hashes are byte-reversed. + hash_bytes.reverse(); + Ok(ChainState::new( self.height .try_into() .map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "Invalid block height"))?, + BlockHash::try_from_slice(&hash_bytes).ok_or_else(|| { + io::Error::new(io::ErrorKind::InvalidData, "Invalid block hash length.") + })?, self.sapling_tree()?.to_frontier(), #[cfg(feature = "orchard")] self.orchard_tree()?.to_frontier(), diff --git a/zcash_client_sqlite/src/testing.rs b/zcash_client_sqlite/src/testing.rs index 472e094cf..1ce7f74df 100644 --- a/zcash_client_sqlite/src/testing.rs +++ b/zcash_client_sqlite/src/testing.rs @@ -6,12 +6,14 @@ use std::{collections::BTreeMap, convert::Infallible}; use std::fs::File; use group::ff::Field; +use incrementalmerkletree::Retention; use nonempty::NonEmpty; use prost::Message; use rand_chacha::ChaChaRng; use rand_core::{CryptoRng, RngCore, SeedableRng}; use rusqlite::{params, Connection}; use secrecy::{Secret, SecretVec}; +use shardtree::error::ShardTreeError; use tempfile::NamedTempFile; #[cfg(feature = "unstable")] @@ -28,13 +30,14 @@ use zcash_client_backend::{ address::Address, data_api::{ self, - chain::{scan_cached_blocks, BlockSource, ScanSummary}, + chain::{scan_cached_blocks, BlockSource, CommitmentTreeRoot, ScanSummary}, wallet::{ create_proposed_transactions, create_spend_to_address, input_selection::{GreedyInputSelector, GreedyInputSelectorError, InputSelector}, propose_standard_transfer_to_address, propose_transfer, spend, }, - AccountBalance, AccountBirthday, WalletRead, WalletSummary, WalletWrite, + AccountBalance, AccountBirthday, WalletCommitmentTrees, WalletRead, WalletSummary, + WalletWrite, }, keys::UnifiedSpendingKey, proposal::Proposal, @@ -190,7 +193,6 @@ impl TestBuilder { #[derive(Clone, Debug)] pub(crate) struct CachedBlock { - hash: BlockHash, chain_state: ChainState, sapling_end_size: u32, orchard_end_size: u32, @@ -199,19 +201,13 @@ pub(crate) struct CachedBlock { impl CachedBlock { fn none(sapling_activation_height: BlockHeight) -> Self { Self { - hash: BlockHash([0; 32]), - chain_state: ChainState::empty(sapling_activation_height), + chain_state: ChainState::empty(sapling_activation_height, BlockHash([0; 32])), sapling_end_size: 0, orchard_end_size: 0, } } - fn at( - hash: BlockHash, - chain_state: ChainState, - sapling_end_size: u32, - orchard_end_size: u32, - ) -> Self { + fn at(chain_state: ChainState, sapling_end_size: u32, orchard_end_size: u32) -> Self { assert_eq!( chain_state.final_sapling_tree().tree_size() as u32, sapling_end_size @@ -223,7 +219,6 @@ impl CachedBlock { ); Self { - hash, chain_state, sapling_end_size, orchard_end_size, @@ -258,9 +253,9 @@ impl CachedBlock { }); Self { - hash: cb.hash(), chain_state: ChainState::new( cb.height(), + cb.hash(), sapling_final_tree, #[cfg(feature = "orchard")] orchard_final_tree, @@ -323,6 +318,67 @@ where self.cache.insert(&compact_block) } + /// Ensure that the provided chain state and subtree roots exist in the wallet's note + /// commitment tree(s). This may result in a conflict if either the provided subtree roots or + /// the chain state conflict with existing note commitment tree data. + pub(crate) fn establish_chain_state( + &mut self, + state: ChainState, + prior_sapling_roots: &[CommitmentTreeRoot], + #[cfg(feature = "orchard")] prior_orchard_roots: &[CommitmentTreeRoot], + ) -> Result<(), ShardTreeError> { + self.wallet_mut() + .put_sapling_subtree_roots(0, prior_sapling_roots)?; + #[cfg(feature = "orchard")] + self.wallet_mut() + .put_orchard_subtree_roots(0, prior_orchard_roots)?; + + self.wallet_mut().with_sapling_tree_mut(|t| { + t.insert_frontier( + state.final_sapling_tree().clone(), + Retention::Checkpoint { + id: state.block_height(), + is_marked: false, + }, + ) + })?; + let final_sapling_tree_size = state.final_sapling_tree().tree_size() as u32; + + #[cfg(feature = "orchard")] + self.wallet_mut().with_orchard_tree_mut(|t| { + t.insert_frontier( + state.final_orchard_tree().clone(), + Retention::Checkpoint { + id: state.block_height(), + is_marked: false, + }, + ) + })?; + + let _final_orchard_tree_size = 0; + #[cfg(feature = "orchard")] + let _final_orchard_tree_size = state.final_orchard_tree().tree_size() as u32; + + self.insert_cached_block(state, final_sapling_tree_size, _final_orchard_tree_size); + Ok(()) + } + + fn insert_cached_block( + &mut self, + chain_state: ChainState, + sapling_end_size: u32, + orchard_end_size: u32, + ) { + self.cached_blocks.insert( + chain_state.block_height(), + CachedBlock { + chain_state, + sapling_end_size, + orchard_end_size, + }, + ); + } + /// Creates a fake block at the expected next height containing a single output of the /// given value, and inserts it into the cache. pub(crate) fn generate_next_block( @@ -337,7 +393,7 @@ where let (res, nf) = self.generate_block_at( height, - prior_cached_block.hash, + prior_cached_block.chain_state.block_hash(), fvk, req, value, @@ -376,7 +432,7 @@ where // we need to generate a new prior cached block that the block to be generated can // successfully chain from, with the provided tree sizes. if prior_cached_block.chain_state.block_height() == height - 1 { - assert_eq!(prev_hash, prior_cached_block.hash); + assert_eq!(prev_hash, prior_cached_block.chain_state.block_hash()); } else { let final_sapling_tree = (prior_cached_block.sapling_end_size..initial_sapling_tree_size).fold( @@ -400,9 +456,9 @@ where ); prior_cached_block = CachedBlock::at( - prev_hash, ChainState::new( height - 1, + prev_hash, final_sapling_tree, #[cfg(feature = "orchard")] final_orchard_tree, @@ -452,7 +508,7 @@ where let cb = fake_compact_block_spending( &self.network(), height, - prior_cached_block.hash, + prior_cached_block.chain_state.block_hash(), note, fvk, to.into(), @@ -508,7 +564,7 @@ where let cb = fake_compact_block_from_tx( height, - prior_cached_block.hash, + prior_cached_block.chain_state.block_hash(), tx_index, tx, prior_cached_block.sapling_end_size, @@ -613,6 +669,11 @@ impl TestState { self.db_data.params } + /// Exposes the random number source for the test state + pub(crate) fn rng(&mut self) -> &mut ChaChaRng { + &mut self.rng + } + /// Convenience method for obtaining the Sapling activation height for the network under test. pub(crate) fn sapling_activation_height(&self) -> BlockHeight { self.db_data diff --git a/zcash_client_sqlite/src/wallet/orchard.rs b/zcash_client_sqlite/src/wallet/orchard.rs index 3d0a575ad..638857859 100644 --- a/zcash_client_sqlite/src/wallet/orchard.rs +++ b/zcash_client_sqlite/src/wallet/orchard.rs @@ -378,6 +378,7 @@ pub(crate) mod tests { impl ShieldedPoolTester for OrchardPoolTester { const SHIELDED_PROTOCOL: ShieldedProtocol = ShieldedProtocol::Orchard; const TABLES_PREFIX: &'static str = ORCHARD_TABLES_PREFIX; + // const MERKLE_TREE_DEPTH: u8 = {orchard::NOTE_COMMITMENT_TREE_DEPTH as u8}; type Sk = SpendingKey; type Fvk = FullViewingKey; diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 14b886100..ddb0fce79 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -399,6 +399,7 @@ pub(crate) mod tests { impl ShieldedPoolTester for SaplingPoolTester { const SHIELDED_PROTOCOL: ShieldedProtocol = ShieldedProtocol::Sapling; const TABLES_PREFIX: &'static str = SAPLING_TABLES_PREFIX; + // const MERKLE_TREE_DEPTH: u8 = sapling::NOTE_COMMITMENT_TREE_DEPTH; type Sk = ExtendedSpendingKey; type Fvk = DiversifiableFullViewingKey; diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index 11f1a9d32..511213e77 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -579,12 +579,14 @@ pub(crate) fn update_chain_tip( #[cfg(test)] pub(crate) mod tests { - use incrementalmerkletree::{frontier::Frontier, Hashable, Level, Position}; + use std::num::NonZeroU8; + + use incrementalmerkletree::{frontier::Frontier, Hashable, Position}; use sapling::Node; use secrecy::SecretVec; use zcash_client_backend::data_api::{ - chain::CommitmentTreeRoot, + chain::{ChainState, CommitmentTreeRoot}, scanning::{spanning_tree::testing::scan_range, ScanPriority}, AccountBirthday, Ratio, WalletRead, WalletWrite, SAPLING_SHARD_HEIGHT, }; @@ -611,9 +613,7 @@ pub(crate) mod tests { zcash_client_backend::data_api::ORCHARD_SHARD_HEIGHT, }; - // FIXME: This requires fixes to the test framework. #[test] - #[cfg(feature = "orchard")] fn sapling_scan_complete() { scan_complete::(); } @@ -624,55 +624,75 @@ pub(crate) mod tests { scan_complete::(); } - // FIXME: This requires fixes to the test framework. - #[allow(dead_code)] fn scan_complete() { use ScanPriority::*; - + let initial_height_offset = 310; let mut st = TestBuilder::new() .with_block_cache() .with_test_account(AccountBirthday::from_sapling_activation) .build(); - let dfvk = T::test_account_fvk(&st); 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 - // distinct. - T::put_subtree_roots( - &mut st, - 0, - &[ - CommitmentTreeRoot::from_parts( - sapling_activation_height + 100, - T::empty_tree_root(Level::from(0)) - ), - CommitmentTreeRoot::from_parts( - sapling_activation_height + 200, - T::empty_tree_root(Level::from(1)) - ), - CommitmentTreeRoot::from_parts( - sapling_activation_height + 300, - T::empty_tree_root(Level::from(2)) - ), - ] - ), - Ok(()) - ); - // We'll start inserting leaf notes 5 notes after the end of the third subtree, with a gap // 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. // We set both Sapling and Orchard to the same initial tree size for simplicity. - let initial_sapling_tree_size = (0x1 << 16) * 3 + 5; - let initial_orchard_tree_size = (0x1 << 16) * 3 + 5; - let initial_height = sapling_activation_height + 310; + let initial_sapling_tree_size: u32 = (0x1 << 16) * 3 + 5; + let initial_orchard_tree_size: u32 = (0x1 << 16) * 3 + 5; + // Construct a fake chain state for the end of block 300 + let (prior_sapling_frontiers, sapling_initial_tree) = + Frontier::random_with_prior_subtree_roots( + st.rng(), + initial_sapling_tree_size.into(), + NonZeroU8::new(16).unwrap(), + ); + let sapling_subtree_roots = prior_sapling_frontiers + .into_iter() + .zip(0u32..) + .map(|(root, i)| { + CommitmentTreeRoot::from_parts(sapling_activation_height + (100 * (i + 1)), root) + }) + .collect::>(); + + #[cfg(feature = "orchard")] + let (prior_orchard_frontiers, orchard_initial_tree) = + Frontier::random_with_prior_subtree_roots( + st.rng(), + initial_orchard_tree_size.into(), + NonZeroU8::new(16).unwrap(), + ); + #[cfg(feature = "orchard")] + let orchard_subtree_roots = prior_orchard_frontiers + .into_iter() + .zip(0u32..) + .map(|(root, i)| { + CommitmentTreeRoot::from_parts(sapling_activation_height + (100 * (i + 1)), root) + }) + .collect::>(); + + let prior_block_hash = BlockHash([0; 32]); + st.establish_chain_state( + ChainState::new( + sapling_activation_height + initial_height_offset - 1, + prior_block_hash, + sapling_initial_tree, + #[cfg(feature = "orchard")] + orchard_initial_tree, + ), + &sapling_subtree_roots, + #[cfg(feature = "orchard")] + &orchard_subtree_roots, + ) + .unwrap(); + + let dfvk = T::test_account_fvk(&st); let value = NonNegativeAmount::const_from_u64(50000); + let initial_height = sapling_activation_height + initial_height_offset; st.generate_block_at( initial_height, - BlockHash([0; 32]), + prior_block_hash, &dfvk, AddressType::DefaultExternal, value,