Skip to content

Commit

Permalink
Fcu v2 defer fork validation (hyperledger#6037)
Browse files Browse the repository at this point in the history 
* more error handling

---------

Signed-off-by: Justin Florentine <[email protected]>
  • Loading branch information
@jflo
jflo authored Oct 18, 2023
1 parent 6d100f3 commit a90e448
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 35 deletions.
53 changes: 35 additions & 18 deletions ...er/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdated.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,17 +113,21 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
if (maybeNewHead.isEmpty()) {
return syncingResponse(requestId, forkChoice);
}
Optional<List<Withdrawal>> 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<List<Withdrawal>> withdrawals = Optional.empty();
if (maybePayloadAttributes.isPresent()) {
final EnginePayloadAttributesParameter payloadAttributes = maybePayloadAttributes.get();
withdrawals =
Expand All @@ -136,7 +140,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
.map(WithdrawalParameter::toWithdrawal)
.collect(toList())));
Optional<JsonRpcErrorResponse> maybeError =
isPayloadAttributesValid(requestId, payloadAttributes, withdrawals, newHead);
isPayloadAttributesValid(requestId, payloadAttributes, withdrawals);
if (maybeError.isPresent()) {
LOG.atWarn()
.setMessage("RpcError {}: {}")
Expand All @@ -156,6 +160,20 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
}
}

final BlockHeader newHead = maybeNewHead.get();
if (maybePayloadAttributes.isPresent()) {
Optional<JsonRpcErrorResponse> 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<RpcErrorType> parameterValidationResult =
validateParameter(forkChoice, maybePayloadAttributes);
if (!parameterValidationResult.isValid()) {
Expand All @@ -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
Expand Down Expand Up @@ -205,27 +223,26 @@ 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<JsonRpcErrorResponse> isPayloadAttributesValid(
protected abstract Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
final Object requestId,
final EnginePayloadAttributesParameter payloadAttributes,
final Optional<List<Withdrawal>> maybeWithdrawals);

protected Optional<JsonRpcErrorResponse> isPayloadAttributeRelevantToNewHead(
final Object requestId,
final EnginePayloadAttributesParameter payloadAttributes,
final Optional<List<Withdrawal>> maybeWithdrawals,
final BlockHeader headBlockHeader) {

if (payloadAttributes.getTimestamp() <= headBlockHeader.getTimestamp()) {
LOG.warn(
"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();
}
Expand Down
14 changes: 14 additions & 0 deletions ...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV1.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -33,6 +39,14 @@ public EngineForkchoiceUpdatedV1(
super(vertx, protocolSchedule, protocolContext, mergeCoordinator, engineCallListener);
}

@Override
protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
final Object requestId,
final EnginePayloadAttributesParameter payloadAttributes,
final Optional<List<Withdrawal>> maybeWithdrawals) {
return Optional.empty();
}

@Override
public String getName() {
return RpcMethod.ENGINE_FORKCHOICE_UPDATED_V1.getMethodName();
Expand Down
13 changes: 5 additions & 8 deletions ...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -55,22 +54,20 @@ public String getName() {
protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
final Object requestId,
final EnginePayloadAttributesParameter payloadAttributes,
final Optional<List<Withdrawal>> maybeWithdrawals,
final BlockHeader headBlockHeader) {
final Optional<List<Withdrawal>> 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();
}
}
}
11 changes: 2 additions & 9 deletions ...erledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdatedV3.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -94,14 +93,8 @@ protected ValidationResult<RpcErrorType> validateForkSupported(final long blockT
protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
final Object requestId,
final EnginePayloadAttributesParameter payloadAttributes,
final Optional<List<Withdrawal>> maybeWithdrawals,
final BlockHeader headBlockHeader) {
Optional<JsonRpcErrorResponse> maybeError =
super.isPayloadAttributesValid(
requestId, payloadAttributes, maybeWithdrawals, headBlockHeader);
if (maybeError.isPresent()) {
return maybeError;
} else if (payloadAttributes.getParentBeaconBlockRoot() == null) {
final Optional<List<Withdrawal>> 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));
Expand Down

0 comments on commit a90e448

Please sign in to comment.