Merge pull request #335 from ethereumjs/remove-contract-specific-commit

Remove contract specific commit

lib/cache.js

@@ -20,7 +20,6 @@ Cache.prototype.get = function (key) {
   var account = this.lookup(key)
   if (!account) {
     account = new Account()
-    account.exists = false
   }
   return account
 }
@@ -32,7 +31,6 @@ Cache.prototype.lookup = function (key) {
   var it = this._cache.find(key)
   if (it.node) {
     var account = new Account(it.value.val)
-    account.exists = it.value.exists
     return account
   }
 }
@@ -42,9 +40,7 @@ Cache.prototype._lookupAccount = function (address, cb) {
   self._trie.get(address, function (err, raw) {
     if (err) return cb(err)
     var account = new Account(raw)
-    var exists = !!raw
-    account.exists = exists
-    cb(null, account, exists)
+    cb(null, account)
   })
 }
 
@@ -54,9 +50,9 @@ Cache.prototype.getOrLoad = function (key, cb) {
   if (account) {
     cb(null, account)
   } else {
-    self._lookupAccount(key, function (err, account, exists) {
+    self._lookupAccount(key, function (err, account) {
       if (err) return cb(err)
-      self._update(key, account, false, exists)
+      self._update(key, account, false)
       cb(null, account)
     })
   }
@@ -74,7 +70,7 @@ Cache.prototype.warm = function (addresses, cb) {
     var address = Buffer.from(addressHex, 'hex')
     self._lookupAccount(address, function (err, account) {
       if (err) return done(err)
-      self._update(address, account, false, account.exists)
+      self._update(address, account, false)
       done()
     })
   }, cb)
@@ -133,20 +129,18 @@ Cache.prototype.del = function (key) {
   this._cache = this._cache.remove(key)
 }
 
-Cache.prototype._update = function (key, val, modified, exists) {
+Cache.prototype._update = function (key, val, modified) {
   key = key.toString('hex')
   var it = this._cache.find(key)
   if (it.node) {
     this._cache = it.update({
       val: val,
-      modified: modified,
-      exists: true
+      modified: modified
     })
   } else {
     this._cache = this._cache.insert(key, {
       val: val,
-      modified: modified,
-      exists: exists
+      modified: modified
     })
   }
 }

lib/opFns.js

@@ -549,44 +549,37 @@ module.exports = {
       subGas(runState, new BN(runState._common.param('gasPrices', 'callValueTransfer')))
     }
 
-    stateManager.exists(toAddress, function (err, exists) {
+    stateManager.accountIsEmpty(toAddress, function (err, empty) {
       if (err) {
         done(err)
         return
       }
 
-      stateManager.accountIsEmpty(toAddress, function (err, empty) {
-        if (err) {
-          done(err)
-          return
-        }
-
-        if (!exists || empty) {
-          if (!value.isZero()) {
-            try {
-              subGas(runState, new BN(runState._common.param('gasPrices', 'callNewAccount')))
-            } catch (e) {
-              done(e.error)
-              return
-            }
+      if (empty) {
+        if (!value.isZero()) {
+          try {
+            subGas(runState, new BN(runState._common.param('gasPrices', 'callNewAccount')))
+          } catch (e) {
+            done(e.error)
+            return
           }
         }
+      }
 
-        try {
-          checkCallMemCost(runState, options, localOpts)
-          checkOutOfGas(runState, options)
-        } catch (e) {
-          done(e.error)
-          return
-        }
+      try {
+        checkCallMemCost(runState, options, localOpts)
+        checkOutOfGas(runState, options)
+      } catch (e) {
+        done(e.error)
+        return
+      }
 
-        if (!value.isZero()) {
-          runState.gasLeft.iaddn(runState._common.param('gasPrices', 'callStipend'))
-          options.gasLimit.iaddn(runState._common.param('gasPrices', 'callStipend'))
-        }
+      if (!value.isZero()) {
+        runState.gasLeft.iaddn(runState._common.param('gasPrices', 'callStipend'))
+        options.gasLimit.iaddn(runState._common.param('gasPrices', 'callStipend'))
+      }
 
-        makeCall(runState, options, localOpts, done)
-      })
+      makeCall(runState, options, localOpts, done)
     })
   },
   CALLCODE: function (gas, toAddress, value, inOffset, inLength, outOffset, outLength, runState, done) {
@@ -705,28 +698,21 @@ module.exports = {
       outLength: outLength
     }
 
-    stateManager.exists(toAddress, function (err, exists) {
+    stateManager.accountIsEmpty(toAddress, function (err, empty) {
       if (err) {
         done(err)
         return
       }
 
-      stateManager.accountIsEmpty(toAddress, function (err, empty) {
-        if (err) {
-          done(err)
-          return
-        }
-
-        try {
-          checkCallMemCost(runState, options, localOpts)
-          checkOutOfGas(runState, options)
-        } catch (e) {
-          done(e.error)
-          return
-        }
+      try {
+        checkCallMemCost(runState, options, localOpts)
+        checkOutOfGas(runState, options)
+      } catch (e) {
+        done(e.error)
+        return
+      }
 
-        makeCall(runState, options, localOpts, done)
-      })
+      makeCall(runState, options, localOpts, done)
     })
   },
   RETURN: function (offset, length, runState) {
@@ -761,7 +747,7 @@ module.exports = {
         }
 
         if ((new BN(contract.balance)).gtn(0)) {
-          if (!toAccount.exists || empty) {
+          if (empty) {
             try {
               subGas(runState, new BN(runState._common.param('gasPrices', 'callNewAccount')))
             } catch (e) {

lib/runCall.js

@@ -200,7 +200,23 @@ module.exports = function (opts, cb) {
       gasUsed = results.gasUsed
       if (err) {
         results.logs = []
-        stateManager.revert(cb)
+        stateManager.revert(function (revertErr) {
+          if (revertErr || !isCompiled) cb(revertErr)
+          else {
+            // Empty precompiled contracts need to be deleted even in case of OOG
+            // because the bug in both Geth and Parity led to deleting RIPEMD precompiled in this case
+            // see https://github.com/ethereum/go-ethereum/pull/3341/files#diff-2433aa143ee4772026454b8abd76b9dd
+            // We mark the account as touched here, so that is can be removed among other touched empty accounts (after tx finalization)
+            if (err === ERROR.OUT_OF_GAS || err.error === ERROR.OUT_OF_GAS) {
+              stateManager.getAccount(toAddress, (getErr, acc) => {
+                if (getErr) cb(getErr)
+                else stateManager.putAccount(toAddress, acc, cb)
+              })
+            } else {
+              cb()
+            }
+          }
+        })
       } else {
         stateManager.commit(cb)
       }

lib/runCode.js

@@ -236,7 +236,6 @@ module.exports = function (opts, cb) {
     // remove any logs on error
     if (err) {
       runState.logs = []
-      stateManager.revertContracts()
       runState.vmError = true
     }
 

lib/runTx.js

@@ -42,7 +42,6 @@ module.exports = function (opts, cb) {
     populateCache,
     runTxHook,
     runCall,
-    saveTries,
     flushCache,
     runAfterTxHook
   ], function (err) {
@@ -194,10 +193,6 @@ module.exports = function (opts, cb) {
     }
   }
 
-  function saveTries (cb) {
-    self.stateManager.commitContracts(cb)
-  }
-
   function flushCache (cb) {
     self.stateManager.cache.flush(function () {
       if (opts.populateCache) {

lib/stateManager.js

@@ -25,6 +25,7 @@ function StateManager (opts = {}) {
   self._storageTries = {} // the storage trie cache
   self.cache = new Cache(self.trie)
   self._touched = new Set()
+  self._touchedStack = []
 }
 
 var proto = StateManager.prototype
@@ -39,13 +40,6 @@ proto.getAccount = function (address, cb) {
   this.cache.getOrLoad(address, cb)
 }
 
-// checks if an account exists
-proto.exists = function (address, cb) {
-  this.cache.getOrLoad(address, function (err, account) {
-    cb(err, account.exists)
-  })
-}
-
 // saves the account
 proto.putAccount = function (address, account, cb) {
   var self = this
@@ -76,10 +70,6 @@ proto.putAccountBalance = function (address, balance, cb) {
       return cb(err)
     }
 
-    if ((new BN(balance)).isZero() && !account.exists) {
-      return cb(null)
-    }
-
     account.balance = balance
     self.putAccount(address, account, cb)
   })
@@ -200,34 +190,14 @@ proto.clearContractStorage = function (address, cb) {
   }, cb)
 }
 
-proto.commitContracts = function (cb) {
-  var self = this
-  async.each(Object.keys(self._storageTries), function (address, cb) {
-    var trie = self._storageTries[address]
-    delete self._storageTries[address]
-    // TODO: this is broken on the block level; all the contracts get written to
-    // disk redardless of whether or not the block is valid
-    if (trie.isCheckpoint) {
-      trie.commit(cb)
-    } else {
-      cb()
-    }
-  }, cb)
-}
-
-proto.revertContracts = function () {
-  var self = this
-  self._storageTries = {}
-  self._touched.clear()
-}
-
 //
 // revision history
 //
 proto.checkpoint = function () {
   var self = this
   self.trie.checkpoint()
   self.cache.checkpoint()
+  self._touchedStack.push(new Set([...self._touched]))
 }
 
 proto.commit = function (cb) {
@@ -236,6 +206,7 @@ proto.commit = function (cb) {
   self.trie.commit(function () {
     // setup cache checkpointing
     self.cache.commit()
+    self._touchedStack.pop()
     cb()
   })
 }
@@ -246,6 +217,8 @@ proto.revert = function (cb) {
   self.trie.revert()
   // setup cache checkpointing
   self.cache.revert()
+  self._storageTries = {}
+  self._touched = self._touchedStack.pop()
   cb()
 }