From 4942fd53b8370a217141d005c17de30bda408c5a Mon Sep 17 00:00:00 2001
From: Dan J Miller <danjm.com@gmail.com>
Date: Thu, 30 May 2024 15:12:17 -0230
Subject: [PATCH] fix: Prevent network request when useCurrencyRateCheck is
 false (#24888)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## **Description**

With https://github.com/MetaMask/core/pull/1805, polling in the
CurrencyRateController happens when the controller is initialized. Once
the extension received an update to use the version containing those
changes, we started making cryptocompare network requests even when the
"Show balance and token price checker" toggle is off.

This PR prevents those requests when that toggle is off by wrapping the
`fetchExchangeRates` method of the CurrencyRateController with a
function that will just return 0 values if that toggle is off.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/24888?quickstart=1)

## **Manual testing steps**

1. Install and build and onboard
2. See requests to cryptocompare in the background console
3. Toggle off "Show balance and token price checker"
4. Reload the extension, there should be no requests to cryptocompare
5. Switch networks, there should be no requests to cryptocompare

1, Install and onboard
2. Toggle off "Show balance and token price checker" in advanced
settings during onboarding
3. There should be not requests to cryptocompare after onboarding

For either of the above scenarios, toggle "Show balance and token price
checker" back on. There should now be requests to cryptocompare

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
---
 app/scripts/metamask-controller.js                 | 13 +++++++++++++
 test/e2e/tests/privacy/basic-functionality.spec.js | 14 ++++++++++++++
 .../privacy-settings/privacy-settings.js           |  1 +
 3 files changed, 28 insertions(+)

diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
index 0ba25ce76f95..f5c042c784fa 100644
--- a/app/scripts/metamask-controller.js
+++ b/app/scripts/metamask-controller.js
@@ -800,6 +800,19 @@ export default class MetamaskController extends EventEmitter {
       messenger: currencyRateMessenger,
       state: initState.CurrencyController,
     });
+    const initialFetchExchangeRate =
+      this.currencyRateController.fetchExchangeRate.bind(
+        this.currencyRateController,
+      );
+    this.currencyRateController.fetchExchangeRate = (...args) => {
+      if (this.preferencesController.store.getState().useCurrencyRateCheck) {
+        return initialFetchExchangeRate(...args);
+      }
+      return {
+        conversionRate: null,
+        usdConversionRate: null,
+      };
+    };
 
     const phishingControllerMessenger = this.controllerMessenger.getRestricted({
       name: 'PhishingController',
diff --git a/test/e2e/tests/privacy/basic-functionality.spec.js b/test/e2e/tests/privacy/basic-functionality.spec.js
index 924e0e15d3df..eea5e02229a4 100644
--- a/test/e2e/tests/privacy/basic-functionality.spec.js
+++ b/test/e2e/tests/privacy/basic-functionality.spec.js
@@ -19,6 +19,17 @@ async function mockApis(mockServer) {
           body: [{ fakedata: true }],
         };
       }),
+    await mockServer
+      .forGet('https://min-api.cryptocompare.com/data/price')
+      .withQuery({ fsym: 'ETH', tsyms: 'USD' })
+      .thenCallback(() => {
+        return {
+          statusCode: 200,
+          json: {
+            fakedata: 0,
+          },
+        };
+      }),
   ];
 }
 
@@ -45,6 +56,9 @@ describe('MetaMask onboarding @no-mmi', function () {
         );
         await driver.clickElement('[id="basic-configuration-checkbox"]');
         await driver.clickElement({ text: 'Turn off', tag: 'button' });
+        await driver.clickElement(
+          '[data-testid="currency-rate-check-toggle"] .toggle-button',
+        );
         await driver.clickElement({ text: 'Done', tag: 'button' });
 
         await driver.clickElement('[data-testid="network-display"]');
diff --git a/ui/pages/onboarding-flow/privacy-settings/privacy-settings.js b/ui/pages/onboarding-flow/privacy-settings/privacy-settings.js
index 3020060bacc6..f4597bdc81d6 100644
--- a/ui/pages/onboarding-flow/privacy-settings/privacy-settings.js
+++ b/ui/pages/onboarding-flow/privacy-settings/privacy-settings.js
@@ -359,6 +359,7 @@ export default function PrivacySettings() {
             value={turnOnCurrencyRateCheck}
             setValue={setTurnOnCurrencyRateCheck}
             title={t('currencyRateCheckToggle')}
+            dataTestId="currency-rate-check-toggle"
             description={t('currencyRateCheckToggleDescription', [
               <a
                 key="coingecko_link"