Merge pull request #4347 from MetaMask/transactions-use-new-block-tracker

Transactions use new block tracker
7 years ago · 3d8f41e912
parent eb2423799d 61caee9d94
commit 3d8f41e912
3 changed files with 44 additions and 40 deletions
--- a/app/scripts/controllers/transactions/index.js
+++ b/app/scripts/controllers/transactions/index.js
@ -57,6 +57,7 @@ class TransactionController extends EventEmitter {
      initState: opts.initState,
      txHistoryLimit: opts.txHistoryLimit,
      getNetwork: this.getNetwork.bind(this),
+      confirmTransaction: this.confirmTransaction.bind(this),
    })
    this._onBootCleanUp()

@ -301,6 +302,11 @@ class TransactionController extends EventEmitter {
    this.txStateManager.setTxStatusSubmitted(txId)
  }

+  confirmTransaction (txId) {
+    this.txStateManager.setTxStatusConfirmed(txId)
+    this._markNonceDuplicatesDropped(txId)
+  }
+
  /**
    Convenience method for the ui thats sets the transaction to rejected
    @param txId {number} - the tx's Id
@ -381,8 +387,6 @@ class TransactionController extends EventEmitter {
    this.pendingTxTracker.on('tx:warning', (txMeta) => {
      this.txStateManager.updateTx(txMeta, 'transactions/pending-tx-tracker#event: tx:warning')
    })
-    this.pendingTxTracker.on('tx:confirmed', (txId) => this.txStateManager.setTxStatusConfirmed(txId))
-    this.pendingTxTracker.on('tx:confirmed', (txId) => this._markNonceDuplicatesDropped(txId))
    this.pendingTxTracker.on('tx:failed', this.txStateManager.setTxStatusFailed.bind(this.txStateManager))
    this.pendingTxTracker.on('tx:block-update', (txMeta, latestBlockNumber) => {
      if (!txMeta.firstRetryBlockNumber) {
--- a/app/scripts/controllers/transactions/pending-tx-tracker.js
+++ b/app/scripts/controllers/transactions/pending-tx-tracker.js
@ -27,6 +27,7 @@ class PendingTransactionTracker extends EventEmitter {
    this.getPendingTransactions = config.getPendingTransactions
    this.getCompletedTransactions = config.getCompletedTransactions
    this.publishTransaction = config.publishTransaction
+    this.confirmTransaction = config.confirmTransaction
    this._checkPendingTxs()
  }

@ -37,7 +38,8 @@ class PendingTransactionTracker extends EventEmitter {
    @emits tx:confirmed
    @emits tx:failed
  */
