Stop adding bad proposed block to bad block manager

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

@@ -270,7 +270,7 @@ public PayloadIdentifier preparePayload(
             .createBlock(Optional.of(Collections.emptyList()), prevRandao, timestamp, withdrawals)
             .getBlock();
 
-    BlockProcessingResult result = validateBlock(emptyBlock);
+    BlockProcessingResult result = validateProposedBlock(emptyBlock);
     if (result.isSuccessful()) {
       mergeContext.putPayloadById(
           payloadIdentifier, new BlockWithReceipts(emptyBlock, result.getReceipts()));
@@ -401,7 +401,7 @@ private void evaluateNewBlock(
 
     if (isBlockCreationCancelled(payloadIdentifier)) return;
 
-    final var resultBest = validateBlock(bestBlock);
+    final var resultBest = validateProposedBlock(bestBlock);
     if (resultBest.isSuccessful()) {
 
       if (isBlockCreationCancelled(payloadIdentifier)) return;
@@ -494,6 +494,23 @@ public BlockProcessingResult validateBlock(final Block block) {
     return validationResult;
   }
 
+  @Override
+  public BlockProcessingResult validateProposedBlock(final Block block) {
+    final var validationResult =
+        protocolSchedule
+            .getByBlockHeader(block.getHeader())
+            .getBlockValidator()
+            .validateAndProcessBlock(
+                protocolContext,
+                block,
+                HeaderValidationMode.FULL,
+                HeaderValidationMode.NONE,
+                false,
+                false);
+
+    return validationResult;
+  }
+
   @Override
   public BlockProcessingResult rememberBlock(final Block block) {
     debugLambda(LOG, "Remember block {}", block::toLogString);

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

@@ -71,6 +71,14 @@ default boolean isCompatibleWithEngineApi() {
    */
   BlockProcessingResult validateBlock(final Block block);
 
+  /**
+   * Validate a proposed block.
+   *
+   * @param block the block
+   * @return the block processing result
+   */
+  BlockProcessingResult validateProposedBlock(final Block block);
+
   /**
    * Update fork choice.
    *

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

@@ -162,6 +162,11 @@ public BlockProcessingResult validateBlock(final Block block) {
     return mergeCoordinator.validateBlock(block);
   }
 
+  @Override
+  public BlockProcessingResult validateProposedBlock(final Block block) {
+    return mergeCoordinator.validateProposedBlock(block);
+  }
+
   @Override
   public ForkchoiceResult updateForkChoice(
       final BlockHeader newHead, final Hash finalizedBlockHash, final Hash safeBlockHash) {

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

@@ -317,6 +317,26 @@ public void exceptionDuringBuildingBlockShouldNotBeInvalid()
     verify(willThrow, never()).addBadBlock(any(), any());
   }
 
+  @Test
+  public void shouldNotRecordProposedBadBlockToBadBlockManager()
+      throws ExecutionException, InterruptedException {
+    // set up invalid parent to simulate one of the many conditions that can cause a block
+    // validation to fail
+    final BlockHeader invalidParentHeader = new BlockHeaderTestFixture().buildHeader();
+
+    blockCreationTask.get();
+
+    coordinator.preparePayload(
+        invalidParentHeader,
+        System.currentTimeMillis() / 1000,
+        Bytes32.ZERO,
+        suggestedFeeRecipient,
+        Optional.empty());
+
+    verify(badBlockManager, never()).addBadBlock(any(), any());
+    assertThat(badBlockManager.getBadBlocks().size()).isEqualTo(0);
+  }
+
   @Test
   public void shouldContinueBuildingBlocksUntilFinalizeIsCalled()
       throws InterruptedException, ExecutionException {

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

@@ -35,6 +35,14 @@ BlockProcessingResult validateAndProcessBlock(
       final HeaderValidationMode ommerValidationMode,
       final boolean shouldPersist);
 
+  BlockProcessingResult validateAndProcessBlock(
+      final ProtocolContext context,
+      final Block block,
+      final HeaderValidationMode headerValidationMode,
+      final HeaderValidationMode ommerValidationMode,
+      final boolean shouldPersist,
+      final boolean shouldRecordBadBlock);
+
   boolean fastBlockValidation(
       final ProtocolContext context,
       final Block block,

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

@@ -71,7 +71,8 @@ public BlockProcessingResult validateAndProcessBlock(
       final Block block,
       final HeaderValidationMode headerValidationMode,
       final HeaderValidationMode ommerValidationMode) {
-    return validateAndProcessBlock(context, block, headerValidationMode, ommerValidationMode, true);
+    return validateAndProcessBlock(
+        context, block, headerValidationMode, ommerValidationMode, true, true);
   }
 
   @Override
@@ -81,6 +82,18 @@ public BlockProcessingResult validateAndProcessBlock(
       final HeaderValidationMode headerValidationMode,
       final HeaderValidationMode ommerValidationMode,
       final boolean shouldPersist) {
+    return validateAndProcessBlock(
+        context, block, headerValidationMode, ommerValidationMode, shouldPersist, true);
+  }
+
+  @Override
+  public BlockProcessingResult validateAndProcessBlock(
+      final ProtocolContext context,
+      final Block block,
+      final HeaderValidationMode headerValidationMode,
+      final HeaderValidationMode ommerValidationMode,
+      final boolean shouldPersist,
+      final boolean shouldRecordBadBlock) {
 
     final BlockHeader header = block.getHeader();
     final BlockHeader parentHeader;
@@ -93,20 +106,20 @@ public BlockProcessingResult validateAndProcessBlock(
         var retval =
             new BlockProcessingResult(
                 "Parent block with hash " + header.getParentHash() + " not present");
-        handleAndLogImportFailure(block, retval);
+        handleAndLogImportFailure(block, retval, shouldRecordBadBlock);
         return retval;
       }
       parentHeader = maybeParentHeader.get();
 
       if (!blockHeaderValidator.validateHeader(
           header, parentHeader, context, headerValidationMode)) {
         var retval = new BlockProcessingResult("header validation rule violated, see logs");
-        handleAndLogImportFailure(block, retval);
+        handleAndLogImportFailure(block, retval, shouldRecordBadBlock);
         return retval;
       }
     } catch (StorageException ex) {
       var retval = new BlockProcessingResult(Optional.empty(), ex);
-      handleAndLogImportFailure(block, retval);
+      handleAndLogImportFailure(block, retval, shouldRecordBadBlock);
       return retval;
     }
 
@@ -129,19 +142,19 @@ public BlockProcessingResult validateAndProcessBlock(
                 "Unable to process block because parent world state "
                     + parentHeader.getStateRoot()
                     + " is not available");
-        handleAndLogImportFailure(block, retval);
+        handleAndLogImportFailure(block, retval, shouldRecordBadBlock);
         return retval;
       }
       var result = processBlock(context, worldState, block);
       if (result.isFailed()) {
-        handleAndLogImportFailure(block, result);
+        handleAndLogImportFailure(block, result, shouldRecordBadBlock);
         return result;
       } else {
         List<TransactionReceipt> receipts =
             result.getYield().map(BlockProcessingOutputs::getReceipts).orElse(new ArrayList<>());
         if (!blockBodyValidator.validateBody(
             context, block, receipts, worldState.rootHash(), ommerValidationMode)) {
-          handleAndLogImportFailure(block, result);
+          handleAndLogImportFailure(block, result, shouldRecordBadBlock);
           return new BlockProcessingResult("failed to validate output of imported block");
         }
         if (result instanceof GoQuorumBlockProcessingResult) {
@@ -174,19 +187,23 @@ public BlockProcessingResult validateAndProcessBlock(
               synchronizer -> synchronizer.healWorldState(ex.getMaybeAddress(), ex.getLocation()),
               () ->
                   handleAndLogImportFailure(
-                      block, new BlockProcessingResult(Optional.empty(), ex)));
+                      block,
+                      new BlockProcessingResult(Optional.empty(), ex),
+                      shouldRecordBadBlock));
       return new BlockProcessingResult(Optional.empty(), ex);
     } catch (StorageException ex) {
       var retval = new BlockProcessingResult(Optional.empty(), ex);
-      handleAndLogImportFailure(block, retval);
+      handleAndLogImportFailure(block, retval, shouldRecordBadBlock);
       return retval;
     } catch (Exception ex) {
       throw new RuntimeException(ex);
     }
   }
 
   private void handleAndLogImportFailure(
-      final Block invalidBlock, final BlockValidationResult result) {
+      final Block invalidBlock,
+      final BlockValidationResult result,
+      final boolean shouldRecordBadBlock) {
     if (result.causedBy().isPresent()) {
       LOG.info(
           "Invalid block {}: {}, caused by {}",
@@ -201,7 +218,11 @@ private void handleAndLogImportFailure(
         LOG.info("Invalid block {}", invalidBlock.toLogString());
       }
     }
-    badBlockManager.addBadBlock(invalidBlock, result.causedBy());
+    if (shouldRecordBadBlock) {
+      badBlockManager.addBadBlock(invalidBlock, result.causedBy());
+    } else {
+      LOG.debug("Invalid block not added to badBlockManager {}", invalidBlock.toLogString());
+    }
   }
 
   /**

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

@@ -223,4 +223,48 @@ public void shouldReturnBadBlockBasedOnTheHash() {
         HeaderValidationMode.DETACHED_ONLY);
     assertThat(badBlockManager.getBadBlock(badBlock.getHash())).containsSame(badBlock);
   }
+
+  @Test
+  public void shouldNotAddBadBlockToBadBlockManagerWhenShouldNotRecordBadBlockFlagIsSetToTrue() {
+    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);
+  }
+
+  @Test
+  public void shouldAddBadBlockToBadBlockManagerWhenShouldNotRecordBadBlockFlagIsSetToFalse() {
+    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);
+  }
+
+  @Test
+  public void shouldAddBadBlockToBadBlockManagerWhenShouldNotRecordBadBlockFlagIsNotSet() {
+    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);
+  }
 }