Bug fixes for incoming transactions (#7043)

* Fix styling of `.transaction-list`
* Filter `incomingTxListSelector` by network as well
* Start and stop block tracker polling in incoming tx controller
* Add fetch with abort in bg for `IncomingTxController`

app/scripts/controllers/incoming-transactions.js

@@ -2,7 +2,7 @@ const ObservableStore = require('obs-store')
 const log = require('loglevel')
 const BN = require('bn.js')
 const createId = require('../lib/random-id')
-const { bnToHex } = require('../lib/util')
+const { bnToHex, fetchWithTimeout } = require('../lib/util')
 const {
   MAINNET_CODE,
   ROPSTEN_CODE,
@@ -14,11 +14,14 @@ const {
   MAINNET,
 } = require('./network/enums')
 const networkTypeToIdMap = {
-  [ROPSTEN]: ROPSTEN_CODE,
+  [ROPSTEN]: String(ROPSTEN_CODE),
-  [RINKEBY]: RINKEYBY_CODE,
+  [RINKEBY]: String(RINKEYBY_CODE),
-  [KOVAN]: KOVAN_CODE,
+  [KOVAN]: String(KOVAN_CODE),
-  [MAINNET]: MAINNET_CODE,
+  [MAINNET]: String(MAINNET_CODE),
 }
+const fetch = fetchWithTimeout({
+  timeout: 30000,
+})
 
 class IncomingTransactionsController {
 
@@ -33,6 +36,15 @@ class IncomingTransactionsController {
     this.preferencesController = preferencesController
     this.getCurrentNetwork = () => networkController.getProviderConfig().type
 
+    this._onLatestBlock = async (newBlockNumberHex) => {
+      const selectedAddress = this.preferencesController.getSelectedAddress()
+      const newBlockNumberDec = parseInt(newBlockNumberHex, 16)
+      await this._update({
+        address: selectedAddress,
+        newBlockNumberDec,
+      })
+    }
+
     const initState = Object.assign({
       incomingTransactions: {},
       incomingTxLastFetchedBlocksByNetwork: {
@@ -51,13 +63,6 @@ class IncomingTransactionsController {
         networkType: newType,
       })
     })
-    this.blockTracker.on('latest', async (newBlockNumberHex) => {
-      const address = this.preferencesController.getSelectedAddress()
-      await this._update({
-        address,
-        newBlockNumberDec: parseInt(newBlockNumberHex, 16),
-      })
-    })
     this.preferencesController.store.subscribe(async ({ selectedAddress }) => {
       await this._update({
         address: selectedAddress,
@@ -65,6 +70,15 @@ class IncomingTransactionsController {
     })
   }
 
+  start () {
+    this.blockTracker.removeListener('latest', this._onLatestBlock)
+    this.blockTracker.addListener('latest', this._onLatestBlock)
+  }
+
+  stop () {
+    this.blockTracker.removeListener('latest', this._onLatestBlock)
+  }
+
   async _update ({ address, newBlockNumberDec, networkType } = {}) {
     try {
       const dataForUpdate = await this._getDataForUpdate({ address, newBlockNumberDec, networkType })

app/scripts/lib/util.js

@@ -144,6 +144,29 @@ function removeListeners (listeners, emitter) {
   })
 }
 
+function fetchWithTimeout ({ timeout = 120000 } = {}) {
+  return async function _fetch (url, opts) {
+    const abortController = new AbortController()
+    const abortSignal = abortController.signal
+    const f = fetch(url, {
+      ...opts,
+      signal: abortSignal,
+    })
+
+    const timer = setTimeout(() => abortController.abort(), timeout)
+
+    try {
+      const res = await f
+      clearTimeout(timer)
+      return res
+    } catch (e) {
+      clearTimeout(timer)
+      throw e
+    }
+  }
+}
+
+
 module.exports = {
   removeListeners,
   applyListeners,
@@ -154,4 +177,5 @@ module.exports = {
   hexToBn,
   bnToHex,
   BnMultiplyByFraction,
+  fetchWithTimeout,
 }

app/scripts/metamask-controller.js

@@ -156,8 +156,10 @@ module.exports = class MetamaskController extends EventEmitter {
     this.on('controllerConnectionChanged', (activeControllerConnections) => {
       if (activeControllerConnections > 0) {
         this.accountTracker.start()
+        this.incomingTransactionsController.start()
       } else {
         this.accountTracker.stop()
+        this.incomingTransactionsController.stop()
       }
     })
 

test/unit/app/controllers/incoming-transactions-test.js

@@ -49,7 +49,8 @@ describe('IncomingTransactionsController', () => {
   }
 
   const MOCK_BLOCKTRACKER = {
-    on: sinon.spy(),
+    addListener: sinon.spy(),
+    removeListener: sinon.spy(),
     testProperty: 'fakeBlockTracker',
     getCurrentBlock: () => '0xa',
   }
@@ -95,17 +96,6 @@ describe('IncomingTransactionsController', () => {
       })
 
       incomingTransactionsController._update.resetHistory()
-
-      assert(incomingTransactionsController.blockTracker.on.calledOnce)
-      assert.equal(incomingTransactionsController.blockTracker.on.getCall(0).args[0], 'latest')
-      const blockTrackerListenerCallback = incomingTransactionsController.blockTracker.on.getCall(0).args[1]
-      assert.equal(incomingTransactionsController._update.callCount, 0)
-      blockTrackerListenerCallback('0xabc')
-      assert.equal(incomingTransactionsController._update.callCount, 1)
-      assert.deepEqual(incomingTransactionsController._update.getCall(0).args[0], {
-        address: '0x0101',
-        newBlockNumberDec: 2748,
-      })
     })
 
     it('should set the store to a provided initial state', () => {
@@ -120,6 +110,31 @@ describe('IncomingTransactionsController', () => {
     })
   })
 
+  describe('#start', () => {
+    it('should set up a listener for the latest block', () => {
+      const incomingTransactionsController = new IncomingTransactionsController({
+        blockTracker: MOCK_BLOCKTRACKER,
+        networkController: MOCK_NETWORK_CONTROLLER,
+        preferencesController: MOCK_PREFERENCES_CONTROLLER,
+        initState: {},
+      })
+      sinon.spy(incomingTransactionsController, '_update')
+
+      incomingTransactionsController.start()
+
+      assert(incomingTransactionsController.blockTracker.addListener.calledOnce)
+      assert.equal(incomingTransactionsController.blockTracker.addListener.getCall(0).args[0], 'latest')
+      const blockTrackerListenerCallback = incomingTransactionsController.blockTracker.addListener.getCall(0).args[1]
+      assert.equal(incomingTransactionsController._update.callCount, 0)
+      blockTrackerListenerCallback('0xabc')
+      assert.equal(incomingTransactionsController._update.callCount, 1)
+      assert.deepEqual(incomingTransactionsController._update.getCall(0).args[0], {
+        address: '0x0101',
+        newBlockNumberDec: 2748,
+      })
+    })
+  })
+
   describe('_getDataForUpdate', () => {
     it('should call fetchAll with the correct params when passed a new block number and the current network has no stored block', async () => {
       const incomingTransactionsController = new IncomingTransactionsController({

ui/app/components/app/transaction-list/index.scss

@@ -11,34 +11,15 @@
   }
 
   &__header {
+    flex: 0 0 auto;
+    font-size: 14px;
+    line-height: 20px;
+    color: $Grey-400;
     border-bottom: 1px solid $Grey-100;
+    padding: 8px 0 8px 20px;
 
-    &__tabs {
+    @media screen and (max-width: $break-small) {
-      display: flex; 
+      padding: 8px 0 8px 16px;
-    }
-
-    &__tab,
-    &__tab--selected {
-      flex: 0 0 auto;
-      font-size: 14px;
-      line-height: 20px;
-      color: $Grey-400;
-      padding: 8px 0 8px 20px;
-      cursor: pointer;
-
-      &:hover {
-        font-weight: bold;
-      }
-
-      @media screen and (max-width: $break-small) {
-        padding: 8px 0 8px 16px;
-      }
-    }
-
-    &__tab--selected {
-      font-weight: bold;
-      color: $Blue-400;
-      cursor: auto;
     }
   }
 

ui/app/selectors/transactions.js

@@ -16,9 +16,12 @@ import txHelper from '../../lib/tx-helper'
 export const shapeShiftTxListSelector = state => state.metamask.shapeShiftTxList
 
 export const incomingTxListSelector = state => {
+  const network = state.metamask.network
   const selectedAddress = state.metamask.selectedAddress
   return Object.values(state.metamask.incomingTransactions)
-    .filter(({ txParams }) => txParams.to === selectedAddress)
+    .filter(({ metamaskNetworkId, txParams }) => (
+      txParams.to === selectedAddress && metamaskNetworkId === network
+    ))
 }
 export const unapprovedMsgsSelector = state => state.metamask.unapprovedMsgs
 export const selectedAddressTxListSelector = state => state.metamask.selectedAddressTxList