fix: Use types to avoid ChainVerifier inconsistencies (#797)

zebra-consensus/src/block.rs

@@ -28,6 +28,8 @@ use tower::{buffer::Buffer, Service, ServiceExt};
 use zebra_chain::block::{Block, BlockHeaderHash};
 use zebra_chain::types::BlockHeight;
 
+/// A service that verifies blocks.
+#[derive(Debug)]
 struct BlockVerifier<S>
 where
     S: Service<zebra_state::Request, Response = zebra_state::Response, Error = Error>

zebra-consensus/src/chain.rs

@@ -37,6 +37,20 @@ use zebra_chain::Network;
 /// Used to identify unexpected high blocks.
 const MAX_EXPECTED_BLOCK_GAP: u32 = 100_000;
 
+/// A wrapper type that holds the `ChainVerifier`'s `CheckpointVerifier`, and
+/// its associated state.
+#[derive(Clone)]
+struct ChainCheckpointVerifier {
+    /// The underlying `CheckpointVerifier`, wrapped in a buffer, so we can
+    /// clone and share it with futures.
+    verifier: Buffer<CheckpointVerifier, Arc<Block>>,
+
+    /// The maximum checkpoint height for `checkpoint_verifier`.
+    max_height: BlockHeight,
+}
+
+/// A service that verifies the chain, using its associated `CheckpointVerifier`
+/// and `BlockVerifier`.
 struct ChainVerifier<BV, S>
 where
     BV: Service<Arc<Block>, Response = BlockHeaderHash, Error = Error> + Send + Clone + 'static,
@@ -50,15 +64,11 @@ where
     /// The underlying `BlockVerifier`, possibly wrapped in other services.
     block_verifier: BV,
 
-    /// The underlying `CheckpointVerifier`, wrapped in a buffer, so we can
+    /// The `ChainVerifier`'s underlying `CheckpointVerifier`, and its
-    /// clone and share it with futures.
+    /// associated state.
     ///
     /// None if all the checkpoints have been verified.
-    checkpoint_verifier: Option<Buffer<CheckpointVerifier, Arc<Block>>>,
+    checkpoint: Option<ChainCheckpointVerifier>,
-    /// The maximum checkpoint height for `checkpoint_verifier`.
-    ///
-    /// None if all the checkpoints have been verified.
-    max_checkpoint_height: Option<BlockHeight>,
 
     /// The underlying `ZebraState`, possibly wrapped in other services.
     state_service: S,
@@ -112,8 +122,8 @@ where
 
         let mut block_verifier = self.block_verifier.clone();
         let mut state_service = self.state_service.clone();
-        let checkpoint_verifier = self.checkpoint_verifier.clone();
+        let checkpoint_verifier = self.checkpoint.clone().map(|c| c.verifier);
-        let max_checkpoint_height = self.max_checkpoint_height;
+        let max_checkpoint_height = self.checkpoint.clone().map(|c| c.max_height);
 
         // Log an info-level message on unexpected high blocks
         let is_unexpected_high_block = match (block_height, self.last_block_height) {
@@ -295,26 +305,29 @@ where
         .activation_height(network)
         .expect("Unexpected network upgrade info: Sapling must have an activation height");
 
-    let (checkpoint_verifier, max_checkpoint_height) = match (initial_height, checkpoint_list, max_checkpoint_height) {
+    let checkpoint = match (initial_height, checkpoint_list, max_checkpoint_height) {
         // If we need to verify pre-Sapling blocks, make sure we have checkpoints for them.
         (None, None, _) => panic!("We have no checkpoints, and we have no cached blocks: Pre-Sapling blocks must be verified by checkpoints"),
         (Some(initial_height), None, _) if (initial_height < sapling_activation) => panic!("We have no checkpoints, and we don't have a cached Sapling activation block: Pre-Sapling blocks must be verified by checkpoints"),
 
         // If we're past the checkpoint range, don't create a checkpoint verifier.
-        (Some(initial_height), _, Some(max_checkpoint_height)) if (initial_height > max_checkpoint_height) => (None, None),
+        (Some(initial_height), _, Some(max_checkpoint_height)) if (initial_height > max_checkpoint_height) => None,
         // No list, no checkpoint verifier
-        (_, None, _) => (None, None),
+        (_, None, _) => None,
 
         (_, Some(_), None) => panic!("Missing max checkpoint height: height must be Some if verifier is Some"),
         // We've done all the checks we need to create a checkpoint verifier
-        (_, Some(list), Some(_)) => (Some(Buffer::new(CheckpointVerifier::from_checkpoint_list(list, initial_tip), 1)), max_checkpoint_height),
+        (_, Some(list), Some(max_height)) => Some(
+            ChainCheckpointVerifier {
+                verifier: Buffer::new(CheckpointVerifier::from_checkpoint_list(list, initial_tip), 1),
+                max_height,
+            }),
     };
 
     Buffer::new(
         ChainVerifier {
             block_verifier,
-            checkpoint_verifier,
+            checkpoint,
-            max_checkpoint_height,
             state_service,
             last_block_height: initial_height,
         },

zebra-consensus/src/chain/tests.rs

@@ -398,12 +398,17 @@ async fn verify_fail_add_block_checkpoint() -> Result<(), Report> {
 
 #[tokio::test]
 async fn continuous_blockchain_test() -> Result<(), Report> {
-    continuous_blockchain().await
+    continuous_blockchain(None).await?;
+    for height in 0..=10 {
+        continuous_blockchain(Some(BlockHeight(height))).await?;
+    }
+    Ok(())
 }
 
-/// Test a continuous blockchain in the BlockVerifier
+/// Test a continuous blockchain in the BlockVerifier and CheckpointVerifier,
+/// restarting verification at `restart_height`.
 #[spandoc::spandoc]
-async fn continuous_blockchain() -> Result<(), Report> {
+async fn continuous_blockchain(restart_height: Option<BlockHeight>) -> Result<(), Report> {
     zebra_test::init();
 
     // A continuous blockchain
@@ -426,21 +431,64 @@ async fn continuous_blockchain() -> Result<(), Report> {
         blockchain.push((block.clone(), block.coinbase_height().unwrap(), hash));
     }
 
-    // Make a checkpoint list containing the genesis block
+    // Parse only some blocks as checkpoints
-    let checkpoint_list: BTreeMap<BlockHeight, BlockHeaderHash> = blockchain
+    let mut checkpoints = Vec::new();
+    for b in &[
+        &zebra_test::vectors::BLOCK_MAINNET_GENESIS_BYTES[..],
+        &zebra_test::vectors::BLOCK_MAINNET_4_BYTES[..],
+    ] {
+        let block = Arc::<Block>::zcash_deserialize(*b)?;
+        let hash: BlockHeaderHash = block.as_ref().into();
+        checkpoints.push((block.clone(), block.coinbase_height().unwrap(), hash));
+    }
+
+    // The checkpoint list will contain only blocks 0 and 4
+    let checkpoint_list: BTreeMap<BlockHeight, BlockHeaderHash> = checkpoints
         .iter()
-        .map(|(_, height, hash)| (*height, *hash))
+        .map(|(_block, height, hash)| (*height, *hash))
-        .take(1)
         .collect();
     let checkpoint_list = CheckpointList::from_list(checkpoint_list).map_err(|e| eyre!(e))?;
 
-    let (mut chain_verifier, _) = verifiers_from_checkpoint_list(Mainnet, checkpoint_list);
+    let mut state_service = zebra_state::in_memory::init();
+    /// SPANDOC: Add blocks to the state from 0..=restart_height {?restart_height}
+    if restart_height.is_some() {
+        for block in blockchain
+            .iter()
+            .take((restart_height.unwrap().0 + 1) as usize)
+            .map(|(block, ..)| block)
+        {
+            state_service
+                .ready_and()
+                .map_err(|e| eyre!(e))
+                .await?
+                .call(zebra_state::Request::AddBlock {
+                    block: block.clone(),
+                })
+                .map_err(|e| eyre!(e))
+                .await?;
+        }
+    }
+    let initial_tip = restart_height
+        .map(|BlockHeight(height)| &blockchain[height as usize].0)
+        .cloned();
+
+    let block_verifier = crate::block::init(state_service.clone());
+    let mut chain_verifier = super::init_from_verifiers(
+        Mainnet,
+        block_verifier,
+        Some(checkpoint_list),
+        state_service.clone(),
+        initial_tip,
+    );
 
     let mut handles = FuturesUnordered::new();
 
-    // Now verify each block
+    /// SPANDOC: Verify blocks, restarting at restart_height {?restart_height}
-    for (block, height, _hash) in blockchain {
+    for (block, height, _hash) in blockchain
-        /// SPANDOC: Make sure the verifier service is ready for block {?height}
+        .iter()
+        .filter(|(_, height, _)| restart_height.map_or(true, |rh| *height > rh))
+    {
+        /// SPANDOC: Make sure the verifier service is ready for the block at height {?height}
         let ready_verifier_service = chain_verifier.ready_and().map_err(|e| eyre!(e)).await?;
 
         /// SPANDOC: Set up the future for block {?height}