Skip to content

Commit

Permalink
fix: Prevent network request when useCurrencyRateCheck is false (#24888)
Browse files Browse the repository at this point in the history
## **Description**

With MetaMask/core#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.
  • Loading branch information
danjm committed May 31, 2024
1 parent d398737 commit 4942fd5
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 0 deletions.
13 changes: 13 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
14 changes: 14 additions & 0 deletions test/e2e/tests/privacy/basic-functionality.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
};
}),
];
}

Expand All @@ -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"]');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 4942fd5

Please sign in to comment.