gst and processTransaction unification

GeneralStateTests.md

@@ -521,7 +521,7 @@ OK: 5/5 Fail: 0/5 Skip: 0/5
 + CallContractToCreateContractWhichWouldCreateContractInInitCode.jsonOK
 + CallRecursiveContract.json                                      OK
 + CallTheContractToCreateEmptyContract.json                       OK
-  NotEnoughCashContractCreation.json                              Skip
++ NotEnoughCashContractCreation.json                              OK
   OutOfGasContractCreation.json                                   Skip
   OutOfGasPrefundedContractCreation.json                          Skip
 + ReturnTest.json                                                 OK
@@ -532,7 +532,7 @@ OK: 5/5 Fail: 0/5 Skip: 0/5
 + TransactionCreateStopInInitcode.json                            OK
 + TransactionCreateSuicideInInitcode.json                         OK
 ```
-OK: 14/18 Fail: 0/18 Skip: 4/18
+OK: 15/18 Fail: 0/18 Skip: 3/18
 ## stLogTests
 ```diff
 + log0_emptyMem.json                                              OK
@@ -1287,7 +1287,7 @@ OK: 0/16 Fail: 0/16 Skip: 16/16
 OK: 321/327 Fail: 0/327 Skip: 6/327
 ## stRandom2
 ```diff
-  201503110226PYTHON_DUP6.json                                    Skip
++ 201503110226PYTHON_DUP6.json                                    OK
 + randomStatetest.json                                            OK
 + randomStatetest384.json                                         OK
 + randomStatetest385.json                                         OK
@@ -1515,7 +1515,7 @@ OK: 321/327 Fail: 0/327 Skip: 6/327
 + randomStatetest646.json                                         OK
   randomStatetest647.json                                         Skip
 ```
-OK: 222/227 Fail: 0/227 Skip: 5/227
+OK: 223/227 Fail: 0/227 Skip: 4/227
 ## stRecursiveCreate
 ```diff
 + recursiveCreate.json                                            OK
@@ -2090,9 +2090,9 @@ OK: 62/67 Fail: 0/67 Skip: 5/67
 + ContractStoreClearsSuccess.json                                 OK
 + CreateMessageReverted.json                                      OK
 + CreateMessageSuccess.json                                       OK
-  CreateTransactionReverted.json                                  Skip
++ CreateTransactionReverted.json                                  OK
 + CreateTransactionSuccess.json                                   OK
-  EmptyTransaction.json                                           Skip
++ EmptyTransaction.json                                           OK
   EmptyTransaction2.json                                          Skip
 + EmptyTransaction3.json                                          OK
 + HighGasLimit.json                                               OK
@@ -2102,10 +2102,10 @@ OK: 62/67 Fail: 0/67 Skip: 5/67
 + InternlCallStoreClearsOOG.json                                  OK
 + InternlCallStoreClearsSucces.json                               OK
 + Opcodes_TransactionInit.json                                    OK
-  OverflowGasRequire.json                                         Skip
++ OverflowGasRequire.json                                         OK
 + OverflowGasRequire2.json                                        OK
-  RefundOverflow.json                                             Skip
++ RefundOverflow.json                                             OK
-  RefundOverflow2.json                                            Skip
++ RefundOverflow2.json                                            OK
 + StoreClearsAndInternlCallStoreClearsOOG.json                    OK
 + StoreClearsAndInternlCallStoreClearsSuccess.json                OK
 + StoreGasOnCreate.json                                           OK
@@ -2120,18 +2120,18 @@ OK: 62/67 Fail: 0/67 Skip: 5/67
 + TransactionFromCoinbaseHittingBlockGasLimit.json                OK
 + TransactionFromCoinbaseHittingBlockGasLimit1.json               OK
 + TransactionFromCoinbaseNotEnoughFounds.json                     OK
-  TransactionNonceCheck.json                                      Skip
++ TransactionNonceCheck.json                                      OK
-  TransactionNonceCheck2.json                                     Skip
++ TransactionNonceCheck2.json                                     OK
 + TransactionSendingToEmpty.json                                  OK
   TransactionSendingToZero.json                                   Skip
 + TransactionToAddressh160minusOne.json                           OK
 + TransactionToItself.json                                        OK
-  TransactionToItselfNotEnoughFounds.json                         Skip
++ TransactionToItselfNotEnoughFounds.json                         OK
-  UserTransactionGasLimitIsTooLowWhenZeroCost.json                Skip
++ UserTransactionGasLimitIsTooLowWhenZeroCost.json                OK
   UserTransactionZeroCost.json                                    Skip
   UserTransactionZeroCostWithData.json                            Skip
 ```
