fix: prioritize fork error over missing payload fields (hyperledger#5765

) additional rpc error code for unsupported fork fixes hyperledger#5738 Signed-off-by: Sally MacFarlane <[email protected]>
shemnon · Aug 17, 2023 · 210927e · 210927e
1 parent 5a766b4
commit 210927e
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 10 deletions.
diff --git a/...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java b/...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java
@@ -105,11 +105,18 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     final EnginePayloadParameter blockParam =
         requestContext.getRequiredParameter(0, EnginePayloadParameter.class);
 
-    Optional<List<String>> maybeVersionedHashParam =
+    final Optional<List<String>> maybeVersionedHashParam =
         requestContext.getOptionalList(1, String.class);
 
-    Object reqId = requestContext.getRequest().getId();
-    Optional<List<VersionedHash>> maybeVersionedHashes;
+    final Object reqId = requestContext.getRequest().getId();
+
+    final ValidationResult<RpcErrorType> forkValidationResult =
+        validateForkSupported(reqId, blockParam);
+    if (!forkValidationResult.isValid()) {
+      return new JsonRpcErrorResponse(reqId, forkValidationResult);
+    }
+
+    final Optional<List<VersionedHash>> maybeVersionedHashes;
     try {
       maybeVersionedHashes = extractVersionedHashes(maybeVersionedHashParam);
     } catch (RuntimeException ex) {
@@ -124,11 +131,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
         .addArgument(() -> Json.encodePrettily(blockParam))
         .log();
 
-    ValidationResult<RpcErrorType> forkValidationResult = validateForkSupported(reqId, blockParam);
-    if (!forkValidationResult.isValid()) {
-      return new JsonRpcErrorResponse(reqId, forkValidationResult);
-    }
-
     final Optional<List<Withdrawal>> maybeWithdrawals =
         Optional.ofNullable(blockParam.getWithdrawals())
             .map(ws -> ws.stream().map(WithdrawalParameter::toWithdrawal).collect(toList()));

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
@@ -54,12 +54,12 @@ protected ValidationResult<RpcErrorType> validateForkSupported(
     if (cancun.isPresent() && payloadParameter.getTimestamp() >= cancun.get().milestone()) {
       if (payloadParameter.getDataGasUsed() == null
           || payloadParameter.getExcessDataGas() == null) {
-        return ValidationResult.invalid(RpcErrorType.INVALID_PARAMS, "Missing data gas fields");
+        return ValidationResult.invalid(RpcErrorType.INVALID_PARAMS, "Missing blob gas fields");
       } else {
         return ValidationResult.valid();
       }
     } else {
-      return ValidationResult.invalid(RpcErrorType.INVALID_PARAMS, "Fork not supported");
+      return ValidationResult.invalid(RpcErrorType.UNSUPPORTED_FORK, "Fork not supported");
     }
   }
 }
diff --git a/...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java b/...c/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/response/RpcErrorType.java
@@ -81,6 +81,7 @@ public enum RpcErrorType {
   INVALID_FORKCHOICE_STATE(-38002, "Invalid forkchoice state"),
   INVALID_PAYLOAD_ATTRIBUTES(-38003, "Invalid payload attributes"),
   INVALID_RANGE_REQUEST_TOO_LARGE(-38004, "Too large request"),
+  UNSUPPORTED_FORK(-38005, "Unsupported fork"),
   // Miner failures
   COINBASE_NOT_SET(-32010, "Coinbase not set. Unable to start mining without a coinbase"),
   NO_HASHES_PER_SECOND(-32011, "No hashes being generated by the current node"),

diff --git a/...hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3Test.java b/...hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3Test.java
@@ -73,7 +73,12 @@ public void before() {
   @Test
   public void shouldInvalidPayloadOnShortVersionedHash() {
     Bytes shortHash = Bytes.fromHexString("0x" + "69".repeat(31));
+
     EnginePayloadParameter payload = mock(EnginePayloadParameter.class);
+    when(payload.getTimestamp()).thenReturn(30l);
+    when(payload.getExcessDataGas()).thenReturn("99");
+    when(payload.getDataGasUsed()).thenReturn(9l);
+
     JsonRpcResponse badParam =
         method.response(
             new JsonRpcRequestContext(