Migrate all tx mutation code out of keyring controller and

Fix up txManager to reflect code review requests

app/scripts/background.js

@@ -101,10 +101,10 @@ txManager.on('update', updateBadge)
 
 function updateBadge () {
   var label = ''
-  var unconfTxLen = controller.txManager.unconfTxCount
+  var unapprovedTxCount = controller.txManager.unapprovedTxCount
   var unconfMsgs = messageManager.unconfirmedMsgs()
   var unconfMsgLen = Object.keys(unconfMsgs).length
-  var count = unconfTxLen + unconfMsgLen
+  var count = unapprovedTxCount + unconfMsgLen
   if (count) {
     label = String(count)
   }

app/scripts/keyring-controller.js

@@ -1,6 +1,5 @@
 const ethUtil = require('ethereumjs-util')
 const bip39 = require('bip39')
-const Transaction = require('ethereumjs-tx')
 const EventEmitter = require('events').EventEmitter
 const filter = require('promise-filter')
 const normalize = require('./lib/sig-util').normalize
@@ -321,28 +320,14 @@ module.exports = class KeyringController extends EventEmitter {
   //
   // This method signs tx and returns a promise for
   // TX Manager to update the state after signing
-  signTransaction (txParams, cb) {
+  signTransaction (ethTx, selectedAddress, txId, cb) {
     try {
-      const address = normalize(txParams.from)
+      const address = normalize(selectedAddress)
       return this.getKeyringForAccount(address)
       .then((keyring) => {
-        // Handle gas pricing
+        return keyring.signTransaction(address, ethTx)
-        var gasMultiplier = this.configManager.getGasMultiplier() || 1
-        var gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16)
-        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)
-        const tx = new Transaction(txParams)
-        return keyring.signTransaction(address, tx)
       }).then((tx) => {
-        return {tx, txParams, cb}
+        this.emit(`${txId}:signed`, {tx, txId, cb})
       })
     } catch (e) {
       cb(e)

app/scripts/metamask-controller.js

@@ -39,6 +39,8 @@ module.exports = class MetamaskController {
     })
     this.publicConfigStore = this.initPublicConfigStore()
 
+
+
     var currentFiat = this.configManager.getCurrentFiat() || 'USD'
     this.configManager.setCurrentFiat(currentFiat)
     this.configManager.updateConversionRate()
@@ -152,8 +154,8 @@ module.exports = class MetamaskController {
       // tx signing
       approveTransaction: this.newUnsignedTransaction.bind(this),
       signTransaction: (...args) => {
-        var signedTxPromise = keyringController.signTransaction(...args)
+        this.setupSigningListners(...args)
-        this.txManager.resolveSignedTransaction(signedTxPromise)
+        this.txManager.formatTxForSigining(...args)
         this.sendUpdate()
       },
 
@@ -232,6 +234,14 @@ module.exports = class MetamaskController {
     })
   }
 
+  setupSigningListners (txParams) {
+    var txId = txParams.metamaskId
+    // apply event listeners for signing and formating events
+    this.txManager.once(`${txId}:formated`, this.keyringController.signTransaction.bind(this.keyringController))
+    this.keyringController.once(`${txId}:signed`, this.txManager.resolveSignedTransaction.bind(this.txManager))
+  }
+
+
   enforceTxValidations (txParams) {
     if (('value' in txParams) && txParams.value.indexOf('-') === 0) {
       const msg = `Invalid transaction value of ${txParams.value} not a positive number.`

app/scripts/transaction-manager.js

@@ -1,8 +1,11 @@
 const EventEmitter = require('events')
 const extend = require('xtend')
 const ethUtil = require('ethereumjs-util')
+const Transaction = require('ethereumjs-tx')
+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) {
@@ -75,7 +78,7 @@ module.exports = class TransactionManager extends EventEmitter {
     this._saveTxList(txList)
   }
 
-  get unconfTxCount () {
+  get unapprovedTxCount () {
     return Object.keys(this.getUnapprovedTxList()).length
   }
 
@@ -128,19 +131,39 @@ module.exports = class TransactionManager extends EventEmitter {
     }
   }
 
-  resolveSignedTransaction (txPromise) {
+  // formats txParams so the keyringController can sign it
-    const self = this
+  formatTxForSigining (txParams, cb) {
+    var address = txParams.from
+    var metaTx = this.getTx(txParams.metamaskId)
+    var gasMultiplier = metaTx.gasMultiplier
+    var gasPrice = new BN(ethUtil.stripHexPrefix(txParams.gasPrice), 16)
+    gasPrice = gasPrice.mul(new BN(gasMultiplier * 100, 10)).div(new BN(100, 10))
+    txParams.gasPrice = ethUtil.intToHex(gasPrice.toNumber())
 
-    txPromise.then(({tx, txParams, cb}) => {
+    // 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)
+    const ethTx = new Transaction(txParams)
+    // this.updateTxParams(txParams.metamaskId, ethTx)
+
+    // listener is assigned in metamaskController
+    this.emit(`${txParams.metamaskId}:formated`, ethTx, address, txParams.metamaskId, cb)
+  }
+
+  // receives a signed tx object and updates the tx hash
+  // and pass it to the cb to be sent off
+  resolveSignedTransaction ({tx, txId, cb}) {
     // Add the tx hash to the persisted meta-tx object
     var txHash = ethUtil.bufferToHex(tx.hash())
-
+    var metaTx = this.getTx(txId)
-      var metaTx = self.getTx(txParams.metamaskId)
     metaTx.hash = txHash
-      // return raw serialized tx
+    this.updateTx(metaTx)
     var rawTx = ethUtil.bufferToHex(tx.serialize())
     cb(null, rawTx)
-    })
   }
 
   /*
@@ -176,10 +199,13 @@ module.exports = class TransactionManager extends EventEmitter {
     })
   }
 
+  // STATUS METHODS
+  // get::set status
+
   // should return the status of the tx.
-  getTxStatus (txId, cb) {
+  getTxStatus (txId) {
     const txMeta = this.getTx(txId)
-    return cb ? cb(txMeta.staus) : txMeta.status
+    return txMeta.status
   }
 
 
@@ -203,7 +229,7 @@ module.exports = class TransactionManager extends EventEmitter {
   // use extend to ensure that all fields are filled
   updateTxParams (txId, txParams) {
     var txMeta = this.getTx(txId)
-    txMeta.txParams = extend(txMeta, txParams)
+    txMeta.txParams = extend(txMeta.txParams, txParams)
     this.updateTx(txMeta)
   }
 
@@ -218,7 +244,7 @@ module.exports = class TransactionManager extends EventEmitter {
       if (!txHash) return
       this.txProviderUtils.query.getTransactionByHash(txHash, (err, txMeta) => {
         if (err || !txMeta) {
-          tx.err = err || 'Tx could possibly have not submitted'
+          tx.err = err || 'Tx could possibly have not been submitted'
           this.updateTx(tx)
           return txMeta ? console.error(err) : console.debug(`txMeta is ${txMeta} for:`, tx)
         }
@@ -229,16 +255,7 @@ module.exports = class TransactionManager extends EventEmitter {
     })
   }
 
-  // Private functions
+  // PRIVATE METHODS
-
-  // Saves the new/updated txList.
-  // Function is intended only for internal use
-  _saveTxList (txList) {
-    this.txList = txList
-    this._setTxList(txList)
-  }
-
-    //   should return the tx
 
   //  Should find the tx in the tx list and
   //  update it.
@@ -255,7 +272,12 @@ module.exports = class TransactionManager extends EventEmitter {
     this.updateTx(txMeta)
   }
 
-
+  // Saves the new/updated txList.
+  // Function is intended only for internal use
+  _saveTxList (txList) {
+    this.txList = txList
+    this._setTxList(txList)
+  }
 }