Forkchoice v2 hive tests

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

@@ -52,7 +52,7 @@
 public abstract class AbstractEngineForkchoiceUpdated extends ExecutionEngineJsonRpcMethod {
   private static final Logger LOG = LoggerFactory.getLogger(AbstractEngineForkchoiceUpdated.class);
   private final MergeMiningCoordinator mergeCoordinator;
-  private final Long cancunTimestamp;
+  protected final Long cancunTimestamp;
 
   public AbstractEngineForkchoiceUpdated(
       final Vertx vertx,
@@ -86,27 +86,56 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
         requestContext.getOptionalParameter(1, EnginePayloadAttributesParameter.class);
 
     LOG.debug("Forkchoice parameters {}", forkChoice);
-    ValidationResult<RpcErrorType> parameterValidationResult =
-        validateParameter(forkChoice, maybePayloadAttributes);
-    if (!parameterValidationResult.isValid()) {
-      return new JsonRpcErrorResponse(requestId, parameterValidationResult);
-    }
+    mergeContext
+        .get()
+        .fireNewUnverifiedForkchoiceEvent(
+            forkChoice.getHeadBlockHash(),
+            forkChoice.getSafeBlockHash(),
+            forkChoice.getFinalizedBlockHash());
+
+    final Optional<BlockHeader> maybeNewHead =
+        mergeCoordinator.getOrSyncHeadByHash(
+            forkChoice.getHeadBlockHash(), forkChoice.getFinalizedBlockHash());
 
+    if (maybeNewHead.isEmpty()) {
+      return syncingResponse(requestId, forkChoice);
+    }
+    Optional<List<Withdrawal>> withdrawals = Optional.empty();
+    final BlockHeader newHead = maybeNewHead.get();
     if (maybePayloadAttributes.isPresent()) {
       final EnginePayloadAttributesParameter payloadAttributes = maybePayloadAttributes.get();
+      withdrawals =
+          maybePayloadAttributes.flatMap(
+              pa ->
+                  Optional.ofNullable(pa.getWithdrawals())
+                      .map(
+                          ws ->
+                              ws.stream()
+                                  .map(WithdrawalParameter::toWithdrawal)
+                                  .collect(toList())));
+      if (!isPayloadAttributesValid(maybePayloadAttributes.get(), withdrawals, newHead)) {
+        LOG.atWarn()
+            .setMessage("Invalid payload attributes: {}")
+            .addArgument(
+                () ->
+                    maybePayloadAttributes
+                        .map(EnginePayloadAttributesParameter::serialize)
+                        .orElse(null))
+            .log();
+        return new JsonRpcErrorResponse(requestId, getInvalidPayloadError());
+      }
       ValidationResult<RpcErrorType> forkValidationResult =
           validateForkSupported(payloadAttributes.getTimestamp());
       if (!forkValidationResult.isValid()) {
         return new JsonRpcErrorResponse(requestId, forkValidationResult);
       }
     }
 
-    mergeContext
-        .get()
-        .fireNewUnverifiedForkchoiceEvent(
-            forkChoice.getHeadBlockHash(),
-            forkChoice.getSafeBlockHash(),
-            forkChoice.getFinalizedBlockHash());
+    ValidationResult<RpcErrorType> parameterValidationResult =
+        validateParameter(forkChoice, maybePayloadAttributes);
+    if (!parameterValidationResult.isValid()) {
+      return new JsonRpcSuccessResponse(requestId, parameterValidationResult);
+    }
 
     if (mergeContext.get().isSyncing()) {
       return syncingResponse(requestId, forkChoice);
@@ -125,16 +154,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
               Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block")));
     }
 
-    final Optional<BlockHeader> maybeNewHead =
-        mergeCoordinator.getOrSyncHeadByHash(
-            forkChoice.getHeadBlockHash(), forkChoice.getFinalizedBlockHash());
-
-    if (maybeNewHead.isEmpty()) {
-      return syncingResponse(requestId, forkChoice);
-    }
-
-    final BlockHeader newHead = maybeNewHead.get();
-
     if (!isValidForkchoiceState(
         forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) {
       logForkchoiceUpdatedCall(INVALID, forkChoice);
@@ -144,31 +163,10 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     maybePayloadAttributes.ifPresentOrElse(
         this::logPayload, () -> LOG.debug("Payload attributes are null"));
 
-    final Optional<List<Withdrawal>> withdrawals =
-        maybePayloadAttributes.flatMap(
-            payloadAttributes ->
-                Optional.ofNullable(payloadAttributes.getWithdrawals())
-                    .map(
-                        ws ->
-                            ws.stream().map(WithdrawalParameter::toWithdrawal).collect(toList())));
-
     ForkchoiceResult result =
         mergeCoordinator.updateForkChoice(
             newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash());
 
-    if (maybePayloadAttributes.isPresent()
-        && !isPayloadAttributesValid(maybePayloadAttributes.get(), withdrawals, newHead)) {
-      LOG.atWarn()
-          .setMessage("Invalid payload attributes: {}")
-          .addArgument(
-              () ->
-                  maybePayloadAttributes
-                      .map(EnginePayloadAttributesParameter::serialize)
-                      .orElse(null))
-          .log();
-      return new JsonRpcErrorResponse(requestId, getInvalidPayloadError());
-    }
-
     if (result.shouldNotProceedToPayloadBuildProcess()) {
       if (ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD.equals(result.getStatus())) {
         logForkchoiceUpdatedCall(VALID, forkChoice);
@@ -179,6 +177,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     }
 
     // begin preparing a block if we have a non-empty payload attributes param
+    final Optional<List<Withdrawal>> finalWithdrawals = withdrawals;
     Optional<PayloadIdentifier> payloadId =
         maybePayloadAttributes.map(
             payloadAttributes ->
@@ -187,7 +186,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
                     payloadAttributes.getTimestamp(),
                     payloadAttributes.getPrevRandao(),
                     payloadAttributes.getSuggestedFeeRecipient(),
-                    withdrawals,
+                    finalWithdrawals,
                     Optional.ofNullable(payloadAttributes.getParentBeaconBlockRoot())));
 
     payloadId.ifPresent(
@@ -209,7 +208,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
             Optional.empty()));
   }
 
-  private boolean isPayloadAttributesValid(
+  protected boolean isPayloadAttributesValid(
       final EnginePayloadAttributesParameter payloadAttributes,
       final Optional<List<Withdrawal>> maybeWithdrawals,
       final BlockHeader headBlockHeader) {

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

@@ -17,8 +17,14 @@
 import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
 import org.hyperledger.besu.ethereum.ProtocolContext;
 import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
+import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadAttributesParameter;
+import org.hyperledger.besu.ethereum.core.BlockHeader;
+import org.hyperledger.besu.ethereum.core.Withdrawal;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 
+import java.util.List;
+import java.util.Optional;
+
 import io.vertx.core.Vertx;
 
 // TODO Withdrawals use composition instead? Want to make it more obvious that there is no
@@ -38,4 +44,16 @@ public EngineForkchoiceUpdatedV2(
   public String getName() {
     return RpcMethod.ENGINE_FORKCHOICE_UPDATED_V2.getMethodName();
   }
+
+  @Override
+  protected boolean isPayloadAttributesValid(
+      final EnginePayloadAttributesParameter payloadAttributes,
+      final Optional<List<Withdrawal>> maybeWithdrawals,
+      final BlockHeader headBlockHeader) {
+    if (payloadAttributes.getTimestamp() >= cancunTimestamp) {
+      return false;
+    } else {
+      return super.isPayloadAttributesValid(payloadAttributes, maybeWithdrawals, headBlockHeader);
+    }
+  }
 }

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

@@ -143,6 +143,7 @@ public void shouldReturnSyncingIfMissingNewHead() {
   public void shouldReturnInvalidWithLatestValidHashOnBadBlock() {
     BlockHeader mockHeader = blockHeaderBuilder.buildHeader();
     Hash latestValidHash = Hash.hash(Bytes32.fromHexStringLenient("0xcafebabe"));
+    when(mergeCoordinator.getOrSyncHeadByHash(any(), any())).thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.isBadBlock(mockHeader.getHash())).thenReturn(true);
     when(mergeCoordinator.getLatestValidHashOfBadBlock(mockHeader.getHash()))
         .thenReturn(Optional.of(latestValidHash));