Enforce user preferences in incoming transactions controller (#19982)

* Enforce user preferences in incoming transactions controller

* Combine various conditions to determine tx lookup

* Update tests to not use private methods

app/scripts/controllers/incoming-transactions.js

@@ -135,15 +135,12 @@ export default class IncomingTransactionsController {
   }
 
   start() {
-    const { featureFlags = {} } = this.preferencesController.store.getState();
-    const { showIncomingTransactions } = featureFlags;
+    const chainId = this.getCurrentChainId();
 
-    if (!showIncomingTransactions) {
-      return;
+    if (this._allowedToMakeFetchIncomingTx(chainId)) {
+      this.blockTracker.removeListener('latest', this._onLatestBlock);
+      this.blockTracker.addListener('latest', this._onLatestBlock);
     }
-
-    this.blockTracker.removeListener('latest', this._onLatestBlock);
-    this.blockTracker.addListener('latest', this._onLatestBlock);
   }
 
   stop() {
@@ -161,13 +158,9 @@ export default class IncomingTransactionsController {
    * @param {number} [newBlockNumberDec] - block number to begin fetching from
    */
   async _update(address, newBlockNumberDec) {
-    const { completedOnboarding } = this.onboardingController.store.getState();
     const chainId = this.getCurrentChainId();
-    if (
-      !Object.hasOwnProperty.call(ETHERSCAN_SUPPORTED_NETWORKS, chainId) ||
-      !address ||
-      !completedOnboarding
-    ) {
+
+    if (!address || !this._allowedToMakeFetchIncomingTx(chainId)) {
       return;
     }
     try {
@@ -302,4 +295,26 @@ export default class IncomingTransactionsController {
       type: TransactionType.incoming,
     };
   }
+
+  /**
+   * @param chainId - {string} The chainId of the current network
+   * @returns {boolean} Whether or not the user has consented to show incoming transactions
+   */
+  _allowedToMakeFetchIncomingTx(chainId) {
+    const { featureFlags = {} } = this.preferencesController.store.getState();
+    const { completedOnboarding } = this.onboardingController.store.getState();
+
+    const hasIncomingTransactionsFeatureEnabled = Boolean(
+      featureFlags.showIncomingTransactions,
+    );
+
+    const isEtherscanSupportedNetwork = Boolean(
+      ETHERSCAN_SUPPORTED_NETWORKS[chainId],
+    );
+    return (
+      completedOnboarding &&
+      isEtherscanSupportedNetwork &&
+      hasIncomingTransactionsFeatureEnabled
+    );
+  }
 }

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

@@ -78,11 +78,11 @@ function getMockPreferencesController({
   };
 }
 
-function getMockOnboardingController() {
+function getMockOnboardingController({ completedOnboarding = true } = {}) {
   return {
     store: {
       getState: sinon.stub().returns({
-        completedOnboarding: true,
+        completedOnboarding,
       }),
       subscribe: sinon.spy(),
     },
@@ -98,6 +98,16 @@ function getMockBlockTracker() {
   };
 }
 
+function getDefaultControllerOpts() {
+  return {
+    blockTracker: getMockBlockTracker(),
+    ...getMockNetworkControllerMethods(CHAIN_IDS.GOERLI),
+    preferencesController: getMockPreferencesController(),
+    onboardingController: getMockOnboardingController(),
+    initState: getEmptyInitState(),
+  };
+}
+
 /**
  * @typedef {import(
  *  '../../../../app/scripts/controllers/incoming-transactions'
@@ -226,6 +236,7 @@ describe('IncomingTransactionsController', function () {
           preferencesController: getMockPreferencesController(),
           onboardingController: getMockOnboardingController(),
           initState: {},
+          getCurrentChainId: () => CHAIN_IDS.GOERLI,
         },
       );
 
@@ -831,6 +842,97 @@ describe('IncomingTransactionsController', function () {
     });
   });
 
+  describe('block explorer lookup', function () {
+    let sandbox;
+
+    beforeEach(function () {
+      sandbox = sinon.createSandbox();
+    });
+
+    afterEach(function () {
+      sandbox.restore();
+    });
+
+    function stubFetch() {
+      return sandbox.stub(window, 'fetch');
+    }
+
+    function assertStubNotCalled(stub) {
+      assert(stub.callCount === 0);
+    }
+
+    async function triggerUpdate(incomingTransactionsController) {
+      const subscription =
+        incomingTransactionsController.preferencesController.store.subscribe.getCall(
+          1,
+        ).args[0];
+
+      // Sets address causing a call to _update
+      await subscription({ selectedAddress: MOCK_SELECTED_ADDRESS });
+    }
+
+    it('should not happen when incoming transactions feature is disabled', async function () {
+      const incomingTransactionsController = new IncomingTransactionsController(
+        {
+          ...getDefaultControllerOpts(),
+          preferencesController: getMockPreferencesController({
+            showIncomingTransactions: false,
+          }),
+        },
+      );
+      const fetchStub = stubFetch();
+      await triggerUpdate(incomingTransactionsController);
+      assertStubNotCalled(fetchStub);
+    });
+
+    it('should not happen when onboarding is in progress', async function () {
+      const incomingTransactionsController = new IncomingTransactionsController(
+        {
+          ...getDefaultControllerOpts(),
+          onboardingController: getMockOnboardingController({
+            completedOnboarding: false,
+          }),
+        },
+      );
+
+      const fetchStub = stubFetch();
+      await triggerUpdate(incomingTransactionsController);
+      assertStubNotCalled(fetchStub);
+    });
+
+    it('should not happen when chain id is not supported', async function () {
+      const incomingTransactionsController = new IncomingTransactionsController(
+        {
+          ...getDefaultControllerOpts(),
+          getCurrentChainId: () => FAKE_CHAIN_ID,
+        },
+      );
+
+      const fetchStub = stubFetch();
+      await triggerUpdate(incomingTransactionsController);
+      assertStubNotCalled(fetchStub);
+    });
+
+    it('should make api call when chain id, incoming features, and onboarding status are ok', async function () {
+      const incomingTransactionsController = new IncomingTransactionsController(
+        {
+          ...getDefaultControllerOpts(),
+          getCurrentChainId: () => CHAIN_IDS.GOERLI,
+          onboardingController: getMockOnboardingController({
+            completedOnboarding: true,
+          }),
+          preferencesController: getMockPreferencesController({
+            showIncomingTransactions: true,
+          }),
+        },
+      );
+
+      const fetchStub = stubFetch();
+      await triggerUpdate(incomingTransactionsController);
+      assert(fetchStub.callCount === 1);
+    });
+  });
+
   describe('_update', function () {
     describe('when state is empty (initialized)', function () {
       it('should use provided block number and update the latest block seen', async function () {