From 2e3400587e018a1c86f4128e35827c680b99b415 Mon Sep 17 00:00:00 2001 From: mbaxter Date: Wed, 28 Feb 2024 18:19:05 -0500 Subject: [PATCH] Clean up BadBlockManager Signed-off-by: mbaxter --- .../besu/ethereum/chain/BadBlockCause.java | 31 +++--------------- .../besu/ethereum/chain/BadBlockManager.java | 17 ++-------- .../ethereum/chain/BadBlockManagerTest.java | 32 ------------------- 3 files changed, 6 insertions(+), 74 deletions(-) diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockCause.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockCause.java index 41054a646d7..d9bfe5674a9 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockCause.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockCause.java @@ -17,8 +17,6 @@ import org.hyperledger.besu.ethereum.core.Block; -import java.util.Optional; - public class BadBlockCause { public enum BadBlockReason { // Standard spec-related validation failures @@ -31,29 +29,20 @@ public enum BadBlockReason { private final BadBlockReason reason; private final String description; - private final Optional 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()); + return new BadBlockCause(BadBlockReason.DESCENDS_FROM_BAD_BLOCK, description); } public static BadBlockCause fromValidationFailure(final String failureMessage) { - return new BadBlockCause( - BadBlockReason.SPEC_VALIDATION_FAILURE, failureMessage, Optional.empty()); + return new BadBlockCause(BadBlockReason.SPEC_VALIDATION_FAILURE, failureMessage); } - private BadBlockCause(BadBlockReason reason, String description, Optional exception) { + private BadBlockCause(BadBlockReason reason, String description) { this.reason = reason; this.description = description; - this.exception = exception; } public BadBlockReason getReason() { @@ -64,20 +53,8 @@ public String getDescription() { return description; } - public Optional getException() { - return exception; - } - @Override public String toString() { - return "BadBlockCause{" - + "reason=" - + reason - + ", description='" - + description - + '\'' - + ", exception=" - + exception - + '}'; + return "BadBlockCause{" + "reason=" + reason + ", description='" + description + '\'' + '}'; } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockManager.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockManager.java index 46155f94bc2..b13f81c877c 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockManager.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/BadBlockManager.java @@ -17,8 +17,6 @@ import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.BlockHeader; -import org.hyperledger.besu.ethereum.trie.MerkleTrieException; -import org.hyperledger.besu.plugin.services.exception.StorageException; import java.util.Collection; import java.util.Optional; @@ -48,10 +46,8 @@ public class BadBlockManager { */ 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); - } + LOG.debug("Register bad block {} with cause: {}", badBlock.toLogString(), cause); + this.badBlocks.put(badBlock.getHash(), badBlock); } public void reset() { @@ -101,13 +97,4 @@ public void addLatestValidHash(final Hash blockHash, final Hash latestValidHash) public Optional getLatestValidHash(final Hash blockHash) { return Optional.ofNullable(latestValidHashes.getIfPresent(blockHash)); } - - private boolean isInternalError(final BadBlockCause cause) { - if (cause.getException().isEmpty()) { - 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; - } } diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/chain/BadBlockManagerTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/chain/BadBlockManagerTest.java index 52288a09066..ba027c64361 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/chain/BadBlockManagerTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/chain/BadBlockManagerTest.java @@ -19,15 +19,8 @@ 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 { @@ -35,12 +28,6 @@ public class BadBlockManagerTest { final Block block = chainUtil.getBlock(1); final BadBlockManager badBlockManager = new BadBlockManager(); - public static Stream 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(); @@ -50,25 +37,6 @@ public void addBadBlock_addsBlock() { 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();