Skip to content

Commit 3e1cdcf

Browse files
authored
feat: replace assets state references for card (#29722)
<!-- 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? --> The new `AssetsController` is being introduced to replace most controllers from `@metamask/assets-controllers`. This is one of many PRs that replace direct access to legacy state with selectors that, using a feature flag, handle the transition between the legacy state and the new state when the flag is turned on. ## **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-2827 ## **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** > Moderate risk because it changes how card token balances and fiat values are sourced (from `Engine.context` to Redux selectors), which could affect displayed balances/pricing if selector wiring or feature-flagged state differs across networks. > > **Overview** > Updates `useAssetBalances` to stop reading rates/market data directly from `Engine.context` and instead use feature-flag-aware selectors from `selectors/assets/assets-migration` (including `allTokens`, currency rates, token market data, and multichain conversion rates). > > Adjusts `useAssetBalances.test.ts` to mock these values via `useSelector` state (not controller state mutation), including a new default selector state and a Solana conversion-rate test that injects `MultichainAssetsRatesController` into the mocked Redux state. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 177704b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent ec11b69 commit 3e1cdcf

2 files changed

Lines changed: 67 additions & 60 deletions

File tree

app/components/UI/Card/hooks/useAssetBalances.test.ts

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -126,38 +126,39 @@ describe('useAssetBalances', () => {
126126
walletAddress: '0xwallet1',
127127
};
128128

129+
const defaultSelectorMockState = {
130+
engine: {
131+
backgroundState: {
132+
TokensController: {
133+
allTokens: {},
134+
allDetectedTokens: {},
135+
},
136+
NetworkController: {
137+
networkConfigurationsByChainId: {
138+
'0xe708': {
139+
nativeCurrency: 'ETH',
140+
},
141+
},
142+
},
143+
CurrencyRateController: {
144+
currencyRates: {
145+
ETH: {
146+
conversionRate: 2000,
147+
},
148+
},
149+
},
150+
},
151+
},
152+
};
153+
129154
beforeEach(() => {
130155
jest.clearAllMocks();
131156

132157
// Default mock implementations
133158
mockUseSelector.mockImplementation((selector: any) => {
134159
if (typeof selector === 'function') {
135160
// Mock state structure - includes TokensController for the refactored useAssetBalances
136-
const state = {
137-
engine: {
138-
backgroundState: {
139-
TokensController: {
140-
allTokens: {},
141-
allDetectedTokens: {},
142-
},
143-
NetworkController: {
144-
networkConfigurationsByChainId: {
145-
'0xe708': {
146-
nativeCurrency: 'ETH',
147-
},
148-
},
149-
},
150-
CurrencyRateController: {
151-
currencyRates: {
152-
ETH: {
153-
conversionRate: 2000,
154-
},
155-
},
156-
},
157-
},
158-
},
159-
};
160-
return selector(state);
161+
return selector(defaultSelectorMockState);
161162
}
162163
return 'USD';
163164
});
@@ -264,14 +265,29 @@ describe('useAssetBalances', () => {
264265
});
265266

266267
it('returns balance info for single Solana token with conversion rate', () => {
267-
(
268-
Engine.context.MultichainAssetsRatesController as any
269-
).state.conversionRates = {
270-
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v':
271-
{
272-
rate: '1.0',
273-
},
274-
};
268+
mockUseSelector.mockImplementation((selector: any) => {
269+
if (typeof selector === 'function') {
270+
const state = {
271+
...defaultSelectorMockState,
272+
engine: {
273+
...defaultSelectorMockState.engine,
274+
backgroundState: {
275+
...defaultSelectorMockState.engine.backgroundState,
276+
MultichainAssetsRatesController: {
277+
conversionRates: {
278+
'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v':
279+
{
280+
rate: '1.0',
281+
},
282+
},
283+
},
284+
},
285+
},
286+
};
287+
return selector(state);
288+
}
289+
return 'USD';
290+
});
275291

276292
mockFormatWithThreshold.mockReturnValue('$250.25');
277293

app/components/UI/Card/hooks/useAssetBalances.tsx

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,19 @@ import { getAssetBalanceKey } from '../util/getAssetBalanceKey';
77
import { useTokensWithBalance } from '../../Bridge/hooks/useTokensWithBalance';
88
import { isSolanaChainId } from '@metamask/bridge-controller';
99
import { selectCurrentCurrency } from '../../../../selectors/currencyRateController';
10-
import Engine from '../../../../core/Engine';
1110
import { safeFormatChainIdToHex } from '../util/safeFormatChainIdToHex';
1211
import { TokenI } from '../../Tokens/types';
1312
import { MarketDataDetails } from '@metamask/assets-controllers';
1413
import { formatWithThreshold } from '../../../../util/assets';
1514
import I18n from '../../../../../locales/i18n';
1615
import { deriveBalanceFromAssetMarketDetails } from '../../Tokens/util';
1716
import { buildTokenIconUrl } from '../util/buildTokenIconUrl';
17+
import {
18+
getCurrencyRateControllerCurrencyRates,
19+
getMultichainAssetsRatesControllerConversionRates,
20+
getTokenRatesControllerMarketData,
21+
getTokensControllerAllTokens,
22+
} from '../../../../selectors/assets/assets-migration';
1823
import { CARD_CHAIN_IDS } from '../constants';
1924
import { balanceToFiatNumber } from '../../../../util/number/bigint';
2025

@@ -84,17 +89,12 @@ export interface AssetBalanceInfo {
8489
export const useAssetBalances = (
8590
tokens: CardFundingToken[],
8691
): Map<string, AssetBalanceInfo> => {
87-
const { MultichainAssetsRatesController, TokenRatesController } =
88-
Engine.context;
8992
const tokensWithBalance = useTokensWithBalance({
9093
chainIds: CARD_CHAIN_IDS,
9194
});
9295

9396
// Get raw state needed for asset lookups - these are stable references from Redux
94-
const allAssets = useSelector(
95-
(state: RootState) =>
96-
state.engine.backgroundState.TokensController.allTokens,
97-
);
97+
const allAssets = useSelector(getTokensControllerAllTokens);
9898
const allDetectedTokens = useSelector(
9999
(state: RootState) =>
100100
state.engine.backgroundState.TokensController.allDetectedTokens,
@@ -104,9 +104,10 @@ export const useAssetBalances = (
104104
state.engine.backgroundState.NetworkController
105105
.networkConfigurationsByChainId,
106106
);
107-
const currencyRates = useSelector(
108-
(state: RootState) =>
109-
state.engine.backgroundState.CurrencyRateController.currencyRates,
107+
const currencyRates = useSelector(getCurrencyRateControllerCurrencyRates);
108+
const marketData = useSelector(getTokenRatesControllerMarketData);
109+
const conversionRates = useSelector(
110+
getMultichainAssetsRatesControllerConversionRates,
110111
);
111112

112113
// Build the walletAssetsMap in useMemo using raw state
@@ -182,28 +183,21 @@ export const useAssetBalances = (
182183

183184
const conversionRate = currencyRateEntry?.conversionRate;
184185

185-
const marketData =
186-
TokenRatesController?.state?.marketData?.[chainId]?.[
187-
token.address?.toLowerCase() as Hex
188-
];
186+
const tokenMarketData =
187+
marketData?.[chainId]?.[token.address?.toLowerCase() as Hex];
189188

190189
// Only require conversionRate to be present (marketData is optional)
191190
if (conversionRate !== undefined && conversionRate !== null) {
192191
map.set(`${chainId}-${token.address?.toLowerCase()}`, {
193192
conversionRate,
194-
marketData,
193+
marketData: tokenMarketData,
195194
});
196195
}
197196
}
198197
});
199198

200199
return map;
201-
}, [
202-
tokens,
203-
networkConfigs,
204-
currencyRates,
205-
TokenRatesController?.state?.marketData,
206-
]);
200+
}, [tokens, networkConfigs, currencyRates, marketData]);
207201

208202
const currentCurrency = useSelector(selectCurrentCurrency);
209203

@@ -256,8 +250,6 @@ export const useAssetBalances = (
256250
token: CardFundingToken,
257251
balanceToUse: string,
258252
): { balanceFiat: string; rawFiatNumber: number | undefined } => {
259-
const conversionRates =
260-
MultichainAssetsRatesController?.state?.conversionRates;
261253
const assetKey =
262254
`${token.caipChainId}/token:${token.address}` as `${string}:${string}/${string}:${string}`;
263255
const assetConversionRate = conversionRates?.[assetKey];
@@ -307,7 +299,7 @@ export const useAssetBalances = (
307299
rawFiatNumber: undefined,
308300
};
309301
},
310-
[MultichainAssetsRatesController?.state?.conversionRates, currentCurrency],
302+
[conversionRates, currentCurrency],
311303
);
312304

313305
// Helper: Calculate fiat with market data
@@ -534,8 +526,7 @@ export const useAssetBalances = (
534526
logo: buildTokenIconUrl(_token.caipChainId, _token.address ?? ''),
535527
} as TokenI);
536528

537-
const allMarketDataForChain =
538-
TokenRatesController?.state?.marketData?.[chainId] || {};
529+
const allMarketDataForChain = marketData?.[chainId] || {};
539530

540531
const derivedBalance = deriveBalanceFromAssetMarketDetails(
541532
mockAsset,
@@ -605,7 +596,7 @@ export const useAssetBalances = (
605596
[
606597
calculateFiatFromMarketData,
607598
calculateProportionalFiat,
608-
TokenRatesController?.state?.marketData,
599+
marketData,
609600
currentCurrency,
610601
],
611602
);

0 commit comments

Comments
 (0)