split state clearing in execComputation to processTransaction, GST +5

GeneralStateTests.md

@@ -2076,14 +2076,14 @@ OK: 1/284 Fail: 0/284 Skip: 283/284
 + suicideCaller.json                                              OK
 + suicideCallerAddresTooBigLeft.json                              OK
 + suicideCallerAddresTooBigRight.json                             OK
-  suicideCoinbase.json                                            Skip
++ suicideCoinbase.json                                            OK
 + suicideNotExistingAccount.json                                  OK
 + suicideOrigin.json                                              OK
 + suicideSendEtherPostDeath.json                                  OK
 + suicideSendEtherToMe.json                                       OK
 + testRandomTest.json                                             OK
 ```
-OK: 62/67 Fail: 0/67 Skip: 5/67
+OK: 63/67 Fail: 0/67 Skip: 4/67
 ## stTransactionTest
 ```diff
 + ContractStoreClearsOOG.json                                     OK
@@ -2093,7 +2093,7 @@ OK: 62/67 Fail: 0/67 Skip: 5/67
 + CreateTransactionReverted.json                                  OK
 + CreateTransactionSuccess.json                                   OK
 + EmptyTransaction.json                                           OK
-  EmptyTransaction2.json                                          Skip
++ EmptyTransaction2.json                                          OK
 + EmptyTransaction3.json                                          OK
 + HighGasLimit.json                                               OK
 + InternalCallHittingGasLimit.json                                OK
@@ -2114,7 +2114,7 @@ OK: 62/67 Fail: 0/67 Skip: 5/67
 + SuicidesAndInternlCallSuicidesOOG.json                          OK
 + SuicidesAndInternlCallSuicidesSuccess.json                      OK
 + SuicidesAndSendMoneyToItselfEtherDestroyed.json                 OK
-  SuicidesMixingCoinbase.json                                     Skip
++ SuicidesMixingCoinbase.json                                     OK
 + SuicidesStopAfterSuicide.json                                   OK
 + TransactionDataCosts652.json                                    OK
 + TransactionFromCoinbaseHittingBlockGasLimit.json                OK
@@ -2128,10 +2128,10 @@ OK: 62/67 Fail: 0/67 Skip: 5/67
 + TransactionToItself.json                                        OK
 + TransactionToItselfNotEnoughFounds.json                         OK
 + UserTransactionGasLimitIsTooLowWhenZeroCost.json                OK
-  UserTransactionZeroCost.json                                    Skip
++ UserTransactionZeroCost.json                                    OK
-  UserTransactionZeroCostWithData.json                            Skip
++ UserTransactionZeroCostWithData.json                            OK
 ```
-OK: 39/44 Fail: 0/44 Skip: 5/44
+OK: 43/44 Fail: 0/44 Skip: 1/44
 ## stTransitionTest
 ```diff
 + createNameRegistratorPerTxsAfter.json                           OK
@@ -2520,4 +2520,4 @@ OK: 5/133 Fail: 0/133 Skip: 128/133
 OK: 0/130 Fail: 0/130 Skip: 130/130
 
 ---TOTAL---
-OK: 1491/2334 Fail: 0/2334 Skip: 843/2334
+OK: 1496/2334 Fail: 0/2334 Skip: 838/2334

nimbus/p2p/executor.nim

@@ -1,4 +1,4 @@
-import options,
+import options, sets,
   eth/[common, bloom], ranges, chronicles, nimcrypto,
   ../db/[db_chain, state_db],
   ../utils, ../constants, ../transaction,
