From bf73ed3a004e73cadcb96938004686f79bb4de19 Mon Sep 17 00:00:00 2001 From: Francisco Gindre Date: Mon, 21 Nov 2022 10:52:44 -0300 Subject: [PATCH] Adds `limit` parameter to `validate_chain()` This allows callers to validate smaller intervals of the given `BlockSourceT` shortening processing times of the function call at the expense of obtaining a partial result on a given section of interest of the block source. `params: &ParamsT` has been removed from the arguments since they were only needed to fall back to `sapling_activation_height` when `None` as passed as the `validate_from` argument. Passing `None` as validation start point on a pre-populated `block_source` would result in an error `ChainError::block_height_discontinuity(sapling_activation_height - 1, current_height)` With this new API callers must specify a concrete `validate_from` argument and assume that `validate_chain` will not take any default fallbacks to chain `ParamsT`. The addition of a `limit` to the chain validation function changes the meaning of its successful output, being now a `BlockHeight, BlockHash)` tuple indicating the block height and block hash up to which the chain as been validated on its continuity of heights and hashes. Callers providing a `limit` aregument are responsible of subsequent calls to `validate_chain()` to complete validating the remaining blocks stored on the `block_source`. Closes zcash/librustzcash#705 --- zcash_client_backend/CHANGELOG.md | 27 ++++++++++ zcash_client_backend/src/data_api/chain.rs | 63 ++++++++++------------ zcash_client_sqlite/CHANGELOG.md | 24 +++++++++ zcash_client_sqlite/src/chain.rs | 58 ++++++++++---------- 4 files changed, 108 insertions(+), 64 deletions(-) diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 0ce98cd68..8b387fd1f 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -9,20 +9,47 @@ and this library adheres to Rust's notion of ### Changed - MSRV is now 1.60.0. + - `zcash_client_backend::data_api::wallet::shield_transparent_funds` now takes a `shielding_threshold` argument that can be used to specify the minimum value allowed as input to a shielding transaction. Previously the shielding threshold was fixed at 100000 zatoshis. + - Note commitments now use `zcash_primitives::sapling::note::ExtractedNoteCommitment` instead of `bls12_381::Scalar` in the following places: - The `cmu` field of `zcash_client_backend::wallet::WalletShieldedOutput`. - `zcash_client_backend::proto::compact_formats::CompactSaplingOutput::cmu`. +- **breaking changes** to `zcash_client_backend::data_api::chain::validate_chain` + - `validate_chain` now requires a non-optional `validate_from` parameter that + indicates the starting point of the `BlockSourceT` validation. An Optional + `limit` can be specified as well. This allows callers to validate smaller + intervals of the given `BlockSourceT` shortening processing times of the + function call at the expense of obtaining a partial result on a given + section of interest of the block source. + + - `params: &ParamsT` has been removed from the arguments since they were only + needed to fall back to `sapling_activation_height` when `None` as passed as + the `validate_from` argument. Passing `None` as validation start point on a + pre-populated `block_source` would result in an error + `ChainError::block_height_discontinuity(sapling_activation_height - 1, current_height)` + + - With this new API callers must specify a concrete `validate_from` argument + and assume that `validate_chain` will not take any default fallbacks to + chain `ParamsT`. + + - The addition of a `limit` to the chain validation function changes the + meaning of its successful output, being now a `BlockHeight, BlockHash)` + tuple indicating the block height and block has up to which the chain as + been validated on its continuity of heights and hashes. Callers providing a + `limit` aregumente are responsible of subsequent calls to + `validate_chain()` to complete validating the totality of the block_source. ### Removed - `zcash_client_backend::data_api`: - `WalletWrite::remove_unmined_tx` (was behind the `unstable` feature flag). + ## [0.6.1] - 2022-12-06 ### Added - `zcash_client_backend::data_api::chain::scan_cached_blocks` now generates diff --git a/zcash_client_backend/src/data_api/chain.rs b/zcash_client_backend/src/data_api/chain.rs index 70df26bf6..07837a21d 100644 --- a/zcash_client_backend/src/data_api/chain.rs +++ b/zcash_client_backend/src/data_api/chain.rs @@ -44,7 +44,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. //! let max_height_hash = db_data.get_max_height_hash().map_err(Error::Wallet)?; -//! if let Err(e) = validate_chain(&network, &block_source, max_height_hash) { +//! if let Err(e) = validate_chain(&block_source, max_height_hash, None) { //! match e { //! Error::Chain(e) => { //! // a) Pick a height to rewind to. @@ -134,59 +134,52 @@ pub trait BlockSource { /// block source is more likely to be accurate than the previously-scanned information. /// This follows from the design (and trust) assumption that the `lightwalletd` server /// provides accurate block information as of the time it was requested. +/// The addition of a `limit` to the chain validation function changes the meaning of +/// its successful output, being now a `BlockHeight, BlockHash)` tuple indicating the +/// block height and block hash up to which the chain as been validated on its continuity +/// of heights and hashes. Callers providing a `limit` argument are responsible of +/// making subsequent calls to `validate_chain()` to complete validating the remaining +/// blocks stored on the `block_source`. /// /// Arguments: -/// - `parameters` Network parameters /// - `block_source` 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` +/// - `validate_from` Height & hash of last validated block; +/// - `limit` Maximum number of blocks that should be validated in this call. /// /// Returns: -/// - `Ok(())` if the combined chain is valid. +/// - `Ok((BlockHeight, BlockHash))` 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. /// /// This function does not mutate either of the databases. -pub fn validate_chain( - parameters: &ParamsT, +pub fn validate_chain( block_source: &BlockSourceT, - validate_from: Option<(BlockHeight, BlockHash)>, -) -> Result<(), Error> + validate_from: (BlockHeight, BlockHash), + limit: Option, +) -> Result<(BlockHeight, BlockHash), Error> where - ParamsT: consensus::Parameters, BlockSourceT: BlockSource, { - let sapling_activation_height = parameters - .activation_height(NetworkUpgrade::Sapling) - .expect("Sapling activation height must be known."); - // The block source 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 block // source 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 mut prev_height = from_height; - let mut prev_hash: Option = validate_from.map(|(_, hash)| hash); - - block_source.with_blocks::<_, Infallible, Infallible>(from_height, None, move |block| { - let current_height = block.height(); - let result = if current_height != prev_height + 1 { - Err(ChainError::block_height_discontinuity(prev_height + 1, current_height).into()) - } else { - match prev_hash { - None => Ok(()), - Some(h) if h == block.prev_hash() => Ok(()), - Some(_) => Err(ChainError::prev_hash_mismatch(current_height).into()), + 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(()) } - }; - - prev_height = current_height; - prev_hash = Some(block.hash()); - result - }) + }) + .map(|_| (valid_height, valid_hash)) } /// Scans at most `limit` new blocks added to the block source for any transactions received by the diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index 48ae206cf..6fb16f18a 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -18,8 +18,32 @@ and this library adheres to Rust's notion of ### Changed - MSRV is now 1.60.0. +- **breaking changes** to `validate_chain`. + - `zcash_client_backend::data_api::chain::validate_chain` now requires a + non-optional `validate_from` parameter that indicates the starting point of + the `BlockSourceT` validation. An Optional `limit` can be specified as + well. This allows callers to validate smaller intervals of the given + `BlockSourceT` shortening processing times of the function call at the + expense of obtaining a partial result on a given section of interest of the + block source. + - `params: &ParamsT` has been removed from the arguments since they were only needed + to fall back to `sapling_activation_height` when `None` as passed as the + `validate_from` argument. Passing `None` as validation start point on a + pre-populated `block_source` would result in an error + `ChainError::block_height_discontinuity(sapling_activation_height - 1, current_height)` + - With this new API callers must specify a concrete `validate_from` argument and + assume that `validate_chain` will not take any default fallbacks to chain + `ParamsT`. + - The addition of a `limit` to the chain validation function changes the + meaning of its successful output, being now a `BlockHeight, BlockHash)` tuple + indicating the block height and block has up to which the chain as been + validated on its continuity of heights and hashes. Callers providing a + `limit` aregumente are responsible of subsequent calls to `validate_chain()` + to complete validating the totality of the block_source. + ### Removed - implementation of unstable function `WalletWrite::remove_unmined_tx`, + ## [0.4.2] - 2022-12-13 ### Fixed - `zcash_client_sqlite::WalletDb::get_transparent_balances` no longer returns an diff --git a/zcash_client_sqlite/src/chain.rs b/zcash_client_sqlite/src/chain.rs index e8bc2d504..69443d4a5 100644 --- a/zcash_client_sqlite/src/chain.rs +++ b/zcash_client_sqlite/src/chain.rs @@ -301,31 +301,31 @@ mod tests { // Add an account to the wallet let (dfvk, _taddr) = init_test_accounts_table(&db_data); - // Empty chain should be valid - validate_chain( - &tests::network(), - &db_cache, - db_data.get_max_height_hash().unwrap(), - ) - .unwrap(); + // Empty chain should return None + assert_matches!(db_data.get_max_height_hash(), Ok(None)); // Create a fake CompactBlock sending value to the address + let fake_block_hash = BlockHash([0; 32]); + let fake_block_height = sapling_activation_height(); + let (cb, _) = fake_compact_block( - sapling_activation_height(), - BlockHash([0; 32]), + fake_block_height, + fake_block_hash, &dfvk, AddressType::DefaultExternal, Amount::from_u64(5).unwrap(), ); + insert_into_cache(&db_cache, &cb); // Cache-only chain should be valid - validate_chain( - &tests::network(), - &db_cache, - db_data.get_max_height_hash().unwrap(), - ) - .unwrap(); + 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)) + ); // Scan the cache let mut db_write = db_data.get_update_ops().unwrap(); @@ -333,9 +333,9 @@ mod tests { // Data-only chain should be valid validate_chain( - &tests::network(), &db_cache, - db_data.get_max_height_hash().unwrap(), + db_data.get_max_height_hash().unwrap().unwrap(), + None, ) .unwrap(); @@ -351,9 +351,9 @@ mod tests { // Data+cache chain should be valid validate_chain( - &tests::network(), &db_cache, - db_data.get_max_height_hash().unwrap(), + db_data.get_max_height_hash().unwrap().unwrap(), + None, ) .unwrap(); @@ -362,9 +362,9 @@ mod tests { // Data-only chain should be valid validate_chain( - &tests::network(), &db_cache, - db_data.get_max_height_hash().unwrap(), + db_data.get_max_height_hash().unwrap().unwrap(), + None, ) .unwrap(); } @@ -406,9 +406,9 @@ mod tests { // Data-only chain should be valid validate_chain( - &tests::network(), &db_cache, - db_data.get_max_height_hash().unwrap(), + db_data.get_max_height_hash().unwrap().unwrap(), + None, ) .unwrap(); @@ -432,9 +432,9 @@ mod tests { // Data+cache chain should be invalid at the data/cache boundary let val_result = validate_chain( - &tests::network(), &db_cache, - db_data.get_max_height_hash().unwrap(), + db_data.get_max_height_hash().unwrap().unwrap(), + None, ); assert_matches!(val_result, Err(Error::Chain(e)) if e.at_height() == sapling_activation_height() + 2); @@ -477,9 +477,9 @@ mod tests { // Data-only chain should be valid validate_chain( - &tests::network(), &db_cache, - db_data.get_max_height_hash().unwrap(), + db_data.get_max_height_hash().unwrap().unwrap(), + None, ) .unwrap(); @@ -503,9 +503,9 @@ mod tests { // Data+cache chain should be invalid inside the cache let val_result = validate_chain( - &tests::network(), &db_cache, - db_data.get_max_height_hash().unwrap(), + db_data.get_max_height_hash().unwrap().unwrap(), + None, ); assert_matches!(val_result, Err(Error::Chain(e)) if e.at_height() == sapling_activation_height() + 3);