check more of the parameters and combine get and validate into one me…

…thod Signed-off-by: Stefan <stefan.pingel@consensys.net>
Gabriel-Trintinalia · Aug 31, 2023 · 62b368d · 62b368d
1 parent 1141b7f
commit 62b368d
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 29 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,20 +105,13 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
 
     EngineNewPayloadRequestParameter requestParameters;
     try {
-      requestParameters = getEngineNewPayloadRequestParams(requestContext);
+      requestParameters = getAndCheckEngineNewPayloadRequestParams(requestContext);
     } catch (InvalidJsonRpcParameters exception) {
       return new JsonRpcErrorResponse(
           requestContext.getRequest().getId(),
           new JsonRpcError(INVALID_PARAMS, exception.getMessage()));
     }
 
-    final ValidationResult<RpcErrorType> parameterValidationResult =
-        validateParameters(requestParameters);
-
-    if (!parameterValidationResult.isValid()) {
-      return new JsonRpcErrorResponse(reqId, parameterValidationResult);
-    }
-
     EngineExecutionPayloadParameter blockParam = requestParameters.getExecutionPayload();
     final ValidationResult<RpcErrorType> forkValidationResult =
         validateForkSupported(blockParam.getTimestamp());
@@ -481,7 +474,7 @@ private Optional<List<VersionedHash>> extractVersionedHashes(
    * @param requestContext the JSON-RPC request context
    * @return a new instance of EngineNewPayloadRequestParameter
    */
-  public abstract EngineNewPayloadRequestParameter getEngineNewPayloadRequestParams(
+  public abstract EngineNewPayloadRequestParameter getAndCheckEngineNewPayloadRequestParams(
       final JsonRpcRequestContext requestContext);
 
   private void logImportedBlockInfo(final Block block, final double timeInS) {

diff --git a/...org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV1.java b/...org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV1.java
@@ -21,6 +21,7 @@
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.exception.InvalidJsonRpcParameters;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EngineExecutionPayloadParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EngineNewPayloadRequestParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
@@ -64,10 +65,26 @@ protected EngineStatus getInvalidBlockHashStatus() {
   }
 
   @Override
-  public EngineNewPayloadRequestParameter getEngineNewPayloadRequestParams(
+  public EngineNewPayloadRequestParameter getAndCheckEngineNewPayloadRequestParams(
       final JsonRpcRequestContext requestContext) {
-    return new EngineNewPayloadRequestParameter(
-        requestContext.getRequiredParameter(0, EngineExecutionPayloadParameter.class));
+    final EngineExecutionPayloadParameter enginePayload =
+        requestContext.getRequiredParameter(0, EngineExecutionPayloadParameter.class);
+    if (enginePayload.getWithdrawals() != null) {
+      throw new InvalidJsonRpcParameters("non-null Withdrawals pre-shanghai");
+    }
+    if (enginePayload.getBlobGasUsed() != null) {
+      throw new InvalidJsonRpcParameters("non-null BlobGasUsed pre-cancun");
+    }
+    if (enginePayload.getExcessBlobGas() != null) {
+      throw new InvalidJsonRpcParameters("non-null ExcessBlobGas pre-cancun");
+    }
+    if (requestContext.getOptionalParameter(1, String[].class).isPresent()) {
+      throw new InvalidJsonRpcParameters("non-null VersionedHashes pre-cancun");
+    }
+    if (requestContext.getOptionalParameter(2, String.class).isPresent()) {
+      throw new InvalidJsonRpcParameters("non-null ParentBeaconBlockRoot pre-cancun");
+    }
+    return new EngineNewPayloadRequestParameter(enginePayload);
   }
 
   @Override

diff --git a/...org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2.java b/...org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2.java
@@ -19,6 +19,7 @@
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.exception.InvalidJsonRpcParameters;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EngineExecutionPayloadParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EngineNewPayloadRequestParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
@@ -52,24 +53,23 @@ public String getName() {
   }
 
   @Override
-  protected ValidationResult<RpcErrorType> validateParameters(
-      final EngineNewPayloadRequestParameter params) {
-    if (params.getExecutionPayload().getBlobGasUsed() != null) {
-      return ValidationResult.invalid(
-          RpcErrorType.INVALID_PARAMS, "non-null BlobGasUsed pre-cancun");
+  public EngineNewPayloadRequestParameter getAndCheckEngineNewPayloadRequestParams(
+      final JsonRpcRequestContext requestContext) {
+    final EngineExecutionPayloadParameter enginePayload =
+        requestContext.getRequiredParameter(0, EngineExecutionPayloadParameter.class);
+    if (enginePayload.getBlobGasUsed() != null) {
+      throw new InvalidJsonRpcParameters("non-null BlobGasUsed pre-cancun");
     }
-    if (params.getExecutionPayload().getExcessBlobGas() != null) {
-      return ValidationResult.invalid(
-          RpcErrorType.INVALID_PARAMS, "non-null ExcessBlobGas pre-cancun");
+    if (enginePayload.getExcessBlobGas() != null) {
+      throw new InvalidJsonRpcParameters("non-null ExcessBlobGas pre-cancun");
     }
-    return ValidationResult.valid();
-  }
-
-  @Override
-  public EngineNewPayloadRequestParameter getEngineNewPayloadRequestParams(
-      final JsonRpcRequestContext requestContext) {
-    return new EngineNewPayloadRequestParameter(
-        requestContext.getRequiredParameter(0, EngineExecutionPayloadParameter.class));
+    if (requestContext.getOptionalParameter(1, String[].class).isPresent()) {
+      throw new InvalidJsonRpcParameters("non-null VersionedHashes pre-cancun");
+    }
+    if (requestContext.getOptionalParameter(2, String.class).isPresent()) {
+      throw new InvalidJsonRpcParameters("non-null ParentBeaconBlockRoot pre-cancun");
+    }
+    return new EngineNewPayloadRequestParameter(enginePayload);
   }
 
   @Override

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
@@ -18,6 +18,7 @@
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.exception.InvalidJsonRpcParameters;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EngineExecutionPayloadParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EngineNewPayloadRequestParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
@@ -59,7 +60,7 @@ public String getName() {
    * @return a new instance of EngineNewPayloadRequestParameter
    */
   @Override
-  public EngineNewPayloadRequestParameter getEngineNewPayloadRequestParams(
+  public EngineNewPayloadRequestParameter getAndCheckEngineNewPayloadRequestParams(
       final JsonRpcRequestContext requestContext) {
 
     final EngineExecutionPayloadParameter payload =
@@ -68,6 +69,10 @@ public EngineNewPayloadRequestParameter getEngineNewPayloadRequestParams(
 
     final String parentBeaconBlockRoot = requestContext.getRequiredParameter(2, String.class);
 
+    if (payload.getBlobGasUsed() == null || payload.getExcessBlobGas() == null) {
+      throw new InvalidJsonRpcParameters("Missing blob gas fields");
+    }
+
     return new EngineNewPayloadRequestParameter(
         payload,
         Optional.of(Arrays.stream(versionedHashes).toList()),