From 458c305c0f68af6e57bbf1490d41d9b6bd539e55 Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Wed, 15 Jul 2026 12:03:08 +0300 Subject: [PATCH 1/3] fix(confirmations): convert pay-flow token amounts to USD instead of relabeling Follow-up to #32631. `useAccountTokens` was formatting `asset.fiat.balance` with `currency: 'USD'` while the numeric balance still came from the assets-controller selector computed with the user's preferred fiat rate. For non-USD preferences, values kept their EUR/GBP/etc. magnitude but rendered with a $ symbol. Extract the pay-currency policy into a new `useAssetFiatFormatter` hook in `hooks/pay/`. It owns the rate lookup, the (native->USD) / (native->preferred) re-scaling, the Intl formatting, and the missing-rate fallback. `useAccountTokens` becomes currency-agnostic and just calls the formatter. Hides fiat (returns undefined) rather than mislabeling when either conversion rate is unavailable, matching the existing testnet-hidden fallback. Preserves identity when preferred currency is already USD. --- .../hooks/pay/useAssetFiatFormatter.test.ts | 282 ++++++++++++++++++ .../hooks/pay/useAssetFiatFormatter.ts | 102 +++++++ .../hooks/send/useAccountTokens.test.ts | 147 ++++----- .../hooks/send/useAccountTokens.ts | 44 +-- 4 files changed, 445 insertions(+), 130 deletions(-) create mode 100644 app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.test.ts create mode 100644 app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.ts diff --git a/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.test.ts b/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.test.ts new file mode 100644 index 000000000000..b1a01a095f05 --- /dev/null +++ b/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.test.ts @@ -0,0 +1,282 @@ +import { renderHook } from '@testing-library/react-hooks'; +import { useSelector } from 'react-redux'; + +import { useAssetFiatFormatter } from './useAssetFiatFormatter'; +import { useTransactionPayCurrency } from './useTransactionPayCurrency'; +import { + selectCurrencyRates, + selectCurrentCurrency, +} from '../../../../../selectors/currencyRateController'; +import { selectEvmNetworkConfigurationsByChainId } from '../../../../../selectors/networkController'; +import { getIntlNumberFormatter } from '../../../../../util/intl'; + +jest.mock('react-redux', () => ({ + useSelector: jest.fn(), +})); + +jest.mock('./useTransactionPayCurrency', () => ({ + useTransactionPayCurrency: jest.fn(), +})); + +jest.mock('../../../../../selectors/currencyRateController', () => ({ + selectCurrentCurrency: jest.fn(), + selectCurrencyRates: jest.fn(), +})); + +jest.mock('../../../../../selectors/networkController', () => ({ + selectEvmNetworkConfigurationsByChainId: jest.fn(), +})); + +jest.mock('../../../../../util/intl', () => ({ + getIntlNumberFormatter: jest.fn(), +})); + +jest.mock('../../../../../../locales/i18n', () => ({ + locale: 'en-US', + strings: jest.fn((key: string) => key), +})); + +const mockUseSelector = jest.mocked(useSelector); +const mockUseTransactionPayCurrency = jest.mocked(useTransactionPayCurrency); +const mockGetIntlNumberFormatter = jest.mocked(getIntlNumberFormatter); + +const mockFormatter = { format: jest.fn() }; + +function buildState({ + preferredCurrency = 'usd', + currencyRates = {} as Record< + string, + { conversionRate?: number; usdConversionRate?: number } + >, + networkConfigs = {} as Record, +} = {}) { + mockUseSelector.mockImplementation((selector) => { + if (selector === selectCurrentCurrency) return preferredCurrency; + if (selector === selectCurrencyRates) return currencyRates; + if (selector === selectEvmNetworkConfigurationsByChainId) + return networkConfigs; + return undefined; + }); +} + +describe('useAssetFiatFormatter', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockUseTransactionPayCurrency.mockReturnValue(undefined); + mockGetIntlNumberFormatter.mockReturnValue( + mockFormatter as unknown as ReturnType, + ); + mockFormatter.format.mockImplementation((n) => `formatted:${String(n)}`); + }); + + describe('outside pay flow', () => { + it('formats using the preferred currency', () => { + buildState({ preferredCurrency: 'eur' }); + + const { result } = renderHook(() => useAssetFiatFormatter()); + const output = result.current.format('100', '0x1'); + + expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith( + 'en-US', + expect.objectContaining({ style: 'currency', currency: 'eur' }), + ); + expect(mockFormatter.format).toHaveBeenCalledWith('100'); + expect(output).toBe('formatted:100'); + }); + + it('exposes the fiatCurrency being used', () => { + buildState({ preferredCurrency: 'eur' }); + + const { result } = renderHook(() => useAssetFiatFormatter()); + expect(result.current.fiatCurrency).toBe('eur'); + }); + + it('uses minimumFractionDigits=0 for integer amounts', () => { + buildState({ preferredCurrency: 'eur' }); + + renderHook(() => useAssetFiatFormatter()).result.current.format( + '100', + '0x1', + ); + + expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith( + 'en-US', + expect.objectContaining({ minimumFractionDigits: 0 }), + ); + }); + + it('uses minimumFractionDigits=2 for non-integer amounts', () => { + buildState({ preferredCurrency: 'eur' }); + + renderHook(() => useAssetFiatFormatter()).result.current.format( + '100.5', + '0x1', + ); + + expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith( + 'en-US', + expect.objectContaining({ minimumFractionDigits: 2 }), + ); + }); + + it('treats undefined/null balances as zero', () => { + buildState({ preferredCurrency: 'eur' }); + + renderHook(() => useAssetFiatFormatter()).result.current.format( + undefined, + '0x1', + ); + + expect(mockFormatter.format).toHaveBeenCalledWith('0'); + }); + }); + + describe('pay flow (USD forced, preferred is EUR)', () => { + const eurUsdRates = { + ETH: { conversionRate: 2000, usdConversionRate: 2200 }, + }; + const ethChainConfig = { '0x1': { nativeCurrency: 'ETH' } }; + + beforeEach(() => { + mockUseTransactionPayCurrency.mockReturnValue('USD'); + }); + + it('re-scales the amount by usdRate/preferredRate', () => { + buildState({ + preferredCurrency: 'eur', + currencyRates: eurUsdRates, + networkConfigs: ethChainConfig, + }); + + renderHook(() => useAssetFiatFormatter()).result.current.format( + '100', + '0x1', + ); + + // 100 EUR * (2200 USD/ETH / 2000 EUR/ETH) = 110 USD + expect(mockFormatter.format).toHaveBeenCalledWith('110'); + expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith( + 'en-US', + expect.objectContaining({ currency: 'USD' }), + ); + }); + + it('exposes fiatCurrency as USD', () => { + buildState({ + preferredCurrency: 'eur', + currencyRates: eurUsdRates, + networkConfigs: ethChainConfig, + }); + + const { result } = renderHook(() => useAssetFiatFormatter()); + expect(result.current.fiatCurrency).toBe('USD'); + }); + + it('returns undefined when usdConversionRate is missing', () => { + buildState({ + preferredCurrency: 'eur', + currencyRates: { ETH: { conversionRate: 2000 } }, + networkConfigs: ethChainConfig, + }); + + const { result } = renderHook(() => useAssetFiatFormatter()); + const output = result.current.format('100', '0x1'); + + expect(output).toBeUndefined(); + expect(mockFormatter.format).not.toHaveBeenCalled(); + }); + + it('returns undefined when preferred conversionRate is missing', () => { + buildState({ + preferredCurrency: 'eur', + currencyRates: { ETH: { usdConversionRate: 2200 } }, + networkConfigs: ethChainConfig, + }); + + const { result } = renderHook(() => useAssetFiatFormatter()); + const output = result.current.format('100', '0x1'); + + expect(output).toBeUndefined(); + expect(mockFormatter.format).not.toHaveBeenCalled(); + }); + + it('returns undefined when chain has no EVM network config', () => { + buildState({ + preferredCurrency: 'eur', + currencyRates: eurUsdRates, + networkConfigs: {}, + }); + + const { result } = renderHook(() => useAssetFiatFormatter()); + const output = result.current.format('100', '0x1'); + + expect(output).toBeUndefined(); + expect(mockFormatter.format).not.toHaveBeenCalled(); + }); + + it('formats zero even when rates or chain config are missing', () => { + buildState({ + preferredCurrency: 'eur', + currencyRates: {}, + networkConfigs: {}, + }); + + const { result } = renderHook(() => useAssetFiatFormatter()); + const output = result.current.format(0, undefined); + + expect(output).toBe('formatted:0'); + expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith( + 'en-US', + expect.objectContaining({ currency: 'USD' }), + ); + }); + }); + + describe('pay flow (USD forced, preferred is already USD)', () => { + it('does not re-scale (identity), so numeric value is unchanged', () => { + mockUseTransactionPayCurrency.mockReturnValue('USD'); + buildState({ + preferredCurrency: 'usd', + currencyRates: { + ETH: { conversionRate: 2200, usdConversionRate: 2200 }, + }, + networkConfigs: { '0x1': { nativeCurrency: 'ETH' } }, + }); + + renderHook(() => useAssetFiatFormatter()).result.current.format( + '100', + '0x1', + ); + + expect(mockFormatter.format).toHaveBeenCalledWith('100'); + }); + + it('still formats when currency rates are missing (no conversion needed)', () => { + mockUseTransactionPayCurrency.mockReturnValue('USD'); + buildState({ + preferredCurrency: 'usd', + currencyRates: {}, + networkConfigs: {}, + }); + + const { result } = renderHook(() => useAssetFiatFormatter()); + const output = result.current.format('100', '0x1'); + + expect(output).toBe('formatted:100'); + }); + }); + + describe('formatter error fallback', () => { + it('falls back to `${value} ${currency}` when Intl throws', () => { + buildState({ preferredCurrency: 'eur' }); + mockGetIntlNumberFormatter.mockImplementation(() => { + throw new Error('boom'); + }); + + const { result } = renderHook(() => useAssetFiatFormatter()); + const output = result.current.format('42', '0x1'); + + expect(output).toBe('42 eur'); + }); + }); +}); diff --git a/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.ts b/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.ts new file mode 100644 index 000000000000..f16eab2d1602 --- /dev/null +++ b/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.ts @@ -0,0 +1,102 @@ +import { useSelector } from 'react-redux'; +import { useCallback } from 'react'; +import { BigNumber } from 'bignumber.js'; +import { Hex } from '@metamask/utils'; + +import I18n from '../../../../../../locales/i18n'; +import Logger from '../../../../../util/Logger'; +import { getIntlNumberFormatter } from '../../../../../util/intl'; +import { + selectCurrencyRates, + selectCurrentCurrency, +} from '../../../../../selectors/currencyRateController'; +import { selectEvmNetworkConfigurationsByChainId } from '../../../../../selectors/networkController'; +import { useTransactionPayCurrency } from './useTransactionPayCurrency'; + +export type AssetFiatFormatter = ( + preferredBalance: BigNumber.Value | undefined, + chainId: string | undefined, +) => string | undefined; + +/** + * Returns a formatter that converts a preferred-currency fiat balance into a + * localized currency string, honoring the pay-flow currency override. + * + * `asset.fiat.balance` from `@metamask/assets-controllers` is denominated in + * the user's preferred currency. When the active confirmation forces a + * different pay currency (currently only USD), the numeric value must be + * re-scaled by `(native→pay) / (native→preferred)` — not just relabeled. + * + * Returns `undefined` when the value cannot be safely rendered in the target + * currency (missing conversion rate, missing chain config). Callers should + * treat `undefined` as "hide fiat", matching the existing testnet-hidden + * fallback in `useAccountTokens`. + */ +export function useAssetFiatFormatter(): { + format: AssetFiatFormatter; + fiatCurrency: string; +} { + const preferredCurrency = useSelector(selectCurrentCurrency); + const payCurrency = useTransactionPayCurrency(); + const currencyRates = useSelector(selectCurrencyRates); + const networkConfigurationsByChainId = useSelector( + selectEvmNetworkConfigurationsByChainId, + ); + + const fiatCurrency = payCurrency ?? preferredCurrency; + + const needsConversion = + payCurrency === 'USD' && + preferredCurrency?.toLowerCase() !== payCurrency.toLowerCase(); + + const format = useCallback( + (preferredBalance, chainId) => { + const preferredAmount = new BigNumber(preferredBalance ?? 0); + + let payAmount: BigNumber | undefined = preferredAmount; + // Zero is currency-invariant, so we skip the rate lookup even when a + // pay-currency conversion would otherwise be required. Keeps the + // enrichment "$0" placeholder working when rates or chain config are + // missing. + if (needsConversion && !preferredAmount.isZero()) { + const nativeCurrency = + networkConfigurationsByChainId?.[chainId as Hex]?.nativeCurrency; + const rateEntry = nativeCurrency + ? currencyRates?.[nativeCurrency] + : undefined; + const preferredRate = rateEntry?.conversionRate; + const usdRate = rateEntry?.usdConversionRate; + + payAmount = + preferredRate && usdRate + ? preferredAmount.multipliedBy(usdRate).dividedBy(preferredRate) + : undefined; + } + + if (payAmount === undefined) { + return undefined; + } + + const hasDecimals = !payAmount.isInteger(); + + try { + return getIntlNumberFormatter(I18n.locale, { + style: 'currency', + currency: fiatCurrency, + minimumFractionDigits: hasDecimals ? 2 : 0, + }).format(payAmount.toFixed() as unknown as number); + } catch (error) { + Logger.error(error as Error); + return `${payAmount.toFixed()} ${fiatCurrency}`; + } + }, + [ + needsConversion, + networkConfigurationsByChainId, + currencyRates, + fiatCurrency, + ], + ); + + return { format, fiatCurrency }; +} diff --git a/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts b/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts index 7b6eedae48a8..a5d96f4ef1ae 100644 --- a/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts +++ b/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts @@ -3,7 +3,6 @@ import { useSelector } from 'react-redux'; import { useAccountTokens } from './useAccountTokens'; import { getNetworkBadgeSource } from '../../utils/network'; -import { getIntlNumberFormatter } from '../../../../../util/intl'; import { TokenStandard } from '../../types/token'; import { selectAssetsBySelectedAccountGroup, @@ -16,7 +15,7 @@ import { useTokensData } from '../../../../hooks/useTokensData/useTokensData'; import { buildEvmCaip19AssetId } from '../../../../../util/multichain/buildEvmCaip19AssetId'; import { Hex } from '@metamask/utils'; import { useTransactionAccountOverride } from '../transactions/useTransactionAccountOverride'; -import { useTransactionPayCurrency } from '../pay/useTransactionPayCurrency'; +import { useAssetFiatFormatter } from '../pay/useAssetFiatFormatter'; import { selectInternalAccountsById } from '../../../../../selectors/accountsController'; import { selectAccountToGroupMap } from '../../../../../selectors/multichainAccounts/accountTreeController'; @@ -28,19 +27,10 @@ jest.mock('../../utils/network', () => ({ getNetworkBadgeSource: jest.fn(), })); -jest.mock('../../../../../util/intl', () => ({ - getIntlNumberFormatter: jest.fn(), -})); - jest.mock('../../../../../util/networks', () => ({ isTestNet: jest.fn(), })); -jest.mock('../../../../../../locales/i18n', () => ({ - locale: 'en-US', - strings: jest.fn((key: string) => key), -})); - jest.mock('../transactions/useTransactionAccountOverride', () => ({ useTransactionAccountOverride: jest.fn(), })); @@ -48,10 +38,10 @@ const useTransactionAccountOverrideMock = jest.mocked( useTransactionAccountOverride, ); -jest.mock('../pay/useTransactionPayCurrency', () => ({ - useTransactionPayCurrency: jest.fn(), +jest.mock('../pay/useAssetFiatFormatter', () => ({ + useAssetFiatFormatter: jest.fn(), })); -const useTransactionPayCurrencyMock = jest.mocked(useTransactionPayCurrency); +const useAssetFiatFormatterMock = jest.mocked(useAssetFiatFormatter); jest.mock('../../../../../selectors/accountsController', () => ({ selectInternalAccountsById: jest.fn(), @@ -82,7 +72,6 @@ jest.mock('../../../../../util/multichain/buildEvmCaip19AssetId'); const mockUseSelector = jest.mocked(useSelector); const mockGetNetworkBadgeSource = jest.mocked(getNetworkBadgeSource); -const mockGetIntlNumberFormatter = jest.mocked(getIntlNumberFormatter); const mockSelectAssetsBySelectedAccountGroup = jest.mocked( selectAssetsBySelectedAccountGroup, ); @@ -91,6 +80,8 @@ const mockIsTestNet = jest.mocked(isTestNet); const mockUseTokensData = jest.mocked(useTokensData); const mockBuildEvmCaip19AssetId = jest.mocked(buildEvmCaip19AssetId); +const mockFormatFiat = jest.fn(); + const mockAssets = { '0x1': [ { @@ -119,16 +110,11 @@ const mockAssets = { ], }; -const mockFormatter = { - format: jest.fn(), -}; - describe('useAccountTokens', () => { beforeEach(() => { jest.clearAllMocks(); useTransactionAccountOverrideMock.mockReturnValue(undefined); - useTransactionPayCurrencyMock.mockReturnValue(undefined); // eslint-disable-next-line @typescript-eslint/no-explicit-any mockSelectAssetsBySelectedAccountGroup.mockReturnValue(mockAssets as any); @@ -150,10 +136,13 @@ describe('useAccountTokens', () => { return undefined; }); + mockFormatFiat.mockReturnValue('$100.50'); + useAssetFiatFormatterMock.mockReturnValue({ + format: mockFormatFiat, + fiatCurrency: 'USD', + }); + mockGetNetworkBadgeSource.mockReturnValue('network-badge-source'); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - mockGetIntlNumberFormatter.mockReturnValue(mockFormatter as any); - mockFormatter.format.mockReturnValue('$100.50'); mockIsTestNet.mockReturnValue(false); mockUseTokensData.mockReturnValue({}); mockBuildEvmCaip19AssetId.mockImplementation( @@ -211,8 +200,8 @@ describe('useAccountTokens', () => { }); }); - it('handles integer amounts without decimals', () => { - const integerAssets = { + it('passes each asset balance to the fiat formatter', () => { + const balanceAssets = { '0x1': [ { chainId: '0x1', @@ -221,66 +210,31 @@ describe('useAccountTokens', () => { rawBalance: '0x1234', symbol: 'TOKEN1', }, - ], - }; - - mockSelectAssetsBySelectedAccountGroup.mockReturnValue( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - integerAssets as any, - ); - mockUseSelector.mockImplementation((selector) => { - if (selector === selectAssetsBySelectedAccountGroup) { - return integerAssets; - } - if (selector === selectCurrentCurrency) { - return 'USD'; - } - return undefined; - }); - - renderHook(() => useAccountTokens()); - - expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith('en-US', { - style: 'currency', - currency: 'USD', - minimumFractionDigits: 0, - }); - }); - - it('handles decimal amounts with fraction digits', () => { - const decimalAssets = { - '0x1': [ { chainId: '0x1', - accountType: 'eip155:1/erc20:0xtoken1', + accountType: 'eip155:1/erc20:0xtoken2', fiat: { balance: '100.50' }, - rawBalance: '0x1234', - symbol: 'TOKEN1', + rawBalance: '0x5678', + symbol: 'TOKEN2', }, ], }; mockSelectAssetsBySelectedAccountGroup.mockReturnValue( // eslint-disable-next-line @typescript-eslint/no-explicit-any - decimalAssets as any, + balanceAssets as any, ); mockUseSelector.mockImplementation((selector) => { if (selector === selectAssetsBySelectedAccountGroup) { - return decimalAssets; - } - if (selector === selectCurrentCurrency) { - return 'USD'; + return balanceAssets; } return undefined; }); renderHook(() => useAccountTokens()); - expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith('en-US', { - style: 'currency', - currency: 'USD', - minimumFractionDigits: 2, - }); + expect(mockFormatFiat).toHaveBeenCalledWith('100', '0x1'); + expect(mockFormatFiat).toHaveBeenCalledWith('100.50', '0x1'); }); }); @@ -987,47 +941,54 @@ describe('useAccountTokens', () => { }); }); - describe('pay currency', () => { - it('uses preferred currency when useTransactionPayCurrency returns undefined', () => { - useTransactionPayCurrencyMock.mockReturnValue(undefined); - mockUseSelector.mockImplementation((selector) => { - if (selector === selectAssetsBySelectedAccountGroup) { - return mockAssets; - } - if (selector === selectCurrentCurrency) { - return 'eur'; - } - return undefined; + describe('fiat formatter delegation', () => { + it('sets balanceInSelectedCurrency to the formatter output', () => { + mockFormatFiat.mockReturnValue('$110.00'); + + const { result } = renderHook(() => useAccountTokens()); + + result.current.forEach((asset) => { + expect(asset.balanceInSelectedCurrency).toBe('$110.00'); }); + }); - renderHook(() => useAccountTokens()); + it('propagates undefined from the formatter as a hidden fiat value', () => { + mockFormatFiat.mockReturnValue(undefined); + + const { result } = renderHook(() => useAccountTokens()); - expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith('en-US', { - style: 'currency', - currency: 'eur', - minimumFractionDigits: 2, + result.current.forEach((asset) => { + expect(asset.balanceInSelectedCurrency).toBeUndefined(); }); }); - it('uses USD from useTransactionPayCurrency over preferred currency', () => { - useTransactionPayCurrencyMock.mockReturnValue('USD'); + it('does not call the formatter for testnet assets when fiat is hidden', () => { + const testnetOnlyAssets = { + '0x5': [ + { + chainId: '0x5', + accountType: 'eip155:5/erc20:0xtoken1', + fiat: { balance: '100.50' }, + rawBalance: '0x1234', + symbol: 'TOKEN1', + }, + ], + }; + + mockIsTestNet.mockReturnValue(true); mockUseSelector.mockImplementation((selector) => { if (selector === selectAssetsBySelectedAccountGroup) { - return mockAssets; + return testnetOnlyAssets; } - if (selector === selectCurrentCurrency) { - return 'eur'; + if (selector === selectShowFiatInTestnets) { + return false; } return undefined; }); renderHook(() => useAccountTokens()); - expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith('en-US', { - style: 'currency', - currency: 'USD', - minimumFractionDigits: 2, - }); + expect(mockFormatFiat).not.toHaveBeenCalled(); }); }); }); diff --git a/app/components/Views/confirmations/hooks/send/useAccountTokens.ts b/app/components/Views/confirmations/hooks/send/useAccountTokens.ts index 285b50d78284..42a8499783c5 100644 --- a/app/components/Views/confirmations/hooks/send/useAccountTokens.ts +++ b/app/components/Views/confirmations/hooks/send/useAccountTokens.ts @@ -10,18 +10,14 @@ import { import { selectInternalAccountsById } from '../../../../../selectors/accountsController'; import { selectAccountToGroupMap } from '../../../../../selectors/multichainAccounts/accountTreeController'; import { isTestNet } from '../../../../../util/networks'; -import Logger from '../../../../../util/Logger'; -import { selectCurrentCurrency } from '../../../../../selectors/currencyRateController'; import { selectShowFiatInTestnets } from '../../../../../selectors/settings'; -import I18n from '../../../../../../locales/i18n'; -import { getIntlNumberFormatter } from '../../../../../util/intl'; import { getNetworkBadgeSource } from '../../utils/network'; import { AssetType, TokenStandard } from '../../types/token'; import { useTokensData } from '../../../../hooks/useTokensData/useTokensData'; import { buildEvmCaip19AssetId } from '../../../../../util/multichain/buildEvmCaip19AssetId'; import type { RootState } from '../../../../../reducers'; import { useTransactionAccountOverride } from '../transactions/useTransactionAccountOverride'; -import { useTransactionPayCurrency } from '../pay/useTransactionPayCurrency'; +import { useAssetFiatFormatter } from '../pay/useAssetFiatFormatter'; import { isNetworkTestnet } from './useNetworkFilter'; export interface EnrichTokenRequest { @@ -75,10 +71,8 @@ export function useAccountTokens({ [accountOverride, accountAssets, globalAssets], ); - const preferredCurrency = useSelector(selectCurrentCurrency); - const payCurrency = useTransactionPayCurrency(); - const fiatCurrency = payCurrency ?? preferredCurrency; const showFiatOnTestnets = useSelector(selectShowFiatInTestnets); + const { format: formatFiat } = useAssetFiatFormatter(); const assetIds = useMemo( () => @@ -130,24 +124,9 @@ export function useAccountTokens({ isNetworkTestnet(chainId as string); const processedAssets = assetsWithBalance.map((asset) => { - const fiatAmount = new BigNumber(asset.fiat?.balance || 0); - const hasDecimals = !fiatAmount.isInteger(); - - let balanceInSelectedCurrency: string | undefined; - if (isFiatHidden(asset.chainId)) { - balanceInSelectedCurrency = undefined; - } else { - try { - balanceInSelectedCurrency = getIntlNumberFormatter(I18n.locale, { - style: 'currency', - currency: fiatCurrency, - minimumFractionDigits: hasDecimals ? 2 : 0, - }).format(fiatAmount.toFixed() as unknown as number); - } catch (error) { - Logger.error(error as Error); - balanceInSelectedCurrency = `${fiatAmount.toFixed()} ${fiatCurrency}`; - } - } + const balanceInSelectedCurrency = isFiatHidden(asset.chainId) + ? undefined + : formatFiat(asset.fiat?.balance, asset.chainId); return { ...asset, @@ -158,16 +137,7 @@ export function useAccountTokens({ }); if (enrichTokenRequests.length > 0) { - let zeroFiat: string; - try { - zeroFiat = getIntlNumberFormatter(I18n.locale, { - style: 'currency', - currency: fiatCurrency, - minimumFractionDigits: 0, - }).format(0); - } catch { - zeroFiat = `0 ${fiatCurrency}`; - } + const zeroFiat = formatFiat(0, undefined) ?? ''; const existingKeys = new Set( processedAssets.map( @@ -215,11 +185,11 @@ export function useAccountTokens({ }, [ assets, includeNoBalance, - fiatCurrency, showFiatOnTestnets, tokenFilter, enrichTokenRequests, assetIds, tokensByAssetId, + formatFiat, ]) as unknown as AssetType[]; } From 98e590d501ac392cbe499a32a87bd8d087896dce Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Wed, 15 Jul 2026 15:13:18 +0300 Subject: [PATCH 2/3] Use useTokenFiatRates --- .../hooks/pay/useAssetFiatFormatter.test.ts | 209 +++--------------- .../hooks/pay/useAssetFiatFormatter.ts | 78 ++----- .../hooks/send/useAccountTokens.test.ts | 79 ++++++- .../hooks/send/useAccountTokens.ts | 85 +++++-- 4 files changed, 190 insertions(+), 261 deletions(-) diff --git a/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.test.ts b/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.test.ts index b1a01a095f05..60cef1a573c2 100644 --- a/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.test.ts +++ b/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.test.ts @@ -3,11 +3,7 @@ import { useSelector } from 'react-redux'; import { useAssetFiatFormatter } from './useAssetFiatFormatter'; import { useTransactionPayCurrency } from './useTransactionPayCurrency'; -import { - selectCurrencyRates, - selectCurrentCurrency, -} from '../../../../../selectors/currencyRateController'; -import { selectEvmNetworkConfigurationsByChainId } from '../../../../../selectors/networkController'; +import { selectCurrentCurrency } from '../../../../../selectors/currencyRateController'; import { getIntlNumberFormatter } from '../../../../../util/intl'; jest.mock('react-redux', () => ({ @@ -20,11 +16,6 @@ jest.mock('./useTransactionPayCurrency', () => ({ jest.mock('../../../../../selectors/currencyRateController', () => ({ selectCurrentCurrency: jest.fn(), - selectCurrencyRates: jest.fn(), -})); - -jest.mock('../../../../../selectors/networkController', () => ({ - selectEvmNetworkConfigurationsByChainId: jest.fn(), })); jest.mock('../../../../../util/intl', () => ({ @@ -42,19 +33,9 @@ const mockGetIntlNumberFormatter = jest.mocked(getIntlNumberFormatter); const mockFormatter = { format: jest.fn() }; -function buildState({ - preferredCurrency = 'usd', - currencyRates = {} as Record< - string, - { conversionRate?: number; usdConversionRate?: number } - >, - networkConfigs = {} as Record, -} = {}) { +function buildState({ preferredCurrency = 'usd' } = {}) { mockUseSelector.mockImplementation((selector) => { if (selector === selectCurrentCurrency) return preferredCurrency; - if (selector === selectCurrencyRates) return currencyRates; - if (selector === selectEvmNetworkConfigurationsByChainId) - return networkConfigs; return undefined; }); } @@ -69,204 +50,86 @@ describe('useAssetFiatFormatter', () => { mockFormatter.format.mockImplementation((n) => `formatted:${String(n)}`); }); - describe('outside pay flow', () => { - it('formats using the preferred currency', () => { + describe('fiatCurrency selection', () => { + it('uses the preferred currency outside pay flow', () => { buildState({ preferredCurrency: 'eur' }); const { result } = renderHook(() => useAssetFiatFormatter()); - const output = result.current.format('100', '0x1'); - expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith( - 'en-US', - expect.objectContaining({ style: 'currency', currency: 'eur' }), - ); - expect(mockFormatter.format).toHaveBeenCalledWith('100'); - expect(output).toBe('formatted:100'); + expect(result.current.fiatCurrency).toBe('eur'); }); - it('exposes the fiatCurrency being used', () => { + it('uses the pay currency when useTransactionPayCurrency returns a value', () => { buildState({ preferredCurrency: 'eur' }); + mockUseTransactionPayCurrency.mockReturnValue('USD'); const { result } = renderHook(() => useAssetFiatFormatter()); - expect(result.current.fiatCurrency).toBe('eur'); + + expect(result.current.fiatCurrency).toBe('USD'); }); + }); - it('uses minimumFractionDigits=0 for integer amounts', () => { + describe('format', () => { + it('formats a numeric amount using Intl currency', () => { buildState({ preferredCurrency: 'eur' }); - renderHook(() => useAssetFiatFormatter()).result.current.format( - '100', - '0x1', - ); + const { result } = renderHook(() => useAssetFiatFormatter()); + const output = result.current.format('100'); expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith( 'en-US', - expect.objectContaining({ minimumFractionDigits: 0 }), + expect.objectContaining({ style: 'currency', currency: 'eur' }), ); + expect(mockFormatter.format).toHaveBeenCalledWith('100'); + expect(output).toBe('formatted:100'); }); - it('uses minimumFractionDigits=2 for non-integer amounts', () => { + it('formats with the pay currency when in pay flow', () => { buildState({ preferredCurrency: 'eur' }); + mockUseTransactionPayCurrency.mockReturnValue('USD'); - renderHook(() => useAssetFiatFormatter()).result.current.format( - '100.5', - '0x1', - ); + const { result } = renderHook(() => useAssetFiatFormatter()); + result.current.format('110'); expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith( 'en-US', - expect.objectContaining({ minimumFractionDigits: 2 }), + expect.objectContaining({ currency: 'USD' }), ); + expect(mockFormatter.format).toHaveBeenCalledWith('110'); }); - it('treats undefined/null balances as zero', () => { + it('uses minimumFractionDigits=0 for integer amounts', () => { buildState({ preferredCurrency: 'eur' }); - renderHook(() => useAssetFiatFormatter()).result.current.format( - undefined, - '0x1', - ); - - expect(mockFormatter.format).toHaveBeenCalledWith('0'); - }); - }); - - describe('pay flow (USD forced, preferred is EUR)', () => { - const eurUsdRates = { - ETH: { conversionRate: 2000, usdConversionRate: 2200 }, - }; - const ethChainConfig = { '0x1': { nativeCurrency: 'ETH' } }; - - beforeEach(() => { - mockUseTransactionPayCurrency.mockReturnValue('USD'); - }); - - it('re-scales the amount by usdRate/preferredRate', () => { - buildState({ - preferredCurrency: 'eur', - currencyRates: eurUsdRates, - networkConfigs: ethChainConfig, - }); - - renderHook(() => useAssetFiatFormatter()).result.current.format( - '100', - '0x1', - ); + renderHook(() => useAssetFiatFormatter()).result.current.format('100'); - // 100 EUR * (2200 USD/ETH / 2000 EUR/ETH) = 110 USD - expect(mockFormatter.format).toHaveBeenCalledWith('110'); expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith( 'en-US', - expect.objectContaining({ currency: 'USD' }), + expect.objectContaining({ minimumFractionDigits: 0 }), ); }); - it('exposes fiatCurrency as USD', () => { - buildState({ - preferredCurrency: 'eur', - currencyRates: eurUsdRates, - networkConfigs: ethChainConfig, - }); - - const { result } = renderHook(() => useAssetFiatFormatter()); - expect(result.current.fiatCurrency).toBe('USD'); - }); - - it('returns undefined when usdConversionRate is missing', () => { - buildState({ - preferredCurrency: 'eur', - currencyRates: { ETH: { conversionRate: 2000 } }, - networkConfigs: ethChainConfig, - }); - - const { result } = renderHook(() => useAssetFiatFormatter()); - const output = result.current.format('100', '0x1'); - - expect(output).toBeUndefined(); - expect(mockFormatter.format).not.toHaveBeenCalled(); - }); - - it('returns undefined when preferred conversionRate is missing', () => { - buildState({ - preferredCurrency: 'eur', - currencyRates: { ETH: { usdConversionRate: 2200 } }, - networkConfigs: ethChainConfig, - }); - - const { result } = renderHook(() => useAssetFiatFormatter()); - const output = result.current.format('100', '0x1'); - - expect(output).toBeUndefined(); - expect(mockFormatter.format).not.toHaveBeenCalled(); - }); - - it('returns undefined when chain has no EVM network config', () => { - buildState({ - preferredCurrency: 'eur', - currencyRates: eurUsdRates, - networkConfigs: {}, - }); - - const { result } = renderHook(() => useAssetFiatFormatter()); - const output = result.current.format('100', '0x1'); - - expect(output).toBeUndefined(); - expect(mockFormatter.format).not.toHaveBeenCalled(); - }); - - it('formats zero even when rates or chain config are missing', () => { - buildState({ - preferredCurrency: 'eur', - currencyRates: {}, - networkConfigs: {}, - }); + it('uses minimumFractionDigits=2 for non-integer amounts', () => { + buildState({ preferredCurrency: 'eur' }); - const { result } = renderHook(() => useAssetFiatFormatter()); - const output = result.current.format(0, undefined); + renderHook(() => useAssetFiatFormatter()).result.current.format('100.5'); - expect(output).toBe('formatted:0'); expect(mockGetIntlNumberFormatter).toHaveBeenCalledWith( 'en-US', - expect.objectContaining({ currency: 'USD' }), - ); - }); - }); - - describe('pay flow (USD forced, preferred is already USD)', () => { - it('does not re-scale (identity), so numeric value is unchanged', () => { - mockUseTransactionPayCurrency.mockReturnValue('USD'); - buildState({ - preferredCurrency: 'usd', - currencyRates: { - ETH: { conversionRate: 2200, usdConversionRate: 2200 }, - }, - networkConfigs: { '0x1': { nativeCurrency: 'ETH' } }, - }); - - renderHook(() => useAssetFiatFormatter()).result.current.format( - '100', - '0x1', + expect.objectContaining({ minimumFractionDigits: 2 }), ); - - expect(mockFormatter.format).toHaveBeenCalledWith('100'); }); - it('still formats when currency rates are missing (no conversion needed)', () => { - mockUseTransactionPayCurrency.mockReturnValue('USD'); - buildState({ - preferredCurrency: 'usd', - currencyRates: {}, - networkConfigs: {}, - }); + it('returns undefined when input is undefined', () => { + buildState({ preferredCurrency: 'eur' }); const { result } = renderHook(() => useAssetFiatFormatter()); - const output = result.current.format('100', '0x1'); + const output = result.current.format(undefined); - expect(output).toBe('formatted:100'); + expect(output).toBeUndefined(); + expect(mockFormatter.format).not.toHaveBeenCalled(); }); - }); - describe('formatter error fallback', () => { it('falls back to `${value} ${currency}` when Intl throws', () => { buildState({ preferredCurrency: 'eur' }); mockGetIntlNumberFormatter.mockImplementation(() => { @@ -274,7 +137,7 @@ describe('useAssetFiatFormatter', () => { }); const { result } = renderHook(() => useAssetFiatFormatter()); - const output = result.current.format('42', '0x1'); + const output = result.current.format('42'); expect(output).toBe('42 eur'); }); diff --git a/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.ts b/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.ts index f16eab2d1602..e4a4ae99cdfb 100644 --- a/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.ts +++ b/app/components/Views/confirmations/hooks/pay/useAssetFiatFormatter.ts @@ -1,36 +1,29 @@ import { useSelector } from 'react-redux'; import { useCallback } from 'react'; import { BigNumber } from 'bignumber.js'; -import { Hex } from '@metamask/utils'; import I18n from '../../../../../../locales/i18n'; import Logger from '../../../../../util/Logger'; import { getIntlNumberFormatter } from '../../../../../util/intl'; -import { - selectCurrencyRates, - selectCurrentCurrency, -} from '../../../../../selectors/currencyRateController'; -import { selectEvmNetworkConfigurationsByChainId } from '../../../../../selectors/networkController'; +import { selectCurrentCurrency } from '../../../../../selectors/currencyRateController'; import { useTransactionPayCurrency } from './useTransactionPayCurrency'; export type AssetFiatFormatter = ( - preferredBalance: BigNumber.Value | undefined, - chainId: string | undefined, + amount: BigNumber.Value | undefined, ) => string | undefined; /** - * Returns a formatter that converts a preferred-currency fiat balance into a - * localized currency string, honoring the pay-flow currency override. + * Returns a currency formatter whose target currency follows the pay-flow + * override: USD inside `PAY_TRANSACTION_TYPES` confirmations, otherwise the + * user's preferred currency. * - * `asset.fiat.balance` from `@metamask/assets-controllers` is denominated in - * the user's preferred currency. When the active confirmation forces a - * different pay currency (currently only USD), the numeric value must be - * re-scaled by `(native→pay) / (native→preferred)` — not just relabeled. + * This hook only formats — it does not derive rates or convert values. + * Callers must pass an `amount` already denominated in `fiatCurrency`. Rate + * derivation should go through `useTokenFiatRates`, which owns the + * stablecoin exception and the USD/preferred-currency rate selection. * - * Returns `undefined` when the value cannot be safely rendered in the target - * currency (missing conversion rate, missing chain config). Callers should - * treat `undefined` as "hide fiat", matching the existing testnet-hidden - * fallback in `useAccountTokens`. + * Returns `undefined` when the input amount is `undefined`, so callers can + * pass through a missing rate as "hide fiat". */ export function useAssetFiatFormatter(): { format: AssetFiatFormatter; @@ -38,64 +31,29 @@ export function useAssetFiatFormatter(): { } { const preferredCurrency = useSelector(selectCurrentCurrency); const payCurrency = useTransactionPayCurrency(); - const currencyRates = useSelector(selectCurrencyRates); - const networkConfigurationsByChainId = useSelector( - selectEvmNetworkConfigurationsByChainId, - ); - const fiatCurrency = payCurrency ?? preferredCurrency; - const needsConversion = - payCurrency === 'USD' && - preferredCurrency?.toLowerCase() !== payCurrency.toLowerCase(); - const format = useCallback( - (preferredBalance, chainId) => { - const preferredAmount = new BigNumber(preferredBalance ?? 0); - - let payAmount: BigNumber | undefined = preferredAmount; - // Zero is currency-invariant, so we skip the rate lookup even when a - // pay-currency conversion would otherwise be required. Keeps the - // enrichment "$0" placeholder working when rates or chain config are - // missing. - if (needsConversion && !preferredAmount.isZero()) { - const nativeCurrency = - networkConfigurationsByChainId?.[chainId as Hex]?.nativeCurrency; - const rateEntry = nativeCurrency - ? currencyRates?.[nativeCurrency] - : undefined; - const preferredRate = rateEntry?.conversionRate; - const usdRate = rateEntry?.usdConversionRate; - - payAmount = - preferredRate && usdRate - ? preferredAmount.multipliedBy(usdRate).dividedBy(preferredRate) - : undefined; - } - - if (payAmount === undefined) { + (amount) => { + if (amount === undefined) { return undefined; } - const hasDecimals = !payAmount.isInteger(); + const value = new BigNumber(amount); + const hasDecimals = !value.isInteger(); try { return getIntlNumberFormatter(I18n.locale, { style: 'currency', currency: fiatCurrency, minimumFractionDigits: hasDecimals ? 2 : 0, - }).format(payAmount.toFixed() as unknown as number); + }).format(value.toFixed() as unknown as number); } catch (error) { Logger.error(error as Error); - return `${payAmount.toFixed()} ${fiatCurrency}`; + return `${value.toFixed()} ${fiatCurrency}`; } }, - [ - needsConversion, - networkConfigurationsByChainId, - currencyRates, - fiatCurrency, - ], + [fiatCurrency], ); return { format, fiatCurrency }; diff --git a/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts b/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts index a5d96f4ef1ae..0bbe45c2d14c 100644 --- a/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts +++ b/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts @@ -16,6 +16,7 @@ import { buildEvmCaip19AssetId } from '../../../../../util/multichain/buildEvmCa import { Hex } from '@metamask/utils'; import { useTransactionAccountOverride } from '../transactions/useTransactionAccountOverride'; import { useAssetFiatFormatter } from '../pay/useAssetFiatFormatter'; +import { useTokenFiatRates } from '../tokens/useTokenFiatRates'; import { selectInternalAccountsById } from '../../../../../selectors/accountsController'; import { selectAccountToGroupMap } from '../../../../../selectors/multichainAccounts/accountTreeController'; @@ -43,6 +44,15 @@ jest.mock('../pay/useAssetFiatFormatter', () => ({ })); const useAssetFiatFormatterMock = jest.mocked(useAssetFiatFormatter); +jest.mock('../tokens/useTokenFiatRates', () => ({ + useTokenFiatRates: jest.fn(), +})); +const useTokenFiatRatesMock = jest.mocked(useTokenFiatRates); + +jest.mock('../../../../../core/Multichain/utils', () => ({ + isNonEvmChainId: jest.fn((chainId: string) => !chainId?.startsWith('0x')), +})); + jest.mock('../../../../../selectors/accountsController', () => ({ selectInternalAccountsById: jest.fn(), })); @@ -86,14 +96,18 @@ const mockAssets = { '0x1': [ { chainId: '0x1', + address: '0xtoken1', accountType: 'eip155:1/erc20:0xtoken1', + balance: '100.50', fiat: { balance: '100.50' }, rawBalance: '0x1234', symbol: 'TOKEN1', }, { chainId: '0x1', + address: '0xtoken2', accountType: 'eip155:1/erc20:0xtoken2', + balance: '0', fiat: { balance: '0' }, rawBalance: '0x0', symbol: 'TOKEN2', @@ -102,7 +116,9 @@ const mockAssets = { 'solana:mainnet': [ { chainId: 'solana:mainnet', + address: 'SolTokenPubkey1', accountType: 'solana:mainnet/spl:0xsoltoken1', + balance: '50.25', fiat: { balance: '50.25' }, rawBalance: '0x5678', symbol: 'SOLTOKEN1', @@ -142,6 +158,10 @@ describe('useAccountTokens', () => { fiatCurrency: 'USD', }); + useTokenFiatRatesMock.mockImplementation((requests) => + requests.map(() => 1), + ); + mockGetNetworkBadgeSource.mockReturnValue('network-badge-source'); mockIsTestNet.mockReturnValue(false); mockUseTokensData.mockReturnValue({}); @@ -192,27 +212,35 @@ describe('useAccountTokens', () => { }); }); - it('formats balance in selected currency', () => { + it('formats balance in selected currency for EVM assets', () => { const { result } = renderHook(() => useAccountTokens()); - result.current.forEach((asset) => { + const evmRows = result.current.filter((a) => + (a.chainId as string).startsWith('0x'), + ); + expect(evmRows.length).toBeGreaterThan(0); + evmRows.forEach((asset) => { expect(asset.balanceInSelectedCurrency).toBe('$100.50'); }); }); - it('passes each asset balance to the fiat formatter', () => { + it('passes each asset balance * fiat rate to the formatter', () => { const balanceAssets = { '0x1': [ { chainId: '0x1', + address: '0xtoken1', accountType: 'eip155:1/erc20:0xtoken1', + balance: '100', fiat: { balance: '100' }, rawBalance: '0x1234', symbol: 'TOKEN1', }, { chainId: '0x1', + address: '0xtoken2', accountType: 'eip155:1/erc20:0xtoken2', + balance: '50', fiat: { balance: '100.50' }, rawBalance: '0x5678', symbol: 'TOKEN2', @@ -230,11 +258,15 @@ describe('useAccountTokens', () => { } return undefined; }); + useTokenFiatRatesMock.mockReturnValue([2, 3]); renderHook(() => useAccountTokens()); - expect(mockFormatFiat).toHaveBeenCalledWith('100', '0x1'); - expect(mockFormatFiat).toHaveBeenCalledWith('100.50', '0x1'); + const calls = mockFormatFiat.mock.calls.map((args) => + args[0]?.toString(), + ); + expect(calls).toContain('200'); + expect(calls).toContain('150'); }); }); @@ -293,7 +325,9 @@ describe('useAccountTokens', () => { '0x1': [ { chainId: '0x1', + address: '0xtoken1', accountType: 'eip155:1/erc20:0xtoken1', + balance: '10', fiat: { balance: '10' }, rawBalance: '0x1234', symbol: 'MAINNET_TOKEN', @@ -302,7 +336,9 @@ describe('useAccountTokens', () => { '0xaa36a7': [ { chainId: '0xaa36a7', + address: '0xnative', accountType: 'eip155:11155111/slip44:60', + balance: '1000', fiat: { balance: '1000' }, rawBalance: '0x5678', symbol: 'SepoliaETH', @@ -942,16 +978,43 @@ describe('useAccountTokens', () => { }); describe('fiat formatter delegation', () => { - it('sets balanceInSelectedCurrency to the formatter output', () => { + it('sets balanceInSelectedCurrency to the formatter output for EVM assets', () => { mockFormatFiat.mockReturnValue('$110.00'); const { result } = renderHook(() => useAccountTokens()); - result.current.forEach((asset) => { + const evmRows = result.current.filter((a) => + (a.chainId as string).startsWith('0x'), + ); + expect(evmRows.length).toBeGreaterThan(0); + evmRows.forEach((asset) => { expect(asset.balanceInSelectedCurrency).toBe('$110.00'); }); }); + it('formats non-EVM assets using their preferred-currency fiat balance', () => { + mockFormatFiat.mockImplementation((v) => + v === undefined ? undefined : `formatted:${String(v)}`, + ); + + const { result } = renderHook(() => useAccountTokens()); + + const nonEvm = result.current.find((a) => a.chainId === 'solana:mainnet'); + expect(nonEvm?.balanceInSelectedCurrency).toBe('formatted:50.25'); + }); + + it('does not use useTokenFiatRates output for non-EVM assets', () => { + useTokenFiatRatesMock.mockReturnValue([999]); + mockFormatFiat.mockImplementation((v) => + v === undefined ? undefined : `formatted:${String(v)}`, + ); + + const { result } = renderHook(() => useAccountTokens()); + + const nonEvm = result.current.find((a) => a.chainId === 'solana:mainnet'); + expect(nonEvm?.balanceInSelectedCurrency).toBe('formatted:50.25'); + }); + it('propagates undefined from the formatter as a hidden fiat value', () => { mockFormatFiat.mockReturnValue(undefined); @@ -967,7 +1030,9 @@ describe('useAccountTokens', () => { '0x5': [ { chainId: '0x5', + address: '0xtoken1', accountType: 'eip155:5/erc20:0xtoken1', + balance: '100.50', fiat: { balance: '100.50' }, rawBalance: '0x1234', symbol: 'TOKEN1', diff --git a/app/components/Views/confirmations/hooks/send/useAccountTokens.ts b/app/components/Views/confirmations/hooks/send/useAccountTokens.ts index 42a8499783c5..ebea4e73c74e 100644 --- a/app/components/Views/confirmations/hooks/send/useAccountTokens.ts +++ b/app/components/Views/confirmations/hooks/send/useAccountTokens.ts @@ -18,6 +18,11 @@ import { buildEvmCaip19AssetId } from '../../../../../util/multichain/buildEvmCa import type { RootState } from '../../../../../reducers'; import { useTransactionAccountOverride } from '../transactions/useTransactionAccountOverride'; import { useAssetFiatFormatter } from '../pay/useAssetFiatFormatter'; +import { + TokenFiatRateRequest, + useTokenFiatRates, +} from '../tokens/useTokenFiatRates'; +import { isNonEvmChainId } from '../../../../../core/Multichain/utils'; import { isNetworkTestnet } from './useNetworkFilter'; export interface EnrichTokenRequest { @@ -72,22 +77,11 @@ export function useAccountTokens({ ); const showFiatOnTestnets = useSelector(selectShowFiatInTestnets); - const { format: formatFiat } = useAssetFiatFormatter(); + const { format: formatFiat, fiatCurrency } = useAssetFiatFormatter(); - const assetIds = useMemo( - () => - enrichTokenRequests.map((req) => - buildEvmCaip19AssetId(req.address, req.chainId), - ), - [enrichTokenRequests], - ); - - const tokensByAssetId = useTokensData(assetIds); - - return useMemo(() => { + const assetsWithBalance = useMemo(() => { const flatAssets = Object.values(assets).flat(); - - const assetsWithBalance = flatAssets.filter((asset) => { + return flatAssets.filter((asset) => { if (tokenFilter) { const address = asset.assetId; if ( @@ -115,7 +109,36 @@ export function useAccountTokens({ return haveBalance || isTestNetAsset; }); + }, [assets, includeNoBalance, tokenFilter]); + + // useTokenFiatRates crashes on non-hex addresses (Solana etc.), so we + // build requests for EVM assets only. Non-EVM assets are handled below in + // the render memo by falling back to `asset.fiat.balance`. + const fiatRateRequests = useMemo( + () => + assetsWithBalance + .filter((asset) => asset.chainId && !isNonEvmChainId(asset.chainId)) + .map((asset) => ({ + address: asset.address as Hex, + chainId: asset.chainId as Hex, + currency: fiatCurrency.toLowerCase(), + })), + [assetsWithBalance, fiatCurrency], + ); + + const fiatRates = useTokenFiatRates(fiatRateRequests); + const assetIds = useMemo( + () => + enrichTokenRequests.map((req) => + buildEvmCaip19AssetId(req.address, req.chainId), + ), + [enrichTokenRequests], + ); + + const tokensByAssetId = useTokensData(assetIds); + + return useMemo(() => { // "Show conversion on test networks" setting: when disabled, testnet // assets must not display fiat values nor be ranked by them. const isFiatHidden = (chainId?: string) => @@ -123,10 +146,31 @@ export function useAccountTokens({ Boolean(chainId) && isNetworkTestnet(chainId as string); + // EVM assets use the useTokenFiatRates-derived rate (picks up stablecoin + // bypass, and lets us convert to USD in pay flow). Non-EVM assets fall + // back to the assets-controller's preferred-currency `fiat.balance`, + // which is the pre-existing behavior for Solana/Bitcoin/etc. + // EVM assets and rates are in lockstep — both arrays derived from the + // same predicate over the same list, so a single index walk pairs them. + let evmIndex = 0; const processedAssets = assetsWithBalance.map((asset) => { - const balanceInSelectedCurrency = isFiatHidden(asset.chainId) - ? undefined - : formatFiat(asset.fiat?.balance, asset.chainId); + const isEvm = asset.chainId && !isNonEvmChainId(asset.chainId); + const rate = isEvm ? fiatRates[evmIndex++] : undefined; + + let fiatAmount: BigNumber.Value | undefined; + if (isFiatHidden(asset.chainId)) { + fiatAmount = undefined; + } else if (isEvm) { + fiatAmount = + rate !== undefined && asset.balance + ? new BigNumber(asset.balance).multipliedBy(rate) + : undefined; + } else { + fiatAmount = asset.fiat?.balance; + } + + const balanceInSelectedCurrency = + fiatAmount !== undefined ? formatFiat(fiatAmount) : undefined; return { ...asset, @@ -137,7 +181,7 @@ export function useAccountTokens({ }); if (enrichTokenRequests.length > 0) { - const zeroFiat = formatFiat(0, undefined) ?? ''; + const zeroFiat = formatFiat(0) ?? ''; const existingKeys = new Set( processedAssets.map( @@ -183,13 +227,12 @@ export function useAccountTokens({ (a, b) => sortableFiatBalance(b).comparedTo(sortableFiatBalance(a)) || 0, ); }, [ - assets, - includeNoBalance, + assetsWithBalance, showFiatOnTestnets, - tokenFilter, enrichTokenRequests, assetIds, tokensByAssetId, formatFiat, + fiatRates, ]) as unknown as AssetType[]; } From 1ad7b830e2af4bbbd1eacadbbe28711482a95e75 Mon Sep 17 00:00:00 2001 From: Goktug Poyraz Date: Wed, 15 Jul 2026 15:37:24 +0300 Subject: [PATCH 3/3] Fix comments and organise helpers --- .../hooks/send/useAccountTokens.test.ts | 124 +++++++++++++ .../hooks/send/useAccountTokens.ts | 164 +++++++++++------- 2 files changed, 230 insertions(+), 58 deletions(-) diff --git a/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts b/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts index 0bbe45c2d14c..9434143646d0 100644 --- a/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts +++ b/app/components/Views/confirmations/hooks/send/useAccountTokens.test.ts @@ -1056,4 +1056,128 @@ describe('useAccountTokens', () => { expect(mockFormatFiat).not.toHaveBeenCalled(); }); }); + + describe('sort key follows displayed fiat', () => { + it('sorts by derived fiat (balance * rate), not asset.fiat.balance', () => { + const stablecoinLikeAssets = { + '0x1': [ + { + chainId: '0x1', + address: '0xstable', + accountType: 'eip155:1/erc20:0xstable', + balance: '1000', + fiat: { balance: '998' }, + rawBalance: '0x1234', + symbol: 'USDC', + }, + { + chainId: '0x1', + address: '0xvolatile', + accountType: 'eip155:1/erc20:0xvolatile', + balance: '10', + fiat: { balance: '999' }, + rawBalance: '0x5678', + symbol: 'VOL', + }, + ], + }; + + mockSelectAssetsBySelectedAccountGroup.mockReturnValue( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + stablecoinLikeAssets as any, + ); + mockUseSelector.mockImplementation((selector) => { + if (selector === selectAssetsBySelectedAccountGroup) { + return stablecoinLikeAssets; + } + return undefined; + }); + // USDC rate = 1 (stablecoin bypass shape), VOL rate = 99.9. + // Derived fiat: USDC=1000*1=1000, VOL=10*99.9=999. + // asset.fiat.balance would sort VOL(999) above USDC(998). + // Correct sort by derived fiat puts USDC(1000) first. + useTokenFiatRatesMock.mockReturnValue([1, 99.9]); + + const { result } = renderHook(() => useAccountTokens()); + + expect(result.current[0].symbol).toBe('USDC'); + expect(result.current[1].symbol).toBe('VOL'); + }); + }); + + describe('zero balance with missing rate', () => { + it('renders $0 for an EVM zero-balance token even when useTokenFiatRates returns undefined', () => { + const zeroBalanceAssets = { + '0x1': [ + { + chainId: '0x1', + address: '0xtoken1', + accountType: 'eip155:1/erc20:0xtoken1', + balance: '0', + fiat: { balance: '0' }, + rawBalance: '0x0', + symbol: 'TOKEN1', + }, + ], + }; + + mockSelectAssetsBySelectedAccountGroup.mockReturnValue( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + zeroBalanceAssets as any, + ); + mockUseSelector.mockImplementation((selector) => { + if (selector === selectAssetsBySelectedAccountGroup) { + return zeroBalanceAssets; + } + return undefined; + }); + useTokenFiatRatesMock.mockReturnValue([undefined]); + mockFormatFiat.mockImplementation((v) => + v === undefined ? undefined : `formatted:${String(v)}`, + ); + + const { result } = renderHook(() => + useAccountTokens({ includeNoBalance: true }), + ); + + const token = result.current.find((a) => a.symbol === 'TOKEN1'); + expect(token?.balanceInSelectedCurrency).toBe('formatted:0'); + }); + + it('still hides fiat for a non-zero EVM balance when rate is missing', () => { + const nonZeroAssets = { + '0x1': [ + { + chainId: '0x1', + address: '0xtoken1', + accountType: 'eip155:1/erc20:0xtoken1', + balance: '10', + fiat: { balance: '10' }, + rawBalance: '0x1234', + symbol: 'TOKEN1', + }, + ], + }; + + mockSelectAssetsBySelectedAccountGroup.mockReturnValue( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + nonZeroAssets as any, + ); + mockUseSelector.mockImplementation((selector) => { + if (selector === selectAssetsBySelectedAccountGroup) { + return nonZeroAssets; + } + return undefined; + }); + useTokenFiatRatesMock.mockReturnValue([undefined]); + mockFormatFiat.mockImplementation((v) => + v === undefined ? undefined : `formatted:${String(v)}`, + ); + + const { result } = renderHook(() => useAccountTokens()); + + const token = result.current.find((a) => a.symbol === 'TOKEN1'); + expect(token?.balanceInSelectedCurrency).toBeUndefined(); + }); + }); }); diff --git a/app/components/Views/confirmations/hooks/send/useAccountTokens.ts b/app/components/Views/confirmations/hooks/send/useAccountTokens.ts index ebea4e73c74e..9d3e9a33cc74 100644 --- a/app/components/Views/confirmations/hooks/send/useAccountTokens.ts +++ b/app/components/Views/confirmations/hooks/send/useAccountTokens.ts @@ -1,3 +1,4 @@ +import type { Asset } from '@metamask/assets-controllers'; import { useSelector } from 'react-redux'; import { useCallback, useMemo } from 'react'; import { BigNumber } from 'bignumber.js'; @@ -32,30 +33,6 @@ export interface EnrichTokenRequest { const EMPTY_REQUESTS: EnrichTokenRequest[] = []; -function useAccountGroupAssets(accountAddress?: string | null) { - const internalAccountsById = useSelector(selectInternalAccountsById); - const accountToGroupMap = useSelector(selectAccountToGroupMap); - - const accountGroupId = useMemo(() => { - if (!accountAddress) return undefined; - const internalAccountId = Object.keys(internalAccountsById).find( - (id) => - internalAccountsById[id].address.toLowerCase() === - accountAddress.toLowerCase(), - ); - if (!internalAccountId) return undefined; - return accountToGroupMap[internalAccountId]?.id; - }, [accountAddress, internalAccountsById, accountToGroupMap]); - - const selectOverrideAssets = useCallback( - (state: RootState) => selectAssetsByAccountGroupId(state, accountGroupId), - [accountGroupId], - ); - - const overrideAssets = useSelector(selectOverrideAssets); - return accountGroupId ? overrideAssets : undefined; -} - export function useAccountTokens({ includeNoBalance = false, tokenFilter, @@ -111,18 +88,11 @@ export function useAccountTokens({ }); }, [assets, includeNoBalance, tokenFilter]); - // useTokenFiatRates crashes on non-hex addresses (Solana etc.), so we - // build requests for EVM assets only. Non-EVM assets are handled below in - // the render memo by falling back to `asset.fiat.balance`. const fiatRateRequests = useMemo( () => assetsWithBalance - .filter((asset) => asset.chainId && !isNonEvmChainId(asset.chainId)) - .map((asset) => ({ - address: asset.address as Hex, - chainId: asset.chainId as Hex, - currency: fiatCurrency.toLowerCase(), - })), + .filter(isEvmRateEligible) + .map((asset) => buildFiatRateRequest(asset, fiatCurrency)), [assetsWithBalance, fiatCurrency], ); @@ -146,38 +116,30 @@ export function useAccountTokens({ Boolean(chainId) && isNetworkTestnet(chainId as string); - // EVM assets use the useTokenFiatRates-derived rate (picks up stablecoin - // bypass, and lets us convert to USD in pay flow). Non-EVM assets fall - // back to the assets-controller's preferred-currency `fiat.balance`, - // which is the pre-existing behavior for Solana/Bitcoin/etc. - // EVM assets and rates are in lockstep — both arrays derived from the - // same predicate over the same list, so a single index walk pairs them. + const sortableFiatByAsset = new WeakMap(); let evmIndex = 0; const processedAssets = assetsWithBalance.map((asset) => { - const isEvm = asset.chainId && !isNonEvmChainId(asset.chainId); - const rate = isEvm ? fiatRates[evmIndex++] : undefined; - - let fiatAmount: BigNumber.Value | undefined; - if (isFiatHidden(asset.chainId)) { - fiatAmount = undefined; - } else if (isEvm) { - fiatAmount = - rate !== undefined && asset.balance - ? new BigNumber(asset.balance).multipliedBy(rate) - : undefined; - } else { - fiatAmount = asset.fiat?.balance; - } + const rate = isEvmRateEligible(asset) ? fiatRates[evmIndex++] : undefined; + const fiatAmount = deriveAssetFiat( + asset, + rate, + isFiatHidden(asset.chainId), + ); const balanceInSelectedCurrency = fiatAmount !== undefined ? formatFiat(fiatAmount) : undefined; - return { + const processed = { ...asset, networkBadgeSource: getNetworkBadgeSource(asset.chainId as Hex), balanceInSelectedCurrency, standard: TokenStandard.ERC20 as const, } as AssetType; + + if (fiatAmount !== undefined) { + sortableFiatByAsset.set(processed, fiatAmount); + } + return processed; }); if (enrichTokenRequests.length > 0) { @@ -218,10 +180,16 @@ export function useAccountTokens({ } } - const sortableFiatBalance = (asset: AssetType) => - isFiatHidden(asset.chainId) - ? new BigNumber(0) - : new BigNumber(asset.fiat?.balance || 0); + // Sort by the same fiat we display. Falls back to the assets-controller + // preferred-currency balance for tokens that have no derived fiat (e.g. + // enrichment placeholders added below with `zeroFiat`), so unknown + // tokens sink to the bottom. + const sortableFiatBalance = (asset: AssetType) => { + if (isFiatHidden(asset.chainId)) return new BigNumber(0); + const derived = sortableFiatByAsset.get(asset); + if (derived !== undefined) return derived; + return new BigNumber(asset.fiat?.balance || 0); + }; return processedAssets.sort( (a, b) => sortableFiatBalance(b).comparedTo(sortableFiatBalance(a)) || 0, @@ -236,3 +204,83 @@ export function useAccountTokens({ fiatRates, ]) as unknown as AssetType[]; } + +function useAccountGroupAssets(accountAddress?: string | null) { + const internalAccountsById = useSelector(selectInternalAccountsById); + const accountToGroupMap = useSelector(selectAccountToGroupMap); + + const accountGroupId = useMemo(() => { + if (!accountAddress) return undefined; + const internalAccountId = Object.keys(internalAccountsById).find( + (id) => + internalAccountsById[id].address.toLowerCase() === + accountAddress.toLowerCase(), + ); + if (!internalAccountId) return undefined; + return accountToGroupMap[internalAccountId]?.id; + }, [accountAddress, internalAccountsById, accountToGroupMap]); + + const selectOverrideAssets = useCallback( + (state: RootState) => selectAssetsByAccountGroupId(state, accountGroupId), + [accountGroupId], + ); + + const overrideAssets = useSelector(selectOverrideAssets); + return accountGroupId ? overrideAssets : undefined; +} + +/** + * Whether the asset is EVM-scoped with a hex address, i.e. eligible for + * `useTokenFiatRates` (which crashes on non-hex addresses like Solana). + * Non-EVM assets fall back to `asset.fiat.balance` in the render loop. + * + * This predicate is the single source of truth for the paired walks over + * `assetsWithBalance` — request build and rate consumption — so they cannot + * drift out of lockstep. + */ +function isEvmRateEligible(asset: Asset): boolean { + return ( + Boolean(asset.chainId) && + !isNonEvmChainId(asset.chainId) && + 'address' in asset && + Boolean(asset.address) + ); +} + +function buildFiatRateRequest( + asset: Asset, + currency: string, +): TokenFiatRateRequest { + return { + address: (asset as { address: Hex }).address, + chainId: asset.chainId as Hex, + currency: currency.toLowerCase(), + }; +} + +/** + * Fiat amount to display for a single asset, or `undefined` to hide. + * + * - Testnet-hidden → undefined. + * - EVM with rate → `balance * rate` (picks up stablecoin bypass). + * - EVM zero balance without rate → `0` (currency-invariant, avoids + * hiding `$0` rows when market data hasn't loaded). + * - EVM non-zero without rate → undefined (can't safely render). + * - Non-EVM → the assets-controller's preferred-currency `fiat.balance`. + */ +function deriveAssetFiat( + asset: Asset, + rate: number | undefined, + isFiatHidden: boolean, +): BigNumber | undefined { + if (isFiatHidden) return undefined; + + if (isEvmRateEligible(asset)) { + const balance = asset.balance ? new BigNumber(asset.balance) : undefined; + if (rate !== undefined && balance) return balance.multipliedBy(rate); + if (balance?.isZero()) return new BigNumber(0); + return undefined; + } + + return asset.fiat?.balance ? new BigNumber(asset.fiat.balance) : undefined; +}