From 924ab372a6afb562214ab2eb6fe6c007af1b91f3 Mon Sep 17 00:00:00 2001 From: Lautaro Emanuel <31224949+emlautarom1@users.noreply.github.com> Date: Thu, 19 Dec 2024 10:10:27 -0300 Subject: [PATCH] OP Holocene - Proper EIP-1559 parameters handling (#7926) --- .../OptimismHeaderValidatorTests.cs | 3 +- .../OptimismPayloadPreparationServiceTests.cs | 63 ++++++++++--------- .../OptimismBaseFeeCalculator.cs | 20 +++--- .../OptimismHeaderValidator.cs | 8 ++- .../OptimismPayloadPreparationService.cs | 40 +++++++----- .../Rpc/OptimismPayloadAttributes.cs | 5 +- .../ChainSpecStyle/ChainSpecLoader.cs | 1 + 7 files changed, 78 insertions(+), 62 deletions(-) diff --git a/src/Nethermind/Nethermind.Optimism.Test/OptimismHeaderValidatorTests.cs b/src/Nethermind/Nethermind.Optimism.Test/OptimismHeaderValidatorTests.cs index 8a160f80032..d80f3a740bd 100644 --- a/src/Nethermind/Nethermind.Optimism.Test/OptimismHeaderValidatorTests.cs +++ b/src/Nethermind/Nethermind.Optimism.Test/OptimismHeaderValidatorTests.cs @@ -23,7 +23,6 @@ public class OptimismHeaderValidatorTests private static IEnumerable<(string, bool)> EIP1559ParametersExtraData() { // Valid - yield return ("0x000000000000000000", true); yield return ("0x000000000100000000", true); yield return ("0x0000000001000001bc", true); yield return ("0x0000000001ffffffff", true); @@ -35,7 +34,9 @@ public class OptimismHeaderValidatorTests yield return ("0xffffaaaa", false); yield return ("0x01ffffffff00000000", false); yield return ("0xff0000000100000001", false); + yield return ("0x000000000000000000", false); yield return ("0x000000000000000001", false); + yield return ("0x00ffffffff000001bc00", false); } [TestCaseSource(nameof(EIP1559ParametersExtraData))] diff --git a/src/Nethermind/Nethermind.Optimism.Test/OptimismPayloadPreparationServiceTests.cs b/src/Nethermind/Nethermind.Optimism.Test/OptimismPayloadPreparationServiceTests.cs index d2143f16b89..83a522e0032 100644 --- a/src/Nethermind/Nethermind.Optimism.Test/OptimismPayloadPreparationServiceTests.cs +++ b/src/Nethermind/Nethermind.Optimism.Test/OptimismPayloadPreparationServiceTests.cs @@ -1,42 +1,54 @@ // SPDX-FileCopyrightText: 2024 Demerzel Solutions Limited // SPDX-License-Identifier: LGPL-3.0-only -using Nethermind.Core.Test.Builders; -using NUnit.Framework; -using FluentAssertions; +using System.Threading.Tasks; +using System.Collections.Generic; using System; +using NUnit.Framework; using NSubstitute; -using Nethermind.Core.Specs; +using Nethermind.State; +using Nethermind.Optimism.Rpc; using Nethermind.Merge.Plugin.BlockProduction; -using Nethermind.Core.Timers; using Nethermind.Logging; -using Nethermind.Optimism.Rpc; +using Nethermind.Int256; +using Nethermind.Evm.Tracing; +using Nethermind.Core.Timers; +using Nethermind.Core.Test.Builders; +using Nethermind.Core.Specs; +using Nethermind.Core.Crypto; +using Nethermind.Core; using Nethermind.Consensus.Transactions; using Nethermind.Consensus.Processing; -using Nethermind.Blockchain; -using Nethermind.State; using Nethermind.Consensus; -using Nethermind.Core; using Nethermind.Config; -using Nethermind.Core.Crypto; -using Nethermind.Evm.Tracing; -using System.Buffers.Binary; -using System.Threading.Tasks; +using Nethermind.Blockchain; +using FluentAssertions; +using Nethermind.Crypto; namespace Nethermind.Optimism.Test; [Parallelizable(ParallelScope.All)] public class OptimismPayloadPreparationServiceTests { - [TestCase(8u, 2u)] - [TestCase(2u, 2u)] - [TestCase(2u, 10u)] - public async Task Writes_EIP1559Params_Into_HeaderExtraData(UInt32 denominator, UInt32 elasticity) + private static IEnumerable<(OptimismPayloadAttributes, EIP1559Parameters?)> TestCases() + { + foreach (var noTxPool in (bool[])[true, false]) + { + yield return (new OptimismPayloadAttributes { EIP1559Params = [0, 0, 0, 8, 0, 0, 0, 2], NoTxPool = noTxPool }, new EIP1559Parameters(0, 8, 2)); + yield return (new OptimismPayloadAttributes { EIP1559Params = [0, 0, 0, 2, 0, 0, 0, 2], NoTxPool = noTxPool }, new EIP1559Parameters(0, 2, 2)); + yield return (new OptimismPayloadAttributes { EIP1559Params = [0, 0, 0, 2, 0, 0, 0, 10], NoTxPool = noTxPool }, new EIP1559Parameters(0, 2, 10)); + yield return (new OptimismPayloadAttributes { EIP1559Params = [0, 0, 0, 0, 0, 0, 0, 0], NoTxPool = noTxPool }, new EIP1559Parameters(0, 250, 6)); + } + } + [TestCaseSource(nameof(TestCases))] + public async Task Writes_EIP1559Params_Into_HeaderExtraData((OptimismPayloadAttributes Attributes, EIP1559Parameters? ExpectedEIP1559Parameters) testCase) { var parent = Build.A.BlockHeader.TestObject; var releaseSpec = Substitute.For(); releaseSpec.IsOpHoloceneEnabled.Returns(true); + releaseSpec.BaseFeeMaxChangeDenominator.Returns((UInt256)250); + releaseSpec.ElasticityMultiplier.Returns(6); var specProvider = Substitute.For(); specProvider.GetSpec(parent).Returns(releaseSpec); @@ -69,23 +81,16 @@ public async Task Writes_EIP1559Params_Into_HeaderExtraData(UInt32 denominator, logManager: TestLogManager.Instance ); - var eip1559Params = new byte[8]; - BinaryPrimitives.WriteUInt32BigEndian(eip1559Params.AsSpan(0, 4), denominator); - BinaryPrimitives.WriteUInt32BigEndian(eip1559Params.AsSpan(4, 4), elasticity); - - var attributes = new OptimismPayloadAttributes() - { - PrevRandao = Hash256.Zero, - SuggestedFeeRecipient = TestItem.AddressA, - EIP1559Params = eip1559Params, - }; + testCase.Attributes.PrevRandao = Hash256.Zero; + testCase.Attributes.SuggestedFeeRecipient = TestItem.AddressA; - var payloadId = service.StartPreparingPayload(parent, attributes); + var payloadId = service.StartPreparingPayload(parent, testCase.Attributes); var context = await service.GetPayload(payloadId); var currentBestBlock = context?.CurrentBestBlock!; currentBestBlock.Should().Be(block); currentBestBlock.Header.TryDecodeEIP1559Parameters(out var parameters, out _).Should().BeTrue(); - parameters.Should().BeEquivalentTo(new EIP1559Parameters(0, denominator, elasticity)); + parameters.Should().BeEquivalentTo(testCase.ExpectedEIP1559Parameters); + currentBestBlock.Header.Hash.Should().BeEquivalentTo(currentBestBlock.Header.CalculateHash()); } } diff --git a/src/Nethermind/Nethermind.Optimism/OptimismBaseFeeCalculator.cs b/src/Nethermind/Nethermind.Optimism/OptimismBaseFeeCalculator.cs index cf1d0d652a2..86410a22303 100644 --- a/src/Nethermind/Nethermind.Optimism/OptimismBaseFeeCalculator.cs +++ b/src/Nethermind/Nethermind.Optimism/OptimismBaseFeeCalculator.cs @@ -23,22 +23,16 @@ public UInt256 Calculate(BlockHeader parent, IEip1559Spec specFor1559) if (parent.Timestamp >= holoceneTimestamp) { // NOTE: This operation should never fail since headers should be valid at this point. - if (!parent.TryDecodeEIP1559Parameters(out EIP1559Parameters eip1559Params, out _)) + if (!parent.TryDecodeEIP1559Parameters(out EIP1559Parameters eip1559Params, out var error)) { - throw new InvalidOperationException($"{nameof(BlockHeader)} was not properly validated: missing {nameof(EIP1559Parameters)}"); + throw new InvalidOperationException($"{nameof(BlockHeader)} was not properly validated: {error}"); } - spec = eip1559Params.IsZero() - ? new OverridableEip1559Spec(specFor1559) - { - ElasticityMultiplier = Eip1559Constants.DefaultElasticityMultiplier, - BaseFeeMaxChangeDenominator = Eip1559Constants.DefaultBaseFeeMaxChangeDenominator - } - : new OverridableEip1559Spec(specFor1559) - { - ElasticityMultiplier = eip1559Params.Elasticity, - BaseFeeMaxChangeDenominator = eip1559Params.Denominator - }; + spec = new OverridableEip1559Spec(specFor1559) + { + ElasticityMultiplier = eip1559Params.Elasticity, + BaseFeeMaxChangeDenominator = eip1559Params.Denominator + }; } return baseFeeCalculator.Calculate(parent, spec); diff --git a/src/Nethermind/Nethermind.Optimism/OptimismHeaderValidator.cs b/src/Nethermind/Nethermind.Optimism/OptimismHeaderValidator.cs index ed511db4933..5b4332e3774 100644 --- a/src/Nethermind/Nethermind.Optimism/OptimismHeaderValidator.cs +++ b/src/Nethermind/Nethermind.Optimism/OptimismHeaderValidator.cs @@ -41,11 +41,17 @@ public override bool Validate(BlockHeader header, BlockHeader? parent, bool isUn IReleaseSpec spec = _specProvider.GetSpec(header); if (spec.IsOpHoloceneEnabled) { - if (!header.TryDecodeEIP1559Parameters(out _, out var decodeError)) + if (!header.TryDecodeEIP1559Parameters(out var parameters, out var decodeError)) { error = decodeError; return false; } + + if (parameters.IsZero()) + { + error = $"{nameof(EIP1559Parameters)} is zero"; + return false; + } } return base.Validate(header, parent, isUncle, out error); diff --git a/src/Nethermind/Nethermind.Optimism/OptimismPayloadPreparationService.cs b/src/Nethermind/Nethermind.Optimism/OptimismPayloadPreparationService.cs index fb712cd8d04..7cdb39780b9 100644 --- a/src/Nethermind/Nethermind.Optimism/OptimismPayloadPreparationService.cs +++ b/src/Nethermind/Nethermind.Optimism/OptimismPayloadPreparationService.cs @@ -6,6 +6,7 @@ using Nethermind.Core; using Nethermind.Core.Specs; using Nethermind.Core.Timers; +using Nethermind.Crypto; using Nethermind.Int256; using Nethermind.Logging; using Nethermind.Merge.Plugin.BlockProduction; @@ -45,6 +46,30 @@ public OptimismPayloadPreparationService( protected override void ImproveBlock(string payloadId, BlockHeader parentHeader, PayloadAttributes payloadAttributes, Block currentBestBlock, DateTimeOffset startDateTime) { + if (payloadAttributes is OptimismPayloadAttributes optimismPayload) + { + var spec = _specProvider.GetSpec(currentBestBlock.Header); + if (spec.IsOpHoloceneEnabled) + { + // NOTE: This operation should never fail since headers should be valid at this point. + if (!optimismPayload.TryDecodeEIP1559Parameters(out EIP1559Parameters eip1559Parameters, out var error)) + { + throw new InvalidOperationException($"{nameof(BlockHeader)} was not properly validated: {error}"); + } + + if (eip1559Parameters.IsZero()) + { + eip1559Parameters = new EIP1559Parameters(eip1559Parameters.Version, (UInt32)spec.BaseFeeMaxChangeDenominator, (UInt32)spec.ElasticityMultiplier); + } + + currentBestBlock.Header.ExtraData = new byte[EIP1559Parameters.ByteLength]; + eip1559Parameters.WriteTo(currentBestBlock.Header.ExtraData); + + // NOTE: Since we updated the `Header` we need to recalculate the hash. + currentBestBlock.Header.Hash = currentBestBlock.Header.CalculateHash(); + } + } + if (payloadAttributes is OptimismPayloadAttributes { NoTxPool: true }) { if (_logger.IsDebug) @@ -56,21 +81,6 @@ protected override void ImproveBlock(string payloadId, BlockHeader parentHeader, } else { - if (payloadAttributes is OptimismPayloadAttributes optimismPayload) - { - var spec = _specProvider.GetSpec(currentBestBlock.Header); - if (spec.IsOpHoloceneEnabled) - { - if (!optimismPayload.TryDecodeEIP1559Parameters(out EIP1559Parameters eip1559Parameters, out var error)) - { - throw new InvalidOperationException($"{nameof(OptimismPayloadAttributes)} was not properly validated: invalid {nameof(OptimismPayloadAttributes.EIP1559Params)}"); - } - - currentBestBlock.Header.ExtraData = new byte[EIP1559Parameters.ByteLength]; - eip1559Parameters.WriteTo(currentBestBlock.Header.ExtraData); - } - } - base.ImproveBlock(payloadId, parentHeader, payloadAttributes, currentBestBlock, startDateTime); } } diff --git a/src/Nethermind/Nethermind.Optimism/Rpc/OptimismPayloadAttributes.cs b/src/Nethermind/Nethermind.Optimism/Rpc/OptimismPayloadAttributes.cs index 6cc73e91920..795934b659c 100644 --- a/src/Nethermind/Nethermind.Optimism/Rpc/OptimismPayloadAttributes.cs +++ b/src/Nethermind/Nethermind.Optimism/Rpc/OptimismPayloadAttributes.cs @@ -35,7 +35,6 @@ public byte[][]? Transactions /// See /// public byte[]? EIP1559Params { get; set; } - private const int EIP1559ParamsLength = 8; private int TransactionsLength => Transactions?.Length ?? 0; @@ -121,9 +120,9 @@ public override PayloadAttributesValidationResult Validate(ISpecProvider specPro error = $"{nameof(EIP1559Params)} should be null before Holocene"; return PayloadAttributesValidationResult.InvalidPayloadAttributes; } - if (releaseSpec.IsOpHoloceneEnabled && EIP1559Params?.Length != EIP1559ParamsLength) + if (releaseSpec.IsOpHoloceneEnabled && !this.TryDecodeEIP1559Parameters(out _, out var decodeError)) { - error = $"{nameof(EIP1559Params)} should be {EIP1559ParamsLength} bytes long"; + error = decodeError; return PayloadAttributesValidationResult.InvalidPayloadAttributes; } diff --git a/src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecLoader.cs b/src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecLoader.cs index 49f18d5ec7b..dc7b75faa8b 100644 --- a/src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecLoader.cs +++ b/src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecLoader.cs @@ -142,6 +142,7 @@ bool GetForInnerPathExistence(KeyValuePair o) => Eip6780TransitionTimestamp = chainSpecJson.Params.Eip6780TransitionTimestamp, Rip7212TransitionTimestamp = chainSpecJson.Params.Rip7212TransitionTimestamp, OpGraniteTransitionTimestamp = chainSpecJson.Params.OpGraniteTransitionTimestamp, + OpHoloceneTransitionTimestamp = chainSpecJson.Params.OpHoloceneTransitionTimestamp, Eip4788TransitionTimestamp = chainSpecJson.Params.Eip4788TransitionTimestamp, Eip7702TransitionTimestamp = chainSpecJson.Params.Eip7702TransitionTimestamp, Eip4788ContractAddress = chainSpecJson.Params.Eip4788ContractAddress ?? Eip4788Constants.BeaconRootsAddress,