Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/bridge-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Bump `@metamask/assets-controllers` from `^108.2.0` to `^108.3.0` ([#8941](https://github.com/MetaMask/core/pull/8941))
- Bump `@metamask/assets-controller` from `^8.1.0` to `^8.2.0` ([#8943](https://github.com/MetaMask/core/pull/8943))

### Fixed

- `selectExchangeRateByAssetId` now resolves fungible asset fiat rates from the unified `AssetsController` `assetsPrice` source, so held tokens (e.g. USDC) produce a fiat value when `useAssetsControllerForRates` is enabled instead of resolving no rate

## [73.2.0]

### Added
Expand Down
108 changes: 108 additions & 0 deletions packages/bridge-controller/src/selectors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { AddressZero } from '@ethersproject/constants';
import type { MarketDataDetails } from '@metamask/assets-controllers';
import { toHex } from '@metamask/controller-utils';
import { SolScope } from '@metamask/keyring-api';
import type { CaipAssetType } from '@metamask/utils';
import { BigNumber } from 'bignumber.js';

import mockQuotesErc20Erc20 from '../tests/mock-quotes-erc20-erc20.json';
Expand Down Expand Up @@ -188,6 +189,113 @@ describe('Bridge Selectors', () => {
),
).toStrictEqual({});
});

describe('assetsPrice resolution (unified assets)', () => {
// DAI, not present in any other rate source in mockExchangeRateSources.
const DAI_ADDRESS = '0x6B175474E89094C44Da98b954EedeAC495271d0F';
const daiAssetIdChecksummed = formatAddressToAssetId(
DAI_ADDRESS,
'1',
) as CaipAssetType;

it('resolves a fungible rate from assetsPrice when legacy slices lack it', () => {
const result = selectExchangeRateByAssetId(
{
...mockExchangeRateSources,
assetsPrice: {
[daiAssetIdChecksummed]: {
assetPriceType: 'fungible',
price: 1.23,
usdPrice: 1.5,
},
},
} as unknown as BridgeAppState,
daiAssetIdChecksummed,
);
expect(result).toStrictEqual({
exchangeRate: '1.23',
usdExchangeRate: '1.5',
});
});

it('matches a checksummed assetsPrice key when the asset ID is lowercased', () => {
const result = selectExchangeRateByAssetId(
{
...mockExchangeRateSources,
assetsPrice: {
[daiAssetIdChecksummed]: {
assetPriceType: 'fungible',
price: 1.23,
usdPrice: 1.5,
},
},
} as unknown as BridgeAppState,
`eip155:1/erc20:${DAI_ADDRESS.toLowerCase()}`,
);
expect(result).toStrictEqual({
exchangeRate: '1.23',
usdExchangeRate: '1.5',
});
});

it('ignores non-fungible (NFT) assetsPrice entries', () => {
const result = selectExchangeRateByAssetId(
{
...mockExchangeRateSources,
assetsPrice: {
[daiAssetIdChecksummed]: {
assetPriceType: 'nft',
floorPrice: 1.23,
},
},
} as unknown as BridgeAppState,
daiAssetIdChecksummed,
);
expect(result).toStrictEqual({});
});

it('ignores assetsPrice entries with non-finite price or usdPrice', () => {
const result = selectExchangeRateByAssetId(
{
...mockExchangeRateSources,
assetsPrice: {
[daiAssetIdChecksummed]: {
assetPriceType: 'fungible',
price: Number.NaN,
usdPrice: undefined,
},
},
} as unknown as BridgeAppState,
daiAssetIdChecksummed,
);
expect(result).toStrictEqual({});
});

it('prefers bridge controller assetExchangeRates over assetsPrice', () => {
const usdcAssetId = formatAddressToAssetId(
MOCK_USDC_ADDRESS,
'1',
) as CaipAssetType;
const result = selectExchangeRateByAssetId(
{
...mockExchangeRateSources,
assetsPrice: {
[usdcAssetId]: {
assetPriceType: 'fungible',
price: 9.99,
usdPrice: 9.99,
},
},
} as unknown as BridgeAppState,
usdcAssetId,
);
// assetExchangeRates entry for USDC (2.5 / 1.5) wins.
expect(result).toStrictEqual({
exchangeRate: '2.5',
usdExchangeRate: '1.5',
});
});
});
});

