payload validation moved earlier, fcu v2 checks for cancun timestamps

Signed-off-by: Justin Florentine <[email protected]>
jflo · Sep 23, 2023 · a444363 · a444363
1 parent 48f6216
commit a444363
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 37 deletions.
diff --git a/...er/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java b/...er/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java
@@ -86,15 +86,15 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
 
     LOG.debug("Forkchoice parameters {}", forkChoice);
     mergeContext
-            .get()
-            .fireNewUnverifiedForkchoiceEvent(
-                    forkChoice.getHeadBlockHash(),
-                    forkChoice.getSafeBlockHash(),
-                    forkChoice.getFinalizedBlockHash());
+        .get()
+        .fireNewUnverifiedForkchoiceEvent(
+            forkChoice.getHeadBlockHash(),
+            forkChoice.getSafeBlockHash(),
+            forkChoice.getFinalizedBlockHash());
 
     final Optional<BlockHeader> maybeNewHead =
-            mergeCoordinator.getOrSyncHeadByHash(
-                    forkChoice.getHeadBlockHash(), forkChoice.getFinalizedBlockHash());
+        mergeCoordinator.getOrSyncHeadByHash(
+            forkChoice.getHeadBlockHash(), forkChoice.getFinalizedBlockHash());
 
     if (maybeNewHead.isEmpty()) {
       return syncingResponse(requestId, forkChoice);
@@ -103,22 +103,24 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     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())));
+      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();
+            .setMessage("Invalid payload attributes: {}")
+            .addArgument(
+                () ->
+                    maybePayloadAttributes
+                        .map(EnginePayloadAttributesParameter::serialize)
+                        .orElse(null))
+            .log();
         return new JsonRpcErrorResponse(requestId, getInvalidPayloadError());
       }
       ValidationResult<RpcErrorType> forkValidationResult =
@@ -133,8 +135,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
       return new JsonRpcSuccessResponse(requestId, parameterValidationResult);
     }
 
-
-
     if (mergeContext.get().isSyncing()) {
       return syncingResponse(requestId, forkChoice);
     }
@@ -152,8 +152,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
               Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block")));
     }
 
-
-
     if (!isValidForkchoiceState(
         forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) {
       logForkchoiceUpdatedCall(INVALID, forkChoice);
@@ -163,14 +161,10 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
     maybePayloadAttributes.ifPresentOrElse(
         this::logPayload, () -> LOG.debug("Payload attributes are null"));
 
-
-
     ForkchoiceResult result =
         mergeCoordinator.updateForkChoice(
             newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash());
 
-
-
     if (result.shouldNotProceedToPayloadBuildProcess()) {
       if (ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD.equals(result.getStatus())) {
         logForkchoiceUpdatedCall(VALID, forkChoice);

diff --git a/...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java b/...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java
@@ -22,11 +22,11 @@
 import org.hyperledger.besu.ethereum.core.Withdrawal;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 
-import io.vertx.core.Vertx;
-
 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
 // difference between V1/V2 code other than the method name
 public class EngineForkchoiceUpdatedV2 extends AbstractEngineForkchoiceUpdated {
@@ -47,10 +47,10 @@ public String getName() {
 
   @Override
   protected boolean isPayloadAttributesValid(
-          final EnginePayloadAttributesParameter payloadAttributes,
-          final Optional<List<Withdrawal>> maybeWithdrawals,
-          final BlockHeader headBlockHeader) {
-    if(payloadAttributes.getTimestamp() >= cancunTimestamp) {
+      final EnginePayloadAttributesParameter payloadAttributes,
+      final Optional<List<Withdrawal>> maybeWithdrawals,
+      final BlockHeader headBlockHeader) {
+    if (payloadAttributes.getTimestamp() >= cancunTimestamp) {
       return false;
     } else {
       return super.isPayloadAttributesValid(payloadAttributes, maybeWithdrawals, headBlockHeader);

diff --git a/...esu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java b/...esu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java
@@ -143,8 +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.getOrSyncHeadByHash(any(), any())).thenReturn(Optional.of(mockHeader));
     when(mergeCoordinator.isBadBlock(mockHeader.getHash())).thenReturn(true);
     when(mergeCoordinator.getLatestValidHashOfBadBlock(mockHeader.getHash()))
         .thenReturn(Optional.of(latestValidHash));