[#6301] Track bad block cause (#6622)

Signed-off-by: mbaxter <mbaxter.dev@gmail.com>

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java

@@ -25,6 +25,7 @@ import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.BlockProcessingResult;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.blockcreation.BlockCreator.BlockCreationResult;
+import org.hyperledger.besu.ethereum.chain.BadBlockCause;
 import org.hyperledger.besu.ethereum.chain.BadBlockManager;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
@@ -781,7 +782,7 @@ public class MergeCoordinator implements MergeMiningCoordinator, BadChainListene
       final Block badBlock,
       final List<Block> badBlockDescendants,
       final List<BlockHeader> badBlockHeaderDescendants) {
-    LOG.trace("Adding bad block {} and all its descendants", badBlock.getHash());
+    LOG.trace("Mark descendents of bad block {} as bad", badBlock.getHash());
     final BadBlockManager badBlockManager = protocolContext.getBadBlockManager();
 
     final Optional<BlockHeader> parentHeader =
@@ -791,12 +792,11 @@ public class MergeCoordinator implements MergeMiningCoordinator, BadChainListene
             ? Optional.of(parentHeader.get().getHash())
             : Optional.empty();
 
-    badBlockManager.addBadBlock(badBlock, Optional.empty());
-
+    // Bad block has already been marked, but we need to mark the bad block's descendants
     badBlockDescendants.forEach(
         block -> {
           LOG.trace("Add descendant block {} to bad blocks", block.getHash());
-          badBlockManager.addBadBlock(block, Optional.empty());
+          badBlockManager.addBadBlock(block, BadBlockCause.fromBadAncestorBlock(badBlock));
           maybeLatestValidHash.ifPresent(
               latestValidHash ->
                   badBlockManager.addLatestValidHash(block.getHash(), latestValidHash));
@@ -805,7 +805,7 @@ public class MergeCoordinator implements MergeMiningCoordinator, BadChainListene
     badBlockHeaderDescendants.forEach(
         header -> {
           LOG.trace("Add descendant header {} to bad blocks", header.getHash());
-          badBlockManager.addBadHeader(header);
+          badBlockManager.addBadHeader(header, BadBlockCause.fromBadAncestorBlock(badBlock));
           maybeLatestValidHash.ifPresent(
               latestValidHash ->
                   badBlockManager.addLatestValidHash(header.getHash(), latestValidHash));
@@ -835,16 +835,10 @@ public class MergeCoordinator implements MergeMiningCoordinator, BadChainListene
     MergeBlockCreator forParams(BlockHeader header, Optional<Address> feeRecipient);
   }
 
-  @Override
-  public void addBadBlock(final Block block, final Optional<Throwable> maybeCause) {
-    protocolContext.getBadBlockManager().addBadBlock(block, maybeCause);
-  }
-
   @Override
   public boolean isBadBlock(final Hash blockHash) {
     final BadBlockManager badBlockManager = protocolContext.getBadBlockManager();
-    return badBlockManager.getBadBlock(blockHash).isPresent()
-        || badBlockManager.getBadHash(blockHash).isPresent();
+    return badBlockManager.isBadBlock(blockHash);
   }
 
   @Override

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeMiningCoordinator.java

@@ -141,14 +141,6 @@ public interface MergeMiningCoordinator extends MiningCoordinator {
    */
   boolean isMiningBeforeMerge();
 
-  /**
-   * Add bad block.
-   *
-   * @param block the block
-   * @param maybeCause the maybe cause
-   */
-  void addBadBlock(final Block block, Optional<Throwable> maybeCause);
-
   /**
    * Is bad block.
    *

consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/TransitionCoordinator.java

@@ -210,11 +210,6 @@ public class TransitionCoordinator extends TransitionUtils<MiningCoordinator>
     return mergeCoordinator.isDescendantOf(ancestorBlock, newBlock);
   }
 
-  @Override
-  public void addBadBlock(final Block block, final Optional<Throwable> maybeCause) {
-    mergeCoordinator.addBadBlock(block, maybeCause);
-  }
-
   @Override
   public boolean isBadBlock(final Hash blockHash) {
     return mergeCoordinator.isBadBlock(blockHash);

consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java

@@ -325,7 +325,6 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper {
             Optional.empty(),
             Optional.empty());
 
-    verify(willThrow, never()).addBadBlock(any(), any());
     blockCreationTask.get();
 
     ArgumentCaptor<PayloadWrapper> payloadWrapper = ArgumentCaptor.forClass(PayloadWrapper.class);
@@ -345,7 +344,6 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper {
     // this only verifies that adding the bad block didn't happen through the mergeCoordinator, it
     // still may be called directly.
     verify(badBlockManager, never()).addBadBlock(any(), any());
-    verify(willThrow, never()).addBadBlock(any(), any());
   }
 
   @Test

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/DebugGetBadBlockTest.java

@@ -27,6 +27,7 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.BadBlockResult;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.BlockResultFactory;
+import org.hyperledger.besu.ethereum.chain.BadBlockCause;
 import org.hyperledger.besu.ethereum.chain.BadBlockManager;
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
@@ -37,7 +38,6 @@ import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
-import java.util.Optional;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -95,8 +95,10 @@ public class DebugGetBadBlockTest {
                     .setBlockHeaderFunctions(new MainnetBlockHeaderFunctions())
                     .setParentHash(parentBlock.getHash()));
 
-    badBlockManager.addBadBlock(badBlockWithTransaction, Optional.empty());
-    badBlockManager.addBadBlock(badBlockWoTransaction, Optional.empty());
+    badBlockManager.addBadBlock(
+        badBlockWithTransaction, BadBlockCause.fromValidationFailure("failed"));
+    badBlockManager.addBadBlock(
+        badBlockWoTransaction, BadBlockCause.fromValidationFailure("failed"));
 
     final JsonRpcRequestContext request =
         new JsonRpcRequestContext(new JsonRpcRequest("2.0", "debug_traceBlock", new Object[] {}));

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/DebugStandardTraceBadBlockToFileTest.java

@@ -28,6 +28,7 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.processor.TransactionT
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.api.query.BlockchainQueries;
+import org.hyperledger.besu.ethereum.chain.BadBlockCause;
 import org.hyperledger.besu.ethereum.chain.BadBlockManager;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.core.Block;
@@ -99,7 +100,7 @@ public class DebugStandardTraceBadBlockToFileTest {
     final List<String> paths = new ArrayList<>();
     paths.add("path-1");
 
-    badBlockManager.addBadBlock(block, Optional.empty());
+    badBlockManager.addBadBlock(block, BadBlockCause.fromValidationFailure("failed"));
 
     when(blockchainQueries.getBlockchain()).thenReturn(blockchain);
     when(transactionTracer.traceTransactionToFile(

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java

@@ -22,7 +22,6 @@ import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.Executi
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -59,7 +58,6 @@ import org.hyperledger.besu.ethereum.core.Withdrawal;
 import org.hyperledger.besu.ethereum.eth.manager.EthPeers;
 import org.hyperledger.besu.ethereum.mainnet.BodyValidation;
 import org.hyperledger.besu.ethereum.mainnet.DepositsValidator;
-import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
 import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator;
 import org.hyperledger.besu.ethereum.trie.MerkleTrieException;
@@ -82,17 +80,6 @@ import org.mockito.junit.jupiter.MockitoExtension;
 @ExtendWith(MockitoExtension.class)
 public abstract class AbstractEngineNewPayloadTest extends AbstractScheduledApiTest {
 
-  @FunctionalInterface
-  interface MethodFactory {
-    AbstractEngineNewPayload create(
-        final Vertx vertx,
-        final ProtocolSchedule protocolSchedule,
-        final ProtocolContext protocolContext,
-        final MergeMiningCoordinator mergeCoordinator,
-        final EthPeers ethPeers,
-        final EngineCallListener engineCallListener);
-  }
-
   protected AbstractEngineNewPayload method;
   protected Optional<Bytes32> maybeParentBeaconBlockRoot = Optional.empty();
 
@@ -229,8 +216,6 @@ public abstract class AbstractEngineNewPayloadTest extends AbstractScheduledApiT
 
     fromErrorResp(resp);
     verify(engineCallListener, times(1)).executionEngineCalled();
-    verify(mergeCoordinator, times(0)).addBadBlock(any(), any());
-    // verify mainnetBlockValidator does not add to bad block manager
   }
 
   @Test
@@ -247,7 +232,6 @@ public abstract class AbstractEngineNewPayloadTest extends AbstractScheduledApiT
     var resp = resp(mockEnginePayload(mockHeader, Collections.emptyList()));
 
     verify(engineCallListener, times(1)).executionEngineCalled();
-    verify(mergeCoordinator, times(0)).addBadBlock(any(), any());
 
     fromErrorResp(resp);
   }
@@ -265,7 +249,6 @@ public abstract class AbstractEngineNewPayloadTest extends AbstractScheduledApiT
     var resp = resp(mockEnginePayload(mockHeader, Collections.emptyList()));
 
     verify(engineCallListener, times(1)).executionEngineCalled();
-    verify(mergeCoordinator, never()).addBadBlock(any(), any());
 
     fromErrorResp(resp);
   }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java

@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.ethereum;
 
+import org.hyperledger.besu.ethereum.chain.BadBlockCause;
 import org.hyperledger.besu.ethereum.chain.BadBlockManager;
 import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
 import org.hyperledger.besu.ethereum.core.Block;
@@ -111,7 +112,8 @@ public class MainnetBlockValidator implements BlockValidator {
 
       if (!blockHeaderValidator.validateHeader(
           header, parentHeader, context, headerValidationMode)) {
-        var retval = new BlockProcessingResult("header validation rule violated, see logs");
+        final String error = String.format("Header validation failed (%s)", headerValidationMode);
+        var retval = new BlockProcessingResult(error);
         handleAndLogImportFailure(block, retval, shouldRecordBadBlock);
         return retval;
       }
@@ -141,8 +143,9 @@ public class MainnetBlockValidator implements BlockValidator {
             result.getYield().map(BlockProcessingOutputs::getReceipts).orElse(new ArrayList<>());
         if (!blockBodyValidator.validateBody(
             context, block, receipts, worldState.rootHash(), ommerValidationMode)) {
+          result = new BlockProcessingResult("failed to validate output of imported block");
           handleAndLogImportFailure(block, result, shouldRecordBadBlock);
-          return new BlockProcessingResult("failed to validate output of imported block");
+          return result;
         }
 
         return new BlockProcessingResult(
@@ -187,7 +190,18 @@ public class MainnetBlockValidator implements BlockValidator {
       }
     }
     if (shouldRecordBadBlock) {
-      badBlockManager.addBadBlock(invalidBlock, result.causedBy());
+      BadBlockCause cause =
+          result
+              .causedBy()
+              .map(BadBlockCause::fromProcessingError)
+              .orElseGet(
+                  () -> {
+                    // Result.errorMessage should not be empty on failure, but add a default to be
+                    // safe
+                    String description = result.errorMessage.orElse("Unknown cause");
+                    return BadBlockCause.fromValidationFailure(description);
+                  });
+      badBlockManager.addBadBlock(invalidBlock, cause);
     } else {
       LOG.debug("Invalid block {} not added to badBlockManager ", invalidBlock.toLogString());
     }
@@ -216,12 +230,14 @@ public class MainnetBlockValidator implements BlockValidator {
       final HeaderValidationMode ommerValidationMode) {
     final BlockHeader header = block.getHeader();
     if (!blockHeaderValidator.validateHeader(header, context, headerValidationMode)) {
-      badBlockManager.addBadBlock(block, Optional.empty());
+      String description = String.format("Failed header validation (%s)", headerValidationMode);
+      badBlockManager.addBadBlock(block, BadBlockCause.fromValidationFailure(description));
       return false;
     }
 
     if (!blockBodyValidator.validateBodyLight(context, block, receipts, ommerValidationMode)) {
-      badBlockManager.addBadBlock(block, Optional.empty());
+      badBlockManager.addBadBlock(
+          block, BadBlockCause.fromValidationFailure("Failed body validation (light)"));
       return false;
     }
     return true;

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockCause.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright Hyperledger Besu Contributors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ */
+package org.hyperledger.besu.ethereum.chain;
+
+import org.hyperledger.besu.ethereum.core.Block;
+
+import java.util.Optional;
+
+public class BadBlockCause {
+  public enum BadBlockReason {
+    // Standard spec-related validation failures
+    SPEC_VALIDATION_FAILURE,
+    // When an unexpected exception occurs during block processing
+    EXCEPTIONAL_BLOCK_PROCESSING,
+    // This block is bad because it descends from a bad block
+    DESCENDS_FROM_BAD_BLOCK,
+  }
+
+  private final BadBlockReason reason;
+  private final String description;
+  private final Optional<Throwable> exception;
+
+  public static BadBlockCause fromProcessingError(final Throwable t) {
+    final String description = t.getLocalizedMessage();
+    return new BadBlockCause(
+        BadBlockReason.EXCEPTIONAL_BLOCK_PROCESSING, description, Optional.of(t));
+  }
+
+  public static BadBlockCause fromBadAncestorBlock(final Block badAncestor) {
+    final String description =
+        String.format("Descends from bad block %s", badAncestor.toLogString());
+    return new BadBlockCause(BadBlockReason.DESCENDS_FROM_BAD_BLOCK, description, Optional.empty());
+  }
+
+  public static BadBlockCause fromValidationFailure(final String failureMessage) {
+    return new BadBlockCause(
+        BadBlockReason.SPEC_VALIDATION_FAILURE, failureMessage, Optional.empty());
+  }
+
+  private BadBlockCause(BadBlockReason reason, String description, Optional<Throwable> exception) {
+    this.reason = reason;
+    this.description = description;
+    this.exception = exception;
+  }
+
+  public BadBlockReason getReason() {
+    return reason;
+  }
+
+  public String getDescription() {
+    return description;
+  }
+
+  public Optional<Throwable> getException() {
+    return exception;
+  }
+
+  @Override
+  public String toString() {
+    return "BadBlockCause{"
+        + "reason="
+        + reason
+        + ", description='"
+        + description
+        + '\''
+        + ", exception="
+        + exception
+        + '}';
+  }
+}

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockManager.java

@@ -23,10 +23,14 @@ import org.hyperledger.besu.plugin.services.exception.StorageException;
 import java.util.Collection;
 import java.util.Optional;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class BadBlockManager {
+  private static final Logger LOG = LoggerFactory.getLogger(BadBlockManager.class);
 
   public static final int MAX_BAD_BLOCKS_SIZE = 100;
   private final Cache<Hash, Block> badBlocks =
@@ -40,13 +44,13 @@ public class BadBlockManager {
    * Add a new invalid block.
    *
    * @param badBlock the invalid block
-   * @param cause optional exception causing the block to be considered invalid
+   * @param cause the cause detailing why the block is considered invalid
    */
-  public void addBadBlock(final Block badBlock, final Optional<Throwable> cause) {
-    if (badBlock != null) {
-      if (cause.isEmpty() || !isInternalError(cause.get())) {
-        this.badBlocks.put(badBlock.getHash(), badBlock);
-      }
+  public void addBadBlock(final Block badBlock, final BadBlockCause cause) {
+    // TODO(#6301) Expose bad block with cause through BesuEvents
+    if (badBlock != null && !isInternalError(cause)) {
+      LOG.debug("Register bad block {} with cause: {}", badBlock.toLogString(), cause);
+      this.badBlocks.put(badBlock.getHash(), badBlock);
     }
   }
 
@@ -65,6 +69,11 @@ public class BadBlockManager {
     return badBlocks.asMap().values();
   }
 
+  @VisibleForTesting
+  public Collection<BlockHeader> getBadHeaders() {
+    return badHeaders.asMap().values();
+  }
+
   /**
    * Return an invalid block based on the hash
    *
@@ -75,12 +84,14 @@ public class BadBlockManager {
     return Optional.ofNullable(badBlocks.getIfPresent(hash));
   }
 
-  public void addBadHeader(final BlockHeader header) {
+  public void addBadHeader(final BlockHeader header, final BadBlockCause cause) {
+    // TODO(#6301) Expose bad block header with cause through BesuEvents
+    LOG.debug("Register bad block header {} with cause: {}", header.toLogString(), cause);
     badHeaders.put(header.getHash(), header);
   }
 
-  public Optional<BlockHeader> getBadHash(final Hash blockHash) {
-    return Optional.ofNullable(badHeaders.getIfPresent(blockHash));
+  public boolean isBadBlock(final Hash blockHash) {
+    return badBlocks.asMap().containsKey(blockHash) || badHeaders.asMap().containsKey(blockHash);
   }
 
   public void addLatestValidHash(final Hash blockHash, final Hash latestValidHash) {
@@ -91,11 +102,12 @@ public class BadBlockManager {
     return Optional.ofNullable(latestValidHashes.getIfPresent(blockHash));
   }
 
-  private boolean isInternalError(final Throwable causedBy) {
-    // As new "internal only" types of exception are discovered, add them here.
-    if (causedBy instanceof StorageException || causedBy instanceof MerkleTrieException) {
-      return true;
+  private boolean isInternalError(final BadBlockCause cause) {
+    if (cause.getException().isEmpty()) {
+      return false;
     }
-    return false;
+    // As new "internal only" types of exception are discovered, add them here.
+    Throwable causedBy = cause.getException().get();
+    return causedBy instanceof StorageException || causedBy instanceof MerkleTrieException;
   }
 }

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/BlockHeader.java

@@ -252,6 +252,7 @@ public class BlockHeader extends SealableBlockHeader
   public String toString() {
     final StringBuilder sb = new StringBuilder();
     sb.append("BlockHeader{");
+    sb.append("number=").append(number).append(", ");
     sb.append("hash=").append(getHash()).append(", ");
     sb.append("parentHash=").append(parentHash).append(", ");
     sb.append("ommersHash=").append(ommersHash).append(", ");
@@ -261,7 +262,6 @@ public class BlockHeader extends SealableBlockHeader
     sb.append("receiptsRoot=").append(receiptsRoot).append(", ");
     sb.append("logsBloom=").append(logsBloom).append(", ");
     sb.append("difficulty=").append(difficulty).append(", ");
-    sb.append("number=").append(number).append(", ");
     sb.append("gasLimit=").append(gasLimit).append(", ");
     sb.append("gasUsed=").append(gasUsed).append(", ");
     sb.append("timestamp=").append(timestamp).append(", ");

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java

@@ -17,26 +17,26 @@ package org.hyperledger.besu.ethereum;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
-import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.chain.BadBlockManager;
 import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
 import org.hyperledger.besu.ethereum.core.Block;
-import org.hyperledger.besu.ethereum.core.BlockDataGenerator;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
-import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture;
+import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil;
 import org.hyperledger.besu.ethereum.core.MutableWorldState;
 import org.hyperledger.besu.ethereum.mainnet.BlockBodyValidator;
 import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator;
 import org.hyperledger.besu.ethereum.mainnet.BlockProcessor;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
-import org.hyperledger.besu.ethereum.mainnet.MainnetBlockHeaderFunctions;
 import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
 
+import java.util.Collections;
 import java.util.Optional;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -44,49 +44,83 @@ import org.junit.jupiter.api.Test;
 
 public class MainnetBlockValidatorTest {
 
-  private final BlockHeaderValidator blockHeaderValidator = mock(BlockHeaderValidator.class);
-  private final BlockBodyValidator blockBodyValidator = mock(BlockBodyValidator.class);
-  private final BlockProcessor blockProcessor = mock(BlockProcessor.class);
+  private final BlockchainSetupUtil chainUtil = BlockchainSetupUtil.forMainnet();
+  private final Block block = chainUtil.getBlock(3);
+  private final Block blockParent = chainUtil.getBlock(2);
+
+  private final MutableBlockchain blockchain = spy(chainUtil.getBlockchain());
   private final ProtocolContext protocolContext = mock(ProtocolContext.class);
-  protected final MutableBlockchain blockchain = mock(MutableBlockchain.class);
-  protected final WorldStateArchive worldStateArchive = mock(WorldStateArchive.class);
+  private final WorldStateArchive worldStateArchive = mock(WorldStateArchive.class);
+  private final MutableWorldState worldState = mock(MutableWorldState.class);
   private final BadBlockManager badBlockManager = new BadBlockManager();
+  private final BlockProcessor blockProcessor = mock(BlockProcessor.class);
+  private final BlockHeaderValidator blockHeaderValidator = mock(BlockHeaderValidator.class);
+  private final BlockBodyValidator blockBodyValidator = mock(BlockBodyValidator.class);
 
-  private MainnetBlockValidator mainnetBlockValidator;
-  private Block badBlock;
+  private final MainnetBlockValidator mainnetBlockValidator =
+      new MainnetBlockValidator(
+          blockHeaderValidator, blockBodyValidator, blockProcessor, badBlockManager);
 
   @BeforeEach
   public void setup() {
+    chainUtil.importFirstBlocks(4);
+    final BlockProcessingResult successfulProcessingResult =
+        new BlockProcessingResult(Optional.empty(), false);
+
     when(protocolContext.getBlockchain()).thenReturn(blockchain);
     when(protocolContext.getWorldStateArchive()).thenReturn(worldStateArchive);
-    mainnetBlockValidator =
-        new MainnetBlockValidator(
-            blockHeaderValidator, blockBodyValidator, blockProcessor, badBlockManager);
-    badBlock =
-        new BlockDataGenerator()
-            .block(
-                BlockDataGenerator.BlockOptions.create()
-                    .setBlockNumber(2)
-                    .hasTransactions(false)
-                    .setBlockHeaderFunctions(new MainnetBlockHeaderFunctions()));
+    when(worldStateArchive.getMutable(any(BlockHeader.class), anyBoolean()))
+        .thenReturn(Optional.of(worldState));
+    when(worldStateArchive.getMutable(any(Hash.class), any(Hash.class)))
+        .thenReturn(Optional.of(worldState));
+    when(worldStateArchive.getMutable()).thenReturn(worldState);
+    when(blockHeaderValidator.validateHeader(any(), any(), any())).thenReturn(true);
+    when(blockHeaderValidator.validateHeader(any(), any(), any(), any())).thenReturn(true);
+    when(blockBodyValidator.validateBody(any(), any(), any(), any(), any())).thenReturn(true);
+    when(blockBodyValidator.validateBodyLight(any(), any(), any(), any())).thenReturn(true);
+    when(blockProcessor.processBlock(any(), any(), any())).thenReturn(successfulProcessingResult);
+    when(blockProcessor.processBlock(any(), any(), any(), any()))
+        .thenReturn(successfulProcessingResult);
+    when(blockProcessor.processBlock(any(), any(), any(), any(), any()))
+        .thenReturn(successfulProcessingResult);
+    when(blockProcessor.processBlock(any(), any(), any(), any(), any(), any(), any(), any()))
+        .thenReturn(successfulProcessingResult);
+
+    assertNoBadBlocks();
   }
 
   @Test
-  public void shouldDetectAndCacheInvalidBlocksWhenParentBlockNotPresent() {
-    when(blockchain.getBlockHeader(anyLong())).thenReturn(Optional.empty());
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(0);
-    mainnetBlockValidator.validateAndProcessBlock(
-        protocolContext,
-        badBlock,
-        HeaderValidationMode.DETACHED_ONLY,
-        HeaderValidationMode.DETACHED_ONLY);
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(1);
+  public void validateAndProcessBlock_onSuccess() {
+    BlockProcessingResult result =
+        mainnetBlockValidator.validateAndProcessBlock(
+            protocolContext,
+            block,
+            HeaderValidationMode.DETACHED_ONLY,
+            HeaderValidationMode.DETACHED_ONLY);
+
+    assertThat(result.isSuccessful()).isTrue();
+    assertNoBadBlocks();
   }
 
   @Test
-  public void shouldDetectAndCacheInvalidBlocksWhenHeaderInvalid() {
-    when(blockchain.getBlockHeader(any(Hash.class)))
-        .thenReturn(Optional.of(new BlockHeaderTestFixture().buildHeader()));
+  public void validateAndProcessBlock_whenParentBlockNotPresent() {
+    final Hash parentHash = blockParent.getHash();
+    doReturn(Optional.empty()).when(blockchain).getBlockHeader(eq(parentHash));
+
+    BlockProcessingResult result =
+        mainnetBlockValidator.validateAndProcessBlock(
+            protocolContext,
+            block,
+            HeaderValidationMode.DETACHED_ONLY,
+            HeaderValidationMode.DETACHED_ONLY);
+
+    final String expectedError = "Parent block with hash " + parentHash + " not present";
+    assertValidationFailed(result, expectedError);
+    assertBadBlockIsTracked(block);
+  }
+
+  @Test
+  public void validateAndProcessBlock_whenHeaderInvalid() {
     when(blockHeaderValidator.validateHeader(
             any(BlockHeader.class),
             any(BlockHeader.class),
@@ -94,177 +128,181 @@ public class MainnetBlockValidatorTest {
             eq(HeaderValidationMode.DETACHED_ONLY)))
         .thenReturn(false);
 
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(0);
-    mainnetBlockValidator.validateAndProcessBlock(
-        protocolContext,
-        badBlock,
-        HeaderValidationMode.DETACHED_ONLY,
-        HeaderValidationMode.DETACHED_ONLY);
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(1);
+    BlockProcessingResult result =
+        mainnetBlockValidator.validateAndProcessBlock(
+            protocolContext,
+            block,
+            HeaderValidationMode.DETACHED_ONLY,
+            HeaderValidationMode.DETACHED_ONLY);
+
+    final String expectedError = "Header validation failed (DETACHED_ONLY)";
+    assertValidationFailed(result, expectedError);
+    assertBadBlockIsTracked(block);
   }
 
   @Test
-  public void shouldDetectAndCacheInvalidBlocksWhenParentWorldStateNotAvailable() {
+  public void validateAndProcessBlock_whenBlockBodyInvalid() {
+    when(blockBodyValidator.validateBody(any(), eq(block), any(), any(), any())).thenReturn(false);
 
-    final BlockHeader blockHeader = new BlockHeaderTestFixture().buildHeader();
-    when(blockchain.getBlockHeader(any(Hash.class))).thenReturn(Optional.of(blockHeader));
-    when(blockHeaderValidator.validateHeader(
-            any(BlockHeader.class),
-            any(BlockHeader.class),
-            eq(protocolContext),
-            eq(HeaderValidationMode.DETACHED_ONLY)))
-        .thenReturn(true);
-    when(worldStateArchive.getMutable(eq(blockHeader), anyBoolean())).thenReturn(Optional.empty());
-
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(0);
-    mainnetBlockValidator.validateAndProcessBlock(
-        protocolContext,
-        badBlock,
-        HeaderValidationMode.DETACHED_ONLY,
-        HeaderValidationMode.DETACHED_ONLY);
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(1);
+    BlockProcessingResult result =
+        mainnetBlockValidator.validateAndProcessBlock(
+            protocolContext,
+            block,
+            HeaderValidationMode.DETACHED_ONLY,
+            HeaderValidationMode.DETACHED_ONLY);
+
+    final String expectedError = "failed to validate output of imported block";
+    assertValidationFailed(result, expectedError);
+    assertBadBlockIsTracked(block);
   }
 
   @Test
-  public void shouldDetectAndCacheInvalidBlocksWhenProcessBlockFailed() {
-    final BlockHeader blockHeader = new BlockHeaderTestFixture().buildHeader();
-    when(blockchain.getBlockHeader(any(Hash.class))).thenReturn(Optional.of(blockHeader));
-    when(blockHeaderValidator.validateHeader(
-            any(BlockHeader.class),
-            any(BlockHeader.class),
-            eq(protocolContext),
-            eq(HeaderValidationMode.DETACHED_ONLY)))
-        .thenReturn(true);
-    when(worldStateArchive.getMutable(eq(blockHeader), anyBoolean()))
-        .thenReturn(Optional.of(mock(MutableWorldState.class)));
-    when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(badBlock)))
-        .thenReturn(new BlockProcessingResult(Optional.empty()));
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(0);
-    mainnetBlockValidator.validateAndProcessBlock(
-        protocolContext,
-        badBlock,
-        HeaderValidationMode.DETACHED_ONLY,
-        HeaderValidationMode.DETACHED_ONLY);
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(1);
+  public void validateAndProcessBlock_whenParentWorldStateNotAvailable() {
+    when(worldStateArchive.getMutable(eq(blockParent.getHeader()), anyBoolean()))
+        .thenReturn(Optional.empty());
+
+    BlockProcessingResult result =
+        mainnetBlockValidator.validateAndProcessBlock(
+            protocolContext,
+            block,
+            HeaderValidationMode.DETACHED_ONLY,
+            HeaderValidationMode.DETACHED_ONLY);
+
+    final String expectedError =
+        "Unable to process block because parent world state "
+            + blockParent.getHeader().getStateRoot()
+            + " is not available";
+    assertValidationFailed(result, expectedError);
+    assertBadBlockIsTracked(block);
   }
 
   @Test
-  public void shouldDetectAndCacheInvalidBlocksWhenBodyInvalid() {
-    final BlockHeader blockHeader = new BlockHeaderTestFixture().buildHeader();
-    when(blockchain.getBlockHeader(any(Hash.class))).thenReturn(Optional.of(blockHeader));
-    when(blockHeaderValidator.validateHeader(
-            any(BlockHeader.class),
-            any(BlockHeader.class),
-            eq(protocolContext),
-            eq(HeaderValidationMode.DETACHED_ONLY)))
-        .thenReturn(true);
-    when(worldStateArchive.getMutable(eq(blockHeader), anyBoolean()))
-        .thenReturn(Optional.of(mock(MutableWorldState.class)));
-    when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(badBlock)))
-        .thenReturn(new BlockProcessingResult(Optional.empty()));
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(0);
-    mainnetBlockValidator.validateAndProcessBlock(
-        protocolContext,
-        badBlock,
-        HeaderValidationMode.DETACHED_ONLY,
-        HeaderValidationMode.DETACHED_ONLY);
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(1);
+  public void validateAndProcessBlock_whenProcessBlockFails() {
+    when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(block)))
+        .thenReturn(BlockProcessingResult.FAILED);
+
+    BlockProcessingResult result =
+        mainnetBlockValidator.validateAndProcessBlock(
+            protocolContext,
+            block,
+            HeaderValidationMode.DETACHED_ONLY,
+            HeaderValidationMode.DETACHED_ONLY);
+
+    final String expectedError = "processing failed";
+    assertValidationFailed(result, expectedError);
+    assertBadBlockIsTracked(block);
   }
 
   @Test
-  public void shouldNotCacheWhenValidBlocks() {
+  public void validateAndProcessBlock_withShouldRecordBadBlockFalse() {
+    when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(block)))
+        .thenReturn(BlockProcessingResult.FAILED);
 
-    MutableWorldState mockWorldState = mock(MutableWorldState.class);
+    BlockProcessingResult result =
+        mainnetBlockValidator.validateAndProcessBlock(
+            protocolContext,
+            block,
+            HeaderValidationMode.DETACHED_ONLY,
+            HeaderValidationMode.DETACHED_ONLY,
+            false,
+            false);
 
-    final BlockHeader blockHeader = new BlockHeaderTestFixture().buildHeader();
-    when(blockchain.getBlockHeader(any(Hash.class))).thenReturn(Optional.of(blockHeader));
-    when(blockHeaderValidator.validateHeader(
-            any(BlockHeader.class),
-            any(BlockHeader.class),
-            eq(protocolContext),
-            eq(HeaderValidationMode.DETACHED_ONLY)))
-        .thenReturn(true);
-    when(worldStateArchive.getMutable(eq(blockHeader), anyBoolean()))
-        .thenReturn(Optional.of(mockWorldState));
-    when(worldStateArchive.getMutable(any(Hash.class), any(Hash.class)))
-        .thenReturn(Optional.of(mockWorldState));
-    when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(badBlock)))
-        .thenReturn(new BlockProcessingResult(Optional.empty()));
-    when(blockBodyValidator.validateBody(
-            eq(protocolContext),
-            eq(badBlock),
-            any(),
-            any(),
-            eq(HeaderValidationMode.DETACHED_ONLY)))
-        .thenReturn(true);
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(0);
-    mainnetBlockValidator.validateAndProcessBlock(
-        protocolContext,
-        badBlock,
-        HeaderValidationMode.DETACHED_ONLY,
-        HeaderValidationMode.DETACHED_ONLY);
-    assertThat(badBlockManager.getBadBlocks()).isEmpty();
+    assertThat(result.isFailed()).isTrue();
+    assertNoBadBlocks();
   }
 
   @Test
-  public void shouldReturnBadBlockBasedOnTheHash() {
-    when(blockchain.getBlockHeader(any(Hash.class)))
-        .thenReturn(Optional.of(new BlockHeaderTestFixture().buildHeader()));
-    when(blockHeaderValidator.validateHeader(
-            any(BlockHeader.class),
-            any(BlockHeader.class),
-            eq(protocolContext),
-            eq(HeaderValidationMode.DETACHED_ONLY)))
-        .thenReturn(false);
-    mainnetBlockValidator.validateAndProcessBlock(
-        protocolContext,
-        badBlock,
-        HeaderValidationMode.DETACHED_ONLY,
-        HeaderValidationMode.DETACHED_ONLY);
-    assertThat(badBlockManager.getBadBlock(badBlock.getHash())).containsSame(badBlock);
+  public void validateAndProcessBlock_withShouldRecordBadBlockTrue() {
+    when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(block)))
+        .thenReturn(BlockProcessingResult.FAILED);
+
+    BlockProcessingResult result =
+        mainnetBlockValidator.validateAndProcessBlock(
+            protocolContext,
+            block,
+            HeaderValidationMode.DETACHED_ONLY,
+            HeaderValidationMode.DETACHED_ONLY,
+            false,
+            true);
+
+    assertThat(result.isFailed()).isTrue();
+    assertBadBlockIsTracked(block);
   }
 
   @Test
-  public void when_shouldRecordBadBlockIsFalse_Expect_BlockNotAddedToBadBlockManager() {
-    when(blockchain.getBlockHeader(any(Hash.class)))
-        .thenReturn(Optional.of(new BlockHeaderTestFixture().buildHeader()));
-    mainnetBlockValidator.validateAndProcessBlock(
-        protocolContext,
-        badBlock,
-        HeaderValidationMode.DETACHED_ONLY,
-        HeaderValidationMode.DETACHED_ONLY,
-        false,
-        false);
-
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(0);
+  public void validateAndProcessBlock_withShouldRecordBadBlockNotSet() {
+    when(blockProcessor.processBlock(eq(blockchain), any(MutableWorldState.class), eq(block)))
+        .thenReturn(BlockProcessingResult.FAILED);
+
+    BlockProcessingResult result =
+        mainnetBlockValidator.validateAndProcessBlock(
+            protocolContext,
+            block,
+            HeaderValidationMode.DETACHED_ONLY,
+            HeaderValidationMode.DETACHED_ONLY,
+            false);
+
+    assertThat(result.isFailed()).isTrue();
+    assertBadBlockIsTracked(block);
   }
 
   @Test
-  public void when_shouldRecordBadBlockIsTrue_Expect_BlockAddedToBadBlockManager() {
-    when(blockchain.getBlockHeader(any(Hash.class)))
-        .thenReturn(Optional.of(new BlockHeaderTestFixture().buildHeader()));
-    mainnetBlockValidator.validateAndProcessBlock(
-        protocolContext,
-        badBlock,
-        HeaderValidationMode.DETACHED_ONLY,
-        HeaderValidationMode.DETACHED_ONLY,
-        false,
-        true);
-
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(1);
+  public void fastBlockValidation_onSuccess() {
+    final boolean isValid =
+        mainnetBlockValidator.fastBlockValidation(
+            protocolContext,
+            block,
+            Collections.emptyList(),
+            HeaderValidationMode.FULL,
+            HeaderValidationMode.FULL);
+
+    assertThat(isValid).isTrue();
+    assertNoBadBlocks();
   }
 
   @Test
-  public void when_shouldRecordBadBlockIsNotSet_Expect_BlockAddedToBadBlockManager() {
-    when(blockchain.getBlockHeader(any(Hash.class)))
-        .thenReturn(Optional.of(new BlockHeaderTestFixture().buildHeader()));
-    mainnetBlockValidator.validateAndProcessBlock(
-        protocolContext,
-        badBlock,
-        HeaderValidationMode.DETACHED_ONLY,
-        HeaderValidationMode.DETACHED_ONLY,
-        false);
-
-    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(1);
+  public void fastBlockValidation_onFailedHeaderValidation() {
+    final HeaderValidationMode validationMode = HeaderValidationMode.FULL;
+    when(blockHeaderValidator.validateHeader(
+            any(BlockHeader.class), eq(protocolContext), eq(validationMode)))
+        .thenReturn(false);
+
+    final boolean isValid =
+        mainnetBlockValidator.fastBlockValidation(
+            protocolContext, block, Collections.emptyList(), validationMode, validationMode);
+
+    assertThat(isValid).isFalse();
+    assertBadBlockIsTracked(block);
+  }
+
+  @Test
+  public void fastBlockValidation_onFailedBodyValidation() {
+    final HeaderValidationMode validationMode = HeaderValidationMode.FULL;
+    when(blockBodyValidator.validateBodyLight(
+            eq(protocolContext), eq(block), any(), eq(validationMode)))
+        .thenReturn(false);
+
+    final boolean isValid =
+        mainnetBlockValidator.fastBlockValidation(
+            protocolContext, block, Collections.emptyList(), validationMode, validationMode);
+
+    assertThat(isValid).isFalse();
+    assertBadBlockIsTracked(block);
+  }
+
+  private void assertNoBadBlocks() {
+    assertThat(badBlockManager.getBadBlocks()).isEmpty();
+  }
+
+  private void assertBadBlockIsTracked(final Block badBlock) {
+    assertThat(badBlockManager.getBadBlocks()).containsExactly(badBlock);
+    assertThat(badBlockManager.getBadBlock(badBlock.getHash())).contains(block);
+  }
+
+  private void assertValidationFailed(
+      final BlockProcessingResult result, final String expectedError) {
+    assertThat(result.isFailed()).isTrue();
+    assertThat(result.errorMessage).isPresent();
+    assertThat(result.errorMessage.get()).containsIgnoringWhitespaces(expectedError);
   }
 }

ethereum/core/src/test/java/org/hyperledger/besu/ethereum/chain/BadBlockManagerTest.java

@@ -0,0 +1,101 @@
+/*
+ * Copyright Hyperledger Besu Contributors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ */
+package org.hyperledger.besu.ethereum.chain;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.hyperledger.besu.ethereum.core.Block;
+import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil;
+import org.hyperledger.besu.ethereum.trie.MerkleTrieException;
+import org.hyperledger.besu.plugin.services.exception.StorageException;
+
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+public class BadBlockManagerTest {
+
+  final BlockchainSetupUtil chainUtil = BlockchainSetupUtil.forMainnet();
+  final Block block = chainUtil.getBlock(1);
+  final BadBlockManager badBlockManager = new BadBlockManager();
+
+  public static Stream<Arguments> getInternalExceptions() {
+    return Stream.of(
+        Arguments.of("StorageException", new StorageException("oops")),
+        Arguments.of("MerkleTrieException", new MerkleTrieException("fail")));
+  }
+
+  @Test
+  public void addBadBlock_addsBlock() {
+    BadBlockManager badBlockManager = new BadBlockManager();
+    final BadBlockCause cause = BadBlockCause.fromValidationFailure("failed");
+    badBlockManager.addBadBlock(block, cause);
+
+    assertThat(badBlockManager.getBadBlocks()).containsExactly(block);
+  }
+
+  @ParameterizedTest(name = "[{index}] {0}")
+  @MethodSource("getInternalExceptions")
+  public void addBadBlock_ignoresInternalError(final String caseName, final Exception err) {
+    BadBlockManager badBlockManager = new BadBlockManager();
+    final BadBlockCause cause = BadBlockCause.fromProcessingError(err);
+    badBlockManager.addBadBlock(block, cause);
+
+    assertThat(badBlockManager.getBadBlocks()).isEmpty();
+  }
+
+  @Test
+  public void addBadBlock_doesNotIgnoreRuntimeException() {
+    final Exception err = new RuntimeException("oops");
+    final BadBlockCause cause = BadBlockCause.fromProcessingError(err);
+    badBlockManager.addBadBlock(block, cause);
+
+    assertThat(badBlockManager.getBadBlocks()).containsExactly(block);
+  }
+
+  @Test
+  public void reset_clearsBadBlocks() {
+    BadBlockManager badBlockManager = new BadBlockManager();
+    final BadBlockCause cause = BadBlockCause.fromValidationFailure("");
+    badBlockManager.addBadBlock(block, cause);
+    assertThat(badBlockManager.getBadBlocks()).containsExactly(block);
+
+    badBlockManager.reset();
+
+    assertThat(badBlockManager.getBadBlocks()).isEmpty();
+  }
+
+  @Test
+  public void isBadBlock_falseWhenEmpty() {
+    assertThat(badBlockManager.isBadBlock(block.getHash())).isFalse();
+  }
+
+  @Test
+  public void isBadBlock_trueForBadHeader() {
+    badBlockManager.addBadHeader(block.getHeader(), BadBlockCause.fromValidationFailure("failed"));
+    assertThat(badBlockManager.isBadBlock(block.getHash())).isTrue();
+  }
+
+  @Test
+  public void isBadBlock_trueForBadBlock() {
+    badBlockManager.addBadBlock(block, BadBlockCause.fromValidationFailure("failed"));
+
+    assertThat(badBlockManager.isBadBlock(block.getHash())).isTrue();
+  }
+}

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/BlockPropagationManager.java

@@ -20,6 +20,7 @@ import org.hyperledger.besu.consensus.merge.ForkchoiceEvent;
 import org.hyperledger.besu.consensus.merge.UnverifiedForkchoiceListener;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.ProtocolContext;
+import org.hyperledger.besu.ethereum.chain.BadBlockCause;
 import org.hyperledger.besu.ethereum.chain.BadBlockManager;
 import org.hyperledger.besu.ethereum.chain.BlockAddedEvent;
 import org.hyperledger.besu.ethereum.chain.BlockAddedEvent.EventType;
@@ -665,13 +666,15 @@ public class BlockPropagationManager implements UnverifiedForkchoiceListener {
       final Block block,
       final BlockHeader parent,
       final BadBlockManager badBlockManager) {
+    final HeaderValidationMode validationMode = HeaderValidationMode.FULL;
     if (blockHeaderValidator.validateHeader(
-        block.getHeader(), parent, protocolContext, HeaderValidationMode.FULL)) {
+        block.getHeader(), parent, protocolContext, validationMode)) {
       ethContext.getScheduler().scheduleSyncWorkerTask(() -> broadcastBlock(block, parent));
       return runImportTask(block);
     } else {
       processingBlocksManager.registerBlockImportDone(block.getHash());
-      badBlockManager.addBadBlock(block, Optional.empty());
+      final String description = String.format("Failed header validation (%s)", validationMode);
+      badBlockManager.addBadBlock(block, BadBlockCause.fromValidationFailure(description));
       LOG.warn(
           "Added to bad block manager for invalid header, failed to import announced block {}",
           block.toLogString());

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/ImportBlocksStep.java

@@ -66,10 +66,7 @@ public class ImportBlocksStep implements Consumer<List<BlockWithReceipts>> {
     final long startTime = System.nanoTime();
     for (final BlockWithReceipts blockWithReceipts : blocksWithReceipts) {
       if (!importBlock(blockWithReceipts)) {
-        throw new InvalidBlockException(
-            "Failed to import block",
-            blockWithReceipts.getHeader().getNumber(),
-            blockWithReceipts.getHash());
+        throw InvalidBlockException.fromInvalidBlock(blockWithReceipts.getHeader());
       }
       LOG.atTrace()
           .setMessage("Imported block {}")

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fullsync/FullImportBlockStep.java

@@ -62,7 +62,7 @@ public class FullImportBlockStep implements Consumer<Block> {
     final BlockImportResult blockImportResult =
         importer.importBlock(protocolContext, block, HeaderValidationMode.SKIP_DETACHED);
     if (!blockImportResult.isImported()) {
-      throw new InvalidBlockException("Failed to import block", blockNumber, block.getHash());
+      throw InvalidBlockException.fromInvalidBlock(block.getHeader());
     }
     gasAccumulator += block.getHeader().getGasUsed();
     int peerCount = -1; // ethContext is not available in tests

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/range/RangeHeadersValidationStep.java

@@ -67,8 +67,7 @@ public class RangeHeadersValidationStep implements Function<RangeHeaders, Stream
                         rangeEndDescription,
                         firstHeader.getNumber(),
                         firstHeader.getHash());
-                throw new InvalidBlockException(
-                    errorMessage, firstHeader.getNumber(), firstHeader.getHash());
+                throw InvalidBlockException.fromInvalidBlock(errorMessage, firstHeader);
               }
             })
         .orElse(Stream.empty());

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTask.java

@@ -19,20 +19,21 @@ import static java.util.Arrays.asList;
 
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.ProtocolContext;
+import org.hyperledger.besu.ethereum.chain.BadBlockCause;
 import org.hyperledger.besu.ethereum.chain.BadBlockManager;
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.eth.manager.EthContext;
 import org.hyperledger.besu.ethereum.eth.manager.EthPeer;
 import org.hyperledger.besu.ethereum.eth.manager.task.AbstractGetHeadersFromPeerTask;
-import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask;
 import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult;
 import org.hyperledger.besu.ethereum.eth.manager.task.AbstractRetryingPeerTask;
-import org.hyperledger.besu.ethereum.eth.manager.task.GetBlockFromPeerTask;
+import org.hyperledger.besu.ethereum.eth.manager.task.GetBodiesFromPeerTask;
 import org.hyperledger.besu.ethereum.eth.manager.task.GetHeadersFromPeerByHashTask;
 import org.hyperledger.besu.ethereum.eth.sync.ValidationPolicy;
 import org.hyperledger.besu.ethereum.eth.sync.tasks.exceptions.InvalidBlockException;
 import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator;
+import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
 import org.hyperledger.besu.ethereum.p2p.rlpx.wire.messages.DisconnectMessage.DisconnectReason;
@@ -44,6 +45,7 @@ import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.primitives.Ints;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -177,7 +179,8 @@ public class DownloadHeaderSequenceTask extends AbstractRetryingPeerTask<List<Bl
         });
   }
 
-  private CompletableFuture<List<BlockHeader>> processHeaders(
+  @VisibleForTesting
+  CompletableFuture<List<BlockHeader>> processHeaders(
       final PeerTaskResult<List<BlockHeader>> headersResult) {
     return executeWorkerSubTask(
         ethContext.getScheduler(),
@@ -199,41 +202,38 @@ public class DownloadHeaderSequenceTask extends AbstractRetryingPeerTask<List<Bl
               child =
                   (headerIndex == segmentLength - 1) ? referenceHeader : headers[headerIndex + 1];
             }
-            final BadBlockManager badBlockManager = protocolContext.getBadBlockManager();
 
-            if (!validateHeader(child, header)) {
-              // Invalid headers - disconnect from peer
-
-              final BlockHeader invalidBlock = child;
-              // even though the header is known bad we are downloading the block body for the
-              // debug_badBlocks RPC
-              final AbstractPeerTask<Block> getBlockTask =
-                  GetBlockFromPeerTask.create(
-                          protocolSchedule,
-                          ethContext,
-                          Optional.of(child.getHash()),
-                          child.getNumber(),
-                          metricsSystem)
-                      .assignPeer(headersResult.getPeer());
-
-              getBlockTask
-                  .run()
-                  .whenComplete(
-                      (blockPeerTaskResult, error) -> {
-                        if (error == null && blockPeerTaskResult.getResult() != null) {
-                          badBlockManager.addBadBlock(
-                              blockPeerTaskResult.getResult(), Optional.ofNullable(error));
-                        }
-                        LOG.debug(
-                            "Received invalid headers from peer (BREACH_OF_PROTOCOL), disconnecting from: {}",
-                            headersResult.getPeer());
-                        headersResult.getPeer().disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
-                        future.completeExceptionally(
-                            new InvalidBlockException(
-                                "Header failed validation.",
-                                invalidBlock.getNumber(),
-                                invalidBlock.getHash()));
-                      });
+            final boolean foundChild = child != null;
+            final boolean headerInRange = checkHeaderInRange(header);
+            final boolean headerInvalid = foundChild && !validateHeader(child, header);
+            if (!headerInRange || !foundChild || headerInvalid) {
+              final BlockHeader invalidHeader = child;
+              final CompletableFuture<?> badBlockHandled =
+                  headerInvalid
+                      ? markBadBlock(invalidHeader, headersResult.getPeer())
+                      : CompletableFuture.completedFuture(null);
+              badBlockHandled.whenComplete(
+                  (res, err) -> {
+                    LOG.debug(
+                        "Received invalid headers from peer (BREACH_OF_PROTOCOL), disconnecting from: {}",
+                        headersResult.getPeer());
+                    headersResult.getPeer().disconnect(DisconnectReason.BREACH_OF_PROTOCOL);
+                    final InvalidBlockException exception;
+                    if (invalidHeader == null) {
+                      final String msg =
+                          String.format(
+                              "Received misordered blocks. Missing child of %s",
+                              header.toLogString());
+                      exception = InvalidBlockException.create(msg);
+                    } else {
+                      final String errorMsg =
+                          headerInvalid
+                              ? "Header failed validation"
+                              : "Out-of-range header received from peer";
+                      exception = InvalidBlockException.fromInvalidBlock(errorMsg, invalidHeader);
+                    }
+                    future.completeExceptionally(exception);
+                  });
 
               return future;
             }
@@ -246,20 +246,41 @@ public class DownloadHeaderSequenceTask extends AbstractRetryingPeerTask<List<Bl
         });
   }
 
-  private boolean validateHeader(final BlockHeader child, final BlockHeader header) {
+  private CompletableFuture<?> markBadBlock(final BlockHeader badHeader, final EthPeer badPeer) {
+    // even though the header is known bad we are downloading the block body for the debug_badBlocks
+    // RPC
+    final BadBlockManager badBlockManager = protocolContext.getBadBlockManager();
+    return GetBodiesFromPeerTask.forHeaders(
+            protocolSchedule, ethContext, List.of(badHeader), metricsSystem)
+        .assignPeer(badPeer)
+        .run()
+        .whenComplete(
+            (blockPeerTaskResult, error) -> {
+              final HeaderValidationMode validationMode =
+                  validationPolicy.getValidationModeForNextBlock();
+              final String description =
+                  String.format("Failed header validation (%s)", validationMode);
+              final BadBlockCause cause = BadBlockCause.fromValidationFailure(description);
+              if (blockPeerTaskResult != null) {
+                final Optional<Block> block = blockPeerTaskResult.getResult().stream().findFirst();
+                block.ifPresentOrElse(
+                    (b) -> badBlockManager.addBadBlock(b, cause),
+                    () -> badBlockManager.addBadHeader(badHeader, cause));
+              } else {
+                badBlockManager.addBadHeader(badHeader, cause);
+              }
+            });
+  }
+
+  private boolean checkHeaderInRange(final BlockHeader header) {
     final long finalBlockNumber = startingBlockNumber + segmentLength;
-    final boolean blockInRange =
-        header.getNumber() >= startingBlockNumber && header.getNumber() < finalBlockNumber;
-    if (!blockInRange) {
-      return false;
-    }
-    if (child == null) {
-      return false;
-    }
+    return header.getNumber() >= startingBlockNumber && header.getNumber() < finalBlockNumber;
+  }
 
-    final ProtocolSpec protocolSpec = protocolSchedule.getByBlockHeader(child);
+  private boolean validateHeader(final BlockHeader header, final BlockHeader parent) {
+    final ProtocolSpec protocolSpec = protocolSchedule.getByBlockHeader(header);
     final BlockHeaderValidator blockHeaderValidator = protocolSpec.getBlockHeaderValidator();
     return blockHeaderValidator.validateHeader(
-        child, header, protocolContext, validationPolicy.getValidationModeForNextBlock());
+        header, parent, protocolContext, validationPolicy.getValidationModeForNextBlock());
   }
 }

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/PersistBlockTask.java

@@ -202,9 +202,7 @@ public class PersistBlockTask extends AbstractEthTask<Block> {
           .log();
       blockImportResult = blockImporter.importBlock(protocolContext, block, validateHeaders);
       if (!blockImportResult.isImported()) {
-        result.completeExceptionally(
-            new InvalidBlockException(
-                "Failed to import block", block.getHeader().getNumber(), block.getHash()));
+        result.completeExceptionally(InvalidBlockException.fromInvalidBlock(block.getHeader()));
         return;
       }
       result.complete(block);

ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/tasks/exceptions/InvalidBlockException.java

@@ -14,11 +14,25 @@
  */
 package org.hyperledger.besu.ethereum.eth.sync.tasks.exceptions;
 
-import org.hyperledger.besu.datatypes.Hash;
+import org.hyperledger.besu.ethereum.core.BlockHeader;
 
 public class InvalidBlockException extends RuntimeException {
 
-  public InvalidBlockException(final String message, final long blockNumber, final Hash blockHash) {
-    super(message + ": Invalid block at #" + blockNumber + " (" + blockHash + ")");
+  private InvalidBlockException(final String message) {
+    super(message);
+  }
+
+  public static InvalidBlockException create(final String message) {
+    return new InvalidBlockException(message);
+  }
+
+  public static InvalidBlockException fromInvalidBlock(final BlockHeader header) {
+    return fromInvalidBlock("Failed to import block", header);
+  }
+
+  public static InvalidBlockException fromInvalidBlock(
+      final String description, final BlockHeader header) {
+    final String message = description + ": Invalid block " + header.toLogString();
+    return new InvalidBlockException(message);
   }
 }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ethtaskutils/AbstractMessageTaskTest.java

@@ -23,6 +23,7 @@ import org.hyperledger.besu.crypto.SECPPublicKey;
 import org.hyperledger.besu.datatypes.Address;
 import org.hyperledger.besu.datatypes.Hash;
 import org.hyperledger.besu.ethereum.ProtocolContext;
+import org.hyperledger.besu.ethereum.chain.BadBlockManager;
 import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.core.BlockchainSetupUtil;
 import org.hyperledger.besu.ethereum.core.MiningParameters;
@@ -105,6 +106,7 @@ public abstract class AbstractMessageTaskTest<T, R> {
 
   @BeforeEach
   public void setupTest() {
+    protocolContext.getBadBlockManager().reset();
     peersDoTimeout = new AtomicBoolean(false);
     peerCountToTimeout = new AtomicInteger(0);
     ethPeers =
@@ -163,9 +165,7 @@ public abstract class AbstractMessageTaskTest<T, R> {
   @Test
   public void completesWhenPeersAreResponsive() {
     // Setup a responsive peer
-    final RespondingEthPeer.Responder responder =
-        RespondingEthPeer.blockchainResponder(
-            blockchain, protocolContext.getWorldStateArchive(), transactionPool);
+    final RespondingEthPeer.Responder responder = getFullResponder();
     final RespondingEthPeer respondingPeer =
         EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 32);
 
@@ -186,6 +186,7 @@ public abstract class AbstractMessageTaskTest<T, R> {
 
     assertThat(done).isTrue();
     assertResultMatchesExpectation(requestedData, actualResult.get(), respondingPeer.getEthPeer());
+    assertNoBadBlocks();
   }
 
   @Test
@@ -225,4 +226,15 @@ public abstract class AbstractMessageTaskTest<T, R> {
     assertThat(future.isCancelled()).isTrue();
     assertThat(task.run().isCancelled()).isTrue();
   }
+
+  protected RespondingEthPeer.Responder getFullResponder() {
+    return RespondingEthPeer.blockchainResponder(
+        blockchain, protocolContext.getWorldStateArchive(), transactionPool);
+  }
+
+  protected void assertNoBadBlocks() {
+    BadBlockManager badBlockManager = protocolContext.getBadBlockManager();
+    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(0);
+    assertThat(badBlockManager.getBadHeaders().size()).isEqualTo(0);
+  }
 }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/PipelineChainDownloaderTest.java

@@ -318,7 +318,7 @@ public class PipelineChainDownloaderTest {
     if (isCancelled) {
       chainDownloader.cancel();
     }
-    selectTargetFuture.completeExceptionally(new InvalidBlockException("", 1, null));
+    selectTargetFuture.completeExceptionally(InvalidBlockException.create("Failed"));
 
     verify(syncState, times(1)).disconnectSyncTarget(DisconnectReason.BREACH_OF_PROTOCOL);
   }

ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/sync/tasks/DownloadHeaderSequenceTaskTest.java

@@ -16,21 +16,35 @@ package org.hyperledger.besu.ethereum.eth.sync.tasks;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
 
+import org.hyperledger.besu.ethereum.chain.BadBlockManager;
+import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.eth.manager.EthProtocolManagerTestUtil;
 import org.hyperledger.besu.ethereum.eth.manager.RespondingEthPeer;
 import org.hyperledger.besu.ethereum.eth.manager.ethtaskutils.RetryingMessageTaskTest;
 import org.hyperledger.besu.ethereum.eth.manager.exceptions.MaxRetriesReachedException;
+import org.hyperledger.besu.ethereum.eth.manager.task.AbstractPeerTask.PeerTaskResult;
 import org.hyperledger.besu.ethereum.eth.manager.task.EthTask;
 import org.hyperledger.besu.ethereum.eth.messages.BlockHeadersMessage;
 import org.hyperledger.besu.ethereum.eth.messages.EthPV62;
 import org.hyperledger.besu.ethereum.eth.sync.ValidationPolicy;
+import org.hyperledger.besu.ethereum.eth.sync.tasks.exceptions.InvalidBlockException;
+import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator;
 import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode;
+import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
+import org.hyperledger.besu.ethereum.mainnet.ProtocolSpec;
 import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
@@ -96,6 +110,7 @@ public class DownloadHeaderSequenceTaskTest extends RetryingMessageTaskTest<List
     assertThat(future.isDone()).isTrue();
     assertThat(future.isCompletedExceptionally()).isTrue();
     assertThatThrownBy(future::get).hasCauseInstanceOf(MaxRetriesReachedException.class);
+    assertNoBadBlocks();
   }
 
   @Test
@@ -118,8 +133,7 @@ public class DownloadHeaderSequenceTaskTest extends RetryingMessageTaskTest<List
     final CompletableFuture<List<BlockHeader>> future = task.run();
 
     // Filter response to include only reference header and previous header
-    final RespondingEthPeer.Responder fullResponder =
-        RespondingEthPeer.blockchainResponder(blockchain);
+    final RespondingEthPeer.Responder fullResponder = getFullResponder();
     final RespondingEthPeer.Responder responder =
         (cap, message) -> {
           final Optional<MessageData> fullResponse = fullResponder.respond(cap, message);
@@ -140,5 +154,262 @@ public class DownloadHeaderSequenceTaskTest extends RetryingMessageTaskTest<List
     assertThat(future.isDone()).isTrue();
     assertThat(future.isCompletedExceptionally()).isTrue();
     assertThatThrownBy(future::get).hasCauseInstanceOf(MaxRetriesReachedException.class);
+    assertNoBadBlocks();
+  }
+
+  @Test
+  public void marksBadBlockWhenHeaderValidationFails() {
+    final RespondingEthPeer respondingPeer =
+        EthProtocolManagerTestUtil.createPeer(ethProtocolManager);
+    // Set up a chain with an invalid block
+    final int blockCount = 5;
+    final long startBlock = blockchain.getChainHeadBlockNumber() - blockCount;
+    final List<Block> chain = getBlockSequence(startBlock, blockCount);
+    final Block badBlock = chain.get(2);
+    ProtocolSchedule protocolScheduleSpy = setupHeaderValidationToFail(badBlock.getHeader());
+
+    // Execute the task
+    final BlockHeader referenceHeader = chain.get(blockCount - 1).getHeader();
+    final EthTask<List<BlockHeader>> task =
+        DownloadHeaderSequenceTask.endingAtHeader(
+            protocolScheduleSpy,
+            protocolContext,
+            ethContext,
+            referenceHeader,
+            blockCount - 1, // The reference header is not included in this count
+            maxRetries,
+            validationPolicy,
+            metricsSystem);
+    final CompletableFuture<List<BlockHeader>> future = task.run();
+    final RespondingEthPeer.Responder fullResponder = getFullResponder();
+    respondingPeer.respondWhile(fullResponder, () -> !future.isDone());
+
+    // Check that the future completed exceptionally
+    assertThat(future.isCompletedExceptionally()).isTrue();
+    assertThatThrownBy(future::get)
+        .hasCauseInstanceOf(InvalidBlockException.class)
+        .hasMessageContaining("Header failed validation");
+
+    // Check bad blocks
+    assertBadBlock(badBlock);
+  }
+
+  @Test
+  public void processHeaders_markBadBlockWhenHeaderValidationFails() {
+    final RespondingEthPeer respondingPeer =
+        EthProtocolManagerTestUtil.createPeer(ethProtocolManager);
+    // Set up a chain with an invalid block
+    final int blockCount = 5;
+    final long startBlock = blockchain.getChainHeadBlockNumber() - blockCount;
+    final List<Block> chain = getBlockSequence(startBlock, blockCount);
+    final Block badBlock = chain.get(2);
+    ProtocolSchedule protocolScheduleSpy = setupHeaderValidationToFail(badBlock.getHeader());
+
+    // Execute the task
+    final BlockHeader referenceHeader = chain.get(blockCount - 1).getHeader();
+    final DownloadHeaderSequenceTask task =
+        DownloadHeaderSequenceTask.endingAtHeader(
+            protocolScheduleSpy,
+            protocolContext,
+            ethContext,
+            referenceHeader,
+            blockCount - 1, // The reference header is not included in this count
+            maxRetries,
+            validationPolicy,
+            metricsSystem);
+
+    // Run
+    final List<BlockHeader> responseHeaders =
+        chain.stream()
+            .map(Block::getHeader)
+            .sorted(Comparator.comparing(BlockHeader::getNumber).reversed())
+            .toList();
+    PeerTaskResult<List<BlockHeader>> peerResponse =
+        new PeerTaskResult<>(respondingPeer.getEthPeer(), responseHeaders);
+    final CompletableFuture<List<BlockHeader>> future = task.processHeaders(peerResponse);
+    respondingPeer.respondWhile(this.getFullResponder(), () -> !future.isDone());
+
+    // Check that the future completed exceptionally
+    assertThat(future.isCompletedExceptionally()).isTrue();
+    assertThatThrownBy(future::get)
+        .hasCauseInstanceOf(InvalidBlockException.class)
+        .hasMessageContaining("Header failed validation");
+
+    // Check bad blocks
+    assertBadBlock(badBlock);
+  }
+
+  @Test
+  public void processHeaders_markBadBlockHashWhenHeaderValidationFailsAndBodyUnavailable() {
+    final RespondingEthPeer respondingPeer =
+        EthProtocolManagerTestUtil.createPeer(ethProtocolManager);
+    // Set up a chain with an invalid block
+    final int blockCount = 5;
+    final long startBlock = blockchain.getChainHeadBlockNumber() - blockCount;
+    final List<Block> chain = getBlockSequence(startBlock, blockCount);
+    final Block badBlock = chain.get(2);
+    ProtocolSchedule protocolScheduleSpy = setupHeaderValidationToFail(badBlock.getHeader());
+
+    // Set up the task
+    final BlockHeader referenceHeader = chain.get(blockCount - 1).getHeader();
+    final DownloadHeaderSequenceTask task =
+        DownloadHeaderSequenceTask.endingAtHeader(
+            protocolScheduleSpy,
+            protocolContext,
+            ethContext,
+            referenceHeader,
+            blockCount - 1, // The reference header is not included in this count
+            maxRetries,
+            validationPolicy,
+            metricsSystem);
+
+    // Run
+    final List<BlockHeader> responseHeaders =
+        chain.stream()
+            .map(Block::getHeader)
+            .sorted(Comparator.comparing(BlockHeader::getNumber).reversed())
+            .toList();
+    PeerTaskResult<List<BlockHeader>> peerResponse =
+        new PeerTaskResult<>(respondingPeer.getEthPeer(), responseHeaders);
+    final CompletableFuture<List<BlockHeader>> future = task.processHeaders(peerResponse);
+    // Use empty responder so block body cannot be retrieved
+    respondingPeer.respondWhile(RespondingEthPeer.emptyResponder(), () -> !future.isDone());
+
+    // Check that the future completed exceptionally
+    assertThat(future.isCompletedExceptionally()).isTrue();
+    assertThatThrownBy(future::get)
+        .hasCauseInstanceOf(InvalidBlockException.class)
+        .hasMessageContaining("Header failed validation");
+
+    // Check bad blocks
+    assertBadHeader(badBlock.getHeader());
+  }
+
+  @Test
+  public void processHeaders_doesNotMarkBadBlockForOutOfRangeResponse() {
+    final RespondingEthPeer respondingPeer =
+        EthProtocolManagerTestUtil.createPeer(ethProtocolManager);
+
+    // Set up the task
+    final int blockCount = 3;
+    final long startBlock = blockchain.getChainHeadBlockNumber() - blockCount;
+    final List<Block> chain = getBlockSequence(startBlock, blockCount);
+    final int segmentLength = blockCount - 1; // The reference header is not included in this count
+    final BlockHeader referenceHeader = chain.get(blockCount - 1).getHeader();
+    final DownloadHeaderSequenceTask task =
+        DownloadHeaderSequenceTask.endingAtHeader(
+            protocolSchedule,
+            protocolContext,
+            ethContext,
+            referenceHeader,
+            segmentLength,
+            maxRetries,
+            validationPolicy,
+            metricsSystem);
+
+    // Run
+    final Block outOfRangeBlock = blockchain.getBlockByNumber(startBlock - 1).get();
+    final List<BlockHeader> headerResponse =
+        chain.stream()
+            .map(Block::getHeader)
+            .sorted(Comparator.comparing(BlockHeader::getNumber).reversed())
+            .collect(Collectors.toCollection(ArrayList::new));
+    headerResponse.add(outOfRangeBlock.getHeader());
+
+    PeerTaskResult<List<BlockHeader>> peerResponse =
+        new PeerTaskResult<>(respondingPeer.getEthPeer(), headerResponse);
+    final CompletableFuture<List<BlockHeader>> future = task.processHeaders(peerResponse);
+
+    // Check that the future completed exceptionally
+    assertThat(future.isCompletedExceptionally()).isTrue();
+    assertThatThrownBy(future::get)
+        .hasCauseInstanceOf(InvalidBlockException.class)
+        .hasMessageNotContaining("Header failed validation")
+        .hasMessageContaining("Out-of-range");
+
+    // Check bad blocks
+    assertNoBadBlocks();
+  }
+
+  @Test
+  public void processHeaders_doesNotMarkBadBlockForMisorderedBlocks() {
+    final RespondingEthPeer respondingPeer =
+        EthProtocolManagerTestUtil.createPeer(ethProtocolManager);
+
+    // Set up the task
+    final int blockCount = 5;
+    final long startBlock = blockchain.getChainHeadBlockNumber() - blockCount;
+    final List<Block> chain = getBlockSequence(startBlock, blockCount);
+    final int segmentLength = blockCount - 1; // The reference header is not included in this count
+    final BlockHeader referenceHeader = blockchain.getChainHeadHeader();
+    final DownloadHeaderSequenceTask task =
+        DownloadHeaderSequenceTask.endingAtHeader(
+            protocolSchedule,
+            protocolContext,
+            ethContext,
+            referenceHeader,
+            segmentLength,
+            maxRetries,
+            validationPolicy,
+            metricsSystem);
+
+    // Return blocks in ascending order
+    final List<BlockHeader> responseHeaders = chain.stream().map(Block::getHeader).toList();
+    PeerTaskResult<List<BlockHeader>> peerResponse =
+        new PeerTaskResult<>(respondingPeer.getEthPeer(), responseHeaders);
+    final CompletableFuture<List<BlockHeader>> future = task.processHeaders(peerResponse);
+
+    // Check that the future completed exceptionally
+    assertThat(future.isCompletedExceptionally()).isTrue();
+    assertThatThrownBy(future::get)
+        .hasCauseInstanceOf(InvalidBlockException.class)
+        .hasMessageNotContaining("Header failed validation")
+        .hasMessageContaining("misordered blocks");
+
+    // Check bad blocks
+    assertNoBadBlocks();
+  }
+
+  private List<Block> getBlockSequence(final long firstBlockNumber, final int blockCount) {
+    final List<Block> blocks = new ArrayList<>(blockCount);
+    for (int i = 0; i < blockCount; i++) {
+      final Block block = blockchain.getBlockByNumber(firstBlockNumber + i).get();
+      blocks.add(block);
+    }
+
+    return blocks;
+  }
+
+  private ProtocolSchedule setupHeaderValidationToFail(final BlockHeader badHeader) {
+    ProtocolSchedule protocolScheduleSpy = spy(protocolSchedule);
+    ProtocolSpec failingProtocolSpec = spy(protocolSchedule.getByBlockHeader(badHeader));
+    BlockHeaderValidator failingValidator = mock(BlockHeaderValidator.class);
+    when(failingValidator.validateHeader(eq(badHeader), any(), any(), any())).thenReturn(false);
+    when(failingProtocolSpec.getBlockHeaderValidator()).thenReturn(failingValidator);
+    doAnswer(
+            invocation -> {
+              BlockHeader header = invocation.getArgument(0);
+              if (header.getNumber() == badHeader.getNumber()) {
+                return failingProtocolSpec;
+              } else {
+                return invocation.callRealMethod();
+              }
+            })
+        .when(protocolScheduleSpy)
+        .getByBlockHeader(any(BlockHeader.class));
+
+    return protocolScheduleSpy;
+  }
+
+  private void assertBadBlock(final Block badBlock) {
+    BadBlockManager badBlockManager = protocolContext.getBadBlockManager();
+    assertThat(badBlockManager.getBadBlocks()).containsExactly(badBlock);
+    assertThat(badBlockManager.getBadHeaders()).isEmpty();
+  }
+
+  private void assertBadHeader(final BlockHeader badHeader) {
+    BadBlockManager badBlockManager = protocolContext.getBadBlockManager();
+    assertThat(badBlockManager.getBadHeaders()).containsExactly(badHeader);
+    assertThat(badBlockManager.getBadBlocks()).isEmpty();
   }
 }