unify coinbase state clearing of gst, evmstate, and t8n

it is troublesome if we have to fix it in three places
in case of a bug, it also reduce code duplication.

Author: jangko

nimbus/common/common.nim

@@ -295,6 +295,9 @@ func toEVMFork*(com: CommonRef, number: BlockNumber): EVMFork =
   let fork = com.toHardFork(number)
   ToEVMFork[fork]
 
+func toEVMFork*(com: CommonRef): EVMFork =
+  ToEVMFork[com.currentFork]
+
 func isLondon*(com: CommonRef, number: BlockNumber): bool =
   # TODO: Fixme, use only London comparator
   com.toHardFork(number) >= London

tests/test_blockchain_json.nim

@@ -390,6 +390,8 @@ proc testFixture(node: JsonNode, testStatusIMPL: var TestStatus, debugMode = fal
         # state root to test against 'postState'
         let stateDB = AccountsCache.init(memDB, header.stateRoot, pruneTrie)
         verifyStateDB(fixture["postState"], ReadOnlyStateDB(stateDB))
+
+      success = lastBlockHash == tester.lastBlockHash
     except ValidationError as E:
       echo fixtureName, " ERROR: ", E.msg
       success = false

tests/test_generalstate_json.nim

@@ -16,6 +16,7 @@ import
   ../nimbus/utils/utils,
   ../tools/common/helpers as chp,
   ../tools/evmstate/helpers,
+  ../tools/common/state_clearing,
   chronicles,
   eth/rlp,
   eth/trie/trie_defs,
@@ -124,28 +125,8 @@ proc testFixtureIndexes(tester: Tester, testStatusIMPL: var TestStatus) =
   if rc.isOk:
     gasUsed = rc.value
 
-  # This is necessary due to the manner in which the state tests are
-  # generated. State tests are generated from the BlockChainTest tests
-  # in which these transactions are included in the larger context of a
-  # block and thus, the mechanisms which would touch/create/clear the
-  # coinbase account based on the mining reward are present during test
-  # generation, but not part of the execution, thus we must artificially
-  # create the account in VMs prior to the state clearing rules,
-  # as well as conditionally cleaning up the coinbase account when left
-  # empty in VMs after the state clearing rules came into effect.
   let miner = tester.header.coinbase
-  if miner in vmState.selfDestructs:
-    vmState.mutateStateDB:
-      db.addBalance(miner, 0.u256)
-      if fork >= FkSpurious:
-        if db.isEmptyAccount(miner):
-          db.deleteAccount(miner)
-
-      # this is an important step when using accounts_cache
-      # it will affect the account storage's location
-      # during the next call to `getComittedStorage`
-      # and the result of rootHash
-      db.persist()
+  coinbaseStateClearing(vmState, miner, fork)
 
 proc testFixture(fixtures: JsonNode, testStatusIMPL: var TestStatus,
                  trace = false, debugMode = false) =

tools/common/state_clearing.nim

@@ -0,0 +1,45 @@
+# Nimbus
+# Copyright (c) 2023 Status Research & Development GmbH
+# Licensed under either of
+#  * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or
+#    http://www.apache.org/licenses/LICENSE-2.0)
+#  * MIT license ([LICENSE-MIT](LICENSE-MIT) or
+#    http://opensource.org/licenses/MIT)
+# at your option. This file may not be copied, modified, or distributed except
+# according to those terms.
+
+import
+  ../../nimbus/common/common,
+  ../../nimbus/[vm_state, vm_types],
+  ../../nimbus/db/accounts_cache
+
+proc coinbaseStateClearing*(vmState: BaseVMState,
+                            miner: EthAddress,
+                            fork: EVMFork,
+                            touched = true) =
+  # This is necessary due to the manner in which the state tests are
+  # generated. State tests are generated from the BlockChainTest tests
+  # in which these transactions are included in the larger context of a
+  # block and thus, the mechanisms which would touch/create/clear the
+  # coinbase account based on the mining reward are present during test
+  # generation, but not part of the execution, thus we must artificially
+  # create the account in VMs prior to the state clearing rules,
+  # as well as conditionally cleaning up the coinbase account when left
+  # empty in VMs after the state clearing rules came into effect.
+
+  vmState.mutateStateDB:
+    if touched:
+      db.addBalance(miner, 0.u256)
+    if fork >= FkSpurious:
+      # EIP158/161 state clearing
+      if db.accountExists(miner) and db.isEmptyAccount(miner):
+        db.deleteAccount(miner)
+
+    # db.persist is an important step when using accounts_cache
+    # it will affect the account storage's location
+    # during the next call to `getComittedStorage`
+    # and the result of rootHash
+
+    # do not clear cache, we need the cache when constructing
+    # post state
+    db.persist(clearCache = false)

tools/evmstate/evmstate.nim

@@ -21,7 +21,8 @@ import
   ../../nimbus/core/executor,
   ../../nimbus/common/common,
   ../common/helpers as chp,
-  "."/[config, helpers]
+  "."/[config, helpers],
+  ../common/state_clearing
 
 type
   StateContext = object
@@ -235,13 +236,7 @@ proc runExecution(ctx: var StateContext, conf: StateConf, pre: JsonNode): StateR
     gasUsed = rc.value
 
   let miner = ctx.header.coinbase
-  if miner in vmState.selfDestructs:
-    vmState.mutateStateDB:
-      db.addBalance(miner, 0.u256)
-      if fork >= FkSpurious:
-        if db.isEmptyAccount(miner):
-          db.deleteAccount(miner)
-      db.persist(clearCache = false)
+  coinbaseStateClearing(vmState, miner, fork)
 
 proc toTracerFlags(conf: Stateconf): set[TracerFlags] =
   result = {

tools/t8n/transition.nim

@@ -13,6 +13,7 @@ import
   eth/[rlp, trie, eip1559],
   stint, chronicles, stew/results,
   "."/[config, types, helpers],
+  ../common/state_clearing,
   ../../nimbus/[vm_types, vm_state, transaction],
   ../../nimbus/common/common,
   ../../nimbus/db/accounts_cache,
@@ -222,14 +223,8 @@ proc exec(ctx: var TransContext,
       db.persist(clearCache = false)
 
   let miner = ctx.env.currentCoinbase
-  if vmState.com.forkGTE(Spurious):
-    # EIP158/161 state clearing
-    vmState.mutateStateDB:
-      if db.accountExists(miner) and db.isEmptyAccount(miner):
-        db.deleteAccount(miner)
-        # do not clear cache, we need the cache when constructing
-        # post state
-        db.persist(clearCache = false)
+  let fork = vmState.com.toEVMFork
+  coinbaseStateClearing(vmState, miner, fork, stateReward.isSome())
 
   let stateDB = vmState.stateDB
   stateDB.postState(result.alloc)