Use Invalid payload attributes error code (hyperledger#6582)

* invalid payload error code * use non-zero timestamp * use invalidPayloadAttributes error for V2 if non-null beacon root pre cancun Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> --------- Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com> Signed-off-by: amsmota <antonio.mota@citi.com>
Citi · Apr 16, 2024 · bd56ef3 · bd56ef3
1 parent 326979b
commit bd56ef3
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 10 deletions.
diff --git a/...nrpc/engine/shanghai/test-cases/05_shanghai_prepare_payload_invalid_null_withdrawals.json b/...nrpc/engine/shanghai/test-cases/05_shanghai_prepare_payload_invalid_null_withdrawals.json
@@ -20,8 +20,8 @@
     "jsonrpc" : "2.0",
     "id" : 67,
     "error" : {
-      "code" : -32602,
-      "message" : "Invalid params"
+      "code" : -38003,
+      "message" : "Invalid payload attributes"
     }
   },
   "statusCode" : 200

diff --git a/...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java b/...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java
@@ -56,14 +56,21 @@ protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
           || payloadAttributes.getParentBeaconBlockRoot().isEmpty()) {
         return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.UNSUPPORTED_FORK));
       } else {
-        return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_PARAMS));
+        return Optional.of(
+            new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_PAYLOAD_ATTRIBUTES));
       }
     } else if (payloadAttributes.getParentBeaconBlockRoot() != null) {
       LOG.error(
           "Parent beacon block root hash present in payload attributes before cancun hardfork");
-      return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_PARAMS));
+      return Optional.of(
+          new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_PAYLOAD_ATTRIBUTES));
     } else {
       return Optional.empty();
     }
   }
+
+  @Override
+  protected RpcErrorType getInvalidPayloadError() {
+    return RpcErrorType.INVALID_PAYLOAD_ATTRIBUTES;
+  }
 }
diff --git a/...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java b/...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java
@@ -56,16 +56,16 @@ protected ValidationResult<RpcErrorType> validateParameter(
       final EngineForkchoiceUpdatedParameter fcuParameter,
       final Optional<EnginePayloadAttributesParameter> maybePayloadAttributes) {
     if (fcuParameter.getHeadBlockHash() == null) {
-      return ValidationResult.invalid(RpcErrorType.INVALID_PARAMS, "Missing head block hash");
+      return ValidationResult.invalid(getInvalidPayloadError(), "Missing head block hash");
     } else if (fcuParameter.getSafeBlockHash() == null) {
-      return ValidationResult.invalid(RpcErrorType.INVALID_PARAMS, "Missing safe block hash");
+      return ValidationResult.invalid(getInvalidPayloadError(), "Missing safe block hash");
     } else if (fcuParameter.getFinalizedBlockHash() == null) {
-      return ValidationResult.invalid(RpcErrorType.INVALID_PARAMS, "Missing finalized block hash");
+      return ValidationResult.invalid(getInvalidPayloadError(), "Missing finalized block hash");
     }
     if (maybePayloadAttributes.isPresent()) {
       if (maybePayloadAttributes.get().getParentBeaconBlockRoot() == null) {
         return ValidationResult.invalid(
-            RpcErrorType.INVALID_PARAMS, "Missing parent beacon block root hash");
+            getInvalidPayloadError(), "Missing parent beacon block root hash");
       }
     }
     return ValidationResult.valid();
@@ -93,11 +93,18 @@ protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
     if (payloadAttributes.getParentBeaconBlockRoot() == null) {
       LOG.error(
           "Parent beacon block root hash not present in payload attributes after cancun hardfork");
-      return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_PARAMS));
+      return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadError()));
+    } else if (payloadAttributes.getTimestamp().longValue() == 0) {
+      return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadError()));
     } else if (payloadAttributes.getTimestamp() < cancun.get().milestone()) {
       return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.UNSUPPORTED_FORK));
     } else {
       return Optional.empty();
     }
   }
+
+  @Override
+  protected RpcErrorType getInvalidPayloadError() {
+    return RpcErrorType.INVALID_PAYLOAD_ATTRIBUTES;
+  }
 }
diff --git a/...esu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java b/...esu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java
@@ -454,7 +454,7 @@ public void shouldIgnoreUpdateToOldHeadAndNotPreparePayload() {
 
   @Test
   public void shouldReturnInvalidIfPayloadTimestampNotGreaterThanHead() {
-    BlockHeader mockParent = blockHeaderBuilder.number(9L).buildHeader();
+    BlockHeader mockParent = blockHeaderBuilder.timestamp(99).number(9L).buildHeader();
     BlockHeader mockHeader =
         blockHeaderBuilder.number(10L).parentHash(mockParent.getHash()).buildHeader();
     setupValidForkchoiceUpdate(mockHeader);

diff --git a/...dger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2Test.java b/...dger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2Test.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.RpcErrorType;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -42,4 +43,9 @@ public void shouldReturnExpectedMethodName() {
   protected String getMethodName() {
     return RpcMethod.ENGINE_FORKCHOICE_UPDATED_V2.getMethodName();
   }
+
+  @Override
+  protected RpcErrorType expectedInvalidPayloadError() {
+    return RpcErrorType.INVALID_PAYLOAD_ATTRIBUTES;
+  }
 }