-  checkForTxInBlock (block) {
+  async checkForTxInBlock (blockNumber) {
+    const block = await this._getBlock(blockNumber)
    const signedTxList = this.getPendingTransactions()
    if (!signedTxList.length) return
    signedTxList.forEach((txMeta) => {
@ -51,9 +53,12 @@ class PendingTransactionTracker extends EventEmitter {
        return
      }

+      if (!block.transactions.length) return

-      block.transactions.forEach((tx) => {
-        if (tx.hash === txHash) this.emit('tx:confirmed', txId)
+      block.transactions.forEach((hash) => {
+        if (hash === txHash) {
+          this.confirmTransaction(txId)
+        }
      })
    })
  }
@ -70,7 +75,7 @@ class PendingTransactionTracker extends EventEmitter {
      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)
+    const diff = Number.parseInt(newBlock, 16) - Number.parseInt(oldBlock, 16)
    if (diff > 1) this._checkPendingTxs()
  }

@ -79,11 +84,11 @@ class PendingTransactionTracker extends EventEmitter {
    @param block {object} - a block object
    @emits tx:warning
  */
-  resubmitPendingTxs (block) {
+  resubmitPendingTxs (blockNumber) {
    const pending = this.getPendingTransactions()
    // only try resubmitting if their are transactions to resubmit
    if (!pending.length) return
-    pending.forEach((txMeta) => this._resubmitTx(txMeta, block.number).catch((err) => {
+    pending.forEach((txMeta) => this._resubmitTx(txMeta, blockNumber).catch((err) => {
      /*
      Dont marked as failed if the error is a "known" transaction warning
      "there is already a transaction with the same sender-nonce
@ -179,7 +184,7 @@ class PendingTransactionTracker extends EventEmitter {
      txParams = await this.query.getTransactionByHash(txHash)
      if (!txParams) return
      if (txParams.blockNumber) {
-        this.emit('tx:confirmed', txId)
+        this.confirmTransaction(txId)
      }
    } catch (err) {
      txMeta.warning = {
@ -206,11 +211,17 @@ class PendingTransactionTracker extends EventEmitter {
    nonceGlobalLock.releaseLock()
  }

+  async _getBlock (blockNumber) {
+    return await this.query.getBlockByNumber(blockNumber, false)
+  }
+
  /**
    checks to see if a confirmed txMeta has the same nonce
    @param txMeta {Object} - txMeta object
    @returns {boolean}
  */
+
+
  async _checkIfNonceIsTaken (txMeta) {
    const address = txMeta.txParams.from
    const completed = this.getCompletedTransactions(address)
--- a/test/unit/app/controllers/transactions/pending-tx-test.js
+++ b/test/unit/app/controllers/transactions/pending-tx-test.js
@ -52,7 +52,10 @@ describe('PendingTransactionTracker', function () {
      getPendingTransactions: () => {return []},
      getCompletedTransactions: () => {return []},
      publishTransaction: () => {},
+      confirmTransaction: () => {},
    })
+
+    pendingTxTracker._getBlock = (blockNumber) => { return {number: blockNumber, transactions: []} }
  })

  describe('_checkPendingTx state management', function () {
@ -120,40 +123,36 @@ describe('PendingTransactionTracker', function () {
      })
      pendingTxTracker.checkForTxInBlock(block)
    })
-    it('should emit \'txConfirmed\' if the tx is in the block', function (done) {
-      const block = { transactions: [txMeta]}
-      pendingTxTracker.getPendingTransactions = () => [txMeta]
-      pendingTxTracker.once('tx:confirmed', (txId) => {
-        assert(txId, txMeta.id, 'should pass txId')
-        done()
-      })
-      pendingTxTracker.once('tx:failed', (_, err) => { done(err) })
-      pendingTxTracker.checkForTxInBlock(block)
-    })
  })
  describe('#queryPendingTxs', function () {
    it('should call #_checkPendingTxs if their is no oldBlock', function (done) {
      let newBlock, oldBlock
-      newBlock = { number: '0x01' }
-      pendingTxTracker._checkPendingTxs = done
+      newBlock = '0x01'
+      const originalFunction = pendingTxTracker._checkPendingTxs
+      pendingTxTracker._checkPendingTxs = () => { done() }
      pendingTxTracker.queryPendingTxs({ oldBlock, newBlock })
+      pendingTxTracker._checkPendingTxs = originalFunction
    })
    it('should call #_checkPendingTxs if oldBlock and the newBlock have a diff of greater then 1', function (done) {
      let newBlock, oldBlock
-      oldBlock = { number: '0x01' }
-      newBlock = { number: '0x03' }
-      pendingTxTracker._checkPendingTxs = done
+      oldBlock = '0x01'
+      newBlock = '0x03'
+      const originalFunction = pendingTxTracker._checkPendingTxs
+      pendingTxTracker._checkPendingTxs = () => { done() }
      pendingTxTracker.queryPendingTxs({ oldBlock, newBlock })
+      pendingTxTracker._checkPendingTxs = originalFunction
    })
    it('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less', function (done) {
      let newBlock, oldBlock
-      oldBlock = { number: '0x1' }
-      newBlock = { number: '0x2' }
+      oldBlock = '0x1'
+      newBlock = '0x2'
+      const originalFunction = pendingTxTracker._checkPendingTxs
      pendingTxTracker._checkPendingTxs = () => {
        const err = new Error('should not call #_checkPendingTxs if oldBlock and the newBlock have a diff of 1 or less')
        done(err)
      }
      pendingTxTracker.queryPendingTxs({ oldBlock, newBlock })
+      pendingTxTracker._checkPendingTxs = originalFunction
      done()
    })
  })
@ -171,16 +170,6 @@ describe('PendingTransactionTracker', function () {
      providerResultStub.eth_getTransactionByHash = null
      pendingTxTracker._checkPendingTx(txMeta)
    })
-
-    it('should emit \'txConfirmed\'', function (done) {
-      providerResultStub.eth_getTransactionByHash = {blockNumber: '0x01'}
-      pendingTxTracker.once('tx:confirmed', (txId) => {
-        assert(txId, txMeta.id, 'should pass txId')
-        done()
-      })
-      pendingTxTracker.once('tx:failed', (_, err) => { done(err) })
-      pendingTxTracker._checkPendingTx(txMeta)
-    })
  })

  describe('#_checkPendingTxs', function () {
@ -207,7 +196,7 @@ describe('PendingTransactionTracker', function () {
  })

  describe('#resubmitPendingTxs', function () {
-    const blockStub = { number: '0x0' };
+    const blockNuberStub = '0x0'
    beforeEach(function () {
    const txMeta2 = txMeta3 = txMeta
    txList = [txMeta, txMeta2, txMeta3].map((tx) => {
@ -225,7 +214,7 @@ describe('PendingTransactionTracker', function () {
      Promise.all(txList.map((tx) => tx.processed))
      .then((txCompletedList) => done())
      .catch(done)
-      pendingTxTracker.resubmitPendingTxs(blockStub)
+      pendingTxTracker.resubmitPendingTxs(blockNuberStub)
    })
    it('should not emit \'tx:failed\' if the txMeta throws a known txError', function (done) {
      knownErrors =[
@ -252,7 +241,7 @@ describe('PendingTransactionTracker', function () {
      .then((txCompletedList) => done())
      .catch(done)

-      pendingTxTracker.resubmitPendingTxs(blockStub)
+      pendingTxTracker.resubmitPendingTxs(blockNuberStub)
    })
    it('should emit \'tx:warning\' if it encountered a real error', function (done) {
      pendingTxTracker.once('tx:warning', (txMeta, err) => {
@ -270,7 +259,7 @@ describe('PendingTransactionTracker', function () {
      .then((txCompletedList) => done())
      .catch(done)

-      pendingTxTracker.resubmitPendingTxs(blockStub)
+      pendingTxTracker.resubmitPendingTxs(blockNuberStub)
    })
  })
  describe('#_resubmitTx', function () {