From 6ef79815820c5796884e562515762641e824d318 Mon Sep 17 00:00:00 2001 From: salimtb Date: Sat, 15 Nov 2025 20:43:39 +0100 Subject: [PATCH 01/10] fix: fix currency rates fallback --- .../src/CurrencyRateController.ts | 111 +++++++++++++++++- 1 file changed, 108 insertions(+), 3 deletions(-) diff --git a/packages/assets-controllers/src/CurrencyRateController.ts b/packages/assets-controllers/src/CurrencyRateController.ts index f4a37426b6c..4cc555b6690 100644 --- a/packages/assets-controllers/src/CurrencyRateController.ts +++ b/packages/assets-controllers/src/CurrencyRateController.ts @@ -8,12 +8,18 @@ import { FALL_BACK_VS_CURRENCY, } from '@metamask/controller-utils'; import type { Messenger } from '@metamask/messenger'; -import type { NetworkControllerGetNetworkClientByIdAction } from '@metamask/network-controller'; +import type { + NetworkControllerGetNetworkClientByIdAction, + NetworkControllerGetStateAction, + NetworkConfiguration, +} from '@metamask/network-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; +import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; import { fetchMultiExchangeRate as defaultFetchMultiExchangeRate } from './crypto-compare-service'; import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; +import { getNativeTokenAddress } from './token-prices-service/codefi-v2'; /** * currencyRates - Object keyed by native currency @@ -54,7 +60,9 @@ export type GetCurrencyRateState = ControllerGetStateAction< export type CurrencyRateControllerActions = GetCurrencyRateState; -type AllowedActions = NetworkControllerGetNetworkClientByIdAction; +type AllowedActions = + | NetworkControllerGetNetworkClientByIdAction + | NetworkControllerGetStateAction; export type CurrencyRateMessenger = Messenger< typeof name, @@ -215,7 +223,104 @@ export class CurrencyRateController extends StaticIntervalPollingController chainId(s) + const currencyToChainIds = Object.entries(nativeCurrenciesToFetch).reduce( + (acc, [nativeCurrency, fetchedCurrency]) => { + // Find chainIds that have this native currency + const chainIds = ( + Object.entries(networkConfigurations) as [ + Hex, + NetworkConfiguration, + ][] + ) + .filter( + ([, config]) => + config.nativeCurrency.toUpperCase() === + fetchedCurrency.toUpperCase(), + ) + .map(([chainId]) => chainId); + + if (chainIds.length > 0) { + acc[nativeCurrency] = { + fetchedCurrency, + chainId: chainIds[0], // Use the first matching chainId + }; + } + + return acc; + }, + {} as Record, + ); + + // Step 3: Fetch token prices for each chainId + const ratesFromTokenPrices = await Promise.all( + Object.entries(currencyToChainIds).map( + async ([nativeCurrency, { chainId }]) => { + try { + const nativeTokenAddress = getNativeTokenAddress(chainId); + const tokenPrices = + await this.#tokenPricesService.fetchTokenPrices({ + chainId, + tokenAddresses: [nativeTokenAddress], + currency: currentCurrency, + }); + + const tokenPrice = tokenPrices[nativeTokenAddress]; + + return { + nativeCurrency, + conversionDate: tokenPrice ? Date.now() / 1000 : null, + conversionRate: tokenPrice?.price ?? null, + usdConversionRate: null, // Token prices service doesn't provide USD rate in this context + }; + } catch (error) { + console.error( + `Failed to fetch token price for ${nativeCurrency} on chain ${chainId}`, + error, + ); + return { + nativeCurrency, + conversionDate: null, + conversionRate: null, + usdConversionRate: null, + }; + } + }, + ), + ); + + // Step 4: Convert to the expected format + const ratesFromTokenPricesService = ratesFromTokenPrices.reduce( + (acc, rate) => { + acc[rate.nativeCurrency] = { + conversionDate: rate.conversionDate, + conversionRate: rate.conversionRate, + usdConversionRate: rate.usdConversionRate, + }; + return acc; + }, + {} as CurrencyRateState['currencyRates'], + ); + + // If we got any rates, return them + if (Object.keys(ratesFromTokenPricesService).length > 0) { + return ratesFromTokenPricesService; + } + } catch (error) { + console.error( + 'Failed to fetch exchange rates from token prices service.', + error, + ); + } try { const fetchExchangeRateResponse = await this.fetchMultiExchangeRate( From 5e246d21475785c82b3058e49d91d76e0aac507a Mon Sep 17 00:00:00 2001 From: salimtb Date: Sat, 15 Nov 2025 21:37:28 +0100 Subject: [PATCH 02/10] fix: fix fallback on tokenRates --- .../src/TokenRatesController.ts | 161 ++++++++++-------- 1 file changed, 90 insertions(+), 71 deletions(-) diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index 8163ab3be20..fce6038cb33 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -24,7 +24,6 @@ import { createDeferredPromise, type Hex } from '@metamask/utils'; import { isEqual } from 'lodash'; import { reduceInBatchesSerially, TOKEN_PRICES_BATCH_SIZE } from './assetsUtil'; -import { fetchExchangeRate as fetchNativeCurrencyExchangeRate } from './crypto-compare-service'; import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; import { getNativeTokenAddress } from './token-prices-service/codefi-v2'; import type { @@ -166,43 +165,6 @@ export type TokenRatesControllerMessenger = Messenger< TokenRatesControllerEvents | AllowedEvents >; -/** - * Uses the CryptoCompare API to fetch the exchange rate between one currency - * and another, i.e., the multiplier to apply the amount of one currency in - * order to convert it to another. - * - * @param args - The arguments to this function. - * @param args.from - The currency to convert from. - * @param args.to - The currency to convert to. - * @returns The exchange rate between `fromCurrency` to `toCurrency` if one - * exists, or null if one does not. - */ -async function getCurrencyConversionRate({ - from, - to, -}: { - from: string; - to: string; -}) { - const includeUSDRate = false; - try { - const result = await fetchNativeCurrencyExchangeRate( - to, - from, - includeUSDRate, - ); - return result.conversionRate; - } catch (error) { - if ( - error instanceof Error && - error.message.includes('market does not exist for this coin pair') - ) { - return null; - } - throw error; - } -} - const tokenRatesControllerMetadata: StateMetadata = { marketData: { includeInStateLogs: false, @@ -775,53 +737,110 @@ export class TokenRatesController extends StaticIntervalPollingController { - const [ - contractExchangeInformations, - fallbackCurrencyToNativeCurrencyConversionRate, - ] = await Promise.all([ + // -1: First convert the chain's native token (e.g. Gnosis native xDAI) + // into the fallback currency USD + // by calling the token prices service: + // + // fetchTokenPrices({ + // tokenAddresses: [nativeTokenAddress], + // chainId, + // currency: FALL_BACK_VS_CURRENCY + // }) + // + // This gives you the price of **1 native token in USD**. + // Example: 1 xDAI = 1.002 USD. + // + // -2: Fetch all ERC-20 token prices in the fallback currency (USD) + // using the same token prices service: + // + // fetchTokenPrices({ + // tokenAddresses, + // chainId, + // currency: FALL_BACK_VS_CURRENCY + // }) + // + // This gives every token priced in USD (e.g. 1 USDC = 1.0001 USD). + // + // -3: Convert each token price from USD → native by using the native + // token's fallback price as the conversion factor. + // + // If: + // native_usd = price of 1 native token in USD + // token_usd = price of 1 token in USD + // + // Then: + // price_in_native = token_usd / native_usd + // + // Example: + // USDC price in USD = 1.0001 + // xDAI price in USD = 1.0020 + // + // USDC in xDAI = 1.0001 / 1.0020 ≈ 0.9981 xDAI + // + // -4: Apply this conversion to all fields that represent amounts + // in the fallback currency (price, marketCap, volume, highs/lows, etc.). + // + // -5: Return the final structure where all tokens are expressed in the + // chain's native currency (e.g. xDAI for Gnosis). + + const nativeTokenAddress = getNativeTokenAddress(chainId); + + // Step -1 & -2: Fetch prices in parallel + const [tokenPricesInUSD, nativeTokenPriceMap] = await Promise.all([ + // -2: All tracked tokens priced in FALL_BACK_VS_CURRENCY (e.g. USD) this.#fetchAndMapExchangeRatesForSupportedNativeCurrency({ tokenAddresses, chainId, nativeCurrency: FALL_BACK_VS_CURRENCY, }), - getCurrencyConversionRate({ - from: FALL_BACK_VS_CURRENCY, - to: nativeCurrency, + // -1: Native token priced in FALL_BACK_VS_CURRENCY (e.g. USD) + this.#fetchAndMapExchangeRatesForSupportedNativeCurrency({ + tokenAddresses: [] as Hex[], // special-case: returns only native token + chainId, + nativeCurrency: FALL_BACK_VS_CURRENCY, }), ]); - if (fallbackCurrencyToNativeCurrencyConversionRate === null) { + const nativeTokenInfo = nativeTokenPriceMap[nativeTokenAddress]; + const nativeTokenPriceInUSD = nativeTokenInfo?.price; + + if (!nativeTokenPriceInUSD || nativeTokenPriceInUSD === 0) { + // If we can't price the native token in the fallback currency, + // we can't safely convert; return empty so callers know there is no data. return {}; } - // Converts the price in the fallback currency to the native currency - const convertFallbackToNative = (value: number | undefined) => - value !== undefined && value !== null - ? value * fallbackCurrencyToNativeCurrencyConversionRate + // Step -3: Convert USD prices to native currency + // Formula: price_in_native = token_usd / native_usd + const convertUSDToNative = (valueInUSD: number | undefined) => + valueInUSD !== undefined && valueInUSD !== null + ? valueInUSD / nativeTokenPriceInUSD : undefined; - const updatedContractExchangeRates = Object.entries( - contractExchangeInformations, - ).reduce((acc, [tokenAddress, token]) => { - acc = { - ...acc, - [tokenAddress]: { - ...token, - currency: nativeCurrency, - price: convertFallbackToNative(token.price), - marketCap: convertFallbackToNative(token.marketCap), - allTimeHigh: convertFallbackToNative(token.allTimeHigh), - allTimeLow: convertFallbackToNative(token.allTimeLow), - totalVolume: convertFallbackToNative(token.totalVolume), - high1d: convertFallbackToNative(token.high1d), - low1d: convertFallbackToNative(token.low1d), - dilutedMarketCap: convertFallbackToNative(token.dilutedMarketCap), - }, - }; - return acc; - }, {}); + // Step -4 & -5: Apply conversion to all token fields and return + const tokenPricesInNative = Object.entries(tokenPricesInUSD).reduce( + (acc, [tokenAddress, tokenData]) => { + acc = { + ...acc, + [tokenAddress]: { + ...tokenData, + currency: nativeCurrency, + price: convertUSDToNative(tokenData.price), + marketCap: convertUSDToNative(tokenData.marketCap), + allTimeHigh: convertUSDToNative(tokenData.allTimeHigh), + allTimeLow: convertUSDToNative(tokenData.allTimeLow), + totalVolume: convertUSDToNative(tokenData.totalVolume), + high1d: convertUSDToNative(tokenData.high1d), + low1d: convertUSDToNative(tokenData.low1d), + dilutedMarketCap: convertUSDToNative(tokenData.dilutedMarketCap), + }, + }; + return acc; + }, + {} as ContractMarketData, + ); - return updatedContractExchangeRates; + return tokenPricesInNative; } /** From c039a3f1f4fc557cc8310adf27241281a7494aed Mon Sep 17 00:00:00 2001 From: salimtb Date: Sun, 16 Nov 2025 00:37:11 +0100 Subject: [PATCH 03/10] fix: clean up --- .../src/CurrencyRateController.test.ts | 567 ++++++++++++++++++ .../src/TokenRatesController.test.ts | 266 ++++++-- .../src/TokenRatesController.ts | 27 +- 3 files changed, 797 insertions(+), 63 deletions(-) diff --git a/packages/assets-controllers/src/CurrencyRateController.test.ts b/packages/assets-controllers/src/CurrencyRateController.test.ts index 7f520926af2..5647bf1f684 100644 --- a/packages/assets-controllers/src/CurrencyRateController.test.ts +++ b/packages/assets-controllers/src/CurrencyRateController.test.ts @@ -11,6 +11,7 @@ import { type MessengerEvents, type MockAnyNamespace, } from '@metamask/messenger'; +import type { NetworkConfiguration } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; import nock from 'nock'; import { useFakeTimers } from 'sinon'; @@ -108,6 +109,68 @@ function getCurrencyRateControllerMessenger(): CurrencyRateMessenger { return currencyRateControllerMessenger; } +/** + * Constructs a messenger for CurrencyRateController with NetworkController:getState action. + * + * @param options - Options object + * @param options.networkConfigurationsByChainId - Network configurations by chain ID + * @returns A controller messenger. + */ +function getCurrencyRateControllerMessengerWithNetworkState({ + networkConfigurationsByChainId, +}: { + networkConfigurationsByChainId: Record; +}): CurrencyRateMessenger { + const messenger: RootMessenger = new Messenger({ + namespace: MOCK_ANY_NAMESPACE, + }); + messenger.registerActionHandler( + 'NetworkController:getNetworkClientById', + jest.fn().mockImplementation((networkClientId) => { + switch (networkClientId) { + case 'mainnet': + return { + configuration: { + type: NetworkType.mainnet, + chainId: ChainId.mainnet, + ticker: NetworksTicker.mainnet, + }, + }; + case 'sepolia': + return { + configuration: { + type: NetworkType.sepolia, + chainId: ChainId.sepolia, + ticker: NetworksTicker.sepolia, + }, + }; + default: + throw new Error('Invalid networkClientId'); + } + }), + ); + messenger.registerActionHandler( + 'NetworkController:getState', + jest.fn().mockReturnValue({ networkConfigurationsByChainId }), + ); + const currencyRateControllerMessenger = new Messenger< + typeof namespace, + AllCurrencyRateControllerActions, + AllCurrencyRateControllerEvents, + RootMessenger + >({ + namespace, + }); + messenger.delegate({ + messenger: currencyRateControllerMessenger, + actions: [ + 'NetworkController:getNetworkClientById', + 'NetworkController:getState', + ], + }); + return currencyRateControllerMessenger; +} + const getStubbedDate = () => { return new Date('2019-04-07T10:20:30Z').getTime(); }; @@ -1225,6 +1288,510 @@ describe('CurrencyRateController', () => { }); }); + describe('fallback to token prices service (lines 233-316)', () => { + it('should fallback to fetchTokenPrices when fetchExchangeRates fails and crypto compare fallback also fails', async () => { + jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); + + const messenger = getCurrencyRateControllerMessengerWithNetworkState({ + networkConfigurationsByChainId: { + '0x1': { + chainId: '0x1', + nativeCurrency: 'ETH', + name: 'Ethereum Mainnet', + rpcEndpoints: [], + blockExplorerUrls: [], + defaultRpcEndpointIndex: 0, + }, + '0x89': { + chainId: '0x89', + nativeCurrency: 'POL', + name: 'Polygon', + rpcEndpoints: [], + blockExplorerUrls: [], + defaultRpcEndpointIndex: 0, + }, + }, + }); + + const tokenPricesService = buildMockTokenPricesService(); + + // Make fetchExchangeRates fail to trigger fallback + jest + .spyOn(tokenPricesService, 'fetchExchangeRates') + .mockRejectedValue(new Error('Price API failed')); + + // Mock fetchTokenPrices to return token prices + jest + .spyOn(tokenPricesService, 'fetchTokenPrices') + .mockImplementation(async ({ chainId }) => { + if (chainId === '0x1') { + return { + '0x0000000000000000000000000000000000000000': { + currency: 'usd', + tokenAddress: '0x0000000000000000000000000000000000000000', + price: 2500.5, + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: 4000, + allTimeLow: 900, + circulatingSupply: 2000, + dilutedMarketCap: 100, + high1d: 200, + low1d: 100, + marketCap: 1000, + marketCapPercentChange1d: 100, + pricePercentChange14d: 100, + pricePercentChange1h: 1, + pricePercentChange1y: 200, + pricePercentChange200d: 300, + pricePercentChange30d: 200, + pricePercentChange7d: 100, + totalVolume: 100, + }, + }; + } + if (chainId === '0x89') { + return { + '0x0000000000000000000000000000000000001010': { + currency: 'usd', + tokenAddress: '0x0000000000000000000000000000000000001010', + price: 0.85, + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: 4000, + allTimeLow: 900, + circulatingSupply: 2000, + dilutedMarketCap: 100, + high1d: 200, + low1d: 100, + marketCap: 1000, + marketCapPercentChange1d: 100, + pricePercentChange14d: 100, + pricePercentChange1h: 1, + pricePercentChange1y: 200, + pricePercentChange200d: 300, + pricePercentChange30d: 200, + pricePercentChange7d: 100, + totalVolume: 100, + }, + }; + } + return {}; + }); + + // Make crypto compare also fail by not mocking it (no nock setup) + const controller = new CurrencyRateController({ + messenger, + state: { currentCurrency: 'usd' }, + tokenPricesService, + }); + + await controller.updateExchangeRate(['ETH', 'POL']); + + const conversionDate = getStubbedDate() / 1000; + expect(controller.state).toStrictEqual({ + currentCurrency: 'usd', + currencyRates: { + ETH: { + conversionDate, + conversionRate: 2500.5, + usdConversionRate: null, // Line 283 + }, + POL: { + conversionDate, + conversionRate: 0.85, + usdConversionRate: null, + }, + }, + }); + + controller.destroy(); + }); + + it('should map native currencies to correct chain IDs (lines 236-262)', async () => { + jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); + + const messenger = getCurrencyRateControllerMessengerWithNetworkState({ + networkConfigurationsByChainId: { + '0x1': { + chainId: '0x1', + nativeCurrency: 'ETH', + name: 'Ethereum Mainnet', + rpcEndpoints: [], + blockExplorerUrls: [], + defaultRpcEndpointIndex: 0, + }, + '0xaa36a7': { + chainId: '0xaa36a7', + nativeCurrency: 'ETH', // Sepolia also uses ETH + name: 'Sepolia', + rpcEndpoints: [], + blockExplorerUrls: [], + defaultRpcEndpointIndex: 0, + }, + }, + }); + + const tokenPricesService = buildMockTokenPricesService(); + + jest + .spyOn(tokenPricesService, 'fetchExchangeRates') + .mockRejectedValue(new Error('Price API failed')); + + const fetchTokenPricesSpy = jest + .spyOn(tokenPricesService, 'fetchTokenPrices') + .mockImplementation(async ({ chainId }) => { + if (chainId === '0x1' || chainId === '0xaa36a7') { + return { + '0x0000000000000000000000000000000000000000': { + currency: 'usd', + tokenAddress: '0x0000000000000000000000000000000000000000', + price: 2500.5, + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: 4000, + allTimeLow: 900, + circulatingSupply: 2000, + dilutedMarketCap: 100, + high1d: 200, + low1d: 100, + marketCap: 1000, + marketCapPercentChange1d: 100, + pricePercentChange14d: 100, + pricePercentChange1h: 1, + pricePercentChange1y: 200, + pricePercentChange200d: 300, + pricePercentChange30d: 200, + pricePercentChange7d: 100, + totalVolume: 100, + }, + }; + } + return {}; + }); + + const controller = new CurrencyRateController({ + messenger, + state: { currentCurrency: 'usd' }, + tokenPricesService, + }); + + await controller.updateExchangeRate(['ETH']); + + // Should only call fetchTokenPrices once, using first matching chainId (line 255) + expect(fetchTokenPricesSpy).toHaveBeenCalledTimes(1); + expect(fetchTokenPricesSpy).toHaveBeenCalledWith({ + chainId: '0x1', // First chainId with ETH as native currency + tokenAddresses: ['0x0000000000000000000000000000000000000000'], + currency: 'usd', + }); + + controller.destroy(); + }); + + it('should handle errors when fetchTokenPrices fails for a specific chain (lines 285-296)', async () => { + jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); + + const messenger = getCurrencyRateControllerMessengerWithNetworkState({ + networkConfigurationsByChainId: { + '0x1': { + chainId: '0x1', + nativeCurrency: 'ETH', + name: 'Ethereum Mainnet', + rpcEndpoints: [], + blockExplorerUrls: [], + defaultRpcEndpointIndex: 0, + }, + '0x89': { + chainId: '0x89', + nativeCurrency: 'POL', + name: 'Polygon', + rpcEndpoints: [], + blockExplorerUrls: [], + defaultRpcEndpointIndex: 0, + }, + }, + }); + + const tokenPricesService = buildMockTokenPricesService(); + + jest + .spyOn(tokenPricesService, 'fetchExchangeRates') + .mockRejectedValue(new Error('Price API failed')); + + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + + jest + .spyOn(tokenPricesService, 'fetchTokenPrices') + .mockImplementation(async ({ chainId }) => { + if (chainId === '0x1') { + // ETH succeeds + return { + '0x0000000000000000000000000000000000000000': { + currency: 'usd', + tokenAddress: '0x0000000000000000000000000000000000000000', + price: 2500.5, + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: 4000, + allTimeLow: 900, + circulatingSupply: 2000, + dilutedMarketCap: 100, + high1d: 200, + low1d: 100, + marketCap: 1000, + marketCapPercentChange1d: 100, + pricePercentChange14d: 100, + pricePercentChange1h: 1, + pricePercentChange1y: 200, + pricePercentChange200d: 300, + pricePercentChange30d: 200, + pricePercentChange7d: 100, + totalVolume: 100, + }, + }; + } + // POL fails + throw new Error('Failed to fetch POL price'); + }); + + const controller = new CurrencyRateController({ + messenger, + state: { currentCurrency: 'usd' }, + tokenPricesService, + }); + + await controller.updateExchangeRate(['ETH', 'POL']); + + // Should log error for POL (line 286-289) + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to fetch token price for POL on chain 0x89', + expect.any(Error), + ); + + const conversionDate = getStubbedDate() / 1000; + expect(controller.state).toStrictEqual({ + currentCurrency: 'usd', + currencyRates: { + ETH: { + conversionDate, + conversionRate: 2500.5, + usdConversionRate: null, + }, + POL: { + conversionDate: null, // Lines 292-294: error case + conversionRate: null, + usdConversionRate: null, + }, + }, + }); + + consoleErrorSpy.mockRestore(); + controller.destroy(); + }); + + it('should set conversionDate to null when token price is not found (line 281)', async () => { + jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); + + const messenger = getCurrencyRateControllerMessengerWithNetworkState({ + networkConfigurationsByChainId: { + '0x1': { + chainId: '0x1', + nativeCurrency: 'ETH', + name: 'Ethereum Mainnet', + rpcEndpoints: [], + blockExplorerUrls: [], + defaultRpcEndpointIndex: 0, + }, + }, + }); + + const tokenPricesService = buildMockTokenPricesService(); + + jest + .spyOn(tokenPricesService, 'fetchExchangeRates') + .mockRejectedValue(new Error('Price API failed')); + + // Return empty object (no token price) + jest.spyOn(tokenPricesService, 'fetchTokenPrices').mockResolvedValue({}); + + const controller = new CurrencyRateController({ + messenger, + state: { currentCurrency: 'usd' }, + tokenPricesService, + }); + + await controller.updateExchangeRate(['ETH']); + + expect(controller.state).toStrictEqual({ + currentCurrency: 'usd', + currencyRates: { + ETH: { + conversionDate: null, // Line 281: tokenPrice is undefined + conversionRate: null, // Line 282: tokenPrice?.price ?? null + usdConversionRate: null, + }, + }, + }); + + controller.destroy(); + }); + + it('should skip currencies not found in network configurations (lines 252-257)', async () => { + jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); + + const messenger = getCurrencyRateControllerMessengerWithNetworkState({ + networkConfigurationsByChainId: { + '0x1': { + chainId: '0x1', + nativeCurrency: 'ETH', + name: 'Ethereum Mainnet', + rpcEndpoints: [], + blockExplorerUrls: [], + defaultRpcEndpointIndex: 0, + }, + }, + }); + + const tokenPricesService = buildMockTokenPricesService(); + + jest + .spyOn(tokenPricesService, 'fetchExchangeRates') + .mockRejectedValue(new Error('Price API failed')); + + const fetchTokenPricesSpy = jest + .spyOn(tokenPricesService, 'fetchTokenPrices') + .mockImplementation(async ({ chainId }) => { + if (chainId === '0x1') { + return { + '0x0000000000000000000000000000000000000000': { + currency: 'usd', + tokenAddress: '0x0000000000000000000000000000000000000000', + price: 2500.5, + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: 4000, + allTimeLow: 900, + circulatingSupply: 2000, + dilutedMarketCap: 100, + high1d: 200, + low1d: 100, + marketCap: 1000, + marketCapPercentChange1d: 100, + pricePercentChange14d: 100, + pricePercentChange1h: 1, + pricePercentChange1y: 200, + pricePercentChange200d: 300, + pricePercentChange30d: 200, + pricePercentChange7d: 100, + totalVolume: 100, + }, + }; + } + return {}; + }); + + const controller = new CurrencyRateController({ + messenger, + state: { currentCurrency: 'usd' }, + tokenPricesService, + }); + + // Request ETH (exists) and BNB (not in network configs) + await controller.updateExchangeRate(['ETH', 'BNB']); + + // Should only call fetchTokenPrices for ETH, not BNB (line 252: if chainIds.length > 0) + expect(fetchTokenPricesSpy).toHaveBeenCalledTimes(1); + expect(fetchTokenPricesSpy).toHaveBeenCalledWith({ + chainId: '0x1', + tokenAddresses: ['0x0000000000000000000000000000000000000000'], + currency: 'usd', + }); + + const conversionDate = getStubbedDate() / 1000; + expect(controller.state).toStrictEqual({ + currentCurrency: 'usd', + currencyRates: { + ETH: { + conversionDate, + conversionRate: 2500.5, + usdConversionRate: null, + }, + // BNB should not be included as it's not in network configurations + }, + }); + + controller.destroy(); + }); + + it('should use correct native token address for Polygon (line 269)', async () => { + jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); + + const messenger = getCurrencyRateControllerMessengerWithNetworkState({ + networkConfigurationsByChainId: { + '0x89': { + chainId: '0x89', + nativeCurrency: 'POL', + name: 'Polygon', + rpcEndpoints: [], + blockExplorerUrls: [], + defaultRpcEndpointIndex: 0, + }, + }, + }); + + const tokenPricesService = buildMockTokenPricesService(); + + jest + .spyOn(tokenPricesService, 'fetchExchangeRates') + .mockRejectedValue(new Error('Price API failed')); + + const fetchTokenPricesSpy = jest + .spyOn(tokenPricesService, 'fetchTokenPrices') + .mockResolvedValue({ + '0x0000000000000000000000000000000000001010': { + currency: 'usd', + tokenAddress: '0x0000000000000000000000000000000000001010', + price: 0.85, + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: 4000, + allTimeLow: 900, + circulatingSupply: 2000, + dilutedMarketCap: 100, + high1d: 200, + low1d: 100, + marketCap: 1000, + marketCapPercentChange1d: 100, + pricePercentChange14d: 100, + pricePercentChange1h: 1, + pricePercentChange1y: 200, + pricePercentChange200d: 300, + pricePercentChange30d: 200, + pricePercentChange7d: 100, + totalVolume: 100, + }, + }); + + const controller = new CurrencyRateController({ + messenger, + state: { currentCurrency: 'usd' }, + tokenPricesService, + }); + + await controller.updateExchangeRate(['POL']); + + // Should use Polygon's native token address (line 269) + expect(fetchTokenPricesSpy).toHaveBeenCalledWith({ + chainId: '0x89', + tokenAddresses: ['0x0000000000000000000000000000000000001010'], + currency: 'usd', + }); + + controller.destroy(); + }); + }); + describe('metadata', () => { it('includes expected state in debug snapshots', () => { const tokenPricesService = buildMockTokenPricesService(); diff --git a/packages/assets-controllers/src/TokenRatesController.test.ts b/packages/assets-controllers/src/TokenRatesController.test.ts index 813789d7bac..0ed2dea39d3 100644 --- a/packages/assets-controllers/src/TokenRatesController.test.ts +++ b/packages/assets-controllers/src/TokenRatesController.test.ts @@ -24,7 +24,6 @@ import type { Hex } from '@metamask/utils'; import { add0x } from '@metamask/utils'; import assert from 'assert'; import type { Patch } from 'immer'; -import nock from 'nock'; import { useFakeTimers } from 'sinon'; import { TOKEN_PRICES_BATCH_SIZE } from './assetsUtil'; @@ -1560,11 +1559,45 @@ describe('TokenRatesController', () => { describe('when the native currency is not supported', () => { const fallbackRate = 0.5; it('returns the exchange rates using ETH as a fallback currency', async () => { - nock('https://min-api.cryptocompare.com') - .get('/data/price?fsym=ETH&tsyms=LOL') - .reply(200, { LOL: fallbackRate }); + const nativeTokenPriceInUSD = 2; + // For mainnet (0x1), native token address is 0x0000...0000 + const nativeTokenAddress = + '0x0000000000000000000000000000000000000000'; const tokenPricesService = buildMockTokenPricesService({ - fetchTokenPrices: fetchTokenPricesWithIncreasingPriceForEachToken, + fetchTokenPrices: async ({ tokenAddresses, currency }) => { + // Handle native token price request (empty tokenAddresses array) + if (tokenAddresses.length === 0 && currency === 'usd') { + return { + [nativeTokenAddress]: { + tokenAddress: nativeTokenAddress, + currency: 'usd', + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: 4000, + allTimeLow: 900, + circulatingSupply: 2000, + dilutedMarketCap: 100, + high1d: 200, + low1d: 100, + marketCap: 1000, + marketCapPercentChange1d: 100, + price: nativeTokenPriceInUSD, + pricePercentChange14d: 100, + pricePercentChange1h: 1, + pricePercentChange1y: 200, + pricePercentChange200d: 300, + pricePercentChange30d: 200, + pricePercentChange7d: 100, + totalVolume: 100, + }, + }; + } + // Handle regular token prices + return fetchTokenPricesWithIncreasingPriceForEachToken({ + tokenAddresses, + currency, + }); + }, validateCurrencySupported(currency: unknown): currency is string { return currency !== 'LOL'; }, @@ -1665,27 +1698,43 @@ describe('TokenRatesController', () => { }); it('returns the an empty object when market does not exist for pair', async () => { - nock('https://min-api.cryptocompare.com') - .get('/data/price?fsym=ETH&tsyms=LOL') - .replyWithError( - new Error('market does not exist for this coin pair'), - ); - - const tokenPricesService = buildMockTokenPricesService(); + // New implementation returns empty object when native token price is unavailable + const tokenPricesService = buildMockTokenPricesService({ + fetchTokenPrices: async ({ tokenAddresses, currency }) => { + // Return empty for native token price request in USD + // This simulates the case where native token price is unavailable + if (tokenAddresses.length === 0 && currency === 'usd') { + return {}; + } + // For regular token requests, also return empty to simulate failure + if (currency === 'usd') { + return fetchTokenPricesWithIncreasingPriceForEachToken({ + tokenAddresses, + currency, + }); + } + // Should not get here since we use 'usd' as fallback + return {}; + }, + validateCurrencySupported(currency: unknown): currency is string { + return currency !== 'LOL'; + }, + }); await withController( { options: { tokenPricesService, }, - mockNetworkClientConfigurationsByNetworkClientId: { - 'AAAA-BBBB-CCCC-DDDD': buildCustomNetworkClientConfiguration({ - chainId: ChainId.mainnet, - ticker: 'LOL', - }), + mockNetworkState: { + networkConfigurationsByChainId: { + [ChainId.mainnet]: buildNetworkConfiguration({ + nativeCurrency: 'LOL', + }), + }, }, mockTokensControllerState: { allTokens: { - '0x1': { + [ChainId.mainnet]: { [defaultSelectedAddress]: [ { address: '0x02', @@ -2154,30 +2203,90 @@ describe('TokenRatesController', () => { '0x0000000000000000000000000000000000000001', '0x0000000000000000000000000000000000000002', ]; + const nativeTokenAddress = '0x0000000000000000000000000000000000001010'; + const nativeTokenPriceInUSD = 2; const tokenPricesService = buildMockTokenPricesService({ - fetchTokenPrices: jest.fn().mockResolvedValue({ - [tokenAddresses[0]]: { - currency: 'ETH', - tokenAddress: tokenAddresses[0], - price: 0.001, - }, - [tokenAddresses[1]]: { - currency: 'ETH', - tokenAddress: tokenAddresses[1], - price: 0.002, - }, - }), + // @ts-expect-error - Simplified mock for testing with partial fields + fetchTokenPrices: async ({ tokenAddresses: addrs, currency }) => { + if (addrs.length === 0 && currency === 'usd') { + // Return native token price + return { + [nativeTokenAddress]: { + currency: 'usd', + tokenAddress: nativeTokenAddress, + price: nativeTokenPriceInUSD, + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: undefined, + allTimeLow: undefined, + circulatingSupply: 0, + dilutedMarketCap: undefined, + high1d: undefined, + low1d: undefined, + marketCap: undefined, + marketCapPercentChange1d: 0, + pricePercentChange14d: 0, + pricePercentChange1h: 0, + pricePercentChange1y: 0, + pricePercentChange200d: 0, + pricePercentChange30d: 0, + pricePercentChange7d: 0, + totalVolume: undefined, + }, + }; + } + // Return token prices in USD + return { + [tokenAddresses[0]]: { + currency: 'usd', + tokenAddress: tokenAddresses[0], + price: 0.001, + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: undefined, + allTimeLow: undefined, + circulatingSupply: 0, + dilutedMarketCap: undefined, + high1d: undefined, + low1d: undefined, + marketCap: undefined, + marketCapPercentChange1d: 0, + pricePercentChange14d: 0, + pricePercentChange1h: 0, + pricePercentChange1y: 0, + pricePercentChange200d: 0, + pricePercentChange30d: 0, + pricePercentChange7d: 0, + totalVolume: undefined, + }, + [tokenAddresses[1]]: { + currency: 'usd', + tokenAddress: tokenAddresses[1], + price: 0.002, + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: undefined, + allTimeLow: undefined, + circulatingSupply: 0, + dilutedMarketCap: undefined, + high1d: undefined, + low1d: undefined, + marketCap: undefined, + marketCapPercentChange1d: 0, + pricePercentChange14d: 0, + pricePercentChange1h: 0, + pricePercentChange1y: 0, + pricePercentChange200d: 0, + pricePercentChange30d: 0, + pricePercentChange7d: 0, + totalVolume: undefined, + }, + }; + }, validateCurrencySupported(_currency: unknown): _currency is string { return false; }, }); - nock('https://min-api.cryptocompare.com') - .get('/data/price') - .query({ - fsym: 'ETH', - tsyms: selectedNetworkClientConfiguration.ticker, - }) - .reply(200, { [selectedNetworkClientConfiguration.ticker]: 0.5 }); // .5 eth to 1 matic await withController( { @@ -2227,31 +2336,51 @@ describe('TokenRatesController', () => { "0x0000000000000000000000000000000000000001": Object { "allTimeHigh": undefined, "allTimeLow": undefined, + "circulatingSupply": 0, "currency": "UNSUPPORTED", "dilutedMarketCap": undefined, "high1d": undefined, "low1d": undefined, "marketCap": undefined, + "marketCapPercentChange1d": 0, "price": 0.0005, + "priceChange1d": 0, + "pricePercentChange14d": 0, + "pricePercentChange1d": 0, + "pricePercentChange1h": 0, + "pricePercentChange1y": 0, + "pricePercentChange200d": 0, + "pricePercentChange30d": 0, + "pricePercentChange7d": 0, "tokenAddress": "0x0000000000000000000000000000000000000001", "totalVolume": undefined, }, "0x0000000000000000000000000000000000000002": Object { "allTimeHigh": undefined, "allTimeLow": undefined, + "circulatingSupply": 0, "currency": "UNSUPPORTED", "dilutedMarketCap": undefined, "high1d": undefined, "low1d": undefined, "marketCap": undefined, + "marketCapPercentChange1d": 0, "price": 0.001, + "priceChange1d": 0, + "pricePercentChange14d": 0, + "pricePercentChange1d": 0, + "pricePercentChange1h": 0, + "pricePercentChange1y": 0, + "pricePercentChange200d": 0, + "pricePercentChange30d": 0, + "pricePercentChange7d": 0, "tokenAddress": "0x0000000000000000000000000000000000000002", "totalVolume": undefined, }, }, }, } - `); + `); }, ); }); @@ -2266,8 +2395,45 @@ describe('TokenRatesController', () => { const tokenAddresses = [...new Array(200).keys()] .map(buildAddress) .sort(); + // New implementation needs native token price in USD + // For chain 999 (0x3e7), native token address is 0x0000...0000 (ZERO_ADDRESS) + const nativeTokenAddress = '0x0000000000000000000000000000000000000000'; + const nativeTokenPriceInUSD = 2; const tokenPricesService = buildMockTokenPricesService({ - fetchTokenPrices: fetchTokenPricesWithIncreasingPriceForEachToken, + fetchTokenPrices: async ({ tokenAddresses: addrs, currency }) => { + // Handle native token price request + if (addrs.length === 0 && currency === 'usd') { + return { + [nativeTokenAddress]: { + tokenAddress: nativeTokenAddress, + currency: 'usd', + pricePercentChange1d: 0, + priceChange1d: 0, + allTimeHigh: 4000, + allTimeLow: 900, + circulatingSupply: 2000, + dilutedMarketCap: 100, + high1d: 200, + low1d: 100, + marketCap: 1000, + marketCapPercentChange1d: 100, + price: nativeTokenPriceInUSD, + pricePercentChange14d: 100, + pricePercentChange1h: 1, + pricePercentChange1y: 200, + pricePercentChange200d: 300, + pricePercentChange30d: 200, + pricePercentChange7d: 100, + totalVolume: 100, + }, + }; + } + // Handle regular token prices + return fetchTokenPricesWithIncreasingPriceForEachToken({ + tokenAddresses: addrs, + currency, + }); + }, validateCurrencySupported: ( currency: unknown, ): currency is string => { @@ -2281,13 +2447,6 @@ describe('TokenRatesController', () => { const tokens = tokenAddresses.map((tokenAddress) => { return buildToken({ address: tokenAddress }); }); - nock('https://min-api.cryptocompare.com') - .get('/data/price') - .query({ - fsym: 'ETH', - tsyms: selectedNetworkClientConfiguration.ticker, - }) - .reply(200, { [selectedNetworkClientConfiguration.ticker]: 0.5 }); await withController( { options: { @@ -2333,16 +2492,25 @@ describe('TokenRatesController', () => { const numBatches = Math.ceil( tokenAddresses.length / TOKEN_PRICES_BATCH_SIZE, ); - expect(fetchTokenPricesSpy).toHaveBeenCalledTimes(numBatches); + // New implementation calls fetchTokenPrices once for native token + numBatches for tokens + expect(fetchTokenPricesSpy).toHaveBeenCalledTimes(numBatches + 1); + // First call is for native token price + expect(fetchTokenPricesSpy).toHaveBeenNthCalledWith(1, { + chainId: selectedNetworkClientConfiguration.chainId, + tokenAddresses: [], + currency: 'usd', + }); + + // Subsequent calls are for token batches in USD for (let i = 1; i <= numBatches; i++) { - expect(fetchTokenPricesSpy).toHaveBeenNthCalledWith(i, { + expect(fetchTokenPricesSpy).toHaveBeenNthCalledWith(i + 1, { chainId: selectedNetworkClientConfiguration.chainId, tokenAddresses: tokenAddresses.slice( (i - 1) * TOKEN_PRICES_BATCH_SIZE, i * TOKEN_PRICES_BATCH_SIZE, ), - currency: 'ETH', + currency: 'usd', }); } }, diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index fce6038cb33..b9a5b5e5fcb 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -11,7 +11,6 @@ import type { import { safelyExecute, toChecksumHexAddress, - FALL_BACK_VS_CURRENCY, } from '@metamask/controller-utils'; import type { Messenger } from '@metamask/messenger'; import type { @@ -785,21 +784,21 @@ export class TokenRatesController extends StaticIntervalPollingController Date: Mon, 17 Nov 2025 09:49:31 +0100 Subject: [PATCH 04/10] fix: add changelog --- packages/assets-controllers/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 53e6242bad2..6443f3746e5 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Remove fallback to CryptoCompare on `CurrencyRatesController` and `TokenRatesController` ([#7167](https://github.com/MetaMask/core/pull/7167)) + ### Fixed - Enable RPC fallback when Accounts API fails or times out in `TokenBalancesController` ([#7155](https://github.com/MetaMask/core/pull/7155)) From 3f4904d08469efaf9d3ed78e6b1b47c845c3a90a Mon Sep 17 00:00:00 2001 From: salimtb Date: Mon, 17 Nov 2025 09:57:11 +0100 Subject: [PATCH 05/10] fix: clean up --- .../src/TokenRatesController.ts | 46 ------------------- 1 file changed, 46 deletions(-) diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index b9a5b5e5fcb..a9417e39282 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -736,52 +736,6 @@ export class TokenRatesController extends StaticIntervalPollingController { - // -1: First convert the chain's native token (e.g. Gnosis native xDAI) - // into the fallback currency USD - // by calling the token prices service: - // - // fetchTokenPrices({ - // tokenAddresses: [nativeTokenAddress], - // chainId, - // currency: FALL_BACK_VS_CURRENCY - // }) - // - // This gives you the price of **1 native token in USD**. - // Example: 1 xDAI = 1.002 USD. - // - // -2: Fetch all ERC-20 token prices in the fallback currency (USD) - // using the same token prices service: - // - // fetchTokenPrices({ - // tokenAddresses, - // chainId, - // currency: FALL_BACK_VS_CURRENCY - // }) - // - // This gives every token priced in USD (e.g. 1 USDC = 1.0001 USD). - // - // -3: Convert each token price from USD → native by using the native - // token's fallback price as the conversion factor. - // - // If: - // native_usd = price of 1 native token in USD - // token_usd = price of 1 token in USD - // - // Then: - // price_in_native = token_usd / native_usd - // - // Example: - // USDC price in USD = 1.0001 - // xDAI price in USD = 1.0020 - // - // USDC in xDAI = 1.0001 / 1.0020 ≈ 0.9981 xDAI - // - // -4: Apply this conversion to all fields that represent amounts - // in the fallback currency (price, marketCap, volume, highs/lows, etc.). - // - // -5: Return the final structure where all tokens are expressed in the - // chain's native currency (e.g. xDAI for Gnosis). - const nativeTokenAddress = getNativeTokenAddress(chainId); // Step -1: First fetch native token priced in USD From 9b6beddc56fac95bd38d594fd59dd154a18d6953 Mon Sep 17 00:00:00 2001 From: salimtb Date: Mon, 17 Nov 2025 16:55:13 +0100 Subject: [PATCH 06/10] fix: fix PR comments --- .../src/CurrencyRateController.test.ts | 5 +- .../src/CurrencyRateController.ts | 86 +++++++++---------- 2 files changed, 45 insertions(+), 46 deletions(-) diff --git a/packages/assets-controllers/src/CurrencyRateController.test.ts b/packages/assets-controllers/src/CurrencyRateController.test.ts index 5647bf1f684..49e234c847b 100644 --- a/packages/assets-controllers/src/CurrencyRateController.test.ts +++ b/packages/assets-controllers/src/CurrencyRateController.test.ts @@ -1395,7 +1395,7 @@ describe('CurrencyRateController', () => { ETH: { conversionDate, conversionRate: 2500.5, - usdConversionRate: null, // Line 283 + usdConversionRate: null, }, POL: { conversionDate, @@ -1563,7 +1563,6 @@ describe('CurrencyRateController', () => { await controller.updateExchangeRate(['ETH', 'POL']); - // Should log error for POL (line 286-289) expect(consoleErrorSpy).toHaveBeenCalledWith( 'Failed to fetch token price for POL on chain 0x89', expect.any(Error), @@ -1579,7 +1578,7 @@ describe('CurrencyRateController', () => { usdConversionRate: null, }, POL: { - conversionDate: null, // Lines 292-294: error case + conversionDate: null, conversionRate: null, usdConversionRate: null, }, diff --git a/packages/assets-controllers/src/CurrencyRateController.ts b/packages/assets-controllers/src/CurrencyRateController.ts index 4cc555b6690..2f4d06f9071 100644 --- a/packages/assets-controllers/src/CurrencyRateController.ts +++ b/packages/assets-controllers/src/CurrencyRateController.ts @@ -17,9 +17,9 @@ import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; +import { reduceInBatchesSerially } from './assetsUtil'; import { fetchMultiExchangeRate as defaultFetchMultiExchangeRate } from './crypto-compare-service'; import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; -import { getNativeTokenAddress } from './token-prices-service/codefi-v2'; /** * currencyRates - Object keyed by native currency @@ -96,6 +96,9 @@ const defaultState = { }, }; +// Maximum number of cryptocurrencies that can be sent to the exchange rates API in a single request +const EXCHANGE_RATES_BATCH_SIZE = 20; + /** The input to start polling for the {@link CurrencyRateController} */ type CurrencyRatePollingInput = { nativeCurrencies: string[]; @@ -261,50 +264,47 @@ export class CurrencyRateController extends StaticIntervalPollingController, ); - // Step 3: Fetch token prices for each chainId - const ratesFromTokenPrices = await Promise.all( - Object.entries(currencyToChainIds).map( - async ([nativeCurrency, { chainId }]) => { - try { - const nativeTokenAddress = getNativeTokenAddress(chainId); - const tokenPrices = - await this.#tokenPricesService.fetchTokenPrices({ - chainId, - tokenAddresses: [nativeTokenAddress], - currency: currentCurrency, - }); - - const tokenPrice = tokenPrices[nativeTokenAddress]; - - return { - nativeCurrency, - conversionDate: tokenPrice ? Date.now() / 1000 : null, - conversionRate: tokenPrice?.price ?? null, - usdConversionRate: null, // Token prices service doesn't provide USD rate in this context - }; - } catch (error) { - console.error( - `Failed to fetch token price for ${nativeCurrency} on chain ${chainId}`, - error, - ); - return { - nativeCurrency, - conversionDate: null, - conversionRate: null, - usdConversionRate: null, - }; - } - }, - ), - ); + // Step 3: Fetch exchange rates for all native currencies using batch processing + const nativeCurrencies = Object.keys(currencyToChainIds); + + type ExchangeRatesResult = Awaited< + ReturnType + >; + + const allExchangeRates = await reduceInBatchesSerially< + string, + ExchangeRatesResult + >({ + values: nativeCurrencies, + batchSize: EXCHANGE_RATES_BATCH_SIZE, + eachBatch: async (workingResult, batch) => { + try { + const batchRates = + await this.#tokenPricesService.fetchExchangeRates({ + baseCurrency: currentCurrency, + includeUsdRate: true, + cryptocurrencies: batch, + }); + return { ...(workingResult || {}), ...batchRates }; + } catch (error) { + console.error( + `Failed to fetch exchange rates for currencies: ${batch.join(', ')}`, + error, + ); + return workingResult || {}; + } + }, + initialResult: {}, + }); // Step 4: Convert to the expected format - const ratesFromTokenPricesService = ratesFromTokenPrices.reduce( - (acc, rate) => { - acc[rate.nativeCurrency] = { - conversionDate: rate.conversionDate, - conversionRate: rate.conversionRate, - usdConversionRate: rate.usdConversionRate, + const ratesFromTokenPricesService = nativeCurrencies.reduce( + (acc, nativeCurrency) => { + const rate = allExchangeRates[nativeCurrency.toLowerCase()]; + acc[nativeCurrency] = { + conversionDate: rate ? Date.now() / 1000 : null, + conversionRate: rate?.value ?? null, + usdConversionRate: rate?.usd ?? null, }; return acc; }, From c007d25a8de377a8c07599cf79b1184e77c34502 Mon Sep 17 00:00:00 2001 From: salimtb Date: Mon, 17 Nov 2025 17:35:59 +0100 Subject: [PATCH 07/10] fix: fix PR comments --- .../src/CurrencyRateController.ts | 86 +++++++++---------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/packages/assets-controllers/src/CurrencyRateController.ts b/packages/assets-controllers/src/CurrencyRateController.ts index 2f4d06f9071..4cc555b6690 100644 --- a/packages/assets-controllers/src/CurrencyRateController.ts +++ b/packages/assets-controllers/src/CurrencyRateController.ts @@ -17,9 +17,9 @@ import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; -import { reduceInBatchesSerially } from './assetsUtil'; import { fetchMultiExchangeRate as defaultFetchMultiExchangeRate } from './crypto-compare-service'; import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; +import { getNativeTokenAddress } from './token-prices-service/codefi-v2'; /** * currencyRates - Object keyed by native currency @@ -96,9 +96,6 @@ const defaultState = { }, }; -// Maximum number of cryptocurrencies that can be sent to the exchange rates API in a single request -const EXCHANGE_RATES_BATCH_SIZE = 20; - /** The input to start polling for the {@link CurrencyRateController} */ type CurrencyRatePollingInput = { nativeCurrencies: string[]; @@ -264,47 +261,50 @@ export class CurrencyRateController extends StaticIntervalPollingController, ); - // Step 3: Fetch exchange rates for all native currencies using batch processing - const nativeCurrencies = Object.keys(currencyToChainIds); - - type ExchangeRatesResult = Awaited< - ReturnType - >; - - const allExchangeRates = await reduceInBatchesSerially< - string, - ExchangeRatesResult - >({ - values: nativeCurrencies, - batchSize: EXCHANGE_RATES_BATCH_SIZE, - eachBatch: async (workingResult, batch) => { - try { - const batchRates = - await this.#tokenPricesService.fetchExchangeRates({ - baseCurrency: currentCurrency, - includeUsdRate: true, - cryptocurrencies: batch, - }); - return { ...(workingResult || {}), ...batchRates }; - } catch (error) { - console.error( - `Failed to fetch exchange rates for currencies: ${batch.join(', ')}`, - error, - ); - return workingResult || {}; - } - }, - initialResult: {}, - }); + // Step 3: Fetch token prices for each chainId + const ratesFromTokenPrices = await Promise.all( + Object.entries(currencyToChainIds).map( + async ([nativeCurrency, { chainId }]) => { + try { + const nativeTokenAddress = getNativeTokenAddress(chainId); + const tokenPrices = + await this.#tokenPricesService.fetchTokenPrices({ + chainId, + tokenAddresses: [nativeTokenAddress], + currency: currentCurrency, + }); + + const tokenPrice = tokenPrices[nativeTokenAddress]; + + return { + nativeCurrency, + conversionDate: tokenPrice ? Date.now() / 1000 : null, + conversionRate: tokenPrice?.price ?? null, + usdConversionRate: null, // Token prices service doesn't provide USD rate in this context + }; + } catch (error) { + console.error( + `Failed to fetch token price for ${nativeCurrency} on chain ${chainId}`, + error, + ); + return { + nativeCurrency, + conversionDate: null, + conversionRate: null, + usdConversionRate: null, + }; + } + }, + ), + ); // Step 4: Convert to the expected format - const ratesFromTokenPricesService = nativeCurrencies.reduce( - (acc, nativeCurrency) => { - const rate = allExchangeRates[nativeCurrency.toLowerCase()]; - acc[nativeCurrency] = { - conversionDate: rate ? Date.now() / 1000 : null, - conversionRate: rate?.value ?? null, - usdConversionRate: rate?.usd ?? null, + const ratesFromTokenPricesService = ratesFromTokenPrices.reduce( + (acc, rate) => { + acc[rate.nativeCurrency] = { + conversionDate: rate.conversionDate, + conversionRate: rate.conversionRate, + usdConversionRate: rate.usdConversionRate, }; return acc; }, From 8647bc1cadee5a8024f81d3c7509e2dd7e971655 Mon Sep 17 00:00:00 2001 From: salimtb Date: Mon, 17 Nov 2025 17:48:22 +0100 Subject: [PATCH 08/10] fix: fix PR comments --- .../src/CurrencyRateController.ts | 66 ++++++++++--------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/packages/assets-controllers/src/CurrencyRateController.ts b/packages/assets-controllers/src/CurrencyRateController.ts index 4cc555b6690..41c8ac49ffb 100644 --- a/packages/assets-controllers/src/CurrencyRateController.ts +++ b/packages/assets-controllers/src/CurrencyRateController.ts @@ -262,42 +262,48 @@ export class CurrencyRateController extends StaticIntervalPollingController { - try { - const nativeTokenAddress = getNativeTokenAddress(chainId); - const tokenPrices = - await this.#tokenPricesService.fetchTokenPrices({ - chainId, - tokenAddresses: [nativeTokenAddress], - currency: currentCurrency, - }); - - const tokenPrice = tokenPrices[nativeTokenAddress]; - - return { - nativeCurrency, - conversionDate: tokenPrice ? Date.now() / 1000 : null, - conversionRate: tokenPrice?.price ?? null, - usdConversionRate: null, // Token prices service doesn't provide USD rate in this context - }; - } catch (error) { - console.error( - `Failed to fetch token price for ${nativeCurrency} on chain ${chainId}`, - error, - ); - return { - nativeCurrency, - conversionDate: null, - conversionRate: null, - usdConversionRate: null, - }; - } + const nativeTokenAddress = getNativeTokenAddress(chainId); + const tokenPrices = await this.#tokenPricesService.fetchTokenPrices( + { + chainId, + tokenAddresses: [nativeTokenAddress], + currency: currentCurrency, + }, + ); + + const tokenPrice = tokenPrices[nativeTokenAddress]; + + return { + nativeCurrency, + conversionDate: tokenPrice ? Date.now() / 1000 : null, + conversionRate: tokenPrice?.price ?? null, + usdConversionRate: null, // Token prices service doesn't provide USD rate in this context + }; }, ), ); + const ratesFromTokenPrices = ratesResults.map((result, index) => { + const [nativeCurrency, { chainId }] = + Object.entries(currencyToChainIds)[index]; + if (result.status === 'fulfilled') { + return result.value; + } + console.error( + `Failed to fetch token price for ${nativeCurrency} on chain ${chainId}`, + result.reason, + ); + return { + nativeCurrency, + conversionDate: null, + conversionRate: null, + usdConversionRate: null, + }; + }); + // Step 4: Convert to the expected format const ratesFromTokenPricesService = ratesFromTokenPrices.reduce( (acc, rate) => { From 4786783ee4a48ce15969f35f0c6c13b3b7a24a02 Mon Sep 17 00:00:00 2001 From: salimtb Date: Tue, 18 Nov 2025 00:14:06 +0100 Subject: [PATCH 09/10] fix: fix PR comments --- packages/assets-controllers/CHANGELOG.md | 2 +- .../src/CurrencyRateController.test.ts | 277 +++--------------- .../src/CurrencyRateController.ts | 60 ++-- 3 files changed, 64 insertions(+), 275 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index f06936fc123..9da3bbcefb2 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Remove fallback to CryptoCompare on `CurrencyRatesController` and `TokenRatesController` ([#7167](https://github.com/MetaMask/core/pull/7167)) +- **BREAKING:** Remove fallback to CryptoCompare on `CurrencyRatesController` and `TokenRatesController` ([#7167](https://github.com/MetaMask/core/pull/7167)) - Bump `@metamask/core-backend` from `^4.0.0` to `^4.1.0` ### Fixed diff --git a/packages/assets-controllers/src/CurrencyRateController.test.ts b/packages/assets-controllers/src/CurrencyRateController.test.ts index 49e234c847b..8af5b63fd8f 100644 --- a/packages/assets-controllers/src/CurrencyRateController.test.ts +++ b/packages/assets-controllers/src/CurrencyRateController.test.ts @@ -13,7 +13,6 @@ import { } from '@metamask/messenger'; import type { NetworkConfiguration } from '@metamask/network-controller'; import type { Hex } from '@metamask/utils'; -import nock from 'nock'; import { useFakeTimers } from 'sinon'; import type { CurrencyRateMessenger } from './CurrencyRateController'; @@ -232,19 +231,21 @@ describe('CurrencyRateController', () => { }); it('should not poll before being started', async () => { - const fetchMultiExchangeRateStub = jest.fn(); const tokenPricesService = buildMockTokenPricesService(); + const fetchExchangeRatesSpy = jest.spyOn( + tokenPricesService, + 'fetchExchangeRates', + ); const messenger = getCurrencyRateControllerMessenger(); const controller = new CurrencyRateController({ interval: 100, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, tokenPricesService, }); await advanceTime({ clock, duration: 200 }); - expect(fetchMultiExchangeRateStub).not.toHaveBeenCalled(); + expect(fetchExchangeRatesSpy).not.toHaveBeenCalled(); controller.destroy(); }); @@ -256,17 +257,6 @@ describe('CurrencyRateController', () => { .spyOn(global.Date, 'now') .mockReturnValueOnce(10000) .mockReturnValueOnce(20000); - const fetchMultiExchangeRateStub = jest - .fn() - .mockResolvedValueOnce({ - eth: { [currentCurrency]: 1, usd: 11 }, - }) - .mockResolvedValueOnce({ - eth: { - [currentCurrency]: 2, - usd: 22, - }, - }); const tokenPricesService = buildMockTokenPricesService(); const fetchExchangeRatesSpy = jest @@ -282,7 +272,6 @@ describe('CurrencyRateController', () => { const messenger = getCurrencyRateControllerMessenger(); const controller = new CurrencyRateController({ interval: 100, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency }, tokenPricesService, @@ -290,7 +279,6 @@ describe('CurrencyRateController', () => { controller.startPolling({ nativeCurrencies: ['ETH'] }); await advanceTime({ clock, duration: 0 }); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); expect(controller.state.currencyRates).toStrictEqual({ @@ -302,12 +290,10 @@ describe('CurrencyRateController', () => { }); await advanceTime({ clock, duration: 99 }); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); await advanceTime({ clock, duration: 1 }); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(2); expect(controller.state.currencyRates).toStrictEqual({ ETH: { @@ -321,7 +307,6 @@ describe('CurrencyRateController', () => { }); it('should not poll after being stopped', async () => { - const fetchMultiExchangeRateStub = jest.fn(); const tokenPricesService = buildMockTokenPricesService(); const fetchExchangeRatesSpy = jest @@ -337,7 +322,6 @@ describe('CurrencyRateController', () => { const messenger = getCurrencyRateControllerMessenger(); const controller = new CurrencyRateController({ interval: 100, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, tokenPricesService, }); @@ -349,20 +333,16 @@ describe('CurrencyRateController', () => { controller.stopAllPolling(); // called once upon initial start - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); await advanceTime({ clock, duration: 150, stepSize: 50 }); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); controller.destroy(); }); it('should poll correctly after being started, stopped, and started again', async () => { - const fetchMultiExchangeRateStub = jest.fn(); - const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); @@ -378,7 +358,6 @@ describe('CurrencyRateController', () => { }); const controller = new CurrencyRateController({ interval: 100, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, tokenPricesService, }); @@ -388,18 +367,15 @@ describe('CurrencyRateController', () => { controller.stopAllPolling(); // called once upon initial start - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); controller.startPolling({ nativeCurrencies: ['ETH'] }); await advanceTime({ clock, duration: 0 }); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(2); await advanceTime({ clock, duration: 100 }); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(3); }); @@ -407,9 +383,6 @@ describe('CurrencyRateController', () => { const currentCurrency = 'cad'; jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); - const fetchMultiExchangeRateStub = jest - .fn() - .mockResolvedValue({ eth: { [currentCurrency]: 10, usd: 111 } }); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); const fetchExchangeRatesSpy = jest @@ -425,7 +398,6 @@ describe('CurrencyRateController', () => { }); const controller = new CurrencyRateController({ interval: 10, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency }, tokenPricesService, @@ -442,8 +414,6 @@ describe('CurrencyRateController', () => { await controller.updateExchangeRate(['ETH']); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); - expect(controller.state.currencyRates).toStrictEqual({ ETH: { conversionDate: getStubbedDate() / 1000, @@ -459,25 +429,6 @@ describe('CurrencyRateController', () => { const currentCurrency = 'cad'; jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); - const fetchMultiExchangeRateStub = jest - .fn() - .mockImplementation((_, cryptocurrencies) => { - const nativeCurrency = cryptocurrencies[0]; - if (nativeCurrency === 'ETH') { - return { - [nativeCurrency.toLowerCase()]: { - [currentCurrency.toLowerCase()]: 10, - usd: 110, - }, - }; - } - return { - [nativeCurrency.toLowerCase()]: { - [currentCurrency.toLowerCase()]: 0, - usd: 100, - }, - }; - }); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); @@ -494,7 +445,6 @@ describe('CurrencyRateController', () => { }, }); const controller = new CurrencyRateController({ - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency }, tokenPricesService, @@ -511,8 +461,6 @@ describe('CurrencyRateController', () => { await controller.updateExchangeRate(['SepoliaETH']); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); - expect(controller.state.currencyRates).toStrictEqual({ ETH: { conversionDate: 0, @@ -532,10 +480,6 @@ describe('CurrencyRateController', () => { it('should update current currency then clear and refetch rates', async () => { const currentCurrency = 'cad'; jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); - const fetchMultiExchangeRateStub = jest.fn().mockResolvedValue({ - eth: { [currentCurrency]: 10, usd: 11 }, - btc: { [currentCurrency]: 10, usd: 11 }, - }); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); jest.spyOn(tokenPricesService, 'fetchExchangeRates').mockResolvedValue({ @@ -556,7 +500,6 @@ describe('CurrencyRateController', () => { }); const controller = new CurrencyRateController({ interval: 10, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currencyRates: { @@ -609,7 +552,6 @@ describe('CurrencyRateController', () => { controller.destroy(); }); it('should add usd rate to state when includeUsdRate is configured true', async () => { - const fetchMultiExchangeRateStub = jest.fn().mockResolvedValue({}); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); const fetchExchangeRatesSpy = jest @@ -625,14 +567,12 @@ describe('CurrencyRateController', () => { }); const controller = new CurrencyRateController({ includeUsdRate: true, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency: 'xyz' }, tokenPricesService, }); await controller.updateExchangeRate(['SepoliaETH']); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy.mock.calls).toMatchObject([ [ { @@ -686,16 +626,7 @@ describe('CurrencyRateController', () => { controller.destroy(); }); - it('should throw unexpected errors when both price api and crypto-compare fail', async () => { - const cryptoCompareHost = 'https://min-api.cryptocompare.com'; - nock(cryptoCompareHost) - .get('/data/pricemulti?fsyms=ETH&tsyms=xyz') - .reply(200, { - Response: 'Error', - Message: 'this method has been deprecated', - }) - .persist(); - + it('should return null state when price api fails', async () => { const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); const fetchExchangeRatesSpy = jest @@ -707,22 +638,24 @@ describe('CurrencyRateController', () => { tokenPricesService, }); - await expect(controller.updateExchangeRate(['ETH'])).rejects.toThrow( - 'this method has been deprecated', - ); + await controller.updateExchangeRate(['ETH']); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); + expect(controller.state).toStrictEqual({ + currentCurrency: 'xyz', + currencyRates: { + ETH: { + conversionDate: null, + conversionRate: null, + usdConversionRate: null, + }, + }, + }); controller.destroy(); }); - it('should not update state on unexpected / transient errors', async () => { - const cryptoCompareHost = 'https://min-api.cryptocompare.com'; - nock(cryptoCompareHost) - .get('/data/pricemulti?fsyms=ETH&tsyms=xyz') - .reply(500) // HTTP 500 transient error - .persist(); - + it('should update state with null values when price api fails', async () => { const state = { currentCurrency: 'xyz', currencyRates: { @@ -744,13 +677,19 @@ describe('CurrencyRateController', () => { tokenPricesService, }); - // Error should still be thrown - await expect(controller.updateExchangeRate(['ETH'])).rejects.toThrow( - `Fetch failed with status '500' for request 'https://min-api.cryptocompare.com/data/pricemulti?fsyms=ETH&tsyms=xyz'`, - ); + await controller.updateExchangeRate(['ETH']); - // But state should not be changed - expect(controller.state).toStrictEqual(state); + // State should be updated with null values + expect(controller.state).toStrictEqual({ + currentCurrency: 'xyz', + currencyRates: { + ETH: { + conversionDate: null, + conversionRate: null, + usdConversionRate: null, + }, + }, + }); controller.destroy(); }); @@ -817,72 +756,21 @@ describe('CurrencyRateController', () => { controller.destroy(); }); - it('fallback to crypto compare when price api fails and fetches exchange rates for multiple native currencies', async () => { - jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); - const cryptoCompareHost = 'https://min-api.cryptocompare.com'; - nock(cryptoCompareHost) - .get('/data/pricemulti?fsyms=ETH,POL,BNB&tsyms=xyz') - .reply(200, { - BNB: { XYZ: 500.1 }, - ETH: { XYZ: 4000.42 }, - POL: { XYZ: 0.3 }, - }) - .persist(); - const messenger = getCurrencyRateControllerMessenger(); - const tokenPricesService = buildMockTokenPricesService(); - jest - .spyOn(tokenPricesService, 'fetchExchangeRates') - .mockRejectedValue(new Error('Failed to fetch')); - const controller = new CurrencyRateController({ - messenger, - state: { currentCurrency: 'xyz' }, - tokenPricesService, - }); - - await controller.updateExchangeRate(['ETH', 'POL', 'BNB']); - - const conversionDate = getStubbedDate() / 1000; - expect(controller.state).toStrictEqual({ - currentCurrency: 'xyz', - currencyRates: { - BNB: { - conversionDate, - conversionRate: 500.1, - usdConversionRate: null, - }, - ETH: { - conversionDate, - conversionRate: 4000.42, - usdConversionRate: null, - }, - POL: { - conversionDate, - conversionRate: 0.3, - usdConversionRate: null, - }, - }, - }); - - controller.destroy(); - }); - - it('skips updating empty or undefined native currencies when calling crypto compare', async () => { + it('skips updating empty or undefined native currencies', async () => { jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); - const cryptoCompareHost = 'https://min-api.cryptocompare.com'; - nock(cryptoCompareHost) - .get('/data/pricemulti?fsyms=ETH&tsyms=xyz') // fsyms query only includes non-empty native currencies - .reply(200, { - ETH: { XYZ: 1000 }, - }) - .persist(); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); - jest - .spyOn(tokenPricesService, 'fetchExchangeRates') - .mockRejectedValue(new Error('Failed to fetch')); + jest.spyOn(tokenPricesService, 'fetchExchangeRates').mockResolvedValue({ + eth: { + name: 'Ethereum', + ticker: 'eth', + value: 0, + currencyType: 'crypto', + }, + }); const controller = new CurrencyRateController({ messenger, state: { currentCurrency: 'xyz' }, @@ -899,7 +787,7 @@ describe('CurrencyRateController', () => { currencyRates: { ETH: { conversionDate, - conversionRate: 1000, + conversionRate: null, usdConversionRate: null, }, }, @@ -995,61 +883,12 @@ describe('CurrencyRateController', () => { controller.destroy(); }); - it('should set conversionDate to null when currency not found in crypto compare response (lines 231-232)', async () => { - jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); - const cryptoCompareHost = 'https://min-api.cryptocompare.com'; - nock(cryptoCompareHost) - .get('/data/pricemulti?fsyms=ETH,BNB&tsyms=xyz') - .reply(200, { - ETH: { XYZ: 4000.42 }, - // BNB is missing from the response - }) - .persist(); - - const messenger = getCurrencyRateControllerMessenger(); - const tokenPricesService = buildMockTokenPricesService(); - - // Make price API fail so it falls back to CryptoCompare - jest - .spyOn(tokenPricesService, 'fetchExchangeRates') - .mockRejectedValue(new Error('Failed to fetch')); - - const controller = new CurrencyRateController({ - messenger, - state: { currentCurrency: 'xyz' }, - tokenPricesService, - }); - - await controller.updateExchangeRate(['ETH', 'BNB']); - - const conversionDate = getStubbedDate() / 1000; - expect(controller.state).toStrictEqual({ - currentCurrency: 'xyz', - currencyRates: { - ETH: { - conversionDate, - conversionRate: 4000.42, - usdConversionRate: null, - }, - BNB: { - conversionDate: null, // Line 231: rate === undefined - conversionRate: null, // Line 232 - usdConversionRate: null, - }, - }, - }); - - controller.destroy(); - }); - describe('useExternalServices', () => { it('should not fetch exchange rates when useExternalServices is false', async () => { - const fetchMultiExchangeRateStub = jest.fn(); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); const controller = new CurrencyRateController({ useExternalServices: () => false, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency: 'usd' }, tokenPricesService, @@ -1057,7 +896,6 @@ describe('CurrencyRateController', () => { await controller.updateExchangeRate(['ETH']); - expect(fetchMultiExchangeRateStub).not.toHaveBeenCalled(); expect(controller.state.currencyRates).toStrictEqual({ ETH: { conversionDate: 0, @@ -1070,13 +908,15 @@ describe('CurrencyRateController', () => { }); it('should not poll when useExternalServices is false', async () => { - const fetchMultiExchangeRateStub = jest.fn(); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); + const fetchExchangeRatesSpy = jest.spyOn( + tokenPricesService, + 'fetchExchangeRates', + ); const controller = new CurrencyRateController({ useExternalServices: () => false, interval: 100, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency: 'usd' }, tokenPricesService, @@ -1085,22 +925,20 @@ describe('CurrencyRateController', () => { controller.startPolling({ nativeCurrencies: ['ETH'] }); await advanceTime({ clock, duration: 0 }); - expect(fetchMultiExchangeRateStub).not.toHaveBeenCalled(); + expect(fetchExchangeRatesSpy).not.toHaveBeenCalled(); await advanceTime({ clock, duration: 100 }); - expect(fetchMultiExchangeRateStub).not.toHaveBeenCalled(); + expect(fetchExchangeRatesSpy).not.toHaveBeenCalled(); controller.destroy(); }); it('should not fetch exchange rates when useExternalServices is false even with multiple currencies', async () => { - const fetchMultiExchangeRateStub = jest.fn(); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); const controller = new CurrencyRateController({ useExternalServices: () => false, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency: 'eur' }, tokenPricesService, @@ -1108,7 +946,6 @@ describe('CurrencyRateController', () => { await controller.updateExchangeRate(['ETH', 'BTC', 'BNB']); - expect(fetchMultiExchangeRateStub).not.toHaveBeenCalled(); expect(controller.state.currencyRates).toStrictEqual({ ETH: { conversionDate: 0, @@ -1121,12 +958,10 @@ describe('CurrencyRateController', () => { }); it('should not fetch exchange rates when useExternalServices is false even with testnet currencies', async () => { - const fetchMultiExchangeRateStub = jest.fn(); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); const controller = new CurrencyRateController({ useExternalServices: () => false, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency: 'cad' }, tokenPricesService, @@ -1134,7 +969,6 @@ describe('CurrencyRateController', () => { await controller.updateExchangeRate(['SepoliaETH', 'GoerliETH']); - expect(fetchMultiExchangeRateStub).not.toHaveBeenCalled(); expect(controller.state.currencyRates).toStrictEqual({ ETH: { conversionDate: 0, @@ -1147,13 +981,11 @@ describe('CurrencyRateController', () => { }); it('should not fetch exchange rates when useExternalServices is false even with includeUsdRate true', async () => { - const fetchMultiExchangeRateStub = jest.fn(); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); const controller = new CurrencyRateController({ useExternalServices: () => false, includeUsdRate: true, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency: 'jpy' }, tokenPricesService, @@ -1161,7 +993,6 @@ describe('CurrencyRateController', () => { await controller.updateExchangeRate(['ETH']); - expect(fetchMultiExchangeRateStub).not.toHaveBeenCalled(); expect(controller.state.currencyRates).toStrictEqual({ ETH: { conversionDate: 0, @@ -1175,9 +1006,6 @@ describe('CurrencyRateController', () => { it('should fetch exchange rates when useExternalServices is true (default behavior)', async () => { jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); - const fetchMultiExchangeRateStub = jest - .fn() - .mockResolvedValue({ eth: { usd: 2000, eur: 1800 } }); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); const fetchExchangeRatesSpy = jest @@ -1193,7 +1021,6 @@ describe('CurrencyRateController', () => { }); const controller = new CurrencyRateController({ useExternalServices: () => true, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency: 'eur' }, tokenPricesService, @@ -1201,7 +1028,6 @@ describe('CurrencyRateController', () => { await controller.updateExchangeRate(['ETH']); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); expect(fetchExchangeRatesSpy).toHaveBeenCalledWith({ baseCurrency: 'eur', @@ -1221,9 +1047,6 @@ describe('CurrencyRateController', () => { it('should default useExternalServices to true when not specified', async () => { jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); - const fetchMultiExchangeRateStub = jest - .fn() - .mockResolvedValue({ eth: { usd: 2000, gbp: 1600 } }); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); @@ -1239,7 +1062,6 @@ describe('CurrencyRateController', () => { }, }); const controller = new CurrencyRateController({ - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency: 'gbp' }, tokenPricesService, @@ -1247,7 +1069,6 @@ describe('CurrencyRateController', () => { await controller.updateExchangeRate(['ETH']); - expect(fetchMultiExchangeRateStub).toHaveBeenCalledTimes(0); expect(fetchExchangeRatesSpy).toHaveBeenCalledTimes(1); expect(fetchExchangeRatesSpy).toHaveBeenCalledWith({ baseCurrency: 'gbp', @@ -1266,14 +1087,10 @@ describe('CurrencyRateController', () => { }); it('should not throw errors when useExternalServices is false even if fetchMultiExchangeRate would fail', async () => { - const fetchMultiExchangeRateStub = jest - .fn() - .mockRejectedValue(new Error('API Error')); const messenger = getCurrencyRateControllerMessenger(); const tokenPricesService = buildMockTokenPricesService(); const controller = new CurrencyRateController({ useExternalServices: () => false, - fetchMultiExchangeRate: fetchMultiExchangeRateStub, messenger, state: { currentCurrency: 'usd' }, tokenPricesService, @@ -1282,8 +1099,6 @@ describe('CurrencyRateController', () => { // Should not throw an error expect(await controller.updateExchangeRate(['ETH'])).toBeUndefined(); - expect(fetchMultiExchangeRateStub).not.toHaveBeenCalled(); - controller.destroy(); }); }); diff --git a/packages/assets-controllers/src/CurrencyRateController.ts b/packages/assets-controllers/src/CurrencyRateController.ts index 41c8ac49ffb..4c561449d06 100644 --- a/packages/assets-controllers/src/CurrencyRateController.ts +++ b/packages/assets-controllers/src/CurrencyRateController.ts @@ -17,7 +17,6 @@ import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; -import { fetchMultiExchangeRate as defaultFetchMultiExchangeRate } from './crypto-compare-service'; import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; import { getNativeTokenAddress } from './token-prices-service/codefi-v2'; @@ -112,8 +111,6 @@ export class CurrencyRateController extends StaticIntervalPollingController { private readonly mutex = new Mutex(); - private readonly fetchMultiExchangeRate; - private readonly includeUsdRate; private readonly useExternalServices: () => boolean; @@ -129,7 +126,6 @@ export class CurrencyRateController extends StaticIntervalPollingController true, messenger, state, - fetchMultiExchangeRate = defaultFetchMultiExchangeRate, tokenPricesService, }: { includeUsdRate?: boolean; @@ -146,7 +141,6 @@ export class CurrencyRateController extends StaticIntervalPollingController; useExternalServices?: () => boolean; - fetchMultiExchangeRate?: typeof defaultFetchMultiExchangeRate; tokenPricesService: AbstractTokenPricesService; }) { super({ @@ -158,7 +152,6 @@ export class CurrencyRateController extends StaticIntervalPollingController chainId(s) const currencyToChainIds = Object.entries(nativeCurrenciesToFetch).reduce( (acc, [nativeCurrency, fetchedCurrency]) => { - // Find chainIds that have this native currency - const chainIds = ( + // Find the first chainId that has this native currency + const matchingEntry = ( Object.entries(networkConfigurations) as [ Hex, NetworkConfiguration, ][] - ) - .filter( - ([, config]) => - config.nativeCurrency.toUpperCase() === - fetchedCurrency.toUpperCase(), - ) - .map(([chainId]) => chainId); - - if (chainIds.length > 0) { + ).find( + ([, config]) => + config.nativeCurrency.toUpperCase() === + fetchedCurrency.toUpperCase(), + ); + + if (matchingEntry) { acc[nativeCurrency] = { fetchedCurrency, - chainId: chainIds[0], // Use the first matching chainId + chainId: matchingEntry[0], }; } @@ -317,41 +308,24 @@ export class CurrencyRateController extends StaticIntervalPollingController 0) { - return ratesFromTokenPricesService; - } + return ratesFromTokenPricesService; } catch (error) { console.error( 'Failed to fetch exchange rates from token prices service.', error, ); - } - - try { - const fetchExchangeRateResponse = await this.fetchMultiExchangeRate( - currentCurrency, - [...new Set(Object.values(nativeCurrenciesToFetch))], - this.includeUsdRate, - ); - - const rates = Object.entries(nativeCurrenciesToFetch).reduce( - (acc, [nativeCurrency, fetchedCurrency]) => { - const rate = fetchExchangeRateResponse[fetchedCurrency.toLowerCase()]; + // Return null state for all requested currencies + return Object.keys(nativeCurrenciesToFetch).reduce( + (acc, nativeCurrency) => { acc[nativeCurrency] = { - conversionDate: rate !== undefined ? Date.now() / 1000 : null, - conversionRate: rate?.[currentCurrency.toLowerCase()] ?? null, - usdConversionRate: rate?.usd ?? null, + conversionDate: null, + conversionRate: null, + usdConversionRate: null, }; return acc; }, {} as CurrencyRateState['currencyRates'], ); - - return rates; - } catch (error) { - console.error('Failed to fetch exchange rates.', error); - throw error; } } From b0cb618e5c02ff71066cff307b024b9f5eb8341e Mon Sep 17 00:00:00 2001 From: salimtb Date: Tue, 18 Nov 2025 00:24:37 +0100 Subject: [PATCH 10/10] fix: fix cursor comment --- .../src/CurrencyRateController.ts | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/packages/assets-controllers/src/CurrencyRateController.ts b/packages/assets-controllers/src/CurrencyRateController.ts index 4c561449d06..01155aa117b 100644 --- a/packages/assets-controllers/src/CurrencyRateController.ts +++ b/packages/assets-controllers/src/CurrencyRateController.ts @@ -253,33 +253,28 @@ export class CurrencyRateController extends StaticIntervalPollingController { - const nativeTokenAddress = getNativeTokenAddress(chainId); - const tokenPrices = await this.#tokenPricesService.fetchTokenPrices( - { - chainId, - tokenAddresses: [nativeTokenAddress], - currency: currentCurrency, - }, - ); - - const tokenPrice = tokenPrices[nativeTokenAddress]; - - return { - nativeCurrency, - conversionDate: tokenPrice ? Date.now() / 1000 : null, - conversionRate: tokenPrice?.price ?? null, - usdConversionRate: null, // Token prices service doesn't provide USD rate in this context - }; - }, - ), + currencyToChainIdsEntries.map(async ([nativeCurrency, { chainId }]) => { + const nativeTokenAddress = getNativeTokenAddress(chainId); + const tokenPrices = await this.#tokenPricesService.fetchTokenPrices({ + chainId, + tokenAddresses: [nativeTokenAddress], + currency: currentCurrency, + }); + + const tokenPrice = tokenPrices[nativeTokenAddress]; + + return { + nativeCurrency, + conversionDate: tokenPrice ? Date.now() / 1000 : null, + conversionRate: tokenPrice?.price ?? null, + usdConversionRate: null, // Token prices service doesn't provide USD rate in this context + }; + }), ); - const ratesFromTokenPrices = ratesResults.map((result, index) => { - const [nativeCurrency, { chainId }] = - Object.entries(currencyToChainIds)[index]; + const [nativeCurrency, { chainId }] = currencyToChainIdsEntries[index]; if (result.status === 'fulfilled') { return result.value; }