From bb5c5d5834ce631f4c611592fcb0832c6636025b Mon Sep 17 00:00:00 2001 From: Alexey Date: Thu, 17 Aug 2023 12:29:22 +0300 Subject: [PATCH] Decline payloadv2 with v3 fields (#6007) * Decline payloads with v3 fields --- .../EngineModuleTests.HelperFunctions.cs | 22 ++++---- .../EngineModuleTests.V3.cs | 56 +++++++++++++++++++ .../Data/ExecutionPayload.cs | 30 ++++++++-- .../Data/ExecutionPayloadV3.cs | 13 ----- 4 files changed, 93 insertions(+), 28 deletions(-) diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs index 2f41754e13c..3334788b008 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.HelperFunctions.cs @@ -84,13 +84,16 @@ private ExecutionPayload CreateParentBlockRequestOnHead(IBlockTree blockTree) }; } - private static ExecutionPayload CreateBlockRequest(ExecutionPayload parent, Address miner, IList? withdrawals = null, Transaction[]? transactions = null) - => CreateBlockRequestInternal(parent, miner, withdrawals, transactions: transactions); + private static ExecutionPayload CreateBlockRequest(ExecutionPayload parent, Address miner, IList? withdrawals = null, + ulong? blobGasUsed = null, ulong? excessBlobGas = null, Transaction[]? transactions = null, Keccak? parentBeaconBlockRoot = null) + => CreateBlockRequestInternal(parent, miner, withdrawals, blobGasUsed, excessBlobGas, transactions: transactions, parentBeaconBlockRoot: parentBeaconBlockRoot); - private static ExecutionPayloadV3 CreateBlockRequestV3(ExecutionPayload parent, Address miner, IList? withdrawals = null, ulong? blobGasUsed = null, ulong? excessBlobGas = null, Transaction[]? transactions = null) - => CreateBlockRequestInternal(parent, miner, withdrawals, blobGasUsed, excessBlobGas, transactions: transactions); + private static ExecutionPayloadV3 CreateBlockRequestV3(ExecutionPayload parent, Address miner, IList? withdrawals = null, + ulong? blobGasUsed = null, ulong? excessBlobGas = null, Transaction[]? transactions = null, Keccak? parentBeaconBlockRoot = null) + => CreateBlockRequestInternal(parent, miner, withdrawals, blobGasUsed, excessBlobGas, transactions: transactions, parentBeaconBlockRoot: parentBeaconBlockRoot); - private static T CreateBlockRequestInternal(ExecutionPayload parent, Address miner, IList? withdrawals = null, ulong? blobGasUsed = null, ulong? excessBlobGas = null, Transaction[]? transactions = null) where T : ExecutionPayload, new() + private static T CreateBlockRequestInternal(ExecutionPayload parent, Address miner, IList? withdrawals = null, + ulong? blobGasUsed = null, ulong? excessBlobGas = null, Transaction[]? transactions = null, Keccak? parentBeaconBlockRoot = null) where T : ExecutionPayload, new() { T blockRequest = new() { @@ -104,14 +107,11 @@ private static ExecutionPayloadV3 CreateBlockRequestV3(ExecutionPayload parent, LogsBloom = Bloom.Empty, Timestamp = parent.Timestamp + 1, Withdrawals = withdrawals, + BlobGasUsed = blobGasUsed, + ExcessBlobGas = excessBlobGas, + ParentBeaconBlockRoot = parentBeaconBlockRoot, }; - if (blockRequest is ExecutionPayloadV3 blockRequestV3) - { - blockRequestV3.BlobGasUsed = blobGasUsed; - blockRequestV3.ExcessBlobGas = excessBlobGas; - } - blockRequest.SetTransactions(transactions ?? Array.Empty()); TryCalculateHash(blockRequest, out Keccak? hash); blockRequest.BlockHash = hash; diff --git a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs index 77189786928..cd3f764c245 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs @@ -60,6 +60,20 @@ public async Task NewPayloadV2_should_decline_post_cancun() Assert.That(result.ErrorCode, Is.EqualTo(ErrorCodes.UnsupportedFork)); } + [TestCaseSource(nameof(CancunFieldsTestSource))] + public async Task NewPayloadV2_should_decline_pre_cancun_with_cancun_fields(ulong? blobGasUsed, ulong? excessBlobGas, Keccak? parentBlockBeaconRoot) + { + MergeTestBlockchain chain = await CreateBlockchain(releaseSpec: Shanghai.Instance); + IEngineRpcModule rpcModule = CreateEngineModule(chain); + ExecutionPayload executionPayload = CreateBlockRequest( + CreateParentBlockRequestOnHead(chain.BlockTree), TestItem.AddressD, withdrawals: Array.Empty(), + blobGasUsed: blobGasUsed, excessBlobGas: excessBlobGas, parentBeaconBlockRoot: parentBlockBeaconRoot); + + ResultWrapper result = await rpcModule.engine_newPayloadV2(executionPayload); + + return result.ErrorCode; + } + [Test] public async Task NewPayloadV3_should_decline_pre_cancun_payloads() { @@ -360,6 +374,48 @@ public static IEnumerable BlobVersionedHashesDoNotMatchTestSource } } + public static IEnumerable CancunFieldsTestSource + { + get + { + yield return new TestCaseData(null, null, null) + { + ExpectedResult = ErrorCodes.None, + TestName = "No Cancun fields", + }; + yield return new TestCaseData(0ul, null, null) + { + ExpectedResult = ErrorCodes.InvalidParams, + TestName = $"{nameof(ExecutionPayloadV3.BlobGasUsed)} is set", + }; + yield return new TestCaseData(null, 0ul, null) + { + ExpectedResult = ErrorCodes.InvalidParams, + TestName = $"{nameof(ExecutionPayloadV3.ExcessBlobGas)} is set", + }; + yield return new TestCaseData(null, null, Keccak.Zero) + { + ExpectedResult = ErrorCodes.InvalidParams, + TestName = $"{nameof(ExecutionPayloadV3.ParentBeaconBlockRoot)} is set", + }; + yield return new TestCaseData(1ul, 1ul, null) + { + ExpectedResult = ErrorCodes.InvalidParams, + TestName = $"Multiple fields #1", + }; + yield return new TestCaseData(1ul, 1ul, Keccak.Zero) + { + ExpectedResult = ErrorCodes.InvalidParams, + TestName = $"Multiple fields #2", + }; + yield return new TestCaseData(1ul, null, Keccak.Zero) + { + ExpectedResult = ErrorCodes.InvalidParams, + TestName = $"Multiple fields #3", + }; + } + } + private async Task SendNewBlockV3(IEngineRpcModule rpc, MergeTestBlockchain chain, IList? withdrawals) { ExecutionPayloadV3 executionPayload = CreateBlockRequestV3( diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs index 445b63a1fa7..8c4e47af93b 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayload.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.Linq; using Nethermind.Core; using Nethermind.Core.Crypto; @@ -12,6 +11,7 @@ using Nethermind.Merge.Plugin.Handlers; using Nethermind.Serialization.Rlp; using Nethermind.State.Proofs; +using Newtonsoft.Json; namespace Nethermind.Merge.Plugin.Data; @@ -92,6 +92,25 @@ public byte[][] Transactions public IEnumerable? Withdrawals { get; set; } + /// + /// Gets or sets as defined in + /// EIP-4844. + /// + public ulong? BlobGasUsed { get; set; } + + /// + /// Gets or sets as defined in + /// EIP-4844. + /// + public ulong? ExcessBlobGas { get; set; } + + /// + /// Gets or sets as defined in + /// EIP-4788. + /// + [JsonIgnore] + public Keccak? ParentBeaconBlockRoot { get; set; } + /// /// Creates the execution block from payload. /// @@ -169,15 +188,18 @@ public void SetTransactions(params Transaction[] transactions) 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 ValidationResult.Fail; } - int actualVersion = GetVersion(); + int actualVersion = this switch + { + { BlobGasUsed: not null } or { ExcessBlobGas: not null } or { ParentBeaconBlockRoot: not null } => 3, + { Withdrawals: not null } => 2, + _ => 1 + }; error = actualVersion switch { diff --git a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs index bc0ff80d92c..ed182eb1466 100644 --- a/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs +++ b/src/Nethermind/Nethermind.Merge.Plugin/Data/ExecutionPayloadV3.cs @@ -1,7 +1,6 @@ // SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only -using System.Diagnostics.CodeAnalysis; using Nethermind.Core; using Nethermind.Core.Specs; using Nethermind.Int256; @@ -23,18 +22,6 @@ public ExecutionPayloadV3(Block block) : base(block) ExcessBlobGas = block.ExcessBlobGas; } - /// - /// Gets or sets as defined in - /// EIP-4844. - /// - public ulong? BlobGasUsed { get; set; } - - /// - /// Gets or sets as defined in - /// EIP-4844. - /// - public ulong? ExcessBlobGas { get; set; } - public override bool TryGetBlock(out Block? block, UInt256? totalDifficulty = null) { if (!base.TryGetBlock(out block, totalDifficulty))