Skip to content

Commit 7f146c2

Browse files
authored
Fix: assets controller custom asset metadata case (#8727)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk due to switching the RPC detector’s token-list API endpoint/caching behavior and adding new required NetworkController events, which can affect asset detection/refresh across networks for consumers. > > **Overview** > RPC token detection is reworked to use the same `token.api.cx.metamask.io/tokens/{chainId}` endpoint and filtering rules as `TokenListController`, including chain-specific `occurrenceFloor` handling, Linea aggregator filtering, and forwarding `iconUrl`/`aggregators` into detection results. A shared optional TanStack Query `queryClient` is introduced to cache/dedupe per-chain token-list fetches, and `AssetsController` now passes its existing API query client to `RpcDataSource` by default. > > `AssetsController` now listens for `NetworkController:networkAdded` and **new** `NetworkController:networkRemoved` events (breaking for restricted messengers) to force-refresh assets after network config changes, and refactors RPC custom-asset supplemental subscription selection into a pure helper. Fixes include making custom-asset spam-filter bypass case-insensitive in `TokenDataSource`, preventing `RpcDataSource` from clobbering enriched native metadata with stubs on refresh/error paths, and only graduating custom assets when upstream sources report a strictly positive balance. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 4249719. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 8bc0a07 commit 7f146c2

15 files changed

Lines changed: 1146 additions & 181 deletions

packages/assets-controller/CHANGELOG.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- RPC token detection now uses the same endpoint as `TokenListController` (`token.api.cx.metamask.io/tokens/{chainId}`) instead of the v3 `tokens.api.cx.metamask.io/v3/chains/.../assets` host, with no client-side `first` cap and a shared TanStack Query cache ([#8727](https://github.com/MetaMask/core/pull/8727))
13+
- Mirrors `TokenListController.getTokensURL` exactly: same query parameters (`occurrenceFloor`, `includeNativeAssets=false`, `includeTokenFees=false`, `includeAssetType=false`, `includeERC20Permit=false`, `includeStorage=false`, `includeRwaData=true`), per-chain occurrence floor (`1` on Linea / MegaETH / Tempo mainnets, `3` elsewhere), and the Linea-mainnet aggregator filter (`lineaTeam`-flagged or ≥ 3 aggregators).
14+
- Detection now also picks up `iconUrl` and `aggregators` returned by the API; previously only `address`/`symbol`/`name`/`decimals`/`occurrences` were forwarded into `TokenListEntry`.
15+
- The previous client-side `first=25` cap is removed; the API still bounds responses server-side via `occurrenceFloor`, but the long tail past the previous 25-token slice is now visible to detection.
16+
- `TokensApiClient` accepts an optional `queryClient` (compatible with `ApiPlatformClient.queryClient`); when provided it caches/dedupes per-chain list responses with a 5 min `staleTime` and 1 h `gcTime` under the `['assets-controller','rpc-detection','token-list',{chainId}]` key, so concurrent detector polls across accounts/instances coalesce into a single network request.
17+
- `RpcDataSource` accepts a new optional `queryClient` option which it forwards to `TokensApiClient`. `AssetsController` defaults this to its existing `queryApiClient.queryClient`, so consumers get the full list and shared cache automatically.
18+
1019
### Changed
1120

21+
- **BREAKING:** `AssetsController` now requires two additional messenger events from `NetworkController`: `NetworkController:networkAdded` and `NetworkController:networkRemoved` ([#8727](https://github.com/MetaMask/core/pull/8727))
22+
- Consumers building restricted controller messengers must include both events in their allowed event set, otherwise TypeScript/action constraint checks will fail.
1223
- Bump `@metamask/account-tree-controller` from `^7.2.0` to `^7.3.0` ([#8722](https://github.com/MetaMask/core/pull/8722))
1324
- Bump `@metamask/keyring-controller` from `^25.4.0` to `^25.5.0` ([#8722](https://github.com/MetaMask/core/pull/8722))
1425
- Bump `@metamask/permission-controller` from `^13.0.0` to `^13.1.0` ([#8722](https://github.com/MetaMask/core/pull/8722))
1526
- Bump `@metamask/transaction-controller` from `^65.1.0` to `^65.2.0` ([#8722](https://github.com/MetaMask/core/pull/8722))
1627

28+
### Fixed
29+
30+
- `TokenDataSource` no longer drops user-imported EVM custom asset metadata when the V3 Tokens API returns the asset ID lower-cased ([#8727](https://github.com/MetaMask/core/pull/8727))
31+
- State stores `customAssets` checksummed (via `normalizeAssetId`), but the API can echo it lower-cased; the spam-filter bypass now compares lower-cased on both sides so customs reliably skip the `MIN_TOKEN_OCCURRENCES` filter and `assetsInfo` is populated for them.
32+
- `RpcDataSource` no longer overwrites richer native token metadata with a minimal stub on every balance refresh ([#8727](https://github.com/MetaMask/core/pull/8727))
33+
- Previously, each balance fetch emitted `{ type: 'native', symbol: chainStatus.nativeCurrency, name: chainStatus.nativeCurrency, decimals: 18 }` for native assets, which clobbered fields like `image`, `description`, `occurrences`, and `aggregators` enriched by the price/info API and renamed e.g. `"Avalanche"` to `"AVAX"`.
34+
- Native token metadata now mirrors the existing ERC-20 behavior: prefer existing metadata in state when present, and only emit the stub when no metadata is in state yet (e.g. first sighting of the asset). The fallback in the balance-fetch error path is gated the same way.
35+
1736
## [6.4.0]
1837

1938
### Added

packages/assets-controller/src/AssetsController.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1905,6 +1905,23 @@ describe('AssetsController', () => {
19051905
expect(true).toBe(true);
19061906
});
19071907
});
1908+
1909+
it('refreshes assets when a network is added or removed', async () => {
1910+
await withController(async ({ messenger }) => {
1911+
(messenger.publish as CallableFunction)(
1912+
'NetworkController:networkAdded',
1913+
{ chainId: '0x89' },
1914+
);
1915+
(messenger.publish as CallableFunction)(
1916+
'NetworkController:networkRemoved',
1917+
{ chainId: '0x89' },
1918+
);
1919+
1920+
await new Promise(process.nextTick);
1921+
1922+
expect(true).toBe(true);
1923+
});
1924+
});
19081925
});
19091926

19101927
describe('account group changes', () => {

packages/assets-controller/src/AssetsController.ts

Lines changed: 56 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import type {
3030
NetworkControllerGetNetworkClientByIdAction,
3131
NetworkControllerGetStateAction,
3232
NetworkControllerNetworkAddedEvent,
33+
NetworkControllerNetworkRemovedEvent,
3334
NetworkControllerStateChangeEvent,
3435
} from '@metamask/network-controller';
3536
import type {
@@ -133,6 +134,7 @@ import type {
133134
TransactionPayLegacyFormat,
134135
} from './utils';
135136
import { ZERO_ADDRESS } from './utils/constants';
137+
import { pickRpcCustomAssetsSupplement } from './utils/customAssetsRpcSupplement';
136138

137139
const NATIVE_ASSETS_QUERY_KEY = ['nativeAssets'];
138140

@@ -319,8 +321,11 @@ type AllowedEvents =
319321
| TransactionControllerUnapprovedTransactionAddedEvent
320322
// RpcDataSource, StakedBalanceDataSource
321323
| NetworkControllerStateChangeEvent
322-
// AssetsController (default-asset seeding when a new network is added)
324+
// AssetsController (default-asset seeding + cross-source asset refresh
325+
// whenever a network configuration is added to or removed from
326+
// NetworkController)
323327
| NetworkControllerNetworkAddedEvent
328+
| NetworkControllerNetworkRemovedEvent
324329
| TransactionControllerTransactionConfirmedEvent
325330
| TransactionControllerIncomingTransactionsReceivedEvent
326331
// StakedBalanceDataSource
@@ -822,6 +827,11 @@ export class AssetsController extends BaseController<
822827
getNativeAssetForChain: (chainId: ChainId): Caip19AssetId =>
823828
this.#getNativeAssetMap()[chainId] ??
824829
`${chainId}/erc20:${ZERO_ADDRESS}`,
830+
// Share the API platform's TanStack Query client so the RPC token
831+
// detector caches/dedupes its top-token-list fetches alongside the rest
832+
// of the package's API calls. Caller-provided rpcConfig.queryClient
833+
// wins via the spread below.
834+
queryClient: queryApiClient.queryClient,
825835
...rpcConfig,
826836
isOnboarded: rpcConfig.isOnboarded ?? isOnboarded,
827837
});
@@ -1011,19 +1021,24 @@ export class AssetsController extends BaseController<
10111021
},
10121022
);
10131023

1014-
// When a new network is added (e.g. the user finally adds Monad
1015-
// to NetworkController), seed the default tracked assets for it
1016-
// — but only if the chain is in our defaults registry. This is
1017-
// what makes mUSD show up on Monad the moment the network is
1018-
// configured, without waiting for it to also be enabled in
1019-
// NetworkEnablementController.
1024+
// When a network is added or removed from NetworkController, refresh
1025+
// assets across every data source so balances, prices, and metadata
1026+
// stay consistent. On add we also seed default tracked assets (e.g.
1027+
// mUSD on Monad) when the chain is in our defaults registry, so the
1028+
// entries appear immediately without waiting for it to also be
1029+
// enabled in NetworkEnablementController.
10201030
this.messenger.subscribe(
10211031
'NetworkController:networkAdded',
10221032
(networkConfiguration) => {
10231033
this.#handleNetworkAdded(networkConfiguration.chainId);
1034+
this.#refreshAssetsAfterNetworkChange();
10241035
},
10251036
);
10261037

1038+
this.messenger.subscribe('NetworkController:networkRemoved', () => {
1039+
this.#refreshAssetsAfterNetworkChange();
1040+
});
1041+
10271042
// Client + Keyring lifecycle: only run when UI is open AND keyring is unlocked
10281043
this.messenger.subscribe(
10291044
'ClientController:stateChange',
@@ -2605,10 +2620,13 @@ export class AssetsController extends BaseController<
26052620
}
26062621

26072622
/**
2608-
* Subscribe RPC to chains where the user has customAssets but another
2609-
* balance data source already owns the chain in regular handoff. Uses a
2610-
* separate subscription key (`customAssetsOnly` mode) so the regular RPC
2611-
* subscription, if any, is unaffected.
2623+
* Guarantee that customAssets are **always** polled by RPC, even when
2624+
* AccountsApi or the websocket data source has claimed the chain in the
2625+
* regular handoff. RPC is the sole balance fetcher for user-imported
2626+
* tokens (see `pickRpcCustomAssetsSupplement` for the full rationale),
2627+
* so we run a dedicated subscription in `customAssetsOnly` mode under a
2628+
* distinct subscription key (`ds:RpcDataSource:custom`) that does not
2629+
* interfere with the regular RPC subscription.
26122630
*
26132631
* @param accounts - Accounts to consider for customAssets.
26142632
* @param chainToAccounts - Map of chain → accounts (built by caller).
@@ -2620,54 +2638,30 @@ export class AssetsController extends BaseController<
26202638
rpcAssignedChains: Set<ChainId>,
26212639
): void {
26222640
const rpc = this.#rpcDataSource;
2623-
const rpcAvailableChains = new Set(rpc.getActiveChainsSync());
2641+
const supplementalKey = `ds:${rpc.getName()}:custom`;
26242642

2625-
// Collect chains that have customAssets for at least one of the given
2626-
// accounts and are NOT already covered by the regular RPC subscription.
2627-
const supplementalChainSet = new Set<ChainId>();
2628-
const accountsWithCustomAssets = new Set<string>();
2629-
for (const account of accounts) {
2630-
const customForAccount = this.state.customAssets[account.id] ?? [];
2631-
if (customForAccount.length === 0) {
2632-
continue;
2633-
}
2634-
accountsWithCustomAssets.add(account.id);
2635-
for (const assetId of customForAccount) {
2636-
let chainId: ChainId;
2637-
try {
2638-
chainId = extractChainId(assetId);
2639-
} catch {
2640-
continue;
2641-
}
2642-
if (rpcAssignedChains.has(chainId)) {
2643-
continue;
2644-
}
2645-
if (!rpcAvailableChains.has(chainId)) {
2646-
continue;
2647-
}
2648-
if (!chainToAccounts.has(chainId)) {
2649-
continue;
2650-
}
2651-
supplementalChainSet.add(chainId);
2652-
}
2653-
}
2643+
const decision = pickRpcCustomAssetsSupplement({
2644+
accountIds: accounts.map((account) => account.id),
2645+
customAssetsByAccount: this.state.customAssets,
2646+
rpcAssignedChains,
2647+
rpcAvailableChains: new Set(rpc.getActiveChainsSync()),
2648+
enabledChains: new Set(chainToAccounts.keys()),
2649+
});
26542650

2655-
const supplementalKey = `ds:${rpc.getName()}:custom`;
2656-
if (supplementalChainSet.size === 0) {
2651+
if (decision.chains.length === 0) {
26572652
this.#unsubscribeBySubscriptionKey(rpc, supplementalKey);
26582653
return;
26592654
}
26602655

2661-
const supplementalChains = [...supplementalChainSet];
26622656
const supplementalAccounts = accounts.filter((account) =>
2663-
accountsWithCustomAssets.has(account.id),
2657+
decision.accountIds.has(account.id),
26642658
);
26652659
if (supplementalAccounts.length === 0) {
26662660
this.#unsubscribeBySubscriptionKey(rpc, supplementalKey);
26672661
return;
26682662
}
26692663

2670-
this.#subscribeDataSource(rpc, supplementalAccounts, supplementalChains, {
2664+
this.#subscribeDataSource(rpc, supplementalAccounts, decision.chains, {
26712665
subscriptionKey: supplementalKey,
26722666
customAssetsOnly: true,
26732667
});
@@ -3042,6 +3036,22 @@ export class AssetsController extends BaseController<
30423036
this.#ensureDefaultTrackedAssetsSeeded([caipChainId]);
30433037
}
30443038

3039+
/**
3040+
* Refresh assets across every data source after a network configuration
3041+
* is added to or removed from NetworkController. Mirrors the
3042+
* `forceUpdate` path used elsewhere (e.g. unapproved tx, account-tree
3043+
* change), so balances/prices/metadata stay consistent for the user's
3044+
* currently-enabled chains without us having to maintain bespoke
3045+
* per-event state surgery.
3046+
*/
3047+
#refreshAssetsAfterNetworkChange(): void {
3048+
this.getAssets(this.#getSelectedAccounts(), {
3049+
forceUpdate: true,
3050+
}).catch((error) => {
3051+
log('Failed to refresh assets after network change', { error });
3052+
});
3053+
}
3054+
30453055
/**
30463056
* Handle assets updated from a data source.
30473057
* Called via the onAssetsUpdate callback passed in SubscriptionRequest when the controller subscribes to a data source.

packages/assets-controller/src/data-sources/RpcDataSource.ts

Lines changed: 57 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import {
5252
import type {
5353
BalancePollingInput,
5454
DetectionPollingInput,
55+
TokenListQueryClient,
5556
} from './evm-rpc-services';
5657
import type {
5758
Address,
@@ -103,6 +104,12 @@ export type RpcDataSourceConfig = {
103104
/** Function returning whether onboarding is complete. When false, fetch and subscribe are no-ops. Defaults to () => true. */
104105
isOnboarded?: () => boolean;
105106
timeout?: number;
107+
/**
108+
* Optional shared TanStack Query client used by `TokensApiClient` to cache
109+
* token-list responses across detector polls. Pass `apiPlatformClient.queryClient`
110+
* to share the cache with other API clients in the host app.
111+
*/
112+
queryClient?: TokenListQueryClient;
106113
};
107114

108115
export type RpcDataSourceOptions = {
@@ -128,6 +135,11 @@ export type RpcDataSourceOptions = {
128135
useExternalService?: () => boolean;
129136
/** Function returning whether onboarding is complete. When false, fetch and subscribe are no-ops. Defaults to () => true. */
130137
isOnboarded?: () => boolean;
138+
/**
139+
* Optional shared TanStack Query client used by `TokensApiClient` to cache
140+
* token-list responses across detector polls.
141+
*/
142+
queryClient?: TokenListQueryClient;
131143
};
132144

133145
/**
@@ -300,8 +312,13 @@ export class RpcDataSource extends AbstractDataSource<
300312
}
301313
});
302314

303-
// Initialize TokenDetector with polling interval
304-
const tokensApiClient = new TokensApiClient();
315+
// Initialize TokenDetector with polling interval. The TokensApiClient is
316+
// configured with the shared TanStack Query client (when the controller
317+
// provides one) so concurrent detector polls/accounts/instances share a
318+
// single in-flight request and cached result per chain.
319+
const tokensApiClient = new TokensApiClient({
320+
queryClient: options.queryClient,
321+
});
305322
this.#tokenDetector = new TokenDetector(
306323
this.#multicallClient,
307324
tokensApiClient,
@@ -356,27 +373,32 @@ export class RpcDataSource extends AbstractDataSource<
356373

357374
const nativeAssetId = this.#getNativeAssetForChain(chainId);
358375
for (const balance of balances) {
376+
const existingMeta = existingMetadata[balance.assetId];
359377
const isNative =
360-
existingMetadata[balance.assetId]?.type === 'native' ||
378+
existingMeta?.type === 'native' ||
361379
balance.assetId.toLowerCase() === nativeAssetId?.toLowerCase();
362380
if (isNative) {
363-
const chainStatus = this.#chainStatuses[chainId];
364-
365-
if (chainStatus) {
366-
assetsInfo[balance.assetId] = {
367-
type: 'native',
368-
symbol: chainStatus.nativeCurrency,
369-
name: chainStatus.nativeCurrency,
370-
decimals: 18,
371-
};
372-
}
373-
} else {
374-
// For ERC20 tokens, use existing metadata from state if available.
375-
// Unknown ERC-20s are omitted until TokenDataSource enriches them.
376-
const existingMeta = existingMetadata[balance.assetId];
381+
// Prefer existing (richer) metadata in state — it may have been
382+
// enriched by the price/info API with image, description, etc.
383+
// Only emit a minimal stub when there's nothing in state yet,
384+
// so we don't clobber that richer metadata on every balance refresh.
377385
if (existingMeta) {
378386
assetsInfo[balance.assetId] = existingMeta;
387+
} else {
388+
const chainStatus = this.#chainStatuses[chainId];
389+
if (chainStatus) {
390+
assetsInfo[balance.assetId] = {
391+
type: 'native',
392+
symbol: chainStatus.nativeCurrency,
393+
name: chainStatus.nativeCurrency,
394+
decimals: 18,
395+
};
396+
}
379397
}
398+
} else if (existingMeta) {
399+
// For ERC20 tokens, use existing metadata from state if available.
400+
// Unknown ERC-20s are omitted until TokenDataSource enriches them.
401+
assetsInfo[balance.assetId] = existingMeta;
380402
}
381403
}
382404

@@ -1021,15 +1043,24 @@ export class RpcDataSource extends AbstractDataSource<
10211043
}
10221044
assetsBalance[accountId][nativeAssetId] = { amount: '0' };
10231045

1024-
// Even on error, include native token metadata
1025-
const chainStatus = this.#chainStatuses[chainId];
1026-
if (chainStatus) {
1027-
assetsInfo[nativeAssetId] = {
1028-
type: 'native',
1029-
symbol: chainStatus.nativeCurrency,
1030-
name: chainStatus.nativeCurrency,
1031-
decimals: 18,
1032-
};
1046+
// Even on error, include native token metadata. Prefer the richer
1047+
// metadata already in state (e.g. enriched with image/description
1048+
// by the price/info API) and fall back to a minimal stub only when
1049+
// nothing is in state yet, so we don't clobber that richer metadata.
1050+
const existingNativeMeta =
1051+
this.#getExistingAssetsMetadata()[nativeAssetId];
1052+
if (existingNativeMeta) {
1053+
assetsInfo[nativeAssetId] = existingNativeMeta;
1054+
} else {
1055+
const chainStatus = this.#chainStatuses[chainId];
1056+
if (chainStatus) {
1057+
assetsInfo[nativeAssetId] = {
1058+
type: 'native',
1059+
symbol: chainStatus.nativeCurrency,
1060+
name: chainStatus.nativeCurrency,
1061+
decimals: 18,
1062+
};
1063+
}
10331064
}
10341065

10351066
if (!failedChains.includes(chainId)) {

0 commit comments

Comments
 (0)