Shanghai Engine API Updates

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

@@ -119,7 +119,8 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     }
 
     // TODO: post-merge cleanup, this should be unnecessary after merge
-    if (!mergeContext.get().isCheckpointPostMergeSync()
+    if (requireTerminalPoWBlockValidation()
+        && !mergeContext.get().isCheckpointPostMergeSync()
         && !mergeCoordinator.latestValidAncestorDescendsFromTerminal(newHead)
         && !mergeContext.get().isChainPruningEnabled()) {
       logForkchoiceUpdatedCall(INVALID, forkChoice);
@@ -154,7 +155,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
           "Invalid payload attributes: {}",
           () ->
               maybePayloadAttributes.map(EnginePayloadAttributesParameter::serialize).orElse(null));
-      return new JsonRpcErrorResponse(requestId, JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES);
+      return new JsonRpcErrorResponse(requestId, getInvalidPayloadError());
     }
 
     if (!result.isValid()) {
@@ -217,9 +218,6 @@ private JsonRpcResponse handleNonValidForkchoiceUpdate(
                 new EngineUpdateForkchoiceResult(
                     INVALID, latestValid.orElse(null), null, result.getErrorMessage()));
         break;
-      case INVALID_PAYLOAD_ATTRIBUTES:
-        response = new JsonRpcErrorResponse(requestId, JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES);
-        break;
       case IGNORE_UPDATE_TO_OLD_HEAD:
         response =
             new JsonRpcSuccessResponse(
@@ -300,6 +298,14 @@ private JsonRpcResponse syncingResponse(
         requestId, new EngineUpdateForkchoiceResult(SYNCING, null, null, Optional.empty()));
   }
 
+  protected boolean requireTerminalPoWBlockValidation() {
+    return false;
+  }
+
+  protected JsonRpcError getInvalidPayloadError() {
+    return JsonRpcError.INVALID_PARAMS;
+  }
+
   // fcU calls are synchronous, no need to make volatile
   private long lastFcuInfoLog = System.currentTimeMillis();
   private static final String logMessage =

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

@@ -165,7 +165,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
               "Computed block hash %s does not match block hash parameter %s",
               newBlockHeader.getBlockHash(), blockParam.getBlockHash());
       LOG.debug(errorMessage);
-      return respondWithInvalid(reqId, blockParam, null, INVALID_BLOCK_HASH, errorMessage);
+      return respondWithInvalid(reqId, blockParam, null, getInvalidBlockHashStatus(), errorMessage);
     }
     // do we already have this payload
     if (protocolContext.getBlockchain().getBlockByHash(newBlockHeader.getBlockHash()).isPresent()) {
@@ -208,7 +208,8 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     }
 
     // TODO: post-merge cleanup
-    if (!mergeContext.get().isCheckpointPostMergeSync()
+    if (requireTerminalPoWBlockValidation()
+        && !mergeContext.get().isCheckpointPostMergeSync()
         && !mergeCoordinator.latestValidAncestorDescendsFromTerminal(newBlockHeader)
         && !mergeContext.get().isChainPruningEnabled()) {
       mergeCoordinator.addBadBlock(block, Optional.empty());
@@ -308,6 +309,14 @@ JsonRpcResponse respondWithInvalid(
             invalidStatus, latestValidHash, Optional.of(validationError)));
   }
 
+  protected boolean requireTerminalPoWBlockValidation() {
+    return false;
+  }
+
+  protected EngineStatus getInvalidBlockHashStatus() {
+    return INVALID;
+  }
+
   private void logImportedBlockInfo(final Block block, final double timeInS) {
     LOG.info(
         String.format(

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

@@ -17,6 +17,7 @@
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
 import org.hyperledger.besu.ethereum.mainnet.TimestampSchedule;
 
 import io.vertx.core.Vertx;
@@ -36,4 +37,14 @@ public EngineForkchoiceUpdatedV1(
   public String getName() {
     return RpcMethod.ENGINE_FORKCHOICE_UPDATED_V1.getMethodName();
   }
+
+  @Override
+  protected boolean requireTerminalPoWBlockValidation() {
+    return true;
+  }
+
+  @Override
+  protected JsonRpcError getInvalidPayloadError() {
+    return JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES;
+  }
 }

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

@@ -14,6 +14,8 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 
+import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID_BLOCK_HASH;
+
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
@@ -39,4 +41,14 @@ public EngineNewPayloadV1(
   public String getName() {
     return RpcMethod.ENGINE_NEW_PAYLOAD_V1.getMethodName();
   }
+
+  @Override
+  protected boolean requireTerminalPoWBlockValidation() {
+    return true;
+  }
+
+  @Override
+  protected EngineStatus getInvalidBlockHashStatus() {
+    return INVALID_BLOCK_HASH;
+  }
 }

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

@@ -140,6 +140,7 @@ public void shouldReturnSyncingIfMissingNewHead() {
 
   @Test
   public void shouldReturnInvalidOnBadTerminalBlock() {
+    if (!validateTerminalPoWBlock()) return;
     BlockHeader mockHeader = blockHeaderBuilder.buildHeader();
 
     when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO))
@@ -181,7 +182,8 @@ public void shouldReturnValidWithoutFinalizedOrPayload() {
     BlockHeader mockHeader = blockHeaderBuilder.buildHeader();
     when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO))
         .thenReturn(Optional.of(mockHeader));
-    when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
+    if (validateTerminalPoWBlock())
+      when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
 
     assertSuccessWithPayloadForForkchoiceResult(
         new EngineForkchoiceUpdatedParameter(mockHeader.getHash(), Hash.ZERO, Hash.ZERO),
@@ -199,7 +201,8 @@ public void shouldReturnInvalidOnOldTimestamp() {
             .timestamp(parent.getTimestamp())
             .buildHeader();
     when(blockchain.getBlockHeader(parent.getHash())).thenReturn(Optional.of(parent));
-    when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
+    if (validateTerminalPoWBlock())
+      when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
     when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true);
     when(mergeContext.isSyncing()).thenReturn(false);
     when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), parent.getHash()))
