Prague Engine API validation fixes (hyperledger#8250)

- Prague newPayload validation fixes - Testing the unsupported fork timestamp cases Signed-off-by: Simon Dudley <[email protected]> Signed-off-by: Bhanu Pulluri <[email protected]>
pullurib · Feb 6, 2025 · 2a9b4d1 · 2a9b4d1
1 parent 31a04b7
commit 2a9b4d1
Show file tree

Hide file tree

Showing 18 changed files with 276 additions and 68 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
@@ -170,11 +170,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
             .log();
         return maybeError.get();
       }
-      ValidationResult<RpcErrorType> forkValidationResult =
-          validateForkSupported(payloadAttributes.getTimestamp());
-      if (!forkValidationResult.isValid()) {
-        return new JsonRpcErrorResponse(requestId, forkValidationResult);
-      }
     }
 
     final BlockHeader newHead = maybeNewHead.get();

diff --git a/...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java b/...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java
@@ -153,6 +153,12 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
           e);
     }
 
+    final ValidationResult<RpcErrorType> forkValidationResult =
+        validateForkSupported(blockParam.getTimestamp());
+    if (!forkValidationResult.isValid()) {
+      return new JsonRpcErrorResponse(reqId, forkValidationResult);
+    }
+
     final ValidationResult<RpcErrorType> parameterValidationResult =
         validateParameters(
             blockParam,
@@ -163,12 +169,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
       return new JsonRpcErrorResponse(reqId, parameterValidationResult);
     }
 
-    final ValidationResult<RpcErrorType> forkValidationResult =
-        validateForkSupported(blockParam.getTimestamp());
-    if (!forkValidationResult.isValid()) {
-      return new JsonRpcErrorResponse(reqId, forkValidationResult);
-    }
-
     final Optional<List<VersionedHash>> maybeVersionedHashes;
     try {
       maybeVersionedHashes = extractVersionedHashes(maybeVersionedHashParam);

diff --git a/...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java b/...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java
@@ -59,6 +59,12 @@ protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
       return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadAttributesError()));
     }
 
+    ValidationResult<RpcErrorType> forkValidationResult =
+        validateForkSupported(payloadAttributes.getTimestamp());
+    if (!forkValidationResult.isValid()) {
+      return Optional.of(new JsonRpcErrorResponse(requestId, forkValidationResult));
+    }
+
     return Optional.empty();
   }
 

diff --git a/...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java b/...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java
@@ -81,6 +81,7 @@ protected ValidationResult<RpcErrorType> validateForkSupported(final long blockT
   @Override
   protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
       final Object requestId, final EnginePayloadAttributesParameter payloadAttributes) {
+
     if (payloadAttributes.getParentBeaconBlockRoot() == null) {
       LOG.error(
           "Parent beacon block root hash not present in payload attributes after cancun hardfork");
@@ -91,8 +92,10 @@ protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
       return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadAttributesError()));
     }
 
-    if (cancunMilestone.isEmpty() || payloadAttributes.getTimestamp() < cancunMilestone.get()) {
-      return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.UNSUPPORTED_FORK));
+    ValidationResult<RpcErrorType> forkValidationResult =
+        validateForkSupported(payloadAttributes.getTimestamp());
+    if (!forkValidationResult.isValid()) {
+      return Optional.of(new JsonRpcErrorResponse(requestId, forkValidationResult));
     }
 
     return Optional.empty();

diff --git a/...org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3.java b/...org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 
 import static org.hyperledger.besu.datatypes.HardforkId.MainnetHardforkId.CANCUN;
