Merge branch 'master' into 3.8.5

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Current Master
 
+- No longer validate nonce client-side in retry loop.
+- Fix bug where insufficient balance error was sometimes shown on successful transactions.
+
 ## 3.8.5 2017-7-7
 
 - Fix transaction resubmit logic to fail slightly less eagerly.

app/scripts/controllers/transactions.js

@@ -417,46 +417,42 @@ module.exports = class TransactionController extends EventEmitter {
     // only try resubmitting if their are transactions to resubmit
     if (!pending.length) return
     const resubmit = denodeify(this._resubmitTx.bind(this))
-    pending.forEach((txMeta) => resubmit(txMeta)
-    .catch((reason) => {
+    pending.forEach((txMeta) => resubmit(txMeta).catch((err) => {
       /*
       Dont marked as failed if the error is a "known" transaction warning
       "there is already a transaction with the same sender-nonce
       but higher/same gas price"
       */
-      const errorMessage = reason.message.toLowerCase()
+      const errorMessage = err.message.toLowerCase()
       const isKnownTx = (
         // geth
-        errorMessage === 'replacement transaction underpriced'
-        || errorMessage.startsWith('known transaction')
+        errorMessage.includes('replacement transaction underpriced')
+        || errorMessage.includes('known transaction')
         // parity
-        || errorMessage === 'gas price too low to replace'
+        || errorMessage.includes('gas price too low to replace')
+        || errorMessage.includes('transaction with the same hash was already imported')
+        // other
+        || errorMessage.includes('gateway timeout')
       )
       // ignore resubmit warnings, return early
-      if (!isKnownTx) this.setTxStatusFailed(txMeta.id, reason.message)
+      if (isKnownTx) return
+      // encountered real error - transition to error state
+      this.setTxStatusFailed(txMeta.id, {
+        errCode: err.errCode || err,
+        message: err.message,
+      })
     }))
   }
 
   _resubmitTx (txMeta, cb) {
     const address = txMeta.txParams.from
     const balance = this.ethStore.getState().accounts[address].balance
-    const nonce = Number.parseInt(this.ethStore.getState().accounts[address].nonce)
-    const txNonce = Number.parseInt(txMeta.txParams.nonce)
-    const gtBalance = Number.parseInt(txMeta.txParams.value) > Number.parseInt(balance)
     if (!('retryCount' in txMeta)) txMeta.retryCount = 0
 
     // if the value of the transaction is greater then the balance, fail.
-    if (gtBalance) {
+    if (!this.txProviderUtils.sufficientBalance(txMeta.txParams, balance)) {
       const message = 'Insufficient balance.'
-      this.setTxStatusFailed(txMeta.id, message)
-      cb()
-      return log.error(message)
-    }
-
-    // if the nonce of the transaction is lower then the accounts nonce, fail.
-    if (txNonce < nonce) {
-      const message = 'Invalid nonce.'
-      this.setTxStatusFailed(txMeta.id, message)
+      this.setTxStatusFailed(txMeta.id, { message })
       cb()
       return log.error(message)
     }

app/scripts/lib/tx-utils.js

@@ -118,6 +118,15 @@ module.exports = class txProviderUtils {
     }
   }
 
+  sufficientBalance (tx, hexBalance) {
+    const balance = hexToBn(hexBalance)
+    const value = hexToBn(tx.value)
+    const gasLimit = hexToBn(tx.gas)
+    const gasPrice = hexToBn(tx.gasPrice)
+
+    const maxCost = value.add(gasLimit.mul(gasPrice))
+    return balance.gte(maxCost)
+  }
 
 }
 

app/scripts/metamask-controller.js

@@ -367,7 +367,7 @@ module.exports = class MetamaskController extends EventEmitter {
     function onResponse (err, request, response) {
       if (err) return console.error(err)
       if (response.error) {
-        console.error('Error in RPC response:\n', response.error)
+        console.error('Error in RPC response:\n', response)
       }
       if (request.isMetamaskInternal) return
       log.info(`RPC (${originDomain}):`, request, '->', response)

app/scripts/migrations/016.js

@@ -0,0 +1,41 @@
+const version = 16
+
+/*
+
+This migration sets transactions with the 'Gave up submitting tx.' err message
+to a 'failed' stated
+
+*/
+
+const clone = require('clone')
+
+module.exports = {
+  version,
+
+  migrate: function (originalVersionedData) {
+    const versionedData = clone(originalVersionedData)
+    versionedData.meta.version = version
+    try {
+      const state = versionedData.data
+      const newState = transformState(state)
+      versionedData.data = newState
+    } catch (err) {
+      console.warn(`MetaMask Migration #${version}` + err.stack)
+    }
+    return Promise.resolve(versionedData)
+  },
+}
+
+function transformState (state) {
+  const newState = state
+  const transactions = newState.TransactionController.transactions
+  newState.TransactionController.transactions = transactions.map((txMeta) => {
+    if (!txMeta.err) return txMeta
+    if (txMeta.err === 'transaction with the same hash was already imported.') {
+      txMeta.status = 'submitted'
+      delete txMeta.err
+    }
+    return txMeta
+  })
+  return newState
+}

app/scripts/migrations/index.js

@@ -26,4 +26,5 @@ module.exports = [
   require('./013'),
   require('./014'),
   require('./015'),
+  require('./016'),
 ]

package.json

@@ -124,7 +124,7 @@
     "valid-url": "^1.0.9",
     "vreme": "^3.0.2",
     "web3": "0.19.1",
-    "web3-provider-engine": "^13.1.1",
+    "web3-provider-engine": "^13.2.8",
     "web3-stream-provider": "^3.0.1",
     "xtend": "^4.0.1"
   },

test/unit/infura-controller-test.js

@@ -1,34 +1,34 @@
 // polyfill fetch
-global.fetch = function () {return Promise.resolve({
-    json: () => { return Promise.resolve({"mainnet": "ok", "ropsten": "degraded", "kovan": "down", "rinkeby": "ok"}) },
-  })
-}
-const assert = require('assert')
-const InfuraController = require('../../app/scripts/controllers/infura')
-
-describe('infura-controller', function () {
-  var infuraController
-
-  beforeEach(function () {
-    infuraController = new InfuraController()
-  })
-
-  describe('network status queries', function () {
-    describe('#checkInfuraNetworkStatus', function () {
-      it('should return an object reflecting the network statuses', function (done) {
-        this.timeout(15000)
-        infuraController.checkInfuraNetworkStatus()
-          .then(() => {
-            const networkStatus = infuraController.store.getState().infuraNetworkStatus
-            assert.equal(Object.keys(networkStatus).length, 4)
-            assert.equal(networkStatus.mainnet, 'ok')
-            assert.equal(networkStatus.ropsten, 'degraded')
-            assert.equal(networkStatus.kovan, 'down')
-          })
-          .then(() => done())
-          .catch(done)
-
-      })
-    })
-  })
-})
+// global.fetch = function () {return Promise.resolve({
+//     json: () => { return Promise.resolve({"mainnet": "ok", "ropsten": "degraded", "kovan": "down", "rinkeby": "ok"}) },
+//   })
+// }
+// const assert = require('assert')
+// const InfuraController = require('../../app/scripts/controllers/infura')
+//
+// describe('infura-controller', function () {
+//   var infuraController
+//
+//   beforeEach(function () {
+//     infuraController = new InfuraController()
+//   })
+//
+//   describe('network status queries', function () {
+//     describe('#checkInfuraNetworkStatus', function () {
+//       it('should return an object reflecting the network statuses', function (done) {
+//         this.timeout(15000)
+//         infuraController.checkInfuraNetworkStatus()
+//           .then(() => {
+//             const networkStatus = infuraController.store.getState().infuraNetworkStatus
+//             assert.equal(Object.keys(networkStatus).length, 4)
+//             assert.equal(networkStatus.mainnet, 'ok')
+//             assert.equal(networkStatus.ropsten, 'degraded')
+//             assert.equal(networkStatus.kovan, 'down')
+//           })
+//           .then(() => done())
+//           .catch(done)
+//
+//       })
+//     })
+//   })
+// })

test/unit/tx-utils-test.js

@@ -16,6 +16,44 @@ describe('txUtils', function () {
     }))
   })
 
+  describe('#sufficientBalance', function () {
+    it('returns true if max tx cost is equal to balance.', function () {
+      const tx = {
+        'value': '0x1',
+        'gas': '0x2',
+        'gasPrice': '0x3',
+      }
+      const balance = '0x8'
+
+      const result = txUtils.sufficientBalance(tx, balance)
+      assert.ok(result, 'sufficient balance found.')
+    })
+
+    it('returns true if max tx cost is less than balance.', function () {
+      const tx = {
+        'value': '0x1',
+        'gas': '0x2',
+        'gasPrice': '0x3',
+      }
+      const balance = '0x9'
+
+      const result = txUtils.sufficientBalance(tx, balance)
+      assert.ok(result, 'sufficient balance found.')
+    })
+
+    it('returns false if max tx cost is more than balance.', function () {
+      const tx = {
+        'value': '0x1',
+        'gas': '0x2',
+        'gasPrice': '0x3',
+      }
+      const balance = '0x6'
+
+      const result = txUtils.sufficientBalance(tx, balance)
+      assert.ok(!result, 'insufficient balance found.')
+    })
+  })
+
   describe('chain Id', function () {
     it('prepares a transaction with the provided chainId', function () {
       const txParams = {