From 04939bea00de2a760ad8e508be3bf63e7ebb1ce0 Mon Sep 17 00:00:00 2001 From: Rohit Ranjan Date: Mon, 3 Feb 2025 18:06:14 +0530 Subject: [PATCH 1/5] requests validation hotfix --- .../Data/IExecutionPayloadParams.cs | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs index 87860936d86..1e8163b9ff3 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs @@ -51,7 +51,29 @@ public ValidationResult ValidateParams(IReleaseSpec spec, int version, out strin if (ExecutionRequests.Length > ExecutionRequestExtensions.MaxRequestsCount) { error = $"Execution requests must have less than {ExecutionRequestExtensions.MaxRequestsCount} items"; - return ValidationResult.Invalid; + return ValidationResult.Fail; + } + + // verification of the requests + for (int i = 0; i < ExecutionRequests.Length; i++) + { + if (ExecutionRequests[i] == null || ExecutionRequests[i].Length <= 1) + { + error = "Execution request data must be longer than 1 byte"; + return ValidationResult.Fail; + } + + if (i > 0 && ExecutionRequests[i][0] <= ExecutionRequests[i - 1][0]) + { + error = "Execution requests must be ordered by request_type in ascending order"; + return ValidationResult.Fail; + } + + if (ExecutionRequests.Skip(i + 1).Any(req => req != null && req[0] == ExecutionRequests[i][0])) + { + error = "Execution requests must not contain duplicate request_type"; + return ValidationResult.Fail; + } } } From 9b347d668fc001bd5d0bc5fb3877369031299ffe Mon Sep 17 00:00:00 2001 From: Rohit Ranjan Date: Mon, 3 Feb 2025 18:53:39 +0530 Subject: [PATCH 2/5] fix test --- src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PragueTests.cs | 2 +- .../Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PragueTests.cs b/src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PragueTests.cs index c6dec949795..566cab07626 100644 --- a/src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PragueTests.cs +++ b/src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PragueTests.cs @@ -11,7 +11,7 @@ namespace Ethereum.Blockchain.Pyspec.Test; [TestFixture] [Parallelizable(ParallelScope.All)] -[Explicit("These tests are not ready yet")] +// [Explicit("These tests are not ready yet")] public class PragueTests : BlockchainTestBase { [TestCaseSource(nameof(LoadTests))] diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs index f480e135dd5..72bbab78962 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs @@ -247,7 +247,7 @@ private async Task> ProduceBranchV4(IEngineRpcMo ExecutionPayloadV3? getPayloadResult = await BuildAndGetPayloadOnBranchV4(rpc, chain, parentHeader, parentBlock.Timestamp + 12, random ?? TestItem.KeccakA, Address.Zero); - PayloadStatusV1 payloadStatusResponse = (await rpc.engine_newPayloadV4(getPayloadResult, [], Keccak.Zero, executionRequests: withRequests ? ExecutionRequestsProcessorMock.Requests : new byte[][] { [], [], [] })).Data; + PayloadStatusV1 payloadStatusResponse = (await rpc.engine_newPayloadV4(getPayloadResult, [], Keccak.Zero, executionRequests: withRequests ? ExecutionRequestsProcessorMock.Requests : new byte[][] { })).Data; payloadStatusResponse.Status.Should().Be(PayloadStatus.Valid); if (setHead) { From d134e1f063f68ca3cb0428b57b84d88154854bf0 Mon Sep 17 00:00:00 2001 From: Rohit Ranjan Date: Mon, 3 Feb 2025 18:54:28 +0530 Subject: [PATCH 3/5] revert prague tests --- src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PragueTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PragueTests.cs b/src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PragueTests.cs index 566cab07626..c6dec949795 100644 --- a/src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PragueTests.cs +++ b/src/Nethermind/Ethereum.Blockchain.Pyspec.Test/PragueTests.cs @@ -11,7 +11,7 @@ namespace Ethereum.Blockchain.Pyspec.Test; [TestFixture] [Parallelizable(ParallelScope.All)] -// [Explicit("These tests are not ready yet")] +[Explicit("These tests are not ready yet")] public class PragueTests : BlockchainTestBase { [TestCaseSource(nameof(LoadTests))] From c45f2d3710b2a19c646a93cfd576cf0c8a1568a8 Mon Sep 17 00:00:00 2001 From: Rohit Ranjan Date: Tue, 4 Feb 2025 14:01:39 +0530 Subject: [PATCH 4/5] add tests --- .../EngineModuleTests.V4.cs | 33 +++++++++++++++++++ .../Data/IExecutionPayloadParams.cs | 8 +---- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs index 72bbab78962..9162ec4fde2 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs @@ -200,6 +200,39 @@ public async Task NewPayloadV4_reject_payload_with_bad_authorization_list_rlp() Assert.That(response.Data.Status, Is.EqualTo("INVALID")); } + [Test] + public async Task NewPayloadV4_reject_payload_with_bad_execution_requests() + { + ExecutionRequestsProcessorMock executionRequestsProcessorMock = new(); + using MergeTestBlockchain chain = await CreateBlockchain(Prague.Instance, null, null, null, executionRequestsProcessorMock); + IEngineRpcModule rpc = CreateEngineModule(chain); + Hash256 lastHash = (await ProduceBranchV4(rpc, chain, 10, CreateParentBlockRequestOnHead(chain.BlockTree), true, withRequests: true)) + .LastOrDefault()?.BlockHash ?? Keccak.Zero; + + Block TestBlock = Build.A.Block.WithNumber(chain.BlockTree.Head!.Number + 1).TestObject; + ExecutionPayloadV3 executionPayload = ExecutionPayloadV3.Create(TestBlock); + + // must reject if execution requests types are not in ascending order + var response = await rpc.engine_newPayloadV4( + executionPayload, + [], + TestBlock.ParentBeaconBlockRoot, + executionRequests: [Bytes.FromHexString("0x0001"), Bytes.FromHexString("0x0101"), Bytes.FromHexString("0x0101")] + ); + + Assert.That(response.ErrorCode, Is.EqualTo(ErrorCodes.InvalidParams)); + + //must reject if one of the execution requests size is <= 1 byte + response = await rpc.engine_newPayloadV4( + executionPayload, + [], + TestBlock.ParentBeaconBlockRoot, + executionRequests: [Bytes.FromHexString("0x0001"), Bytes.FromHexString("0x01"), Bytes.FromHexString("0x0101")] + ); + + Assert.That(response.ErrorCode, Is.EqualTo(ErrorCodes.InvalidParams)); + } + [TestCase(30)] public async Task can_progress_chain_one_by_one_v4(int count) { diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs index 1e8163b9ff3..41e6f931b3f 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs @@ -65,13 +65,7 @@ public ValidationResult ValidateParams(IReleaseSpec spec, int version, out strin if (i > 0 && ExecutionRequests[i][0] <= ExecutionRequests[i - 1][0]) { - error = "Execution requests must be ordered by request_type in ascending order"; - return ValidationResult.Fail; - } - - if (ExecutionRequests.Skip(i + 1).Any(req => req != null && req[0] == ExecutionRequests[i][0])) - { - error = "Execution requests must not contain duplicate request_type"; + error = "Execution requests must not contain duplicates and be ordered by request_type in ascending order"; return ValidationResult.Fail; } } From a226fb11fae712938fc9bf837fc9178f0d117cb3 Mon Sep 17 00:00:00 2001 From: Rohit Ranjan Date: Tue, 4 Feb 2025 14:03:27 +0530 Subject: [PATCH 5/5] format files --- .../Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs index 9162ec4fde2..16c80668503 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V4.cs @@ -216,7 +216,7 @@ public async Task NewPayloadV4_reject_payload_with_bad_execution_requests() var response = await rpc.engine_newPayloadV4( executionPayload, [], - TestBlock.ParentBeaconBlockRoot, + TestBlock.ParentBeaconBlockRoot, executionRequests: [Bytes.FromHexString("0x0001"), Bytes.FromHexString("0x0101"), Bytes.FromHexString("0x0101")] ); @@ -226,7 +226,7 @@ public async Task NewPayloadV4_reject_payload_with_bad_execution_requests() response = await rpc.engine_newPayloadV4( executionPayload, [], - TestBlock.ParentBeaconBlockRoot, + TestBlock.ParentBeaconBlockRoot, executionRequests: [Bytes.FromHexString("0x0001"), Bytes.FromHexString("0x01"), Bytes.FromHexString("0x0101")] );