+import static org.hyperledger.besu.datatypes.HardforkId.MainnetHardforkId.PRAGUE;
 
 import org.hyperledger.besu.consensus.merge.PayloadWrapper;
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
@@ -68,6 +69,11 @@ protected JsonRpcResponse createResponse(
 
   @Override
   protected ValidationResult<RpcErrorType> validateForkSupported(final long blockTimestamp) {
-    return ForkSupportHelper.validateForkSupported(CANCUN, cancunMilestone, blockTimestamp);
+    return ForkSupportHelper.validateForkSupported(
+        CANCUN,
+        cancunMilestone,
+        PRAGUE,
+        protocolSchedule.flatMap(s -> s.milestoneFor(PRAGUE)),
+        blockTimestamp);
   }
 }
diff --git a/...org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3.java b/...org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 
 import static org.hyperledger.besu.datatypes.HardforkId.MainnetHardforkId.CANCUN;
+import static org.hyperledger.besu.datatypes.HardforkId.MainnetHardforkId.PRAGUE;
 
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
 import org.hyperledger.besu.ethereum.ProtocolContext;
@@ -89,6 +90,11 @@ protected ValidationResult<RpcErrorType> validateParameters(
 
   @Override
   protected ValidationResult<RpcErrorType> validateForkSupported(final long blockTimestamp) {
-    return ForkSupportHelper.validateForkSupported(CANCUN, cancunMilestone, blockTimestamp);
+    return ForkSupportHelper.validateForkSupported(
+        CANCUN,
+        cancunMilestone,
+        PRAGUE,
+        protocolSchedule.flatMap(s -> s.milestoneFor(PRAGUE)),
+        blockTimestamp);
   }
 }
