make addUnapprovedTransaction async function and use promise based ethQuery

6 files changed, 219 insertions, 241 deletions

diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js
index 5f2d75b47..d3e852ef9 100644
--- a/app/scripts/controllers/transactions.js
+++ b/app/scripts/controllers/transactions.js
@@ -1,10 +1,11 @@
 const EventEmitter = require('events')
 const async = require('async')
 const extend = require('xtend')
+const pify = require('pify')
 const clone = require('clone')
 const ObservableStore = require('obs-store')
 const ethUtil = require('ethereumjs-util')
-const pify = require('pify')
+const EthQuery = require('ethjs-query');
 const TxProviderUtil = require('../lib/tx-utils')
 const getStack = require('../lib/util').getStack
 const createId = require('../lib/random-id')
@@ -33,7 +34,7 @@ module.exports = class TransactionController extends EventEmitter {
         })
       },
     })
-    this.query = opts.ethQuery
+    this.query = new EthQuery(this.provider)
     this.txProviderUtils = new TxProviderUtil(this.query)
     this.blockTracker.on('rawBlock', this.checkForTxInBlock.bind(this))
     // this is a little messy but until ethstore has been either
@@ -62,13 +63,6 @@ module.exports = class TransactionController extends EventEmitter {
     return this.preferencesStore.getState().selectedAddress
   }
 
