EngineNewPayloadV2 - Reject non-null Blob fields (#5802)

* Validate parameters pre cancun

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

* Add unit tests

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

* Fix unit test

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

* Fix unit tests

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

* Simplify Tests

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

* Update ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

* Update ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

* Update ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

* Update ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java

Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>

---------

Signed-off-by: Gabriel-Trintinalia <gabriel.trintinalia@consensys.net>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
Co-authored-by: Stefan Pingel <16143240+pinges@users.noreply.github.com>

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2.java

@@ -18,6 +18,7 @@ import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator
 import org.hyperledger.besu.datatypes.VersionedHash;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadParameter;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
 import org.hyperledger.besu.ethereum.core.BlockHeader;
 import org.hyperledger.besu.ethereum.core.Transaction;
@@ -48,6 +49,22 @@ public class EngineNewPayloadV2 extends AbstractEngineNewPayload {
     return RpcMethod.ENGINE_NEW_PAYLOAD_V2.getMethodName();
   }
 
+  @Override
+  protected ValidationResult<RpcErrorType> validateParameters(
+      final EnginePayloadParameter payloadParameter,
+      final Optional<List<String>> maybeVersionedHashParam,
+      final Optional<String> maybeBeaconBlockRootParam) {
+    if (payloadParameter.getBlobGasUsed() != null) {
+      return ValidationResult.invalid(
+          RpcErrorType.INVALID_PARAMS, "non-null BlobGasUsed pre-cancun");
+    }
+    if (payloadParameter.getExcessBlobGas() != null) {
+      return ValidationResult.invalid(
+          RpcErrorType.INVALID_PARAMS, "non-null ExcessBlobGas pre-cancun");
+    }
+    return ValidationResult.valid();
+  }
+
   @Override
   protected ValidationResult<RpcErrorType> validateBlobs(
       final List<Transaction> transactions,

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java

@@ -468,19 +468,22 @@ public abstract class AbstractEngineNewPayloadTest extends AbstractScheduledApiT
   protected BlockHeader createBlockHeader(
       final Optional<List<Withdrawal>> maybeWithdrawals,
       final Optional<List<Deposit>> maybeDeposits) {
+    return createBlockHeaderFixture(maybeWithdrawals, maybeDeposits).buildHeader();
+  }
+
+  protected BlockHeaderTestFixture createBlockHeaderFixture(
+      final Optional<List<Withdrawal>> maybeWithdrawals,
+      final Optional<List<Deposit>> maybeDeposits) {
     BlockHeader parentBlockHeader =
         new BlockHeaderTestFixture().baseFeePerGas(Wei.ONE).buildHeader();
-    BlockHeader mockHeader =
-        new BlockHeaderTestFixture()
-            .baseFeePerGas(Wei.ONE)
-            .parentHash(parentBlockHeader.getParentHash())
-            .number(parentBlockHeader.getNumber() + 1)
-            .timestamp(parentBlockHeader.getTimestamp() + 1)
-            .withdrawalsRoot(maybeWithdrawals.map(BodyValidation::withdrawalsRoot).orElse(null))
-            .depositsRoot(maybeDeposits.map(BodyValidation::depositsRoot).orElse(null))
-            .parentBeaconBlockRoot(maybeParentBeaconBlockRoot)
-            .buildHeader();
-    return mockHeader;
+    return new BlockHeaderTestFixture()
+        .baseFeePerGas(Wei.ONE)
+        .parentHash(parentBlockHeader.getParentHash())
+        .number(parentBlockHeader.getNumber() + 1)
+        .timestamp(parentBlockHeader.getTimestamp() + 1)
+        .withdrawalsRoot(maybeWithdrawals.map(BodyValidation::withdrawalsRoot).orElse(null))
+        .depositsRoot(maybeDeposits.map(BodyValidation::depositsRoot).orElse(null))
+        .parentBeaconBlockRoot(maybeParentBeaconBlockRoot);
   }
 
   protected void assertValidResponse(final BlockHeader mockHeader, final JsonRpcResponse resp) {

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java

@@ -24,6 +24,7 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import org.hyperledger.besu.datatypes.BlobGas;
 import org.hyperledger.besu.ethereum.BlockProcessingOutputs;
 import org.hyperledger.besu.ethereum.BlockProcessingResult;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod;
@@ -127,6 +128,37 @@ public class EngineNewPayloadV2Test extends AbstractEngineNewPayloadTest {
     verify(engineCallListener, times(1)).executionEngineCalled();
   }
 
+  @Test
+  public void shouldValidateBlobGasUsedCorrectly() {
+    // V2 should return error if non-null blobGasUsed
+    BlockHeader blockHeader =
+        createBlockHeaderFixture(Optional.of(Collections.emptyList()), Optional.empty())
+            .blobGasUsed(100L)
+            .buildHeader();
+
+    var resp = resp(mockEnginePayload(blockHeader, Collections.emptyList(), List.of(), null));
+    final JsonRpcError jsonRpcError = fromErrorResp(resp);
+    assertThat(jsonRpcError.getCode()).isEqualTo(INVALID_PARAMS.getCode());
+    assertThat(jsonRpcError.getData()).isEqualTo("non-null BlobGasUsed pre-cancun");
+    verify(engineCallListener, times(1)).executionEngineCalled();
+  }
+
+  @Test
+  public void shouldValidateExcessBlobGasCorrectly() {
+    // V2 should return error if non-null ExcessBlobGas
+    BlockHeader blockHeader =
+        createBlockHeaderFixture(Optional.of(Collections.emptyList()), Optional.empty())
+            .excessBlobGas(BlobGas.MAX_BLOB_GAS)
+            .buildHeader();
+
+    var resp = resp(mockEnginePayload(blockHeader, Collections.emptyList(), List.of(), null));
+
+    final JsonRpcError jsonRpcError = fromErrorResp(resp);
+    assertThat(jsonRpcError.getCode()).isEqualTo(INVALID_PARAMS.getCode());
+    assertThat(jsonRpcError.getData()).isEqualTo("non-null ExcessBlobGas pre-cancun");
+    verify(engineCallListener, times(1)).executionEngineCalled();
+  }
+
   @Test
   public void shouldReturnInvalidIfWithdrawalsIsNull_WhenWithdrawalsAllowed() {
     final List<WithdrawalParameter> withdrawals = null;

ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3Test.java

@@ -16,8 +16,11 @@ 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.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID;
+import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.INVALID_PARAMS;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import org.hyperledger.besu.datatypes.BlobGas;
@@ -28,6 +31,7 @@ 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.parameters.EnginePayloadParameter;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
 import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EnginePayloadStatusResult;
@@ -154,4 +158,40 @@ public class EngineNewPayloadV3Test extends EngineNewPayloadV2Test {
   public void shouldReturnValidIfProtocolScheduleIsEmpty() {
     // no longer the case, blob validation requires a protocol schedule
   }
+
+  @Test
+  @Override
+  public void shouldValidateBlobGasUsedCorrectly() {
+    // V3 must return error if null blobGasUsed
+    BlockHeader blockHeader =
+        createBlockHeaderFixture(Optional.of(Collections.emptyList()), Optional.empty())
+            .excessBlobGas(BlobGas.MAX_BLOB_GAS)
+            .blobGasUsed(null)
+            .buildHeader();
+
+    var resp = resp(mockEnginePayload(blockHeader, Collections.emptyList(), List.of(), null));
+
+    final JsonRpcError jsonRpcError = fromErrorResp(resp);
+    assertThat(jsonRpcError.getCode()).isEqualTo(INVALID_PARAMS.getCode());
+    assertThat(jsonRpcError.getData()).isEqualTo("Missing blob gas fields");
+    verify(engineCallListener, times(1)).executionEngineCalled();
+  }
+
+  @Test
+  @Override
+  public void shouldValidateExcessBlobGasCorrectly() {
+    // V3 must return error if null excessBlobGas
+    BlockHeader blockHeader =
+        createBlockHeaderFixture(Optional.of(Collections.emptyList()), Optional.empty())
+            .excessBlobGas(null)
+            .blobGasUsed(100L)
+            .buildHeader();
+
+    var resp = resp(mockEnginePayload(blockHeader, Collections.emptyList(), List.of(), null));
+
+    final JsonRpcError jsonRpcError = fromErrorResp(resp);
+    assertThat(jsonRpcError.getCode()).isEqualTo(INVALID_PARAMS.getCode());
+    assertThat(jsonRpcError.getData()).isEqualTo("Missing blob gas fields");
+    verify(engineCallListener, times(1)).executionEngineCalled();
+  }
 }