From 870a0be928de1c8fa18960d425bab77afc79f6a0 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 10 Jul 2020 17:46:27 +1000 Subject: [PATCH] fix: Make sure checkpoint heights and hashes are unique Previously, CheckpointList would silently ignore duplicate heights or hashes. Closes #632. --- zebra-consensus/src/checkpoint/list.rs | 197 ++++++++++++++++++++++++- 1 file changed, 192 insertions(+), 5 deletions(-) diff --git a/zebra-consensus/src/checkpoint/list.rs b/zebra-consensus/src/checkpoint/list.rs index 3d640747..2a8c4897 100644 --- a/zebra-consensus/src/checkpoint/list.rs +++ b/zebra-consensus/src/checkpoint/list.rs @@ -5,7 +5,11 @@ //! Checkpoints can be used to verify their ancestors, by chaining backwards //! to another checkpoint, via each block's parent block hash. -use std::{collections::BTreeMap, error, ops::RangeBounds}; +use std::{ + collections::{BTreeMap, HashSet}, + error, + ops::RangeBounds, +}; use zebra_chain::block::BlockHeaderHash; use zebra_chain::types::BlockHeight; @@ -20,19 +24,27 @@ type Error = Box; /// which only happen in the last few hundred blocks in the chain. /// (zcashd allows chain reorganizations up to 99 blocks, and prunes /// orphaned side-chains after 288 blocks.) -/// -/// There must be a checkpoint for the genesis block at BlockHeight 0. -/// (All other checkpoints are optional.) #[derive(Debug)] pub struct CheckpointList(BTreeMap); impl CheckpointList { /// Create a new checkpoint list from `checkpoint_list`. + /// + /// Checkpoint heights and checkpoint hashes must be unique. + /// + /// There must be a checkpoint for the genesis block at BlockHeight 0. + /// (All other checkpoints are optional.) pub fn new( checkpoint_list: impl IntoIterator, ) -> Result { - let checkpoints: BTreeMap = + // BTreeMap silently ignores duplicates, so we count the checkpoints + // before adding them to the map + let original_checkpoints: Vec<(BlockHeight, BlockHeaderHash)> = checkpoint_list.into_iter().collect(); + let original_len = original_checkpoints.len(); + + let checkpoints: BTreeMap = + original_checkpoints.into_iter().collect(); // An empty checkpoint list can't actually verify any blocks. match checkpoints.keys().next() { @@ -41,6 +53,17 @@ impl CheckpointList { _ => Err("checkpoints must start at the genesis block height 0")?, }; + // This check rejects duplicate heights, whether they have the same or + // different hashes + if checkpoints.len() != original_len { + Err("checkpoint heights must be unique")?; + } + + let block_hashes: HashSet<&BlockHeaderHash> = checkpoints.values().collect(); + if block_hashes.len() != original_len { + Err("checkpoint hashes must be unique")?; + } + Ok(CheckpointList(checkpoints)) } @@ -81,3 +104,167 @@ impl CheckpointList { self.0.range(range).map(|(height, _)| *height).next_back() } } + +#[cfg(test)] +mod tests { + use super::*; + + use std::sync::Arc; + + use zebra_chain::{block::Block, serialization::ZcashDeserialize}; + + /// Make a checkpoint list containing only the genesis block + #[test] + fn checkpoint_list_genesis() -> Result<(), Error> { + // Parse the genesis block + let mut checkpoint_data = Vec::new(); + let block = + Arc::::zcash_deserialize(&zebra_test::vectors::BLOCK_MAINNET_GENESIS_BYTES[..])?; + let hash: BlockHeaderHash = block.as_ref().into(); + checkpoint_data.push(( + block.coinbase_height().expect("test block has height"), + hash, + )); + + // Make a checkpoint list containing the genesis block + let checkpoint_list: BTreeMap = + checkpoint_data.iter().cloned().collect(); + let _ = CheckpointList::new(checkpoint_list)?; + + Ok(()) + } + + /// Make a checkpoint list containing multiple blocks + #[test] + fn checkpoint_list_multiple() -> Result<(), Error> { + // Parse all the blocks + let mut checkpoint_data = Vec::new(); + for b in &[ + &zebra_test::vectors::BLOCK_MAINNET_GENESIS_BYTES[..], + &zebra_test::vectors::BLOCK_MAINNET_1_BYTES[..], + &zebra_test::vectors::BLOCK_MAINNET_415000_BYTES[..], + &zebra_test::vectors::BLOCK_MAINNET_434873_BYTES[..], + ] { + let block = Arc::::zcash_deserialize(*b)?; + let hash: BlockHeaderHash = block.as_ref().into(); + checkpoint_data.push(( + block.coinbase_height().expect("test block has height"), + hash, + )); + } + + // Make a checkpoint list containing all the blocks + let checkpoint_list: BTreeMap = + checkpoint_data.iter().cloned().collect(); + let _ = CheckpointList::new(checkpoint_list)?; + + Ok(()) + } + + /// Make sure that an empty checkpoint list fails + #[test] + fn checkpoint_list_empty_fail() -> Result<(), Error> { + let _ = CheckpointList::new(Vec::new()).expect_err("empty checkpoint lists should fail"); + + Ok(()) + } + + /// Make sure a checkpoint list that doesn't contain the genesis block fails + #[test] + fn checkpoint_list_no_genesis_fail() -> Result<(), Error> { + // Parse a non-genesis block + let mut checkpoint_data = Vec::new(); + let block = + Arc::::zcash_deserialize(&zebra_test::vectors::BLOCK_MAINNET_1_BYTES[..])?; + let hash: BlockHeaderHash = block.as_ref().into(); + checkpoint_data.push(( + block.coinbase_height().expect("test block has height"), + hash, + )); + + // Make a checkpoint list containing the non-genesis block + let checkpoint_list: BTreeMap = + checkpoint_data.iter().cloned().collect(); + let _ = CheckpointList::new(checkpoint_list) + .expect_err("a checkpoint list with no genesis block should fail"); + + Ok(()) + } + + /// Make sure that a checkpoint list containing duplicate blocks fails + #[test] + fn checkpoint_list_duplicate_blocks_fail() -> Result<(), Error> { + // Parse some blocks twice + let mut checkpoint_data = Vec::new(); + for b in &[ + &zebra_test::vectors::BLOCK_MAINNET_GENESIS_BYTES[..], + &zebra_test::vectors::BLOCK_MAINNET_1_BYTES[..], + &zebra_test::vectors::BLOCK_MAINNET_1_BYTES[..], + ] { + let block = Arc::::zcash_deserialize(*b)?; + let hash: BlockHeaderHash = block.as_ref().into(); + checkpoint_data.push(( + block.coinbase_height().expect("test block has height"), + hash, + )); + } + + // Make a checkpoint list containing some duplicate blocks + let _ = CheckpointList::new(checkpoint_data) + .expect_err("checkpoint lists with duplicate blocks should fail"); + + Ok(()) + } + + /// Make sure that a checkpoint list containing duplicate heights + /// (with different hashes) fails + #[test] + fn checkpoint_list_duplicate_heights_fail() -> Result<(), Error> { + // Parse the genesis block + let mut checkpoint_data = Vec::new(); + for b in &[&zebra_test::vectors::BLOCK_MAINNET_GENESIS_BYTES[..]] { + let block = Arc::::zcash_deserialize(*b)?; + let hash: BlockHeaderHash = block.as_ref().into(); + checkpoint_data.push(( + block.coinbase_height().expect("test block has height"), + hash, + )); + } + + // Then add some fake entries with duplicate heights + checkpoint_data.push((BlockHeight(1), BlockHeaderHash([0xaa; 32]))); + checkpoint_data.push((BlockHeight(1), BlockHeaderHash([0xbb; 32]))); + + // Make a checkpoint list containing some duplicate blocks + let _ = CheckpointList::new(checkpoint_data) + .expect_err("checkpoint lists with duplicate heights should fail"); + + Ok(()) + } + + /// Make sure that a checkpoint list containing duplicate hashes + /// (at different heights) fails + #[test] + fn checkpoint_list_duplicate_hashes_fail() -> Result<(), Error> { + // Parse the genesis block + let mut checkpoint_data = Vec::new(); + for b in &[&zebra_test::vectors::BLOCK_MAINNET_GENESIS_BYTES[..]] { + let block = Arc::::zcash_deserialize(*b)?; + let hash: BlockHeaderHash = block.as_ref().into(); + checkpoint_data.push(( + block.coinbase_height().expect("test block has height"), + hash, + )); + } + + // Then add some fake entries with duplicate hashes + checkpoint_data.push((BlockHeight(1), BlockHeaderHash([0xcc; 32]))); + checkpoint_data.push((BlockHeight(2), BlockHeaderHash([0xcc; 32]))); + + // Make a checkpoint list containing some duplicate blocks + let _ = CheckpointList::new(checkpoint_data) + .expect_err("checkpoint lists with duplicate hashes should fail"); + + Ok(()) + } +}