@@ -244,7 +247,8 @@ public void shouldReturnValidWithoutFinalizedWithPayload() {
     BlockHeader mockHeader = blockHeaderBuilder.buildHeader();
     when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO))
         .thenReturn(Optional.of(mockHeader));
-    when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
+    if (validateTerminalPoWBlock())
+      when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
 
     var payloadParams =
         new EnginePayloadAttributesParameter(
@@ -427,7 +431,8 @@ public void shouldIgnoreUpdateToOldHeadAndNotPreparePayload() {
 
     when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO))
         .thenReturn(Optional.of(mockHeader));
-    when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
+    if (validateTerminalPoWBlock())
+      when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
 
     var ignoreOldHeadUpdateRes = ForkchoiceResult.withIgnoreUpdateToOldHead(mockHeader);
     when(mergeCoordinator.updateForkChoice(any(), any(), any())).thenReturn(ignoreOldHeadUpdateRes);
@@ -481,7 +486,7 @@ public void shouldReturnInvalidIfPayloadTimestampNotGreaterThanHead() {
                 mockHeader.getHash(), Hash.ZERO, mockParent.getHash()),
             Optional.of(payloadParams));
 
-    assertInvalidForkchoiceState(resp, JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES);
+    assertInvalidForkchoiceState(resp, expectedInvalidPayloadError());
     verify(engineCallListener, times(1)).executionEngineCalled();
   }
 
@@ -505,7 +510,7 @@ public void shouldReturnInvalidIfWithdrawalsIsNotNull_WhenWithdrawalsProhibited(
                 mockHeader.getHash(), Hash.ZERO, mockParent.getHash()),
             Optional.of(payloadParams));
 
-    assertInvalidForkchoiceState(resp, JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES);
+    assertInvalidForkchoiceState(resp, expectedInvalidPayloadError());
     verify(engineCallListener, times(1)).executionEngineCalled();
   }
 
@@ -569,7 +574,7 @@ public void shouldReturnInvalidIfWithdrawalsIsNull_WhenWithdrawalsAllowed() {
                 mockHeader.getHash(), Hash.ZERO, mockParent.getHash()),
             Optional.of(payloadParams));
 
-    assertInvalidForkchoiceState(resp, JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES);
+    assertInvalidForkchoiceState(resp, expectedInvalidPayloadError());
     verify(engineCallListener, times(1)).executionEngineCalled();
   }
 
@@ -667,7 +672,8 @@ private void setupValidForkchoiceUpdate(final BlockHeader mockHeader) {
     when(blockchain.getBlockHeader(any())).thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.getOrSyncHeadByHash(mockHeader.getHash(), Hash.ZERO))
         .thenReturn(Optional.of(mockHeader));
-    when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
+    if (validateTerminalPoWBlock())
+      when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(mockHeader)).thenReturn(true);
     when(mergeCoordinator.isDescendantOf(any(), any())).thenReturn(true);
   }
 
@@ -724,6 +730,14 @@ private EngineUpdateForkchoiceResult assertSuccessWithPayloadForForkchoiceResult
     return res;
   }
 
+  protected boolean validateTerminalPoWBlock() {
+    return false;
+  }
+
+  protected JsonRpcError expectedInvalidPayloadError() {
+    return JsonRpcError.INVALID_PARAMS;
+  }
+
   private JsonRpcResponse resp(
       final EngineForkchoiceUpdatedParameter forkchoiceParam,
       final Optional<EnginePayloadAttributesParameter> payloadParam) {

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

@@ -17,7 +17,6 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.ACCEPTED;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID;
-import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID_BLOCK_HASH;
 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.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.WithdrawalParameterTestFixture.WITHDRAWAL_PARAM_1;
@@ -42,6 +41,7 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest;
 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.parameters.UnsignedLongParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.WithdrawalParameter;
@@ -169,8 +169,9 @@ public void shouldReturnAcceptedOnLatestValidAncestorEmpty() {
         .thenReturn(Optional.of(mock(BlockHeader.class)));
     when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class)))
         .thenReturn(Optional.empty());
