Merge pull request #147 from jwasinger/wip/spurious-dragon

Spurious Dragon

README.md

@@ -162,23 +162,48 @@ Emits the result of the transaction.
 
 # TESTING
 
+### Running Tests
+
 _Note: Requires at least Node.js `8.0.0` installed to run the tests, this is because `ethereumjs-testing` uses `async/await` and other ES2015 language features_
 
+Tests can be found in the ``tests`` directory, with ``FORK_CONFIG`` set in ``tests/tester.js``.
+
+There are test runners for [State tests](http://www.ethdocs.org/en/latest/contracts-and-transactions/ethereum-tests/state_tests/index.html) and [Blockchain tests](http://www.ethdocs.org/en/latest/contracts-and-transactions/ethereum-tests/blockchain_tests/index.html). VM tests are disabled since Frontier gas costs are not supported any more.
+
+Tests are then executed by the [ethereumjs-testing](https://github.com/ethereumjs/ethereumjs-testing) utility library using the official client-independent [Ethereum tests](https://github.com/ethereum/tests).
+
+Running all the tests:
+
 `npm test`
-if you want to just run the State tests run
+
+Running the State tests:
+
 `node ./tests/tester -s`
-if you want to just run the Blockchain tests run
+
+Running the Blockchain tests:
+
 `node ./tests/tester -b`
 
-To run the all the blockchain tests in a file:
+State tests run significantly faster than Blockchain tests, so it is often a good choice to start fixing State tests.
+
+Running all the blockchain tests in a file:
+
 `node ./tests/tester -b --file='randomStatetest303'`
 
-To run a specific state test case:
+Running a specific state test case:
+
 `node ./tests/tester -s --test='stackOverflow'`
 
+### Debugging
+
 Blockchain tests support `--debug` to verify the postState:
+
 `node ./tests/tester -b  --debug --test='ZeroValue_SELFDESTRUCT_ToOneStorageKey_OOGRevert_d0g0v0_EIP158'`
 
+All/most State tests are replicated as Blockchain tests in a ``GeneralStateTests`` [sub directory](https://github.com/ethereum/tests/tree/develop/BlockchainTests/GeneralStateTests) in the Ethereum tests repo, so for debugging single test cases the Blockchain test version of the State test can be used.
+
+For comparing ``EVM`` traces [here](https://gist.github.com/cdetrio/41172f374ae32047a6c9e97fa9d09ad0) are some instructions for setting up ``pyethereum`` to generate corresponding traces for state tests.
+
 # Internal Structure
 The VM processes state changes at many levels.
 

lib/opFns.js

@@ -36,6 +36,7 @@ module.exports = {
   DIV: function (a, b, runState) {
     a = new BN(a)
     b = new BN(b)
+
     var r
     if (b.isZero()) {
       r = [0]
@@ -61,7 +62,6 @@ module.exports = {
     a = new BN(a)
     b = new BN(b)
     var r
-
     if (b.isZero()) {
       r = [0]
     } else {
@@ -550,14 +550,22 @@ module.exports = {
         return
       }
 
-      if (!exists) {
-        try {
-          subGas(runState, new BN(fees.callNewAccountGas.v))
-        } catch (e) {
-          done(e.error)
-          return
+      stateManager.accountIsEmpty(toAddress, function (err, empty) {
+        if (err) {
+          done(err)
         }
-      }
+
+        if (!exists || empty) {
+          if (!value.isZero()) {
+            try {
+              subGas(runState, new BN(fees.callNewAccountGas.v))
+            } catch (e) {
+              done(e.error)
+              return
+            }
+          }
+        }
+      })
 
       try {
         checkCallMemCost(runState, options, localOpts)
@@ -678,6 +686,7 @@ module.exports = {
     var stateManager = runState.stateManager
     var contract = runState.contract
     var contractAddress = runState.address
+    var zeroBalance = new BN(0)
     suicideToAddress = utils.setLengthLeft(suicideToAddress, 20)
 
     stateManager.getAccount(suicideToAddress, function (err, toAccount) {
@@ -687,27 +696,36 @@ module.exports = {
         return
       }
 
-      if (!toAccount.exists) {
-        try {
-          subGas(runState, new BN(fees.callNewAccountGas.v))
-        } catch (e) {
-          cb(e.error)
+      stateManager.accountIsEmpty(suicideToAddress, function (error, empty) {
+        if (error) {
+          cb(error)
           return
         }
-      }
 
-      // only add to refund if this is the first suicide for the address
-      if (!runState.suicides[contractAddress.toString('hex')]) {
-        runState.gasRefund = runState.gasRefund.add(new BN(fees.suicideRefundGas.v))
-      }
-      runState.suicides[contractAddress.toString('hex')] = suicideToAddress
-      runState.stopped = true
-
-      var newBalance = new Buffer(new BN(contract.balance).add(new BN(toAccount.balance)).toArray())
-      async.series([
-        stateManager.putAccountBalance.bind(stateManager, suicideToAddress, newBalance),
-        stateManager.putAccountBalance.bind(stateManager, contractAddress, new BN(0))
-      ], cb)
+        if ((new BN(contract.balance)).gt(zeroBalance)) {
+          if (!toAccount.exists || empty) {
+            try {
+              subGas(runState, new BN(fees.callNewAccountGas.v))
+            } catch (e) {
+              cb(e.error)
+              return
+            }
+          }
+        }
+
+        // only add to refund if this is the first suicide for the address
+        if (!runState.suicides[contractAddress.toString('hex')]) {
+          runState.gasRefund = runState.gasRefund.add(new BN(fees.suicideRefundGas.v))
+        }
+        runState.suicides[contractAddress.toString('hex')] = suicideToAddress
+        runState.stopped = true
+
+        var newBalance = new Buffer(new BN(contract.balance).add(new BN(toAccount.balance)).toArray())
+        async.series([
+          stateManager.putAccountBalance.bind(stateManager, suicideToAddress, newBalance),
+          stateManager.putAccountBalance.bind(stateManager, contractAddress, new BN(0))
+        ], cb)
+      })
     })
   }
 }

lib/runCall.js

@@ -69,6 +69,8 @@ module.exports = function (opts, cb) {
       createdAddress = toAddress = ethUtil.generateAddress(caller, newNonce.toArray())
       stateManager.getAccount(createdAddress, function (err, account) {
         toAccount = account
+        const NONCE_OFFSET = 1
+        toAccount.nonce = new BN(toAccount.nonce).addn(NONCE_OFFSET).toBuffer()
         done(err)
       })
     } else {
@@ -93,6 +95,7 @@ module.exports = function (opts, cb) {
     // add the amount sent to the `to` account
     toAccount.balance = new BN(toAccount.balance).add(txValue)
     stateManager.cache.put(toAddress, toAccount)
+    stateManager.touched.push(toAddress)
   }
 
   function loadCode (cb) {

lib/runCode.js

@@ -224,6 +224,7 @@ module.exports = function (opts, cb) {
     if (results.exceptionError) {
       delete results.gasRefund
       delete results.suicides
+      self.stateManager.touched = []
     }
 
     if (err) {

lib/runTx.js

@@ -74,6 +74,8 @@ module.exports = function (opts, cb) {
     accounts.add(tx.to.toString('hex'))
     accounts.add(block.header.coinbase.toString('hex'))
 
+    self.stateManager.touched.push(tx.to)
+
     if (opts.populateCache === false) {
       return cb()
     }
@@ -156,14 +158,17 @@ module.exports = function (opts, cb) {
         .add(new BN(fromAccount.balance))
 
       self.stateManager.cache.put(tx.from, fromAccount)
+      self.stateManager.touched.push(tx.from)
 
       var minerAccount = self.stateManager.cache.get(block.header.coinbase)
       // add the amount spent on gas to the miner's account
       minerAccount.balance = new BN(minerAccount.balance)
         .add(results.amountSpent)
 
       // save the miner's account
-      self.stateManager.cache.put(block.header.coinbase, minerAccount)
+      if (!(new BN(minerAccount.balance).isZero())) {
+        self.stateManager.cache.put(block.header.coinbase, minerAccount)
+      }
 
       if (!results.vm.suicides) {
         results.vm.suicides = {}
@@ -175,7 +180,25 @@ module.exports = function (opts, cb) {
         self.stateManager.cache.del(new Buffer(s, 'hex'))
       })
 
-      cb()
+      // delete all touched accounts
+      var touched = self.stateManager.touched
+      async.forEach(touched, function (address, next) {
+        self.stateManager.accountIsEmpty(address, function (err, empty) {
+          if (err) {
+            next(err)
+            return
+          }
+
+          if (empty) {
+            self.stateManager.cache.del(address)
+          }
+          next(null)
+        })
+      },
+      function () {
+        self.stateManager.touched = []
+        cb()
+      })
     }
   }
 

lib/stateManager.js

@@ -27,6 +27,7 @@ function StateManager (opts) {
   self.trie = trie
   self._storageTries = {} // the storage trie cache
   self.cache = new Cache(trie)
+  self.touched = []
 }
 
 var proto = StateManager.prototype
@@ -52,6 +53,7 @@ proto._putAccount = function (address, account, cb) {
   // if (toAccount.balance.toString('hex') === '00') {
   // if they have money or a non-zero nonce or code, then write to tree
   self.cache.put(addressHex, account)
+  self.touched.push(address)
   // self.trie.put(addressHex, account.serialize(), cb)
   cb()
 }
@@ -68,10 +70,16 @@ proto.getAccountBalance = function (address, cb) {
 
 proto.putAccountBalance = function (address, balance, cb) {
   var self = this
+
   self.getAccount(address, function (err, account) {
     if (err) {
       return cb(err)
     }
+
+    if ((new BN(balance)).isZero() && !account.exists) {
+      return cb(null)
+    }
+
     account.balance = balance
     self._putAccount(address, account, cb)
   })
@@ -115,6 +123,7 @@ proto._lookupStorageTrie = function (address, cb) {
     }
     var storageTrie = self.trie.copy()
     storageTrie.root = account.stateRoot
+    self.touched.push(address)
     storageTrie._checkpoints = []
     cb(null, storageTrie)
   })
@@ -172,6 +181,7 @@ proto.putContractStorage = function (address, key, value, cb) {
       var contract = self.cache.get(address)
       contract.stateRoot = storageTrie.root
       self._putAccount(address, contract, cb)
+      self.touched.push(address)
     }
   })
 }
@@ -304,3 +314,14 @@ proto.generateGenesis = function (initState, cb) {
     self.trie.put(address, account.serialize(), done)
   }, cb)
 }
+
+proto.accountIsEmpty = function (address, cb) {
+  var self = this
+  self.getAccount(address, function (err, account) {
+    if (err) {
+      return cb(err)
+    }
+
+    cb(null, account.nonce.toString('hex') === '' && account.balance.toString('hex') === '' && account.codeHash.toString('hex') === utils.SHA3_NULL_S)
+  })
+}

package.json

@@ -6,7 +6,7 @@
   "dependencies": {
     "async": "^2.1.2",
     "async-eventemitter": "^0.2.2",
-    "ethereum-common": "0.0.18",
+    "ethereum-common": "0.1.0",
     "ethereumjs-account": "^2.0.3",
     "ethereumjs-block": "^1.2.2",
     "ethereumjs-util": "4.5.0",
@@ -19,7 +19,7 @@
     "babelify": "^7.3.0",
     "ethereumjs-blockchain": "^1.4.1",
     "ethereumjs-testing": "https://github.com/ethereumjs/ethereumjs-testing",
-    "ethereumjs-tx": "1.1.4",
+    "ethereumjs-tx": "1.3.3",
     "level": "^1.4.0",
     "leveldown": "^1.4.6",
     "levelup": "^1.3.2",

tests/tester.js

@@ -2,7 +2,7 @@ const argv = require('minimist')(process.argv.slice(2))
 const async = require('async')
 const tape = require('tape')
 const testing = require('ethereumjs-testing')
-const FORK_CONFIG = argv.fork || 'EIP150'
+const FORK_CONFIG = argv.fork || 'EIP158'
 const skip = [
   'CreateHashCollision', // impossible hash collision on generating address
   'SuicidesMixingCoinbase', // sucides to the coinbase, since we run a blockLevel we create coinbase account.