From a90e4487fc67f6d9315cf9499fda534490156f50 Mon Sep 17 00:00:00 2001 From: Justin Florentine Date: Tue, 17 Oct 2023 20:55:35 -0400 Subject: [PATCH] Fcu v2 defer fork validation (#6037) * more error handling --------- Signed-off-by: Justin Florentine --- .../AbstractEngineForkchoiceUpdated.java | 53 ++++++++++++------- .../engine/EngineForkchoiceUpdatedV1.java | 14 +++++ .../engine/EngineForkchoiceUpdatedV2.java | 13 ++--- .../engine/EngineForkchoiceUpdatedV3.java | 11 +--- 4 files changed, 56 insertions(+), 35 deletions(-) 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 7c7cfca2c44..6f65dc5bd3d 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 @@ -113,17 +113,21 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) if (maybeNewHead.isEmpty()) { return syncingResponse(requestId, forkChoice); } - Optional> withdrawals = Optional.empty(); - final BlockHeader newHead = maybeNewHead.get(); + + ForkchoiceResult forkchoiceResult = null; if (!isValidForkchoiceState( - forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) { + forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), maybeNewHead.get())) { logForkchoiceUpdatedCall(INVALID, forkChoice); return new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_FORKCHOICE_STATE); + } else { + forkchoiceResult = + mergeCoordinator.updateForkChoice( + maybeNewHead.get(), + forkChoice.getFinalizedBlockHash(), + forkChoice.getSafeBlockHash()); } - ForkchoiceResult result = - mergeCoordinator.updateForkChoice( - newHead, forkChoice.getFinalizedBlockHash(), forkChoice.getSafeBlockHash()); + Optional> withdrawals = Optional.empty(); if (maybePayloadAttributes.isPresent()) { final EnginePayloadAttributesParameter payloadAttributes = maybePayloadAttributes.get(); withdrawals = @@ -136,7 +140,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) .map(WithdrawalParameter::toWithdrawal) .collect(toList()))); Optional maybeError = - isPayloadAttributesValid(requestId, payloadAttributes, withdrawals, newHead); + isPayloadAttributesValid(requestId, payloadAttributes, withdrawals); if (maybeError.isPresent()) { LOG.atWarn() .setMessage("RpcError {}: {}") @@ -156,6 +160,20 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) } } + final BlockHeader newHead = maybeNewHead.get(); + if (maybePayloadAttributes.isPresent()) { + Optional maybeError = + isPayloadAttributeRelevantToNewHead(requestId, maybePayloadAttributes.get(), newHead); + if (maybeError.isPresent()) { + return maybeError.get(); + } + if (!getWithdrawalsValidator( + protocolSchedule.get(), newHead, maybePayloadAttributes.get().getTimestamp()) + .validateWithdrawals(withdrawals)) { + return new JsonRpcErrorResponse(requestId, getInvalidPayloadError()); + } + } + ValidationResult parameterValidationResult = validateParameter(forkChoice, maybePayloadAttributes); if (!parameterValidationResult.isValid()) { @@ -169,13 +187,13 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) maybePayloadAttributes.ifPresentOrElse( this::logPayload, () -> LOG.debug("Payload attributes are null")); - if (result.shouldNotProceedToPayloadBuildProcess()) { - if (ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD.equals(result.getStatus())) { + if (forkchoiceResult.shouldNotProceedToPayloadBuildProcess()) { + if (ForkchoiceResult.Status.IGNORE_UPDATE_TO_OLD_HEAD.equals(forkchoiceResult.getStatus())) { logForkchoiceUpdatedCall(VALID, forkChoice); } else { logForkchoiceUpdatedCall(INVALID, forkChoice); } - return handleNonValidForkchoiceUpdate(requestId, result); + return handleNonValidForkchoiceUpdate(requestId, forkchoiceResult); } // begin preparing a block if we have a non-empty payload attributes param @@ -205,15 +223,19 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) requestId, new EngineUpdateForkchoiceResult( VALID, - result.getNewHead().map(BlockHeader::getHash).orElse(null), + forkchoiceResult.getNewHead().map(BlockHeader::getHash).orElse(null), payloadId.orElse(null), Optional.empty())); } - protected Optional isPayloadAttributesValid( + protected abstract Optional isPayloadAttributesValid( + final Object requestId, + final EnginePayloadAttributesParameter payloadAttributes, + final Optional> maybeWithdrawals); + + protected Optional isPayloadAttributeRelevantToNewHead( final Object requestId, final EnginePayloadAttributesParameter payloadAttributes, - final Optional> maybeWithdrawals, final BlockHeader headBlockHeader) { if (payloadAttributes.getTimestamp() <= headBlockHeader.getTimestamp()) { @@ -221,11 +243,6 @@ protected Optional isPayloadAttributesValid( "Payload attributes timestamp is smaller than timestamp of header in fork choice update"); return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadError())); } - if (!getWithdrawalsValidator( - protocolSchedule.get(), headBlockHeader, payloadAttributes.getTimestamp()) - .validateWithdrawals(maybeWithdrawals)) { - return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadError())); - } return Optional.empty(); } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java index a5b9eeb9ed2..24d63fbd434 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java @@ -17,9 +17,15 @@ 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.api.jsonrpc.internal.response.JsonRpcErrorResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; +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; public class EngineForkchoiceUpdatedV1 extends AbstractEngineForkchoiceUpdated { @@ -33,6 +39,14 @@ public EngineForkchoiceUpdatedV1( super(vertx, protocolSchedule, protocolContext, mergeCoordinator, engineCallListener); } + @Override + protected Optional isPayloadAttributesValid( + final Object requestId, + final EnginePayloadAttributesParameter payloadAttributes, + final Optional> maybeWithdrawals) { + return Optional.empty(); + } + @Override public String getName() { return RpcMethod.ENGINE_FORKCHOICE_UPDATED_V1.getMethodName(); 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 e7a3ba9f537..c4daa688d47 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 @@ -20,7 +20,6 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadAttributesParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; -import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.Withdrawal; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; @@ -55,22 +54,20 @@ public String getName() { protected Optional isPayloadAttributesValid( final Object requestId, final EnginePayloadAttributesParameter payloadAttributes, - final Optional> maybeWithdrawals, - final BlockHeader headBlockHeader) { + final Optional> maybeWithdrawals) { if (payloadAttributes.getTimestamp() >= cancunTimestamp) { if (payloadAttributes.getParentBeaconBlockRoot() == null - || payloadAttributes.getParentBeaconBlockRoot().isEmpty() - || payloadAttributes.getParentBeaconBlockRoot().isZero()) { + || payloadAttributes.getParentBeaconBlockRoot().isEmpty()) { + return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.UNSUPPORTED_FORK)); + } else { return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_PARAMS)); } - return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.UNSUPPORTED_FORK)); } else if (payloadAttributes.getParentBeaconBlockRoot() != null) { LOG.error( "Parent beacon block root hash present in payload attributes before cancun hardfork"); return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_PARAMS)); } else { - return super.isPayloadAttributesValid( - requestId, payloadAttributes, maybeWithdrawals, headBlockHeader); + return Optional.empty(); } } } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java index 4284fdd01dc..5b33f653fb3 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java @@ -21,7 +21,6 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadAttributesParameter; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType; -import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.Withdrawal; import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule; import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec; @@ -94,14 +93,8 @@ protected ValidationResult validateForkSupported(final long blockT protected Optional isPayloadAttributesValid( final Object requestId, final EnginePayloadAttributesParameter payloadAttributes, - final Optional> maybeWithdrawals, - final BlockHeader headBlockHeader) { - Optional maybeError = - super.isPayloadAttributesValid( - requestId, payloadAttributes, maybeWithdrawals, headBlockHeader); - if (maybeError.isPresent()) { - return maybeError; - } else if (payloadAttributes.getParentBeaconBlockRoot() == null) { + final Optional> maybeWithdrawals) { + if (payloadAttributes.getParentBeaconBlockRoot() == null) { LOG.error( "Parent beacon block root hash not present in payload attributes after cancun hardfork"); return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_PARAMS));