From 9469435dd05e6a8864c3f0febed5c07ae497a1f3 Mon Sep 17 00:00:00 2001 From: Nicholas Ellul Date: Tue, 18 Jul 2023 16:21:30 -0400 Subject: [PATCH] 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 --- .../controllers/incoming-transactions.js | 41 ++++--- .../controllers/incoming-transactions.test.js | 106 +++++++++++++++++- 2 files changed, 132 insertions(+), 15 deletions(-) diff --git a/app/scripts/controllers/incoming-transactions.js b/app/scripts/controllers/incoming-transactions.js index 987f0d87b..9b0b85f33 100644 --- a/app/scripts/controllers/incoming-transactions.js +++ b/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 + ); + } } diff --git a/app/scripts/controllers/incoming-transactions.test.js b/app/scripts/controllers/incoming-transactions.test.js index bdfac55a7..c46c3190b 100644 --- a/app/scripts/controllers/incoming-transactions.test.js +++ b/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 () {