Skip to content

Commit 5ceea8b

Browse files
authored
chore: remove allDetectedTokens Earn references (#30237)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until this PR meets the canonical Definition of Ready For Review in `docs/readme/ready-for-review.md`. In short: the template must be materially complete (not just section titles present), all status checks must be currently passing, and the only expected follow-up commits must be reviewer-driven. --> ## **Description** <!-- 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? --> `allDetectedTokens` has been deprecated and empty for a long time. We are now removing all remaining references to that piece of state since we are in the process of deprecating many assets controllers. This should not have any effect in the app, as the content of that piece of state is always empty. ## **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: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/ASSETS-3197 ## **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** <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [X] 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). - [X] I've completed the PR template to the best of my ability - [X] I've included tests if applicable - [X] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [X] 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. #### Performance checks (if applicable) - [ ] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [ ] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [ ] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] 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** > Changes Earn network polling behavior by removing the `TokensController.allDetectedTokens` -> `addTokens` import path and by always triggering `detectTokens` on mount/account change, which could affect token-detection frequency and performance across lending chains. > > **Overview** > Simplifies `useEarnNetworkPolling` by removing all usage of deprecated `TokensController.allDetectedTokens` and the follow-up `TokensController.addTokens` import flow; the hook now only triggers `TokenDetectionController.detectTokens`. > > Polling hooks are updated to use the static lending `LENDING_CHAIN_IDS` list directly (no local state), and tests are adjusted accordingly, including dropping `addTokens` expectations and removing `allDetectedTokens` from Earn hook test fixtures. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3d4bf79. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 51ee6aa commit 5ceea8b

4 files changed

Lines changed: 20 additions & 165 deletions

File tree

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

Lines changed: 13 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,11 @@ jest.mock('../../../../core/Engine', () => ({
2323
TokenDetectionController: {
2424
detectTokens: jest.fn(),
2525
},
26-
TokensController: {
27-
addTokens: jest.fn(),
28-
},
2926
},
3027
}));
3128

29+
import { toHex } from '@metamask/controller-utils';
30+
import { CHAIN_ID_TO_AAVE_POOL_CONTRACT } from '@metamask/stake-sdk';
3231
import { renderHookWithProvider } from '../../../../util/test/renderWithProvider';
3332
import useEarnNetworkPolling from './useEarnNetworkPolling';
3433
import { RootState } from '../../../../reducers';
@@ -39,6 +38,10 @@ import useTokenRatesPolling from '../../../hooks/AssetPolling/useTokenRatesPolli
3938
import useTokenDetectionPolling from '../../../hooks/AssetPolling/useTokenDetectionPolling';
4039
import Engine from '../../../../core/Engine';
4140

