diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index 5890cb863..ef18d1b4b 100644 --- a/zebra-consensus/src/block.rs +++ b/zebra-consensus/src/block.rs @@ -77,9 +77,24 @@ where // TODO(jlusby): Error = Report, handle errors from state_service. async move { + let hash = block.hash(); + + // These checks only apply to generated blocks. We check the block + // height for parsed blocks when we deserialize them. + let height = block + .coinbase_height() + .ok_or("Invalid block: missing block height.")?; + if height > BlockHeight::MAX { + Err("Invalid block height: greater than the maximum height.")?; + } + + // Check that this block is actually a new block + if BlockVerifier::get_block(&mut state, hash).await?.is_some() { + Err(format!("Block has already been verified. {:?} {:?}", height, hash))?; + } + // Do the difficulty checks first, to raise the threshold for // attacks that use any other fields. - let hash = block.hash(); let difficulty_threshold = block.header.difficulty_threshold.to_expanded().ok_or("Invalid difficulty threshold in block header.")?; if hash > difficulty_threshold { Err("Block failed the difficulty filter: hash must be less than or equal to the difficulty threshold.")?; @@ -94,15 +109,6 @@ where block.header.is_time_valid_at(now)?; block.is_coinbase_first()?; - // These checks only apply to generated blocks. We check the block - // height for parsed blocks when we deserialize them. - let height = block - .coinbase_height() - .ok_or("Invalid block: missing block height.")?; - if height > BlockHeight::MAX { - Err("Invalid block height: greater than the maximum height.")?; - } - // TODO: // - context-free header verification: merkle root // - contextual verification diff --git a/zebra-consensus/src/chain.rs b/zebra-consensus/src/chain.rs index 8a15f1be6..65188924f 100644 --- a/zebra-consensus/src/chain.rs +++ b/zebra-consensus/src/chain.rs @@ -78,7 +78,6 @@ where /// Used for debugging. /// /// Updated before verification: the block at this height might not be valid. - /// Not updated for unexpected high blocks. last_block_height: Option, } @@ -130,10 +129,10 @@ where (Some(BlockHeight(block_height)), Some(BlockHeight(last_block_height))) if (block_height > last_block_height + MAX_EXPECTED_BLOCK_GAP) => { + self.last_block_height = Some(BlockHeight(block_height)); true } (Some(block_height), _) => { - // Update the last height if the block height was expected self.last_block_height = Some(block_height); false } @@ -147,11 +146,15 @@ where // Call a verifier based on the block height and checkpoints. if is_higher_than_max_checkpoint(block_height, max_checkpoint_height) { - // Log an info-level message on early high blocks. + // Log a message on early high blocks. // The sync service rejects most of these blocks, but we // still want to know if a large number get through. + // + // This message can also happen if we keep getting unexpected + // low blocks. (We can't distinguish between these cases, until + // we've verified the blocks.) if is_unexpected_high_block { - tracing::info!(?block_height, "unexpected high block"); + tracing::debug!(?block_height, "unexpected high block, or recent unexpected low blocks"); } block_verifier diff --git a/zebra-consensus/src/checkpoint.rs b/zebra-consensus/src/checkpoint.rs index 313f5aff8..8f3ff5aec 100644 --- a/zebra-consensus/src/checkpoint.rs +++ b/zebra-consensus/src/checkpoint.rs @@ -330,7 +330,7 @@ impl CheckpointVerifier { InitialTip(previous_height) | PreviousCheckpoint(previous_height) if (height <= previous_height) => { - Err("block height has already been verified")? + Err(format!("Block has already been verified. {:?}", height))? } InitialTip(_) | PreviousCheckpoint(_) => {} // We're finished, so no checkpoint height is valid @@ -394,7 +394,9 @@ impl CheckpointVerifier { let height = match self.check_block(&block) { Ok(height) => height, Err(error) => { - tracing::warn!(?error); + // Block errors happen frequently on mainnet, due to bad peers. + tracing::debug!(?error); + // Sending might fail, depending on what the caller does with rx, // but there's nothing we can do about it. let _ = tx.send(Err(error)); @@ -573,7 +575,7 @@ impl CheckpointVerifier { } else { // The last block height we processed did not have any blocks // with a matching hash, so chain verification has failed. - tracing::warn!( + tracing::info!( ?current_height, ?current_range, "No valid blocks at height in CheckpointVerifier"