Clean up BadBlockManager

Signed-off-by: mbaxter <[email protected]>
jflo · Mar 4, 2024 · 2e34005 · 2e34005
1 parent ce2a9ea
commit 2e34005
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 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
@@ -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<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());
+    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<Throwable> 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<Throwable> 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
@@ -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<Hash> 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
@@ -19,28 +19,15 @@
 
 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();
@@ -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();