41+
const LENDING_CHAIN_IDS = Object.keys(CHAIN_ID_TO_AAVE_POOL_CONTRACT).map(
42+
(chainId) => toHex(chainId),
43+
);
44+
4245
// Mock console.warn to avoid noise in tests
4346
const originalConsoleWarn = console.warn;
4447
beforeAll(() => {
@@ -61,7 +64,6 @@ describe('useEarnNetworkPolling', () => {
6164
const mockDetectTokens = jest.mocked(
6265
Engine.context.TokenDetectionController.detectTokens,
6366
);
64-
const mockAddTokens = jest.mocked(Engine.context.TokensController.addTokens);
6567

6668
const mockSelectedAccount =
6769
MOCK_ACCOUNTS_CONTROLLER_STATE.internalAccounts.accounts[
@@ -91,32 +93,6 @@ describe('useEarnNetworkPolling', () => {
9193
PreferencesController: {
9294
useTokenDetection: true,
9395
},
94-
TokensController: {
95-
allDetectedTokens: {
96-
'0x1': {
97-
[mockSelectedAccount.address]: {
98-
'0x123': {
99-
address: '0x123',
100-
symbol: 'TEST',
101-
decimals: 18,
102-
image: 'test-image.png',
103-
name: 'Test Token',
104-
},
105-
},
106-
},
107-
'0x89': {
108-
[mockSelectedAccount.address]: {
109-
'0x456': {
110-
address: '0x456',
111-
symbol: 'TEST2',
112-
decimals: 6,
113-
image: 'test2-image.png',
114-
name: 'Test Token 2',
115-
},
116-
},
117-
},
118-
},
119-
},
12096
},
12197
},
12298
} as unknown as RootState;
@@ -129,7 +105,6 @@ describe('useEarnNetworkPolling', () => {
129105
throw new Error(`Network client not found for chain ${chainId}`);
130106
});
131107
mockDetectTokens.mockResolvedValue(undefined);
132-
mockAddTokens.mockResolvedValue(undefined);
133108
});
134109

135110
it('should call all polling hooks when mounted', () => {
@@ -152,23 +127,24 @@ describe('useEarnNetworkPolling', () => {
152127
});
153128
});
154129

155-
it('should initialize with empty chain IDs and network client IDs', () => {
130+
it('should initialize with lending chain IDs', () => {
156131
renderHookWithProvider(() => useEarnNetworkPolling(), {
157132
state: mockState,
158133
});
159134

160-
// Initially called with empty arrays
135+
const expectedChainIds = expect.arrayContaining(LENDING_CHAIN_IDS);
136+
161137
expect(mockUseTokenBalancesPolling).toHaveBeenCalledWith({
162-
chainIds: [],
138+
chainIds: expectedChainIds,
163139
});
164140
expect(mockUseCurrencyRatePolling).toHaveBeenCalledWith({
165-
chainIds: [],
141+
chainIds: expectedChainIds,
166142
});
167143
expect(mockUseTokenRatesPolling).toHaveBeenCalledWith({
168-
chainIds: [],
144+
chainIds: expectedChainIds,
169145
});
170146
expect(mockUseTokenDetectionPolling).toHaveBeenCalledWith({
171-
chainIds: [],
147+
chainIds: expectedChainIds,
172148
address: mockSelectedAccount.address,
173149
});
174150
});
@@ -242,54 +218,6 @@ describe('useEarnNetworkPolling', () => {
242218
});
243219
});
244220

245-
it('should call TokensController.addTokens for detected tokens', async () => {
246-
renderHookWithProvider(() => useEarnNetworkPolling(), {
247-
state: mockState,
248-
});
249-
250-
// Wait for async operations to complete
251-
await new Promise((resolve) => setTimeout(resolve, 0));
252-
253-
// Verify tokens are added (order may vary based on LENDING_CHAIN_IDS)
254-
expect(mockAddTokens).toHaveBeenCalledWith(
255-
[
256-
{
257-
address: '0x123',
258-
symbol: 'TEST',
259-
decimals: 18,
260-
image: 'test-image.png',
261-
name: 'Test Token',
262-
isERC721: false,
263-
},
264-
],
265-
'mainnet',
266-
);
267-
});
268-
269-
it('should not call addTokens when no detected tokens', async () => {
270-
const stateWithoutDetectedTokens = {
271-
...mockState,
272-
engine: {
273-
...mockState.engine,
274-
backgroundState: {
275-
...mockState.engine.backgroundState,
276-
TokensController: {
277-
allDetectedTokens: {},
278-
},
279-
},
280-
},
281-
} as unknown as RootState;
282-
283-
renderHookWithProvider(() => useEarnNetworkPolling(), {
284-
state: stateWithoutDetectedTokens,
285-
});
286-
287-
// Wait for async operations to complete
288-
await new Promise((resolve) => setTimeout(resolve, 0));
289-
290-
expect(mockAddTokens).not.toHaveBeenCalled();
291-
});
292-
293221
it('should pass empty chainIds to useTokenDetectionPolling when useTokenDetection is false', () => {
294222
const stateWithoutTokenDetection = {
295223
...mockState,
@@ -314,19 +242,6 @@ describe('useEarnNetworkPolling', () => {
314242
});
315243
});
316244

317-
it('should handle addTokens errors gracefully', async () => {
318-
mockAddTokens.mockRejectedValue(new Error('Failed to add tokens'));
319-
320-
expect(() => {
321-
renderHookWithProvider(() => useEarnNetworkPolling(), {
322-
state: mockState,
323-
});
324-
}).not.toThrow();
325-
326-
// Wait for async operations to complete
327-
await new Promise((resolve) => setTimeout(resolve, 0));
328-
});
329-
330245
it('should handle detectTokens errors gracefully', async () => {
331246
mockDetectTokens.mockRejectedValue(new Error('Failed to detect tokens'));
332247

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

Lines changed: 7 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
import { Token } from '@metamask/assets-controllers';
21
import { toHex } from '@metamask/controller-utils';
32
import { CHAIN_ID_TO_AAVE_POOL_CONTRACT } from '@metamask/stake-sdk';
43
import { Hex } from '@metamask/utils';
5-
import { useEffect, useState } from 'react';
4+
import { useEffect } from 'react';
65
import { useSelector } from 'react-redux';
76
import Engine from '../../../../core/Engine';
87
import { selectSelectedInternalAccountByScope } from '../../../../selectors/multichainAccounts/accounts';
@@ -11,7 +10,6 @@ import useCurrencyRatePolling from '../../../hooks/AssetPolling/useCurrencyRateP
1110
import useTokenBalancesPolling from '../../../hooks/AssetPolling/useTokenBalancesPolling';
1211
import useTokenDetectionPolling from '../../../hooks/AssetPolling/useTokenDetectionPolling';
1312
import useTokenRatesPolling from '../../../hooks/AssetPolling/useTokenRatesPolling';
14-
import { RootState } from '../../BasicFunctionality/BasicFunctionalityModal/BasicFunctionalityModal.test';
1513
import { EVM_SCOPE } from '../constants/networks';
1614

1715
/**
@@ -55,78 +53,22 @@ export const useEarnNetworkPolling = () => {
5553
EVM_SCOPE,
5654
);
5755
const useTokenDetection = useSelector(selectUseTokenDetection);
58-
const tokensState = useSelector(
59-
(state: RootState) => state.engine?.backgroundState?.TokensController,
60-
);
61-
const [lendingChainIds, setLendingChainIds] = useState<Hex[]>([]);
6256

63-
useTokenBalancesPolling({ chainIds: lendingChainIds });
64-
useCurrencyRatePolling({ chainIds: lendingChainIds });
65-
useTokenRatesPolling({ chainIds: lendingChainIds });
57+
useTokenBalancesPolling({ chainIds: LENDING_CHAIN_IDS });
58+
useCurrencyRatePolling({ chainIds: LENDING_CHAIN_IDS });
59+
useTokenRatesPolling({ chainIds: LENDING_CHAIN_IDS });
6660
useTokenDetectionPolling({
67-
chainIds: useTokenDetection ? lendingChainIds : [],
61+
chainIds: useTokenDetection ? LENDING_CHAIN_IDS : [],
6862
address: selectedAccount?.address as Hex,
6963
});
7064

71-
useEffect(() => {
72-
const validChainIds: Hex[] = [];
73-
74-
LENDING_CHAIN_IDS.forEach((chainId) => {
75-
validChainIds.push(chainId);
76-
});
77-
78-
setLendingChainIds(validChainIds);
79-
}, [setLendingChainIds]);
80-
8165
// Import tokens from all lending chains
8266
useEffect(() => {
83-
const importLendingTokens = async () => {
84-
if (!selectedAccount?.address || !useTokenDetection) return;
85-
86-
const { TokensController } = Engine.context;
87-
const allDetectedTokens = tokensState?.allDetectedTokens || {};
88-
89-
for (const chainId of LENDING_CHAIN_IDS) {
90-
const chainDetectedTokens =
91-
allDetectedTokens[chainId]?.[selectedAccount.address];
92-
if (
93-
chainDetectedTokens &&
94-
Object.keys(chainDetectedTokens).length > 0
95-
) {
96-
const tokensToImport = Object.values(chainDetectedTokens).map(
97-
(token: Token) => ({
98-
address: token.address,
99-
symbol: token.symbol,
100-
decimals: token.decimals,
101-
image: token.image,
102-
name: token.name,
103-
isERC721: false,
104-
}),
105-
);
106-
107-
const networkClientId =
108-
Engine.context.NetworkController.findNetworkClientIdByChainId(
109-
chainId,
110-
);
111-
112-
if (networkClientId && tokensToImport.length > 0) {
113-
await TokensController.addTokens(tokensToImport, networkClientId);
114-
}
115-
}
116-
}
117-
};
118-
11967
Engine.context.TokenDetectionController.detectTokens({
12068
chainIds: LENDING_CHAIN_IDS,
12169
selectedAddress: selectedAccount?.address as Hex,
122-
})
123-
.then(importLendingTokens)
124-
.catch(console.error);
125-
}, [
126-
tokensState?.allDetectedTokens,
127-
selectedAccount?.address,
128-
useTokenDetection,
129-
]);
70+
}).catch(console.error);
71+
}, [selectedAccount?.address]);
13072

13173
return null;
13274
};

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ const mockState = {
163163
},
164164
},
165165
allIgnoredTokens: {},
166-
allDetectedTokens: {},
167166
} as TokensControllerState,
168167
TokenBalancesController: {
169168
tokenBalances: {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,6 @@ const mockState = {
174174
},
175175
},
176176
allIgnoredTokens: {},
177-
allDetectedTokens: {},
178177
} as TokensControllerState,
179178
TokenBalancesController: {
180179
tokenBalances: {

0 commit comments

Comments
 (0)