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 + } } }