diff --git a/src/Nethermind/Nethermind.Consensus/Producers/PayloadAttributes.cs b/src/Nethermind/Nethermind.Consensus/Producers/PayloadAttributes.cs index 447e59afbd5..4f2a7f8bd3e 100644 --- a/src/Nethermind/Nethermind.Consensus/Producers/PayloadAttributes.cs +++ b/src/Nethermind/Nethermind.Consensus/Producers/PayloadAttributes.cs @@ -55,6 +55,8 @@ public string ToString(string indentation) } } +public enum PayloadAttributesValidationResult : byte { Success, InvalidParams, UnsupportedFork }; + public static class PayloadAttributesExtensions { public static string ComputePayloadId(this PayloadAttributes payloadAttributes, BlockHeader parentHeader) @@ -100,38 +102,69 @@ public static int GetVersion(this PayloadAttributes executionPayload) => public static int ExpectedEngineSpecVersion(this IReleaseSpec spec) => spec switch { - { WithdrawalsEnabled: true, IsEip4844Enabled: true } => EngineApiVersions.Cancun, + { IsEip4844Enabled: true } => EngineApiVersions.Cancun, { WithdrawalsEnabled: true } => EngineApiVersions.Shanghai, _ => EngineApiVersions.Paris }; - public static bool Validate( - this PayloadAttributes payloadAttributes, - IReleaseSpec spec, - int version, + public static PayloadAttributesValidationResult Validate( + this PayloadAttributes payloadAttributes, + ISpecProvider specProvider, + int apiVersion, + [NotNullWhen(false)] out string? error) => + Validate( + apiVersion: apiVersion, + actualVersion: payloadAttributes.GetVersion(), + expectedVersion: specProvider.GetSpec(ForkActivation.TimestampOnly(payloadAttributes.Timestamp)) + .ExpectedEngineSpecVersion(), + "PayloadAttributesV", + out error); + + public static PayloadAttributesValidationResult Validate( + int apiVersion, + int actualVersion, + int expectedVersion, + string methodName, [NotNullWhen(false)] out string? error) { - int actualVersion = payloadAttributes.GetVersion(); - int expectedVersion = spec.ExpectedEngineSpecVersion(); - - error = null; - if (actualVersion != expectedVersion) + if (apiVersion >= EngineApiVersions.Cancun) { - error = $"PayloadAttributesV{expectedVersion} expected"; + if (actualVersion == apiVersion && expectedVersion != apiVersion) + { + error = $"{methodName}{expectedVersion} expected"; + return PayloadAttributesValidationResult.UnsupportedFork; + } } - else if (actualVersion > version) + else if (apiVersion == EngineApiVersions.Shanghai) { - error = $"PayloadAttributesV{version} expected"; + if (actualVersion == apiVersion && expectedVersion >= EngineApiVersions.Cancun) + { + error = $"{methodName}{expectedVersion} expected"; + return PayloadAttributesValidationResult.UnsupportedFork; + } } - return error is null; - } - public static bool Validate(this PayloadAttributes payloadAttributes, - ISpecProvider specProvider, - int version, - [NotNullWhen(false)] out string? error) => - payloadAttributes.Validate( - specProvider.GetSpec(ForkActivation.TimestampOnly(payloadAttributes.Timestamp)), - version, - out error); + if (actualVersion == expectedVersion) + { + if (apiVersion >= EngineApiVersions.Cancun) + { + if (actualVersion == apiVersion) + { + error = null; + return PayloadAttributesValidationResult.Success; + } + } + else + { + if (apiVersion >= actualVersion) + { + error = null; + return PayloadAttributesValidationResult.Success; + } + } + } + + error = $"{methodName}{expectedVersion} expected"; + return PayloadAttributesValidationResult.InvalidParams; + } } diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs index 87d4df909ce..98a68e46382 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs @@ -133,7 +133,7 @@ public async Task GetPayloadV3_should_fail_on_unknown_payload() public async Task GetPayloadV3_should_return_all_the_blobs(int blobTxCount) { (IEngineRpcModule rpcModule, string payloadId, _) = await BuildAndGetPayloadV3Result(Cancun.Instance, blobTxCount); - var result = await rpcModule.engine_getPayloadV3(Bytes.FromHexString(payloadId)); + ResultWrapper result = await rpcModule.engine_getPayloadV3(Bytes.FromHexString(payloadId)); BlobsBundleV1 getPayloadResultBlobsBundle = result.Data!.BlobsBundle!; Assert.That(result.Data.ExecutionPayload.BlobGasUsed, Is.EqualTo(BlobGasCalculator.CalculateBlobGas(blobTxCount))); Assert.That(getPayloadResultBlobsBundle.Blobs!.Length, Is.EqualTo(blobTxCount)); @@ -163,7 +163,6 @@ public virtual async Task NewPayloadV3_should_decline_mempool_encoding(bool inMe [Test] public async Task NewPayloadV3_should_decline_null_blobversionedhashes() { - (JsonRpcService jsonRpcService, JsonRpcContext context, EthereumJsonSerializer serializer, ExecutionPayloadV3 executionPayload) = await PreparePayloadRequestEnv(); @@ -239,6 +238,31 @@ public async Task NewPayloadV3_should_decline_empty_fields() } } + [TestCaseSource(nameof(ForkchoiceUpdatedV3DeclinedTestCaseSource))] + [TestCaseSource(nameof(ForkchoiceUpdatedV3AcceptedTestCaseSource))] + + public async Task ForkChoiceUpdated_should_return_proper_error_code(IReleaseSpec releaseSpec, string method, bool isBeaconRootSet) + { + MergeTestBlockchain chain = await CreateBlockchain(releaseSpec: releaseSpec); + IEngineRpcModule rpcModule = CreateEngineModule(chain); + ForkchoiceStateV1 fcuState = new(Keccak.Zero, Keccak.Zero, Keccak.Zero); + PayloadAttributes payloadAttributes = new() + { + Timestamp = chain.BlockTree.Head!.Timestamp, + PrevRandao = Keccak.Zero, + SuggestedFeeRecipient = Address.Zero, + Withdrawals = new List(), + ParentBeaconBlockRoot = isBeaconRootSet ? Keccak.Zero : null, + }; + + string response = await RpcTest.TestSerializedRequest(rpcModule, method, + chain.JsonSerializer.Serialize(fcuState), + chain.JsonSerializer.Serialize(payloadAttributes)); + JsonRpcErrorResponse errorResponse = chain.JsonSerializer.Deserialize(response); + + return errorResponse.Error?.Code ?? ErrorCodes.None; + } + private const string FurtherValidationStatus = "FurtherValidation"; [TestCaseSource(nameof(BlobVersionedHashesMatchTestSource))] @@ -327,6 +351,61 @@ public async Task NewPayloadV3_should_verify_blob_versioned_hashes_again return result.Data.Status; } + public static IEnumerable ForkchoiceUpdatedV3DeclinedTestCaseSource + { + get + { + yield return new TestCaseData(Shanghai.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV3), false) + { + TestName = "ForkchoiceUpdatedV3 To Request Shanghai Payload, Nil Beacon Root", + ExpectedResult = ErrorCodes.InvalidParams, + }; + yield return new TestCaseData(Shanghai.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV3), true) + { + TestName = "ForkchoiceUpdatedV3 To Request Shanghai Payload, Zero Beacon Root", + ExpectedResult = ErrorCodes.UnsupportedFork, + }; + yield return new TestCaseData(Shanghai.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV2), true) + { + TestName = "ForkchoiceUpdatedV2 To Request Shanghai Payload, Zero Beacon Root", + ExpectedResult = ErrorCodes.InvalidParams, + }; + + yield return new TestCaseData(Cancun.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV2), true) + { + TestName = "ForkchoiceUpdatedV2 To Request Cancun Payload, Zero Beacon Root", + ExpectedResult = ErrorCodes.InvalidParams, + }; + yield return new TestCaseData(Cancun.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV2), false) + { + TestName = "ForkchoiceUpdatedV2 To Request Cancun Payload, Nil Beacon Root", + ExpectedResult = ErrorCodes.UnsupportedFork, + }; + yield return new TestCaseData(Cancun.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV3), false) + { + TestName = "ForkchoiceUpdatedV3 To Request Cancun Payload, Nil Beacon Root", + ExpectedResult = ErrorCodes.InvalidParams, + }; + } + } + + public static IEnumerable ForkchoiceUpdatedV3AcceptedTestCaseSource + { + get + { + yield return new TestCaseData(Shanghai.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV2), false) + { + TestName = "ForkchoiceUpdatedV2 To Request Shanghai Payload, Nil Beacon Root", + ExpectedResult = ErrorCodes.None, + }; + yield return new TestCaseData(Cancun.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV3), true) + { + TestName = "ForkchoiceUpdatedV3 To Request Cancun Payload, Zero Beacon Root", + ExpectedResult = ErrorCodes.None, + }; + } + } + public static IEnumerable BlobVersionedHashesMatchTestSource { get diff --git a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs index 23a4c9740c9..ac4514765d9 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs @@ -39,10 +39,13 @@ public async Task> engine_newPayloadV1(ExecutionP private async Task> ForkchoiceUpdated(ForkchoiceStateV1 forkchoiceState, PayloadAttributes? payloadAttributes, int version) { - if (payloadAttributes?.Validate(_specProvider, version, out string? error) == false) + string? error = null; + switch (payloadAttributes?.Validate(_specProvider, version, out error)) { - if (_logger.IsWarn) _logger.Warn(error); - return ResultWrapper.Fail(error, version >= EngineApiVersions.Cancun ? ErrorCodes.UnsupportedFork : ErrorCodes.InvalidParams); + case PayloadAttributesValidationResult.InvalidParams: + return ResultWrapper.Fail(error!, ErrorCodes.InvalidParams); + case PayloadAttributesValidationResult.UnsupportedFork: + return ResultWrapper.Fail(error!, ErrorCodes.UnsupportedFork); } if (await _locker.WaitAsync(_timeout))