From 2a9b4d1a58bc33a1af45967a9bb6601f8079aa38 Mon Sep 17 00:00:00 2001 From: Simon Dudley Date: Thu, 6 Feb 2025 12:02:29 +1000 Subject: [PATCH] Prague Engine API validation fixes (#8250) - Prague newPayload validation fixes - Testing the unsupported fork timestamp cases Signed-off-by: Simon Dudley Signed-off-by: Bhanu Pulluri --- .../AbstractEngineForkchoiceUpdated.java | 5 -- .../engine/AbstractEngineNewPayload.java | 12 +-- .../engine/EngineForkchoiceUpdatedV2.java | 6 ++ .../engine/EngineForkchoiceUpdatedV3.java | 7 +- .../methods/engine/EngineGetPayloadV3.java | 8 +- .../methods/engine/EngineNewPayloadV3.java | 8 +- .../methods/engine/ForkSupportHelper.java | 40 +++++++++- .../engine/AbstractEngineNewPayloadTest.java | 10 +-- .../engine/AbstractScheduledApiTest.java | 4 +- .../methods/engine/EngineGetBlobsV1Test.java | 10 +-- .../EngineGetPayloadBodiesByHashV1Test.java | 11 +-- .../engine/EngineGetPayloadV3Test.java | 76 +++++++++++++++++++ .../engine/EngineGetPayloadV4Test.java | 18 ++--- .../engine/EngineNewPayloadV2Test.java | 9 ++- .../engine/EngineNewPayloadV3Test.java | 55 ++++++++++++-- .../engine/EngineNewPayloadV4Test.java | 28 ++++++- .../methods/engine/EngineTestSupport.java | 35 +++++++++ .../feemarket/ExcessBlobGasCalculator.java | 2 +- 18 files changed, 276 insertions(+), 68 deletions(-) create mode 100644 ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineTestSupport.java 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 727677aa8ed..fd40f2262cd 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 @@ -170,11 +170,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) .log(); return maybeError.get(); } - ValidationResult forkValidationResult = - validateForkSupported(payloadAttributes.getTimestamp()); - if (!forkValidationResult.isValid()) { - return new JsonRpcErrorResponse(requestId, forkValidationResult); - } } final BlockHeader newHead = maybeNewHead.get(); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java index a4f267be971..6acf4e5a9c3 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayload.java @@ -153,6 +153,12 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) e); } + final ValidationResult forkValidationResult = + validateForkSupported(blockParam.getTimestamp()); + if (!forkValidationResult.isValid()) { + return new JsonRpcErrorResponse(reqId, forkValidationResult); + } + final ValidationResult parameterValidationResult = validateParameters( blockParam, @@ -163,12 +169,6 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext) return new JsonRpcErrorResponse(reqId, parameterValidationResult); } - final ValidationResult forkValidationResult = - validateForkSupported(blockParam.getTimestamp()); - if (!forkValidationResult.isValid()) { - return new JsonRpcErrorResponse(reqId, forkValidationResult); - } - final Optional> maybeVersionedHashes; try { maybeVersionedHashes = extractVersionedHashes(maybeVersionedHashParam); 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 1dedf2919a3..6ade5b6ad6e 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 @@ -59,6 +59,12 @@ protected Optional isPayloadAttributesValid( return Optional.of(new JsonRpcErrorResponse(requestId, getInvalidPayloadAttributesError())); } + ValidationResult forkValidationResult = + validateForkSupported(payloadAttributes.getTimestamp()); + if (!forkValidationResult.isValid()) { + return Optional.of(new JsonRpcErrorResponse(requestId, forkValidationResult)); + } + 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 c06b119328f..55d76cfc903 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 @@ -81,6 +81,7 @@ protected ValidationResult validateForkSupported(final long blockT @Override protected Optional 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"); @@ -91,8 +92,10 @@ protected Optional 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 forkValidationResult = + validateForkSupported(payloadAttributes.getTimestamp()); + if (!forkValidationResult.isValid()) { + return Optional.of(new JsonRpcErrorResponse(requestId, forkValidationResult)); } return Optional.empty(); diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3.java index f0b026ef450..25e7ac487d3 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3.java @@ -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; @@ -68,6 +69,11 @@ protected JsonRpcResponse createResponse( @Override protected ValidationResult validateForkSupported(final long blockTimestamp) { - return ForkSupportHelper.validateForkSupported(CANCUN, cancunMilestone, blockTimestamp); + return ForkSupportHelper.validateForkSupported( + CANCUN, + cancunMilestone, + PRAGUE, + protocolSchedule.flatMap(s -> s.milestoneFor(PRAGUE)), + blockTimestamp); } } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3.java index beb566cfc98..fe9cddad309 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3.java @@ -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; @@ -89,6 +90,11 @@ protected ValidationResult validateParameters( @Override protected ValidationResult validateForkSupported(final long blockTimestamp) { - return ForkSupportHelper.validateForkSupported(CANCUN, cancunMilestone, blockTimestamp); + return ForkSupportHelper.validateForkSupported( + CANCUN, + cancunMilestone, + PRAGUE, + protocolSchedule.flatMap(s -> s.milestoneFor(PRAGUE)), + blockTimestamp); } } diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/ForkSupportHelper.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/ForkSupportHelper.java index c5a022f997f..e8780f93d28 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/ForkSupportHelper.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/ForkSupportHelper.java @@ -21,22 +21,54 @@ import java.util.Optional; public class ForkSupportHelper { + public static ValidationResult validateForkSupported( final HardforkId firstSupportedHardforkId, - final Optional maybeForkMilestone, + final Optional 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 validateForkSupported( + final HardforkId firstSupportedHardforkId, + final Optional maybeFirstSupportedForkMilestone, + final HardforkId firstUnsupportedHardforkId, + final Optional 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(); diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java index 97e775cdb03..63c40ee4656 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractEngineNewPayloadTest.java @@ -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; @@ -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; @@ -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> maybeWithdrawals) { return createBlockHeaderFixture(maybeWithdrawals).buildHeader(); } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractScheduledApiTest.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractScheduledApiTest.java index 46fe7d3b6ce..46e12cba86b 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractScheduledApiTest.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/AbstractScheduledApiTest.java @@ -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; diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetBlobsV1Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetBlobsV1Test.java index 29c7ae82ed5..3b017712feb 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetBlobsV1Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetBlobsV1Test.java @@ -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; @@ -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; @@ -256,12 +256,4 @@ private List 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(); - } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadBodiesByHashV1Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadBodiesByHashV1Test.java index f7752a7977f..f1eec592335 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadBodiesByHashV1Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadBodiesByHashV1Test.java @@ -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; @@ -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; @@ -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(); - } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3Test.java index 1fe3b44d451..f656ac91a89 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV3Test.java @@ -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; @@ -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; @@ -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() { diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV4Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV4Test.java index 4a241ce3867..d706b42a41d 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV4Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineGetPayloadV4Test.java @@ -93,6 +93,15 @@ public void shouldReturnExpectedMethodName() { assertThat(method.getName()).isEqualTo("engine_getPayloadV4"); } + @Test + public void shouldReturnUnsupportedForkIfBlockTimestampIsBeforePragueMilestone() { + final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V4.getMethodName(), mockPid); + + assertThat(resp).isInstanceOf(JsonRpcErrorResponse.class); + assertThat(((JsonRpcErrorResponse) resp).getErrorType()) + .isEqualTo(RpcErrorType.UNSUPPORTED_FORK); + } + @Override @Test public void shouldReturnBlockForKnownPayloadId() { @@ -220,15 +229,6 @@ public void shouldExcludeEmptyRequestsInRequestsList() { }); } - @Test - public void shouldReturnUnsupportedFork() { - final var resp = resp(RpcMethod.ENGINE_GET_PAYLOAD_V4.getMethodName(), mockPid); - - assertThat(resp).isInstanceOf(JsonRpcErrorResponse.class); - assertThat(((JsonRpcErrorResponse) resp).getErrorType()) - .isEqualTo(RpcErrorType.UNSUPPORTED_FORK); - } - @Override protected String getMethodName() { return RpcMethod.ENGINE_GET_PAYLOAD_V4.getMethodName(); diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java index 3632084accd..963bbf66ef1 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV2Test.java @@ -16,6 +16,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID; +import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine.EngineTestSupport.fromErrorResp; import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters.WithdrawalParameterTestFixture.WITHDRAWAL_PARAM_1; import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.INVALID_BLOB_GAS_USED_PARAMS; import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.INVALID_PARAMS; @@ -34,12 +35,14 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcError; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.Withdrawal; +import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec; import org.hyperledger.besu.ethereum.mainnet.WithdrawalsValidator; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -52,6 +55,10 @@ @MockitoSettings(strictness = Strictness.LENIENT) public class EngineNewPayloadV2Test extends AbstractEngineNewPayloadTest { + protected Set supportedHardforks() { + return Set.of(shanghaiHardfork); + } + public EngineNewPayloadV2Test() {} @Override @@ -177,7 +184,7 @@ public void shouldReturnInvalidIfWithdrawalsIsNull_WhenWithdrawalsAllowed() { @Test public void shouldReturnUnsupportedForkIfBlockTimestampIsAfterCancunMilestone() { - // Cancun starte at timestamp 30 + // Cancun started at timestamp 30 final long blockTimestamp = 31L; BlockHeader blockHeader = createBlockHeaderFixture(Optional.of(Collections.emptyList())) diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3Test.java index 2fb45124821..cf3da05a1ed 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV3Test.java @@ -17,7 +17,9 @@ import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.ExecutionEngineJsonRpcMethod.EngineStatus.INVALID; +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_PARAMS; +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; @@ -52,6 +54,7 @@ import org.hyperledger.besu.ethereum.core.encoding.TransactionEncoder; import org.hyperledger.besu.ethereum.mainnet.BodyValidation; import org.hyperledger.besu.ethereum.mainnet.CancunTargetingGasLimitCalculator; +import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec; import org.hyperledger.besu.ethereum.mainnet.ValidationResult; import org.hyperledger.besu.evm.gascalculator.CancunGasCalculator; import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem; @@ -59,13 +62,14 @@ import java.math.BigInteger; import java.util.List; import java.util.Optional; +import java.util.Random; +import java.util.Set; import java.util.function.Supplier; import com.google.common.base.Suppliers; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; @@ -78,6 +82,11 @@ public class EngineNewPayloadV3Test extends EngineNewPayloadV2Test { public EngineNewPayloadV3Test() {} + @Override + protected Set supportedHardforks() { + return Set.of(cancunHardfork); + } + @Override @Test public void shouldReturnExpectedMethodName() { @@ -104,6 +113,37 @@ public void before() { .thenReturn(mock(CancunTargetingGasLimitCalculator.class)); } + @Override + public void shouldReturnUnsupportedForkIfBlockTimestampIsAfterCancunMilestone() { + // only relevant for v2 + } + + @Test + public void shouldReturnUnsupportedForkIfBlockTimestampIsAfterPragueMilestone() { + final BlockHeader pragueHeader = + createBlockHeaderFixture(Optional.of(emptyList())) + .timestamp(pragueHardfork.milestone()) + .buildHeader(); + + var resp = resp(mockEnginePayload(pragueHeader, emptyList(), null)); + final JsonRpcError jsonRpcError = fromErrorResp(resp); + assertThat(jsonRpcError.getCode()).isEqualTo(UNSUPPORTED_FORK.getCode()); + verify(engineCallListener, times(1)).executionEngineCalled(); + } + + @Test + public void shouldReturnUnsupportedForkIfBlockTimestampIsBeforeCancunMilestone() { + final BlockHeader shanghaiHeader = + createBlockHeaderFixture(Optional.of(emptyList())) + .timestamp(cancunHardfork.milestone() - 1) + .buildHeader(); + + var resp = resp(mockEnginePayload(shanghaiHeader, emptyList(), null)); + final JsonRpcError jsonRpcError = fromErrorResp(resp); + assertThat(jsonRpcError.getCode()).isEqualTo(UNSUPPORTED_FORK.getCode()); + verify(engineCallListener, times(1)).executionEngineCalled(); + } + @Test public void shouldInvalidVersionedHash_whenShortVersionedHash() { final Bytes shortHash = Bytes.fromHexString("0x" + "69".repeat(31)); @@ -213,6 +253,7 @@ public void shouldValidateBlobGasUsedCorrectly() { // V3 must return error if null blobGasUsed BlockHeader blockHeader = createBlockHeaderFixture(Optional.of(emptyList())) + .timestamp(getTimestampForSupportedFork()) .excessBlobGas(BlobGas.MAX_BLOB_GAS) .blobGasUsed(null) .buildHeader(); @@ -231,6 +272,7 @@ public void shouldValidateExcessBlobGasCorrectly() { // V3 must return error if null excessBlobGas BlockHeader blockHeader = createBlockHeaderFixture(Optional.of(emptyList())) + .timestamp(getTimestampForSupportedFork()) .excessBlobGas(null) .blobGasUsed(100L) .buildHeader(); @@ -280,12 +322,6 @@ private Transaction createTransactionWithBlobs() { .createTransaction(senderKeys); } - @Override - @Disabled - public void shouldReturnUnsupportedForkIfBlockTimestampIsAfterCancunMilestone() { - // only relevant for v2 - } - @Override protected JsonRpcResponse resp(final EnginePayloadParameter payload) { Object[] params = @@ -295,4 +331,9 @@ protected JsonRpcResponse resp(final EnginePayloadParameter payload) { return method.response( new JsonRpcRequestContext(new JsonRpcRequest("2.0", this.method.getName(), params))); } + + protected long getTimestampForSupportedFork() { + final int index = new Random().nextInt(supportedHardforks().size()); + return supportedHardforks().stream().toList().get(index).milestone(); + } } diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV4Test.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV4Test.java index 8dc84418bc2..913add4f16c 100644 --- a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV4Test.java +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayloadV4Test.java @@ -17,7 +17,9 @@ import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; import static org.hyperledger.besu.ethereum.api.graphql.internal.response.GraphQLError.INVALID_PARAMS; +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_EXECUTION_REQUESTS_PARAMS; +import static org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.RpcErrorType.UNSUPPORTED_FORK; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; @@ -41,6 +43,7 @@ import org.hyperledger.besu.ethereum.core.Request; import org.hyperledger.besu.ethereum.core.Withdrawal; import org.hyperledger.besu.ethereum.mainnet.BodyValidation; +import org.hyperledger.besu.ethereum.mainnet.ScheduledProtocolSpec; import org.hyperledger.besu.ethereum.mainnet.ValidationResult; import org.hyperledger.besu.ethereum.mainnet.requests.MainnetRequestsValidator; import org.hyperledger.besu.ethereum.mainnet.requests.ProhibitedRequestValidator; @@ -50,6 +53,7 @@ import java.util.Comparator; import java.util.List; import java.util.Optional; +import java.util.Set; import org.apache.tuweni.bytes.Bytes; import org.apache.tuweni.bytes.Bytes32; @@ -63,6 +67,11 @@ public class EngineNewPayloadV4Test extends EngineNewPayloadV3Test { public EngineNewPayloadV4Test() {} + @Override + protected Set supportedHardforks() { + return Set.of(pragueHardfork); + } + private static final List VALID_REQUESTS = List.of( new Request(RequestType.DEPOSIT, Bytes.of(1)), @@ -93,6 +102,22 @@ public void shouldReturnExpectedMethodName() { assertThat(method.getName()).isEqualTo("engine_newPayloadV4"); } + @Override + public void shouldReturnUnsupportedForkIfBlockTimestampIsAfterPragueMilestone() { + // Only relevant for V3 + } + + @Test + public void shouldReturnUnsupportedForkIfBlockTimestampIsBeforePragueMilestone() { + final BlockHeader cancunHeader = createBlockHeaderFixtureForV3(Optional.empty()).buildHeader(); + + var resp = resp(mockEnginePayload(cancunHeader, emptyList())); + + final JsonRpcError jsonRpcError = fromErrorResp(resp); + assertThat(jsonRpcError.getCode()).isEqualTo(UNSUPPORTED_FORK.getCode()); + verify(engineCallListener, times(1)).executionEngineCalled(); + } + @Test public void shouldReturnInvalidIfRequestsIsNull_WhenRequestsAllowed() { var resp = @@ -173,6 +198,7 @@ private BlockHeader createValidBlockHeaderForV4( final Optional> maybeWithdrawals) { return createBlockHeaderFixtureForV3(maybeWithdrawals) .requestsHash(BodyValidation.requestsHash(VALID_REQUESTS)) + .timestamp(pragueHardfork.milestone()) .buildHeader(); } @@ -181,7 +207,7 @@ private BlockHeaderTestFixture createBlockHeaderFixtureForV3( BlockHeader parentBlockHeader = new BlockHeaderTestFixture() .baseFeePerGas(Wei.ONE) - .timestamp(pragueHardfork.milestone()) + .timestamp(pragueHardfork.milestone() - 2) // cancun parent .excessBlobGas(BlobGas.ZERO) .blobGasUsed(0L) .buildHeader(); diff --git a/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineTestSupport.java b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineTestSupport.java new file mode 100644 index 00000000000..688d78aedc6 --- /dev/null +++ b/ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineTestSupport.java @@ -0,0 +1,35 @@ +/* + * Copyright contributors to Besu. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.engine; + +import static org.assertj.core.api.Assertions.assertThat; + +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.plugin.services.rpc.RpcResponseType; + +import java.util.Optional; + +public class EngineTestSupport { + + static JsonRpcError fromErrorResp(final JsonRpcResponse resp) { + assertThat(resp.getType()).isEqualTo(RpcResponseType.ERROR); + return Optional.of(resp) + .map(JsonRpcErrorResponse.class::cast) + .map(JsonRpcErrorResponse::getError) + .get(); + } +} diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/feemarket/ExcessBlobGasCalculator.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/feemarket/ExcessBlobGasCalculator.java index f6372097b7f..5f4d1f28049 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/feemarket/ExcessBlobGasCalculator.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/feemarket/ExcessBlobGasCalculator.java @@ -24,7 +24,7 @@ public class ExcessBlobGasCalculator { * public class ExcessBlobGasCalculator { /** Calculates the excess blob gas for a parent block * header. * - * @param protocolSpec The protocol specification. + * @param protocolSpec The protocol specification of the current block. * @param parentHeader The parent block header. * @return The excess blob gas. */