Merge pull request #1002 from MetaMask/bug-submitTx

handle tx finalization in controllers instead of provider-engine

app/scripts/background.js

@@ -22,7 +22,7 @@ const controller = new MetamaskController({
   setData,
   loadData,
 })
-const txManager = controller.txManager
+
 function triggerUi () {
   if (!popupIsOpen) notification.show()
 }
@@ -93,7 +93,8 @@ function setupControllerConnection (stream) {
 // plugin badge text
 //
 
-txManager.on('updateBadge', updateBadge)
+controller.txManager.on('updateBadge', updateBadge)
+updateBadge()
 
 function updateBadge () {
   var label = ''

app/scripts/keyring-controller.js

@@ -316,13 +316,11 @@ module.exports = class KeyringController extends EventEmitter {
   // This method signs tx and returns a promise for
   // TX Manager to update the state after signing
 
-  signTransaction (ethTx, selectedAddress, txId) {
+  signTransaction (ethTx, _fromAddress) {
-    const address = normalize(selectedAddress)
+    const fromAddress = normalize(_fromAddress)
-    return this.getKeyringForAccount(address)
+    return this.getKeyringForAccount(fromAddress)
     .then((keyring) => {
-      return keyring.signTransaction(address, ethTx)
+      return keyring.signTransaction(fromAddress, ethTx)
-    }).then((tx) => {
-      return {tx, txId}
     })
   }
   // Add Unconfirmed Message

app/scripts/lib/eth-store.js

@@ -43,7 +43,9 @@ EthereumStore.prototype.addAccount = function (address) {
   self._currentState.accounts[address] = {}
   self._didUpdate()
   if (!self.currentBlockNumber) return
-  self._updateAccount(address, noop)
+  self._updateAccount(address, () => {
+    self._didUpdate()
+  })
 }
 
 EthereumStore.prototype.removeAccount = function (address) {

app/scripts/lib/tx-utils.js

@@ -1,6 +1,8 @@
 const async = require('async')
 const EthQuery = require('eth-query')
 const ethUtil = require('ethereumjs-util')
+const Transaction = require('ethereumjs-tx')
+const normalize = require('./sig-util').normalize
 const BN = ethUtil.BN
 
 /*
@@ -14,6 +16,7 @@ module.exports = class txProviderUtils {
     this.provider = provider
     this.query = new EthQuery(provider)
   }
+
   analyzeGasUsage (txData, cb) {
     var self = this
     this.query.getBlockByNumber('latest', true, (err, block) => {
@@ -71,4 +74,59 @@ module.exports = class txProviderUtils {
     const correct = bnGas.add(gasBuffer)
     return ethUtil.addHexPrefix(correct.toString(16))
   }
+
+  fillInTxParams (txParams, cb) {
+    let fromAddress = txParams.from
+    let reqs = {}
+
+    if (isUndef(txParams.gas)) reqs.gas = (cb) => this.query.estimateGas(txParams, cb)
+    if (isUndef(txParams.gasPrice)) reqs.gasPrice = (cb) => this.query.gasPrice(cb)
+    if (isUndef(txParams.nonce)) reqs.nonce = (cb) => this.query.getTransactionCount(fromAddress, 'pending', cb)
+
+    async.parallel(reqs, function(err, result) {
+      if (err) return cb(err)
+      // write results to txParams obj
+      Object.assign(txParams, result)
+      cb()
+    })
+  }
+
+  // builds ethTx from txParams object
+  buildEthTxFromParams (txParams, gasMultiplier = 1) {
+    // apply gas multiplyer
+    let gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16)
+    // multiply and divide by 100 so as to add percision to integer mul
+    gasPrice = gasPrice.mul(new BN(gasMultiplier * 100, 10)).div(new BN(100, 10))
+    txParams.gasPrice = ethUtil.intToHex(gasPrice.toNumber())
+    // normalize values
+    txParams.to = normalize(txParams.to)
+    txParams.from = normalize(txParams.from)
+    txParams.value = normalize(txParams.value)
+    txParams.data = normalize(txParams.data)
+    txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas)
+    txParams.nonce = normalize(txParams.nonce)
+    // build ethTx
+    const ethTx = new Transaction(txParams)
+    return ethTx
+  }
+
+  publishTransaction (rawTx, cb) {
+    this.query.sendRawTransaction(rawTx, cb)
+  }
+
+  validateTxParams (txParams, cb) {
+    if (('value' in txParams) && txParams.value.indexOf('-') === 0) {
+      cb(new Error(`Invalid transaction value of ${txParams.value} not a positive number.`))
+    } else {
+      cb()
+    }
+  }
+
+
+}
+
+// util
+
+function isUndef(value) {
+  return value === undefined
 }

app/scripts/metamask-controller.js

@@ -45,6 +45,7 @@ module.exports = class MetamaskController extends EventEmitter {
       getSelectedAccount: this.configManager.getSelectedAccount.bind(this.configManager),
       getGasMultiplier: this.configManager.getGasMultiplier.bind(this.configManager),
       getNetwork: this.getStateNetwork.bind(this),
+      signTransaction: this.keyringController.signTransaction.bind(this.keyringController),
       provider: this.provider,
       blockTracker: this.provider,
     })
@@ -189,26 +190,7 @@ module.exports = class MetamaskController extends EventEmitter {
         cb(null, result)
       },
       // tx signing
-      approveTransaction: this.newUnsignedTransaction.bind(this),
+      processTransaction: (txParams, cb) => this.newUnapprovedTransaction(txParams, cb),
-      signTransaction: (txParams, cb) => {
-        this.txManager.formatTxForSigining(txParams)
-        .then(({ethTx, address, txId}) => {
-          return this.keyringController.signTransaction(ethTx, address, txId)
-        })
-        .then(({tx, txId}) => {
-          return this.txManager.resolveSignedTransaction({tx, txId})
-        })
-        .then((rawTx) => {
-          cb(null, rawTx)
-          this.sendUpdate()
-          this.txManager.emit(`${txParams.metamaskId}:signingComplete`)
-        })
-        .catch((err) => {
-          console.error(err)
-          cb(err)
-        })
-      },
-
       // msg signing
       approveMessage: this.newUnsignedMessage.bind(this),
       signMessage: (...args) => {
@@ -257,22 +239,24 @@ module.exports = class MetamaskController extends EventEmitter {
     return publicConfigStore
   }
 
-  newUnsignedTransaction (txParams, onTxDoneCb) {
+  newUnapprovedTransaction (txParams, cb) {
-    const txManager = this.txManager
+    const self = this
-    const err = this.enforceTxValidations(txParams)
+    self.txManager.addUnapprovedTransaction(txParams, (err, txMeta) => {
-    if (err) return onTxDoneCb(err)
+      if (err) return cb(err)
-    txManager.addUnapprovedTransaction(txParams, onTxDoneCb, (err, txData) => {
+      self.sendUpdate()
-      if (err) return onTxDoneCb(err)
+      self.opts.showUnapprovedTx(txMeta)
-      this.sendUpdate()
+      // listen for tx completion (success, fail)
-      this.opts.showUnapprovedTx(txParams, txData, onTxDoneCb)
+      self.txManager.once(`${txMeta.id}:finished`, (status) => {
+        switch (status) {
+          case 'submitted':
+            return cb(null, txMeta.hash)
+          case 'rejected':
+            return cb(new Error('MetaMask Tx Signature: User denied transaction signature.'))
+          default:
+            return cb(new Error(`MetaMask Tx Signature: Unknown problem: ${JSON.stringify(txMeta.txParams)}`))
+        }
+      })
     })
-  }
-
-  enforceTxValidations (txParams) {
-    if (('value' in txParams) && txParams.value.indexOf('-') === 0) {
-      const msg = `Invalid transaction value of ${txParams.value} not a positive number.`
-      return new Error(msg)
-    }
   }
 
   newUnsignedMessage (msgParams, cb) {

app/scripts/transaction-manager.js

@@ -1,11 +1,11 @@
 const EventEmitter = require('events')
+const async = require('async')
 const extend = require('xtend')
+const Semaphore = require('semaphore')
 const ethUtil = require('ethereumjs-util')
-const Transaction = require('ethereumjs-tx')
+const BN = require('ethereumjs-util').BN
-const BN = ethUtil.BN
 const TxProviderUtil = require('./lib/tx-utils')
 const createId = require('./lib/random-id')
-const normalize = require('./lib/sig-util').normalize
 
 module.exports = class TransactionManager extends EventEmitter {
   constructor (opts) {
@@ -20,6 +20,8 @@ module.exports = class TransactionManager extends EventEmitter {
     this.blockTracker.on('block', this.checkForTxInBlock.bind(this))
     this.getGasMultiplier = opts.getGasMultiplier
     this.getNetwork = opts.getNetwork
+    this.signEthTx = opts.signTransaction
+    this.nonceLock = Semaphore(1)
   }
 
   getState () {
@@ -33,11 +35,12 @@ module.exports = class TransactionManager extends EventEmitter {
 
 //   Returns the tx list
   getTxList () {
-    return this.txList
+    let network = this.getNetwork()
+    return this.txList.filter(txMeta => txMeta.metamaskNetworkId === network)
   }
 
   // Adds a tx to the txlist
-  addTx (txMeta, onTxDoneCb = warn) {
+  addTx (txMeta) {
     var txList = this.getTxList()
     var txHistoryLimit = this.txHistoryLimit
 
@@ -53,16 +56,11 @@ module.exports = class TransactionManager extends EventEmitter {
     txList.push(txMeta)
 
     this._saveTxList(txList)
-    // keep the onTxDoneCb around in a listener
-    // for after approval/denial (requires user interaction)
-    // This onTxDoneCb fires completion to the Dapp's write operation.
     this.once(`${txMeta.id}:signed`, function (txId) {
       this.removeAllListeners(`${txMeta.id}:rejected`)
-      onTxDoneCb(null, true)
     })
     this.once(`${txMeta.id}:rejected`, function (txId) {
       this.removeAllListeners(`${txMeta.id}:signed`)
-      onTxDoneCb(null, false)
     })
 
     this.emit('updateBadge')
@@ -94,36 +92,40 @@ module.exports = class TransactionManager extends EventEmitter {
     return this.getTxsByMetaData('status', 'signed').length
   }
 
-  addUnapprovedTransaction (txParams, onTxDoneCb, cb) {
+  addUnapprovedTransaction (txParams, done) {
-    // create txData obj with parameters and meta data
+    let txMeta
-    var time = (new Date()).getTime()
+    async.waterfall([
-    var txId = createId()
+      // validate
+      (cb) => this.txProviderUtils.validateTxParams(txParams, cb),
+      // prepare txMeta
+      (cb) => {
+        // create txMeta obj with parameters and meta data
+        let time = (new Date()).getTime()
+        let txId = createId()
         txParams.metamaskId = txId
         txParams.metamaskNetworkId = this.getNetwork()
-    var txData = {
+        txMeta = {
           id: txId,
-      txParams: txParams,
           time: time,
           status: 'unapproved',
           gasMultiplier: this.getGasMultiplier() || 1,
           metamaskNetworkId: this.getNetwork(),
+          txParams: txParams,
         }
-    this.txProviderUtils.analyzeGasUsage(txData, this.txDidComplete.bind(this, txData, onTxDoneCb, cb))
         // calculate metadata for tx
-  }
+        this.txProviderUtils.analyzeGasUsage(txMeta, cb)
-
+      },
-  txDidComplete (txMeta, onTxDoneCb, cb, err) {
+      // save txMeta
-    if (err) return cb(err)
+      (cb) => {
-    var {maxCost, txFee} = this.getMaxTxCostAndFee(txMeta)
+        this.addTx(txMeta)
-    txMeta.maxCost = maxCost
+        this.setMaxTxCostAndFee(txMeta)
-    txMeta.txFee = txFee
-    this.addTx(txMeta, onTxDoneCb)
         cb(null, txMeta)
+      },
+    ], done)
   }
 
-  getMaxTxCostAndFee (txMeta) {
+  setMaxTxCostAndFee (txMeta) {
     var txParams = txMeta.txParams
-
     var gasMultiplier = txMeta.gasMultiplier
     var gasCost = new BN(ethUtil.stripHexPrefix(txParams.gas || txMeta.estimatedGas), 16)
     var gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice || '0x4a817c800'), 16)
@@ -131,7 +133,10 @@ module.exports = class TransactionManager extends EventEmitter {
     var txFee = gasCost.mul(gasPrice)
     var txValue = new BN(ethUtil.stripHexPrefix(txParams.value || '0x0'), 16)
     var maxCost = txValue.add(txFee)
-    return {maxCost, txFee}
+    txMeta.txFee = txFee
+    txMeta.txValue = txValue
+    txMeta.maxCost = maxCost
+    this.updateTx(txMeta)
   }
 
   getUnapprovedTxList () {
@@ -144,8 +149,25 @@ module.exports = class TransactionManager extends EventEmitter {
   }
 
   approveTransaction (txId, cb = warn) {
-    this.setTxStatusSigned(txId)
+    const self = this
-    this.once(`${txId}:signingComplete`, cb)
+    // 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.setTxStatusFailed(txId)
+          return cb(err)
+        }
+        cb()
+      })
+    })
   }
 
   cancelTransaction (txId, cb = warn) {
@@ -153,37 +175,44 @@ module.exports = class TransactionManager extends EventEmitter {
     cb()
   }
 
-  // formats txParams so the keyringController can sign it
+  fillInTxParams (txId, cb) {
-  formatTxForSigining (txParams) {
+    let txMeta = this.getTx(txId)
-    var address = txParams.from
+    this.txProviderUtils.fillInTxParams(txMeta.txParams, (err) => {
-    var metaTx = this.getTx(txParams.metamaskId)
+      if (err) return cb(err)
-    var gasMultiplier = metaTx.gasMultiplier
+      this.updateTx(txMeta)
-    var gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16)
+      cb()
-    gasPrice = gasPrice.mul(new BN(gasMultiplier * 100, 10)).div(new BN(100, 10))
+    })
-    txParams.gasPrice = ethUtil.intToHex(gasPrice.toNumber())
+  }
 
-    // normalize values
+  signTransaction (txId, cb) {
-    txParams.to = normalize(txParams.to)
+    let txMeta = this.getTx(txId)
-    txParams.from = normalize(txParams.from)
+    let txParams = txMeta.txParams
-    txParams.value = normalize(txParams.value)
+    let fromAddress = txParams.from
-    txParams.data = normalize(txParams.data)
+    let ethTx = this.txProviderUtils.buildEthTxFromParams(txParams, txMeta.gasMultiplier)
-    txParams.gasLimit = normalize(txParams.gasLimit || txParams.gas)
+    this.signEthTx(ethTx, fromAddress).then(() => {
-    txParams.nonce = normalize(txParams.nonce)
+      this.updateTxAsSigned(txMeta.id, ethTx)
-    const ethTx = new Transaction(txParams)
+      cb(null, ethUtil.bufferToHex(ethTx.serialize()))
-    var txId = txParams.metamaskId
+    }).catch((err) => {
-    return Promise.resolve({ethTx, address, txId})
+      cb(err)
+    })
+  }
+
+  publishTransaction (txId, rawTx, cb) {
+    this.txProviderUtils.publishTransaction(rawTx, (err) => {
+      if (err) return cb(err)
+      this.setTxStatusSubmitted(txId)
+      cb()
+    })
   }
 
   // receives a signed tx object and updates the tx hash
-  // and pass it to the cb to be sent off
+  updateTxAsSigned (txId, ethTx) {
-  resolveSignedTransaction ({tx, txId, cb = warn}) {
     // Add the tx hash to the persisted meta-tx object
-    var txHash = ethUtil.bufferToHex(tx.hash())
+    let txHash = ethUtil.bufferToHex(ethTx.hash())
-    var metaTx = this.getTx(txId)
+    let txMeta = this.getTx(txId)
-    metaTx.hash = txHash
+    txMeta.hash = txHash
-    this.updateTx(metaTx)
+    this.updateTx(txMeta)
-    var rawTx = ethUtil.bufferToHex(tx.serialize())
+    this.setTxStatusSigned(txMeta.id)
-    return Promise.resolve(rawTx)
   }
 
   /*
@@ -228,23 +257,35 @@ module.exports = class TransactionManager extends EventEmitter {
     return txMeta.status
   }
 
+  // should update the status of the tx to 'rejected'.
+  setTxStatusRejected (txId) {
+    this._setTxStatus(txId, 'rejected')
+  }
+
+  // should update the status of the tx to 'approved'.
+  setTxStatusApproved (txId) {
+    this._setTxStatus(txId, 'approved')
+  }
 
   // should update the status of the tx to 'signed'.
   setTxStatusSigned (txId) {
     this._setTxStatus(txId, 'signed')
-    this.emit('updateBadge')
   }
 
-  // should update the status of the tx to 'rejected'.
+  // should update the status of the tx to 'submitted'.
-  setTxStatusRejected (txId) {
+  setTxStatusSubmitted (txId) {
-    this._setTxStatus(txId, 'rejected')
+    this._setTxStatus(txId, 'submitted')
-    this.emit('updateBadge')
   }
 
+  // should update the status of the tx to 'confirmed'.
   setTxStatusConfirmed (txId) {
     this._setTxStatus(txId, 'confirmed')
   }
 
+  setTxStatusFailed (txId) {
+    this._setTxStatus(txId, 'failed')
+  }
+
   // merges txParams obj onto txData.txParams
   // use extend to ensure that all fields are filled
   updateTxParams (txId, txParams) {
@@ -267,7 +308,7 @@ module.exports = class TransactionManager extends EventEmitter {
           message: 'We had an error while submitting this transaction, please try again.',
         }
         this.updateTx(txMeta)
-        return this._setTxStatus(txId, 'failed')
+        return this.setTxStatusFailed(txId)
       }
       this.txProviderUtils.query.getTransactionByHash(txHash, (err, txParams) => {
         if (err || !txParams) {
@@ -294,6 +335,7 @@ module.exports = class TransactionManager extends EventEmitter {
   //  should set the status in txData
   //    - `'unapproved'` the user has not responded
   //    - `'rejected'` the user has responded no!
+  //    - `'approved'` the user has approved the tx
   //    - `'signed'` the tx is signed
   //    - `'submitted'` the tx is sent to a server
   //    - `'confirmed'` the tx has been included in a block.
@@ -301,7 +343,11 @@ module.exports = class TransactionManager extends EventEmitter {
     var txMeta = this.getTx(txId)
     txMeta.status = status
     this.emit(`${txMeta.id}:${status}`, txId)
+    if (status === 'submitted' || status === 'rejected') {
+      this.emit(`${txMeta.id}:finished`, status)
+    }
     this.updateTx(txMeta)
+    this.emit('updateBadge')
   }
 
   // Saves the new/updated txList.

package.json

@@ -89,13 +89,14 @@
     "redux-logger": "^2.3.1",
     "redux-thunk": "^1.0.2",
     "sandwich-expando": "^1.0.5",
+    "semaphore": "^1.0.5",
     "textarea-caret": "^3.0.1",
     "three.js": "^0.73.2",
     "through2": "^2.0.1",
     "valid-url": "^1.0.9",
     "vreme": "^3.0.2",
     "web3": "0.17.0-beta",
-    "web3-provider-engine": "^8.2.0",
+    "web3-provider-engine": "^8.4.0",
     "web3-stream-provider": "^2.0.6",
     "xtend": "^4.0.1"
   },

test/unit/metamask-controller-test.js

@@ -25,24 +25,6 @@ describe('MetaMaskController', function() {
     this.sinon.restore()
   })
 
-  describe('#enforceTxValidations', function () {
-    it('returns null for positive values', function() {
-      var sample = {
-        value: '0x01'
-      }
-      var res = controller.enforceTxValidations(sample)
-      assert.equal(res, null, 'no error')
-    })
-
-
-    it('returns error for negative values', function() {
-      var sample = {
-        value: '-0x01'
-      }
-      var res = controller.enforceTxValidations(sample)
-      assert.ok(res, 'error')
-    })
-  })
 })
 
 

test/unit/notice-controller-test.js

@@ -5,13 +5,14 @@ const nock = require('nock')
 const configManagerGen = require('../lib/mock-config-manager')
 const NoticeController = require('../../app/scripts/notice-controller')
 const STORAGE_KEY = 'metamask-persistance-key'
-// Hacking localStorage support into JSDom
-window.localStorage = {}
 
 describe('notice-controller', function() {
   var noticeController
 
   beforeEach(function() {
+    // simple localStorage polyfill
+    window.localStorage = {}
+    if (window.localStorage.clear) window.localStorage.clear()
     let configManager = configManagerGen()
     noticeController = new NoticeController({
       configManager: configManager,

test/unit/tx-manager-test.js

@@ -15,6 +15,28 @@ describe('Transaction Manager', function() {
       provider: "testnet",
       txHistoryLimit: 10,
       blockTracker: new EventEmitter(),
+      getNetwork: function(){ return 'unit test' }
+    })
+  })
+
+  describe('#validateTxParams', function () {
+    it('returns null for positive values', function() {
+      var sample = {
+        value: '0x01'
+      }
+      var res = txManager.txProviderUtils.validateTxParams(sample, (err) => {
+        assert.equal(err, null, 'no error')
+      })
+    })
+
+
+    it('returns error for negative values', function() {
+      var sample = {
+        value: '-0x01'
+      }
+      var res = txManager.txProviderUtils.validateTxParams(sample, (err) => {
+        assert.ok(err, 'error')
+      })
     })
   })
 
@@ -31,7 +53,7 @@ describe('Transaction Manager', function() {
 
   describe('#_saveTxList', function() {
     it('saves the submitted data to the tx list', function() {
-      var target = [{ foo: 'bar' }]
+      var target = [{ foo: 'bar', metamaskNetworkId: 'unit test' }]
       txManager._saveTxList(target)
       var result = txManager.getTxList()
       assert.equal(result[0].foo, 'bar')
@@ -40,7 +62,7 @@ describe('Transaction Manager', function() {
 
   describe('#addTx', function() {
     it('adds a tx returned in getTxList', function() {
-      var tx = { id: 1, status: 'confirmed',}
+      var tx = { id: 1, status: 'confirmed', metamaskNetworkId: 'unit test' }
       txManager.addTx(tx, onTxDoneCb)
       var result = txManager.getTxList()
       assert.ok(Array.isArray(result))
@@ -51,7 +73,7 @@ describe('Transaction Manager', function() {
     it('cuts off early txs beyond a limit', function() {
       const limit = txManager.txHistoryLimit
       for (let i = 0; i < limit + 1; i++) {
-        let tx = { id: i, time: new Date(), status: 'confirmed'}
+        let tx = { id: i, time: new Date(), status: 'confirmed', metamaskNetworkId: 'unit test' }
         txManager.addTx(tx, onTxDoneCb)
       }
       var result = txManager.getTxList()
@@ -59,10 +81,10 @@ describe('Transaction Manager', function() {
       assert.equal(result[0].id, 1, 'early txs truncted')
     })
 
-    it('cuts off early txs beyond a limit weather or not it is confirmed or rejected', function() {
+    it('cuts off early txs beyond a limit whether or not it is confirmed or rejected', function() {
       const limit = txManager.txHistoryLimit
       for (let i = 0; i < limit + 1; i++) {
-        let tx = { id: i, time: new Date(), status: 'rejected'}
+        let tx = { id: i, time: new Date(), status: 'rejected', metamaskNetworkId: 'unit test' }
         txManager.addTx(tx, onTxDoneCb)
       }
       var result = txManager.getTxList()
@@ -71,11 +93,11 @@ describe('Transaction Manager', function() {
     })
 
     it('cuts off early txs beyond a limit but does not cut unapproved txs', function() {
-      var unconfirmedTx = { id: 0, time: new Date(), status: 'unapproved'}
+      var unconfirmedTx = { id: 0, time: new Date(), status: 'unapproved', metamaskNetworkId: 'unit test' }
       txManager.addTx(unconfirmedTx, onTxDoneCb)
       const limit = txManager.txHistoryLimit
       for (let i = 1; i < limit + 1; i++) {
-        let tx = { id: i, time: new Date(), status: 'confirmed'}
+        let tx = { id: i, time: new Date(), status: 'confirmed', metamaskNetworkId: 'unit test' }
         txManager.addTx(tx, onTxDoneCb)
       }
       var result = txManager.getTxList()
@@ -88,7 +110,7 @@ describe('Transaction Manager', function() {
 
   describe('#setTxStatusSigned', function() {
     it('sets the tx status to signed', function() {
-      var tx = { id: 1, status: 'unapproved' }
+      var tx = { id: 1, status: 'unapproved', metamaskNetworkId: 'unit test' }
       txManager.addTx(tx, onTxDoneCb)
       txManager.setTxStatusSigned(1)
       var result = txManager.getTxList()
@@ -99,20 +121,21 @@ describe('Transaction Manager', function() {
 
     it('should emit a signed event to signal the exciton of callback', (done) => {
       this.timeout(10000)
-      var tx = { id: 1, status: 'unapproved' }
+      var tx = { id: 1, status: 'unapproved', metamaskNetworkId: 'unit test' }
-      let onTxDoneCb = function (err, txId) {
+      let onTxDoneCb = function () {
         assert(true, 'event listener has been triggered and onTxDoneCb executed')
         done()
       }
-      txManager.addTx(tx, onTxDoneCb)
+      txManager.addTx(tx)
+      txManager.on('1:signed', onTxDoneCb)
       txManager.setTxStatusSigned(1)
     })
   })
 
   describe('#setTxStatusRejected', function() {
     it('sets the tx status to rejected', function() {
-      var tx = { id: 1, status: 'unapproved' }
+      var tx = { id: 1, status: 'unapproved', metamaskNetworkId: 'unit test' }
-      txManager.addTx(tx, onTxDoneCb)
+      txManager.addTx(tx)
       txManager.setTxStatusRejected(1)
       var result = txManager.getTxList()
       assert.ok(Array.isArray(result))
@@ -122,12 +145,13 @@ describe('Transaction Manager', function() {
 
     it('should emit a rejected event to signal the exciton of callback', (done) => {
       this.timeout(10000)
-      var tx = { id: 1, status: 'unapproved' }
+      var tx = { id: 1, status: 'unapproved', metamaskNetworkId: 'unit test' }
+      txManager.addTx(tx)
       let onTxDoneCb = function (err, txId) {
         assert(true, 'event listener has been triggered and onTxDoneCb executed')
         done()
       }
-      txManager.addTx(tx, onTxDoneCb)
+      txManager.on('1:rejected', onTxDoneCb)
       txManager.setTxStatusRejected(1)
     })
 
@@ -135,9 +159,9 @@ describe('Transaction Manager', function() {
 
   describe('#updateTx', function() {
     it('replaces the tx with the same id', function() {
-      txManager.addTx({ id: '1', status: 'unapproved' }, onTxDoneCb)
+      txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: 'unit test' }, onTxDoneCb)
-      txManager.addTx({ id: '2', status: 'confirmed' }, onTxDoneCb)
+      txManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: 'unit test' }, onTxDoneCb)
-      txManager.updateTx({ id: '1', status: 'blah', hash: 'foo' })
+      txManager.updateTx({ id: '1', status: 'blah', hash: 'foo', metamaskNetworkId: 'unit test' })
       var result = txManager.getTx('1')
       assert.equal(result.hash, 'foo')
     })
@@ -145,8 +169,8 @@ describe('Transaction Manager', function() {
 
   describe('#getUnapprovedTxList', function() {
     it('returns unapproved txs in a hash', function() {
-      txManager.addTx({ id: '1', status: 'unapproved' }, onTxDoneCb)
+      txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: 'unit test' }, onTxDoneCb)
-      txManager.addTx({ id: '2', status: 'confirmed' }, onTxDoneCb)
+      txManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: 'unit test' }, onTxDoneCb)
       let result = txManager.getUnapprovedTxList()
       assert.equal(typeof result, 'object')
       assert.equal(result['1'].status, 'unapproved')
@@ -156,8 +180,8 @@ describe('Transaction Manager', function() {
 
   describe('#getTx', function() {
     it('returns a tx with the requested id', function() {
-      txManager.addTx({ id: '1', status: 'unapproved' }, onTxDoneCb)
+      txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: 'unit test' }, onTxDoneCb)
-      txManager.addTx({ id: '2', status: 'confirmed' }, onTxDoneCb)
+      txManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: 'unit test' }, onTxDoneCb)
       assert.equal(txManager.getTx('1').status, 'unapproved')
       assert.equal(txManager.getTx('2').status, 'confirmed')
     })
@@ -171,6 +195,7 @@ describe('Transaction Manager', function() {
         let everyOther = i % 2
         txManager.addTx({ id: i,
           status: everyOther ? 'unapproved' : 'confirmed',
+          metamaskNetworkId: 'unit test',
           txParams: {
             from: everyOther ? 'foop' : 'zoop',
             to: everyOther ? 'zoop' : 'foop',