From 59d315d7e2dfce9b3df72b2b1b92f233be35538c Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Fri, 14 Jul 2023 13:43:28 +0200 Subject: [PATCH 1/9] Introduce ExecutionPayloadParams concept to be able to better compose and validate complex method signatures --- .../Data/ExecutionPayload.cs | 7 ++- .../Data/ExecutionPayloadV3.cs | 9 +--- .../Data/IExecutionPayloadParams.cs | 48 +++++++++++++++++++ .../EngineRpcModule.Cancun.cs | 39 +-------------- .../EngineRpcModule.Paris.cs | 14 +++++- .../EngineRpcModule.Shanghai.cs | 2 +- 6 files changed, 66 insertions(+), 53 deletions(-) create mode 100644 src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs index 5638afebb00..8104468c7dc 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs @@ -18,7 +18,7 @@ namespace Nethermind.Merge.Plugin.Data; /// /// Represents an object mapping the ExecutionPayload structure of the beacon chain spec. /// -public class ExecutionPayload : IForkValidator +public class ExecutionPayload : IForkValidator, IExecutionPayloadParams { public ExecutionPayload() { } // Needed for tests @@ -165,6 +165,8 @@ public void SetTransactions(params Transaction[] transactions) public override string ToString() => $"{BlockNumber} ({BlockHash.ToShortString()})"; + ExecutionPayload IExecutionPayloadParams.ExecutionPayload => this; + public virtual bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(false)] out string? error) { int GetVersion() => Withdrawals is null ? 1 : 2; @@ -189,7 +191,4 @@ public virtual bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen( public virtual bool ValidateFork(ISpecProvider specProvider) => !specProvider.GetSpec(BlockNumber, Timestamp).IsEip4844Enabled; - - public bool ValidateParams(ISpecProvider specProvider, int version, [NotNullWhen(false)] out string? error) => - ValidateParams(specProvider.GetSpec(BlockNumber, Timestamp), version, out error); } diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs index 1c6dbc4b1dc..088d6cacbe5 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs @@ -47,13 +47,6 @@ public override bool TryGetBlock(out Block? block, UInt256? totalDifficulty = nu return true; } - public override bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(false)] out string? error) - { - // handled by `JsonObject` attribute of the class - error = null; - return true; - } - public override bool ValidateFork(ISpecProvider specProvider) => - specProvider.GetSpec(BlockNumber, Timestamp).IsEip4844Enabled; + !specProvider.GetSpec(BlockNumber, Timestamp).IsEip4844Enabled; } diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs new file mode 100644 index 00000000000..4dee6f34257 --- /dev/null +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs @@ -0,0 +1,48 @@ +// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited +// SPDX-License-Identifier: LGPL-3.0-only + +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using Nethermind.Core.Extensions; +using Nethermind.Core.Specs; + +namespace Nethermind.Merge.Plugin.Data; + +public interface IExecutionPayloadParams +{ + ExecutionPayload ExecutionPayload { get; } + bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(false)] out string? error); +} + +public class ExecutionPayloadV3Params : IExecutionPayloadParams +{ + private readonly ExecutionPayloadV3 _executionPayload; + private readonly byte[]?[] _blobVersionedHashes; + + public ExecutionPayloadV3Params(ExecutionPayloadV3 executionPayload, byte[]?[] blobVersionedHashes) + { + _executionPayload = executionPayload; + _blobVersionedHashes = blobVersionedHashes; + } + + public ExecutionPayload ExecutionPayload => _executionPayload; + + public bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(false)] out string? error) + { + error = null; + + static IEnumerable FlattenHashesFromTransactions(ExecutionPayloadV3 payload) => + payload.GetTransactions() + .Where(t => t.BlobVersionedHashes is not null) + .SelectMany(t => t.BlobVersionedHashes!); + + if (FlattenHashesFromTransactions(_executionPayload).SequenceEqual(_blobVersionedHashes, Bytes.NullableEqualityComparer)) + { + return true; + } + + error = "Blob versioned hashes do not match"; + return false; + } +} diff --git a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs index 470d191e67d..41aa57d7b61 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Cancun.cs @@ -17,44 +17,7 @@ public partial class EngineRpcModule : IEngineRpcModule private readonly IAsyncHandler _getPayloadHandlerV3; public Task> engine_newPayloadV3(ExecutionPayloadV3 executionPayload, byte[]?[] blobVersionedHashes) => - ValidateFork(executionPayload) ?? PreValidatePayload(executionPayload, blobVersionedHashes) ?? NewPayload(executionPayload, 3); - - private ResultWrapper? PreValidatePayload(ExecutionPayloadV3 executionPayload, byte[]?[] blobVersionedHashes) - { - ResultWrapper ErrorResult(string error) - { - if (_logger.IsWarn) _logger.Warn(error); - return ResultWrapper.Success( - new PayloadStatusV1 - { - Status = PayloadStatus.Invalid, - LatestValidHash = null, - ValidationError = error - }); - } - - static IEnumerable FlattenHashesFromTransactions(ExecutionPayloadV3 payload) => - payload.GetTransactions() - .Where(t => t.BlobVersionedHashes is not null) - .SelectMany(t => t.BlobVersionedHashes!); - - return !FlattenHashesFromTransactions(executionPayload).SequenceEqual(blobVersionedHashes, Bytes.NullableEqualityComparer) - ? ErrorResult("Blob versioned hashes do not match") - : null; - } - - private ResultWrapper? ValidateFork(ExecutionPayload executionPayload) - { - if (executionPayload.ValidateFork(_specProvider)) - { - return null; - } - else - { - if (_logger.IsWarn) _logger.Warn($"The payload is not supported by the current fork"); - return ResultWrapper.Fail("unsupported fork", ErrorCodes.UnsupportedFork); - } - } + NewPayload(new ExecutionPayloadV3Params(executionPayload, blobVersionedHashes), 3); public async Task> engine_getPayloadV3(byte[] payloadId) => await _getPayloadHandlerV3.HandleAsync(payloadId); diff --git a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs index 8a09b6399ce..00a3a34300c 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs @@ -6,6 +6,7 @@ using System.Threading; using System.Threading.Tasks; using Nethermind.Consensus.Producers; +using Nethermind.Core.Specs; using Nethermind.JsonRpc; using Nethermind.Merge.Plugin.Data; using Nethermind.Merge.Plugin.GC; @@ -64,9 +65,18 @@ private async Task> ForkchoiceUpdated(F } } - private async Task> NewPayload(ExecutionPayload executionPayload, int version) + private async Task> NewPayload(IExecutionPayloadParams executionPayloadParams, int version) { - if (!executionPayload.ValidateParams(_specProvider, version, out string? error)) + ExecutionPayload executionPayload = executionPayloadParams.ExecutionPayload; + + if (!executionPayload.ValidateFork(_specProvider)) + { + if (_logger.IsWarn) _logger.Warn($"The payload is not supported by the current fork"); + return ResultWrapper.Fail("unsupported fork", ErrorCodes.UnsupportedFork); + } + + IReleaseSpec releaseSpec = _specProvider.GetSpec(executionPayload.BlockNumber, executionPayload.Timestamp); + if (!executionPayloadParams.ValidateParams(releaseSpec, version, out string? error)) { if (_logger.IsWarn) _logger.Warn(error); return ResultWrapper.Fail(error, ErrorCodes.InvalidParams); diff --git a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Shanghai.cs b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Shanghai.cs index ec1e72e540b..99792015ae3 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Shanghai.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Shanghai.cs @@ -30,5 +30,5 @@ public Task> engine_forkchoiceUpdatedV2 => _executionGetPayloadBodiesByRangeV1Handler.Handle(start, count); public Task> engine_newPayloadV2(ExecutionPayload executionPayload) - => ValidateFork(executionPayload) ?? NewPayload(executionPayload, 2); + => NewPayload(executionPayload, 2); } From 3ab4ee5adf53fde8b606dbbcd3a9101e1c7ddeb0 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Fri, 14 Jul 2023 13:47:47 +0200 Subject: [PATCH 2/9] whitespace --- .../Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs index 4dee6f34257..ba7df9b6f25 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs @@ -1,4 +1,4 @@ -// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited +// SPDX-FileCopyrightText: 2023 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only using System.Collections.Generic; From 21c72e4e1ad32f2b956c8afe047719915f739e49 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Fri, 14 Jul 2023 14:17:03 +0200 Subject: [PATCH 3/9] fix CreateBlockRequest --- .../EngineModuleTests.HelperFunctions.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs index 8f48a6e3d70..a41e96748e4 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs @@ -88,7 +88,12 @@ private static ExecutionPayload CreateBlockRequest(ExecutionPayload parent, Addr => CreateBlockRequestInternal(parent, miner, withdrawals, transactions: transactions); private static ExecutionPayloadV3 CreateBlockRequestV3(ExecutionPayload parent, Address miner, IList? withdrawals = null, ulong? dataGasUsed = null, ulong? excessDataGas = null, Transaction[]? transactions = null) - => CreateBlockRequestInternal(parent, miner, withdrawals, dataGasUsed, excessDataGas, transactions: transactions); + { + ExecutionPayloadV3 blockRequestV3 = CreateBlockRequestInternal(parent, miner, withdrawals, dataGasUsed, excessDataGas, transactions: transactions) + blockRequestV3.DataGasUsed = dataGasUsed; + blockRequestV3.ExcessDataGas = excessDataGas; + return blockRequestV3; + } private static T CreateBlockRequestInternal(ExecutionPayload parent, Address miner, IList? withdrawals = null, ulong? dataGasUsed = null, ulong? excessDataGas = null, Transaction[]? transactions = null) where T : ExecutionPayload, new() { @@ -106,12 +111,6 @@ private static ExecutionPayloadV3 CreateBlockRequestV3(ExecutionPayload parent, Withdrawals = withdrawals, }; - if (blockRequest is ExecutionPayloadV3 blockRequestV3) - { - blockRequestV3.DataGasUsed = dataGasUsed; - blockRequestV3.ExcessDataGas = excessDataGas; - } - blockRequest.SetTransactions(transactions ?? Array.Empty()); TryCalculateHash(blockRequest, out Keccak? hash); blockRequest.BlockHash = hash; From ea7ca63100e55f734a8b25c386ddc651a3434b8d Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Fri, 14 Jul 2023 14:21:58 +0200 Subject: [PATCH 4/9] Revert "fix CreateBlockRequest" This reverts commit 21c72e4e1ad32f2b956c8afe047719915f739e49. --- .../EngineModuleTests.HelperFunctions.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs index a41e96748e4..8f48a6e3d70 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs @@ -88,12 +88,7 @@ private static ExecutionPayload CreateBlockRequest(ExecutionPayload parent, Addr => CreateBlockRequestInternal(parent, miner, withdrawals, transactions: transactions); private static ExecutionPayloadV3 CreateBlockRequestV3(ExecutionPayload parent, Address miner, IList? withdrawals = null, ulong? dataGasUsed = null, ulong? excessDataGas = null, Transaction[]? transactions = null) - { - ExecutionPayloadV3 blockRequestV3 = CreateBlockRequestInternal(parent, miner, withdrawals, dataGasUsed, excessDataGas, transactions: transactions) - blockRequestV3.DataGasUsed = dataGasUsed; - blockRequestV3.ExcessDataGas = excessDataGas; - return blockRequestV3; - } + => CreateBlockRequestInternal(parent, miner, withdrawals, dataGasUsed, excessDataGas, transactions: transactions); private static T CreateBlockRequestInternal(ExecutionPayload parent, Address miner, IList? withdrawals = null, ulong? dataGasUsed = null, ulong? excessDataGas = null, Transaction[]? transactions = null) where T : ExecutionPayload, new() { @@ -111,6 +106,12 @@ private static ExecutionPayloadV3 CreateBlockRequestV3(ExecutionPayload parent, Withdrawals = withdrawals, }; + if (blockRequest is ExecutionPayloadV3 blockRequestV3) + { + blockRequestV3.DataGasUsed = dataGasUsed; + blockRequestV3.ExcessDataGas = excessDataGas; + } + blockRequest.SetTransactions(transactions ?? Array.Empty()); TryCalculateHash(blockRequest, out Keccak? hash); blockRequest.BlockHash = hash; From 0d910ee8904439bd3e1208291a456c7784f48bcb Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Mon, 17 Jul 2023 12:21:35 +0200 Subject: [PATCH 5/9] fix --- .../Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs index 088d6cacbe5..94f7170cd02 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs @@ -48,5 +48,5 @@ public override bool TryGetBlock(out Block? block, UInt256? totalDifficulty = nu } public override bool ValidateFork(ISpecProvider specProvider) => - !specProvider.GetSpec(BlockNumber, Timestamp).IsEip4844Enabled; + specProvider.GetSpec(BlockNumber, Timestamp).IsEip4844Enabled; } From 9d1c05101bc484c301362a72f414bf30e3c78a09 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Mon, 17 Jul 2023 13:00:31 +0200 Subject: [PATCH 6/9] fixes --- .../EngineModuleTests.V3.cs | 2 +- .../Data/ExecutionPayload.cs | 6 +++--- .../Data/IExecutionPayloadParams.cs | 13 +++++++------ .../Nethermind.Merge.Plugin/Data/PayloadStatusV1.cs | 5 +++-- .../EngineRpcModule.Paris.cs | 7 +++++-- 5 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs index 28e355fa24a..16efa3bb8a3 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs @@ -43,7 +43,7 @@ public async Task NewPayloadV1_should_decline_post_cancun() ResultWrapper errorCode = (await rpcModule.engine_newPayloadV1(executionPayload)); - Assert.That(errorCode.ErrorCode, Is.EqualTo(ErrorCodes.InvalidParams)); + Assert.That(errorCode.ErrorCode, Is.EqualTo(ErrorCodes.UnsupportedFork)); } [Test] diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs index 8104468c7dc..e94c999adc1 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs @@ -167,14 +167,14 @@ public void SetTransactions(params Transaction[] transactions) ExecutionPayload IExecutionPayloadParams.ExecutionPayload => this; - public virtual bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(false)] out string? error) + public virtual ValidationResult ValidateParams(IReleaseSpec spec, int version, out string? error) { int GetVersion() => Withdrawals is null ? 1 : 2; if (spec.IsEip4844Enabled) { error = "ExecutionPayloadV3 expected"; - return false; + return ValidationResult.Fail; } int actualVersion = GetVersion(); @@ -186,7 +186,7 @@ public virtual bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen( _ => actualVersion > version ? $"ExecutionPayloadV{version} expected" : null }; - return error is null; + return ValidationResult.Fail; } public virtual bool ValidateFork(ISpecProvider specProvider) => diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs index ba7df9b6f25..51723767251 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs @@ -12,9 +12,11 @@ namespace Nethermind.Merge.Plugin.Data; public interface IExecutionPayloadParams { ExecutionPayload ExecutionPayload { get; } - bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(false)] out string? error); + ValidationResult ValidateParams(IReleaseSpec spec, int version, out string? error); } +public enum ValidationResult : byte { Success, Fail, Invalid }; + public class ExecutionPayloadV3Params : IExecutionPayloadParams { private readonly ExecutionPayloadV3 _executionPayload; @@ -28,10 +30,8 @@ public ExecutionPayloadV3Params(ExecutionPayloadV3 executionPayload, byte[]?[] b public ExecutionPayload ExecutionPayload => _executionPayload; - public bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(false)] out string? error) + public ValidationResult ValidateParams(IReleaseSpec spec, int version, out string? error) { - error = null; - static IEnumerable FlattenHashesFromTransactions(ExecutionPayloadV3 payload) => payload.GetTransactions() .Where(t => t.BlobVersionedHashes is not null) @@ -39,10 +39,11 @@ public bool ValidateParams(IReleaseSpec spec, int version, [NotNullWhen(false)] if (FlattenHashesFromTransactions(_executionPayload).SequenceEqual(_blobVersionedHashes, Bytes.NullableEqualityComparer)) { - return true; + error = null; + return ValidationResult.Success; } error = "Blob versioned hashes do not match"; - return false; + return ValidationResult.Invalid; } } diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/PayloadStatusV1.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/PayloadStatusV1.cs index c92f8b9475d..8a27a65fb10 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/PayloadStatusV1.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/PayloadStatusV1.cs @@ -17,10 +17,11 @@ public class PayloadStatusV1 public static readonly PayloadStatusV1 Accepted = new() { Status = PayloadStatus.Accepted }; - public static PayloadStatusV1 Invalid(Keccak? latestValidHash) => new() + public static PayloadStatusV1 Invalid(Keccak? latestValidHash, string? validationError = null) => new() { Status = PayloadStatus.Invalid, - LatestValidHash = latestValidHash + LatestValidHash = latestValidHash, + ValidationError = validationError }; /// diff --git a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs index 00a3a34300c..1b515f0c8ff 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs @@ -76,10 +76,13 @@ private async Task> NewPayload(IExecutionPayloadP } IReleaseSpec releaseSpec = _specProvider.GetSpec(executionPayload.BlockNumber, executionPayload.Timestamp); - if (!executionPayloadParams.ValidateParams(releaseSpec, version, out string? error)) + ValidationResult validationResult = executionPayloadParams.ValidateParams(releaseSpec, version, out string? error); + if (validationResult != ValidationResult.Success) { if (_logger.IsWarn) _logger.Warn(error); - return ResultWrapper.Fail(error, ErrorCodes.InvalidParams); + return validationResult == ValidationResult.Fail + ? ResultWrapper.Fail(error!, ErrorCodes.InvalidParams) + : ResultWrapper.Success(PayloadStatusV1.Invalid(null, error)); } if (await _locker.WaitAsync(_timeout)) From 2c5baa4efda7c21a9f136c204462386bf5d6d7bd Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Mon, 17 Jul 2023 13:29:34 +0200 Subject: [PATCH 7/9] fix --- src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs index e94c999adc1..d8a3f79b253 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs @@ -186,7 +186,7 @@ public virtual ValidationResult ValidateParams(IReleaseSpec spec, int version, o _ => actualVersion > version ? $"ExecutionPayloadV{version} expected" : null }; - return ValidationResult.Fail; + return error is null? ValidationResult.Success : ValidationResult.Fail; } public virtual bool ValidateFork(ISpecProvider specProvider) => From a5defc0270c811ec038d7b3403461e0374c6cd8a Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Mon, 17 Jul 2023 13:33:28 +0200 Subject: [PATCH 8/9] fix whitespace --- src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs index d8a3f79b253..445b63a1fa7 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs @@ -186,7 +186,7 @@ public virtual ValidationResult ValidateParams(IReleaseSpec spec, int version, o _ => actualVersion > version ? $"ExecutionPayloadV{version} expected" : null }; - return error is null? ValidationResult.Success : ValidationResult.Fail; + return error is null ? ValidationResult.Success : ValidationResult.Fail; } public virtual bool ValidateFork(ISpecProvider specProvider) => From 4cf5ec5182072c0f3395dc159eef1212db9130d8 Mon Sep 17 00:00:00 2001 From: "lukasz.rozmej" Date: Mon, 17 Jul 2023 13:49:00 +0200 Subject: [PATCH 9/9] support V1 error codes --- .../Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs | 2 +- src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs index 16efa3bb8a3..28e355fa24a 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs @@ -43,7 +43,7 @@ public async Task NewPayloadV1_should_decline_post_cancun() ResultWrapper errorCode = (await rpcModule.engine_newPayloadV1(executionPayload)); - Assert.That(errorCode.ErrorCode, Is.EqualTo(ErrorCodes.UnsupportedFork)); + Assert.That(errorCode.ErrorCode, Is.EqualTo(ErrorCodes.InvalidParams)); } [Test] diff --git a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs index 1b515f0c8ff..585af416360 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs @@ -72,7 +72,7 @@ private async Task> NewPayload(IExecutionPayloadP if (!executionPayload.ValidateFork(_specProvider)) { if (_logger.IsWarn) _logger.Warn($"The payload is not supported by the current fork"); - return ResultWrapper.Fail("unsupported fork", ErrorCodes.UnsupportedFork); + return ResultWrapper.Fail("unsupported fork", version < 2 ? ErrorCodes.InvalidParams : ErrorCodes.UnsupportedFork); } IReleaseSpec releaseSpec = _specProvider.GetSpec(executionPayload.BlockNumber, executionPayload.Timestamp);