Skip to content

Commit aafd5e0

Browse files
authored
chore: remove usage of tokensChainsCache from useMusdConversionStatus (#28644)
<!-- 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** Removes the usage of `selectERC20TokensByChain` (which reads from `tokensChainsCache`) in `useMusdConversionStatus`. This is part of a broader effort to eliminate all usages of `tokensChainsCache` from the codebase so it can eventually be removed from `TokenListController`. Previously, `getTokenData` inside the hook's `useEffect` looked up a token's `symbol` and `name` from the full `tokensChainsCache` via a `useSelector` + `ref` pattern. This required subscribing to the entire chain-indexed token list on every render. The replacement uses `selectSingleTokenByAddressAndChainId` called directly against `store.getState()` at the moment the transaction event fires — the same point-in-time store access pattern already used in this hook for `getTransactionPayQuotes`. Since `metamaskPay.tokenAddress` is always a token the user holds in their wallet, it is tracked in `TokensController.allTokens`, making the `tokensChainsCache` lookup redundant. Also removes the now-unnecessary `safeToChecksumAddress` import (previously used to handle both checksummed and lowercase address lookups, which `selectSingleTokenByAddressAndChainId` handles internally), and drops the `tokensCacheRef` ref pattern along with the `useSelector` / `react-redux` import. <!-- 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: remove usage of tokensChainsCache from useMusdConversionStatus ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-2956 ## **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 metadata is resolved for mUSD conversion toasts/metrics (switching data sources and lookup behavior), which could alter displayed symbols and analytics properties in edge cases. > > **Overview** > `useMusdConversionStatus` no longer reads `tokensChainsCache` via `react-redux`/`selectERC20TokensByChain`; it now pulls token `symbol`/`name` on-demand from `store.getState()` using `selectSingleTokenByAddressAndChainId`. > > Tests are updated to mock the new selector-based lookup, remove the old chain token-cache setup (including lowercase/checksum fallback coverage), and adjust the fallback case to treat missing wallet tokens as `"Token"`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 9de0418. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 96ee8a5 commit aafd5e0

File tree

2 files changed

+31
-118
lines changed

2 files changed

+31
-118
lines changed

app/components/UI/Earn/hooks/useMusdConversionStatus.test.ts

Lines changed: 22 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,8 @@ jest.mock('./useEarnToasts');
1818
jest.mock('../../../hooks/useAnalytics/useAnalytics', () => ({
1919
useAnalytics: jest.fn(),
2020
}));
21-
jest.mock('react-redux', () => ({
22-
useSelector: jest.fn(),
23-
}));
24-
jest.mock('../../../../selectors/tokenListController', () => ({
25-
selectERC20TokensByChain: jest.fn(),
21+
jest.mock('../../../../selectors/tokensController', () => ({
22+
selectSingleTokenByAddressAndChainId: jest.fn(),
2623
}));
2724
jest.mock('../../../../util/transactions', () => {
2825
const actual = jest.requireActual('../../../../util/transactions');
@@ -32,9 +29,6 @@ jest.mock('../../../../util/transactions', () => {
3229
};
3330
});
3431
jest.mock('../../../../util/networks', () => ({}));
35-
jest.mock('../../../../selectors/networkController', () => ({
36-
selectEvmNetworkConfigurationsByChainId: jest.fn(),
37-
}));
3832
jest.mock('../../../../util/trace', () => ({
3933
trace: jest.fn(),
4034
endTrace: jest.fn(),
@@ -54,12 +48,10 @@ jest.mock('../../../../selectors/transactionPayController', () => ({
5448
selectTransactionPayQuotesByTransactionId: jest.fn(),
5549
}));
5650

57-
import { useSelector } from 'react-redux';
58-
import { selectERC20TokensByChain } from '../../../../selectors/tokenListController';
51+
import { selectSingleTokenByAddressAndChainId } from '../../../../selectors/tokensController';
5952
import { MetaMetricsEvents } from '../../../../core/Analytics';
6053
import { useAnalytics } from '../../../hooks/useAnalytics/useAnalytics';
6154
import { decodeTransferData } from '../../../../util/transactions';
62-
import { selectEvmNetworkConfigurationsByChainId } from '../../../../selectors/networkController';
6355
import {
6456
trace,
6557
endTrace,
@@ -79,13 +71,11 @@ const mockSelectTransactionPayQuotesByTransactionId = jest.mocked(
7971
selectTransactionPayQuotesByTransactionId,
8072
);
8173

82-
const mockUseSelector = jest.mocked(useSelector);
83-
const mockSelectERC20TokensByChain = jest.mocked(selectERC20TokensByChain);
74+
const mockSelectSingleTokenByAddressAndChainId = jest.mocked(
75+
selectSingleTokenByAddressAndChainId,
76+
);
8477
const mockUseAnalytics = jest.mocked(useAnalytics);
8578
const mockDecodeTransferData = jest.mocked(decodeTransferData);
86-
const mockSelectEvmNetworkConfigurationsByChainId = jest.mocked(
87-
selectEvmNetworkConfigurationsByChainId,
88-
);
8979

9080
type TransactionStatusUpdatedHandler = (event: {
9181
transactionMeta: TransactionMeta;
@@ -192,9 +182,6 @@ describe('useMusdConversionStatus', () => {
192182
},
193183
};
194184

195-
// Default mock data
196-
const defaultTokensChainsCache = {};
197-
198185
beforeEach(() => {
199186
jest.clearAllMocks();
200187
jest.useFakeTimers();
@@ -223,28 +210,18 @@ describe('useMusdConversionStatus', () => {
223210
EarnToastOptions: mockEarnToastOptions,
224211
});
225212

226-
// Setup useSelector to return different values based on selector
227-
mockUseSelector.mockImplementation((selector) => {
228-
if (selector === mockSelectERC20TokensByChain) {
229-
return defaultTokensChainsCache;
230-
}
231-
if (selector === mockSelectEvmNetworkConfigurationsByChainId) {
232-
return { '0x1': { name: 'Ethereum Mainnet' } };
233-
}
234-
return {};
235-
});
213+
// Default: token not found
214+
mockSelectSingleTokenByAddressAndChainId.mockReturnValue(undefined);
236215
});
237216

238-
// Helper to setup token cache mock
239-
const setupTokensCacheMock = (tokenData: Record<string, unknown>) => {
240-
mockUseSelector.mockImplementation((selector) => {
241-
if (selector === mockSelectERC20TokensByChain) {
242-
return tokenData;
243-
}
244-
if (selector === mockSelectEvmNetworkConfigurationsByChainId) {
245-
return { '0x1': { name: 'Ethereum Mainnet' } };
246-
}
247-
return {};
217+
// Helper to setup token lookup mock for a specific address+chain
218+
const setupTokenLookupMock = (symbol: string, name = '') => {
219+
mockSelectSingleTokenByAddressAndChainId.mockReturnValue({
220+
symbol,
221+
name,
222+
address: '',
223+
decimals: 18,
224+
aggregators: [],
248225
});
249226
};
250227

@@ -364,14 +341,7 @@ describe('useMusdConversionStatus', () => {
364341
it('passes token symbol from metamaskPay data to in-progress toast', () => {
365342
const tokenAddress = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
366343
const chainId = '0x89';
367-
const mockTokenData = {
368-
[chainId]: {
369-
data: {
370-
[tokenAddress]: { symbol: 'USDC' },
371-
},
372-
},
373-
};
374-
setupTokensCacheMock(mockTokenData);
344+
setupTokenLookupMock('USDC');
375345

376346
renderHook(() => useMusdConversionStatus());
377347

@@ -390,41 +360,10 @@ describe('useMusdConversionStatus', () => {
390360
});
391361
});
392362

393-
it('uses lowercase token address as fallback for symbol lookup', () => {
394-
const tokenAddress = '0x6B175474E89094C44Da98b954EedeAC495271d0F';
395-
const chainId = '0x1';
396-
const mockTokenData = {
397-
[chainId]: {
398-
data: {
399-
[tokenAddress.toLowerCase()]: { symbol: 'DAI' },
400-
},
401-
},
402-
};
403-
setupTokensCacheMock(mockTokenData);
404-
405-
renderHook(() => useMusdConversionStatus());
406-
407-
const handler = getSubscribedHandler();
408-
const transactionMeta = createTransactionMeta(
409-
TransactionStatus.approved,
410-
'test-tx-lowercase',
411-
TransactionType.musdConversion,
412-
{ chainId, tokenAddress },
413-
);
414-
415-
handler({ transactionMeta });
416-
417-
expect(mockInProgressFn).toHaveBeenCalledWith(
418-
expect.objectContaining({ tokenSymbol: 'DAI' }),
419-
);
420-
});
421-
422-
it('uses "Token" as fallback when token symbol is not found', () => {
363+
it('uses "Token" as fallback when token is not found in wallet', () => {
423364
const tokenAddress = '0x1111111111111111111111111111111111111111';
424365
const chainId = '0x1';
425-
setupTokensCacheMock({
426-
[chainId]: { data: {} },
427-
});
366+
mockSelectSingleTokenByAddressAndChainId.mockReturnValue(undefined);
428367

429368
renderHook(() => useMusdConversionStatus());
430369

@@ -856,13 +795,7 @@ describe('useMusdConversionStatus', () => {
856795
it('tracks status updated event when transaction status is approved', () => {
857796
const tokenAddress = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
858797
const chainId = '0x1';
859-
setupTokensCacheMock({
860-
[chainId]: {
861-
data: {
862-
[tokenAddress]: { symbol: 'USDC', name: 'USD Coin' },
863-
},
864-
},
865-
});
798+
setupTokenLookupMock('USDC', 'USD Coin');
866799

867800
renderHook(() => useMusdConversionStatus());
868801

@@ -902,13 +835,7 @@ describe('useMusdConversionStatus', () => {
902835
it('tracks status updated event when transaction status is confirmed', () => {
903836
const tokenAddress = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
904837
const chainId = '0x1';
905-
setupTokensCacheMock({
906-
[chainId]: {
907-
data: {
908-
[tokenAddress]: { symbol: 'USDC', name: 'USD Coin' },
909-
},
910-
},
911-
});
838+
setupTokenLookupMock('USDC', 'USD Coin');
912839

913840
renderHook(() => useMusdConversionStatus());
914841

@@ -948,13 +875,7 @@ describe('useMusdConversionStatus', () => {
948875
it('tracks status updated event when transaction status is failed', () => {
949876
const tokenAddress = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48';
950877
const chainId = '0x1';
951-
setupTokensCacheMock({
952-
[chainId]: {
953-
data: {
954-
[tokenAddress]: { symbol: 'USDC', name: 'USD Coin' },
955-
},
956-
},
957-
});
878+
setupTokenLookupMock('USDC', 'USD Coin');
958879

959880
renderHook(() => useMusdConversionStatus());
960881

app/components/UI/Earn/hooks/useMusdConversionStatus.ts

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@ import {
55
} from '@metamask/transaction-controller';
66
import { Hex } from '@metamask/utils';
77
import { useCallback, useEffect, useRef } from 'react';
8-
import { useSelector } from 'react-redux';
98
import Engine from '../../../../core/Engine';
10-
import { selectERC20TokensByChain } from '../../../../selectors/tokenListController';
11-
import { safeToChecksumAddress } from '../../../../util/address';
9+
import { selectSingleTokenByAddressAndChainId } from '../../../../selectors/tokensController';
1210
import useEarnToasts from './useEarnToasts';
1311
import { useAnalytics } from '../../../hooks/useAnalytics/useAnalytics';
1412
import { MetaMetricsEvents } from '../../../../core/Analytics';
@@ -47,13 +45,10 @@ function getTransactionPayQuotes(transactionId: string) {
4745
*/
4846
export const useMusdConversionStatus = () => {
4947
const { showToast, EarnToastOptions } = useEarnToasts();
50-
const tokensChainsCache = useSelector(selectERC20TokensByChain);
5148

5249
const { trackEvent, createEventBuilder } = useAnalytics();
5350

5451
const shownToastsRef = useRef<Set<string>>(new Set());
55-
const tokensCacheRef = useRef(tokensChainsCache);
56-
tokensCacheRef.current = tokensChainsCache;
5752

5853
const submitConversionEvent = useCallback(
5954
(
@@ -102,18 +97,15 @@ export const useMusdConversionStatus = () => {
10297

10398
useEffect(() => {
10499
const getTokenData = (chainId: Hex, tokenAddress: string) => {
105-
const chainTokens = tokensCacheRef.current?.[chainId]?.data;
106-
if (!chainTokens) return { symbol: '', name: '' };
107-
108-
const checksumAddress = safeToChecksumAddress(tokenAddress);
109-
const tokenData =
110-
chainTokens[checksumAddress as string] ||
111-
chainTokens[tokenAddress.toLowerCase()];
112-
100+
const state = store.getState();
101+
const token = selectSingleTokenByAddressAndChainId(
102+
state,
103+
tokenAddress as Hex,
104+
chainId,
105+
);
113106
return {
114-
symbol: tokenData?.symbol || '',
115-
iconUrl: tokenData?.iconUrl,
116-
name: tokenData?.name || '',
107+
symbol: token?.symbol || '',
108+
name: token?.name || '',
117109
};
118110
};
119111

0 commit comments

Comments
 (0)