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

@@ -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

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<ParamsT, BlockSourceT>(
-    parameters: &ParamsT,
+pub fn validate_chain<BlockSourceT>(
     block_source: &BlockSourceT,
-    validate_from: Option<(BlockHeight, BlockHash)>,
-) -> Result<(), Error<Infallible, BlockSourceT::Error, Infallible>>
+    validate_from: (BlockHeight, BlockHash),
+    limit: Option<u32>,
+) -> Result<(BlockHeight, BlockHash), Error<Infallible, BlockSourceT::Error, Infallible>>
 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<BlockHash> = 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

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

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);