-  // Returns the tx list
-  getTxList () {
-    const network = this.getNetwork()
-    const fullTxList = this.getFullTxList()
-    return fullTxList.filter(txMeta => txMeta.metamaskNetworkId === network)
-  }
-
   // Returns the number of txs for the current network.
   getTxCount () {
     return this.getTxList().length
@@ -79,6 +73,50 @@ module.exports = class TransactionController extends EventEmitter {
     return this.store.getState().transactions
   }
 
+  get unapprovedTxCount () {
+    return Object.keys(this.getUnapprovedTxList()).length
+  }
+
+  get pendingTxCount () {
+    return this.getTxsByMetaData('status', 'signed').length
+  }
+
+  // Returns the tx list
+  getTxList () {
+    const network = this.getNetwork()
+    const fullTxList = this.getFullTxList()
+    return this.getTxsByMetaData('metamaskNetworkId', network, fullTxList)
+  }
+
+  // gets tx by Id and returns it
+  getTx (txId) {
+    const txList = this.getTxList()
+    const txMeta = txList.find(txData => txData.id === txId)
+    return txMeta
+  }
+  getUnapprovedTxList () {
+    let txList = this.getTxList()
+    return txList.filter((txMeta) => txMeta.status === 'unapproved')
+    .reduce((result, tx) => {
+      result[tx.id] = tx
+      return result
+    }, {})
+  }
+
+  updateTx (txMeta) {
+    const txMetaForHistory = clone(txMeta)
+    txMetaForHistory.stack = getStack()
+    const txId = txMeta.id
+    const txList = this.getFullTxList()
+    const index = txList.findIndex(txData => txData.id === txId)
+    if (!txMeta.history) txMeta.history = []
+    txMeta.history.push(txMetaForHistory)
+
+    txList[index] = txMeta
+    this._saveTxList(txList)
+    this.emit('update')
+  }
+
   // Adds a tx to the txlist
   addTx (txMeta) {
     const txCount = this.getTxCount()
@@ -92,7 +130,7 @@ module.exports = class TransactionController extends EventEmitter {
     // or rejected tx's.
     // not tx's that are pending or unapproved
     if (txCount > txHistoryLimit - 1) {
-      var index = fullTxList.findIndex((metaTx) => ((metaTx.status === 'confirmed' || metaTx.status === 'rejected') && network === txMeta.metamaskNetworkId))
+      let index = fullTxList.findIndex((metaTx) => ((metaTx.status === 'confirmed' || metaTx.status === 'rejected') && network === txMeta.metamaskNetworkId))
       fullTxList.splice(index, 1)
     }
     fullTxList.push(txMeta)
@@ -110,86 +148,40 @@ module.exports = class TransactionController extends EventEmitter {
     this.emit(`${txMeta.id}:unapproved`, txMeta)
   }
 
-  // gets tx by Id and returns it
-  getTx (txId, cb) {
-    var txList = this.getTxList()
-    var txMeta = txList.find(txData => txData.id === txId)
-    return cb ? cb(txMeta) : txMeta
-  }
-
-  //
-  updateTx (txMeta) {
-    const txMetaForHistory = clone(txMeta)
-    txMetaForHistory.stack = getStack()
-    var txId = txMeta.id
-    var txList = this.getFullTxList()
-    var index = txList.findIndex(txData => txData.id === txId)
-    if (!txMeta.history) txMeta.history = []
-    txMeta.history.push(txMetaForHistory)
-
-    txList[index] = txMeta
-    this._saveTxList(txList)
-    this.emit('update')
-  }
-
-  get unapprovedTxCount () {
-    return Object.keys(this.getUnapprovedTxList()).length
-  }
-
-  get pendingTxCount () {
-    return this.getTxsByMetaData('status', 'signed').length
-  }
-
-  addUnapprovedTransaction (txParams, done) {
-    let txMeta = {}
-    async.waterfall([
-      // validate
-      (cb) => this.txProviderUtils.validateTxParams(txParams, cb),
-      // construct txMeta
-      (cb) => {
-        txMeta = {
-          id: createId(),
-          time: (new Date()).getTime(),
-          status: 'unapproved',
-          metamaskNetworkId: this.getNetwork(),
-          txParams: txParams,
-          history: [],
-        }
-        cb()
-      },
-      // add default tx params
-      (cb) => this.addTxDefaults(txMeta, cb),
-      // save txMeta
-      (cb) => {
-        this.addTx(txMeta)
-        cb(null, txMeta)
-      },
-    ], done)
+  async addUnapprovedTransaction (txParams) {
+    // validate
+    await this.txProviderUtils.validateTxParams(txParams)
+    // construct txMeta
+    const txMeta = {
+      id: createId(),
+      time: (new Date()).getTime(),
+      status: 'unapproved',
+      metamaskNetworkId: this.getNetwork(),
+      txParams: txParams,
+      history: [],
+    }
+    // add default tx params
+    await this.addTxDefaults(txMeta),
+    // save txMeta
+    this.addTx(txMeta)
+    return txMeta
   }
 
-  addTxDefaults (txMeta, cb) {
+  async addTxDefaults (txMeta) {
     const txParams = txMeta.txParams
     // ensure value
     txParams.value = txParams.value || '0x0'
     if (!txParams.gasPrice) {
-      this.query.gasPrice((err, gasPrice) => {
-
-        if (err) return cb(err)
-        // set gasPrice
-        txParams.gasPrice = gasPrice
-      })
+      gassPrice = await this.query.gasPrice()
+      txParams.gasPrice = gasPrice
     }
     // set gasLimit
-    this.txProviderUtils.analyzeGasUsage(txMeta, cb)
+    return await this.txProviderUtils.analyzeGasUsage(txMeta)
   }
 
-  getUnapprovedTxList () {
-    var txList = this.getTxList()
-    return txList.filter((txMeta) => txMeta.status === 'unapproved')
-    .reduce((result, tx) => {
-      result[tx.id] = tx
-      return result
-    }, {})
+  async updateAndApproveTransaction (txMeta) {
+    this.updateTx(txMeta)
+    await this.approveTransaction(txMeta.id)
   }
 
   async approveTransaction (txId) {
@@ -221,26 +213,6 @@ module.exports = class TransactionController extends EventEmitter {
     }
   }
 
-  cancelTransaction (txId, cb = warn) {
-    this.setTxStatusRejected(txId)
-    cb()
-  }
-
-  async updateAndApproveTransaction (txMeta) {
-    this.updateTx(txMeta)
-    await this.approveTransaction(txMeta.id)
-  }
-
-  getChainId () {
-    const networkState = this.networkStore.getState()
-    const getChainId = parseInt(networkState)
-    if (Number.isNaN(getChainId)) {
-      return 0
-    } else {
-      return getChainId
-    }
-  }
-
   async signTransaction (txId) {
     const txMeta = this.getTx(txId)
     const txParams = txMeta.txParams
@@ -265,6 +237,22 @@ module.exports = class TransactionController extends EventEmitter {
     })
   }
 
+  cancelTransaction (txId) {
+    this.setTxStatusRejected(txId)
+    return Promise.resolve()
+  }
+
+
+  getChainId () {
+    const networkState = this.networkStore.getState()
+    const getChainId = parseInt(networkState)
+    if (Number.isNaN(getChainId)) {
+      return 0
+    } else {
+      return getChainId
+    }
+  }
+
   // receives a txHash records the tx as signed
   setTxHash (txId, txHash) {
     // Add the tx hash to the persisted meta-tx object
@@ -275,7 +263,7 @@ module.exports = class TransactionController extends EventEmitter {
 
   /*
   Takes an object of fields to search for eg:
-  var thingsToLookFor = {
+  let thingsToLookFor = {
     to: '0x0..',
     from: '0x0..',
     status: 'signed',
@@ -298,7 +286,7 @@ module.exports = class TransactionController extends EventEmitter {
   and that have been 'confirmed'
   */
   getFilteredTxList (opts) {
-    var filteredTxList
+    let filteredTxList
     Object.keys(opts).forEach((key) => {
       filteredTxList = this.getTxsByMetaData(key, opts[key], filteredTxList)
     })
@@ -359,7 +347,7 @@ module.exports = class TransactionController extends EventEmitter {
   // merges txParams obj onto txData.txParams
   // use extend to ensure that all fields are filled
   updateTxParams (txId, txParams) {
-    var txMeta = this.getTx(txId)
+    const txMeta = this.getTx(txId)
     txMeta.txParams = extend(txMeta.txParams, txParams)
     this.updateTx(txMeta)
   }
@@ -367,20 +355,19 @@ module.exports = class TransactionController extends EventEmitter {
   //  checks if a signed tx is in a block and
   // if included sets the tx status as 'confirmed'
   checkForTxInBlock (block) {
-    var signedTxList = this.getFilteredTxList({status: 'submitted'})
+    const signedTxList = this.getFilteredTxList({status: 'submitted'})
     if (!signedTxList.length) return
     signedTxList.forEach((txMeta) => {
-      var txHash = txMeta.hash
-      var txId = txMeta.id
+      const txHash = txMeta.hash
+      const txId = txMeta.id
 
       if (!txHash) {
-        return this.setTxStatusFailed(txId, {
-          stack: 'checkForTxInBlock: custom tx-controller error message',
-          errCode: 'No hash was provided',
-          message: 'We had an error while submitting this transaction, please try again.',
-        })
+        const noTxHash = new Error('We had an error while submitting this transaction, please try again.')
+        noTxHash.name = 'NoTxHashError'
+        this.setTxStatusFailed(noTxHash)
       }
 
+
       block.transactions.forEach((tx) => {
         if (tx.hash === txHash) this.setTxStatusConfirmed(txId)
       })
@@ -398,6 +385,39 @@ module.exports = class TransactionController extends EventEmitter {
     if (diff > 1) this._checkPendingTxs()
   }
 
+  resubmitPendingTxs () {
+    const pending = this.getTxsByMetaData('status', 'submitted')
+    // only try resubmitting if their are transactions to resubmit
+    if (!pending.length) return
+    pending.forEach((txMeta) => this._resubmitTx(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 = err.message.toLowerCase()
+      const isKnownTx = (
+        // geth
+        errorMessage.includes('replacement transaction underpriced')
+        || errorMessage.includes('known transaction')
+        // parity
+        || errorMessage.includes('gas price too low to replace')
+        || errorMessage.includes('transaction with the same hash was already imported')
+        // other
+        || errorMessage.includes('gateway timeout')
+        || errorMessage.includes('nonce too low')
+      )
+      // ignore resubmit warnings, return early
+      if (isKnownTx) return
+      // encountered real error - transition to error state
+      this.setTxStatusFailed(txMeta.id, {
+        stack: err.stack || err.message,
+        errCode: err.errCode || err,
+        message: err.message,
+      })
+    }))
+  }
+
   // PRIVATE METHODS
 
   //  Should find the tx in the tx list and
@@ -411,7 +431,7 @@ module.exports = class TransactionController extends EventEmitter {
   //    - `'confirmed'` the tx has been included in a block.
   //    - `'failed'` the tx failed for some reason, included on tx data.
   _setTxStatus (txId, status) {
-    var txMeta = this.getTx(txId)
+    const txMeta = this.getTx(txId)
     txMeta.status = status
     this.emit(`${txMeta.id}:${status}`, txId)
     if (status === 'submitted' || status === 'rejected') {
@@ -436,39 +456,6 @@ module.exports = class TransactionController extends EventEmitter {
     this.memStore.updateState({ unapprovedTxs, selectedAddressTxList })
   }
 
-  resubmitPendingTxs () {
-    const pending = this.getTxsByMetaData('status', 'submitted')
-    // only try resubmitting if their are transactions to resubmit
-    if (!pending.length) return
-    pending.forEach((txMeta) => this._resubmitTx(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 = err.message.toLowerCase()
-      const isKnownTx = (
-        // geth
-        errorMessage.includes('replacement transaction underpriced')
-        || errorMessage.includes('known transaction')
-        // parity
-        || errorMessage.includes('gas price too low to replace')
-        || errorMessage.includes('transaction with the same hash was already imported')
-        // other
-        || errorMessage.includes('gateway timeout')
-        || errorMessage.includes('nonce too low')
-      )
-      // ignore resubmit warnings, return early
-      if (isKnownTx) return
-      // encountered real error - transition to error state
-      this.setTxStatusFailed(txMeta.id, {
-        stack: err.stack || err.message,
-        errCode: err.errCode || err,
-        message: err.message,
-      })
-    }))
-  }
-
   async _resubmitTx (txMeta) {
     const address = txMeta.txParams.from
     const balance = this.ethStore.getState().accounts[address].balance
@@ -515,17 +502,14 @@ module.exports = class TransactionController extends EventEmitter {
     // extra check in case there was an uncaught error during the
     // signature and submission process
     if (!txHash) {
-      this.setTxStatusFailed(txId, {
-        stack: '_checkPendingTxs: custom tx-controller error message',
-        errCode: 'No hash was provided',
-        message: 'We had an error while submitting this transaction, please try again.',
-      })
-      return
+      const noTxHash = new Error('We had an error while submitting this transaction, please try again.')
+      noTxHash.name = 'NoTxHashError'
+      this.setTxStatusFailed(noTxHash)
     }
     // get latest transaction status
     let txParams
     try {
-      txParams = await pify((cb) => this.query.getTransactionByHash(txHash, cb))()
+      txParams = await this.query.getTransactionByHash(txHash)
       if (!txParams) return
       if (txParams.blockNumber) {
         this.setTxStatusConfirmed(txId)
@@ -538,12 +522,8 @@ module.exports = class TransactionController extends EventEmitter {
           message: 'There was a problem loading this transaction.',
         }
         this.updateTx(txMeta)
-        log.error(err)
+        throw err
       }
     }
   }
-
-}
-
-
-const warn = () => log.warn('warn was used no cb provided')
+}
\ No newline at end of file
diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js
index 8f6943937..43928feaf 100644
--- a/app/scripts/lib/tx-utils.js
+++ b/app/scripts/lib/tx-utils.js
@@ -10,24 +10,18 @@ its passed ethquery
 and used to do things like calculate gas of a tx.
 */
 
-module.exports = class txProviderUtils {
-
+module.exports = class txProvideUtils {
   constructor (ethQuery) {
     this.query = ethQuery
   }
 
-  analyzeGasUsage (txMeta, cb) {
-    var self = this
-    this.query.getBlockByNumber('latest', true, (err, block) => {
-      if (err) return cb(err)
-      async.waterfall([
-        self.estimateTxGas.bind(self, txMeta, block.gasLimit),
-        self.setTxGas.bind(self, txMeta, block.gasLimit),
-      ], cb)
-    })
+  async analyzeGasUsage (txMeta) {
+    const block = await this.query.getBlockByNumber('latest', true)
+    const estimatedGasHex = await this.estimateTxGas(txMeta, block.gasLimit)
+    this.setTxGas(txMeta, block.gasLimit, estimatedGasHex)
   }
 
-  estimateTxGas (txMeta, blockGasLimitHex, cb) {
+  async estimateTxGas (txMeta, blockGasLimitHex) {
     const txParams = txMeta.txParams
     // check if gasLimit is already specified
     txMeta.gasLimitSpecified = Boolean(txParams.gas)
@@ -38,10 +32,10 @@ module.exports = class txProviderUtils {
       txParams.gas = bnToHex(saferGasLimitBN)
     }
     // run tx, see if it will OOG
-    this.query.estimateGas(txParams, cb)
+    return await this.query.estimateGas(txParams)
   }
 
-  setTxGas (txMeta, blockGasLimitHex, estimatedGasHex, cb) {
+  setTxGas (txMeta, blockGasLimitHex, estimatedGasHex) {
     txMeta.estimatedGas = estimatedGasHex
     const txParams = txMeta.txParams
 
@@ -49,14 +43,12 @@ module.exports = class txProviderUtils {
     // use original specified amount
     if (txMeta.gasLimitSpecified) {
       txMeta.estimatedGas = txParams.gas
-      cb()
       return
     }
     // if gasLimit not originally specified,
     // try adding an additional gas buffer to our estimation for safety
     const recommendedGasHex = this.addGasBuffer(txMeta.estimatedGas, blockGasLimitHex)
     txParams.gas = recommendedGasHex
-    cb()
     return
   }
 
@@ -74,22 +66,6 @@ module.exports = class txProviderUtils {
     return bnToHex(upperGasLimitBn)
   }
 
-  fillInTxParams (txParams, cb) {
-    const fromAddress = txParams.from
-    const 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) {
     // normalize values
@@ -107,20 +83,17 @@ module.exports = class txProviderUtils {
   }
 
   publishTransaction (rawTx) {
-    return new Promise((resolve, reject) => {
-      this.query.sendRawTransaction(rawTx, (err, ress) => {
-        if (err) reject(err)
-        else resolve(ress)
-      })
-    })
+    return this.query.sendRawTransaction(rawTx)
   }
 
-  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()
-    }
+  validateTxParams (txParams) {
+    return new Promise ((resolve, reject) => {
+      if (('value' in txParams) && txParams.value.indexOf('-') === 0) {
+        reject(new Error(`Invalid transaction value of ${txParams.value} not a positive number.`))
+      } else {
+        resolve()
+      }
+    })
   }
 
   sufficientBalance (txParams, hexBalance) {
diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index 11dcde2c1..f7c92e618 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -195,7 +195,7 @@ module.exports = class MetamaskController extends EventEmitter {
         cb(null, result)
       },
       // tx signing
-      processTransaction: (txParams, cb) => this.newUnapprovedTransaction(txParams, cb),
+      processTransaction: nodeify(this.newUnapprovedTransaction, this),
       // old style msg signing
       processMessage: this.newUnsignedMessage.bind(this),
 
@@ -308,7 +308,7 @@ module.exports = class MetamaskController extends EventEmitter {
       exportAccount: nodeify(keyringController.exportAccount, keyringController),
 
       // txController
-      cancelTransaction: txController.cancelTransaction.bind(txController),
+      cancelTransaction: nodeify(txController.cancelTransaction, txController),
       updateAndApproveTransaction: nodeify(txController.updateAndApproveTransaction, txController),
 
       // messageManager
@@ -440,22 +440,21 @@ module.exports = class MetamaskController extends EventEmitter {
   // Identity Management
   //
 
-  newUnapprovedTransaction (txParams, cb) {
+  async newUnapprovedTransaction (txParams) {
     log.debug(`MetaMaskController newUnapprovedTransaction ${JSON.stringify(txParams)}`)
-    const self = this
-    self.txController.addUnapprovedTransaction(txParams, (err, txMeta) => {
-      if (err) return cb(err)
-      self.sendUpdate()
-      self.opts.showUnapprovedTx(txMeta)
-      // listen for tx completion (success, fail)
-      self.txController.once(`${txMeta.id}:finished`, (completedTx) => {
+    const txMeta = await this.txController.addUnapprovedTransaction(txParams)
+    this.sendUpdate()
+    this.opts.showUnapprovedTx(txMeta)
+    // listen for tx completion (success, fail)
+    return new Promise ((resolve, reject) => {
+      this.txController.once(`${txMeta.id}:finished`, (completedTx) => {
         switch (completedTx.status) {
           case 'submitted':
-            return cb(null, completedTx.hash)
+            return reoslve(completedTx.hash)
           case 'rejected':
-            return cb(new Error('MetaMask Tx Signature: User denied transaction signature.'))
+            return reject(new Error('MetaMask Tx Signature: User denied transaction signature.'))
           default:
-            return cb(new Error(`MetaMask Tx Signature: Unknown problem: ${JSON.stringify(completedTx.txParams)}`))
+            return reject(new Error(`MetaMask Tx Signature: Unknown problem: ${JSON.stringify(completedTx.txParams)}`))
         }
       })
     })
@@ -646,6 +645,4 @@ module.exports = class MetamaskController extends EventEmitter {
       return Promise.resolve(rpcTarget)
     })
   }
-
-
-}
+}
\ No newline at end of file
diff --git a/package.json b/package.json
index dcd25cda6..94232d46d 100644
--- a/package.json
+++ b/package.json
@@ -77,6 +77,7 @@
     "ethereumjs-util": "ethereumjs/ethereumjs-util#ac5d0908536b447083ea422b435da27f26615de9",
     "ethereumjs-wallet": "^0.6.0",
     "ethjs-ens": "^2.0.0",
+    "ethjs-query": "^0.2.6",
     "express": "^4.14.0",
     "extension-link-enabler": "^1.0.0",
     "extensionizer": "^1.0.0",
diff --git a/test/unit/tx-controller-test.js b/test/unit/tx-controller-test.js
index 31908569a..6e42e8e68 100644
--- a/test/unit/tx-controller-test.js
+++ b/test/unit/tx-controller-test.js
@@ -1,7 +1,6 @@
 const assert = require('assert')
 const ethUtil = require('ethereumjs-util')
 const EthTx = require('ethereumjs-tx')
-const EthQuery = require('eth-query')
 const ObservableStore = require('obs-store')
 const clone = require('clone')
 const sinon = require('sinon')
@@ -11,7 +10,7 @@ const currentNetworkId = 42
 const otherNetworkId = 36
 const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex')
 
-describe('Transaction Controller', function () {
+describe.only('Transaction Controller', function () {
   let txController
 
   beforeEach(function () {
@@ -20,7 +19,6 @@ describe('Transaction Controller', function () {
       txHistoryLimit: 10,
       blockTracker: { getCurrentBlock: noop, on: noop, once: noop },
       provider: { sendAsync: noop },
-      ethQuery: new EthQuery({ sendAsync: noop }),
       ethStore: { getState: noop },
       signTransaction: (ethTx) => new Promise((resolve) => {
         ethTx.sign(privKey)
@@ -28,24 +26,59 @@ describe('Transaction Controller', function () {
       }),
     })
     txController.nonceTracker.getNonceLock = () => Promise.resolve({ nextNonce: 0, releaseLock: noop })
+    const queryStubResult = {}
+    txController.query = new Proxy({}, {
+      get: (_, key) => {
+        if (key === 'stubResult') {
+          return function (method, ...args) {
+            queryStubResult[method] = args
+          }
+        } else {
+          returnValue = queryStubResult[key]
+          return () => Promise.resolve(...returnValue)
+        }
+      },
+    })
+  })
+
+  describe('#addUnapprovedTransaction', function () {
+    it('should add an unapproved transaction and return a valid txMeta', function (done) {
+      const addTxDefaultsStub = sinon.stub(txController, 'addTxDefaults').callsFake(() => Promise.resolve)
+      txController.addUnapprovedTransaction({})
+      .then((txMeta) => {
+        assert(('id' in txMeta), 'should have a id')
+        assert(('time' in txMeta), 'should have a time stamp')
+        assert(('metamaskNetworkId' in txMeta), 'should have a metamaskNetworkId')
+        assert(('txParams' in txMeta), 'should have a txParams')
+        assert(('history' in txMeta), 'should have a history')
+
+        const memTxMeta = txController.getTx(txMeta.id)
+        assert.deepEqual(txMeta, memTxMeta, `txMeta should be stored in txController after adding it\n  expected: ${txMeta} \n  got: ${memTxMeta}`)
+        addTxDefaultsStub.restore()
+        done()
+      }).catch(done)
+    })
   })
 
   describe('#validateTxParams', function () {
-    it('returns null for positive values', function () {
+    it('does not throw for positive values', function (done) {
       var sample = {
         value: '0x01',
       }
-      txController.txProviderUtils.validateTxParams(sample, (err) => {
-        assert.equal(err, null, 'no error')
+      txController.txProviderUtils.validateTxParams(sample).then(() => {
+        done()
       })
     })
 
-    it('returns error for negative values', function () {
+    it('returns error for negative values', function (done) {
       var sample = {
         value: '-0x01',
       }
-      txController.txProviderUtils.validateTxParams(sample, (err) => {
+      txController.txProviderUtils.validateTxParams(sample)
+      .then(() => done('expected to thrown on negativity values but didn\'t'))
+      .catch((err) => {
         assert.ok(err, 'error')
+        done()
       })
     })
   })
@@ -56,9 +89,6 @@ describe('Transaction Controller', function () {
       assert.ok(Array.isArray(result))
       assert.equal(result.length, 0)
     })
-    it('should also return transactions from local storage if any', function () {
-
-    })
   })
 
   describe('#addTx', function () {
@@ -276,16 +306,15 @@ describe('Transaction Controller', function () {
 
       txController.addTx(txMeta)
 
-      const estimateStub = sinon.stub(txController.txProviderUtils.query, 'estimateGas')
-      .callsArgWithAsync(1, null, wrongValue)
-
-      const priceStub = sinon.stub(txController.txProviderUtils.query, 'gasPrice')
-      .callsArgWithAsync(0, null, wrongValue)
+      txController.query.stubResult('estimateGas', wrongValue)
+      txController.query.stubResult('gasPrice', wrongValue)
 
+      const signStub = sinon.stub(txController, 'signTransaction').callsFake(() => Promise.resolve())
 
-      const signStub = sinon.stub(txController, 'signTransaction', () => Promise.resolve())
-
-      const pubStub = sinon.stub(txController.txProviderUtils, 'publishTransaction', () => Promise.resolve(originalValue))
+      const pubStub = sinon.stub(txController, 'publishTransaction').callsFake(() => {
+        txController.setTxHash('1', originalValue)
+        txController.setTxStatusSubmitted('1')
+      })
 
       txController.approveTransaction(txMeta.id).then(() => {
         const result = txController.getTx(txMeta.id)
@@ -294,9 +323,6 @@ describe('Transaction Controller', function () {
         assert.equal(params.gas, originalValue, 'gas unmodified')
         assert.equal(params.gasPrice, originalValue, 'gas price unmodified')
         assert.equal(result.hash, originalValue, `hash was set \n got: ${result.hash} \n expected: ${originalValue}`)
-
-        estimateStub.restore()
-        priceStub.restore()
         signStub.restore()
         pubStub.restore()
         done()
@@ -352,7 +378,7 @@ describe('Transaction Controller', function () {
       })
       .catch((err) => {
         assert.ifError(err, 'should not throw an error')
-        done()
+        done(err)
       })
     })
   })
diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js
index a43bcfb35..43807922a 100644
--- a/test/unit/tx-utils-test.js
+++ b/test/unit/tx-utils-test.js
@@ -1,4 +1,5 @@
 const assert = require('assert')
+const sinon = require('sinon')
 const ethUtil = require('ethereumjs-util')
 const BN = ethUtil.BN