Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes #8204 by using BadBlockManager from protocol context #8207

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
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;
import org.hyperledger.besu.ethereum.core.BlockHeader;
Expand Down Expand Up @@ -56,27 +55,21 @@ public class MainnetBlockValidator implements BlockValidator {
/** The BlockProcessor used to process blocks. */
protected final BlockProcessor blockProcessor;

/** The BadBlockManager used to manage bad blocks. */
protected final BadBlockManager badBlockManager;

/**
* Constructs a new MainnetBlockValidator with the given BlockHeaderValidator, BlockBodyValidator,
* BlockProcessor, and BadBlockManager.
*
* @param blockHeaderValidator the BlockHeaderValidator used to validate block headers
* @param blockBodyValidator the BlockBodyValidator used to validate block bodies
* @param blockProcessor the BlockProcessor used to process blocks
* @param badBlockManager the BadBlockManager used to manage bad blocks
*/
public MainnetBlockValidator(
final BlockHeaderValidator blockHeaderValidator,
final BlockBodyValidator blockBodyValidator,
final BlockProcessor blockProcessor,
final BadBlockManager badBlockManager) {
final BlockProcessor blockProcessor) {
this.blockHeaderValidator = blockHeaderValidator;
this.blockBodyValidator = blockBodyValidator;
this.blockProcessor = blockProcessor;
this.badBlockManager = badBlockManager;
}

/**
Expand Down Expand Up @@ -131,7 +124,7 @@ public BlockProcessingResult validateAndProcessBlock(
new BlockProcessingResult(
"Parent block with hash " + header.getParentHash() + " not present");
// Blocks should not be marked bad due to missing data
handleFailedBlockProcessing(block, retval, false);
handleFailedBlockProcessing(block, retval, false, context);
return retval;
}
parentHeader = maybeParentHeader.get();
Expand All @@ -140,13 +133,13 @@ public BlockProcessingResult validateAndProcessBlock(
header, parentHeader, context, headerValidationMode)) {
final String error = String.format("Header validation failed (%s)", headerValidationMode);
var retval = new BlockProcessingResult(error);
handleFailedBlockProcessing(block, retval, shouldRecordBadBlock);
handleFailedBlockProcessing(block, retval, shouldRecordBadBlock, context);
return retval;
}
} catch (StorageException ex) {
var retval = new BlockProcessingResult(Optional.empty(), ex);
// Blocks should not be marked bad due to a local storage failure
handleFailedBlockProcessing(block, retval, false);
handleFailedBlockProcessing(block, retval, false, context);
return retval;
}

Expand All @@ -165,12 +158,12 @@ public BlockProcessingResult validateAndProcessBlock(
+ parentHeader.getStateRoot()
+ " is not available");
// Blocks should not be marked bad due to missing data
handleFailedBlockProcessing(block, retval, false);
handleFailedBlockProcessing(block, retval, false, context);
return retval;
}
var result = processBlock(context, worldState, block);
if (result.isFailed()) {
handleFailedBlockProcessing(block, result, shouldRecordBadBlock);
handleFailedBlockProcessing(block, result, shouldRecordBadBlock, context);
return result;
} else {
List<TransactionReceipt> receipts =
Expand All @@ -185,7 +178,7 @@ public BlockProcessingResult validateAndProcessBlock(
ommerValidationMode,
BodyValidationMode.FULL)) {
result = new BlockProcessingResult("failed to validate output of imported block");
handleFailedBlockProcessing(block, result, shouldRecordBadBlock);
handleFailedBlockProcessing(block, result, shouldRecordBadBlock, context);
return result;
}

Expand All @@ -199,7 +192,7 @@ public BlockProcessingResult validateAndProcessBlock(
} catch (StorageException ex) {
var retval = new BlockProcessingResult(Optional.empty(), ex);
// Do not record bad block due to a local storage issue
handleFailedBlockProcessing(block, retval, false);
handleFailedBlockProcessing(block, retval, false, context);
return retval;
} catch (Exception ex) {
// Wrap checked autocloseable exception from try-with-resources
Expand All @@ -210,7 +203,8 @@ public BlockProcessingResult validateAndProcessBlock(
private void handleFailedBlockProcessing(
final Block failedBlock,
final BlockValidationResult result,
final boolean shouldRecordBadBlock) {
final boolean shouldRecordBadBlock,
final ProtocolContext context) {
if (result.causedBy().isPresent()) {
// Block processing failed exceptionally, we cannot assume the block was intrinsically invalid
LOG.info(
Expand All @@ -230,7 +224,7 @@ private void handleFailedBlockProcessing(
// Result.errorMessage should not be empty on failure, but add a default to be safe
String description = result.errorMessage.orElse("Unknown cause");
final BadBlockCause cause = BadBlockCause.fromValidationFailure(description);
badBlockManager.addBadBlock(failedBlock, cause);
context.getBadBlockManager().addBadBlock(failedBlock, cause);
} else {
LOG.debug("Invalid block {} not added to badBlockManager ", failedBlock.toLogString());
}
Expand Down Expand Up @@ -268,7 +262,9 @@ public boolean validateBlockForSyncing(
final BlockHeader header = block.getHeader();
if (!blockHeaderValidator.validateHeader(header, context, headerValidationMode)) {
String description = String.format("Failed header validation (%s)", headerValidationMode);
badBlockManager.addBadBlock(block, BadBlockCause.fromValidationFailure(description));
context
.getBadBlockManager()
.addBadBlock(block, BadBlockCause.fromValidationFailure(description));
return false;
}

Expand All @@ -277,8 +273,10 @@ public boolean validateBlockForSyncing(
}

if (!blockBodyValidator.validateBodyLight(context, block, receipts, ommerValidationMode)) {
badBlockManager.addBadBlock(
block, BadBlockCause.fromValidationFailure("Failed body validation (light)"));
context
.getBadBlockManager()
.addBadBlock(
block, BadBlockCause.fromValidationFailure("Failed body validation (light)"));
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,7 @@ public ProtocolSpec build(final ProtocolSchedule protocolSchedule) {
}

final BlockValidator blockValidator =
blockValidatorBuilder.apply(
blockHeaderValidator, blockBodyValidator, blockProcessor, badBlockManager);
blockValidatorBuilder.apply(blockHeaderValidator, blockBodyValidator, blockProcessor);
final BlockImporter blockImporter = blockImporterBuilder.apply(blockValidator);
return new ProtocolSpec(
name,
Expand Down Expand Up @@ -501,8 +500,7 @@ public interface BlockValidatorBuilder {
BlockValidator apply(
BlockHeaderValidator blockHeaderValidator,
BlockBodyValidator blockBodyValidator,
BlockProcessor blockProcessor,
BadBlockManager badBlockManager);
BlockProcessor blockProcessor);
}

public interface BlockImporterBuilder {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,7 @@ public void setup() {
when(protocolSpec.getGasCalculator()).thenReturn(gasCalculator);
when(protocolSpec.getFeeMarket()).thenReturn(feeMarket);
mainnetBlockValidator =
new MainnetBlockValidator(
blockHeaderValidator, blockBodyValidator, blockProcessor, badBlockManager);
new MainnetBlockValidator(blockHeaderValidator, blockBodyValidator, blockProcessor);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ public class MainnetBlockValidatorTest {
private final BlockBodyValidator blockBodyValidator = mock(BlockBodyValidator.class);

private final MainnetBlockValidator mainnetBlockValidator =
new MainnetBlockValidator(
blockHeaderValidator, blockBodyValidator, blockProcessor, badBlockManager);
new MainnetBlockValidator(blockHeaderValidator, blockBodyValidator, blockProcessor);

public static Stream<Arguments> getStorageExceptions() {
return Stream.of(
Expand All @@ -92,6 +91,7 @@ public void setup() {
new BlockProcessingResult(Optional.empty(), false);

when(protocolContext.getBlockchain()).thenReturn(blockchain);
when(protocolContext.getBadBlockManager()).thenReturn(badBlockManager);
when(protocolContext.getWorldStateArchive()).thenReturn(worldStateArchive);
when(worldStateArchive.getWorldState(any())).thenReturn(Optional.of(worldState));
when(worldStateArchive.getWorldState(any())).thenReturn(Optional.of(worldState));
Expand Down