describe('selectIsAssetExchangeRateInState', () => {
Expand Down
35 changes: 31 additions & 4 deletions packages/bridge-controller/src/selectors.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* eslint-disable @typescript-eslint/explicit-function-return-type */
import { getAddress } from '@ethersproject/address';
import { normalizeAssetId } from '@metamask/assets-controller';
import type { AssetsControllerState } from '@metamask/assets-controller';
import type {
CurrencyRateState,
MultichainAssetsRatesControllerState,
Expand Down Expand Up @@ -62,7 +64,8 @@ import { getDefaultSlippagePercentage } from './utils/slippage';
type ExchangeRateControllerState = MultichainAssetsRatesControllerState &
TokenRatesControllerState &
CurrencyRateState &
Pick<BridgeControllerState, 'assetExchangeRates'>;
Pick<BridgeControllerState, 'assetExchangeRates'> &
Partial<Pick<AssetsControllerState, 'assetsPrice'>>;
/**
* The state of the bridge controller and all its dependency controllers
*/
Expand All @@ -81,7 +84,8 @@ export type ExchangeRateSourcesForLookup = Pick<
'assetExchangeRates'
> &
Partial<Pick<CurrencyRateState, 'currencyRates'>> &
Partial<Pick<MultichainAssetsRatesControllerState, 'historicalPrices'>> & {
Partial<Pick<MultichainAssetsRatesControllerState, 'historicalPrices'>> &
Partial<Pick<AssetsControllerState, 'assetsPrice'>> & {
marketData?:
| TokenRatesControllerState['marketData']
| Record<string, Record<string, { price?: number; currency?: string }>>;
Expand Down Expand Up @@ -173,8 +177,13 @@ export const selectExchangeRateByAssetId = (
return {};
}

const { assetExchangeRates, currencyRates, marketData, conversionRates } =
exchangeRateSources;
const {
assetExchangeRates,
currencyRates,
marketData,
conversionRates,
assetsPrice,
} = exchangeRateSources;

// If the asset exchange rate is available in the bridge controller, use it
// This is defined if the token's rate is not available from the assets controllers
Expand All @@ -188,6 +197,24 @@ export const selectExchangeRateByAssetId = (
return bridgeControllerRate;
}

// When the unified assets system is active, held-token fiat rates live in
// `assetsPrice` (keyed by checksummed CAIP-19 asset ID) rather than the
// legacy market/currency-rate slices. Resolve fungible prices directly so
// held tokens (e.g. USDC) produce a fiat value. `normalizeAssetId` checksums
// EVM ERC-20 references so a lower-cased input matches the stored key.
const assetPrice =
assetsPrice?.[normalizeAssetId(assetId)] ?? assetsPrice?.[assetId];
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Malformed ERC20 lookup throws

Medium Severity

selectExchangeRateByAssetId now always calls normalizeAssetId before legacy rate paths. For invalid short EVM ERC-20 references, normalizeAssetId checksums via toChecksumAddress, which throws on non–40-character hex. That breaks the prior contract of returning {} without throwing for malformed asset IDs.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ffab58f. Configure here.

if (
assetPrice?.assetPriceType === 'fungible' &&
Number.isFinite(assetPrice.price) &&
Number.isFinite(assetPrice.usdPrice)
) {
return {
exchangeRate: String(assetPrice.price),
usdExchangeRate: String(assetPrice.usdPrice),
};
}

const { chainId } = parseCaipAssetType(assetId);

// If the chain is a non-EVM chain, use the conversion rate from the multichain assets controller
Expand Down
Loading