fix nonceTracker

3 files changed, 25 insertions, 58 deletions

diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js
index c2f98e66a..59c8be8b7 100644
--- a/app/scripts/controllers/transactions.js
+++ b/app/scripts/controllers/transactions.js
@@ -25,7 +25,7 @@ module.exports = class TransactionController extends EventEmitter {
     this.blockTracker = opts.blockTracker
     this.nonceTracker = new NonceTracker({
       provider: this.provider,
-      blockTracker: this.blockTracker,
+      blockTracker: this.provider._blockTracker,
       getPendingTransactions: (address) => this.getFilteredTxList({ from: address, status: 'submitted' }),
     })
     this.query = opts.ethQuery
@@ -176,33 +176,7 @@ module.exports = class TransactionController extends EventEmitter {
     }, {})
   }
 
-  // approveTransaction (txId, cb = warn) {
-  //   promiseToCallback((async () => {
-  //     // approve
-  //     self.setTxStatusApproved(txId)
-  //     // get next nonce
-  //     const txMeta = this.getTx(txId)
-  //     const fromAddress = txMeta.txParams.from
-  //     const { nextNonce, releaseLock } = await this.nonceTracker.getNonceLock(fromAddress)
-  //     txMeta.txParams.nonce = nonce
-  //     this.updateTx(txMeta)
-  //     // sign transaction
-  //     const rawTx = await denodeify(self.signTransaction.bind(self))(txId)
-  //     await denodeify(self.publishTransaction.bind(self))(txId, rawTx)
-  //   })())((err) => {
-  //     if (err) {
-  //       this.setTxStatusFailed(txId, {
-  //         errCode: err.errCode || err,
-  //         message: err.message || 'Transaction failed during approval',
-  //       })
-  //     }
-  //     // must set transaction to submitted/failed before releasing lock
-  //     releaseLock()
-  //     cb(err)
-  //   })
-  // }
-
-  async approveTransaction (txId) {
+  async approveTransaction (txId, cb = warn) {
     let nonceLock
     try {
       // approve
@@ -215,9 +189,10 @@ module.exports = class TransactionController extends EventEmitter {
       this.updateTx(txMeta)
       // sign transaction
       const rawTx = await denodeify(this.signTransaction.bind(this))(txId)
-      await denodeify(this.publishTransaction.bind(this))(txId, rawTx)
+      await this.publishTransaction(txId, rawTx)
       // must set transaction to submitted/failed before releasing lock
       nonceLock.releaseLock()
+      cb()
     } catch (err) {
       this.setTxStatusFailed(txId, {
         errCode: err.errCode || err,
@@ -226,7 +201,7 @@ module.exports = class TransactionController extends EventEmitter {
       // must set transaction to submitted/failed before releasing lock
       if (nonceLock) nonceLock.releaseLock()
       // continue with error chain
-      throw err
+      cb(err)
     }
   }
 
@@ -260,16 +235,17 @@ module.exports = class TransactionController extends EventEmitter {
     })
   }
 
-  publishTransaction (txId, rawTx, cb = warn) {
+  publishTransaction (txId, rawTx) {
     const txMeta = this.getTx(txId)
     txMeta.rawTx = rawTx
     this.updateTx(txMeta)
-
-    this.txProviderUtils.publishTransaction(rawTx, (err, txHash) => {
-      if (err) return cb(err)
-      this.setTxHash(txId, txHash)
-      this.setTxStatusSubmitted(txId)
-      cb()
+    return new Promise((resolve, reject) => {
+      this.txProviderUtils.publishTransaction(rawTx, (err, txHash) => {
+        if (err) reject(err)
+        this.setTxHash(txId, txHash)
+        this.setTxStatusSubmitted(txId)
+        resolve()
+      })
     })
   }
 
@@ -414,7 +390,6 @@ module.exports = class TransactionController extends EventEmitter {
     this.emit(`${txMeta.id}:${status}`, txId)
     if (status === 'submitted' || status === 'rejected') {
       this.emit(`${txMeta.id}:finished`, txMeta)
-
     }
     this.updateTx(txMeta)
     this.emit('updateBadge')
diff --git a/app/scripts/lib/nonce-tracker.js b/app/scripts/lib/nonce-tracker.js
index ff2317a91..4ea511dec 100644
--- a/app/scripts/lib/nonce-tracker.js
+++ b/app/scripts/lib/nonce-tracker.js
@@ -2,7 +2,7 @@ const EthQuery = require('eth-query')
 
 class NonceTracker {
 
-  constructor({ blockTracker, provider, getPendingTransactions }) {
+  constructor ({ blockTracker, provider, getPendingTransactions }) {
     this.blockTracker = blockTracker
     this.ethQuery = new EthQuery(provider)
     this.getPendingTransactions = getPendingTransactions
@@ -11,7 +11,7 @@ class NonceTracker {
 
   // releaseLock must be called
   // releaseLock must be called after adding signed tx to pending transactions (or discarding)
-  async getNonceLock(address) {
+  async getNonceLock (address) {
     // await lock free
     await this.lockMap[address]
     // take lock
@@ -21,12 +21,12 @@ class NonceTracker {
     const blockNumber = currentBlock.number
     const pendingTransactions = this.getPendingTransactions(address)
     const baseCount = await this._getTxCount(address, blockNumber)
-    const nextNonce = parseInt(baseCount) + pendingTransactions.length + 1
+    const nextNonce = parseInt(baseCount) + pendingTransactions.length
     // return next nonce and release cb
     return { nextNonce: nextNonce.toString(16), releaseLock }
   }
 
-  async _getCurrentBlock() {
+  async _getCurrentBlock () {
     const currentBlock = this.blockTracker.getCurrentBlock()
     if (currentBlock) return currentBlock
     return await Promise((reject, resolve) => {
@@ -34,15 +34,13 @@ class NonceTracker {
     })
   }
 
-  _takeLock(lockId) {
+  _takeLock (lockId) {
     let releaseLock = null
     // create and store lock
     const lock = new Promise((resolve, reject) => { releaseLock = resolve })
     this.lockMap[lockId] = lock
     // setup lock teardown
-    lock.then(() => {
-      delete this.lockMap[lockId]
-    })
+    lock.then(() => delete this.lockMap[lockId])
     return releaseLock
   }
 
diff --git a/test/unit/nonce-tracker-test.js b/test/unit/nonce-tracker-test.js
index 5a05d6170..16cd6d008 100644
--- a/test/unit/nonce-tracker-test.js
+++ b/test/unit/nonce-tracker-test.js
@@ -3,31 +3,25 @@ const NonceTracker = require('../../app/scripts/lib/nonce-tracker')
 
 describe('Nonce Tracker', function () {
   let nonceTracker, provider, getPendingTransactions, pendingTxs
-  const noop = () => {}
 
 
   beforeEach(function () {
-    pendingTxs =[{
+    pendingTxs = [{
       'status': 'submitted',
       'txParams': {
         'from': '0x7d3517b0d011698406d6e0aed8453f0be2697926',
         'gas': '0x30d40',
         'value': '0x0',
-        'nonce': '0x1',
+        'nonce': '0x0',
       },
     }]
 
 
     getPendingTransactions = () => pendingTxs
-    provider = { sendAsync: (_, cb) => { cb(undefined , {result: '0x0'}) }, }
+    provider = { sendAsync: (_, cb) => { cb(undefined, {result: '0x0'}) } }
     nonceTracker = new NonceTracker({
       blockTracker: {
-        getCurrentBlock: () =>  '0x11b568',
-        once: (...args) => {
-          setTimeout(() => {
-            args.pop()()
-          }, 5000)
-        }
+        getCurrentBlock: () => '0x11b568',
       },
       provider,
       getPendingTransactions,
@@ -38,8 +32,8 @@ describe('Nonce Tracker', function () {
     it('should work', async function (done) {
       this.timeout(15000)
       const nonceLock = await nonceTracker.getNonceLock('0x7d3517b0d011698406d6e0aed8453f0be2697926')
-      assert.equal(nonceLock.nextNonce, '2', 'nonce should be 2')
-      nonceLock.releaseLock()
+      assert.equal(nonceLock.nextNonce, '1', 'nonce should be 1')
+      await nonceLock.releaseLock()
       done()
     })
   })