diff --git a/zebra-chain/src/block/header.rs b/zebra-chain/src/block/header.rs index f3441e18..8674b69d 100644 --- a/zebra-chain/src/block/header.rs +++ b/zebra-chain/src/block/header.rs @@ -124,7 +124,7 @@ impl Header { /// A header with a count of the number of transactions in its block. /// /// This structure is used in the Bitcoin network protocol. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] pub struct CountedHeader { /// The header for a block diff --git a/zebra-chain/src/block/tests/preallocate.rs b/zebra-chain/src/block/tests/preallocate.rs index 81f1be22..77b29cb1 100644 --- a/zebra-chain/src/block/tests/preallocate.rs +++ b/zebra-chain/src/block/tests/preallocate.rs @@ -76,7 +76,7 @@ proptest! { let max_allocation: usize = CountedHeader::max_allocation().try_into().unwrap(); let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1); for _ in 0..(CountedHeader::max_allocation()+1) { - smallest_disallowed_vec.push(header.clone()); + smallest_disallowed_vec.push(header); } let smallest_disallowed_serialized = smallest_disallowed_vec.zcash_serialize_to_vec().expect("Serialization to vec must succeed"); // Check that our smallest_disallowed_vec is only one item larger than the limit diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index 3153f852..2672f928 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -1,4 +1,5 @@ use std::{ + convert::TryInto, future::Future, pin::Pin, sync::Arc, @@ -411,7 +412,8 @@ impl StateService { /// * adding the `stop` hash to the list, if it is in the best chain, or /// * adding `max_len` hashes to the list. /// - /// Returns an empty list if the state is empty. + /// Returns an empty list if the state is empty, + /// or if the `intersection` is the best chain tip. pub fn collect_best_chain_hashes( &self, intersection: Option, @@ -424,6 +426,11 @@ impl StateService { let chain_tip_height = if let Some((height, _)) = self.best_tip() { height } else { + tracing::info!( + response_len = ?0, + "responding to peer GetBlocks or GetHeaders with empty state", + ); + return Vec::new(); }; @@ -437,7 +444,12 @@ impl StateService { .expect("the Find response height does not exceed Height::MAX") } else { // start at genesis, and return max_len hashes - block::Height((max_len - 1) as _) + let max_height = block::Height( + max_len + .try_into() + .expect("max_len does not exceed Height::MAX"), + ); + (max_height - 1).expect("max_len is at least 1") }; let stop_height = stop.map(|hash| self.best_height_by_hash(hash)).flatten(); @@ -489,11 +501,12 @@ impl StateService { .unwrap_or(true), "the list must not contain the intersection hash" ); - assert!( - stop.map(|hash| !res[..(res.len() - 1)].contains(&hash)) - .unwrap_or(true), - "if the stop hash is in the list, it must be the final hash" - ); + if let (Some(stop), Some((_, res_except_last))) = (stop, res.split_last()) { + assert!( + !res_except_last.contains(&stop), + "if the stop hash is in the list, it must be the final hash" + ); + } res } diff --git a/zebra-state/src/service/tests.rs b/zebra-state/src/service/tests.rs index be35cc45..61dc712d 100644 --- a/zebra-state/src/service/tests.rs +++ b/zebra-state/src/service/tests.rs @@ -4,7 +4,7 @@ use futures::stream::FuturesUnordered; use tower::{buffer::Buffer, util::BoxService, Service, ServiceExt}; use zebra_chain::{ - block::{self, Block}, + block::{self, Block, CountedHeader}, chain_tip::ChainTip, fmt::SummaryDebug, parameters::{Network, NetworkUpgrade}, @@ -52,15 +52,61 @@ async fn populated_state( async fn test_populated_state_responds_correctly( mut state: Buffer, Request>, ) -> Result<()> { - let blocks = zebra_test::vectors::MAINNET_BLOCKS + let blocks: Vec> = zebra_test::vectors::MAINNET_BLOCKS .range(0..=LAST_BLOCK_HEIGHT) - .map(|(_, block_bytes)| block_bytes.zcash_deserialize_into::>().unwrap()); + .map(|(_, block_bytes)| block_bytes.zcash_deserialize_into().unwrap()) + .collect(); + + let block_hashes: Vec = blocks.iter().map(|block| block.hash()).collect(); + let block_headers: Vec = blocks + .iter() + .map(|block| CountedHeader { + header: block.header, + transaction_count: block.transactions.len(), + }) + .collect(); for (ind, block) in blocks.into_iter().enumerate() { let mut transcript = vec![]; let height = block.coinbase_height().unwrap(); let hash = block.hash(); + transcript.push(( + Request::Depth(block.hash()), + Ok(Response::Depth(Some(LAST_BLOCK_HEIGHT - height.0))), + )); + + // these requests don't have any arguments, so we just do them once + if ind == LAST_BLOCK_HEIGHT as usize { + transcript.push((Request::Tip, Ok(Response::Tip(Some((height, hash)))))); + + let locator_hashes = vec![ + block_hashes[LAST_BLOCK_HEIGHT as usize], + block_hashes[(LAST_BLOCK_HEIGHT - 1) as usize], + block_hashes[(LAST_BLOCK_HEIGHT - 2) as usize], + block_hashes[(LAST_BLOCK_HEIGHT - 4) as usize], + block_hashes[(LAST_BLOCK_HEIGHT - 8) as usize], + block_hashes[0], + ]; + + transcript.push(( + Request::BlockLocator, + Ok(Response::BlockLocator(locator_hashes)), + )); + } + + // Spec: transactions in the genesis block are ignored. + if height.0 != 0 { + for transaction in &block.transactions { + let transaction_hash = transaction.hash(); + + transcript.push(( + Request::Transaction(transaction_hash), + Ok(Response::Transaction(Some(transaction.clone()))), + )); + } + } + transcript.push(( Request::Block(hash.into()), Ok(Response::Block(Some(block.clone()))), @@ -71,26 +117,11 @@ async fn test_populated_state_responds_correctly( Ok(Response::Block(Some(block.clone()))), )); - transcript.push(( - Request::Depth(block.hash()), - Ok(Response::Depth(Some(LAST_BLOCK_HEIGHT - height.0))), - )); - - if ind == LAST_BLOCK_HEIGHT as usize { - transcript.push((Request::Tip, Ok(Response::Tip(Some((height, hash)))))); - } - - // Consensus-critical bug in zcashd: transactions in the genesis block - // are ignored. + // Spec: transactions in the genesis block are ignored. if height.0 != 0 { for transaction in &block.transactions { let transaction_hash = transaction.hash(); - transcript.push(( - Request::Transaction(transaction_hash), - Ok(Response::Transaction(Some(transaction.clone()))), - )); - let from_coinbase = transaction.has_valid_coinbase_transaction_inputs(); for (index, output) in transaction.outputs().iter().cloned().enumerate() { let outpoint = transparent::OutPoint { @@ -108,6 +139,76 @@ async fn test_populated_state_responds_correctly( } } + let mut append_locator_transcript = |split_ind| { + let block_hashes = block_hashes.clone(); + let (known_hashes, next_hashes) = block_hashes.split_at(split_ind); + + let block_headers = block_headers.clone(); + let (_, next_headers) = block_headers.split_at(split_ind); + + // no stop + transcript.push(( + Request::FindBlockHashes { + known_blocks: known_hashes.iter().rev().cloned().collect(), + stop: None, + }, + Ok(Response::BlockHashes(next_hashes.to_vec())), + )); + + transcript.push(( + Request::FindBlockHeaders { + known_blocks: known_hashes.iter().rev().cloned().collect(), + stop: None, + }, + Ok(Response::BlockHeaders(next_headers.to_vec())), + )); + + // stop at the next block + transcript.push(( + Request::FindBlockHashes { + known_blocks: known_hashes.iter().rev().cloned().collect(), + stop: next_hashes.get(0).cloned(), + }, + Ok(Response::BlockHashes( + next_hashes.get(0).iter().cloned().cloned().collect(), + )), + )); + + transcript.push(( + Request::FindBlockHeaders { + known_blocks: known_hashes.iter().rev().cloned().collect(), + stop: next_hashes.get(0).cloned(), + }, + Ok(Response::BlockHeaders( + next_headers.get(0).iter().cloned().cloned().collect(), + )), + )); + + // stop at a block that isn't actually in the chain + // tests bug #2789 + transcript.push(( + Request::FindBlockHashes { + known_blocks: known_hashes.iter().rev().cloned().collect(), + stop: Some(block::Hash([0xff; 32])), + }, + Ok(Response::BlockHashes(next_hashes.to_vec())), + )); + + transcript.push(( + Request::FindBlockHeaders { + known_blocks: known_hashes.iter().rev().cloned().collect(), + stop: Some(block::Hash([0xff; 32])), + }, + Ok(Response::BlockHeaders(next_headers.to_vec())), + )); + }; + + // split before the current block, and locate the current block + append_locator_transcript(ind); + + // split after the current block, and locate the next block + append_locator_transcript(ind + 1); + let transcript = Transcript::from(transcript); transcript.check(&mut state).await?; } @@ -157,6 +258,20 @@ async fn empty_state_still_responds_to_requests() -> Result<()> { Ok(Response::Block(None)), ), // No check for AwaitUTXO because it will wait if the UTXO isn't present + ( + Request::FindBlockHashes { + known_blocks: vec![block.hash()], + stop: None, + }, + Ok(Response::BlockHashes(Vec::new())), + ), + ( + Request::FindBlockHeaders { + known_blocks: vec![block.hash()], + stop: None, + }, + Ok(Response::BlockHeaders(Vec::new())), + ), ] .into_iter(); let transcript = Transcript::from(iter);