Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prague Engine API validation fixes #8250

Merged
merged 3 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
.log();
return maybeError.get();
}
ValidationResult<RpcErrorType> forkValidationResult =
validateForkSupported(payloadAttributes.getTimestamp());
if (!forkValidationResult.isValid()) {
return new JsonRpcErrorResponse(requestId, forkValidationResult);
}
}

final BlockHeader newHead = maybeNewHead.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
e);
}

final ValidationResult<RpcErrorType> forkValidationResult =
validateForkSupported(blockParam.getTimestamp());
if (!forkValidationResult.isValid()) {
return new JsonRpcErrorResponse(reqId, forkValidationResult);
}

final ValidationResult<RpcErrorType> parameterValidationResult =
validateParameters(
blockParam,
Expand All @@ -163,12 +169,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
return new JsonRpcErrorResponse(reqId, parameterValidationResult);
}

final ValidationResult<RpcErrorType> forkValidationResult =
validateForkSupported(blockParam.getTimestamp());
if (!forkValidationResult.isValid()) {
return new JsonRpcErrorResponse(reqId, forkValidationResult);
}

final Optional<List<VersionedHash>> maybeVersionedHashes;
try {
maybeVersionedHashes = extractVersionedHashes(maybeVersionedHashParam);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadAttributesError()));
}

ValidationResult<RpcErrorType> forkValidationResult =
validateForkSupported(payloadAttributes.getTimestamp());
if (!forkValidationResult.isValid()) {
return Optional.of(new JsonRpcErrorResponse(requestId, forkValidationResult));
}

return Optional.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ protected ValidationResult<RpcErrorType> validateForkSupported(final long blockT
@Override
protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
final Object requestId, final EnginePayloadAttributesParameter payloadAttributes) {

if (payloadAttributes.getParentBeaconBlockRoot() == null) {
LOG.error(
"Parent beacon block root hash not present in payload attributes after cancun hardfork");
Expand All @@ -91,8 +92,10 @@ protected Optional<JsonRpcErrorResponse> isPayloadAttributesValid(
return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadAttributesError()));
}