-    when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class)))
-        .thenReturn(true);
+    if (validateTerminalPoWBlock())
+      when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class)))
+          .thenReturn(true);
 
     var resp = resp(mockPayload(mockHeader, Collections.emptyList()));
 
@@ -198,6 +199,7 @@ public void shouldReturnSuccessOnAlreadyPresent() {
 
   @Test
   public void shouldReturnInvalidOnBadTerminalBlock() {
+    if (!validateTerminalPoWBlock()) return;
     BlockHeader mockHeader = createBlockHeader();
     when(blockchain.getBlockByHash(mockHeader.getHash())).thenReturn(Optional.empty());
     when(blockchain.getBlockHeader(mockHeader.getParentHash()))
@@ -268,8 +270,9 @@ public void shouldNotReturnInvalidOnThrownMerkleTrieException() {
         .thenReturn(Optional.of(mock(BlockHeader.class)));
     when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class)))
         .thenReturn(Optional.of(mockHash));
-    when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class)))
-        .thenReturn(true);
+    if (validateTerminalPoWBlock())
+      when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class)))
+          .thenReturn(true);
     when(mergeCoordinator.rememberBlock(any())).thenThrow(new MerkleTrieException("missing leaf"));
 
     var resp = resp(mockPayload(mockHeader, Collections.emptyList()));
@@ -288,7 +291,7 @@ public void shouldReturnInvalidBlockHashOnBadHashParameter() {
 
     EnginePayloadStatusResult res = fromSuccessResp(resp);
     assertThat(res.getLatestValidHash()).isEmpty();
-    assertThat(res.getStatusAsString()).isEqualTo(INVALID_BLOCK_HASH.name());
+    assertThat(res.getStatusAsString()).isEqualTo(getCorrectInvalidBlockHashStatus().name());
     verify(engineCallListener, times(1)).executionEngineCalled();
   }
 
@@ -302,7 +305,7 @@ public void shouldCheckBlockValidityBeforeCheckingByHashForExisting() {
 
     EnginePayloadStatusResult res = fromSuccessResp(resp);
     assertThat(res.getLatestValidHash()).isEmpty();
-    assertThat(res.getStatusAsString()).isEqualTo(INVALID_BLOCK_HASH.name());
+    assertThat(res.getStatusAsString()).isEqualTo(getCorrectInvalidBlockHashStatus().name());
     verify(engineCallListener, times(1)).executionEngineCalled();
   }
 
@@ -526,12 +529,21 @@ private BlockHeader setupValidPayload(final BlockProcessingResult value) {
         .thenReturn(Optional.of(mock(BlockHeader.class)));
     when(mergeCoordinator.getLatestValidAncestor(any(BlockHeader.class)))
         .thenReturn(Optional.of(mockHash));
-    when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class)))
-        .thenReturn(true);
+    if (validateTerminalPoWBlock())
+      when(mergeCoordinator.latestValidAncestorDescendsFromTerminal(any(BlockHeader.class)))
+          .thenReturn(true);
     when(mergeCoordinator.rememberBlock(any())).thenReturn(value);
     return mockHeader;
   }
 
+  protected boolean validateTerminalPoWBlock() {
+    return false;
+  }
+
+  protected ExecutionEngineJsonRpcMethod.EngineStatus getCorrectInvalidBlockHashStatus() {
+    return INVALID;
+  }
+
   private EnginePayloadStatusResult fromSuccessResp(final JsonRpcResponse resp) {
     assertThat(resp.getType()).isEqualTo(JsonRpcResponseType.SUCCESS);
     return Optional.of(resp)

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

@@ -17,6 +17,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -39,4 +40,14 @@ public void shouldReturnExpectedMethodName() {
   protected String getMethodName() {
     return RpcMethod.ENGINE_FORKCHOICE_UPDATED_V1.getMethodName();
   }
+
+  @Override
+  protected boolean validateTerminalPoWBlock() {
+    return true;
+  }
+
+  @Override
+  protected JsonRpcError expectedInvalidPayloadError() {
+    return JsonRpcError.INVALID_PAYLOAD_ATTRIBUTES;
+  }
 }

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

@@ -15,6 +15,9 @@
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID_BLOCK_HASH;
+
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod;
 
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -32,4 +35,14 @@ public EngineNewPayloadV1Test() {
   public void shouldReturnExpectedMethodName() {
     assertThat(method.getName()).isEqualTo("engine_newPayloadV1");
   }
+
+  @Override
+  protected boolean validateTerminalPoWBlock() {
+    return true;
+  }
+
+  @Override
+  protected ExecutionEngineJsonRpcMethod.EngineStatus getCorrectInvalidBlockHashStatus() {
+    return INVALID_BLOCK_HASH;
+  }
 }