Skip to content

Commit

Permalink
Forkchoice v2 hive tests (hyperledger#5949)
Browse files Browse the repository at this point in the history 
* payload validation moved earlier, fcu v2 checks for cancun timestamps

Signed-off-by: Justin Florentine <[email protected]>

* payload validation moved earlier, fcu v2 checks for cancun timestamps

Signed-off-by: Justin Florentine <[email protected]>

* build passes

Signed-off-by: Justin Florentine <[email protected]>

---------

Signed-off-by: Justin Florentine <[email protected]>
  • Loading branch information
@jflo
jflo committed Nov 10, 2023
1 parent 1ca5ca0 commit 56bb1e1
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 45 deletions.
89 changes: 44 additions & 45 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 @@ -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,
Expand Down Expand Up @@ -86,27 +86,56 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
requestContext.getOptionalParameter(1, EnginePayloadAttributesParameter.class);

LOG.debug("Forkchoice parameters {}", forkChoice);
ValidationResult<RpcErrorType> parameterValidationResult =
validateParameter(forkChoice, maybePayloadAttributes);
if (!parameterValidationResult.isValid()) {
return new JsonRpcErrorResponse(requestId, parameterValidationResult);
}
mergeContext
.get()
.fireNewUnverifiedForkchoiceEvent(
forkChoice.getHeadBlockHash(),
forkChoice.getSafeBlockHash(),
forkChoice.getFinalizedBlockHash());

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

if (maybeNewHead.isEmpty()) {
return syncingResponse(requestId, forkChoice);
}
Optional<List<Withdrawal>> 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<RpcErrorType> forkValidationResult =
validateForkSupported(payloadAttributes.getTimestamp());
if (!forkValidationResult.isValid()) {
return new JsonRpcErrorResponse(requestId, forkValidationResult);
}
}

mergeContext
.get()
.fireNewUnverifiedForkchoiceEvent(
forkChoice.getHeadBlockHash(),
forkChoice.getSafeBlockHash(),
forkChoice.getFinalizedBlockHash());
ValidationResult<RpcErrorType> parameterValidationResult =
validateParameter(forkChoice, maybePayloadAttributes);
if (!parameterValidationResult.isValid()) {
return new JsonRpcSuccessResponse(requestId, parameterValidationResult);
}

if (mergeContext.get().isSyncing()) {
return syncingResponse(requestId, forkChoice);
Expand All @@ -125,16 +154,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
Optional.of(forkChoice.getHeadBlockHash() + " is an invalid block")));
}

final Optional<BlockHeader> 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);
Expand All @@ -144,31 +163,10 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
maybePayloadAttributes.ifPresentOrElse(
this::logPayload, () -> LOG.debug("Payload attributes are null"));

final Optional<List<Withdrawal>> 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);
Expand All @@ -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<List<Withdrawal>> finalWithdrawals = withdrawals;
Optional<PayloadIdentifier> payloadId =
maybePayloadAttributes.map(
payloadAttributes ->
Expand All @@ -187,7 +186,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
payloadAttributes.getTimestamp(),
payloadAttributes.getPrevRandao(),
payloadAttributes.getSuggestedFeeRecipient(),
withdrawals,
finalWithdrawals,
Optional.ofNullable(payloadAttributes.getParentBeaconBlockRoot())));

payloadId.ifPresent(
Expand All @@ -209,7 +208,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
Optional.empty()));
}

private boolean isPayloadAttributesValid(
protected boolean isPayloadAttributesValid(
final EnginePayloadAttributesParameter payloadAttributes,
final Optional<List<Withdrawal>> maybeWithdrawals,
final BlockHeader headBlockHeader) {
Expand Down
18 changes: 18 additions & 0 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 @@ -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
Expand All @@ -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<List<Withdrawal>> maybeWithdrawals,
final BlockHeader headBlockHeader) {
if (payloadAttributes.getTimestamp() >= cancunTimestamp) {
return false;
} else {
return super.isPayloadAttributesValid(payloadAttributes, maybeWithdrawals, headBlockHeader);
}
}
}
1 change: 1 addition & 0 deletions ...esu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineForkchoiceUpdatedTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down

0 comments on commit 56bb1e1

Please sign in to comment.