Skip to content

Commit

Permalink
more error handling
Browse files Browse the repository at this point in the history 
Signed-off-by: Justin Florentine <[email protected]>
  • Loading branch information
@jflo
jflo committed Oct 13, 2023
1 parent 6aa63b1 commit e46f812
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 49 deletions.
81 changes: 48 additions & 33 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 @@ -93,37 +93,26 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
forkChoice.getSafeBlockHash(),
forkChoice.getFinalizedBlockHash());

if (mergeCoordinator.isBadBlock(forkChoice.getHeadBlockHash())) {
logForkchoiceUpdatedCall(INVALID, forkChoice);
return new JsonRpcSuccessResponse(
requestId,
new EngineUpdateForkchoiceResult(
INVALID,
mergeCoordinator
.getLatestValidHashOfBadBlock(forkChoice.getHeadBlockHash())
.orElse(Hash.ZERO),
null,
Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block")));
}

final Optional<BlockHeader> maybeNewHead =
mergeCoordinator.getOrSyncHeadByHash(
forkChoice.getHeadBlockHash(), forkChoice.getFinalizedBlockHash());

ForkchoiceResult forkchoiceResult = null;
if (maybeNewHead.isEmpty()) {
return syncingResponse(requestId, forkChoice);
}
Optional<List<Withdrawal>> withdrawals = Optional.empty();
final BlockHeader newHead = maybeNewHead.get();
if (!isValidForkchoiceState(
forkChoice.getSafeBlockHash(), forkChoice.getFinalizedBlockHash(), newHead)) {
} else if (!isValidForkchoiceState(
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 +125,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 +145,33 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
}
}

if (mergeCoordinator.isBadBlock(forkChoice.getHeadBlockHash())) {
logForkchoiceUpdatedCall(INVALID, forkChoice);
return new JsonRpcSuccessResponse(
requestId,
new EngineUpdateForkchoiceResult(
INVALID,
mergeCoordinator
.getLatestValidHashOfBadBlock(forkChoice.getHeadBlockHash())
.orElse(Hash.ZERO),
null,
Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block")));
}

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 +185,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 +221,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
10 changes: 3 additions & 7 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,23 +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()) {
return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.UNSUPPORTED_FORK));
} else {
return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.INVALID_PARAMS));
}
} else if (payloadAttributes.getParentBeaconBlockRoot() != null
|| !payloadAttributes.getParentBeaconBlockRoot().isEmpty()) {
} 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 e46f812

Please sign in to comment.