diff --git a/src/Nethermind/Ethereum.Test.Base/GeneralStateTest.cs b/src/Nethermind/Ethereum.Test.Base/GeneralStateTest.cs index 800ff17e8437..098b7cc86821 100644 --- a/src/Nethermind/Ethereum.Test.Base/GeneralStateTest.cs +++ b/src/Nethermind/Ethereum.Test.Base/GeneralStateTest.cs @@ -13,6 +13,12 @@ namespace Ethereum.Test.Base { public class GeneralStateTest : EthereumTest { + /// + /// When true, uses legacy coinbase behavior (create before tx) for backward compatibility + /// with old test expectations that were computed with buggy coinbase timing. + /// + public bool IsLegacy { get; set; } + public IReleaseSpec? Fork { get; set; } public string? ForkName { get; set; } public Address? CurrentCoinbase { get; set; } diff --git a/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs b/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs index 39dbc1517905..e402b1716b7e 100644 --- a/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs +++ b/src/Nethermind/Ethereum.Test.Base/GeneralTestBase.cs @@ -89,7 +89,17 @@ protected EthereumTestResult RunTest(GeneralStateTest test, ITxTracer txTracer) using IDisposable _ = stateProvider.BeginScope(null); ITransactionProcessor transactionProcessor = mainBlockProcessingContext.TransactionProcessor; - InitializeTestState(test.Pre, test.CurrentCoinbase, stateProvider, specProvider); + InitializeTestState(test.Pre, stateProvider, specProvider); + + // Legacy tests expect coinbase to be created BEFORE transaction execution + // (old buggy behavior that was baked into expected state roots). + // Modern tests correctly create coinbase only after successful tx. + if (test.IsLegacy && test.CurrentCoinbase is not null) + { + stateProvider.CreateAccountIfNotExists(test.CurrentCoinbase, UInt256.Zero); + stateProvider.Commit(specProvider.GetSpec((ForkActivation)1)); + stateProvider.RecalculateStateRoot(); + } BlockHeader header = new( test.PreviousHash, @@ -154,11 +164,29 @@ protected EthereumTestResult RunTest(GeneralStateTest test, ITxTracer txTracer) { stateProvider.Commit(specProvider.GetSpec((ForkActivation)1)); stateProvider.CommitTree(1); + + // '@winsvega added a 0-wei reward to the miner, so we had to add that into the state test execution phase. He needed it for retesteth.' + // This must only happen after successful transaction execution, not when tx fails validation. + // For legacy tests, coinbase was already created before tx execution. + if (!test.IsLegacy) + { + stateProvider.CreateAccountIfNotExists(test.CurrentCoinbase, UInt256.Zero); + } + stateProvider.Commit(specProvider.GetSpec((ForkActivation)1)); stateProvider.RecalculateStateRoot(); } else { - stateProvider.Reset(); + // For legacy tests with failed tx, we need to recalculate root since coinbase was created + if (test.IsLegacy) + { + stateProvider.CommitTree(0); + stateProvider.RecalculateStateRoot(); + } + else + { + stateProvider.Reset(); + } } List differences = RunAssertions(test, stateProvider); @@ -176,7 +204,7 @@ protected EthereumTestResult RunTest(GeneralStateTest test, ITxTracer txTracer) return testResult; } - public static void InitializeTestState(Dictionary preState, Address coinbase, IWorldState stateProvider, ISpecProvider specProvider) + public static void InitializeTestState(Dictionary preState, IWorldState stateProvider, ISpecProvider specProvider) { foreach (KeyValuePair accountState in preState) { @@ -194,13 +222,6 @@ public static void InitializeTestState(Dictionary preStat stateProvider.Commit(specProvider.GenesisSpec); stateProvider.CommitTree(0); stateProvider.Reset(); - - if (!stateProvider.AccountExists(coinbase)) - { - stateProvider.CreateAccount(coinbase, 0); - stateProvider.Commit(specProvider.GetSpec((ForkActivation)1)); - stateProvider.RecalculateStateRoot(); - } } private List RunAssertions(GeneralStateTest test, IWorldState stateProvider) diff --git a/src/Nethermind/Ethereum.Test.Base/LoadLegacyGeneralStateTestsStrategy.cs b/src/Nethermind/Ethereum.Test.Base/LoadLegacyGeneralStateTestsStrategy.cs index 93670a28c31b..1c3a2309ad27 100644 --- a/src/Nethermind/Ethereum.Test.Base/LoadLegacyGeneralStateTestsStrategy.cs +++ b/src/Nethermind/Ethereum.Test.Base/LoadLegacyGeneralStateTestsStrategy.cs @@ -50,9 +50,14 @@ private IEnumerable LoadTestsFromDirectory(string testDir, string { FileTestsSource fileTestsSource = new(testFile, wildcard); var tests = fileTestsSource.LoadTests(TestType.State); - foreach (EthereumTest blockchainTest in tests) + foreach (EthereumTest ethereumTest in tests) { - blockchainTest.Category = testDir; + ethereumTest.Category = testDir; + // Mark legacy tests to use old coinbase behavior for backward compatibility + if (ethereumTest is GeneralStateTest generalStateTest) + { + generalStateTest.IsLegacy = true; + } } testsByName.AddRange(tests); diff --git a/tools/Evm/T8n/T8nExecutor.cs b/tools/Evm/T8n/T8nExecutor.cs index 7298e7149f51..95780e5c8e1e 100644 --- a/tools/Evm/T8n/T8nExecutor.cs +++ b/tools/Evm/T8n/T8nExecutor.cs @@ -52,7 +52,7 @@ public static T8nExecutionResult Execute(T8nCommandArguments arguments) _logManager); stateProvider.CreateAccount(test.CurrentCoinbase, 0); - GeneralStateTestBase.InitializeTestState(test.Alloc, test.CurrentCoinbase, stateProvider, test.SpecProvider); + GeneralStateTestBase.InitializeTestState(test.Alloc, stateProvider, test.SpecProvider); Block block = test.ConstructBlock(); var withdrawalProcessor = new WithdrawalProcessor(stateProvider, _logManager);