Hive engine suite conformance: Invalid Timestamp, Invalid Transition Payload (#4042)

* make invalid timestamp error return success response with INVALID status
* fix newPayloadTest expectation for invalid timestamp, add check in bad blocks for heads not in blockchain, return Hash.ZERO for latest valid ancestors that are PoW
* assert latestValidHash returns ZERO if PoW and ancestor hash if not.

Signed-off-by: garyschulte <garyschulte@gmail.com>

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

@@ -487,7 +487,15 @@ public class MergeCoordinator implements MergeMiningCoordinator {
     // check chain first
     return chain
         .getBlockHeader(parentHash)
-        .map(BlockHeader::getHash)
+        .map(
+            header -> {
+              // if block is PoW, return ZERO hash
+              if (header.getDifficulty().greaterThan(Difficulty.ZERO)) {
+                return Hash.ZERO;
+              } else {
+                return header.getHash();
+              }
+            })
         .map(Optional::of)
         .orElseGet(
             () ->

consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java

@@ -166,13 +166,21 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper {
 
     BlockHeader parentHeader = nextBlockHeader(terminalHeader);
     Block parent = new Block(parentHeader, BlockBody.empty());
-    sendNewPayloadAndForkchoiceUpdate(parent, Optional.empty(), terminalHeader.getHash());
 
+    // if latest valid ancestor is PoW, then latest valid hash should be Hash.ZERO
+    var lvh = this.coordinator.getLatestValidAncestor(parentHeader);
+    assertThat(lvh).isPresent();
+    assertThat(lvh.get()).isEqualTo(Hash.ZERO);
+
+    sendNewPayloadAndForkchoiceUpdate(parent, Optional.empty(), terminalHeader.getHash());
     BlockHeader childHeader = nextBlockHeader(parentHeader);
     Block child = new Block(childHeader, BlockBody.empty());
     coordinator.validateBlock(child);
     assertThat(this.coordinator.latestValidAncestorDescendsFromTerminal(child.getHeader()))
         .isTrue();
+    var nextLvh = this.coordinator.getLatestValidAncestor(childHeader);
+    assertThat(nextLvh).isPresent();
+    assertThat(nextLvh.get()).isEqualTo(parentHeader.getHash());
   }
 
   @Test
@@ -183,8 +191,13 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper {
 
     BlockHeader grandParentHeader = nextBlockHeader(terminalHeader);
     Block grandParent = new Block(grandParentHeader, BlockBody.empty());
-    sendNewPayloadAndForkchoiceUpdate(grandParent, Optional.empty(), terminalHeader.getHash());
 
+    // if latest valid ancestor is PoW, then latest valid hash should be Hash.ZERO
+    var lvh = this.coordinator.getLatestValidAncestor(grandParentHeader);
+    assertThat(lvh).isPresent();
+    assertThat(lvh.get()).isEqualTo(Hash.ZERO);
+
+    sendNewPayloadAndForkchoiceUpdate(grandParent, Optional.empty(), terminalHeader.getHash());
     BlockHeader parentHeader = nextBlockHeader(grandParentHeader);
     Block parent = new Block(parentHeader, BlockBody.empty());
     sendNewPayloadAndForkchoiceUpdate(
@@ -196,6 +209,11 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper {
 
     assertThat(this.coordinator.latestValidAncestorDescendsFromTerminal(child.getHeader()))
         .isTrue();
+
+    var nextLvh = this.coordinator.getLatestValidAncestor(childHeader);
+    assertThat(nextLvh).isPresent();
+    assertThat(nextLvh.get()).isEqualTo(parentHeader.getHash());
+
     verify(mergeContext, never()).getTerminalPoWBlock();
   }
 

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

@@ -80,6 +80,16 @@ public class EngineForkchoiceUpdated extends ExecutionEngineJsonRpcMethod {
       return syncingResponse(requestId);
     }
 
+    if (mergeCoordinator.isBadBlock(forkChoice.getHeadBlockHash())) {
+      return new JsonRpcSuccessResponse(
+          requestId,
+          new EngineUpdateForkchoiceResult(
+              INVALID,
+              Hash.ZERO,
+              null,
+              Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block")));
+    }
+
     Optional<BlockHeader> newHead =
         protocolContext.getBlockchain().getBlockHeader(forkChoice.getHeadBlockHash());
 

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

@@ -29,8 +29,6 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadParameter;
-import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
-import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EnginePayloadStatusResult;
@@ -149,7 +147,10 @@ public class EngineNewPayload extends ExecutionEngineJsonRpcMethod {
     if (parentHeader.isPresent()
         && (blockParam.getTimestamp() <= parentHeader.get().getTimestamp())) {
       LOG.info("method parameter timestamp not greater than parent");
-      return new JsonRpcErrorResponse(reqId, JsonRpcError.INVALID_PARAMS);
+      return respondWithInvalid(
+          reqId,
+          mergeCoordinator.getLatestValidAncestor(blockParam.getParentHash()).orElse(null),
+          "block timestamp not greater than parent");
     }
 
     final var block =

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

@@ -231,7 +231,12 @@ public class EngineNewPayloadTest {
     when(blockchain.getBlockHeader(parent.getHash())).thenReturn(Optional.of(parent));
     var resp = resp(mockPayload(mockHeader, Collections.emptyList()));
 
-    assertThat(resp.getType()).isEqualTo(JsonRpcResponseType.ERROR);
+    assertThat(resp.getType()).isEqualTo(JsonRpcResponseType.SUCCESS);
+    var res = ((JsonRpcSuccessResponse) resp).getResult();
+    assertThat(res).isInstanceOf(EnginePayloadStatusResult.class);
+    var payloadStatusResult = (EnginePayloadStatusResult) res;
+    assertThat(payloadStatusResult.getStatus()).isEqualTo(INVALID);
+    assertThat(payloadStatusResult.getError()).isEqualTo("block timestamp not greater than parent");
   }
 
   @Test