diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java index 56030131ea1..00ada80dc24 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java +++ b/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,22 @@ public BlockProcessingResult validateBlock(final Block block) { return validationResult; } + private 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); diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java index 12d8afd673e..6ceeed263de 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java +++ b/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 { diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/BlockValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/BlockValidator.java index 370eafb3326..bed189c4935 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/BlockValidator.java +++ b/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, diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java index d3a78b6bf94..1a95033d2a1 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java +++ b/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,7 +106,7 @@ 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(); @@ -101,12 +114,12 @@ public BlockProcessingResult validateAndProcessBlock( 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 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,11 +187,13 @@ 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); @@ -186,7 +201,9 @@ public BlockProcessingResult validateAndProcessBlock( } 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()); + } } /** diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java index eac1c5d7c43..636bc73230a 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/MainnetBlockValidatorTest.java +++ b/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 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); + } + + @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); + } + + @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); + } }