if (cancunMilestone.isEmpty() || payloadAttributes.getTimestamp() < cancunMilestone.get()) {
return Optional.of(new JsonRpcErrorResponse(requestId, RpcErrorType.UNSUPPORTED_FORK));
ValidationResult<RpcErrorType> forkValidationResult =
validateForkSupported(payloadAttributes.getTimestamp());
if (!forkValidationResult.isValid()) {
return Optional.of(new JsonRpcErrorResponse(requestId, forkValidationResult));
}

return Optional.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;

import static org.hyperledger.besu.datatypes.HardforkId.MainnetHardforkId.CANCUN;
import static org.hyperledger.besu.datatypes.HardforkId.MainnetHardforkId.PRAGUE;

import org.hyperledger.besu.consensus.merge.PayloadWrapper;
import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
Expand Down Expand Up @@ -68,6 +69,11 @@ protected JsonRpcResponse createResponse(

@Override
protected ValidationResult<RpcErrorType> validateForkSupported(final long blockTimestamp) {
return ForkSupportHelper.validateForkSupported(CANCUN, cancunMilestone, blockTimestamp);
return ForkSupportHelper.validateForkSupported(
CANCUN,
cancunMilestone,
PRAGUE,
protocolSchedule.flatMap(s -> s.milestoneFor(PRAGUE)),
blockTimestamp);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;

import static org.hyperledger.besu.datatypes.HardforkId.MainnetHardforkId.CANCUN;
import static org.hyperledger.besu.datatypes.HardforkId.MainnetHardforkId.PRAGUE;

import org.hyperledger.besu.consensus.merge.blockcreation.MergeMiningCoordinator;
import org.hyperledger.besu.ethereum.ProtocolContext;
Expand Down Expand Up @@ -89,6 +90,11 @@ protected ValidationResult<RpcErrorType> validateParameters(

@Override
protected ValidationResult<RpcErrorType> validateForkSupported(final long blockTimestamp) {
return ForkSupportHelper.validateForkSupported(CANCUN, cancunMilestone, blockTimestamp);
return ForkSupportHelper.validateForkSupported(
CANCUN,
cancunMilestone,
PRAGUE,
protocolSchedule.flatMap(s -> s.milestoneFor(PRAGUE)),
blockTimestamp);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,54 @@
import java.util.Optional;

public class ForkSupportHelper {

public static ValidationResult<RpcErrorType> validateForkSupported(
final HardforkId firstSupportedHardforkId,
final Optional<Long> maybeForkMilestone,
final Optional<Long> maybeFirstSupportedForkMilestone,
final long blockTimestamp) {
if (maybeForkMilestone.isEmpty()) {

if (maybeFirstSupportedForkMilestone.isEmpty()) {
return ValidationResult.invalid(
RpcErrorType.UNSUPPORTED_FORK,
"Configuration error, no schedule for " + firstSupportedHardforkId.name() + " fork set");
}

if (Long.compareUnsigned(blockTimestamp, maybeForkMilestone.get()) < 0) {
if (Long.compareUnsigned(blockTimestamp, maybeFirstSupportedForkMilestone.get()) < 0) {
return ValidationResult.invalid(
RpcErrorType.UNSUPPORTED_FORK,
firstSupportedHardforkId.name()
+ " configured to start at timestamp: "
+ maybeForkMilestone.get());
+ maybeFirstSupportedForkMilestone.get());
}

return ValidationResult.valid();
}

public static ValidationResult<RpcErrorType> validateForkSupported(
final HardforkId firstSupportedHardforkId,
final Optional<Long> maybeFirstSupportedForkMilestone,
final HardforkId firstUnsupportedHardforkId,
final Optional<Long> maybeFirstUnsupportedMilestone,
final long blockTimestamp) {

var result =
validateForkSupported(
firstSupportedHardforkId, maybeFirstSupportedForkMilestone, blockTimestamp);

if (!result.isValid()) {
return result;
}

if (maybeFirstUnsupportedMilestone.isPresent()
&& Long.compareUnsigned(blockTimestamp, maybeFirstUnsupportedMilestone.get()) >= 0) {
return ValidationResult.invalid(
RpcErrorType.UNSUPPORTED_FORK,
"block timestamp "
+ blockTimestamp
+ " is after the first unsupported milestone: "
+ firstUnsupportedHardforkId.name()
+ " at timestamp "
+ maybeFirstUnsupportedMilestone.get());
}

return ValidationResult.valid();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.SYNCING;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.VALID;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineTestSupport.fromErrorResp;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
Expand All @@ -43,7 +44,6 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.EnginePayloadParameter;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.WithdrawalParameter;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EnginePayloadStatusResult;
Expand Down Expand Up @@ -429,14 +429,6 @@ protected EnginePayloadStatusResult fromSuccessResp(final JsonRpcResponse resp)
.get();
}

protected JsonRpcError fromErrorResp(final JsonRpcResponse resp) {
assertThat(resp.getType()).isEqualTo(RpcResponseType.ERROR);
return Optional.of(resp)
.map(JsonRpcErrorResponse.class::cast)
.map(JsonRpcErrorResponse::getError)
.get();
}

protected BlockHeader createBlockHeader(final Optional<List<Withdrawal>> maybeWithdrawals) {
return createBlockHeaderFixture(maybeWithdrawals).buildHeader();
}
Expand Down
macfarla marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ public class AbstractScheduledApiTest {
protected final ScheduledProtocolSpec.Hardfork cancunHardfork =
new ScheduledProtocolSpec.Hardfork("Cancun", 30);
protected final ScheduledProtocolSpec.Hardfork pragueHardfork =
new ScheduledProtocolSpec.Hardfork("Prague", 40);
new ScheduledProtocolSpec.Hardfork("Prague", 50);
protected final ScheduledProtocolSpec.Hardfork experimentalHardfork =
new ScheduledProtocolSpec.Hardfork("ExperimentalEips", 50);
new ScheduledProtocolSpec.Hardfork("ExperimentalEips", 70);

@Mock protected DefaultProtocolSchedule protocolSchedule;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;

import static org.assertj.core.api.Assertions.assertThat;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineTestSupport.fromErrorResp;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

Expand All @@ -32,7 +33,6 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
Expand Down Expand Up @@ -256,12 +256,4 @@ private List<BlobAndProofV1> fromSuccessResp(final JsonRpcResponse resp) {
list.forEach(obj -> blobAndProofV1s.add((BlobAndProofV1) obj));
return blobAndProofV1s;
}

private RpcErrorType fromErrorResp(final JsonRpcResponse resp) {
assertThat(resp.getType()).isEqualTo(RpcResponseType.ERROR);
return Optional.of(resp)
.map(JsonRpcErrorResponse.class::cast)
.map(JsonRpcErrorResponse::getErrorType)
.get();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;

import static org.assertj.core.api.Assertions.assertThat;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineTestSupport.fromErrorResp;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.INVALID_RANGE_REQUEST_TOO_LARGE;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.spy;
Expand All @@ -29,10 +30,8 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequest;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.BlockResultFactory;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EngineGetPayloadBodiesResultV1;
import org.hyperledger.besu.ethereum.chain.MutableBlockchain;
Expand Down Expand Up @@ -259,12 +258,4 @@ private EngineGetPayloadBodiesResultV1 fromSuccessResp(final JsonRpcResponse res
.map(EngineGetPayloadBodiesResultV1.class::cast)
.get();
}

private RpcErrorType fromErrorResp(final JsonRpcResponse resp) {
assertThat(resp.getType()).isEqualTo(RpcResponseType.ERROR);
return Optional.of(resp)
.map(JsonRpcErrorResponse.class::cast)
.map(JsonRpcErrorResponse::getErrorType)
.get();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
*/
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine;

import static java.util.Collections.emptyList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineTestSupport.fromErrorResp;
import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.UNSUPPORTED_FORK;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
Expand All @@ -30,7 +33,10 @@
import org.hyperledger.besu.datatypes.TransactionType;
import org.hyperledger.besu.datatypes.Wei;
import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.EngineGetPayloadResultV3;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.results.Quantity;
import org.hyperledger.besu.ethereum.core.BlobTestFixture;
Expand Down Expand Up @@ -86,6 +92,76 @@ public void shouldReturnExpectedMethodName() {
assertThat(method.getName()).isEqualTo("engine_getPayloadV3");
}

@Test
public void shouldReturnUnsupportedForkIfBlockTimestampIsBeforeCancunMilestone() {
BlockHeader shanghaiHeader =
new BlockHeaderTestFixture()
.prevRandao(Bytes32.random())
.timestamp(cancunHardfork.milestone() - 1)
.excessBlobGas(BlobGas.of(10L))
.buildHeader();

PayloadIdentifier shanghaiPid =
PayloadIdentifier.forPayloadParams(
Hash.ZERO,
cancunHardfork.milestone() - 1,
Bytes32.random(),
Address.fromHexString("0x42"),
Optional.empty(),
Optional.empty());

BlockWithReceipts shanghaiBlock =
new BlockWithReceipts(
new Block(
shanghaiHeader, new BlockBody(emptyList(), emptyList(), Optional.of(emptyList()))),
emptyList());
PayloadWrapper payloadShanghai =
new PayloadWrapper(shanghaiPid, shanghaiBlock, Optional.empty());

when(mergeContext.retrievePayloadById(shanghaiPid)).thenReturn(Optional.of(payloadShanghai));

final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V3.getMethodName(), shanghaiPid);

assertThat(resp).isInstanceOf(JsonRpcErrorResponse.class);
assertThat(((JsonRpcErrorResponse) resp).getErrorType())
.isEqualTo(RpcErrorType.UNSUPPORTED_FORK);
}

@Test
public void shouldReturnUnsupportedForkIfBlockTimestampIsAfterPragueMilestone() {
BlockHeader pragueHeader =
new BlockHeaderTestFixture()
.prevRandao(Bytes32.random())
.timestamp(pragueHardfork.milestone())
.excessBlobGas(BlobGas.of(10L))
.buildHeader();

PayloadIdentifier postCancunPid =
PayloadIdentifier.forPayloadParams(
Hash.ZERO,
cancunHardfork.milestone(),
Bytes32.random(),
Address.fromHexString("0x42"),
Optional.empty(),
Optional.empty());

BlockWithReceipts pragueBlock =
new BlockWithReceipts(
new Block(
pragueHeader, new BlockBody(emptyList(), emptyList(), Optional.of(emptyList()))),
emptyList());
PayloadWrapper payloadPostCancun =
new PayloadWrapper(postCancunPid, pragueBlock, Optional.empty());

when(mergeContext.retrievePayloadById(postCancunPid))
.thenReturn(Optional.of(payloadPostCancun));

final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V3.getMethodName(), postCancunPid);
final JsonRpcError jsonRpcError = fromErrorResp(resp);
assertThat(jsonRpcError.getCode()).isEqualTo(UNSUPPORTED_FORK.getCode());
verify(engineCallListener, times(1)).executionEngineCalled();
}

@Override
@Test
public void shouldReturnBlockForKnownPayloadId() {
Expand Down
Loading