diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index 07837a21d..5ac1d82c8 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -88,7 +88,7 @@ use std::convert::Infallible; use zcash_primitives::{ block::BlockHash, - consensus::{self, BlockHeight, NetworkUpgrade}, + consensus::{self, BlockHeight}, merkle_tree::CommitmentTree, sapling::{note_encryption::PreparedIncomingViewingKey, Nullifier}, zip32::Scope, @@ -111,7 +111,8 @@ pub trait BlockSource { type Error; /// Scan the specified `limit` number of blocks from the blockchain, starting at - /// `from_height`, applying the provided callback to each block. + /// `from_height`, applying the provided callback to each block. If `from_height` + /// is `None` then scanning will begin at the first available block. /// /// * `WalletErrT`: the types of errors produced by the wallet operations performed /// as part of processing each row. @@ -119,7 +120,7 @@ pub trait BlockSource { /// reporting errors related to specific notes. fn with_blocks( &self, - from_height: BlockHeight, + from_height: Option, limit: Option, with_row: F, ) -> Result<(), error::Error> @@ -147,7 +148,7 @@ pub trait BlockSource { /// - `limit` Maximum number of blocks that should be validated in this call. /// /// Returns: -/// - `Ok((BlockHeight, BlockHash))` if the combined chain is valid up to the given height +/// - `Ok(())` if the combined chain is valid up to the given height /// and block hash. /// - `Err(Error::Chain(cause))` if the combined chain is invalid. /// - `Err(e)` if there was an error during validation unrelated to chain validity. @@ -155,9 +156,9 @@ pub trait BlockSource { /// This function does not mutate either of the databases. pub fn validate_chain( block_source: &BlockSourceT, - validate_from: (BlockHeight, BlockHash), + mut validate_from: Option<(BlockHeight, BlockHash)>, limit: Option, -) -> Result<(BlockHeight, BlockHash), Error> +) -> Result<(), Error> where BlockSourceT: BlockSource, { @@ -166,20 +167,26 @@ where // source at the `validate_from` height, which can then be used to verify chain integrity by // comparing against the `validate_from` hash. - let (mut valid_height, mut valid_hash) = validate_from; - block_source - .with_blocks::<_, Infallible, Infallible>(valid_height, limit, move |block| { - if block.height() != valid_height + 1 { - Err(ChainError::block_height_discontinuity(valid_height + 1, block.height()).into()) - } else if block.prev_hash() != valid_hash { - Err(ChainError::prev_hash_mismatch(block.height()).into()) - } else { - valid_height = block.height(); - valid_hash = block.hash(); - Ok(()) + block_source.with_blocks::<_, Infallible, Infallible>( + validate_from.map(|(h, _)| h), + limit, + move |block| { + if let Some((valid_height, valid_hash)) = validate_from { + if block.height() != valid_height + 1 { + return Err(ChainError::block_height_discontinuity( + valid_height + 1, + block.height(), + ) + .into()); + } else if block.prev_hash() != valid_hash { + return Err(ChainError::prev_hash_mismatch(block.height()).into()); + } } - }) - .map(|_| (valid_height, valid_hash)) + + validate_from = Some((block.height(), block.hash())); + Ok(()) + }, + ) } /// Scans at most `limit` new blocks added to the block source for any transactions received by the @@ -215,19 +222,11 @@ where BlockSourceT: BlockSource, DbT: WalletWrite, { - let sapling_activation_height = params - .activation_height(NetworkUpgrade::Sapling) - .expect("Sapling activation height is known."); - // Recall where we synced up to previously. - // If we have never synced, use sapling activation height to select all cached CompactBlocks. let mut last_height = data_db .block_height_extrema() - .map(|opt| { - opt.map(|(_, max)| max) - .unwrap_or(sapling_activation_height - 1) - }) - .map_err(Error::Wallet)?; + .map_err(Error::Wallet)? + .map(|(_, max)| max); // Fetch the UnifiedFullViewingKeys we are tracking let ufvks = data_db @@ -241,13 +240,21 @@ where .collect(); // Get the most recent CommitmentTree - let mut tree = data_db - .get_commitment_tree(last_height) - .map(|t| t.unwrap_or_else(CommitmentTree::empty)) - .map_err(Error::Wallet)?; + let mut tree = last_height.map_or_else( + || Ok(CommitmentTree::empty()), + |h| { + data_db + .get_commitment_tree(h) + .map(|t| t.unwrap_or_else(CommitmentTree::empty)) + .map_err(Error::Wallet) + }, + )?; // Get most recent incremental witnesses for the notes we are tracking - let mut witnesses = data_db.get_witnesses(last_height).map_err(Error::Wallet)?; + let mut witnesses = last_height.map_or_else( + || Ok(vec![]), + |h| data_db.get_witnesses(h).map_err(Error::Wallet), + )?; // Get the nullifiers for the notes we are tracking let mut nullifiers = data_db.get_nullifiers().map_err(Error::Wallet)?; @@ -283,12 +290,12 @@ where let current_height = block.height(); // Scanned blocks MUST be height-sequential. - if current_height != (last_height + 1) { - return Err(ChainError::block_height_discontinuity( - last_height + 1, - current_height, - ) - .into()); + if let Some(h) = last_height { + if current_height != (h + 1) { + return Err( + ChainError::block_height_discontinuity(h + 1, current_height).into(), + ); + } } let block_hash = BlockHash::from_slice(&block.hash); @@ -359,7 +366,7 @@ where witnesses.extend(new_witnesses); - last_height = current_height; + last_height = Some(current_height); Ok(()) }, @@ -384,7 +391,7 @@ pub mod testing { fn with_blocks( &self, - _from_height: BlockHeight, + _from_height: Option, _limit: Option, _with_row: F, ) -> Result<(), Error> diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 69443d4a5..44d24b769 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -28,7 +28,7 @@ pub mod migrations; /// blocks are traversed up to the maximum height. pub(crate) fn blockdb_with_blocks( block_source: &BlockDb, - last_scanned_height: BlockHeight, + last_scanned_height: Option, limit: Option, mut with_row: F, ) -> Result<(), Error> @@ -51,7 +51,7 @@ where let mut rows = stmt_blocks .query(params![ - u32::from(last_scanned_height), + last_scanned_height.map_or(0u32, u32::from), limit.unwrap_or(u32::max_value()), ]) .map_err(to_chain_error)?; @@ -197,7 +197,7 @@ pub(crate) fn blockmetadb_find_block( #[cfg(feature = "unstable")] pub(crate) fn fsblockdb_with_blocks( cache: &FsBlockDb, - last_scanned_height: BlockHeight, + last_scanned_height: Option, limit: Option, mut with_block: F, ) -> Result<(), Error> @@ -213,16 +213,16 @@ where .conn .prepare( "SELECT height, blockhash, time, sapling_outputs_count, orchard_actions_count - FROM compactblocks_meta - WHERE height > ? - ORDER BY height ASC LIMIT ?", + FROM compactblocks_meta + WHERE height > ? + ORDER BY height ASC LIMIT ?", ) .map_err(to_chain_error)?; let rows = stmt_blocks .query_map( params![ - u32::from(last_scanned_height), + last_scanned_height.map_or(0u32, u32::from), limit.unwrap_or(u32::max_value()), ], |row| { @@ -319,25 +319,20 @@ mod tests { insert_into_cache(&db_cache, &cb); // Cache-only chain should be valid - let validate_chain_result = - validate_chain(&db_cache, (fake_block_height, fake_block_hash), Some(1)); - - assert_matches!( - validate_chain_result, - Ok((_fake_block_height, _fake_block_hash)) + let validate_chain_result = validate_chain( + &db_cache, + Some((fake_block_height, fake_block_hash)), + Some(1), ); + assert_matches!(validate_chain_result, Ok(())); + // Scan the cache let mut db_write = db_data.get_update_ops().unwrap(); scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); // Data-only chain should be valid - validate_chain( - &db_cache, - db_data.get_max_height_hash().unwrap().unwrap(), - None, - ) - .unwrap(); + validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None).unwrap(); // Create a second fake CompactBlock sending more value to the address let (cb2, _) = fake_compact_block( @@ -350,23 +345,13 @@ mod tests { insert_into_cache(&db_cache, &cb2); // Data+cache chain should be valid - validate_chain( - &db_cache, - db_data.get_max_height_hash().unwrap().unwrap(), - None, - ) - .unwrap(); + validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None).unwrap(); // Scan the cache again scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); // Data-only chain should be valid - validate_chain( - &db_cache, - db_data.get_max_height_hash().unwrap().unwrap(), - None, - ) - .unwrap(); + validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None).unwrap(); } #[test] @@ -405,12 +390,7 @@ mod tests { scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); // Data-only chain should be valid - validate_chain( - &db_cache, - db_data.get_max_height_hash().unwrap().unwrap(), - None, - ) - .unwrap(); + validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None).unwrap(); // Create more fake CompactBlocks that don't connect to the scanned ones let (cb3, _) = fake_compact_block( @@ -431,11 +411,7 @@ mod tests { insert_into_cache(&db_cache, &cb4); // Data+cache chain should be invalid at the data/cache boundary - let val_result = validate_chain( - &db_cache, - db_data.get_max_height_hash().unwrap().unwrap(), - None, - ); + let val_result = validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None); assert_matches!(val_result, Err(Error::Chain(e)) if e.at_height() == sapling_activation_height() + 2); } @@ -476,12 +452,7 @@ mod tests { scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap(); // Data-only chain should be valid - validate_chain( - &db_cache, - db_data.get_max_height_hash().unwrap().unwrap(), - None, - ) - .unwrap(); + validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None).unwrap(); // Create more fake CompactBlocks that contain a reorg let (cb3, _) = fake_compact_block( @@ -502,11 +473,7 @@ mod tests { insert_into_cache(&db_cache, &cb4); // Data+cache chain should be invalid inside the cache - let val_result = validate_chain( - &db_cache, - db_data.get_max_height_hash().unwrap().unwrap(), - None, - ); + let val_result = validate_chain(&db_cache, db_data.get_max_height_hash().unwrap(), None); assert_matches!(val_result, Err(Error::Chain(e)) if e.at_height() == sapling_activation_height() + 3); } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index a0c2cd00a..98933beb5 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -723,7 +723,7 @@ impl BlockSource for BlockDb { fn with_blocks( &self, - from_height: BlockHeight, + from_height: Option, limit: Option, with_row: F, ) -> Result<(), data_api::chain::error::Error> @@ -904,7 +904,7 @@ impl BlockSource for FsBlockDb { fn with_blocks( &self, - from_height: BlockHeight, + from_height: Option, limit: Option, with_row: F, ) -> Result<(), data_api::chain::error::Error>