Merge pull request #1861 from MetaMask/transactionControllerRefractor

Transaction controller refractor part 2

4 files changed, 241 insertions, 186 deletions

diff --git a/app/scripts/controllers/transactions.js b/app/scripts/controllers/transactions.js
index 308d43cb0..664dbff6b 100644
--- a/app/scripts/controllers/transactions.js
+++ b/app/scripts/controllers/transactions.js
@@ -5,6 +5,7 @@ const ObservableStore = require('obs-store')
 const ethUtil = require('ethereumjs-util')
 const EthQuery = require('ethjs-query')
 const TxProviderUtil = require('../lib/tx-utils')
+const PendingTransactionTracker = require('../lib/pending-tx-tracker')
 const createId = require('../lib/random-id')
 const NonceTracker = require('../lib/nonce-tracker')
 
@@ -20,6 +21,9 @@ module.exports = class TransactionController extends EventEmitter {
     this.txHistoryLimit = opts.txHistoryLimit
     this.provider = opts.provider
     this.blockTracker = opts.blockTracker
+    this.signEthTx = opts.signTransaction
+    this.ethStore = opts.ethStore
+
     this.nonceTracker = new NonceTracker({
       provider: this.provider,
       getPendingTransactions: (address) => {
@@ -31,15 +35,37 @@ module.exports = class TransactionController extends EventEmitter {
       },
     })
     this.query = new EthQuery(this.provider)
-    this.txProviderUtils = new TxProviderUtil(this.query)
-    this.blockTracker.on('rawBlock', this.checkForTxInBlock.bind(this))
+    this.txProviderUtil = new TxProviderUtil(this.provider)
+
+    this.pendingTxTracker = new PendingTransactionTracker({
+      provider: this.provider,
+      nonceTracker: this.nonceTracker,
+      getBalance: async (address) => {
+        const account = this.ethStore.getState().accounts[address]
+        if (!account) return
+        return account.balance
+      },
+      publishTransaction: this.txProviderUtil.publishTransaction.bind(this.txProviderUtil),
+      getPendingTransactions: (address) => {
+        return this.getFilteredTxList({
+          from: address,
+          status: 'submitted',
+        })
+      },
+    })
+
+    this.pendingTxTracker.on('txWarning', this.updateTx.bind(this))
+    this.pendingTxTracker.on('txFailed', this.setTxStatusFailed.bind(this))
+    this.pendingTxTracker.on('txConfirmed', this.setTxStatusConfirmed.bind(this))
+
+    this.blockTracker.on('rawBlock', this.pendingTxTracker.checkForTxInBlock.bind(this.pendingTxTracker))
     // this is a little messy but until ethstore has been either
     // removed or redone this is to guard against the race condition
     // where ethStore hasent been populated by the results yet
-    this.blockTracker.once('latest', () => this.blockTracker.on('latest', this.resubmitPendingTxs.bind(this)))
-    this.blockTracker.on('sync', this.queryPendingTxs.bind(this))
-    this.signEthTx = opts.signTransaction
-    this.ethStore = opts.ethStore
+    this.blockTracker.once('latest', () => {
+      this.blockTracker.on('latest', this.pendingTxTracker.resubmitPendingTxs.bind(this.pendingTxTracker))
+    })
+    this.blockTracker.on('sync', this.pendingTxTracker.queryPendingTxs.bind(this.pendingTxTracker))
     // memstore is computed from a few different stores
     this._updateMemstore()
     this.store.subscribe(() => this._updateMemstore())
@@ -171,7 +197,7 @@ module.exports = class TransactionController extends EventEmitter {
 
   async addUnapprovedTransaction (txParams) {
     // validate
-    await this.txProviderUtils.validateTxParams(txParams)
+    await this.txProviderUtil.validateTxParams(txParams)
     // construct txMeta
     const txMeta = {
       id: createId(),
@@ -197,7 +223,7 @@ module.exports = class TransactionController extends EventEmitter {
       txParams.gasPrice = gasPrice
     }
     // set gasLimit
-    return await this.txProviderUtils.analyzeGasUsage(txMeta)
+    return await this.txProviderUtil.analyzeGasUsage(txMeta)
   }
 
   async updateAndApproveTransaction (txMeta) {
@@ -226,11 +252,7 @@ module.exports = class TransactionController extends EventEmitter {
       // must set transaction to submitted/failed before releasing lock
       nonceLock.releaseLock()
     } catch (err) {
-      this.setTxStatusFailed(txId, {
-        stack: err.stack || err.message,
-        errCode: err.errCode || err,
-        message: err.message || 'Transaction failed during approval',
-      })
+      this.setTxStatusFailed(txId, err)
       // must set transaction to submitted/failed before releasing lock
       if (nonceLock) nonceLock.releaseLock()
       // continue with error chain
@@ -244,7 +266,7 @@ module.exports = class TransactionController extends EventEmitter {
     const fromAddress = txParams.from
     // add network/chain id
     txParams.chainId = this.getChainId()
-    const ethTx = this.txProviderUtils.buildEthTxFromParams(txParams)
+    const ethTx = this.txProviderUtil.buildEthTxFromParams(txParams)
     await this.signEthTx(ethTx, fromAddress)
     this.setTxStatusSigned(txMeta.id)
     const rawTx = ethUtil.bufferToHex(ethTx.serialize())
@@ -255,7 +277,7 @@ module.exports = class TransactionController extends EventEmitter {
     const txMeta = this.getTx(txId)
     txMeta.rawTx = rawTx
     this.updateTx(txMeta)
-    const txHash = await this.txProviderUtils.publishTransaction(rawTx)
+    const txHash = await this.txProviderUtil.publishTransaction(rawTx)
     this.setTxHash(txId, txHash)
     this.setTxStatusSubmitted(txId)
   }
@@ -374,73 +396,6 @@ module.exports = class TransactionController extends EventEmitter {
     this.updateTx(txMeta)
   }
 
-  //  checks if a signed tx is in a block and
-  // if included sets the tx status as 'confirmed'
-  checkForTxInBlock (block) {
-    const signedTxList = this.getFilteredTxList({status: 'submitted'})
-    if (!signedTxList.length) return
-    signedTxList.forEach((txMeta) => {
-      const txHash = txMeta.hash
-      const txId = txMeta.id
-
-      if (!txHash) {
-        const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.')
-        noTxHashErr.name = 'NoTxHashError'
-        this.setTxStatusFailed(txId, noTxHashErr)
-      }
-
-
-      block.transactions.forEach((tx) => {
-        if (tx.hash === txHash) this.setTxStatusConfirmed(txId)
-      })
-    })
-  }
-
-  queryPendingTxs ({oldBlock, newBlock}) {
-    // check pending transactions on start
-    if (!oldBlock) {
-      this._checkPendingTxs()
-      return
-    }
-    // if we synced by more than one block, check for missed pending transactions
-    const diff = Number.parseInt(newBlock.number) - Number.parseInt(oldBlock.number)
-    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            |
@@ -482,75 +437,4 @@ module.exports = class TransactionController extends EventEmitter {
     })
     this.memStore.updateState({ unapprovedTxs, selectedAddressTxList })
   }
-
-  async _resubmitTx (txMeta) {
-    const address = txMeta.txParams.from
-    const balance = this.ethStore.getState().accounts[address].balance
-    if (!('retryCount' in txMeta)) txMeta.retryCount = 0
-
-    // if the value of the transaction is greater then the balance, fail.
-    if (!this.txProviderUtils.sufficientBalance(txMeta.txParams, balance)) {
-      const message = 'Insufficient balance.'
-      this.setTxStatusFailed(txMeta.id, {
-        stack: '_resubmitTx: custom tx-controller error',
-        message,
-      })
-      log.error(message)
-      return
-    }
-
-    // Only auto-submit already-signed txs:
-    if (!('rawTx' in txMeta)) return
-
-    // Increment a try counter.
-    txMeta.retryCount++
-    const rawTx = txMeta.rawTx
-    return await this.txProviderUtils.publishTransaction(rawTx)
-  }
-
-  // checks the network for signed txs and
-  // if confirmed sets the tx status as 'confirmed'
-  async _checkPendingTxs () {
-    const signedTxList = this.getFilteredTxList({status: 'submitted'})
-    // in order to keep the nonceTracker accurate we block it while updating pending transactions
-    const nonceGlobalLock = await this.nonceTracker.getGlobalLock()
-    try {
-      await Promise.all(signedTxList.map((txMeta) => this._checkPendingTx(txMeta)))
-    } catch (err) {
-      console.error('TransactionController - Error updating pending transactions')
-      console.error(err)
-    }
-    nonceGlobalLock.releaseLock()
-  }
-
-  async _checkPendingTx (txMeta) {
-    const txHash = txMeta.hash
-    const txId = txMeta.id
-    // extra check in case there was an uncaught error during the
-    // signature and submission process
-    if (!txHash) {
-      const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.')
-      noTxHashErr.name = 'NoTxHashError'
-      this.setTxStatusFailed(txId, noTxHashErr)
-    }
-    // get latest transaction status
-    let txParams
-    try {
-      txParams = await this.query.getTransactionByHash(txHash)
-      if (!txParams) return
-      if (txParams.blockNumber) {
-        this.setTxStatusConfirmed(txId)
-      }
-    } catch (err) {
-      if (err || !txParams) {
-        txMeta.err = {
-          isWarning: true,
-          errorCode: err,
-          message: 'There was a problem loading this transaction.',
-        }
-        this.updateTx(txMeta)
-        throw err
-      }
-    }
-  }
 }
\ No newline at end of file
diff --git a/app/scripts/lib/pending-tx-tracker.js b/app/scripts/lib/pending-tx-tracker.js
new file mode 100644
index 000000000..6921997b2
--- /dev/null
+++ b/app/scripts/lib/pending-tx-tracker.js
@@ -0,0 +1,163 @@
+const EventEmitter = require('events')
+const EthQuery = require('ethjs-query')
+const sufficientBalance = require('./util').sufficientBalance
+/*
+
+  Utility class for tracking the transactions as they
+  go from a pending state to a confirmed (mined in a block) state
+
+  As well as continues broadcast while in the pending state
+
+                ~config is not optional~
+  requires a: {
+    provider: //,
+    nonceTracker: //see nonce tracker,
+    getBalnce: //(address) a function for getting balances,
+    getPendingTransactions: //() a function for getting an array of transactions,
+    publishTransaction: //(rawTx) a async function for publishing raw transactions,
+  }
+
+*/
+
+module.exports = class PendingTransactionTracker extends EventEmitter {
+  constructor (config) {
+    super()
+    this.query = new EthQuery(config.provider)
+    this.nonceTracker = config.nonceTracker
+
+    this.getBalance = config.getBalance
+    this.getPendingTransactions = config.getPendingTransactions
+    this.publishTransaction = config.publishTransaction
+  }
+
+  //  checks if a signed tx is in a block and
+  // if included sets the tx status as 'confirmed'
+  checkForTxInBlock (block) {
+    const signedTxList = this.getPendingTransactions()
+    if (!signedTxList.length) return
+    signedTxList.forEach((txMeta) => {
+      const txHash = txMeta.hash
+      const txId = txMeta.id
+
+      if (!txHash) {
+        const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.')
+        noTxHashErr.name = 'NoTxHashError'
+        this.emit('txFailed', txId, noTxHashErr)
+        return
+      }
+
+
+      block.transactions.forEach((tx) => {
+        if (tx.hash === txHash) this.emit('txConfirmed', txId)
+      })
+    })
+  }
+
+  queryPendingTxs ({oldBlock, newBlock}) {
+    // check pending transactions on start
+    if (!oldBlock) {
+      this._checkPendingTxs()
+      return
+    }
+    // if we synced by more than one block, check for missed pending transactions
+    const diff = Number.parseInt(newBlock.number, 16) - Number.parseInt(oldBlock.number, 16)
+    if (diff > 1) this._checkPendingTxs()
+  }
+
+
+  resubmitPendingTxs () {
+    const pending = this.getPendingTransactions('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.emit('txFailed', txMeta.id, err)
+    }))
+  }
+
+  async _resubmitTx (txMeta) {
+    const address = txMeta.txParams.from
+    const balance = this.getBalance(address)
+    if (balance === undefined) return
+    if (!('retryCount' in txMeta)) txMeta.retryCount = 0
+
+    // if the value of the transaction is greater then the balance, fail.
+    if (!sufficientBalance(txMeta.txParams, balance)) {
+      const insufficientFundsError = new Error('Insufficient balance during rebroadcast.')
+      this.emit('txFailed', txMeta.id, insufficientFundsError)
+      log.error(insufficientFundsError)
+      return
+    }
+
+    // Only auto-submit already-signed txs:
+    if (!('rawTx' in txMeta)) return
+
+    // Increment a try counter.
+    txMeta.retryCount++
+    const rawTx = txMeta.rawTx
+    return await this.publishTransaction(rawTx)
+  }
+
+  async _checkPendingTx (txMeta) {
+    const txHash = txMeta.hash
+    const txId = txMeta.id
+    // extra check in case there was an uncaught error during the
+    // signature and submission process
+    if (!txHash) {
+      const noTxHashErr = new Error('We had an error while submitting this transaction, please try again.')
+      noTxHashErr.name = 'NoTxHashError'
+      this.emit('txFailed', txId, noTxHashErr)
+      return
+    }
+    // get latest transaction status
+    let txParams
+    try {
+      txParams = await this.query.getTransactionByHash(txHash)
+      if (!txParams) return
+      if (txParams.blockNumber) {
+        this.emit('txConfirmed', txId)
+      }
+    } catch (err) {
+      txMeta.warning = {
+        error: err,
+        message: 'There was a problem loading this transaction.',
+      }
+      this.emit('txWarning', txMeta)
+      throw err
+    }
+  }
+
+  // checks the network for signed txs and
+  // if confirmed sets the tx status as 'confirmed'
+  async _checkPendingTxs () {
+    const signedTxList = this.getPendingTransactions()
+    // in order to keep the nonceTracker accurate we block it while updating pending transactions
+    const nonceGlobalLock = await this.nonceTracker.getGlobalLock()
+    try {
+      await Promise.all(signedTxList.map((txMeta) => this._checkPendingTx(txMeta)))
+    } catch (err) {
+      console.error('PendingTransactionWatcher - Error updating pending transactions')
+      console.error(err)
+    }
+    nonceGlobalLock.releaseLock()
+  }
+}
\ No newline at end of file
diff --git a/app/scripts/lib/tx-utils.js b/app/scripts/lib/tx-utils.js
index 3687a9652..b64ea6712 100644
--- a/app/scripts/lib/tx-utils.js
+++ b/app/scripts/lib/tx-utils.js
@@ -1,7 +1,11 @@
-const ethUtil = require('ethereumjs-util')
+const EthQuery = require('ethjs-query')
 const Transaction = require('ethereumjs-tx')
 const normalize = require('eth-sig-util').normalize
-const BN = ethUtil.BN
+const {
+  hexToBn,
+  BnMultiplyByFraction,
+  bnToHex,
+} = require('./util')
 
 /*
 tx-utils are utility methods for Transaction manager
@@ -9,9 +13,9 @@ its passed ethquery
 and used to do things like calculate gas of a tx.
 */
 
-module.exports = class txProvideUtils {
-  constructor (ethQuery) {
-    this.query = ethQuery
+module.exports = class txProvideUtil {
+  constructor (provider) {
+    this.query = new EthQuery(provider)
   }
 
   async analyzeGasUsage (txMeta) {
@@ -91,31 +95,4 @@ module.exports = class txProvideUtils {
       throw new Error(`Invalid transaction value of ${txParams.value} not a positive number.`)
     }
   }
-
-  sufficientBalance (txParams, hexBalance) {
-    const balance = hexToBn(hexBalance)
-    const value = hexToBn(txParams.value)
-    const gasLimit = hexToBn(txParams.gas)
-    const gasPrice = hexToBn(txParams.gasPrice)
-
-    const maxCost = value.add(gasLimit.mul(gasPrice))
-    return balance.gte(maxCost)
-  }
-
-}
-
-// util
-
-function bnToHex (inputBn) {
-  return ethUtil.addHexPrefix(inputBn.toString(16))
-}
-
-function hexToBn (inputHex) {
-  return new BN(ethUtil.stripHexPrefix(inputHex), 16)
-}
-
-function BnMultiplyByFraction (targetBN, numerator, denominator) {
-  const numBN = new BN(numerator)
-  const denomBN = new BN(denominator)
-  return targetBN.mul(numBN).div(denomBN)
-}
+}
\ No newline at end of file
diff --git a/app/scripts/lib/util.js b/app/scripts/lib/util.js
index bddd60ee8..70390e95c 100644
--- a/app/scripts/lib/util.js
+++ b/app/scripts/lib/util.js
@@ -1,8 +1,39 @@
+const ethUtil = require('ethereumjs-util')
+const BN = ethUtil.BN
+
 module.exports = {
   getStack,
+  sufficientBalance,
+  hexToBn,
+  bnToHex,
+  BnMultiplyByFraction,
 }
 
 function getStack () {
   const stack = new Error('Stack trace generator - not an error').stack
   return stack
 }
+
+function sufficientBalance (txParams, hexBalance) {
+  const balance = hexToBn(hexBalance)
+  const value = hexToBn(txParams.value)
+  const gasLimit = hexToBn(txParams.gas)
+  const gasPrice = hexToBn(txParams.gasPrice)
+
+  const maxCost = value.add(gasLimit.mul(gasPrice))
+  return balance.gte(maxCost)
+}
+
+function bnToHex (inputBn) {
+  return ethUtil.addHexPrefix(inputBn.toString(16))
+}
+
+function hexToBn (inputHex) {
+  return new BN(ethUtil.stripHexPrefix(inputHex), 16)
+}
+
+function BnMultiplyByFraction (targetBN, numerator, denominator) {
+  const numBN = new BN(numerator)
+  const denomBN = new BN(denominator)
+  return targetBN.mul(numBN).div(denomBN)
+}