diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index 22bd169ca..355af9b4d 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -53,6 +53,16 @@ pub trait DBOps { fn get_block_hash(&self, block_height: BlockHeight) -> Result, Self::Error>; + fn get_max_height_hash(&self) -> Result, Self::Error> { + self.block_height_extrema().and_then(|extrema_opt| { + extrema_opt.map(|(_, max_height)| { + self.get_block_hash(max_height).map(|hash_opt| + hash_opt.map(move |hash| (max_height, hash)) + ) + }).transpose() + }).map(|oo| oo.flatten()) + } + fn get_tx_height(&self, txid: TxId) -> Result, Self::Error>; fn get_address( diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index 80312667e..d7d9a2756 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -24,6 +24,12 @@ use crate::{ /// This follows from the design (and trust) assumption that the `lightwalletd` server /// provides accurate block information as of the time it was requested. /// +/// Arguments: +/// - `parameters` Network parameters +/// - `cache` Source of compact blocks +/// - `from_tip` Height & hash of last validated block; if no validation has previously +/// been performed, this will begin scanning from `sapling_activation_height - 1` +/// /// Returns: /// - `Ok(())` if the combined chain is valid. /// - `Err(ErrorKind::InvalidChain(upper_bound, cause))` if the combined chain is invalid. @@ -32,30 +38,26 @@ use crate::{ /// - `Err(e)` if there was an error during validation unrelated to chain validity. /// /// This function does not mutate either of the databases. -pub fn validate_combined_chain<'db, E0, N, E, P, C, D>( +pub fn validate_combined_chain<'db, E0, N, E, P, C>( parameters: &P, cache: &C, - data: &'db D, + validate_from: Option<(BlockHeight, BlockHash)> ) -> Result<(), E> where E: From>, P: consensus::Parameters, C: CacheOps, - &'db D: DBOps, { let sapling_activation_height = parameters .activation_height(NetworkUpgrade::Sapling) .ok_or(Error::SaplingNotActive)?; - // Recall where we synced up to previously. - // If we have never synced, use Sapling activation height to select all cached CompactBlocks. - let data_max_height = data.block_height_extrema()?.map(|(_, max)| max); - - // The cache will contain blocks above the maximum height of data in the database; - // validate from that maximum height up to the chain tip, returning the - // hash of the block at data_max_height - let from_height = data_max_height.unwrap_or(sapling_activation_height - 1); - let cached_hash_opt = cache.validate_chain(from_height, |top_block, next_block| { + // The cache will contain blocks above the `validate_from` height. Validate from that maximum + // height up to the chain tip, returning the hash of the block found in the cache at the + // `validate_from` height, which can then be used to verify chain integrity by comparing + // against the `validate_from` hash. + let from_height = validate_from.map(|(height, _)| height).unwrap_or(sapling_activation_height - 1); + let scan_start_hash = cache.validate_chain(from_height, |top_block, next_block| { if next_block.height() != top_block.height() - 1 { Err( ChainInvalid::block_height_mismatch(top_block.height() - 1, next_block.height()) @@ -68,20 +70,14 @@ where } })?; - match (cached_hash_opt, data_max_height) { - (Some(cached_hash), Some(h)) => match data.get_block_hash(h)? { - Some(data_scan_max_hash) => { - if cached_hash == data_scan_max_hash { - Ok(()) - } else { - Err(ChainInvalid::prev_hash_mismatch(h).into()) - } + match (scan_start_hash, validate_from) { + (Some(scan_start_hash), Some((validate_from_height, validate_from_hash))) => { + if scan_start_hash == validate_from_hash { + Ok(()) + } else { + Err(ChainInvalid::prev_hash_mismatch(validate_from_height).into()) } - None => Err(Error::CorruptedData( - "No block hash available for block at maximum chain height.", - ) - .into()), - }, + } _ => { // No cached blocks are present, or the max data height is absent, this is fine. Ok(()) diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index 13a778e84..c080f2ce5 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -10,6 +10,7 @@ //! //! use zcash_client_backend::{ //! data_api::{ +//! DBOps, //! chain::{ //! validate_combined_chain, //! scan_cached_blocks, @@ -36,7 +37,7 @@ //! // //! // Given that we assume the server always gives us correct-at-the-time blocks, any //! // errors are in the blocks we have previously cached or scanned. -//! if let Err(e) = validate_combined_chain(&network, &db_cache, &db_data) { +//! if let Err(e) = validate_combined_chain(&network, &db_cache, (&db_data).get_max_height_hash().unwrap()) { //! match e.0 { //! Error::InvalidChain(upper_bound, _) => { //! // a) Pick a height to rewind to. @@ -192,6 +193,7 @@ mod tests { zip32::{ExtendedFullViewingKey, ExtendedSpendingKey}, }; + use zcash_client_backend::data_api::DBOps; use zcash_client_backend::data_api::{ chain::{scan_cached_blocks, validate_combined_chain}, error::{ChainInvalid, Error}, @@ -227,7 +229,7 @@ mod tests { init_accounts_table(&db_data, &tests::network(), &[extfvk.clone()]).unwrap(); // Empty chain should be valid - validate_combined_chain(&tests::network(), &db_cache, &db_data).unwrap(); + validate_combined_chain(&tests::network(), &db_cache, (&db_data).get_max_height_hash().unwrap()).unwrap(); // Create a fake CompactBlock sending value to the address let (cb, _) = fake_compact_block( @@ -239,13 +241,13 @@ mod tests { insert_into_cache(&db_cache, &cb); // Cache-only chain should be valid - validate_combined_chain(&tests::network(), &db_cache, &db_data).unwrap(); + validate_combined_chain(&tests::network(), &db_cache, (&db_data).get_max_height_hash().unwrap()).unwrap(); // Scan the cache scan_cached_blocks(&tests::network(), &db_cache, &db_data, None).unwrap(); // Data-only chain should be valid - validate_combined_chain(&tests::network(), &db_cache, &db_data).unwrap(); + validate_combined_chain(&tests::network(), &db_cache, (&db_data).get_max_height_hash().unwrap()).unwrap(); // Create a second fake CompactBlock sending more value to the address let (cb2, _) = fake_compact_block( @@ -257,13 +259,13 @@ mod tests { insert_into_cache(&db_cache, &cb2); // Data+cache chain should be valid - validate_combined_chain(&tests::network(), &db_cache, &db_data).unwrap(); + validate_combined_chain(&tests::network(), &db_cache, (&db_data).get_max_height_hash().unwrap()).unwrap(); // Scan the cache again scan_cached_blocks(&tests::network(), &db_cache, &db_data, None).unwrap(); // Data-only chain should be valid - validate_combined_chain(&tests::network(), &db_cache, &db_data).unwrap(); + validate_combined_chain(&tests::network(), &db_cache, (&db_data).get_max_height_hash().unwrap()).unwrap(); } #[test] @@ -301,7 +303,7 @@ mod tests { scan_cached_blocks(&tests::network(), &db_cache, &db_data, None).unwrap(); // Data-only chain should be valid - validate_combined_chain(&tests::network(), &db_cache, &db_data).unwrap(); + validate_combined_chain(&tests::network(), &db_cache, (&db_data).get_max_height_hash().unwrap()).unwrap(); // Create more fake CompactBlocks that don't connect to the scanned ones let (cb3, _) = fake_compact_block( @@ -320,7 +322,7 @@ mod tests { insert_into_cache(&db_cache, &cb4); // Data+cache chain should be invalid at the data/cache boundary - match validate_combined_chain(&tests::network(), &db_cache, &db_data).map_err(|e| e.0) { + match validate_combined_chain(&tests::network(), &db_cache, (&db_data).get_max_height_hash().unwrap()).map_err(|e| e.0) { Err(Error::InvalidChain(upper_bound, _)) => { assert_eq!(upper_bound, sapling_activation_height() + 1) } @@ -363,7 +365,7 @@ mod tests { scan_cached_blocks(&tests::network(), &db_cache, &db_data, None).unwrap(); // Data-only chain should be valid - validate_combined_chain(&tests::network(), &db_cache, &db_data).unwrap(); + validate_combined_chain(&tests::network(), &db_cache, (&db_data).get_max_height_hash().unwrap()).unwrap(); // Create more fake CompactBlocks that contain a reorg let (cb3, _) = fake_compact_block( @@ -382,7 +384,7 @@ mod tests { insert_into_cache(&db_cache, &cb4); // Data+cache chain should be invalid inside the cache - match validate_combined_chain(&tests::network(), &db_cache, &db_data).map_err(|e| e.0) { + match validate_combined_chain(&tests::network(), &db_cache, (&db_data).get_max_height_hash().unwrap()).map_err(|e| e.0) { Err(Error::InvalidChain(upper_bound, _)) => { assert_eq!(upper_bound, sapling_activation_height() + 2) }