transaction controller - use nonce-tracker

app/scripts/controllers/transactions.js

@@ -4,9 +4,10 @@ const extend = require('xtend')
 const Semaphore = require('semaphore')
 const ObservableStore = require('obs-store')
 const ethUtil = require('ethereumjs-util')
+const denodeify = require('denodeify')
 const TxProviderUtil = require('../lib/tx-utils')
 const createId = require('../lib/random-id')
-const denodeify = require('denodeify')
+const NonceTracker = require('../lib/nonce-tracker')
 
 const RETRY_LIMIT = 200
 
@@ -22,6 +23,11 @@ module.exports = class TransactionController extends EventEmitter {
     this.txHistoryLimit = opts.txHistoryLimit
     this.provider = opts.provider
     this.blockTracker = opts.blockTracker
+    this.nonceTracker = new NonceTracker({
+      provider: this.provider,
+      blockTracker: this.blockTracker,
+      getPendingTransactions: (address) => this.getFilteredTxList({ from: address, status: 'submitted' }),
+    })
     this.query = opts.ethQuery
     this.txProviderUtils = new TxProviderUtil(this.query)
     this.blockTracker.on('block', this.checkForTxInBlock.bind(this))
@@ -169,29 +175,58 @@ module.exports = class TransactionController extends EventEmitter {
     }, {})
   }
 
-  approveTransaction (txId, cb = warn) {
-    const self = this
+  // 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) {
+    let nonceLock
+    try {
       // approve
-    self.setTxStatusApproved(txId)
-    // only allow one tx at a time for atomic nonce usage
-    self.nonceLock.take(() => {
-      // begin signature process
-      async.waterfall([
-        (cb) => self.fillInTxParams(txId, cb),
-        (cb) => self.signTransaction(txId, cb),
-        (rawTx, cb) => self.publishTransaction(txId, rawTx, cb),
-      ], (err) => {
-        self.nonceLock.leave()
-        if (err) {
+      this.setTxStatusApproved(txId)
+      // get next nonce
+      const txMeta = this.getTx(txId)
+      const fromAddress = txMeta.txParams.from
+      nonceLock = await this.nonceTracker.getNonceLock(fromAddress)
+      txMeta.txParams.nonce = nonceLock.nextNonce
+      this.updateTx(txMeta)
+      // sign transaction
+      const rawTx = await denodeify(this.signTransaction.bind(this))(txId)
+      await denodeify(this.publishTransaction.bind(this))(txId, rawTx)
+      // must set transaction to submitted/failed before releasing lock
+      nonceLock.releaseLock()
+    } catch (err) {
       this.setTxStatusFailed(txId, {
         errCode: err.errCode || err,
         message: err.message || 'Transaction failed during approval',
       })
-          return cb(err)
+      // must set transaction to submitted/failed before releasing lock
+      if (nonceLock) nonceLock.releaseLock()
+      // continue with error chain
+      throw err
     }
-        cb()
-      })
-    })
   }
 
   cancelTransaction (txId, cb = warn) {
@@ -199,15 +234,6 @@ module.exports = class TransactionController extends EventEmitter {
     cb()
   }
 
-  fillInTxParams (txId, cb) {
-    const txMeta = this.getTx(txId)
-    this.txProviderUtils.fillInTxParams(txMeta.txParams, (err) => {
-      if (err) return cb(err)
-      this.updateTx(txMeta)
-      cb()
-    })
-  }
-
   getChainId () {
     const networkState = this.networkStore.getState()
     const getChainId = parseInt(networkState)

app/scripts/metamask-controller.js

@@ -290,7 +290,7 @@ module.exports = class MetamaskController extends EventEmitter {
       exportAccount: nodeify(keyringController.exportAccount).bind(keyringController),
 
       // txController
-      approveTransaction: txController.approveTransaction.bind(txController),
+      approveTransaction: nodeify(txController.approveTransaction).bind(txController),
       cancelTransaction: txController.cancelTransaction.bind(txController),
       updateAndApproveTransaction: this.updateAndApproveTx.bind(this),
 

test/unit/tx-controller-test.js

@@ -1,5 +1,4 @@
 const assert = require('assert')
-const EventEmitter = require('events')
 const ethUtil = require('ethereumjs-util')
 const EthTx = require('ethereumjs-tx')
 const EthQuery = require('eth-query')
@@ -19,13 +18,15 @@ describe('Transaction Controller', function () {
     txController = new TransactionController({
       networkStore: new ObservableStore(currentNetworkId),
       txHistoryLimit: 10,
-      blockTracker: new EventEmitter(),
-      ethQuery: new EthQuery(new EventEmitter()),
+      blockTracker: { getCurrentBlock: noop, on: noop },
+      provider: { sendAsync: noop },
+      ethQuery: new EthQuery({ sendAsync: noop }),
       signTransaction: (ethTx) => new Promise((resolve) => {
         ethTx.sign(privKey)
         resolve()
       }),
     })
+    txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop })
   })
 
   describe('#validateTxParams', function () {