New payload: add invalid to bad blocks (#4080)

* if block did not descend from the terminal block add it to the bad blocks

Signed-off-by: Daniel Lehrner <daniel.lehrner@consensys.net>

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## 22.7.0-RC2
 
 ### Additions and Improvements
+- Add a block to the bad blocks if it did not descend from the terminal block [#4080](https://github.com/hyperledger/besu/pull/4080)
 
 ### Bug Fixes
 - Return the correct latest valid hash in case of bad block when calling engine methods [#4056](https://github.com/hyperledger/besu/pull/4056)

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

@@ -260,12 +260,7 @@ public class MergeCoordinator implements MergeMiningCoordinator {
                 HeaderValidationMode.NONE,
                 false);
 
-    validationResult.errorMessage.ifPresent(
+    validationResult.errorMessage.ifPresent(errMsg -> addBadBlock(block));
-        errMsg ->
-            protocolSchedule
-                .getByBlockNumber(chain.getChainHeadBlockNumber())
-                .getBadBlocksManager()
-                .addBadBlock(block));
 
     return validationResult;
   }
@@ -546,6 +541,14 @@ public class MergeCoordinator implements MergeMiningCoordinator {
     MergeBlockCreator forParams(BlockHeader header, Optional<Address> feeRecipient);
   }
 
+  @Override
+  public void addBadBlock(final Block block) {
+    protocolSchedule
+        .getByBlockNumber(protocolContext.getBlockchain().getChainHeadBlockNumber())
+        .getBadBlocksManager()
+        .addBadBlock(block);
+  }
+
   @Override
   public boolean isBadBlock(final Hash blockHash) {
     final BadBlockManager badBlocksManager =

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

@@ -64,6 +64,8 @@ public interface MergeMiningCoordinator extends MiningCoordinator {
 
   Optional<BlockHeader> getOrSyncHeaderByHash(Hash blockHash, Hash finalizedBlockHash);
 
+  void addBadBlock(final Block block);
+
   boolean isBadBlock(Hash blockHash);
 
   class ForkchoiceResult {

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

@@ -199,6 +199,11 @@ public class TransitionCoordinator extends TransitionUtils<MiningCoordinator>
     return mergeCoordinator.isDescendantOf(ancestorBlock, newBlock);
   }
 
+  @Override
+  public void addBadBlock(final Block block) {
+    mergeCoordinator.addBadBlock(block);
+  }
+
   @Override
   public boolean isBadBlock(final Hash blockHash) {
     return mergeCoordinator.isBadBlock(blockHash);

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayload.java

@@ -171,6 +171,7 @@ public class EngineNewPayload extends ExecutionEngineJsonRpcMethod {
 
     // TODO: post-merge cleanup
     if (!mergeCoordinator.latestValidAncestorDescendsFromTerminal(newBlockHeader)) {
+      mergeCoordinator.addBadBlock(block);
       return respondWithInvalid(
           reqId,
           blockParam,

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadTest.java

@@ -21,8 +21,10 @@ import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.Executi
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.SYNCING;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.VALID;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.consensus.merge.MergeContext;
@@ -177,6 +179,7 @@ public class EngineNewPayloadTest {
     EnginePayloadStatusResult res = fromSuccessResp(resp);
     assertThat(res.getLatestValidHash()).isEqualTo(Optional.of(Hash.ZERO));
     assertThat(res.getStatusAsString()).isEqualTo(INVALID.name());
+    verify(mergeCoordinator, atLeastOnce()).addBadBlock(any());
   }
 
   @Test