diff --git a/zebra-consensus/src/chain.rs b/zebra-consensus/src/chain.rs index fa76f7ac..3e3722ae 100644 --- a/zebra-consensus/src/chain.rs +++ b/zebra-consensus/src/chain.rs @@ -27,11 +27,6 @@ use crate::{ BoxError, Config, }; -/// The maximum expected gap between blocks. -/// -/// Used to identify unexpected out of order blocks. -const MAX_EXPECTED_BLOCK_GAP: u32 = 100_000; - /// The chain verifier routes requests to either the checkpoint verifier or the /// block verifier, depending on the maximum checkpoint height. struct ChainVerifier @@ -42,7 +37,6 @@ where block: BlockVerifier, checkpoint: CheckpointVerifier, max_checkpoint_height: block::Height, - last_block_height: Option, } #[derive(Debug, Display, Error)] @@ -76,60 +70,20 @@ where } fn call(&mut self, block: Arc) -> Self::Future { - let height = block.coinbase_height(); - let span = tracing::info_span!("ChainVerifier::call", ?height); - let _entered = span.enter(); - tracing::debug!("verifying new block"); - - // TODO: do we still need this logging? - // Log an info-level message on unexpected out of order blocks - let is_unexpected_gap = match (height, self.last_block_height) { - (Some(block::Height(height)), Some(block::Height(last_block_height))) - if (height > last_block_height + MAX_EXPECTED_BLOCK_GAP) - || (height + MAX_EXPECTED_BLOCK_GAP < last_block_height) => - { - self.last_block_height = Some(block::Height(height)); - true - } - (Some(height), _) => { - self.last_block_height = Some(height); - false - } - // The other cases are covered by the verifiers - _ => false, - }; - // Log a message on unexpected out of order blocks. - // - // The sync service rejects most of these blocks, but we - // still want to know if a large number get through. - if is_unexpected_gap { - tracing::debug!( - "large block height gap: this block or the previous block is out of order" - ); + match block.coinbase_height() { + Some(height) if height <= self.max_checkpoint_height => self + .checkpoint + .call(block) + .map_err(VerifyChainError::Checkpoint) + .boxed(), + // This also covers blocks with no height, which the block verifier + // will reject immediately. + _ => self + .block + .call(block) + .map_err(VerifyChainError::Block) + .boxed(), } - - self.last_block_height = height; - - if let Some(height) = height { - if height <= self.max_checkpoint_height { - return self - .checkpoint - .call(block) - .map_err(VerifyChainError::Checkpoint) - .boxed(); - } - } - - // For the purposes of routing requests, we can send blocks - // with no height to the block verifier, which will reject them. - // - // We choose the block verifier because it doesn't have any - // internal state, so it will always return the same error for a - // block with no height. - self.block - .call(block) - .map_err(VerifyChainError::Block) - .boxed() } } @@ -177,7 +131,6 @@ where block, checkpoint, max_checkpoint_height, - last_block_height: None, }), 3, ) diff --git a/zebra-consensus/src/checkpoint.rs b/zebra-consensus/src/checkpoint.rs index bcb93a98..6151b07a 100644 --- a/zebra-consensus/src/checkpoint.rs +++ b/zebra-consensus/src/checkpoint.rs @@ -499,15 +499,7 @@ where ); let is_checkpoint = self.checkpoint_list.contains(height); - tracing::trace!(?height, ?hash, ?is_checkpoint, "Queued block"); - - // TODO(teor): - // - Remove this log once the CheckpointVerifier is working? - // - Modify the default filter or add another log, so users see - // regular download progress info (vs verification info) - if is_checkpoint { - tracing::info!(?height, ?hash, ?is_checkpoint, "Queued checkpoint block"); - } + tracing::debug!(?height, ?hash, ?is_checkpoint, "queued block"); rx } @@ -808,7 +800,7 @@ where Poll::Ready(Ok(())) } - #[instrument(name = "checkpoint_call", skip(self, block))] + #[instrument(name = "checkpoint", skip(self, block))] fn call(&mut self, block: Arc) -> Self::Future { // Immediately reject all incoming blocks that arrive after we've finished. if let FinalCheckpoint = self.previous_checkpoint_height() {