From 86a4af76e71275206518518d216c3c49540945b5 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 5 Jul 2023 17:14:34 -0600 Subject: [PATCH] Implement `update_chain_tip` and document the scan process. --- zcash_client_backend/CHANGELOG.md | 8 +- zcash_client_backend/proto/service.proto | 24 +++ zcash_client_backend/src/data_api.rs | 7 +- zcash_client_backend/src/data_api/chain.rs | 126 +++++++++++--- zcash_client_backend/src/data_api/scanning.rs | 5 + zcash_client_backend/src/proto/service.rs | 148 ++++++++++++++++ zcash_client_backend/src/scanning.rs | 35 +++- zcash_client_sqlite/src/chain.rs | 140 +++++----------- zcash_client_sqlite/src/lib.rs | 8 +- zcash_client_sqlite/src/wallet.rs | 13 +- zcash_client_sqlite/src/wallet/scanning.rs | 158 ++++++++++-------- zcash_primitives/src/consensus.rs | 4 + 12 files changed, 471 insertions(+), 205 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 02297d45ad..fe9716d54d 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -16,7 +16,7 @@ and this library adheres to Rust's notion of - `ScannedBlock` - `ShieldedProtocol` - `WalletCommitmentTrees` - - `WalletRead::{block_metadata, block_fully_scanned, suggest_scan_ranges}` + - `WalletRead::{chain_tip, block_metadata, block_fully_scanned, suggest_scan_ranges}` - `WalletWrite::put_blocks` - `chain::CommitmentTreeRoot` - `testing::MockWalletDb::new` @@ -36,8 +36,10 @@ and this library adheres to Rust's notion of and its signature has changed; it now subsumes the removed `WalletRead::get_all_nullifiers`. - `WalletRead::get_target_and_anchor_heights` now takes its argument as a `NonZeroU32` - `chain::scan_cached_blocks` now takes a `from_height` argument that - permits the caller to control the starting position of the scan range + permits the caller to control the starting position of the scan range. In addition, the `limit` parameter is now required. + - `chain::BlockSource::with_blocks` now takes its limit as an `Option` + instead of `Option`. - A new `CommitmentTree` variant has been added to `data_api::error::Error` - `data_api::wallet::{create_spend_to_address, create_proposed_transaction, shield_transparent_funds}` all now require that `WalletCommitmentTrees` be @@ -81,8 +83,6 @@ and this library adheres to Rust's notion of 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` TODO: document how - to handle validation given out-of-order blocks. - `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}` diff --git a/zcash_client_backend/proto/service.proto b/zcash_client_backend/proto/service.proto index d7f11dcd69..0945661478 100644 --- a/zcash_client_backend/proto/service.proto +++ b/zcash_client_backend/proto/service.proto @@ -118,6 +118,22 @@ message TreeState { string orchardTree = 6; // orchard commitment tree state } +enum ShieldedProtocol { + sapling = 0; + orchard = 1; +} + +message GetSubtreeRootsArg { + uint32 startIndex = 1; // Index identifying where to start returning subtree roots + ShieldedProtocol shieldedProtocol = 2; // Shielded protocol to return subtree roots for + uint32 maxEntries = 3; // Maximum number of entries to return, or 0 for all entries. +} +message SubtreeRoot { + bytes rootHash = 2; // The 32-byte Merkle root of the subtree. + bytes completingBlockHash = 3; // The hash of the block that completed this subtree. + uint64 completingBlockHeight = 4; // The height of the block that completed this subtree in the main chain. +} + // Results are sorted by height, which makes it easy to issue another // request that picks up from where the previous left off. message GetAddressUtxosArg { @@ -142,8 +158,12 @@ service CompactTxStreamer { rpc GetLatestBlock(ChainSpec) returns (BlockID) {} // Return the compact block corresponding to the given block identifier rpc GetBlock(BlockID) returns (CompactBlock) {} + // Same as GetBlock except actions contain only nullifiers + rpc GetBlockNullifiers(BlockID) returns (CompactBlock) {} // Return a list of consecutive compact blocks rpc GetBlockRange(BlockRange) returns (stream CompactBlock) {} + // Same as GetBlockRange except actions contain only nullifiers + rpc GetBlockRangeNullifiers(BlockRange) returns (stream CompactBlock) {} // Return the requested full (not compact) transaction (as from zcashd) rpc GetTransaction(TxFilter) returns (RawTransaction) {} @@ -177,6 +197,10 @@ service CompactTxStreamer { rpc GetTreeState(BlockID) returns (TreeState) {} rpc GetLatestTreeState(Empty) returns (TreeState) {} + // Returns a stream of information about roots of subtrees of the Sapling and Orchard + // note commitment trees. + rpc GetSubtreeRoots(GetSubtreeRootsArg) returns (stream SubtreeRoot) {} + rpc GetAddressUtxos(GetAddressUtxosArg) returns (GetAddressUtxosReplyList) {} rpc GetAddressUtxosStream(GetAddressUtxosArg) returns (stream GetAddressUtxosReply) {} diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 442f513c19..4f4c80c869 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -502,10 +502,11 @@ pub trait WalletWrite: WalletRead { /// Updates the wallet's view of the blockchain. /// /// This method is used to provide the wallet with information about the state of the - /// blockchain. It should be called on wallet startup prior to calling + /// blockchain, and detect any previously scanned that needs to be re-validated before + /// proceeding with scanning. It should be called at wallet startup prior to calling /// [`WalletRead::suggest_scan_ranges`] in order to provide the wallet with the information it /// needs to correctly prioritize scanning operations. - fn update_chain_tip(&mut self, block_metadata: BlockMetadata) -> Result<(), Self::Error>; + fn update_chain_tip(&mut self, tip_height: BlockHeight) -> Result<(), Self::Error>; /// Caches a decrypted transaction in the persistent wallet store. fn store_decrypted_tx( @@ -782,7 +783,7 @@ pub mod testing { Ok(vec![]) } - fn update_chain_tip(&mut self, _block_metadata: BlockMetadata) -> Result<(), Self::Error> { + fn update_chain_tip(&mut self, _tip_height: BlockHeight) -> Result<(), Self::Error> { Ok(()) } diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index 7b45bb090e..826d9c4894 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -7,7 +7,8 @@ //! # #[cfg(feature = "test-dependencies")] //! # { //! use zcash_primitives::{ -//! consensus::{BlockHeight, Network, Parameters} +//! consensus::{BlockHeight, Network, Parameters}, +//! sapling //! }; //! //! use zcash_client_backend::{ @@ -15,10 +16,12 @@ //! WalletRead, WalletWrite, //! chain::{ //! BlockSource, +//! CommitmentTreeRoot, //! error::Error, //! scan_cached_blocks, //! testing as chain_testing, //! }, +//! scanning::ScanPriority, //! testing, //! }, //! }; @@ -32,20 +35,105 @@ //! # fn test() -> Result<(), Error<(), Infallible>> { //! let network = Network::TestNetwork; //! let block_source = chain_testing::MockBlockSource; -//! let mut db_data = testing::MockWalletDb::new(Network::TestNetwork); +//! let mut wallet_db = testing::MockWalletDb::new(Network::TestNetwork); //! -//! // 1) Download new CompactBlocks into block_source. -//! // -//! // 2) FIXME: Obtain necessary block metadata for continuity checking? -//! // -//! // 3) Scan cached blocks. -//! // -//! // FIXME: update documentation on how to detect when a rewind is required. -//! // -//! // At this point, the cache and scanned data are locally consistent (though not -//! // necessarily consistent with the latest chain tip - this would be discovered the -//! // next time this codepath is executed after new blocks are received). -//! scan_cached_blocks(&network, &block_source, &mut db_data, BlockHeight::from(0), 10) +//! // 1) Download note commitment tree data from lightwalletd +//! let roots: Vec> = unimplemented!(); +//! +//! // 2) Download chain tip metadata from lightwalletd +//! let tip_height: BlockHeight = unimplemented!(); +//! +//! // 3) Notify the wallet of the updated chain tip. +//! wallet_db.update_chain_tip(tip_height).map_err(Error::Wallet)?; +//! +//! // 4) Get the suggested scan ranges from the wallet database +//! let mut scan_ranges = wallet_db.suggest_scan_ranges().map_err(Error::Wallet)?; +//! +//! // 5) Run the following loop until the wallet's view of the chain tip as of the previous wallet +//! // session is valid. +//! loop { +//! // If there is a range of blocks that needs to be verified, it will always be returned as +//! // the first element of the vector of suggested ranges. +//! match scan_ranges.first() { +//! Some(scan_range) if scan_range.priority() == ScanPriority::Verify => { +//! // Download the blocks in `scan_range` into the block source +//! unimplemented!(); +//! +//! // Scan the downloaded blocks +//! let scan_result = scan_cached_blocks( +//! &network, +//! &block_source, +//! &mut wallet_db, +//! scan_range.block_range().start, +//! scan_range.len() +//! ); +//! +//! // Check for scanning errors that indicate that the wallet's chain tip is out of +//! // sync with blockchain history. +//! match scan_result { +//! Ok(_) => { +//! // At this point, the cache and scanned data are locally consistent (though +//! // not necessarily consistent with the latest chain tip - this would be +//! // discovered the next time this codepath is executed after new blocks are +//! // received) so we can break out of the loop. +//! break; +//! } +//! Err(Error::Scan(err)) if err.is_continuity_error() => { +//! // Pick a height to rewind to, which must be at least one block before +//! // the height at which the error occurred, but may be an earlier height +//! // determined based on heuristics such as the platform, available bandwidth, +//! // size of recent CompactBlocks, etc. +//! let rewind_height = err.at_height().saturating_sub(10); +//! +//! // Rewind to the chosen height. +//! wallet_db.truncate_to_height(rewind_height).map_err(Error::Wallet)?; +//! +//! // Delete cached blocks from rewind_height onwards. +//! // +//! // This does imply that assumed-valid blocks will be re-downloaded, but it +//! // is also possible that in the intervening time, a chain reorg has +//! // occurred that orphaned some of those blocks. +//! unimplemented!(); +//! } +//! Err(other) => { +//! // Handle or return other errors +//! } +//! } +//! +//! // Truncation will have updated the suggested scan ranges, so we now +//! // re_request +//! scan_ranges = wallet_db.suggest_scan_ranges().map_err(Error::Wallet)?; +//! } +//! _ => { +//! // Nothing to verify; break out of the loop +//! break; +//! } +//! } +//! } +//! +//! // 5) Loop over the remaining suggested scan ranges, retrieving the requested data and calling +//! // `scan_cached_blocks` on each range. Periodically, or if a continuity error is +//! // encountered, this process should be repeated starting at step (2). +//! let scan_ranges = wallet_db.suggest_scan_ranges().map_err(Error::Wallet)?; +//! for scan_range in scan_ranges { +//! // Download the blocks in `scan_range` into the block source. While in this example this +//! // step is performed in-line, it's fine for the download of scan ranges to be asynchronous +//! // and for the scanner to process the downloaded ranges as they become available in a +//! // separate thread. +//! unimplemented!(); +//! +//! // Scan the downloaded blocks, +//! let scan_result = scan_cached_blocks( +//! &network, +//! &block_source, +//! &mut wallet_db, +//! scan_range.block_range().start, +//! scan_range.len() +//! )?; +//! +//! // Handle scan errors, etc. +//! } +//! # Ok(()) //! # } //! # } //! ``` @@ -58,14 +146,12 @@ use zcash_primitives::{ }; use crate::{ - data_api::{NullifierQuery, WalletWrite}, + data_api::{BlockMetadata, NullifierQuery, WalletWrite}, proto::compact_formats::CompactBlock, scan::BatchRunner, scanning::{add_block_to_runner, check_continuity, scan_block_with_runner}, }; -use super::BlockMetadata; - pub mod error; use error::Error; @@ -114,7 +200,7 @@ pub trait BlockSource { fn with_blocks( &self, from_height: Option, - limit: Option, + limit: Option, with_row: F, ) -> Result<(), error::Error> where @@ -145,7 +231,7 @@ pub fn scan_cached_blocks( block_source: &BlockSourceT, data_db: &mut DbT, from_height: BlockHeight, - limit: u32, + limit: usize, ) -> Result<(), Error> where ParamsT: consensus::Parameters + Send + 'static, @@ -292,7 +378,7 @@ pub mod testing { fn with_blocks( &self, _from_height: Option, - _limit: Option, + _limit: Option, _with_row: F, ) -> Result<(), Error> where diff --git a/zcash_client_backend/src/data_api/scanning.rs b/zcash_client_backend/src/data_api/scanning.rs index a410d0e83e..6201de3761 100644 --- a/zcash_client_backend/src/data_api/scanning.rs +++ b/zcash_client_backend/src/data_api/scanning.rs @@ -26,6 +26,7 @@ pub struct ScanRange { impl ScanRange { pub fn from_parts(block_range: Range, priority: ScanPriority) -> Self { + assert!(block_range.end >= block_range.start); ScanRange { block_range, priority, @@ -38,6 +39,10 @@ impl ScanRange { pub fn priority(&self) -> ScanPriority { self.priority } + pub fn len(&self) -> usize { + usize::try_from(u32::from(self.block_range.end) - u32::from(self.block_range.start)) + .unwrap() + } pub fn truncate_left(&self, block_height: BlockHeight) -> Option { if block_height >= self.block_range.end { diff --git a/zcash_client_backend/src/proto/service.rs b/zcash_client_backend/src/proto/service.rs index 38b15abdbf..677e43e307 100644 --- a/zcash_client_backend/src/proto/service.rs +++ b/zcash_client_backend/src/proto/service.rs @@ -187,6 +187,32 @@ pub struct TreeState { #[prost(string, tag = "6")] pub orchard_tree: ::prost::alloc::string::String, } +#[allow(clippy::derive_partial_eq_without_eq)] +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct GetSubtreeRootsArg { + /// Index identifying where to start returning subtree roots + #[prost(uint32, tag = "1")] + pub start_index: u32, + /// Shielded protocol to return subtree roots for + #[prost(enumeration = "ShieldedProtocol", tag = "2")] + pub shielded_protocol: i32, + /// Maximum number of entries to return, or 0 for all entries. + #[prost(uint32, tag = "3")] + pub max_entries: u32, +} +#[allow(clippy::derive_partial_eq_without_eq)] +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct SubtreeRoot { + /// The 32-byte Merkle root of the subtree. + #[prost(bytes = "vec", tag = "2")] + pub root_hash: ::prost::alloc::vec::Vec, + /// The hash of the block that completed this subtree. + #[prost(bytes = "vec", tag = "3")] + pub completing_block_hash: ::prost::alloc::vec::Vec, + /// The height of the block that completed this subtree in the main chain. + #[prost(uint64, tag = "4")] + pub completing_block_height: u64, +} /// Results are sorted by height, which makes it easy to issue another /// request that picks up from where the previous left off. #[allow(clippy::derive_partial_eq_without_eq)] @@ -222,6 +248,32 @@ pub struct GetAddressUtxosReplyList { #[prost(message, repeated, tag = "1")] pub address_utxos: ::prost::alloc::vec::Vec, } +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)] +#[repr(i32)] +pub enum ShieldedProtocol { + Sapling = 0, + Orchard = 1, +} +impl ShieldedProtocol { + /// String value of the enum field names used in the ProtoBuf definition. + /// + /// The values are not transformed in any way and thus are considered stable + /// (if the ProtoBuf definition does not change) and safe for programmatic use. + pub fn as_str_name(&self) -> &'static str { + match self { + ShieldedProtocol::Sapling => "sapling", + ShieldedProtocol::Orchard => "orchard", + } + } + /// Creates an enum from field names used in the ProtoBuf definition. + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "sapling" => Some(Self::Sapling), + "orchard" => Some(Self::Orchard), + _ => None, + } + } +} /// Generated client implementations. pub mod compact_tx_streamer_client { #![allow(unused_variables, dead_code, missing_docs, clippy::let_unit_value)] @@ -366,6 +418,37 @@ pub mod compact_tx_streamer_client { ); self.inner.unary(req, path, codec).await } + /// Same as GetBlock except actions contain only nullifiers + pub async fn get_block_nullifiers( + &mut self, + request: impl tonic::IntoRequest, + ) -> std::result::Result< + tonic::Response, + tonic::Status, + > { + self.inner + .ready() + .await + .map_err(|e| { + tonic::Status::new( + tonic::Code::Unknown, + format!("Service was not ready: {}", e.into()), + ) + })?; + let codec = tonic::codec::ProstCodec::default(); + let path = http::uri::PathAndQuery::from_static( + "/cash.z.wallet.sdk.rpc.CompactTxStreamer/GetBlockNullifiers", + ); + let mut req = request.into_request(); + req.extensions_mut() + .insert( + GrpcMethod::new( + "cash.z.wallet.sdk.rpc.CompactTxStreamer", + "GetBlockNullifiers", + ), + ); + self.inner.unary(req, path, codec).await + } /// Return a list of consecutive compact blocks pub async fn get_block_range( &mut self, @@ -399,6 +482,39 @@ pub mod compact_tx_streamer_client { ); self.inner.server_streaming(req, path, codec).await } + /// Same as GetBlockRange except actions contain only nullifiers + pub async fn get_block_range_nullifiers( + &mut self, + request: impl tonic::IntoRequest, + ) -> std::result::Result< + tonic::Response< + tonic::codec::Streaming, + >, + tonic::Status, + > { + self.inner + .ready() + .await + .map_err(|e| { + tonic::Status::new( + tonic::Code::Unknown, + format!("Service was not ready: {}", e.into()), + ) + })?; + let codec = tonic::codec::ProstCodec::default(); + let path = http::uri::PathAndQuery::from_static( + "/cash.z.wallet.sdk.rpc.CompactTxStreamer/GetBlockRangeNullifiers", + ); + let mut req = request.into_request(); + req.extensions_mut() + .insert( + GrpcMethod::new( + "cash.z.wallet.sdk.rpc.CompactTxStreamer", + "GetBlockRangeNullifiers", + ), + ); + self.inner.server_streaming(req, path, codec).await + } /// Return the requested full (not compact) transaction (as from zcashd) pub async fn get_transaction( &mut self, @@ -671,6 +787,38 @@ pub mod compact_tx_streamer_client { ); self.inner.unary(req, path, codec).await } + /// Returns a stream of information about roots of subtrees of the Sapling and Orchard + /// note commitment trees. + pub async fn get_subtree_roots( + &mut self, + request: impl tonic::IntoRequest, + ) -> std::result::Result< + tonic::Response>, + tonic::Status, + > { + self.inner + .ready() + .await + .map_err(|e| { + tonic::Status::new( + tonic::Code::Unknown, + format!("Service was not ready: {}", e.into()), + ) + })?; + let codec = tonic::codec::ProstCodec::default(); + let path = http::uri::PathAndQuery::from_static( + "/cash.z.wallet.sdk.rpc.CompactTxStreamer/GetSubtreeRoots", + ); + let mut req = request.into_request(); + req.extensions_mut() + .insert( + GrpcMethod::new( + "cash.z.wallet.sdk.rpc.CompactTxStreamer", + "GetSubtreeRoots", + ), + ); + self.inner.server_streaming(req, path, codec).await + } pub async fn get_address_utxos( &mut self, request: impl tonic::IntoRequest, diff --git a/zcash_client_backend/src/scanning.rs b/zcash_client_backend/src/scanning.rs index d2f419acd4..e49f71331c 100644 --- a/zcash_client_backend/src/scanning.rs +++ b/zcash_client_backend/src/scanning.rs @@ -110,7 +110,7 @@ impl ScanningKey for SaplingIvk { } /// Errors that may occur in chain scanning -#[derive(Copy, Clone, Debug)] +#[derive(Clone, Debug)] pub enum ScanError { /// The hash of the parent block given by a proposed new chain tip does not match the hash of /// the current chain tip. @@ -141,21 +141,46 @@ pub enum ScanError { }, } +impl ScanError { + /// Returns whether this error is the result of a failed continuity check + pub fn is_continuity_error(&self) -> bool { + use ScanError::*; + match self { + PrevHashMismatch { .. } => true, + BlockHeightDiscontinuity { .. } => true, + TreeSizeMismatch { .. } => true, + TreeSizeUnknown { .. } => false, + } + } + + /// Returns the block height at which the scan error occurred + pub fn at_height(&self) -> BlockHeight { + use ScanError::*; + match self { + PrevHashMismatch { at_height } => *at_height, + BlockHeightDiscontinuity { new_height, .. } => *new_height, + TreeSizeMismatch { at_height, .. } => *at_height, + TreeSizeUnknown { at_height, .. } => *at_height, + } + } +} + impl fmt::Display for ScanError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use ScanError::*; match &self { - ScanError::PrevHashMismatch { at_height } => write!( + PrevHashMismatch { at_height } => write!( f, "The parent hash of proposed block does not correspond to the block hash at height {}.", at_height ), - ScanError::BlockHeightDiscontinuity { prev_height, new_height } => { + BlockHeightDiscontinuity { prev_height, new_height } => { write!(f, "Block height discontinuity at height {}; next height is : {}", prev_height, new_height) } - ScanError::TreeSizeMismatch { protocol, at_height, given, computed } => { + TreeSizeMismatch { protocol, at_height, given, computed } => { write!(f, "The {:?} note commitment tree size provided by a compact block did not match the expected size at height {}; given {}, expected {}", protocol, at_height, given, computed) } - ScanError::TreeSizeUnknown { protocol, at_height } => { + TreeSizeUnknown { protocol, at_height } => { write!(f, "Unable to determine {:?} note commitment tree size at height {}", protocol, at_height) } } diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 41d1ae313a..9091899754 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -26,10 +26,14 @@ pub mod migrations; /// Starting at `from_height`, the `with_row` callback is invoked with each block retrieved from /// the backing store. If the `limit` value provided is `None`, all blocks are traversed up to the /// maximum height. +/// +/// # Panics +/// +/// Panics if the provided `limit` value exceeds the range of a u32 pub(crate) fn blockdb_with_blocks( block_source: &BlockDb, from_height: Option, - limit: Option, + limit: Option, mut with_row: F, ) -> Result<(), Error> where @@ -196,11 +200,15 @@ pub(crate) fn blockmetadb_find_block( /// Starting at `from_height`, the `with_row` callback is invoked with each block retrieved from /// the backing store. If the `limit` value provided is `None`, all blocks are traversed up to the /// maximum height for which metadata is available. +/// +/// # Panics +/// +/// Panics if the provided `limit` value exceeds the range of a u32 #[cfg(feature = "unstable")] pub(crate) fn fsblockdb_with_blocks( cache: &FsBlockDb, from_height: Option, - limit: Option, + limit: Option, mut with_block: F, ) -> Result<(), Error> where @@ -225,7 +233,9 @@ where .query_map( params![ from_height.map_or(0u32, u32::from), - limit.unwrap_or(u32::max_value()), + limit + .and_then(|l| u32::try_from(l).ok()) + .unwrap_or(u32::MAX) ], |row| { Ok(BlockMeta { @@ -281,11 +291,12 @@ mod tests { use zcash_client_backend::{ address::RecipientAddress, data_api::{ - chain::scan_cached_blocks, + chain::{error::Error, scan_cached_blocks}, wallet::{input_selection::GreedyInputSelector, spend}, WalletRead, WalletWrite, }, fees::{zip317::SingleOutputChangeStrategy, DustOutputPolicy}, + scanning::ScanError, wallet::OvkPolicy, zip321::{Payment, TransactionRequest}, }; @@ -317,12 +328,9 @@ mod tests { assert_matches!(db_data.get_max_height_hash(), Ok(None)); // Create a fake CompactBlock sending value to the address - let fake_block_hash = BlockHash([0; 32]); - let fake_block_height = sapling_activation_height(); - let (cb, _) = fake_compact_block( - fake_block_height, - fake_block_hash, + sapling_activation_height(), + BlockHash([0; 32]), &dfvk, AddressType::DefaultExternal, Amount::from_u64(5).unwrap(), @@ -350,97 +358,24 @@ mod tests { Amount::from_u64(7).unwrap(), 1, ); - insert_into_cache(&db_cache, &cb2); - - // Scan the cache again - scan_cached_blocks( - &tests::network(), - &db_cache, - &mut db_data, - sapling_activation_height() + 1, - 1, - ) - .unwrap(); - } - - #[test] - fn invalid_chain_cache_disconnected() { - let cache_file = NamedTempFile::new().unwrap(); - let db_cache = BlockDb::for_path(cache_file.path()).unwrap(); - init_cache_database(&db_cache).unwrap(); - let data_file = NamedTempFile::new().unwrap(); - let mut db_data = WalletDb::for_path(data_file.path(), tests::network()).unwrap(); - init_wallet_db(&mut db_data, Some(Secret::new(vec![]))).unwrap(); - - // Add an account to the wallet - let (dfvk, _taddr) = init_test_accounts_table(&mut db_data); - - // Create some fake CompactBlocks - let (cb, _) = fake_compact_block( - sapling_activation_height(), - BlockHash([0; 32]), - &dfvk, - AddressType::DefaultExternal, - Amount::from_u64(5).unwrap(), - 0, - ); - let (cb2, _) = fake_compact_block( - sapling_activation_height() + 1, - cb.hash(), - &dfvk, - AddressType::DefaultExternal, - Amount::from_u64(7).unwrap(), - 1, - ); - insert_into_cache(&db_cache, &cb); insert_into_cache(&db_cache, &cb2); - // Scan the cache - scan_cached_blocks( - &tests::network(), - &db_cache, - &mut db_data, - sapling_activation_height(), - 2, - ) - .unwrap(); - - // Create more fake CompactBlocks that don't connect to the scanned ones - let (cb3, _) = fake_compact_block( - sapling_activation_height() + 2, - BlockHash([1; 32]), - &dfvk, - AddressType::DefaultExternal, - Amount::from_u64(8).unwrap(), - 2, - ); - let (cb4, _) = fake_compact_block( - sapling_activation_height() + 3, - cb3.hash(), - &dfvk, - AddressType::DefaultExternal, - Amount::from_u64(3).unwrap(), - 3, - ); - insert_into_cache(&db_cache, &cb3); - insert_into_cache(&db_cache, &cb4); - - // Data+cache chain should be invalid at the data/cache boundary + // Scanning should detect no inconsistencies assert_matches!( scan_cached_blocks( &tests::network(), &db_cache, &mut db_data, - sapling_activation_height() + 2, - 2 + sapling_activation_height() + 1, + 1, ), - Err(_) // FIXME: check error result more closely + Ok(()) ); } #[test] - fn invalid_chain_cache_reorg() { + fn invalid_chain_cache_disconnected() { let cache_file = NamedTempFile::new().unwrap(); let db_cache = BlockDb::for_path(cache_file.path()).unwrap(); init_cache_database(&db_cache).unwrap(); @@ -472,20 +407,22 @@ mod tests { insert_into_cache(&db_cache, &cb); insert_into_cache(&db_cache, &cb2); - // Scan the cache - scan_cached_blocks( - &tests::network(), - &db_cache, - &mut db_data, - sapling_activation_height(), - 2, - ) - .unwrap(); + // Scanning the cache should find no inconsistencies + assert_matches!( + scan_cached_blocks( + &tests::network(), + &db_cache, + &mut db_data, + sapling_activation_height(), + 2, + ), + Ok(()) + ); - // Create more fake CompactBlocks that contain a reorg + // Create more fake CompactBlocks that don't connect to the scanned ones let (cb3, _) = fake_compact_block( sapling_activation_height() + 2, - cb2.hash(), + BlockHash([1; 32]), &dfvk, AddressType::DefaultExternal, Amount::from_u64(8).unwrap(), @@ -493,7 +430,7 @@ mod tests { ); let (cb4, _) = fake_compact_block( sapling_activation_height() + 3, - BlockHash([1; 32]), + cb3.hash(), &dfvk, AddressType::DefaultExternal, Amount::from_u64(3).unwrap(), @@ -502,7 +439,7 @@ mod tests { insert_into_cache(&db_cache, &cb3); insert_into_cache(&db_cache, &cb4); - // Data+cache chain should be invalid inside the cache + // Data+cache chain should be invalid at the data/cache boundary assert_matches!( scan_cached_blocks( &tests::network(), @@ -511,7 +448,8 @@ mod tests { sapling_activation_height() + 2, 2 ), - Err(_) // FIXME: check error result more closely + Err(Error::Scan(ScanError::PrevHashMismatch { at_height })) + if at_height == sapling_activation_height() + 2 ); } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index ebc8fb56e2..f564f5447e 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -486,9 +486,9 @@ impl WalletWrite for WalletDb }) } - fn update_chain_tip(&mut self, block_metadata: BlockMetadata) -> Result<(), Self::Error> { + fn update_chain_tip(&mut self, tip_height: BlockHeight) -> Result<(), Self::Error> { let tx = self.conn.transaction()?; - wallet::scanning::update_chain_tip(&tx, &self.params, block_metadata)?; + wallet::scanning::update_chain_tip(&tx, &self.params, tip_height)?; tx.commit()?; Ok(()) } @@ -779,7 +779,7 @@ impl BlockSource for BlockDb { fn with_blocks( &self, from_height: Option, - limit: Option, + limit: Option, with_row: F, ) -> Result<(), data_api::chain::error::Error> where @@ -957,7 +957,7 @@ impl BlockSource for FsBlockDb { fn with_blocks( &self, from_height: Option, - limit: Option, + limit: Option, with_row: F, ) -> Result<(), data_api::chain::error::Error> where diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 659b5f5191..a6aa6914c3 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -68,6 +68,7 @@ use rusqlite::{self, named_params, OptionalExtension, ToSql}; use std::collections::HashMap; use std::convert::TryFrom; use std::io::{self, Cursor}; +use zcash_client_backend::data_api::scanning::{ScanPriority, ScanRange}; use zcash_client_backend::data_api::ShieldedProtocol; use zcash_primitives::transaction::TransactionData; @@ -92,10 +93,13 @@ use zcash_client_backend::{ wallet::WalletTx, }; +use crate::VALIDATION_DEPTH; use crate::{ error::SqliteClientError, SqlTransaction, WalletCommitmentTrees, WalletDb, PRUNING_DEPTH, }; +use self::scanning::replace_queue_entries; + #[cfg(feature = "transparent-inputs")] use { crate::UtxoId, @@ -631,8 +635,8 @@ pub(crate) fn block_metadata( ) -> Result, SqliteClientError> { conn.query_row( "SELECT height, hash, sapling_commitment_tree_size, sapling_tree - FROM blocks - WHERE height = :block_height", + FROM blocks + WHERE height = :block_height", named_params![":block_height": u32::from(block_height)], |row| { let height: u32 = row.get(0)?; @@ -811,6 +815,11 @@ pub(crate) fn truncate_to_height( "DELETE FROM blocks WHERE height > ?", [u32::from(block_height)], )?; + + // Prioritize the range starting at the height we just rewound to for verification + let query_range = block_height..(block_height + VALIDATION_DEPTH); + let scan_range = ScanRange::from_parts(query_range.clone(), ScanPriority::Verify); + replace_queue_entries(conn, &query_range, Some(scan_range).into_iter())?; } Ok(()) diff --git a/zcash_client_sqlite/src/wallet/scanning.rs b/zcash_client_sqlite/src/wallet/scanning.rs index 6b43e47c95..921e24e3b3 100644 --- a/zcash_client_sqlite/src/wallet/scanning.rs +++ b/zcash_client_sqlite/src/wallet/scanning.rs @@ -24,7 +24,7 @@ use zcash_client_backend::data_api::scanning::{ScanPriority, ScanRange}; use incrementalmerkletree::{Address, Position}; use zcash_primitives::consensus::{self, BlockHeight, NetworkUpgrade}; -use zcash_client_backend::data_api::{BlockMetadata, SAPLING_SHARD_HEIGHT}; +use zcash_client_backend::data_api::SAPLING_SHARD_HEIGHT; use crate::error::SqliteClientError; use crate::{PRUNING_DEPTH, VALIDATION_DEPTH}; @@ -70,7 +70,7 @@ pub(crate) fn suggest_scan_ranges( "SELECT block_range_start, block_range_end, priority FROM scan_queue WHERE priority > :scanned_priority - ORDER BY priority DESC", + ORDER BY priority, block_range_end DESC", )?; let mut rows = stmt_scan_ranges.query(named_params![ @@ -453,6 +453,28 @@ impl SpanningTree { } } +pub(crate) fn insert_queue_entries<'a>( + conn: &rusqlite::Connection, + entries: impl Iterator, +) -> Result<(), rusqlite::Error> { + let mut stmt = conn.prepare_cached( + "INSERT INTO scan_queue (block_range_start, block_range_end, priority) + VALUES (:block_range_start, :block_range_end, :priority)", + )?; + + for entry in entries { + if entry.block_range().end > entry.block_range().start { + stmt.execute(named_params![ + ":block_range_start": u32::from(entry.block_range().start) , + ":block_range_end": u32::from(entry.block_range().end), + ":priority": priority_code(&entry.priority()) + ])?; + } + } + + Ok(()) +} + pub(crate) fn replace_queue_entries( conn: &rusqlite::Connection, query_range: &Range, @@ -543,26 +565,6 @@ pub(crate) fn replace_queue_entries( Ok(()) } -pub(crate) fn insert_queue_entries<'a>( - conn: &rusqlite::Connection, - entries: impl Iterator, -) -> Result<(), rusqlite::Error> { - let mut stmt = conn.prepare_cached( - "INSERT INTO scan_queue (block_range_start, block_range_end, priority) - VALUES (:block_range_start, :block_range_end, :priority)", - )?; - - for entry in entries { - stmt.execute(named_params![ - ":block_range_start": u32::from(entry.block_range().start) , - ":block_range_end": u32::from(entry.block_range().end), - ":priority": priority_code(&entry.priority()) - ])?; - } - - Ok(()) -} - pub(crate) fn scan_complete( conn: &rusqlite::Transaction<'_>, params: &P, @@ -658,58 +660,82 @@ pub(crate) fn scan_complete( pub(crate) fn update_chain_tip( conn: &rusqlite::Transaction<'_>, params: &P, - block_metadata: BlockMetadata, + new_tip: BlockHeight, ) -> Result<(), SqliteClientError> { - // get the index of the shard that contains the chain tip - let tip_shard_idx = block_metadata.sapling_tree_size() >> SAPLING_SHARD_HEIGHT; - let shard_start_height = if tip_shard_idx > 0 { - // Read the start height from the shards table. The shard at the tip is likely to only be - // partially full, so we query for the last two entries. - let shard_idxs = vec![Value::from(tip_shard_idx), Value::from(tip_shard_idx - 1)]; - let shard_idxs_ptr = Rc::new(shard_idxs); - conn.query_row( - "SELECT MAX(subtree_end_height) - FROM sapling_tree_shards - WHERE shard_index IN rarray(:shard_indices)", - named_params![":shard_indices": shard_idxs_ptr], - |row| Ok(row.get::<_, Option>(1)?.map(BlockHeight::from)), - )? - } else { - params.activation_height(NetworkUpgrade::Sapling) - }; + // Read the maximum height from the shards table. + let shard_start_height = conn.query_row( + "SELECT MAX(subtree_end_height) + FROM sapling_tree_shards", + [], + |row| Ok(row.get::<_, Option>(1)?.map(BlockHeight::from)), + )?; - // create a scanning range for the fragment of the last shard leading up to the shard height + // Create a scanning range for the fragment of the last shard leading up to new tip. + // However, only do so if the start of the shard is at a stable height. let shard_entry = shard_start_height - .filter(|h| h < &block_metadata.block_height()) - .map(|h| ScanRange::from_parts(h..block_metadata.block_height(), ScanPriority::ChainTip)); - - // get the wallet's prior view of the chain tip - let tip_entry = block_height_extrema(conn)?.map(|(_, max_height)| { - // if the wallet's prior tip is within PRUNING_DEPTH of the provided chain tip, prioritize the - // difference as `ChainTip`. Otherwise, prioritize the `VALIDATION_DEPTH` blocks up to the - // wallet's prior tip as `Verify` - if max_height + PRUNING_DEPTH >= block_metadata.block_height() { - ScanRange::from_parts( - max_height..block_metadata.block_height(), - ScanPriority::ChainTip, - ) + .filter(|h| h < &new_tip) + .map(|h| ScanRange::from_parts(h..new_tip, ScanPriority::ChainTip)); + + // Create scanning ranges to either validate potentially invalid blocks at the wallet's view + // of the chain tip, + let tip_entry = block_height_extrema(conn)?.map(|(_, prior_tip)| { + // If we don't have shard metadata, this means we're doing linear scanning, so create a + // scan range from the prior tip to the current tip with `Historic` priority. + if shard_entry.is_none() { + ScanRange::from_parts(prior_tip..new_tip, ScanPriority::Historic) } else { - ScanRange::from_parts( - BlockHeight::from(u32::from(max_height).saturating_sub(VALIDATION_DEPTH)) - ..(max_height + 1), - ScanPriority::Verify, - ) + // Determine the height to which we expect blocks retrieved from the block source to be stable + // and not subject to being reorg'ed. + let stable_height = new_tip.saturating_sub(PRUNING_DEPTH); + + // if the wallet's prior tip is above the stable height, prioritize the range between + // it and the new tip as `ChainTip`. Otherwise, prioritize the `VALIDATION_DEPTH` + // blocks above the wallet's prior tip as `Verify`. Since `scan_cached_blocks` + // retrieves the metadata for the block being connected to, the connectivity to the + // prior tip will always be checked. Since `Verify` ranges have maximum priority, even + // if the block source begins downloading blocks from the shard scan range (which ends + // at the stable height) the scanner should not attempt to scan those blocks until the + // tip range has been completely checked and any required rewinds have been performed. + if prior_tip >= stable_height { + // This may overlap the `shard_entry` range and if so will be coalesced with it. + ScanRange::from_parts(prior_tip..new_tip, ScanPriority::ChainTip) + } else { + // The prior tip is in the range that we now expect to be stable, so we need to verify + // and advance it up to at most the stable height. The shard entry will then cover + // the range to the new tip at the lower `ChainTip` priority. + ScanRange::from_parts( + prior_tip..max(stable_height, prior_tip + VALIDATION_DEPTH), + ScanPriority::Verify, + ) + } } }); - if let Some(se) = shard_entry { - let range = se.block_range().clone(); - replace_queue_entries(conn, &range, Some(se).into_iter())?; - } + let query_range = match (shard_entry.as_ref(), tip_entry.as_ref()) { + (Some(se), Some(te)) => Some(Range { + start: min(se.block_range().start, te.block_range().start), + end: max(se.block_range().end, te.block_range().end), + }), + (Some(se), None) => Some(se.block_range().clone()), + (None, Some(te)) => Some(te.block_range().clone()), + (None, None) => None, + }; - if let Some(te) = tip_entry { - let range = te.block_range().clone(); - replace_queue_entries(conn, &range, Some(te).into_iter())?; + if let Some(query_range) = query_range { + replace_queue_entries( + conn, + &query_range, + shard_entry.into_iter().chain(tip_entry.into_iter()), + )?; + } else { + // If we have neither shard data nor any existing block data in the database, we should also + // have no existing scan queue entries and can fall back to linear scanning from Sapling + // activation. + if let Some(sapling_activation) = params.activation_height(NetworkUpgrade::Sapling) { + let scan_range = + ScanRange::from_parts(sapling_activation..new_tip, ScanPriority::Historic); + insert_queue_entries(conn, Some(scan_range).iter())?; + } } Ok(()) diff --git a/zcash_primitives/src/consensus.rs b/zcash_primitives/src/consensus.rs index 563c698067..35dea22dfb 100644 --- a/zcash_primitives/src/consensus.rs +++ b/zcash_primitives/src/consensus.rs @@ -23,6 +23,10 @@ impl BlockHeight { pub const fn from_u32(v: u32) -> BlockHeight { BlockHeight(v) } + + pub fn saturating_sub(self, v: u32) -> BlockHeight { + BlockHeight(self.0.saturating_sub(v)) + } } impl fmt::Display for BlockHeight {