diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java index b64fca8d429..aca6ee9dc6b 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java +++ b/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,14 +86,44 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) requestContext.getOptionalParameter(1, EnginePayloadAttributesParameter.class); LOG.debug("Forkchoice parameters {}", forkChoice); - ValidationResult parameterValidationResult = - validateParameter(forkChoice, maybePayloadAttributes); - if (!parameterValidationResult.isValid()) { - return new JsonRpcErrorResponse(requestId, parameterValidationResult); - } + mergeContext + .get() + .fireNewUnverifiedForkchoiceEvent( + forkChoice.getHeadBlockHash(), + forkChoice.getSafeBlockHash(), + forkChoice.getFinalizedBlockHash()); + + final Optional maybeNewHead = + mergeCoordinator.getOrSyncHeadByHash( + forkChoice.getHeadBlockHash(), forkChoice.getFinalizedBlockHash()); + if (maybeNewHead.isEmpty()) { + return syncingResponse(requestId, forkChoice); + } + Optional> 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 forkValidationResult = validateForkSupported(payloadAttributes.getTimestamp()); if (!forkValidationResult.isValid()) { @@ -101,12 +131,11 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } } - mergeContext - .get() - .fireNewUnverifiedForkchoiceEvent( - forkChoice.getHeadBlockHash(), - forkChoice.getSafeBlockHash(), - forkChoice.getFinalizedBlockHash()); + ValidationResult 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 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> 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> finalWithdrawals = withdrawals; Optional 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> maybeWithdrawals, final BlockHeader headBlockHeader) { diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java index 84459b43c9c..d8e0fd715a3 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java +++ b/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> maybeWithdrawals, + final BlockHeader headBlockHeader) { + if (payloadAttributes.getTimestamp() >= cancunTimestamp) { + return false; + } else { + return super.isPayloadAttributesValid(payloadAttributes, maybeWithdrawals, headBlockHeader); + } + } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java index 63713c4bd0f..693271bf689 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java +++ b/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));