consensus: simplify block verify tracing output

The previous debug output printed a message that the chain verifier had
recieved a block.  But this provides no additional information compared
to printing no message in chain::Verifier and a message in whichever
verifier the block was sent to, since the resulting spans indicate where
the block was dispatched.

This commit also removes the "unexpected high block" detection; this was
an artefact of the original sync algorithm failing to handle block
advertisements, but we don't have that problem any more, so we can
simplify the code by eliminating that logic.

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<S>
@@ -42,7 +37,6 @@ where
     block: BlockVerifier<S>,
     checkpoint: CheckpointVerifier<S>,
     max_checkpoint_height: block::Height,
-    last_block_height: Option<block::Height>,
 }
 
 #[derive(Debug, Display, Error)]
@@ -76,60 +70,20 @@ where
     }
 
     fn call(&mut self, block: Arc<Block>) -> Self::Future {
-        let height = block.coinbase_height();
+        match block.coinbase_height() {
-        let span = tracing::info_span!("ChainVerifier::call", ?height);
+            Some(height) if height <= self.max_checkpoint_height => self
-        let _entered = span.enter();
+                .checkpoint
-        tracing::debug!("verifying new block");
+                .call(block)
-
+                .map_err(VerifyChainError::Checkpoint)
-        // TODO: do we still need this logging?
+                .boxed(),
-        // Log an info-level message on unexpected out of order blocks
+            // This also covers blocks with no height, which the block verifier
-        let is_unexpected_gap = match (height, self.last_block_height) {
+            // will reject immediately.
-            (Some(block::Height(height)), Some(block::Height(last_block_height)))
+            _ => self
-                if (height > last_block_height + MAX_EXPECTED_BLOCK_GAP)
+                .block
-                    || (height + MAX_EXPECTED_BLOCK_GAP < last_block_height) =>
+                .call(block)
-            {
+                .map_err(VerifyChainError::Block)
-                self.last_block_height = Some(block::Height(height));
+                .boxed(),
-                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"
-            );
         }
-
-        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,
     )

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");
+        tracing::debug!(?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");
-        }
 
         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<Block>) -> Self::Future {
         // Immediately reject all incoming blocks that arrive after we've finished.
         if let FinalCheckpoint = self.previous_checkpoint_height() {