@@ -32,7 +32,7 @@ proc processTransaction*(tx: Transaction, sender: EthAddress, vmState: BaseVMSta
     let isCollision = vmState.readOnlyStateDb().hasCodeOrNonce(recipient)
 
     var computation = setupComputation(vmState, tx, sender, recipient, forkOverride)
-    if computation.isNil:
+    if computation.isNil: # OOG in setupComputation
       gasUsed = 0
       break
 
@@ -54,6 +54,12 @@ proc processTransaction*(tx: Transaction, sender: EthAddress, vmState: BaseVMSta
   vmState.mutateStateDB:
     db.addBalance(vmState.blockHeader.coinbase, txFee)
 
+    # EIP158 state clearing
+    for account in vmState.touchedAccounts:
+      debug "state clearing", account
+      if db.accountExists(account) and db.isEmptyAccount(account):
+        db.deleteAccount(account)
+
   result = gasUsed
 
 type

nimbus/vm_state.nim

@@ -36,6 +36,7 @@ proc init*(self: BaseVMState, prevStateRoot: Hash256, header: BlockHeader,
   self.logEntries = @[]
   self.blockHeader.stateRoot = prevStateRoot
   self.accountDb = newAccountStateDB(chainDB.db, prevStateRoot, chainDB.pruneTrie)
+  self.touchedAccounts = initSet[EthAddress]()
 
 proc newBaseVMState*(prevStateRoot: Hash256, header: BlockHeader,
                      chainDB: BaseChainDB, tracerFlags: set[TracerFlags] = {}): BaseVMState =

nimbus/vm_state_transactions.nim

@@ -6,7 +6,7 @@
 # at your option. This file may not be copied, modified, or distributed except according to those terms.
 
 import
-  ranges/typedranges, sequtils, strformat, tables, options, sets,
+  ranges/typedranges, sequtils, strformat, tables, options,
   eth/common, chronicles, ./db/[db_chain, state_db],
   constants, errors, transaction, vm_types, vm_state, utils,
   ./vm/[computation, interpreter], ./vm/interpreter/gas_costs
@@ -14,7 +14,7 @@ import
 proc validateTransaction*(vmState: BaseVMState, transaction: Transaction, sender: EthAddress): bool =
   # XXX: https://github.com/status-im/nimbus/issues/35#issuecomment-391726518
   # XXX: lots of avoidable u256 construction
-  let 
+  let
     account = vmState.readOnlyStateDB.getAccount(sender)
     gasLimit = transaction.gasLimit.u256
     limitAndValue = gasLimit + transaction.value
@@ -81,13 +81,8 @@ proc execComputation*(computation: var BaseComputation): bool =
     const RefundSelfDestruct = 24_000
     computation.gasMeter.refundGas(RefundSelfDestruct * suicidedCount)
 
-    if computation.getFork >= FkSpurious:
+  if computation.getFork >= FkSpurious:
-      var touchedAccounts = initSet[EthAddress]()
+    computation.collectTouchedAccounts(computation.vmState.touchedAccounts)
-      computation.collectTouchedAccounts(touchedAccounts)
-      for account in touchedAccounts:
-        debug "state clearing", account
-        if db.accountExists(account) and db.isEmptyAccount(account):
-          db.deleteAccount(account)
 
   result = computation.isSuccess
   if result:

nimbus/vm_types.nim

@@ -23,8 +23,9 @@ type
     tracer*        : TransactionTracer
     logEntries*    : seq[Log]
     receipts*      : seq[Receipt]
-    accountDb*     : AccountStateDB    
+    accountDb*     : AccountStateDB
     cumulativeGasUsed*: GasInt
+    touchedAccounts*: HashSet[EthAddress]
 
   AccessLogs* = ref object
     reads*: Table[string, string]

tests/test_generalstate_failing.nim

@@ -21,17 +21,12 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool =
     "doubleSelfdestructTest2.json",
 
     # Spurious Dragon failed GST
-    "CallContractToCreateContractOOG.json",    
+    "CallContractToCreateContractOOG.json",
     "OutOfGasContractCreation.json",
     "OutOfGasPrefundedContractCreation.json",
     "RevertOpcodeInInit.json",
     "RevertOpcodeWithBigOutputInInit.json",
     "failed_tx_xcf416c53.json",
-    "suicideCoinbase.json",
-    "EmptyTransaction2.json",
-    "SuicidesMixingCoinbase.json",
-    "UserTransactionZeroCost.json",
-    "UserTransactionZeroCostWithData.json",
     "createNameRegistratorPerTxsNotEnoughGasAfter.json",
     "createNameRegistratorPerTxsNotEnoughGasAt.json",
     "createNameRegistratorPerTxsNotEnoughGasBefore.json",
@@ -41,13 +36,18 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool =
     "NonZeroValue_CALLCODE_ToOneStorageKey.json",
     "TransactionSendingToZero.json"
     
+    #"suicideCoinbase.json",
+    #"EmptyTransaction2.json",
+    #"SuicidesMixingCoinbase.json",
+    #"UserTransactionZeroCost.json",
+    #"UserTransactionZeroCostWithData.json",
     #"NotEnoughCashContractCreation.json",
     #"201503110226PYTHON_DUP6.json",
     #"CreateTransactionReverted.json",
     #"EmptyTransaction.json",
     #"OverflowGasRequire.json",
     #"RefundOverflow.json",
-    #"RefundOverflow2.json",    
+    #"RefundOverflow2.json",
     #"TransactionNonceCheck.json",
     #"TransactionNonceCheck2.json",
     #"TransactionToItselfNotEnoughFounds.json",