-
-
Notifications
You must be signed in to change notification settings - Fork 256
fix: remove crypto compare fallback #7167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
|
Tested extension - confirmed that I do not see any crypto-compare API calls |
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
…st (#7181) ## Explanation **What is the current state of things and why does it need to change?** The `CurrencyRateController` has a fallback mechanism that uses spot prices from the token prices service when the primary exchange rates API fails. However, this fallback was sending duplicate native token addresses (the 0x0000...0000 address) to the API endpoint, causing requests to fail with the error: `"All tokenAddresses's elements must be unique"`. **What is the solution your changes offer and how does it work?** The issue occurred because: 1. The `CurrencyRateController` was explicitly passing the native token address in the `tokenAddresses` array: `tokenAddresses: [nativeTokenAddress]` 2. The `fetchTokenPrices` method in `CodefiTokenPricesServiceV2` automatically prepends the native token address to any token addresses passed to it (line 496 in `codefi-v2.ts`) 3. This resulted in the native token address appearing twice in the API URL The fix changes `tokenAddresses: [nativeTokenAddress]` to `tokenAddresses: []` since the service already handles including the native token address automatically. This ensures each address appears only once in the final API request. **Are there any changes whose purpose might not be obvious to those unfamiliar with the domain?** The empty array `[]` might seem counterintuitive, but it's the correct approach because the `fetchTokenPrices` method is designed to always include the native token address by default for any chain. ## References Fixes the duplicate token address validation error in the spot price fallback mechanism introduced after removing the CryptoCompare fallback in #7167. ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Stops sending the native token address explicitly to `fetchTokenPrices` (pass `tokenAddresses: []`) to avoid duplicates in spot price fallback requests. > > - **CurrencyRateController**: > - Update fallback spot price logic to call `fetchTokenPrices({ chainId, tokenAddresses: [], currency })`, relying on the service to include the native token automatically. > - **Tests**: > - Adjust expectations in `CurrencyRateController.test.ts` to assert `tokenAddresses: []` for ETH/BNB/POL scenarios and related calls. > - **Changelog**: > - Add Unreleased fix entry describing duplicate native token address issue in spot price fallback. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 64c9a6b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
Explanation
This PR removes the fallback to CryptoCompare API in both
CurrencyRatesControllerandTokenRatesController.Current State
Previously, when the primary price data source failed, both controllers would fall back to the CryptoCompare API to fetch currency and token rates. This added complexity to the codebase and introduced a dependency on an additional external service.
Solution
This change removes the CryptoCompare fallback logic from both controllers, simplifying the rate fetching flow and reducing external dependencies. The controllers will now rely solely on the primary price data source without attempting to fall back to CryptoCompare when the primary source fails.
Rationale
extension PR: MetaMask/metamask-extension#37884
mobile PR: MetaMask/metamask-mobile#22772
References
Checklist
Note
Eliminates CryptoCompare fallbacks in currency and token rate controllers, replacing them with a token-prices service–based fallback and updating tests/behaviors accordingly.
CurrencyRateControllerandTokenRatesController.fetchMultiExchangeRate; primary source remainsfetchExchangeRatesfrom token-prices service.fetchTokenPricesmapped viaNetworkController:getStateto chain IDs andgetNativeTokenAddress.nullvalues instead of throwing; skip empty/undefined native currencies.includeUsdRate; maintain polling behavior; addNetworkController:getStateto allowed actions.usd, then convert to native using native token USD price.getNativeTokenAddress; return empty when native USD price unavailable; keep batch fetching and consolidate state updates.nock/CryptoCompare stubs; update expectations to new fallback and failure semantics; add network state helpers.CHANGELOG.mdnoting breaking removal of CryptoCompare fallback; bump@metamask/core-backend.Written by Cursor Bugbot for commit 7a43688. This will update automatically on new commits. Configure here.