Skip to content

Commit 9d2b478

Browse files
authored
fix: checksum EVM token addresses used throughout the bridge experience (#38971)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Changes - Updates the `toAssetId` util to **checksum EVM assetIds** and **transform Tron assets correctly** - Explicitly checksums addresses (if needed) before passing them to the bridge-controller - Removes unused bridge-status utils <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/38971?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: fix: toAssetId util should checksum EVM assetIds and support Tron tokens ## **Related issues** Fixes: ## **Manual testing steps** 1. Source and dest fiat values should be correct for evm and non-evm tokens 2. Block explorer link in dest input should link to correct URL ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > - **Asset ID + image handling**: Updates `toAssetId` to accept hex/CAIP chain IDs, checksum EVM addresses, and generate Solana `token:` and Tron `trc20:` asset IDs; `getAssetImageUrl` now lowercases EVM paths while preserving non‑EVM case and builds URLs from CAIP asset IDs. > - **Metadata fetch**: Refactors `fetchAssetMetadata`/`fetchAssetMetadataForAssetIds` to use new `toAssetId` and `getAssetImageUrl`, normalize chain IDs, and return non‑EVM addresses via CAIP references; adds `isEvmChainId` and `isTronResource` helpers. > - **Bridge integration**: Reworks selectors/utils to build assetIds with `toAssetId(fromToken.address, fromChain.chainId)`, generate images via CDN, and format quote params with CAIP references; updates tests/e2e to expect checksummed ERC20 assetIds/addresses and new image URLs. > - **Constants + cleanup**: Fixes Tron common token address format in `BRIDGE_CHAINID_COMMON_TOKEN_PAIR`; removes unused bridge-status metrics utils and related types. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 9e831bf. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent a41914f commit 9d2b478

File tree

20 files changed

+150
-235
lines changed

20 files changed

+150
-235
lines changed

app/scripts/lib/bridge-status/metrics-utils.ts

Lines changed: 0 additions & 69 deletions
This file was deleted.

shared/constants/bridge.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ export const BRIDGE_CHAINID_COMMON_TOKEN_PAIR: Partial<
204204
///: BEGIN:ONLY_INCLUDE_IF(tron)
205205
[MultichainNetworks.TRON]: {
206206
// TRX -> USDT on Tron
207-
address: 'tron:728126428/trc20:TR7NHqjeKQxGTCi8q8ZY4pL8otSzgjLj6t',
207+
address: 'TR7NHqjeKQxGTCi8q8ZY4pL8otSzgjLj6t',
208208
symbol: 'USDT',
209209
decimals: 6,
210210
name: 'Tether USD',

shared/lib/asset-utils.test.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ describe('asset-utils', () => {
128128
const chainId = 'eip155:1' as CaipChainId;
129129

130130
const result = toAssetId(address, chainId);
131-
expect(result).toBe(`eip155:1/erc20:${address.toLowerCase()}`);
131+
expect(result).toBe(`eip155:1/erc20:${address}`);
132132
expect(CaipAssetTypeStruct.validate(result)).toStrictEqual([
133133
undefined,
134134
result,
@@ -147,7 +147,8 @@ describe('asset-utils', () => {
147147
it('should return correct image URL for non-hex CAIP asset ID', () => {
148148
const assetId =
149149
`${MultichainNetworks.SOLANA}/token:aBCD` as CaipAssetType;
150-
const expectedUrl = `${STATIC_METAMASK_BASE_URL}/api/v2/tokenIcons/assets/solana/5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token/aBCD.png`;
150+
const expectedUrl =
151+
'https://static.cx.metamask.io/api/v2/tokenIcons/assets/solana/5eykt4usfv8p8njdtrepy1vzqkqzkvdp/token/abcd.png';
151152

152153
expect(getAssetImageUrl(assetId, 'eip155:1')).toBe(expectedUrl);
153154
});
@@ -188,7 +189,7 @@ describe('asset-utils', () => {
188189
);
189190

190191
expect(mockFetchWithTimeout).toHaveBeenCalledWith(
191-
`${TOKEN_API_V3_BASE_URL}/assets?assetIds=${mockAssetId + 'ABcDe'.toLowerCase()}`,
192+
`${TOKEN_API_V3_BASE_URL}/assets?assetIds=${mockAssetId + 'ABcDe'}`,
192193
{
193194
method: 'GET',
194195
headers: { 'X-Client-Id': 'extension' },
@@ -200,7 +201,7 @@ describe('asset-utils', () => {
200201
decimals: 18,
201202
image:
202203
'https://static.cx.metamask.io/api/v2/tokenIcons/assets/eip155/1/erc20/0x123abcde.png',
203-
assetId: 'eip155:1/erc20:0x123abcde',
204+
assetId: 'eip155:1/erc20:0x123ABcDe',
204205
address: '0x123abcde',
205206
chainId: mockHexChainId,
206207
});

shared/lib/asset-utils.ts

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,17 @@ import {
1010
parseCaipAssetType,
1111
KnownCaipNamespace,
1212
} from '@metamask/utils';
13-
13+
import { toChecksumHexAddress } from '@metamask/controller-utils';
1414
import { toEvmCaipChainId } from '@metamask/multichain-network-controller';
15-
import { MultichainNetwork } from '@metamask/multichain-transactions-controller';
1615
import {
1716
getNativeAssetForChainId,
1817
isNativeAddress,
18+
isNonEvmChainId,
1919
} from '@metamask/bridge-controller';
2020
import { Asset } from '@metamask/assets-controllers';
2121
import getFetchWithTimeout from '../modules/fetch-with-timeout';
2222
import { decimalToPrefixedHex } from '../modules/conversion.utils';
23+
import { MultichainNetworks } from '../constants/multichain/networks';
2324
import {
2425
TRON_RESOURCE_SYMBOLS_SET,
2526
TronResourceSymbol,
@@ -31,21 +32,38 @@ const STATIC_METAMASK_BASE_URL = 'https://static.cx.metamask.io';
3132

3233
export const toAssetId = (
3334
address: Hex | CaipAssetType | string,
34-
chainId: CaipChainId,
35+
chainId?: CaipChainId | Hex,
3536
): CaipAssetType | undefined => {
37+
let addressToUse = address;
38+
let chainIdToUse = isStrictHexString(chainId)
39+
? toEvmCaipChainId(chainId)
40+
: chainId;
41+
42+
// Use chainId and address from caip assetId if provided
3643
if (isCaipAssetType(address)) {
37-
return address;
44+
const { assetReference, chainId: chainIdFromCaipAssetId } =
45+
parseCaipAssetType(address);
46+
addressToUse = assetReference;
47+
chainIdToUse = chainIdFromCaipAssetId;
3848
}
39-
if (isNativeAddress(address)) {
40-
return getNativeAssetForChainId(chainId)?.assetId;
49+
if (!chainIdToUse) {
50+
return undefined;
4151
}
42-
if (chainId === MultichainNetwork.Solana) {
43-
return CaipAssetTypeStruct.create(`${chainId}/token:${address}`);
52+
53+
if (isNativeAddress(addressToUse)) {
54+
return getNativeAssetForChainId(chainIdToUse)?.assetId;
55+
}
56+
if (chainIdToUse === MultichainNetworks.SOLANA) {
57+
return CaipAssetTypeStruct.create(`${chainIdToUse}/token:${addressToUse}`);
58+
}
59+
if (chainIdToUse === MultichainNetworks.TRON) {
60+
return CaipAssetTypeStruct.create(`${chainIdToUse}/trc20:${addressToUse}`);
4461
}
4562
// EVM assets
46-
if (isStrictHexString(address)) {
63+
const checksummedAddress = toChecksumHexAddress(addressToUse) ?? addressToUse;
64+
if (isStrictHexString(checksummedAddress)) {
4765
return CaipAssetTypeStruct.create(
48-
`${chainId}/erc20:${address.toLowerCase()}`,
66+
`${chainIdToUse}/erc20:${checksummedAddress}`,
4967
);
5068
}
5169
return undefined;
@@ -62,19 +80,17 @@ export const getAssetImageUrl = (
6280
assetId: CaipAssetType | string,
6381
chainId: CaipChainId | Hex,
6482
) => {
65-
const chainIdInCaip = isCaipChainId(chainId)
66-
? chainId
67-
: toEvmCaipChainId(chainId);
68-
69-
const assetIdInCaip = toAssetId(assetId, chainIdInCaip);
83+
const assetIdInCaip = toAssetId(assetId, chainId);
7084
if (!assetIdInCaip) {
7185
return undefined;
7286
}
87+
const normalizedAssetId = (
88+
isNonEvmChainId(chainId) ? assetIdInCaip : assetIdInCaip.toLowerCase()
89+
).replaceAll(':', '/');
7390

74-
return `${STATIC_METAMASK_BASE_URL}/api/v2/tokenIcons/assets/${assetIdInCaip.replaceAll(
75-
':',
76-
'/',
77-
)}.png`;
91+
return `${STATIC_METAMASK_BASE_URL}/api/v2/tokenIcons/assets/${
92+
normalizedAssetId
93+
}.png`;
7894
};
7995

8096
export type AssetMetadata = {
@@ -98,11 +114,7 @@ export const fetchAssetMetadata = async (
98114
abortSignal?: AbortSignal,
99115
) => {
100116
try {
101-
const chainIdInCaip = isCaipChainId(chainId)
102-
? chainId
103-
: toEvmCaipChainId(chainId);
104-
105-
const assetId = toAssetId(address, chainIdInCaip);
117+
const assetId = toAssetId(address, chainId);
106118

107119
if (!assetId) {
108120
return undefined;
@@ -123,11 +135,11 @@ export const fetchAssetMetadata = async (
123135
const commonFields = {
124136
symbol: assetMetadata.symbol,
125137
decimals: assetMetadata.decimals,
126-
image: getAssetImageUrl(assetId, chainIdInCaip),
138+
image: getAssetImageUrl(assetId, chainId),
127139
assetId,
128140
};
129141

130-
if (chainId === MultichainNetwork.Solana && assetId) {
142+
if (isNonEvmChainId(chainId) && assetId) {
131143
const { assetReference } = parseCaipAssetType(assetId);
132144
return {
133145
...commonFields,
@@ -137,11 +149,13 @@ export const fetchAssetMetadata = async (
137149
};
138150
}
139151

140-
const { reference } = parseCaipChainId(chainIdInCaip);
152+
const hexChainId = isStrictHexString(chainId)
153+
? chainId
154+
: decimalToPrefixedHex(parseCaipChainId(chainId).reference);
141155
return {
142156
...commonFields,
143157
address: address.toLowerCase(),
144-
chainId: decimalToPrefixedHex(reference),
158+
chainId: hexChainId,
145159
};
146160
} catch (error) {
147161
return undefined;

shared/types/bridge-status.ts

Lines changed: 0 additions & 16 deletions
This file was deleted.

shared/types/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
export type { FlattenedBackgroundStateProxy } from './background';
2-
export * from './bridge-status';
32
export * from './confirm';
43
export * from './metametrics';
54
export * from './origin-throttling';

test/e2e/tests/metrics/bridge.spec.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { strict as assert } from 'assert';
22
import { Suite } from 'mocha';
3+
import { toChecksumHexAddress } from '@metamask/controller-utils';
34
import {
45
assertInAnyOrder,
56
getEventPayloads,
@@ -154,7 +155,7 @@ describe('Bridge tests', function (this: Suite) {
154155
event.properties.chain_id_source === 'eip155:1' &&
155156
event.properties.chain_id_destination === 'eip155:59144' &&
156157
event.properties.token_address_source ===
157-
'eip155:1/erc20:0x6b175474e89094c44da98b954eedeac495271d0f' &&
158+
`eip155:1/erc20:${toChecksumHexAddress('0x6b175474e89094c44da98b954eedeac495271d0f')}` &&
158159
event.properties.token_address_destination ===
159160
'eip155:59144/slip44:60' &&
160161
event.properties.swap_type === 'crosschain' &&
@@ -175,7 +176,7 @@ describe('Bridge tests', function (this: Suite) {
175176
crossChainQuotesReceived[0].properties.chain_id_destination ===
176177
'eip155:59144' &&
177178
crossChainQuotesReceived[0].properties.token_address_source ===
178-
'eip155:1/erc20:0x6b175474e89094c44da98b954eedeac495271d0f' &&
179+
`eip155:1/erc20:${toChecksumHexAddress('0x6b175474e89094c44da98b954eedeac495271d0f')}` &&
179180
crossChainQuotesReceived[0].properties.token_address_destination ===
180181
'eip155:59144/slip44:60' &&
181182
crossChainQuotesReceived[0].properties.swap_type === 'crosschain',

ui/ducks/bridge-status/selectors.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
11
import { createSelector } from 'reselect';
2-
import { type BridgeHistoryItem } from '@metamask/bridge-status-controller';
2+
import type {
3+
BridgeHistoryItem,
4+
BridgeStatusControllerState,
5+
} from '@metamask/bridge-status-controller';
36
import { Numeric } from '../../../shared/modules/Numeric';
47
import { getCurrentChainId } from '../../../shared/modules/selectors/networks';
5-
import { type BridgeStatusAppState } from '../../../shared/types/bridge-status';
68
import { getSwapsTokensReceivedFromTxMeta } from '../../../shared/lib/transactions-controller-utils';
79
import {
810
getInternalAccountsFromGroupById,
911
getSelectedAccountGroup,
1012
} from '../../selectors/multichain-accounts/account-tree';
1113

14+
type BridgeStatusAppState = {
15+
metamask: BridgeStatusControllerState;
16+
};
17+
1218
const selectBridgeHistory = (state: BridgeStatusAppState) =>
1319
state.metamask.txHistory;
1420

ui/ducks/bridge/selectors.test.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,8 @@ describe('Bridge selectors', () => {
282282
assetId: 'eip155:1/slip44:60',
283283
chainId: 'eip155:1',
284284
decimals: 18,
285-
image: './images/eth_logo.svg',
285+
image:
286+
'https://static.cx.metamask.io/api/v2/tokenIcons/assets/eip155/1/slip44/60.png',
286287
name: 'Ether',
287288
symbol: 'ETH',
288289
balance: '0',
@@ -300,7 +301,8 @@ describe('Bridge selectors', () => {
300301
assetId: 'eip155:1/slip44:60',
301302
chainId: 'eip155:1',
302303
decimals: 18,
303-
image: './images/eth_logo.svg',
304+
image:
305+
'https://static.cx.metamask.io/api/v2/tokenIcons/assets/eip155/1/slip44/60.png',
304306
name: 'Ether',
305307
symbol: 'ETH',
306308
balance: '0',
@@ -349,7 +351,7 @@ describe('Bridge selectors', () => {
349351

350352
expect(result).toStrictEqual({
351353
address: '0xaca92e438df0b2401ff60da7e4337b687a2435da',
352-
assetId: 'eip155:1/erc20:0xaca92e438df0b2401ff60da7e4337b687a2435da',
354+
assetId: 'eip155:1/erc20:0xacA92E438df0B2401fF60dA7E4337B687a2435DA',
353355
balance: '0',
354356
chainId: 'eip155:1',
355357
decimals: 6,
@@ -443,7 +445,8 @@ describe('Bridge selectors', () => {
443445
chainId: 'eip155:1',
444446
decimals: 18,
445447
iconUrl: '',
446-
image: './images/eth_logo.svg',
448+
image:
449+
'https://static.cx.metamask.io/api/v2/tokenIcons/assets/eip155/1/slip44/60.png',
447450
name: 'Ether',
448451
symbol: 'ETH',
449452
});

ui/ducks/bridge/selectors.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -603,10 +603,7 @@ export const getFromTokenConversionRate = createSelector(
603603
const nativeAssetId = getNativeAssetForChainId(
604604
fromChain.chainId,
605605
)?.assetId;
606-
const tokenAssetId = toAssetId(
607-
fromToken.address,
608-
formatChainIdToCaip(fromChain.chainId),
609-
);
606+
const tokenAssetId = toAssetId(fromToken.address, fromChain.chainId);
610607
const nativeToCurrencyRate = isNonEvmChain(fromChain.chainId)
611608
? Number(
612609
rates?.[fromChain.nativeCurrency?.toLowerCase()]?.conversionRate ??

0 commit comments

Comments
 (0)