diff --git a/.../org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/ForkSupportHelper.java b/.../org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/ForkSupportHelper.java
@@ -21,22 +21,54 @@
 import java.util.Optional;
 
 public class ForkSupportHelper {
+
   public static ValidationResult<RpcErrorType> validateForkSupported(
       final HardforkId firstSupportedHardforkId,
-      final Optional<Long> maybeForkMilestone,
+      final Optional<Long> maybeFirstSupportedForkMilestone,
       final long blockTimestamp) {
-    if (maybeForkMilestone.isEmpty()) {
+
+    if (maybeFirstSupportedForkMilestone.isEmpty()) {
       return ValidationResult.invalid(
           RpcErrorType.UNSUPPORTED_FORK,
           "Configuration error, no schedule for " + firstSupportedHardforkId.name() + " fork set");
     }
 
-    if (Long.compareUnsigned(blockTimestamp, maybeForkMilestone.get()) < 0) {
+    if (Long.compareUnsigned(blockTimestamp, maybeFirstSupportedForkMilestone.get()) < 0) {
       return ValidationResult.invalid(
           RpcErrorType.UNSUPPORTED_FORK,
           firstSupportedHardforkId.name()
               + " configured to start at timestamp: "
-              + maybeForkMilestone.get());
+              + maybeFirstSupportedForkMilestone.get());
+    }
+
+    return ValidationResult.valid();
+  }
+
+  public static ValidationResult<RpcErrorType> validateForkSupported(
+      final HardforkId firstSupportedHardforkId,
+      final Optional<Long> maybeFirstSupportedForkMilestone,
+      final HardforkId firstUnsupportedHardforkId,
+      final Optional<Long> maybeFirstUnsupportedMilestone,
+      final long blockTimestamp) {
+
+    var result =
+        validateForkSupported(
+            firstSupportedHardforkId, maybeFirstSupportedForkMilestone, blockTimestamp);
+
+    if (!result.isValid()) {
+      return result;
+    }
+
+    if (maybeFirstUnsupportedMilestone.isPresent()
+        && Long.compareUnsigned(blockTimestamp, maybeFirstUnsupportedMilestone.get()) >= 0) {
+      return ValidationResult.invalid(
+          RpcErrorType.UNSUPPORTED_FORK,
+          "block timestamp "
+              + blockTimestamp
+              + " is after the first unsupported milestone: "
+              + firstUnsupportedHardforkId.name()
+              + " at timestamp "
+              + maybeFirstUnsupportedMilestone.get());
     }
 
     return ValidationResult.valid();

diff --git a/...edger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java b/...edger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java
@@ -20,6 +20,7 @@
 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.SYNCING;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.VALID;
+import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineTestSupport.fromErrorResp;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
@@ -43,7 +44,6 @@
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.WithdrawalParameter;
 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;
@@ -429,14 +429,6 @@ protected EnginePayloadStatusResult fromSuccessResp(final JsonRpcResponse resp)
         .get();
   }
 
-  protected JsonRpcError fromErrorResp(final JsonRpcResponse resp) {
-    assertThat(resp.getType()).isEqualTo(RpcResponseType.ERROR);
-    return Optional.of(resp)
-        .map(JsonRpcErrorResponse.class::cast)
-        .map(JsonRpcErrorResponse::getError)
-        .get();
-  }
-
   protected BlockHeader createBlockHeader(final Optional<List<Withdrawal>> maybeWithdrawals) {
     return createBlockHeaderFixture(maybeWithdrawals).buildHeader();
   }

diff --git a/...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractScheduledApiTest.java b/...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractScheduledApiTest.java
@@ -44,9 +44,9 @@ public class AbstractScheduledApiTest {
   protected final ScheduledProtocolSpec.Hardfork cancunHardfork =
       new ScheduledProtocolSpec.Hardfork("Cancun", 30);
   protected final ScheduledProtocolSpec.Hardfork pragueHardfork =
-      new ScheduledProtocolSpec.Hardfork("Prague", 40);
+      new ScheduledProtocolSpec.Hardfork("Prague", 50);
   protected final ScheduledProtocolSpec.Hardfork experimentalHardfork =
-      new ScheduledProtocolSpec.Hardfork("ExperimentalEips", 50);
+      new ScheduledProtocolSpec.Hardfork("ExperimentalEips", 70);
 
   @Mock protected DefaultProtocolSchedule protocolSchedule;
 

diff --git a/...g/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetBlobsV1Test.java b/...g/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetBlobsV1Test.java
@@ -15,6 +15,7 @@
 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.engine.EngineTestSupport.fromErrorResp;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
@@ -32,7 +33,6 @@
 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.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.response.RpcErrorType;
@@ -256,12 +256,4 @@ private List<BlobAndProofV1> fromSuccessResp(final JsonRpcResponse resp) {
     list.forEach(obj -> blobAndProofV1s.add((BlobAndProofV1) obj));
     return blobAndProofV1s;
   }
-
-  private RpcErrorType fromErrorResp(final JsonRpcResponse resp) {
-    assertThat(resp.getType()).isEqualTo(RpcResponseType.ERROR);
-    return Optional.of(resp)
-        .map(JsonRpcErrorResponse.class::cast)
-        .map(JsonRpcErrorResponse::getErrorType)
-        .get();
-  }
 }
diff --git a/...besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadBodiesByHashV1Test.java b/...besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadBodiesByHashV1Test.java
@@ -15,6 +15,7 @@
 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.engine.EngineTestSupport.fromErrorResp;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.INVALID_RANGE_REQUEST_TOO_LARGE;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.spy;
@@ -29,10 +30,8 @@
 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.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.response.RpcErrorType;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.BlockResultFactory;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EngineGetPayloadBodiesResultV1;
 import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
@@ -259,12 +258,4 @@ private EngineGetPayloadBodiesResultV1 fromSuccessResp(final JsonRpcResponse res
         .map(EngineGetPayloadBodiesResultV1.class::cast)
         .get();
   }
-
-  private RpcErrorType fromErrorResp(final JsonRpcResponse resp) {
-    assertThat(resp.getType()).isEqualTo(RpcResponseType.ERROR);
-    return Optional.of(resp)
-        .map(JsonRpcErrorResponse.class::cast)
-        .map(JsonRpcErrorResponse::getErrorType)
-        .get();
-  }
 }
diff --git a/...hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3Test.java b/...hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3Test.java
@@ -14,7 +14,10 @@
  */
 package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;
 
+import static java.util.Collections.emptyList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineTestSupport.fromErrorResp;
+import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.UNSUPPORTED_FORK;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -30,7 +33,10 @@
 import org.hyperledger.besu.datatypes.TransactionType;
 import org.hyperledger.besu.datatypes.Wei;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
+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.JsonRpcSuccessResponse;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EngineGetPayloadResultV3;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.Quantity;
 import org.hyperledger.besu.ethereum.core.BlobTestFixture;
@@ -86,6 +92,76 @@ public void shouldReturnExpectedMethodName() {
     assertThat(method.getName()).isEqualTo("engine_getPayloadV3");
   }
 
+  @Test
+  public void shouldReturnUnsupportedForkIfBlockTimestampIsBeforeCancunMilestone() {
+    BlockHeader shanghaiHeader =
+        new BlockHeaderTestFixture()
+            .prevRandao(Bytes32.random())
+            .timestamp(cancunHardfork.milestone() - 1)
+            .excessBlobGas(BlobGas.of(10L))
+            .buildHeader();
+
+    PayloadIdentifier shanghaiPid =
+        PayloadIdentifier.forPayloadParams(
+            Hash.ZERO,
+            cancunHardfork.milestone() - 1,
+            Bytes32.random(),
+            Address.fromHexString("0x42"),
+            Optional.empty(),
+            Optional.empty());
+
+    BlockWithReceipts shanghaiBlock =
+        new BlockWithReceipts(
+            new Block(
+                shanghaiHeader, new BlockBody(emptyList(), emptyList(), Optional.of(emptyList()))),
+            emptyList());
+    PayloadWrapper payloadShanghai =
+        new PayloadWrapper(shanghaiPid, shanghaiBlock, Optional.empty());
+
+    when(mergeContext.retrievePayloadById(shanghaiPid)).thenReturn(Optional.of(payloadShanghai));
+
+    final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V3.getMethodName(), shanghaiPid);
+
+    assertThat(resp).isInstanceOf(JsonRpcErrorResponse.class);
+    assertThat(((JsonRpcErrorResponse) resp).getErrorType())
+        .isEqualTo(RpcErrorType.UNSUPPORTED_FORK);
+  }
+
+  @Test
+  public void shouldReturnUnsupportedForkIfBlockTimestampIsAfterPragueMilestone() {
+    BlockHeader pragueHeader =
+        new BlockHeaderTestFixture()
+            .prevRandao(Bytes32.random())
+            .timestamp(pragueHardfork.milestone())
+            .excessBlobGas(BlobGas.of(10L))
+            .buildHeader();
+
+    PayloadIdentifier postCancunPid =
+        PayloadIdentifier.forPayloadParams(
+            Hash.ZERO,
+            cancunHardfork.milestone(),
+            Bytes32.random(),
+            Address.fromHexString("0x42"),
+            Optional.empty(),
+            Optional.empty());
+
+    BlockWithReceipts pragueBlock =
+        new BlockWithReceipts(
+            new Block(
+                pragueHeader, new BlockBody(emptyList(), emptyList(), Optional.of(emptyList()))),
+            emptyList());
+    PayloadWrapper payloadPostCancun =
+        new PayloadWrapper(postCancunPid, pragueBlock, Optional.empty());
+
+    when(mergeContext.retrievePayloadById(postCancunPid))
+        .thenReturn(Optional.of(payloadPostCancun));
+
+    final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V3.getMethodName(), postCancunPid);
+    final JsonRpcError jsonRpcError = fromErrorResp(resp);
+    assertThat(jsonRpcError.getCode()).isEqualTo(UNSUPPORTED_FORK.getCode());
+    verify(engineCallListener, times(1)).executionEngineCalled();
+  }
+
   @Override
   @Test
   public void shouldReturnBlockForKnownPayloadId() {