From e3aafdad19d0097819f0c6f5925e4e063d3fd474 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Sat, 1 Jul 2023 18:16:23 -0600 Subject: [PATCH] Move chain continuity checks into `scan_block_with_runner` In preparation for out-of-order range-based scanning, it is necessary to ensure that the size of the Sapling note commitment tree is carried along through the scan process and that stored blocks are always persisted with the updated note commitment tree size. --- zcash_client_backend/CHANGELOG.md | 25 +-- zcash_client_backend/src/data_api.rs | 121 +++++++++++-- zcash_client_backend/src/data_api/chain.rs | 142 ++++++--------- .../src/data_api/chain/error.rs | 156 ++--------------- zcash_client_backend/src/data_api/wallet.rs | 6 +- zcash_client_backend/src/lib.rs | 2 +- zcash_client_backend/src/welding_rig.rs | 162 ++++++++++++------ zcash_client_sqlite/CHANGELOG.md | 6 + zcash_client_sqlite/src/chain.rs | 16 +- zcash_client_sqlite/src/error.rs | 13 +- zcash_client_sqlite/src/lib.rs | 99 +++++------ zcash_client_sqlite/src/wallet.rs | 145 +++++++++++----- zcash_client_sqlite/src/wallet/init.rs | 49 +++++- zcash_client_sqlite/src/wallet/sapling.rs | 45 +++-- zcash_primitives/src/block.rs | 18 +- 15 files changed, 548 insertions(+), 457 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 8a6fc7f36..5df41eb40 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -11,14 +11,16 @@ 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`: - - `WalletRead::{fully_scanned_height, suggest_scan_ranges}` + - `WalletRead::{block_metadata, block_fully_scanned, suggest_scan_ranges}` - `WalletWrite::put_block` - `WalletCommitmentTrees` - `testing::MockWalletDb::new` - `NullifierQuery` for use with `WalletRead::get_sapling_nullifiers` + - `BlockMetadata` + - `ScannedBlock` - `wallet::input_sellection::Proposal::{min_target_height, min_anchor_height}`: - `zcash_client_backend::wallet::WalletSaplingOutput::note_commitment_tree_position` -- `zcash_client_backend::welding_rig::SyncError` +- `zcash_client_backend::welding_rig::ScanError` ### Changed - MSRV is now 1.65.0. @@ -56,27 +58,28 @@ and this library adheres to Rust's notion of - `zcash_client_backend::welding_rig::ScanningKey::sapling_nf` has been changed to take a note position instead of an incremental witness for the note. - Arguments to `zcash_client_backend::welding_rig::scan_block` have changed. This - method now takes an optional `CommitmentTreeMeta` argument instead of a base commitment - tree and incremental witnesses for each previously-known note. + method now takes an optional `BlockMetadata` argument instead of a base commitment + tree and incremental witnesses for each previously-known note. In addition, the + return type has now been updated to return a `Result` + in the case of scan failure. ### Removed - `zcash_client_backend::data_api`: - `WalletRead::get_all_nullifiers` - - `WalletRead::{get_commitment_tree, get_witnesses}` (use - `WalletRead::fully_scanned_height` instead). + - `WalletRead::{get_commitment_tree, get_witnesses}` have been removed + without replacement. The utility of these methods is now subsumed + by those available from the `WalletCommitmentTrees` trait. - `WalletWrite::advance_by_block` (use `WalletWrite::put_block` instead). - - The `commitment_tree` and `transactions` properties of the `PrunedBlock` - struct are now owned values instead of references, making this a fully owned type. - It is now parameterized by the nullifier type instead of by a lifetime for the - fields that were previously references. In addition, two new properties, - `sapling_commitment_tree_size` and `sapling_commitments` have been added. + - `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. - `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::welding_rig::ScanError` - `zcash_client_backend::wallet::WalletSaplingOutput::{witness, witness_mut}` have been removed as individual incremental witnesses are no longer tracked on a per-note basis. The global note commitment tree for the wallet should be used diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 5b199b422..6c5f8a115 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -66,15 +66,17 @@ pub trait WalletRead { /// This will return `Ok(None)` if no block data is present in the database. fn block_height_extrema(&self) -> Result, Self::Error>; - /// Returns the height to which the wallet has been fully scanned. + /// Returns the available block metadata for the block at the specified height, if any. + fn block_metadata(&self, height: BlockHeight) -> Result, Self::Error>; + + /// Returns the metadata for the block at the height to which the wallet has been fully + /// scanned. /// /// This is the height for which the wallet has fully trial-decrypted this and all preceding /// blocks above the wallet's birthday height. Along with this height, this method returns /// metadata describing the state of the wallet's note commitment trees as of the end of that /// block. - fn fully_scanned_height( - &self, - ) -> Result, Self::Error>; + fn block_fully_scanned(&self) -> Result, Self::Error>; /// Returns a vector of suggested scan ranges based upon the current wallet state. /// @@ -248,17 +250,99 @@ pub trait WalletRead { ) -> Result, Self::Error>; } +/// Metadata describing the sizes of the zcash note commitment trees as of a particular block. +#[derive(Debug, Clone, Copy)] +pub struct BlockMetadata { + block_height: BlockHeight, + block_hash: BlockHash, + sapling_tree_size: u32, + //TODO: orchard_tree_size: u32 +} + +impl BlockMetadata { + /// Constructs a new [`BlockMetadata`] value from its constituent parts. + pub fn from_parts( + block_height: BlockHeight, + block_hash: BlockHash, + sapling_tree_size: u32, + ) -> Self { + Self { + block_height, + block_hash, + sapling_tree_size, + } + } + + /// Returns the block height. + pub fn block_height(&self) -> BlockHeight { + self.block_height + } + + /// Returns the hash of the block + pub fn block_hash(&self) -> BlockHash { + self.block_hash + } + + /// Returns the size of the Sapling note commitment tree as of the block that this + /// [`BlockMetadata`] describes. + pub fn sapling_tree_size(&self) -> u32 { + self.sapling_tree_size + } +} + /// The subset of information that is relevant to this wallet that has been /// decrypted and extracted from a [`CompactBlock`]. /// /// [`CompactBlock`]: crate::proto::compact_formats::CompactBlock -pub struct PrunedBlock { - pub block_height: BlockHeight, - pub block_hash: BlockHash, - pub block_time: u32, - pub transactions: Vec>, - pub sapling_commitment_tree_size: Option, - pub sapling_commitments: Vec<(sapling::Node, Retention)>, +pub struct ScannedBlock { + metadata: BlockMetadata, + block_time: u32, + transactions: Vec>, + sapling_commitments: Vec<(sapling::Node, Retention)>, +} + +impl ScannedBlock { + pub fn from_parts( + metadata: BlockMetadata, + block_time: u32, + transactions: Vec>, + sapling_commitments: Vec<(sapling::Node, Retention)>, + ) -> Self { + Self { + metadata, + block_time, + transactions, + sapling_commitments, + } + } + + pub fn height(&self) -> BlockHeight { + self.metadata.block_height + } + + pub fn block_hash(&self) -> BlockHash { + self.metadata.block_hash + } + + pub fn block_time(&self) -> u32 { + self.block_time + } + + pub fn metadata(&self) -> &BlockMetadata { + &self.metadata + } + + pub fn transactions(&self) -> &[WalletTx] { + &self.transactions + } + + pub fn sapling_commitments(&self) -> &[(sapling::Node, Retention)] { + &self.sapling_commitments + } + + pub fn take_sapling_commitments(self) -> Vec<(sapling::Node, Retention)> { + self.sapling_commitments + } } /// A transaction that was detected during scanning of the blockchain, @@ -404,7 +488,7 @@ pub trait WalletWrite: WalletRead { #[allow(clippy::type_complexity)] fn put_block( &mut self, - block: PrunedBlock, + block: ScannedBlock, ) -> Result, Self::Error>; /// Caches a decrypted transaction in the persistent wallet store. @@ -489,7 +573,7 @@ pub mod testing { }; use super::{ - chain, DecryptedTransaction, NullifierQuery, PrunedBlock, SentTransaction, + BlockMetadata, DecryptedTransaction, NullifierQuery, ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletWrite, SAPLING_SHARD_HEIGHT, }; @@ -520,9 +604,14 @@ pub mod testing { Ok(None) } - fn fully_scanned_height( + fn block_metadata( &self, - ) -> Result, Self::Error> { + _height: BlockHeight, + ) -> Result, Self::Error> { + Ok(None) + } + + fn block_fully_scanned(&self) -> Result, Self::Error> { Ok(None) } @@ -667,7 +756,7 @@ pub mod testing { #[allow(clippy::type_complexity)] fn put_block( &mut self, - _block: PrunedBlock, + _block: ScannedBlock, ) -> Result, Self::Error> { Ok(vec![]) } diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index a55470307..8e1ceb864 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -17,7 +17,6 @@ //! BlockSource, //! error::Error, //! scan_cached_blocks, -//! validate_chain, //! testing as chain_testing, //! }, //! testing, @@ -30,7 +29,7 @@ //! # test(); //! # } //! # -//! # fn test() -> Result<(), Error<(), Infallible, u32>> { +//! # fn test() -> Result<(), Error<(), Infallible>> { //! let network = Network::TestNetwork; //! let block_source = chain_testing::MockBlockSource; //! let mut db_data = testing::MockWalletDb::new(Network::TestNetwork); @@ -38,7 +37,7 @@ //! // 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. @@ -65,26 +64,7 @@ use crate::{ }; pub mod error; -use error::{ChainError, Error}; - -/// Metadata describing the sizes of the zcash note commitment trees as of a particular block. -pub struct CommitmentTreeMeta { - sapling_tree_size: u32, - //TODO: orchard_tree_size: u32 -} - -impl CommitmentTreeMeta { - /// Constructs a new [`CommitmentTreeMeta`] value from its constituent parts. - pub fn from_parts(sapling_tree_size: u32) -> Self { - Self { sapling_tree_size } - } - - /// Returns the size of the Sapling note commitment tree as of the block that this - /// [`CommitmentTreeMeta`] describes. - pub fn sapling_tree_size(&self) -> u32 { - self.sapling_tree_size - } -} +use error::Error; /// This trait provides sequential access to raw blockchain data via a callback-oriented /// API. @@ -99,14 +79,14 @@ pub trait BlockSource { /// as part of processing each row. /// * `NoteRefT`: the type of note identifiers in the wallet data store, for use in /// reporting errors related to specific notes. - fn with_blocks( + fn with_blocks( &self, from_height: Option, limit: Option, with_row: F, - ) -> Result<(), error::Error> + ) -> Result<(), error::Error> where - F: FnMut(CompactBlock) -> Result<(), error::Error>; + F: FnMut(CompactBlock) -> Result<(), error::Error>; } /// Scans at most `limit` new blocks added to the block source for any transactions received by the @@ -133,7 +113,7 @@ pub fn scan_cached_blocks( data_db: &mut DbT, from_height: Option, limit: Option, -) -> Result<(), Error> +) -> Result<(), Error> where ParamsT: consensus::Parameters + Send + 'static, BlockSourceT: BlockSource, @@ -169,74 +149,60 @@ where ); // Start at either the provided height, or where we synced up to previously. - let (scan_from, commitment_tree_meta) = from_height.map_or_else( - || { - data_db.fully_scanned_height().map_or_else( - |e| Err(Error::Wallet(e)), - |last_scanned| { - Ok(last_scanned.map_or_else(|| (None, None), |(h, m)| (Some(h + 1), Some(m)))) + let (scan_from, mut prior_block_metadata) = match from_height { + Some(h) => { + // if we are provided with a starting height, obtain the metadata for the previous + // block (if any is available) + ( + Some(h), + if h > BlockHeight::from(0) { + data_db.block_metadata(h - 1).map_err(Error::Wallet)? + } else { + None }, ) - }, - |h| Ok((Some(h), None)), - )?; + } + None => { + let last_scanned = data_db.block_fully_scanned().map_err(Error::Wallet)?; + last_scanned.map_or_else(|| (None, None), |m| (Some(m.block_height + 1), Some(m))) + } + }; - block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>( - scan_from, - limit, - |block: CompactBlock| { - add_block_to_runner(params, block, &mut batch_runner); - Ok(()) - }, - )?; + block_source.with_blocks::<_, DbT::Error>(scan_from, limit, |block: CompactBlock| { + add_block_to_runner(params, block, &mut batch_runner); + Ok(()) + })?; batch_runner.flush(); - let mut last_scanned = None; - block_source.with_blocks::<_, DbT::Error, DbT::NoteRef>( - scan_from, - limit, - |block: CompactBlock| { - // block heights should be sequential within a single scan range - let block_height = block.height(); - if let Some(h) = last_scanned { - if block_height != h + 1 { - return Err(Error::Chain(ChainError::block_height_discontinuity( - block.height(), - h, - ))); - } - } + block_source.with_blocks::<_, DbT::Error>(scan_from, limit, |block: CompactBlock| { + let scanned_block = scan_block_with_runner( + params, + block, + &dfvks, + &sapling_nullifiers, + prior_block_metadata.as_ref(), + Some(&mut batch_runner), + ) + .map_err(Error::Scan)?; - let pruned_block = scan_block_with_runner( - params, - block, - &dfvks, - &sapling_nullifiers, - commitment_tree_meta.as_ref(), - Some(&mut batch_runner), - ) - .map_err(Error::Sync)?; + let spent_nf: Vec<&sapling::Nullifier> = scanned_block + .transactions + .iter() + .flat_map(|tx| tx.sapling_spends.iter().map(|spend| spend.nf())) + .collect(); - let spent_nf: Vec<&sapling::Nullifier> = pruned_block - .transactions + sapling_nullifiers.retain(|(_, nf)| !spent_nf.contains(&nf)); + sapling_nullifiers.extend(scanned_block.transactions.iter().flat_map(|tx| { + tx.sapling_outputs .iter() - .flat_map(|tx| tx.sapling_spends.iter().map(|spend| spend.nf())) - .collect(); + .map(|out| (out.account(), *out.nf())) + })); - sapling_nullifiers.retain(|(_, nf)| !spent_nf.contains(&nf)); - sapling_nullifiers.extend(pruned_block.transactions.iter().flat_map(|tx| { - tx.sapling_outputs - .iter() - .map(|out| (out.account(), *out.nf())) - })); - - data_db.put_block(pruned_block).map_err(Error::Wallet)?; - - last_scanned = Some(block_height); - Ok(()) - }, - )?; + prior_block_metadata = Some(*scanned_block.metadata()); + data_db.put_block(scanned_block).map_err(Error::Wallet)?; + Ok(()) + })?; Ok(()) } @@ -255,14 +221,14 @@ pub mod testing { impl BlockSource for MockBlockSource { type Error = Infallible; - fn with_blocks( + fn with_blocks( &self, _from_height: Option, _limit: Option, _with_row: F, - ) -> Result<(), Error> + ) -> Result<(), Error> where - F: FnMut(CompactBlock) -> Result<(), Error>, + F: FnMut(CompactBlock) -> Result<(), Error>, { Ok(()) } diff --git a/zcash_client_backend/src/data_api/chain/error.rs b/zcash_client_backend/src/data_api/chain/error.rs index b28380d13..c1c78cf61 100644 --- a/zcash_client_backend/src/data_api/chain/error.rs +++ b/zcash_client_backend/src/data_api/chain/error.rs @@ -3,136 +3,11 @@ use std::error; use std::fmt::{self, Debug, Display}; -use zcash_primitives::{consensus::BlockHeight, sapling, transaction::TxId}; - -use crate::welding_rig::SyncError; - -/// The underlying cause of a [`ChainError`]. -#[derive(Copy, Clone, Debug)] -pub enum Cause { - /// The hash of the parent block given by a proposed new chain tip does not match the hash of - /// the current chain tip. - PrevHashMismatch, - - /// The block height field of the proposed new chain tip is not equal to the height of the - /// previous chain tip + 1. This variant stores a copy of the incorrect height value for - /// reporting purposes. - BlockHeightDiscontinuity(BlockHeight), - - /// The root of an output's witness tree in a newly arrived transaction does not correspond to - /// root of the stored commitment tree at the recorded height. - /// - /// This error is currently only produced when performing the slow checks that are enabled by - /// compiling with `-C debug-assertions`. - InvalidNewWitnessAnchor { - /// The id of the transaction containing the mismatched witness. - txid: TxId, - /// The index of the shielded output within the transaction where the witness root does not - /// match. - index: usize, - /// The root of the witness that failed to match the root of the current note commitment - /// tree. - node: sapling::Node, - }, - - /// The root of an output's witness tree in a previously stored transaction does not correspond - /// to root of the current commitment tree. - /// - /// This error is currently only produced when performing the slow checks that are enabled by - /// compiling with `-C debug-assertions`. - InvalidWitnessAnchor(NoteRef), -} - -/// Errors that may occur in chain scanning or validation. -#[derive(Copy, Clone, Debug)] -pub struct ChainError { - at_height: BlockHeight, - cause: Cause, -} - -impl fmt::Display for ChainError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match &self.cause { - Cause::PrevHashMismatch => write!( - f, - "The parent hash of proposed block does not correspond to the block hash at height {}.", - self.at_height - ), - Cause::BlockHeightDiscontinuity(h) => { - write!(f, "Block height discontinuity at height {}; next height is : {}", self.at_height, h) - } - Cause::InvalidNewWitnessAnchor { txid, index, node } => write!( - f, - "New witness for output {} in tx {} at height {} has incorrect anchor: {:?}", - index, txid, self.at_height, node, - ), - Cause::InvalidWitnessAnchor(id_note) => { - write!(f, "Witness for note {} has incorrect anchor for height {}", id_note, self.at_height) - } - } - } -} - -impl ChainError { - /// Constructs an error that indicates block hashes failed to chain. - /// - /// * `at_height` the height of the block whose parent hash does not match the hash of the - /// previous block - pub fn prev_hash_mismatch(at_height: BlockHeight) -> Self { - ChainError { - at_height, - cause: Cause::PrevHashMismatch, - } - } - - /// Constructs an error that indicates a gap in block heights. - /// - /// * `at_height` the height of the block being added to the chain. - /// * `prev_chain_tip` the height of the previous chain tip. - pub fn block_height_discontinuity(at_height: BlockHeight, prev_chain_tip: BlockHeight) -> Self { - ChainError { - at_height, - cause: Cause::BlockHeightDiscontinuity(prev_chain_tip), - } - } - - /// Constructs an error that indicates a mismatch between an updated note's witness and the - /// root of the current note commitment tree. - pub fn invalid_witness_anchor(at_height: BlockHeight, note_ref: NoteRef) -> Self { - ChainError { - at_height, - cause: Cause::InvalidWitnessAnchor(note_ref), - } - } - - /// Constructs an error that indicates a mismatch between a new note's witness and the root of - /// the current note commitment tree. - pub fn invalid_new_witness_anchor( - at_height: BlockHeight, - txid: TxId, - index: usize, - node: sapling::Node, - ) -> Self { - ChainError { - at_height, - cause: Cause::InvalidNewWitnessAnchor { txid, index, node }, - } - } - - /// Returns the block height at which this error was discovered. - pub fn at_height(&self) -> BlockHeight { - self.at_height - } - - /// Returns the cause of this error. - pub fn cause(&self) -> &Cause { - &self.cause - } -} +use crate::welding_rig::ScanError; /// Errors related to chain validation and scanning. #[derive(Debug)] -pub enum Error { +pub enum Error { /// An error that was produced by wallet operations in the course of scanning the chain. Wallet(WalletError), @@ -143,13 +18,10 @@ pub enum Error { /// A block that was received violated rules related to chain continuity or contained note /// commitments that could not be reconciled with the note commitment tree(s) maintained by the /// wallet. - Chain(ChainError), - - /// An error occorred in block scanning. - Sync(SyncError), + Scan(ScanError), } -impl fmt::Display for Error { +impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match &self { Error::Wallet(e) => { @@ -166,25 +38,17 @@ impl fmt::Display for Error { - write!(f, "{}", err) - } - Error::Sync(SyncError::SaplingTreeSizeUnknown(h)) => { - write!( - f, - "Sync failed due to missing Sapling note commitment tree size at height {}", - h - ) + Error::Scan(e) => { + write!(f, "Scanning produced the following error: {}", e) } } } } -impl error::Error for Error +impl error::Error for Error where WE: Debug + Display + error::Error + 'static, BE: Debug + Display + error::Error + 'static, - N: Debug + Display, { fn source(&self) -> Option<&(dyn error::Error + 'static)> { match &self { @@ -195,8 +59,8 @@ where } } -impl From> for Error { - fn from(e: ChainError) -> Self { - Error::Chain(e) +impl From for Error { + fn from(e: ScanError) -> Self { + Error::Scan(e) } } diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index c423c02ea..7f3d4ce6a 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -1,5 +1,5 @@ -use std::{convert::Infallible, num::NonZeroU32}; use std::fmt::Debug; +use std::{convert::Infallible, num::NonZeroU32}; use shardtree::{ShardStore, ShardTree, ShardTreeError}; use zcash_primitives::{ @@ -394,7 +394,7 @@ pub fn propose_shielding( input_selector: &InputsT, shielding_threshold: NonNegativeAmount, from_addrs: &[TransparentAddress], - min_confirmations: NonZeroU32 + min_confirmations: NonZeroU32, ) -> Result< Proposal, Error< @@ -497,7 +497,7 @@ where selected, usk.sapling(), &dfvk, - usize::try_from(u32::from(min_confirmations) - 1).unwrap() + usize::try_from(u32::from(min_confirmations) - 1).unwrap(), )? .ok_or(Error::NoteMismatch(selected.note_id))?; diff --git a/zcash_client_backend/src/lib.rs b/zcash_client_backend/src/lib.rs index f73711662..1cb87bc9f 100644 --- a/zcash_client_backend/src/lib.rs +++ b/zcash_client_backend/src/lib.rs @@ -16,8 +16,8 @@ pub mod fees; pub mod keys; pub mod proto; pub mod scan; +pub mod scanning; pub mod wallet; -pub mod welding_rig; pub mod zip321; pub use decrypt::{decrypt_transaction, DecryptedOutput, TransferType}; diff --git a/zcash_client_backend/src/welding_rig.rs b/zcash_client_backend/src/welding_rig.rs index 79e3a4c39..a5d8e9c5d 100644 --- a/zcash_client_backend/src/welding_rig.rs +++ b/zcash_client_backend/src/welding_rig.rs @@ -4,6 +4,7 @@ use std::collections::{HashMap, HashSet}; use std::convert::TryFrom; +use std::fmt::{self, Debug}; use incrementalmerkletree::{Position, Retention}; use subtle::{ConditionallySelectable, ConstantTimeEq, CtOption}; @@ -20,8 +21,7 @@ use zcash_primitives::{ zip32::{sapling::DiversifiableFullViewingKey, AccountId, Scope}, }; -use crate::data_api::chain::CommitmentTreeMeta; -use crate::data_api::PrunedBlock; +use crate::data_api::{BlockMetadata, ScannedBlock}; use crate::{ proto::compact_formats::CompactBlock, scan::{Batch, BatchRunner, Tasks}, @@ -109,12 +109,42 @@ impl ScanningKey for SaplingIvk { fn sapling_nf(_key: &Self::SaplingNk, _note: &sapling::Note, _position: Position) {} } -/// Errors that can occur in block scanning. -#[derive(Debug)] -pub enum SyncError { +/// Errors that may occur in chain scanning +#[derive(Copy, 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. + PrevHashMismatch { at_height: BlockHeight }, + + /// The block height field of the proposed new chain tip is not equal to the height of the + /// previous chain tip + 1. This variant stores a copy of the incorrect height value for + /// reporting purposes. + BlockHeightDiscontinuity { + previous_tip: BlockHeight, + new_height: BlockHeight, + }, + /// The size of the Sapling note commitment tree was not provided as part of a [`CompactBlock`] /// being scanned, making it impossible to construct the nullifier for a detected note. - SaplingTreeSizeUnknown(BlockHeight), + SaplingTreeSizeUnknown { at_height: BlockHeight }, +} + +impl fmt::Display for ScanError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match &self { + ScanError::PrevHashMismatch { at_height } => write!( + f, + "The parent hash of proposed block does not correspond to the block hash at height {}.", + at_height + ), + ScanError::BlockHeightDiscontinuity { previous_tip, new_height } => { + write!(f, "Block height discontinuity at height {}; next height is : {}", previous_tip, new_height) + } + ScanError::SaplingTreeSizeUnknown { at_height } => { + write!(f, "Unable to determine Sapling note commitment tree size at height {}", at_height) + } + } + } } /// Scans a [`CompactBlock`] with a set of [`ScanningKey`]s. @@ -145,14 +175,14 @@ pub fn scan_block( block: CompactBlock, vks: &[(&AccountId, &K)], sapling_nullifiers: &[(AccountId, sapling::Nullifier)], - initial_commitment_tree_meta: Option<&CommitmentTreeMeta>, -) -> Result, SyncError> { + prior_block_metadata: Option<&BlockMetadata>, +) -> Result, ScanError> { scan_block_with_runner::<_, _, ()>( params, block, vks, sapling_nullifiers, - initial_commitment_tree_meta, + prior_block_metadata, None, ) } @@ -204,13 +234,28 @@ pub(crate) fn scan_block_with_runner< block: CompactBlock, vks: &[(&AccountId, &K)], nullifiers: &[(AccountId, sapling::Nullifier)], - initial_commitment_tree_meta: Option<&CommitmentTreeMeta>, + prior_block_metadata: Option<&BlockMetadata>, mut batch_runner: Option<&mut TaggedBatchRunner>, -) -> Result, SyncError> { +) -> Result, ScanError> { let mut wtxs: Vec> = vec![]; let mut sapling_note_commitments: Vec<(sapling::Node, Retention)> = vec![]; - let block_height = block.height(); - let block_hash = block.hash(); + let cur_height = block.height(); + let cur_hash = block.hash(); + + if let Some(prev) = prior_block_metadata { + if cur_height != prev.block_height() + 1 { + return Err(ScanError::BlockHeightDiscontinuity { + previous_tip: prev.block_height(), + new_height: cur_height, + }); + } + + if block.prev_hash() != prev.block_hash() { + return Err(ScanError::PrevHashMismatch { + at_height: cur_height, + }); + } + } // It's possible to make progress without a Sapling tree position if we don't have any Sapling // notes in the block, since we only use the position for constructing nullifiers for our own @@ -235,7 +280,10 @@ pub(crate) fn scan_block_with_runner< Some(m.sapling_commitment_tree_size - block_note_count) } }) - .or_else(|| initial_commitment_tree_meta.map(|m| m.sapling_tree_size())); + .or_else(|| prior_block_metadata.map(|m| m.sapling_tree_size())) + .ok_or(ScanError::SaplingTreeSizeUnknown { + at_height: cur_height, + })?; let block_tx_count = block.vtx.len(); for (tx_idx, tx) in block.vtx.into_iter().enumerate() { @@ -283,7 +331,7 @@ pub(crate) fn scan_block_with_runner< .into_iter() .map(|output| { ( - SaplingDomain::for_height(params.clone(), block_height), + SaplingDomain::for_height(params.clone(), cur_height), CompactOutputDescription::try_from(output) .expect("Invalid output found in compact block decoding."), ) @@ -300,7 +348,7 @@ pub(crate) fn scan_block_with_runner< }) .collect::>(); - let mut decrypted = runner.collect_results(block_hash, txid); + let mut decrypted = runner.collect_results(cur_hash, txid); (0..decoded.len()) .map(|i| { decrypted.remove(&(txid, i)).map(|d_note| { @@ -347,7 +395,7 @@ pub(crate) fn scan_block_with_runner< let is_checkpoint = output_idx + 1 == decoded.len() && tx_idx + 1 == block_tx_count; let retention = match (dec_output.is_some(), is_checkpoint) { (is_marked, true) => Retention::Checkpoint { - id: block_height, + id: cur_height, is_marked, }, (true, false) => Retention::Marked, @@ -362,9 +410,9 @@ pub(crate) fn scan_block_with_runner< // - Notes created by consolidation transactions. // - Notes sent from one account to itself. let is_change = spent_from_accounts.contains(&account); - let note_commitment_tree_position = sapling_commitment_tree_size - .map(|s| Position::from(u64::from(s + u32::try_from(output_idx).unwrap()))) - .ok_or(SyncError::SaplingTreeSizeUnknown(block_height))?; + let note_commitment_tree_position = Position::from(u64::from( + sapling_commitment_tree_size + u32::try_from(output_idx).unwrap(), + )); let nf = K::sapling_nf(&nk, ¬e, note_commitment_tree_position); shielded_outputs.push(WalletSaplingOutput::from_parts( @@ -392,17 +440,15 @@ pub(crate) fn scan_block_with_runner< }); } - sapling_commitment_tree_size = sapling_commitment_tree_size.map(|s| s + tx_outputs_len); + sapling_commitment_tree_size += tx_outputs_len; } - Ok(PrunedBlock { - block_height, - block_hash, - block_time: block.time, - transactions: wtxs, - sapling_commitment_tree_size, - sapling_commitments: sapling_note_commitments, - }) + Ok(ScannedBlock::from_parts( + BlockMetadata::from_parts(cur_height, cur_hash, sapling_commitment_tree_size), + block.time, + wtxs, + sapling_note_commitments, + )) } #[cfg(test)] @@ -415,6 +461,7 @@ mod tests { use rand_core::{OsRng, RngCore}; use zcash_note_encryption::Domain; use zcash_primitives::{ + block::BlockHash, consensus::{BlockHeight, Network}, constants::SPENDING_KEY_GENERATOR, memo::MemoBytes, @@ -430,9 +477,9 @@ mod tests { }; use crate::{ - data_api::chain::CommitmentTreeMeta, + data_api::BlockMetadata, proto::compact_formats::{ - BlockMetadata, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, + self as compact, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, }, scan::BatchRunner, }; @@ -479,6 +526,7 @@ mod tests { /// from a `lightwalletd` that is not currently tracking note commitment tree sizes. fn fake_compact_block( height: BlockHeight, + prev_hash: BlockHash, nf: Nullifier, dfvk: &DiversifiableFullViewingKey, value: Amount, @@ -510,6 +558,7 @@ mod tests { rng.fill_bytes(&mut hash); hash }, + prev_hash: prev_hash.0.to_vec(), height: height.into(), ..Default::default() }; @@ -543,7 +592,7 @@ mod tests { cb.vtx.push(tx); } - cb.block_metadata = initial_sapling_tree_size.map(|s| BlockMetadata { + cb.block_metadata = initial_sapling_tree_size.map(|s| compact::BlockMetadata { sapling_commitment_tree_size: s + cb .vtx .iter() @@ -564,6 +613,7 @@ mod tests { let cb = fake_compact_block( 1u32.into(), + BlockHash([0; 32]), Nullifier([0; 32]), &dfvk, Amount::from_u64(5).unwrap(), @@ -589,16 +639,20 @@ mod tests { None }; - let pruned_block = scan_block_with_runner( + let scanned_block = scan_block_with_runner( &Network::TestNetwork, cb, &[(&account, &dfvk)], &[], - Some(&CommitmentTreeMeta::from_parts(0)), + Some(&BlockMetadata::from_parts( + BlockHeight::from(0), + BlockHash([0u8; 32]), + 0, + )), batch_runner.as_mut(), ) .unwrap(); - let txs = pruned_block.transactions; + let txs = scanned_block.transactions(); assert_eq!(txs.len(), 1); let tx = &txs[0]; @@ -613,17 +667,17 @@ mod tests { Position::from(1) ); - assert_eq!(pruned_block.sapling_commitment_tree_size, Some(2)); + assert_eq!(scanned_block.metadata().sapling_tree_size(), 2); assert_eq!( - pruned_block - .sapling_commitments + scanned_block + .sapling_commitments() .iter() .map(|(_, retention)| *retention) .collect::>(), vec![ Retention::Ephemeral, Retention::Checkpoint { - id: pruned_block.block_height, + id: scanned_block.height(), is_marked: true } ] @@ -643,6 +697,7 @@ mod tests { let cb = fake_compact_block( 1u32.into(), + BlockHash([0; 32]), Nullifier([0; 32]), &dfvk, Amount::from_u64(5).unwrap(), @@ -668,7 +723,7 @@ mod tests { None }; - let pruned_block = scan_block_with_runner( + let scanned_block = scan_block_with_runner( &Network::TestNetwork, cb, &[(&AccountId::from(0), &dfvk)], @@ -677,7 +732,7 @@ mod tests { batch_runner.as_mut(), ) .unwrap(); - let txs = pruned_block.transactions; + let txs = scanned_block.transactions(); assert_eq!(txs.len(), 1); let tx = &txs[0]; @@ -689,8 +744,8 @@ mod tests { assert_eq!(tx.sapling_outputs[0].note().value().inner(), 5); assert_eq!( - pruned_block - .sapling_commitments + scanned_block + .sapling_commitments() .iter() .map(|(_, retention)| *retention) .collect::>(), @@ -698,7 +753,7 @@ mod tests { Retention::Ephemeral, Retention::Marked, Retention::Checkpoint { - id: pruned_block.block_height, + id: scanned_block.height(), is_marked: false } ] @@ -718,6 +773,7 @@ mod tests { let cb = fake_compact_block( 1u32.into(), + BlockHash([0; 32]), nf, &dfvk, Amount::from_u64(5).unwrap(), @@ -727,15 +783,9 @@ mod tests { assert_eq!(cb.vtx.len(), 2); let vks: Vec<(&AccountId, &SaplingIvk)> = vec![]; - let pruned_block = scan_block( - &Network::TestNetwork, - cb, - &vks[..], - &[(account, nf)], - Some(&CommitmentTreeMeta::from_parts(0)), - ) - .unwrap(); - let txs = pruned_block.transactions; + let scanned_block = + scan_block(&Network::TestNetwork, cb, &vks[..], &[(account, nf)], None).unwrap(); + let txs = scanned_block.transactions(); assert_eq!(txs.len(), 1); let tx = &txs[0]; @@ -747,15 +797,15 @@ mod tests { assert_eq!(tx.sapling_spends[0].account(), account); assert_eq!( - pruned_block - .sapling_commitments + scanned_block + .sapling_commitments() .iter() .map(|(_, retention)| *retention) .collect::>(), vec![ Retention::Ephemeral, Retention::Checkpoint { - id: pruned_block.block_height, + id: scanned_block.height(), is_marked: false } ] diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index b1e332913..68ffd2ff3 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -19,10 +19,16 @@ and this library adheres to Rust's notion of a note could be spent with fewer than `min_confirmations` confirmations if the wallet did not contain enough observed blocks to satisfy the `min_confirmations` value specified; this situation is now treated as an error. +- A `BlockConflict` variant has been added to `zcash_client_sqlite::error::SqliteClientError` ### Removed - The empty `wallet::transact` module has been removed. +### Fixed +- Fixed an off-by-one error in the `BlockSource` implementation for the SQLite-backed + `BlockDb` block database which could result in blocks being skipped at the start of + scan ranges. + ## [0.7.1] - 2023-05-17 ### Fixed diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 4d968bb36..478a3bf45 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -26,16 +26,16 @@ 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. -pub(crate) fn blockdb_with_blocks( +pub(crate) fn blockdb_with_blocks( block_source: &BlockDb, from_height: Option, limit: Option, mut with_row: F, -) -> Result<(), Error> +) -> Result<(), Error> where - F: FnMut(CompactBlock) -> Result<(), Error>, + F: FnMut(CompactBlock) -> Result<(), Error>, { - fn to_chain_error, N>(err: E) -> Error { + fn to_chain_error>(err: E) -> Error { Error::BlockSource(err.into()) } @@ -195,16 +195,16 @@ pub(crate) fn blockmetadb_find_block( /// the backing store. If the `limit` value provided is `None`, all blocks are traversed up to the /// maximum height for which metadata is available. #[cfg(feature = "unstable")] -pub(crate) fn fsblockdb_with_blocks( +pub(crate) fn fsblockdb_with_blocks( cache: &FsBlockDb, from_height: Option, limit: Option, mut with_block: F, -) -> Result<(), Error> +) -> Result<(), Error> where - F: FnMut(CompactBlock) -> Result<(), Error>, + F: FnMut(CompactBlock) -> Result<(), Error>, { - fn to_chain_error, N>(err: E) -> Error { + fn to_chain_error>(err: E) -> Error { Error::BlockSource(err.into()) } diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 6eb0939f2..db122f1a2 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -53,13 +53,15 @@ pub enum SqliteClientError { /// A received memo cannot be interpreted as a UTF-8 string. InvalidMemo(zcash_primitives::memo::Error), - /// A requested rewind would violate invariants of the - /// storage layer. The payload returned with this error is - /// (safe rewind height, requested height). + /// An attempt to update block data would overwrite the current hash for a block with a + /// different hash. This indicates that a required rewind was not performed. + BlockConflict(BlockHeight), + + /// A requested rewind would violate invariants of the storage layer. The payload returned with + /// this error is (safe rewind height, requested height). RequestedRewindInvalid(BlockHeight, BlockHeight), - /// The space of allocatable diversifier indices has been exhausted for - /// the given account. + /// The space of allocatable diversifier indices has been exhausted for the given account. DiversifierIndexOutOfRange, /// An error occurred deriving a spending key from a seed and an account @@ -115,6 +117,7 @@ impl fmt::Display for SqliteClientError { SqliteClientError::DbError(e) => write!(f, "{}", e), SqliteClientError::Io(e) => write!(f, "{}", e), SqliteClientError::InvalidMemo(e) => write!(f, "{}", e), + SqliteClientError::BlockConflict(h) => write!(f, "A block hash conflict occurred at height {}; rewind required.", u32::from(*h)), SqliteClientError::DiversifierIndexOutOfRange => write!(f, "The space of available diversifier indices is exhausted"), SqliteClientError::KeyDerivationError(acct_id) => write!(f, "Key derivation failed for account {:?}", acct_id), SqliteClientError::AccountIdDiscontinuity => write!(f, "Wallet account identifiers must be sequential."), diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 01c613c2d..c94cf9129 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -55,10 +55,9 @@ use zcash_primitives::{ use zcash_client_backend::{ address::{AddressMetadata, UnifiedAddress}, data_api::{ - self, - chain::{BlockSource, CommitmentTreeMeta}, - DecryptedTransaction, NullifierQuery, PoolType, PrunedBlock, Recipient, SentTransaction, - WalletCommitmentTrees, WalletRead, WalletWrite, SAPLING_SHARD_HEIGHT, + self, chain::BlockSource, BlockMetadata, DecryptedTransaction, NullifierQuery, PoolType, + Recipient, ScannedBlock, SentTransaction, WalletCommitmentTrees, WalletRead, WalletWrite, + SAPLING_SHARD_HEIGHT, }, keys::{UnifiedFullViewingKey, UnifiedSpendingKey}, proto::compact_formats::CompactBlock, @@ -85,7 +84,7 @@ pub mod wallet; /// this delta from the chain tip to be pruned. pub(crate) const PRUNING_HEIGHT: u32 = 100; -pub(crate) const SAPLING_TABLES_PREFIX: &'static str = "sapling"; +pub(crate) const SAPLING_TABLES_PREFIX: &str = "sapling"; /// A newtype wrapper for sqlite primary key values for the notes /// table. @@ -157,10 +156,12 @@ impl, P: consensus::Parameters> WalletRead for W wallet::block_height_extrema(self.conn.borrow()).map_err(SqliteClientError::from) } - fn fully_scanned_height( - &self, - ) -> Result, Self::Error> { - wallet::fully_scanned_height(self.conn.borrow()) + fn block_metadata(&self, height: BlockHeight) -> Result, Self::Error> { + wallet::block_metadata(self.conn.borrow(), height) + } + + fn block_fully_scanned(&self) -> Result, Self::Error> { + wallet::block_fully_scanned(self.conn.borrow()) } fn suggest_scan_ranges( @@ -183,6 +184,14 @@ impl, P: consensus::Parameters> WalletRead for W wallet::get_tx_height(self.conn.borrow(), txid).map_err(SqliteClientError::from) } + fn get_current_address( + &self, + account: AccountId, + ) -> Result, Self::Error> { + wallet::get_current_address(self.conn.borrow(), &self.params, account) + .map(|res| res.map(|(addr, _)| addr)) + } + fn get_unified_full_viewing_keys( &self, ) -> Result, Self::Error> { @@ -196,14 +205,6 @@ impl, P: consensus::Parameters> WalletRead for W wallet::get_account_for_ufvk(self.conn.borrow(), &self.params, ufvk) } - fn get_current_address( - &self, - account: AccountId, - ) -> Result, Self::Error> { - wallet::get_current_address(self.conn.borrow(), &self.params, account) - .map(|res| res.map(|(addr, _)| addr)) - } - fn is_valid_account_extfvk( &self, account: AccountId, @@ -220,10 +221,6 @@ impl, P: consensus::Parameters> WalletRead for W wallet::get_balance_at(self.conn.borrow(), account, anchor_height) } - fn get_transaction(&self, id_tx: i64) -> Result { - wallet::get_transaction(self.conn.borrow(), &self.params, id_tx) - } - fn get_memo(&self, id_note: Self::NoteRef) -> Result, Self::Error> { match id_note { NoteId::SentNoteId(id_note) => wallet::get_sent_memo(self.conn.borrow(), id_note), @@ -233,6 +230,10 @@ impl, P: consensus::Parameters> WalletRead for W } } + fn get_transaction(&self, id_tx: i64) -> Result { + wallet::get_transaction(self.conn.borrow(), &self.params, id_tx) + } + fn get_sapling_nullifiers( &self, query: NullifierQuery, @@ -390,25 +391,25 @@ impl WalletWrite for WalletDb ) } - #[tracing::instrument(skip_all, fields(height = u32::from(block.block_height)))] + #[tracing::instrument(skip_all, fields(height = u32::from(block.height())))] #[allow(clippy::type_complexity)] fn put_block( &mut self, - block: PrunedBlock, + block: ScannedBlock, ) -> Result, Self::Error> { self.transactionally(|wdb| { // Insert the block into the database. wallet::put_block( wdb.conn.0, - block.block_height, - block.block_hash, - block.block_time, - block.sapling_commitment_tree_size.map(|s| s.into()), + block.height(), + block.block_hash(), + block.block_time(), + block.metadata().sapling_tree_size(), )?; let mut wallet_note_ids = vec![]; - for tx in &block.transactions { - let tx_row = wallet::put_tx_meta(wdb.conn.0, tx, block.block_height)?; + for tx in block.transactions() { + let tx_row = wallet::put_tx_meta(wdb.conn.0, tx, block.height())?; // Mark notes as spent and remove them from the scanning cache for spend in &tx.sapling_spends { @@ -424,19 +425,19 @@ impl WalletWrite for WalletDb } } - let sapling_commitments_len = block.sapling_commitments.len(); - let mut sapling_commitments = block.sapling_commitments.into_iter(); + let block_height = block.height(); + let sapling_tree_size = block.metadata().sapling_tree_size(); + let sapling_commitments_len = block.sapling_commitments().len(); + let mut sapling_commitments = block.take_sapling_commitments().into_iter(); wdb.with_sapling_tree_mut::<_, _, SqliteClientError>(move |sapling_tree| { - if let Some(sapling_tree_size) = block.sapling_commitment_tree_size { - let start_position = Position::from(u64::from(sapling_tree_size)) - - u64::try_from(sapling_commitments_len).unwrap(); - sapling_tree.batch_insert(start_position, &mut sapling_commitments)?; - } + let start_position = Position::from(u64::from(sapling_tree_size)) + - u64::try_from(sapling_commitments_len).unwrap(); + sapling_tree.batch_insert(start_position, &mut sapling_commitments)?; Ok(()) })?; // Update now-expired transactions that didn't get mined. - wallet::update_expired_notes(wdb.conn.0, block.block_height)?; + wallet::update_expired_notes(wdb.conn.0, block_height)?; Ok(wallet_note_ids) }) @@ -688,17 +689,14 @@ impl BlockDb { impl BlockSource for BlockDb { type Error = SqliteClientError; - fn with_blocks( + fn with_blocks( &self, from_height: Option, limit: Option, with_row: F, - ) -> Result<(), data_api::chain::error::Error> + ) -> Result<(), data_api::chain::error::Error> where - F: FnMut( - CompactBlock, - ) - -> Result<(), data_api::chain::error::Error>, + F: FnMut(CompactBlock) -> Result<(), data_api::chain::error::Error>, { chain::blockdb_with_blocks(self, from_height, limit, with_row) } @@ -869,17 +867,14 @@ impl FsBlockDb { impl BlockSource for FsBlockDb { type Error = FsBlockDbError; - fn with_blocks( + fn with_blocks( &self, from_height: Option, limit: Option, with_row: F, - ) -> Result<(), data_api::chain::error::Error> + ) -> Result<(), data_api::chain::error::Error> where - F: FnMut( - CompactBlock, - ) - -> Result<(), data_api::chain::error::Error>, + F: FnMut(CompactBlock) -> Result<(), data_api::chain::error::Error>, { fsblockdb_with_blocks(self, from_height, limit, with_row) } @@ -967,7 +962,7 @@ mod tests { data_api::{WalletRead, WalletWrite}, keys::{sapling, UnifiedFullViewingKey}, proto::compact_formats::{ - BlockMetadata, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, + self as compact, CompactBlock, CompactSaplingOutput, CompactSaplingSpend, CompactTx, }, }; @@ -1112,7 +1107,7 @@ mod tests { }; cb.prev_hash.extend_from_slice(&prev_hash.0); cb.vtx.push(ctx); - cb.block_metadata = Some(BlockMetadata { + cb.block_metadata = Some(compact::BlockMetadata { sapling_commitment_tree_size: initial_sapling_tree_size + cb.vtx.iter().map(|tx| tx.outputs.len() as u32).sum::(), ..Default::default() @@ -1203,7 +1198,7 @@ mod tests { }; cb.prev_hash.extend_from_slice(&prev_hash.0); cb.vtx.push(ctx); - cb.block_metadata = Some(BlockMetadata { + cb.block_metadata = Some(compact::BlockMetadata { sapling_commitment_tree_size: initial_sapling_tree_size + cb.vtx.iter().map(|tx| tx.outputs.len() as u32).sum::(), ..Default::default() diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 722153dbf..13d7ab8c0 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -65,9 +65,9 @@ //! - `memo` the shielded memo associated with the output, if any. use rusqlite::{self, named_params, OptionalExtension, ToSql}; -use std::collections::HashMap; use std::convert::TryFrom; use std::io::Cursor; +use std::{collections::HashMap, io}; use zcash_primitives::{ block::BlockHash, @@ -83,7 +83,7 @@ use zcash_primitives::{ use zcash_client_backend::{ address::{RecipientAddress, UnifiedAddress}, - data_api::{chain::CommitmentTreeMeta, PoolType, Recipient, SentTransactionOutput}, + data_api::{BlockMetadata, PoolType, Recipient, SentTransactionOutput}, encoding::AddressCodec, keys::UnifiedFullViewingKey, wallet::WalletTx, @@ -541,24 +541,58 @@ pub(crate) fn block_height_extrema( }) } -pub(crate) fn fully_scanned_height( +fn parse_block_metadata( + row: (BlockHeight, Vec, Option, Vec), +) -> Option> { + let (block_height, hash_data, sapling_tree_size_opt, sapling_tree) = row; + let sapling_tree_size = sapling_tree_size_opt.map(Ok).or_else(|| { + if sapling_tree == BLOCK_SAPLING_FRONTIER_ABSENT { + None + } else { + // parse the legacy commitment tree data + read_commitment_tree::< + zcash_primitives::sapling::Node, + _, + { zcash_primitives::sapling::NOTE_COMMITMENT_TREE_DEPTH }, + >(Cursor::new(sapling_tree)) + .map(|tree| Some(tree.size().try_into().unwrap())) + .map_err(SqliteClientError::from) + .transpose() + } + })?; + + let block_hash = BlockHash::try_from_slice(&hash_data).ok_or_else(|| { + SqliteClientError::from(io::Error::new( + io::ErrorKind::InvalidData, + format!("Invalid block hash length: {}", hash_data.len()), + )) + }); + + Some(sapling_tree_size.and_then(|sapling_tree_size| { + block_hash.map(|block_hash| { + BlockMetadata::from_parts(block_height, block_hash, sapling_tree_size) + }) + })) +} + +pub(crate) fn block_metadata( conn: &rusqlite::Connection, -) -> Result, SqliteClientError> { - // FIXME: this will need to be rewritten once out-of-order scan range suggestion - // is implemented. + block_height: BlockHeight, +) -> Result, SqliteClientError> { let res_opt = conn .query_row( - "SELECT height, sapling_commitment_tree_size, sapling_tree + "SELECT height, hash, sapling_commitment_tree_size, sapling_tree FROM blocks - ORDER BY height DESC - LIMIT 1", - [], + WHERE height = :block_height", + named_params![":block_height": u32::from(block_height)], |row| { - let max_height: u32 = row.get(0)?; - let sapling_tree_size: Option = row.get(1)?; - let sapling_tree: Vec = row.get(2)?; + let height: u32 = row.get(0)?; + let block_hash: Vec = row.get(1)?; + let sapling_tree_size: Option = row.get(2)?; + let sapling_tree: Vec = row.get(3)?; Ok(( - BlockHeight::from(max_height), + BlockHeight::from(height), + block_hash, sapling_tree_size, sapling_tree, )) @@ -566,32 +600,37 @@ pub(crate) fn fully_scanned_height( ) .optional()?; - res_opt - .and_then(|(max_height, sapling_tree_size, sapling_tree)| { - sapling_tree_size - .map(|s| Ok(CommitmentTreeMeta::from_parts(s))) - .or_else(|| { - if &sapling_tree == BLOCK_SAPLING_FRONTIER_ABSENT { - None - } else { - // parse the legacy commitment tree data - read_commitment_tree::< - zcash_primitives::sapling::Node, - _, - { zcash_primitives::sapling::NOTE_COMMITMENT_TREE_DEPTH }, - >(Cursor::new(sapling_tree)) - .map(|tree| { - Some(CommitmentTreeMeta::from_parts( - tree.size().try_into().unwrap(), - )) - }) - .map_err(SqliteClientError::from) - .transpose() - } - }) - .map(|meta_res| meta_res.map(|meta| (max_height, meta))) - }) - .transpose() + res_opt.and_then(parse_block_metadata).transpose() +} + +pub(crate) fn block_fully_scanned( + conn: &rusqlite::Connection, +) -> Result, SqliteClientError> { + // FIXME: this will need to be rewritten once out-of-order scan range suggestion + // is implemented. + let res_opt = conn + .query_row( + "SELECT height, hash, sapling_commitment_tree_size, sapling_tree + FROM blocks + ORDER BY height DESC + LIMIT 1", + [], + |row| { + let height: u32 = row.get(0)?; + let block_hash: Vec = row.get(1)?; + let sapling_tree_size: Option = row.get(2)?; + let sapling_tree: Vec = row.get(3)?; + Ok(( + BlockHeight::from(height), + block_hash, + sapling_tree_size, + sapling_tree, + )) + }, + ) + .optional()?; + + res_opt.and_then(parse_block_metadata).transpose() } /// Returns the block height at which the specified transaction was mined, @@ -834,12 +873,34 @@ pub(crate) fn get_transparent_balances( /// Inserts information about a scanned block into the database. pub(crate) fn put_block( - conn: &rusqlite::Connection, + conn: &rusqlite::Transaction<'_>, block_height: BlockHeight, block_hash: BlockHash, block_time: u32, - sapling_commitment_tree_size: Option, + sapling_commitment_tree_size: u32, ) -> Result<(), SqliteClientError> { + let block_hash_data = conn + .query_row( + "SELECT hash FROM blocks WHERE height = ?", + [u32::from(block_height)], + |row| row.get::<_, Vec>(0), + ) + .optional()?; + + // Ensure that in the case of an upsert, we don't overwrite block data + // with information for a block with a different hash. + if let Some(bytes) = block_hash_data { + let expected_hash = BlockHash::try_from_slice(&bytes).ok_or_else(|| { + SqliteClientError::CorruptedData(format!( + "Invalid block hash at height {}", + u32::from(block_height) + )) + })?; + if expected_hash != block_hash { + return Err(SqliteClientError::BlockConflict(block_height)); + } + } + let mut stmt_upsert_block = conn.prepare_cached( "INSERT INTO blocks ( height, diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index c730aea73..05ccb72a4 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -1,24 +1,29 @@ //! Functions for initializing the various databases. use either::Either; +use incrementalmerkletree::Retention; use std::{collections::HashMap, fmt, io}; use rusqlite::{self, types::ToSql}; use schemer::{Migrator, MigratorError}; use schemer_rusqlite::RusqliteAdapter; use secrecy::SecretVec; -use shardtree::ShardTreeError; +use shardtree::{ShardTree, ShardTreeError}; use uuid::Uuid; use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight}, + merkle_tree::read_commitment_tree, + sapling, transaction::components::amount::BalanceError, zip32::AccountId, }; -use zcash_client_backend::keys::UnifiedFullViewingKey; +use zcash_client_backend::{data_api::SAPLING_SHARD_HEIGHT, keys::UnifiedFullViewingKey}; -use crate::{error::SqliteClientError, wallet, WalletDb}; +use crate::{error::SqliteClientError, wallet, WalletDb, SAPLING_TABLES_PREFIX}; + +use super::commitment_tree::SqliteShardStore; mod migrations; @@ -289,9 +294,21 @@ pub fn init_blocks_table( return Err(SqliteClientError::TableNotEmpty); } + let block_end_tree = + read_commitment_tree::( + sapling_tree, + ) + .map_err(|e| { + rusqlite::Error::FromSqlConversionFailure( + sapling_tree.len(), + rusqlite::types::Type::Blob, + Box::new(e), + ) + })?; + wdb.conn.0.execute( "INSERT INTO blocks (height, hash, time, sapling_tree) - VALUES (?, ?, ?, ?)", + VALUES (?, ?, ?, ?)", [ u32::from(height).to_sql()?, hash.0.to_sql()?, @@ -300,6 +317,26 @@ pub fn init_blocks_table( ], )?; + if let Some(nonempty_frontier) = block_end_tree.to_frontier().value() { + let shard_store = + SqliteShardStore::<_, sapling::Node, SAPLING_SHARD_HEIGHT>::from_connection( + wdb.conn.0, + SAPLING_TABLES_PREFIX, + )?; + let mut shard_tree: ShardTree< + _, + { sapling::NOTE_COMMITMENT_TREE_DEPTH }, + SAPLING_SHARD_HEIGHT, + > = ShardTree::new(shard_store, 100); + shard_tree.insert_frontier_nodes( + nonempty_frontier.clone(), + Retention::Checkpoint { + id: height, + is_marked: false, + }, + )?; + } + Ok(()) }) } @@ -1154,7 +1191,7 @@ mod tests { BlockHeight::from(1u32), BlockHash([1; 32]), 1, - &[], + &[0x0, 0x0, 0x0], ) .unwrap(); @@ -1164,7 +1201,7 @@ mod tests { BlockHeight::from(2u32), BlockHash([2; 32]), 2, - &[], + &[0x0, 0x0, 0x0], ) .unwrap_err(); } diff --git a/zcash_client_sqlite/src/wallet/sapling.rs b/zcash_client_sqlite/src/wallet/sapling.rs index 85e101c73..a686ec617 100644 --- a/zcash_client_sqlite/src/wallet/sapling.rs +++ b/zcash_client_sqlite/src/wallet/sapling.rs @@ -508,7 +508,7 @@ pub(crate) mod tests { BlockHeight::from(1u32), BlockHash([1; 32]), 1, - &[], + &[0x0, 0x0, 0x0], ) .unwrap(); @@ -562,7 +562,7 @@ pub(crate) mod tests { // Add funds to the wallet in a single note let value = Amount::from_u64(50000).unwrap(); - let (cb, _) = fake_compact_block( + let (mut cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), &dfvk, @@ -588,14 +588,15 @@ pub(crate) mod tests { ); // Add more funds to the wallet in a second note - let (cb, _) = fake_compact_block( + cb = fake_compact_block( sapling_activation_height() + 1, cb.hash(), &dfvk, AddressType::DefaultExternal, value, 1, - ); + ) + .0; insert_into_cache(&db_cache, &cb); scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap(); @@ -639,14 +640,15 @@ pub(crate) mod tests { // Mine blocks SAPLING_ACTIVATION_HEIGHT + 2 to 9 until just before the second // note is verified for i in 2..10 { - let (cb, _) = fake_compact_block( + cb = fake_compact_block( sapling_activation_height() + i, cb.hash(), &dfvk, AddressType::DefaultExternal, value, i, - ); + ) + .0; insert_into_cache(&db_cache, &cb); } scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap(); @@ -673,14 +675,15 @@ pub(crate) mod tests { ); // Mine block 11 so that the second note becomes verified - let (cb, _) = fake_compact_block( + cb = fake_compact_block( sapling_activation_height() + 10, cb.hash(), &dfvk, AddressType::DefaultExternal, value, 11, - ); + ) + .0; insert_into_cache(&db_cache, &cb); scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap(); @@ -718,7 +721,7 @@ pub(crate) mod tests { // Add funds to the wallet in a single note let value = Amount::from_u64(50000).unwrap(); - let (cb, _) = fake_compact_block( + let (mut cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), &dfvk, @@ -774,14 +777,15 @@ pub(crate) mod tests { // Mine blocks SAPLING_ACTIVATION_HEIGHT + 1 to 41 (that don't send us funds) // until just before the first transaction expires for i in 1..42 { - let (cb, _) = fake_compact_block( + cb = fake_compact_block( sapling_activation_height() + i, cb.hash(), &ExtendedSpendingKey::master(&[i as u8]).to_diversifiable_full_viewing_key(), AddressType::DefaultExternal, value, i, - ); + ) + .0; insert_into_cache(&db_cache, &cb); } scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap(); @@ -807,14 +811,15 @@ pub(crate) mod tests { ); // Mine block SAPLING_ACTIVATION_HEIGHT + 42 so that the first transaction expires - let (cb, _) = fake_compact_block( + cb = fake_compact_block( sapling_activation_height() + 42, cb.hash(), &ExtendedSpendingKey::master(&[42]).to_diversifiable_full_viewing_key(), AddressType::DefaultExternal, value, 42, - ); + ) + .0; insert_into_cache(&db_cache, &cb); scan_cached_blocks(&tests::network(), &db_cache, &mut db_data, None, None).unwrap(); @@ -851,7 +856,7 @@ pub(crate) mod tests { // Add funds to the wallet in a single note let value = Amount::from_u64(50000).unwrap(); - let (cb, _) = fake_compact_block( + let (mut cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), &dfvk, @@ -922,14 +927,15 @@ pub(crate) mod tests { // Mine blocks SAPLING_ACTIVATION_HEIGHT + 1 to 42 (that don't send us funds) // so that the first transaction expires for i in 1..=42 { - let (cb, _) = fake_compact_block( + cb = fake_compact_block( sapling_activation_height() + i, cb.hash(), &ExtendedSpendingKey::master(&[i as u8]).to_diversifiable_full_viewing_key(), AddressType::DefaultExternal, value, i, - ); + ) + .0; insert_into_cache(&db_cache, &cb); } scan_cached_blocks(&network, &db_cache, &mut db_data, None, None).unwrap(); @@ -1073,7 +1079,7 @@ pub(crate) mod tests { let dfvk = usk.sapling().to_diversifiable_full_viewing_key(); // Add funds to the wallet - let (cb, _) = fake_compact_block( + let (mut cb, _) = fake_compact_block( sapling_activation_height(), BlockHash([0; 32]), &dfvk, @@ -1085,14 +1091,15 @@ pub(crate) mod tests { // Add 10 dust notes to the wallet for i in 1..=10 { - let (cb, _) = fake_compact_block( + cb = fake_compact_block( sapling_activation_height() + i, cb.hash(), &dfvk, AddressType::DefaultExternal, Amount::from_u64(1000).unwrap(), i, - ); + ) + .0; insert_into_cache(&db_cache, &cb); } diff --git a/zcash_primitives/src/block.rs b/zcash_primitives/src/block.rs index 6271e0535..b9ef9a5ca 100644 --- a/zcash_primitives/src/block.rs +++ b/zcash_primitives/src/block.rs @@ -39,10 +39,20 @@ impl BlockHash { /// /// This function will panic if the slice is not exactly 32 bytes. pub fn from_slice(bytes: &[u8]) -> Self { - assert_eq!(bytes.len(), 32); - let mut hash = [0; 32]; - hash.copy_from_slice(bytes); - BlockHash(hash) + Self::try_from_slice(bytes).unwrap() + } + + /// Constructs a [`BlockHash`] from the given slice. + /// + /// Returns `None` if `bytes` has any length other than 32 + pub fn try_from_slice(bytes: &[u8]) -> Option { + if bytes.len() == 32 { + let mut hash = [0; 32]; + hash.copy_from_slice(bytes); + Some(BlockHash(hash)) + } else { + None + } } }