Apply fcu even on invalid payload (hyperledger#5961)

* payload validation moved earlier, fcu v2 checks for cancun timestamps * allow fcu when payload invalid --------- Signed-off-by: Justin Florentine <[email protected]>
jflo · Sep 28, 2023 · ef2d418 · ef2d418
1 parent c4f66c4
commit ef2d418
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 11 deletions.
diff --git a/...er/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java b/...er/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java
@@ -102,6 +102,15 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     }
     Optional<List<Withdrawal>> withdrawals = Optional.empty();
     final BlockHeader newHead = maybeNewHead.get();
+    if (!isValidForkchoiceState(
+        forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) {
+      logForkchoiceUpdatedCall(INVALID, forkChoice);
+      return new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_FORKCHOICE_STATE);
+    }
+    ForkchoiceResult result =
+        mergeCoordinator.updateForkChoice(
+            newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash());
+
     if (maybePayloadAttributes.isPresent()) {
       final EnginePayloadAttributesParameter payloadAttributes = maybePayloadAttributes.get();
       withdrawals =
@@ -154,19 +163,9 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
               Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block")));
     }
 
-    if (!isValidForkchoiceState(
-        forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) {
-      logForkchoiceUpdatedCall(INVALID, forkChoice);
-      return new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_FORKCHOICE_STATE);
-    }
-
     maybePayloadAttributes.ifPresentOrElse(
         this::logPayload, () -> LOG.debug("Payload attributes are null"));
 
-    ForkchoiceResult result =
-        mergeCoordinator.updateForkChoice(
-            newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash());
-
     if (result.shouldNotProceedToPayloadBuildProcess()) {
       if (ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD.equals(result.getStatus())) {
         logForkchoiceUpdatedCall(VALID, forkChoice);

diff --git a/...esu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java b/...esu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java
@@ -141,16 +141,21 @@ public void shouldReturnSyncingIfMissingNewHead() {
 
   @Test
   public void shouldReturnInvalidWithLatestValidHashOnBadBlock() {
+    BlockHeader mockParent = blockHeaderBuilder.buildHeader();
+    blockHeaderBuilder.parentHash(mockParent.getHash());
     BlockHeader mockHeader = blockHeaderBuilder.buildHeader();
     Hash latestValidHash = Hash.hash(Bytes32.fromHexStringLenient("0xcafebabe"));
+    when(blockchain.getBlockHeader(mockHeader.getHash())).thenReturn(Optional.of(mockHeader));
+    when(blockchain.getBlockHeader(mockHeader.getParentHash())).thenReturn(Optional.of(mockParent));
     when(mergeCoordinator.getOrSyncHeadByHash(any(), any())).thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.isBadBlock(mockHeader.getHash())).thenReturn(true);
+    when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true);
     when(mergeCoordinator.getLatestValidHashOfBadBlock(mockHeader.getHash()))
         .thenReturn(Optional.of(latestValidHash));
 
     assertSuccessWithPayloadForForkchoiceResult(
         new EngineForkchoiceUpdatedParameter(
-            mockHeader.getHash(), Hash.ZERO, mockHeader.getParentHash()),
+            mockHeader.getHash(), mockHeader.getParentHash(), mockHeader.getParentHash()),
         Optional.empty(),
         mock(ForkchoiceResult.class),
         INVALID,