Skip to content

Commit 3cc9298

Browse files
juanmigdrjuanmigdr
authored
chore: remove usage of tokensChainsCache in token details page (#28533)
<!-- 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** Stops using the token list cache (`tokensChainsCache`) on the token details section of the asset overview. Market data now comes from token rates only, and contract decimals and aggregator labels come from the asset itself, so token details no longer depend on merging token list metadata with market data. The old `selectEvmTokenMarketData` selector was removed and tests were updated accordingly. <!-- 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? --> ## **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: Stopped using the token list cache for asset overview token details; decimals, aggregators, and market data now come from the asset and token rates ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-3027 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **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 Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/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-mobile/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] > **Medium Risk** > Moderate risk because it changes how token details/market data are sourced and displayed, which could affect what appears on the asset overview for tokens with missing/incorrect asset fields. No auth/security-sensitive logic involved. > > **Overview** > Updates the asset overview `TokenDetails` flow to **stop merging token-list metadata with market data**: EVM market data is now read directly from `selectTokenMarketData` (or fetched via the spot-prices API when not cached), and token decimals/aggregator labels come from the `asset` itself. > > Removes the `selectEvmTokenMarketData` selector and simplifies `getTokenDetails` to no longer accept metadata, adjusting the `TokenDetailsList` render condition to rely on `contractAddress`/`decimals` presence. Related unit tests are updated to mock `selectTokenMarketData` and validate the new behavior. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b75296f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent ad0f41c commit 3cc9298

5 files changed

Lines changed: 40 additions & 156 deletions

File tree

‎app/components/UI/AssetOverview/TokenDetails/TokenDetails.test.tsx‎

Lines changed: 14 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -151,17 +151,10 @@ describe('TokenDetails', () => {
151151
useSelectorSpy.mockImplementation((selectorOrCallback) => {
152152
const SELECTOR_MOCKS = {
153153
selectIsEvmNetworkSelected: true,
154-
selectEvmTokenMarketData: {
155-
marketData:
156-
mockTokenMarketDataByChainId['0x1'][
157-
'0x6B175474E89094C44Da98b954EedeAC495271d0F'
158-
],
159-
metadata: {
160-
decimals: 18,
161-
conversionRate: 2712.15,
162-
aggregators: ['Metamask', 'Coinmarketcap'],
163-
},
164-
},
154+
selectTokenMarketData:
155+
mockTokenMarketDataByChainId['0x1'][
156+
'0x6B175474E89094C44Da98b954EedeAC495271d0F'
157+
],
165158
selectTokenMarketDataByChainId: {},
166159
selectConversionRateBySymbol: mockExchangeRate,
167160
selectNativeCurrencyByChainId: 'ETH',
@@ -242,17 +235,6 @@ describe('TokenDetails', () => {
242235
selectConversionRateBySymbol: mockExchangeRate,
243236
selectNativeCurrencyByChainId: 'ETH',
244237
selectMultichainAssetsRates: {},
245-
selectEvmTokenMarketData: {
246-
marketData:
247-
mockTokenMarketDataByChainId['0x1'][
248-
'0x6b175474e89094c44da98b954eedeac495271d0f'
249-
],
250-
metadata: {
251-
decimals: 18,
252-
conversionRate: 2712.15,
253-
aggregators: ['Metamask', 'Coinmarketcap'],
254-
},
255-
},
256238
} as const;
257239

258240
useSelectorSpy.mockImplementation((selectorOrCallback) => {
@@ -301,16 +283,10 @@ describe('TokenDetails', () => {
301283
const useSelectorSpy = jest.spyOn(reactRedux, 'useSelector');
302284
useSelectorSpy.mockImplementation((selectorOrCallback) => {
303285
const SELECTOR_MOCKS = {
304-
selectEvmTokenMarketData: {
305-
marketData:
306-
mockTokenMarketDataByChainId['0x1'][
307-
'0x6B175474E89094C44Da98b954EedeAC495271d0F'
308-
],
309-
// null metadata ensures:
310-
// 1. tokenList is null (no aggregators array in metadata)
311-
// 2. tokenMetadata is null (so tokenMetadata condition is false)
312-
metadata: null,
313-
},
286+
selectTokenMarketData:
287+
mockTokenMarketDataByChainId['0x1'][
288+
'0x6B175474E89094C44Da98b954EedeAC495271d0F'
289+
],
314290
selectConversionRateBySymbol: mockExchangeRate,
315291
selectNativeCurrencyByChainId: 'ETH',
316292
} as const;
@@ -408,7 +384,7 @@ describe('TokenDetails', () => {
408384

409385
jest.spyOn(reactRedux, 'useSelector').mockImplementation(
410386
mockUseSelectorImplementation({
411-
selectEvmTokenMarketData: null,
387+
selectTokenMarketData: null,
412388
selectConversionRateBySymbol: mockExchangeRate,
413389
selectNativeCurrencyByChainId: 'ETH',
414390
isEvmNetwork: true,
@@ -436,7 +412,7 @@ describe('TokenDetails', () => {
436412

437413
jest.spyOn(reactRedux, 'useSelector').mockImplementation(
438414
mockUseSelectorImplementation({
439-
selectEvmTokenMarketData: null,
415+
selectTokenMarketData: null,
440416
isEvmNetwork: false,
441417
}),
442418
);
@@ -457,11 +433,9 @@ describe('TokenDetails', () => {
457433

458434
jest.spyOn(reactRedux, 'useSelector').mockImplementation(
459435
mockUseSelectorImplementation({
460-
selectEvmTokenMarketData: {
461-
marketData: {
462-
price: 0.0005,
463-
marketCap: '5000000',
464-
},
436+
selectTokenMarketData: {
437+
price: 0.0005,
438+
marketCap: '5000000',
465439
},
466440
selectConversionRateBySymbol: mockExchangeRate,
467441
selectNativeCurrencyByChainId: 'ETH',
@@ -483,7 +457,7 @@ describe('TokenDetails', () => {
483457

484458
jest.spyOn(reactRedux, 'useSelector').mockImplementation(
485459
mockUseSelectorImplementation({
486-
selectEvmTokenMarketData: null,
460+
selectTokenMarketData: null,
487461
multichainRates: {
488462
[mockSolanaToken.address]: {
489463
rate: 0.431111,

‎app/components/UI/AssetOverview/TokenDetails/TokenDetails.tsx‎

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727
isAssetFromSearch,
2828
selectTokenDisplayData,
2929
} from '../../../../selectors/tokenSearchDiscoveryDataController';
30-
import { selectEvmTokenMarketData } from '../../../../selectors/multichain/evm';
30+
import { selectTokenMarketData } from '../../../../selectors/tokenRatesController';
3131
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
3232
import { selectMultichainAssetsRates } from '../../../../selectors/multichain';
3333
///: END:ONLY_INCLUDE_IF
@@ -59,11 +59,6 @@ interface TokenDetailsProps {
5959
asset: TokenI;
6060
}
6161

62-
interface EvmMarketData {
63-
metadata?: Record<string, string | number | string[]>;
64-
marketData?: MarketDataDetails;
65-
}
66-
6762
const TokenDetails: React.FC<TokenDetailsProps> = ({ asset }) => {
6863
// For non evm assets, the resultChainId is equal to the asset.chainId; while for evm assets; the resultChainId === "eip155:1" !== asset.chainId
6964
const resultChainId = formatChainIdToCaip(asset.chainId as Hex);
@@ -96,17 +91,15 @@ const TokenDetails: React.FC<TokenDetailsProps> = ({ asset }) => {
9691
///: END:ONLY_INCLUDE_IF
9792

9893
const evmMarketData = useSelector((state: RootState) =>
99-
!isNonEvmAsset
100-
? selectEvmTokenMarketData(state, {
101-
chainId: asset.chainId as Hex,
102-
tokenAddress: asset.address,
103-
})
94+
!isNonEvmAsset && tokenContractAddress
95+
? (selectTokenMarketData(state)?.[asset.chainId as Hex]?.[
96+
tokenContractAddress as Hex
97+
] ?? null)
10498
: null,
105-
) as EvmMarketData | null;
99+
) as MarketDataDetails | null;
106100

107101
const conversionRate = isAssetFromSearch(asset) ? 1 : conversionRateBySymbol;
108102

109-
let tokenMetadata;
110103
let cachedMarketData: MarketDataDetails | undefined;
111104

112105
if (
@@ -116,11 +109,9 @@ const TokenDetails: React.FC<TokenDetailsProps> = ({ asset }) => {
116109
) {
117110
// Search results have market data
118111
cachedMarketData = tokenSearchResult.price;
119-
tokenMetadata = tokenSearchResult.token;
120112
} else {
121-
tokenMetadata = !isNonEvmAsset ? evmMarketData?.metadata : null;
122113
cachedMarketData = !isNonEvmAsset
123-
? evmMarketData?.marketData
114+
? (evmMarketData ?? undefined)
124115
: (nonEvmMarketData as MarketDataDetails | undefined);
125116
}
126117

@@ -181,14 +172,8 @@ const TokenDetails: React.FC<TokenDetailsProps> = ({ asset }) => {
181172
const needsConversion = isUsingCachedData && !isNonEvmAsset;
182173

183174
const tokenDetails = useMemo(
184-
() =>
185-
getTokenDetails(
186-
asset,
187-
isNonEvmAsset,
188-
tokenContractAddress,
189-
tokenMetadata as Record<string, string | number | string[]>,
190-
),
191-
[asset, isNonEvmAsset, tokenContractAddress, tokenMetadata],
175+
() => getTokenDetails(asset, isNonEvmAsset, tokenContractAddress),
176+
[asset, isNonEvmAsset, tokenContractAddress],
192177
);
193178

194179
const marketDetails = useMemo(() => {
@@ -231,13 +216,10 @@ const TokenDetails: React.FC<TokenDetailsProps> = ({ asset }) => {
231216
}, [marketData, currentCurrency, needsConversion, conversionRate]);
232217

233218
const hasAddressAndDecimals =
234-
tokenDetails.contractAddress && tokenDetails.tokenDecimal;
219+
tokenDetails.contractAddress !== null && tokenDetails.tokenDecimal !== null;
235220
return (
236221
<View style={styles.tokenDetailsContainer}>
237-
{(asset.isETH ||
238-
tokenMetadata ||
239-
isNonEvmAsset ||
240-
hasAddressAndDecimals) && (
222+
{(asset.isETH || isNonEvmAsset || hasAddressAndDecimals) && (
241223
<TokenDetailsList tokenDetails={tokenDetails} />
242224
)}
243225
{marketData && marketDetails && (

‎app/components/UI/AssetOverview/utils/getTokenDetails.test.ts‎

Lines changed: 12 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,6 @@ describe('getTokenDetails', () => {
2525
logo: 'https://example.com/logo.png',
2626
};
2727

28-
const mockEvmMetadata = {
29-
decimals: 18,
30-
aggregators: ['uniswap', '1inch'],
31-
};
32-
3328
beforeEach(() => {
3429
// Clear mock calls before each test for proper isolation
3530
(parseCaipAssetType as jest.Mock).mockClear();
@@ -51,7 +46,6 @@ describe('getTokenDetails', () => {
5146
mockAsset,
5247
true, // isNonEvmAsset
5348
undefined,
54-
mockEvmMetadata,
5549
);
5650

5751
expect(result).toEqual({
@@ -75,7 +69,6 @@ describe('getTokenDetails', () => {
7569
mockAsset,
7670
true, // isNonEvmAsset
7771
undefined,
78-
mockEvmMetadata,
7972
);
8073

8174
expect(result).toEqual({
@@ -95,7 +88,6 @@ describe('getTokenDetails', () => {
9588
ethAsset,
9689
false, // isNonEvmAsset
9790
undefined,
98-
mockEvmMetadata,
9991
);
10092

10193
expect(result).toEqual({
@@ -106,12 +98,7 @@ describe('getTokenDetails', () => {
10698
});
10799

108100
it('should format regular token details for EVM networks', () => {
109-
const result = getTokenDetails(
110-
mockAsset,
111-
false,
112-
'0x456',
113-
mockEvmMetadata,
114-
);
101+
const result = getTokenDetails(mockAsset, false, '0x456');
115102

116103
expect(result).toEqual({
117104
contractAddress: '0x456',
@@ -141,7 +128,6 @@ describe('getTokenDetails', () => {
141128
solanaAsset,
142129
true, // isNonEvmAsset
143130
undefined,
144-
mockEvmMetadata,
145131
);
146132

147133
// Verify parseCaipAssetType was called with the converted CAIP format
@@ -179,7 +165,6 @@ describe('getTokenDetails', () => {
179165
solanaAssetWithCaipAddress,
180166
true, // isNonEvmAsset
181167
undefined,
182-
mockEvmMetadata,
183168
);
184169

185170
// Verify parseCaipAssetType was called with the original CAIP address (no conversion needed)
@@ -215,7 +200,6 @@ describe('getTokenDetails', () => {
215200
assetWithoutAddress,
216201
true, // isNonEvmAsset
217202
undefined,
218-
mockEvmMetadata,
219203
);
220204

221205
expect(result).toEqual({
@@ -226,39 +210,30 @@ describe('getTokenDetails', () => {
226210
});
227211
});
228212

229-
describe('Metadata handling', () => {
230-
it('should handle missing decimals in token metadata', () => {
231-
const metadataWithoutDecimals = {
232-
aggregators: ['uniswap'],
233-
};
234-
213+
describe('Asset property handling for EVM tokens', () => {
214+
it('should return null tokenDecimal when asset has no decimals', () => {
235215
const { decimals, ...assetWithoutDecimals } = mockAsset;
236216

237217
const result = getTokenDetails(
238218
assetWithoutDecimals as TokenI,
239219
false,
240220
'0x456',
241-
metadataWithoutDecimals,
242221
);
243222

244223
expect(result).toEqual({
245224
contractAddress: '0x456',
246225
tokenDecimal: null,
247-
tokenList: 'uniswap',
226+
tokenList: 'uniswap, 1inch',
248227
});
249228
});
250229

251-
it('should handle missing aggregators in token metadata', () => {
252-
const metadataWithoutAggregators = {
253-
decimals: 18,
230+
it('should return null tokenList when asset has no aggregators', () => {
231+
const assetWithoutAggregators: TokenI = {
232+
...mockAsset,
233+
aggregators: undefined as unknown as string[],
254234
};
255235

256-
const result = getTokenDetails(
257-
mockAsset,
258-
false,
259-
'0x456',
260-
metadataWithoutAggregators,
261-
);
236+
const result = getTokenDetails(assetWithoutAggregators, false, '0x456');
262237

263238
expect(result).toEqual({
264239
contractAddress: '0x456',
@@ -267,23 +242,13 @@ describe('getTokenDetails', () => {
267242
});
268243
});
269244

270-
it('should handle invalid aggregators type in token metadata', () => {
271-
const metadataWithInvalidAggregators = {
272-
decimals: 18,
273-
aggregators: 'uniswap' as unknown as string[],
274-
};
275-
276-
const result = getTokenDetails(
277-
mockAsset,
278-
false, // isNonEvmAsset
279-
'0x456',
280-
metadataWithInvalidAggregators,
281-
);
245+
it('should return aggregators joined as tokenList', () => {
246+
const result = getTokenDetails(mockAsset, false, '0x456');
282247

283248
expect(result).toEqual({
284249
contractAddress: '0x456',
285250
tokenDecimal: 18,
286-
tokenList: null,
251+
tokenList: 'uniswap, 1inch',
287252
});
288253
});
289254
});
@@ -308,7 +273,6 @@ describe('getTokenDetails', () => {
308273
assetWithoutDecimals,
309274
true, // isNonEvmAsset
310275
undefined,
311-
mockEvmMetadata,
312276
);
313277

314278
expect(result).toEqual({
@@ -337,7 +301,6 @@ describe('getTokenDetails', () => {
337301
assetWithoutAggregators,
338302
true, // isNonEvmAsset
339303
undefined,
340-
mockEvmMetadata,
341304
);
342305

343306
expect(result).toEqual({

‎app/components/UI/AssetOverview/utils/getTokenDetails.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ export const getTokenDetails = (
77
asset: TokenI,
88
isNonEvmAsset: boolean,
99
tokenContractAddress: string | undefined,
10-
tokenMetadata: Record<string, string | number | string[]>,
1110
): TokenDetails => {
1211
if (isNonEvmAsset) {
1312
// Use the same approach as useTokenHistoricalPrices
@@ -38,12 +37,9 @@ export const getTokenDetails = (
3837
}
3938
return {
4039
contractAddress: tokenContractAddress ?? null,
41-
tokenDecimal:
42-
typeof tokenMetadata?.decimals === 'number'
43-
? tokenMetadata.decimals
44-
: (asset.decimals ?? null),
45-
tokenList: Array.isArray(tokenMetadata?.aggregators)
46-
? tokenMetadata.aggregators.join(', ')
40+
tokenDecimal: asset.decimals ?? null,
41+
tokenList: Array.isArray(asset.aggregators)
42+
? asset.aggregators.join(', ')
4743
: null,
4844
};
4945
};

0 commit comments

Comments
 (0)