From d4d1edad5a30441dbdb933e9c0a3843ac56eb15c Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 31 Jul 2020 10:21:20 +1000 Subject: [PATCH] fix: Use types to avoid ChainVerifier inconsistencies (#797) --- zebra-consensus/src/block.rs | 2 + zebra-consensus/src/chain.rs | 43 +++++++++++------- zebra-consensus/src/chain/tests.rs | 70 +++++++++++++++++++++++++----- 3 files changed, 89 insertions(+), 26 deletions(-) diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index 7d43aa8d3..18df21b6c 100644 --- a/zebra-consensus/src/block.rs +++ b/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 where S: Service diff --git a/zebra-consensus/src/chain.rs b/zebra-consensus/src/chain.rs index 9c2a42549..8a15f1be6 100644 --- a/zebra-consensus/src/chain.rs +++ b/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>, + + /// The maximum checkpoint height for `checkpoint_verifier`. + max_height: BlockHeight, +} + +/// A service that verifies the chain, using its associated `CheckpointVerifier` +/// and `BlockVerifier`. struct ChainVerifier where BV: Service, 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 - /// clone and share it with futures. + /// The `ChainVerifier`'s underlying `CheckpointVerifier`, and its + /// associated state. /// /// None if all the checkpoints have been verified. - checkpoint_verifier: Option>>, - /// The maximum checkpoint height for `checkpoint_verifier`. - /// - /// None if all the checkpoints have been verified. - max_checkpoint_height: Option, + checkpoint: Option, /// 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 max_checkpoint_height = self.max_checkpoint_height; + let checkpoint_verifier = self.checkpoint.clone().map(|c| c.verifier); + 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, - max_checkpoint_height, + checkpoint, state_service, last_block_height: initial_height, }, diff --git a/zebra-consensus/src/chain/tests.rs b/zebra-consensus/src/chain/tests.rs index 28e4ef472..f06ca79d8 100644 --- a/zebra-consensus/src/chain/tests.rs +++ b/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) -> 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 - let checkpoint_list: BTreeMap = blockchain + // Parse only some blocks as checkpoints + 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::::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 = checkpoints .iter() - .map(|(_, height, hash)| (*height, *hash)) - .take(1) + .map(|(_block, height, hash)| (*height, *hash)) .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 - for (block, height, _hash) in blockchain { - /// SPANDOC: Make sure the verifier service is ready for block {?height} + /// SPANDOC: Verify blocks, restarting at restart_height {?restart_height} + for (block, height, _hash) in blockchain + .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}