fix: only touch coinbase after successful transaction in state tests

src/Nethermind/Ethereum.Test.Base/GeneralStateTest.cs

@@ -13,6 +13,12 @@ namespace Ethereum.Test.Base
 {
     public class GeneralStateTest : EthereumTest
     {
+        /// <summary>
+        /// When true, uses legacy coinbase behavior (create before tx) for backward compatibility
+        /// with old test expectations that were computed with buggy coinbase timing.
+        /// </summary>
+        public bool IsLegacy { get; set; }
+
         public IReleaseSpec? Fork { get; set; }
         public string? ForkName { get; set; }
         public Address? CurrentCoinbase { get; set; }

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<string> differences = RunAssertions(test, stateProvider);
@@ -176,7 +204,7 @@ protected EthereumTestResult RunTest(GeneralStateTest test, ITxTracer txTracer)
             return testResult;
         }
 
-        public static void InitializeTestState(Dictionary<Address, AccountState> preState, Address coinbase, IWorldState stateProvider, ISpecProvider specProvider)
+        public static void InitializeTestState(Dictionary<Address, AccountState> preState, IWorldState stateProvider, ISpecProvider specProvider)
         {
             foreach (KeyValuePair<Address, AccountState> accountState in preState)
             {
@@ -194,13 +222,6 @@ public static void InitializeTestState(Dictionary<Address, AccountState> 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<string> RunAssertions(GeneralStateTest test, IWorldState stateProvider)

src/Nethermind/Ethereum.Test.Base/LoadLegacyGeneralStateTestsStrategy.cs

@@ -50,9 +50,14 @@ private IEnumerable<EthereumTest> 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);

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);