-OK: 30/44 Fail: 0/44 Skip: 14/44
+OK: 39/44 Fail: 0/44 Skip: 5/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: 1480/2334 Fail: 0/2334 Skip: 854/2334
+OK: 1491/2334 Fail: 0/2334 Skip: 843/2334

nimbus/p2p/executor.nim

@@ -13,31 +13,48 @@ proc processTransaction*(tx: Transaction, sender: EthAddress, vmState: BaseVMSta
   trace "Sender", sender
   trace "txHash", rlpHash = tx.rlpHash
 
-  let upfrontGasCost = tx.gasLimit.u256 * tx.gasPrice.u256
+  var gasUsed = tx.gasLimit
-  var balance = vmState.readOnlyStateDb().getBalance(sender)
-  if balance < upfrontGasCost:
-    return tx.gasLimit
 
-  let recipient = tx.getRecipient()
+  block:
-  let isCollision = vmState.readOnlyStateDb().hasCodeOrNonce(recipient)
+    if vmState.cumulativeGasUsed + gasUsed > vmState.blockHeader.gasLimit:
+      debug "invalid tx: block header gasLimit reached",
+        blockGasLimit=vmState.blockHeader.gasLimit,
+        gasUsed=gasUsed,
+        txGasLimit=tx.gasLimit
+      gasUsed = 0
+      break
 
-  var computation = setupComputation(vmState, tx, sender, recipient, forkOverride)
+    let upfrontGasCost = tx.gasLimit.u256 * tx.gasPrice.u256
-  if computation.isNil:
+    var balance = vmState.readOnlyStateDb().getBalance(sender)
-    return 0
+    if balance < upfrontGasCost: break
 
+    let recipient = tx.getRecipient()
+    let isCollision = vmState.readOnlyStateDb().hasCodeOrNonce(recipient)
+
+    var computation = setupComputation(vmState, tx, sender, recipient, forkOverride)
+    if computation.isNil:
+      gasUsed = 0
+      break
+
+    vmState.mutateStateDB:
+      db.incNonce(sender)
+      db.subBalance(sender, upfrontGasCost)
+
+    if tx.isContractCreation and isCollision: break
+    if execComputation(computation):
+      gasUsed = computation.refundGas(tx, sender)
+
+    if computation.isSuicided(vmState.blockHeader.coinbase):
+      gasUsed = 0
+
+  vmState.cumulativeGasUsed += gasUsed
+
+  # miner fee
+  let txFee = gasUsed.u256 * tx.gasPrice.u256
   vmState.mutateStateDB:
-    db.incNonce(sender)
+    db.addBalance(vmState.blockHeader.coinbase, txFee)
-    db.subBalance(sender, upfrontGasCost)
 
-  if tx.isContractCreation and isCollision:
+  result = gasUsed
-    return tx.gasLimit
-
-  result = tx.gasLimit
-  if execComputation(computation):
-    result = computation.refundGas(tx, sender)
-
-  if computation.isSuicided(vmState.blockHeader.coinbase):
-    return 0
 
 type
   # TODO: these types need to be removed
@@ -58,7 +75,7 @@ func createBloom*(receipts: openArray[Receipt]): Bloom =
     bloom.value = bloom.value or logsBloom(receipt.logs).value
   result = bloom.value.toByteArrayBE
 
-proc makeReceipt(vmState: BaseVMState, cumulativeGasUsed: GasInt, fork = FkFrontier): Receipt =
+proc makeReceipt(vmState: BaseVMState, fork = FkFrontier): Receipt =
   if fork < FkByzantium:
     result.stateRootOrStatus = hashOrStatus(vmState.accountDb.rootHash)
   else:
@@ -66,7 +83,7 @@ proc makeReceipt(vmState: BaseVMState, cumulativeGasUsed: GasInt, fork = FkFront
     let vmStatus = true # success or failure
     result.stateRootOrStatus = hashOrStatus(vmStatus)
 
-  result.cumulativeGasUsed = cumulativeGasUsed
+  result.cumulativeGasUsed = vmState.cumulativeGasUsed
   result.logs = vmState.getAndClearLogEntries()
   result.bloom = logsBloom(result.logs).value.toByteArrayBE
 
@@ -89,26 +106,15 @@ proc processBlock*(chainDB: BaseChainDB, header: BlockHeader, body: BlockBody, v
       trace "Has transactions", blockNumber = header.blockNumber, blockHash = header.blockHash
 
       vmState.receipts = newSeq[Receipt](body.transactions.len)
-      var cumulativeGasUsed = GasInt(0)
+      vmState.cumulativeGasUsed = 0
       for txIndex, tx in body.transactions:
-        if cumulativeGasUsed + tx.gasLimit > header.gasLimit:
+        var sender: EthAddress
-          vmState.mutateStateDB:
+        if tx.getSender(sender):
-            db.addBalance(header.coinbase, 0.u256)
+          let gasUsed = processTransaction(tx, sender, vmState)
-          # TODO: do we need to break or continue execution?
         else:
-          var sender: EthAddress
+          debug "Could not get sender", txIndex, tx
-          if tx.getSender(sender):
+          return ValidationResult.Error
-            let gasUsed = processTransaction(tx, sender, vmState)
+        vmState.receipts[txIndex] = makeReceipt(vmState)
-            cumulativeGasUsed += gasUsed
-
-            # miner fee
-            let txFee = gasUsed.u256 * tx.gasPrice.u256
-            vmState.mutateStateDB:
-              db.addBalance(header.coinbase, txFee)
-          else:
-            debug "Could not get sender", txIndex, tx
-            return ValidationResult.Error
-          vmState.receipts[txIndex] = makeReceipt(vmState, cumulativeGasUsed)
 
   var mainReward = blockReward
   if header.ommersHash != EMPTY_UNCLE_HASH:

nimbus/tracer.nim

@@ -103,9 +103,6 @@ proc traceTransaction*(db: BaseChainDB, header: BlockHeader,
       beforeRoot = stateDb.rootHash
 
     gasUsed = processTransaction(tx, sender, vmState)
-    vmState.mutateStateDB:
-      let txFee = gasUsed.u256 * tx.gasPrice.u256
-      db.addBalance(header.coinbase, txFee)
 
     if idx == txIndex:
       after.captureAccount(stateDb, sender, senderName)

nimbus/vm_state_transactions.nim

@@ -14,15 +14,17 @@ 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
-  var readOnlyDB = vmState.readOnlyStateDB
+  let 
-  let limitAndValue = transaction.gasLimit.u256 + transaction.value
+    account = vmState.readOnlyStateDB.getAccount(sender)
-  let gasCost = transaction.gasLimit.u256 * transaction.gasPrice.u256
+    gasLimit = transaction.gasLimit.u256
+    limitAndValue = gasLimit + transaction.value
+    gasCost = gasLimit * transaction.gasPrice.u256
 
   transaction.gasLimit >= transaction.intrinsicGas and
     #transaction.gasPrice <= (1 shl 34) and
-    limitAndValue <= readOnlyDB.getBalance(sender) and
+    limitAndValue <= account.balance and
-    transaction.accountNonce == readOnlyDB.getNonce(sender) and
+    transaction.accountNonce == account.nonce and
-    readOnlyDB.getBalance(sender) >= gasCost
+    account.balance >= gasCost
 
 proc setupComputation*(vmState: BaseVMState, tx: Transaction, sender, recipient: EthAddress, forkOverride=none(Fork)) : BaseComputation =
   let fork =

nimbus/vm_types.nim

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

tests/test_generalstate_failing.nim

@@ -21,26 +21,15 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool =
     "doubleSelfdestructTest2.json",
 
     # Spurious Dragon failed GST
-    "CallContractToCreateContractOOG.json",
+    "CallContractToCreateContractOOG.json",    
-    "NotEnoughCashContractCreation.json",
     "OutOfGasContractCreation.json",
     "OutOfGasPrefundedContractCreation.json",
-    "201503110226PYTHON_DUP6.json",
     "RevertOpcodeInInit.json",
     "RevertOpcodeWithBigOutputInInit.json",
     "failed_tx_xcf416c53.json",
     "suicideCoinbase.json",
-    "CreateTransactionReverted.json",
-    "EmptyTransaction.json",
     "EmptyTransaction2.json",
-    "OverflowGasRequire.json",
-    "RefundOverflow.json",
-    "RefundOverflow2.json",
     "SuicidesMixingCoinbase.json",
-    "TransactionNonceCheck.json",
-    "TransactionNonceCheck2.json",
-    "TransactionToItselfNotEnoughFounds.json",
-    "UserTransactionGasLimitIsTooLowWhenZeroCost.json",
     "UserTransactionZeroCost.json",
     "UserTransactionZeroCostWithData.json",
     "createNameRegistratorPerTxsNotEnoughGasAfter.json",
@@ -51,6 +40,18 @@ func allowedFailingGeneralStateTest*(folder, name: string): bool =
     "NonZeroValue_CALLCODE_ToEmpty.json",
     "NonZeroValue_CALLCODE_ToOneStorageKey.json",
     "TransactionSendingToZero.json"
+    
+    #"NotEnoughCashContractCreation.json",
+    #"201503110226PYTHON_DUP6.json",
+    #"CreateTransactionReverted.json",
+    #"EmptyTransaction.json",
+    #"OverflowGasRequire.json",
+    #"RefundOverflow.json",
+    #"RefundOverflow2.json",    
+    #"TransactionNonceCheck.json",
+    #"TransactionNonceCheck2.json",
+    #"TransactionToItselfNotEnoughFounds.json",
+    #"UserTransactionGasLimitIsTooLowWhenZeroCost.json",
 
     # Homestead recursives
     #["ContractCreationSpam.json",

tests/test_generalstate_json.nim

@@ -107,19 +107,22 @@ proc testFixtureIndexes(tester: Tester, testStatusIMPL: var TestStatus) =
       # pre-EIP158 (e.g., Byzantium) should ensure currentCoinbase exists
       # in later forks, don't create at all
       db.addBalance(tester.header.coinbase, 0.u256)
+
+      # TODO: this feels not right to be here
+      # perhaps the entire validateTransaction block
+      # should be moved into processTransaction
+      if tester.fork >= FkSpurious:
+        let recipient = tester.tx.getRecipient()
+        let miner = tester.header.coinbase
+        let touchedAccounts = [sender, miner, recipient]
+        for account in touchedAccounts:
+          debug "state clearing", account
+          if db.accountExists(account) and db.isEmptyAccount(account):
+            db.deleteAccount(account)
+
     return
 
-  if gasUsed + tester.tx.gasLimit <= tester.header.gasLimit:
+  gasUsed = tester.tx.processTransaction(sender, vmState, some(tester.fork))
-    vmState.mutateStateDB:
-      gasUsed = tester.tx.processTransaction(sender, vmState, some(tester.fork))
-      db.addBalance(tester.header.coinbase, gasUsed.u256 * tester.tx.gasPrice.u256)
-  else:
-    debug "invalid tx: block header gasLimit reached",
-      blockGasLimit=tester.header.gasLimit,
-      gasUsed=gasUsed,
-      txGasLimit=tester.tx.gasLimit
-    vmState.mutateStateDB:
-      db.addBalance(tester.header.coinbase, 0.u256)
 
 proc testFixture(fixtures: JsonNode, testStatusIMPL: var TestStatus,
                  debugMode = false, supportedForks: set[Fork] = supportedForks) =