hyperledger · macfarla · May 27, 2025 · May 27, 2025 · May 27, 2025 · May 27, 2025
diff --git a/...s/jsonrpc/engine/prague/test-cases/17_prague_newPayloadV4_invalid_duplicate_requests.json b/...s/jsonrpc/engine/prague/test-cases/17_prague_newPayloadV4_invalid_duplicate_requests.json
@@ -0,0 +1,63 @@
+{
+  "request": {
+    "jsonrpc": "2.0",
+    "method": "engine_newPayloadV4",
+    "params": [
+      {
+        "parentHash": "0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a",
+        "feeRecipient": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b",
+        "stateRoot": "0xca3149fa9e37db08d1cd49c9061db1002ef1cd58db2210f2115c8c989b2bdf45",
+        "receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
+        "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
+        "prevRandao": "0xc130d5e63c61c935f6089e61140ca9136172677cf6aa5800dcc1cf0a02152a14",
+        "blockNumber": "0x112720f",
+        "gasLimit": "0x1c9c380",
+        "gasUsed": "0xbad2e8",
+        "timestamp": "0x64e7785b",
+        "extraData": "0x",
+        "baseFeePerGas": "0x7",
+        "blockHash": "0x3559e851470f6e7bbed1db474980683e8c315bfce99b2a6ef47c057c04de7858",
+        "transactions": [
+          "0x03f88f0780843b9aca008506fc23ac00830186a09400000000000000000000000000000000000001008080c001e1a0010657f37554c781402a22917dee2f75def7ab966d7b770905398eba3c44401401a0840650aa8f74d2b07f40067dc33b715078d73422f01da17abdbd11e02bbdfda9a04b2260f6022bf53eadb337b3e59514936f7317d872defb891a708ee279bdca90",
+          "0x03f88f0701843b9aca008506fc23ac00830186a09400000000000000000000000000000000000001008080c001e1a001521d528ad0c760354a4f0496776cf14a92fe1fb5d50e959dcea1a489c7c83101a0a86c1fd8c2e74820686937f5c1bfe836e2fb622ac9fcbebdc4ab4357f2dbbc61a05c3b2b44ff8252f78d70aeb33f8ba09beaeadad1b376a57d34fa720bbc4a18ee",
+          "0x03f88f0702843b9aca008506fc23ac00830186a09400000000000000000000000000000000000001008080c001e1a001453362c360fdd8832e3539d463e6d64b2ee320ac6a08885df6083644a063e701a037a728aec08aefffa702a2ca620db89caf3e46ab7f25f7646fc951510991badca065d846f046357af39bb739b161233fce73ddfe0bb87f2d28ef60dfe6dbb0128d"
+        ],
+        "withdrawals": [
+          {
+            "index": "0xf0",
+            "validatorIndex": "0xf0",
+            "address": "0x00000000000000000000000000000000000010f0",
+            "amount": "0x1"
+          },
+          {
+            "index": "0xf1",
+            "validatorIndex": "0xf1",
+            "address": "0x00000000000000000000000000000000000010f1",
+            "amount": "0x1"
+          }
+        ],
+        "blobGasUsed": "0x0",
+        "excessBlobGas": "0x0"
+      },
+      [
+        "0x0100000000000000000000000000000000000000000000000000000000000000"
+      ],
+      "0x0100000000000000000000000000000000000000000000000000000000000000",
+      [
+        "0x96a96086cff07df17668f35f7418ef8798079167e3f4f9b72ecde17b28226137cf454ab1dd20ef5d924786ab3483c2f9003f5102dabe0a27b1746098d1dc17a5d3fbd478759fea9287e4e419b3c3cef20100000000000000b1acdb2c4d3df3f1b8d3bfd33421660df358d84d78d16c4603551935f4b67643373e7eb63dcb16ec359be0ec41fee33b03a16e80745f2374ff1d3c352508ac5d857c6476d3c3bcf7e6ca37427c9209f17be3af5264c0e2132b3dd1156c28b4e9f000000000000000a5c85a60ba2905c215f6a12872e62b1ee037051364244043a5f639aa81b04a204c55e7cc851f29c7c183be253ea1510b001db70c485b6264692f26b8aeaab5b0c384180df8e2184a21a808a3ec8e86ca01000000000000009561731785b48cf1886412234531e4940064584463e96ac63a1a154320227e333fb51addc4a89b7e0d3f862d7c1fd4ea03bd8eb3d8806f1e7daf591cbbbb92b0beb74d13c01617f22c5026b4f9f9f294a8a7c32db895de3b01bee0132c9209e1f100000000000000",
+        "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b85103a5617937691dfeeb89b86a80d5dc9e3c9d3a1a0e7ce311e26e0bb732eabaa47ffa288f0d54de28209a62a7d29d0000000000000000000000000000000000000000000000000000010f698daeed734da114470da559bd4b4c7259e1f7952555241dcbc90cf194a2ef676fc6005f3672fada2a3645edb297a75530100000000000000",
+        "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b85103a5617937691dfeeb89b86a80d5dc9e3c9d3a1a0e7ce311e26e0bb732eabaa47ffa288f0d54de28209a62a7d29d098daeed734da114470da559bd4b4c7259e1f7952555241dcbc90cf194a2ef676fc6005f3672fada2a3645edb297a7553"
+      ]
+    ],
+    "id": 67
+  },
+  "response": {
+    "jsonrpc": "2.0",
+    "id": 67,
+    "error": {
+      "code": -32602,
+      "message": "Invalid engine params"
+    }
+  },
+  "statusCode": 200
+}
diff --git a/...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java b/...perledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java
@@ -173,7 +173,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     try {
       maybeVersionedHashes = extractVersionedHashes(maybeVersionedHashParam);
     } catch (RuntimeException ex) {
-      return respondWithInvalid(
+      return respondWithError(
           reqId,
           blockParam,
           mergeCoordinator.getLatestValidAncestor(blockParam.getParentHash()).orElse(null),
@@ -203,7 +203,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     try {
       maybeRequests = extractRequests(maybeRequestsParam);
     } catch (RequestType.InvalidRequestTypeException ex) {
-      return respondWithInvalid(
+      return respondWithError(
           reqId,
           blockParam,
           mergeCoordinator.getLatestValidAncestor(blockParam.getParentHash()).orElse(null),
@@ -285,6 +285,15 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
               "Computed block hash %s does not match block hash parameter %s",
               newBlockHeader.getBlockHash(), blockParam.getBlockHash());
       LOG.debug(errorMessage);
+      // If the block is invalid, we want to return INVALID
+      // from the spec:
+      // https://github.com/ethereum/execution-apis/blob/main/src/engine/prague.md#engine_newpayloadv4
+      // Given the executionRequests, client software MUST compute the execution requests commitment
+      // and incorporate it into the blockHash validation process.
+      // That is, if the computed commitment does not match the corresponding commitment in the
+      // execution layer block header,
+      // the call MUST return {status: INVALID, latestValidHash: null, validationError: errorMessage
+      // | null}.
       return respondWithInvalid(reqId, blockParam, null, getInvalidBlockHashStatus(), errorMessage);
     }
 
@@ -299,7 +308,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
             maybeVersionedHashes,
             protocolSchedule.get().getByBlockHeader(newBlockHeader));
     if (!blobValidationResult.isValid()) {
-      return respondWithInvalid(
+      return respondWithError(
           reqId,
           blockParam,
           mergeCoordinator.getLatestValidAncestor(blockParam.getParentHash()).orElse(null),
@@ -326,7 +335,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     if (maybeParentHeader.isPresent()
         && (Long.compareUnsigned(maybeParentHeader.get().getTimestamp(), blockParam.getTimestamp())
             >= 0)) {
-      return respondWithInvalid(
+      return respondWithError(
           reqId,
           blockParam,
           mergeCoordinator.getLatestValidAncestor(blockParam.getParentHash()).orElse(null),
@@ -373,8 +382,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
         Throwable causedBy = executionResult.causedBy().get();
         if (causedBy instanceof StorageException || causedBy instanceof MerkleTrieException) {
           RpcErrorType error = RpcErrorType.INTERNAL_ERROR;
-          JsonRpcErrorResponse response = new JsonRpcErrorResponse(reqId, error);
-          return response;
+          return new JsonRpcErrorResponse(reqId, error);
         }
       }
       LOG.debug("New payload is invalid: {}", executionResult.errorMessage.get());
@@ -456,7 +464,7 @@ JsonRpcResponse respondWith(
   JsonRpcResponse respondWithInvalid(
       final Object requestId,
       final EnginePayloadParameter param,
-      final Hash latestValidHash,
+      final Hash latestValidHash, // this should be null for invalid
       final EngineStatus invalidStatus,
       final String validationError) {
     if (!INVALID.equals(invalidStatus) && !INVALID_BLOCK_HASH.equals(invalidStatus)) {
@@ -485,6 +493,36 @@ JsonRpcResponse respondWithInvalid(
             invalidStatus, latestValidHash, Optional.of(validationError)));
   }
 
+  JsonRpcResponse respondWithError(
+      final Object requestId,
+      final EnginePayloadParameter param,
+      final Hash latestValidHash,
+      final EngineStatus invalidStatus,
+      final String validationError) {
+    if (!INVALID.equals(invalidStatus) && !INVALID_BLOCK_HASH.equals(invalidStatus)) {
+      throw new IllegalArgumentException(
+          "Don't call respondWithError() with non-invalid status of " + invalidStatus.toString());
+    }
+    final String invalidBlockLogMessage =
+        String.format(
+            "Invalid new payload: number: %s, hash: %s, parentHash: %s, latestValidHash: %s, status: %s, validationError: %s",
+            param.getBlockNumber(),
+            param.getBlockHash(),
+            param.getParentHash(),
+            latestValidHash == null ? null : latestValidHash.toHexString(),
+            invalidStatus.name(),
+            validationError);
+    // always log invalid at DEBUG
+    LOG.debug(invalidBlockLogMessage);
+    // periodically log at WARN
+    if (lastInvalidWarn + ENGINE_API_LOGGING_THRESHOLD < System.currentTimeMillis()) {
+      lastInvalidWarn = System.currentTimeMillis();
+      LOG.warn(invalidBlockLogMessage);
+    }
+    // return ErrorResponse
+    return new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_ENGINE_PARAMS);
+  }
+
   protected EngineStatus getInvalidBlockHashStatus() {
     return INVALID;
   }

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
@@ -61,6 +61,7 @@ public enum RpcErrorType implements RpcMethodError {
   INVALID_EXCESS_BLOB_GAS_PARAMS(
       INVALID_PARAMS_ERROR_CODE, "Invalid excess blob gas params (missing or invalid)"),
   INVALID_EXECUTION_REQUESTS_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid execution requests params"),
+  INVALID_ENGINE_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid engine params"),
   INVALID_EXTRA_DATA_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid extra data params"),
   INVALID_FILTER_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid filter params"),
   INVALID_HASH_RATE_PARAMS(INVALID_PARAMS_ERROR_CODE, "Invalid hash rate params"),

diff --git a/...hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3Test.java b/...hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3Test.java
@@ -16,9 +16,10 @@
 
 import static java.util.Collections.emptyList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.hyperledger.besu.ethereum.api.graphql.internal.response.GraphQLError.INVALID_PARAMS;
 import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID;
 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_PARAMS;
+import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.INVALID_ENGINE_PARAMS;
 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;
@@ -176,9 +177,12 @@ public void shouldInvalidVersionedHash_whenShortVersionedHash() {
                       List.of(shortHash.toHexString()),
                       "0x0000000000000000000000000000000000000000000000000000000000000000"
                     })));
-    final EnginePayloadStatusResult res = fromSuccessResp(badParam);
-    assertThat(res.getStatusAsString()).isEqualTo(INVALID.name());
-    assertThat(res.getError()).isEqualTo("Invalid versionedHash");
+
+    // Should return an error response with INVALID_EXECUTION_REQUESTS_PARAMS
+    final JsonRpcError jsonRpcError = fromErrorResp(badParam);
+    assertThat(jsonRpcError.getCode()).isEqualTo(INVALID_PARAMS.getCode());
+    assertThat(jsonRpcError.getMessage()).isEqualTo(INVALID_ENGINE_PARAMS.getMessage());
+    verify(engineCallListener, times(1)).executionEngineCalled();
   }
 
   @Test
@@ -227,19 +231,16 @@ protected BlockHeader createBlockHeader(final Optional<List<Withdrawal>> maybeWi
     when(blockchain.getBlockHeader(parentBlockHeader.getBlockHash()))
         .thenReturn(Optional.of(parentBlockHeader));
     when(protocolContext.getBlockchain()).thenReturn(blockchain);
-    BlockHeader mockHeader =
-        new BlockHeaderTestFixture()
-            .baseFeePerGas(Wei.ONE)
-            .parentHash(parentBlockHeader.getParentHash())
-            .number(parentBlockHeader.getNumber() + 1)
-            .timestamp(parentBlockHeader.getTimestamp() + 12)
-            .withdrawalsRoot(maybeWithdrawals.map(BodyValidation::withdrawalsRoot).orElse(null))
-            .excessBlobGas(BlobGas.ZERO)
-            .blobGasUsed(0L)
-            .parentBeaconBlockRoot(
-                maybeParentBeaconBlockRoot.isPresent() ? maybeParentBeaconBlockRoot : null)
-            .buildHeader();
-    return mockHeader;
+    return new BlockHeaderTestFixture()
+        .baseFeePerGas(Wei.ONE)
+        .parentHash(parentBlockHeader.getParentHash())
+        .number(parentBlockHeader.getNumber() + 1)
+        .timestamp(parentBlockHeader.getTimestamp() + 12)
+        .withdrawalsRoot(maybeWithdrawals.map(BodyValidation::withdrawalsRoot).orElse(null))
+        .excessBlobGas(BlobGas.ZERO)
+        .blobGasUsed(0L)
+        .parentBeaconBlockRoot(maybeParentBeaconBlockRoot)
+        .buildHeader();
   }
 
   @Override

diff --git a/...hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV4Test.java b/...hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV4Test.java
@@ -78,6 +78,11 @@ protected Set<ScheduledProtocolSpec.Hardfork> supportedHardforks() {
           new Request(RequestType.WITHDRAWAL, Bytes.of(1)),
           new Request(RequestType.CONSOLIDATION, Bytes.of(1)));
 
+  private static final List<Request> INVALID_REQUESTS_DUPLICATES =
+      List.of(
+          new Request(RequestType.CONSOLIDATION, Bytes.of(1)),
+          new Request(RequestType.CONSOLIDATION, Bytes.of(1)));
+
   @BeforeEach
   @Override
   public void before() {
@@ -130,6 +135,27 @@ public void shouldReturnInvalidIfRequestsIsNull_WhenRequestsAllowed() {
     verify(engineCallListener, times(1)).executionEngineCalled();
   }
 
+  @Test
+  public void shouldReturnInvalidIfRequestsInvalidWithDuplicates() {
+    BlockHeader mockHeader =
+        setupValidPayload(
+            new BlockProcessingResult(
+                Optional.of(
+                    new BlockProcessingOutputs(
+                        null, List.of(), Optional.of(INVALID_REQUESTS_DUPLICATES)))),
+            Optional.empty());
+    when(blockchain.getBlockHeader(mockHeader.getParentHash()))
+        .thenReturn(Optional.of(mock(BlockHeader.class)));
+    when(mergeCoordinator.getLatestValidAncestor(mockHeader))
+        .thenReturn(Optional.of(mockHeader.getHash()));
+    var resp = respWithInvalidDuplicateRequests(mockEnginePayload(mockHeader, emptyList()));
+
+    assertThat(fromErrorResp(resp).getCode()).isEqualTo(INVALID_PARAMS.getCode());
+    assertThat(fromErrorResp(resp).getMessage())
+        .isEqualTo(INVALID_EXECUTION_REQUESTS_PARAMS.getMessage());
+    verify(engineCallListener, times(1)).executionEngineCalled();
+  }
+
   @Test
   public void shouldReturnValidIfRequestsIsNotNull_WhenRequestsAllowed() {
     BlockHeader mockHeader =
@@ -264,6 +290,26 @@ protected JsonRpcResponse respWithInvalidRequests(final EnginePayloadParameter p
         new JsonRpcRequestContext(new JsonRpcRequest("2.0", this.method.getName(), params)));
   }
 
+  protected JsonRpcResponse respWithInvalidDuplicateRequests(final EnginePayloadParameter payload) {
+    final List<String> requestsWithoutRequestId =
+        INVALID_REQUESTS_DUPLICATES.stream() // don't sort them
+            .map(
+                r ->
+                    Bytes.concatenate(Bytes.of(r.getType().getSerializedType()), r.getData())
+                        .toHexString())
+            .toList();
+    Object[] params =
+        maybeParentBeaconBlockRoot
+            .map(
+                bytes32 ->
+                    new Object[] {
+                      payload, emptyList(), bytes32.toHexString(), requestsWithoutRequestId
+                    })
+            .orElseGet(() -> new Object[] {payload});
+    return method.response(
+        new JsonRpcRequestContext(new JsonRpcRequest("2.0", this.method.getName(), params)));
+  }
+
   private void mockProhibitedRequestsValidator() {
     var validator = new ProhibitedRequestValidator();
     when(protocolSpec.getRequestsValidator()).thenReturn(validator);