Fix: assets controller custom asset metadata case

packages/assets-controller/CHANGELOG.md

@@ -7,13 +7,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- 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))
+  - 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).
+  - Detection now also picks up `iconUrl` and `aggregators` returned by the API; previously only `address`/`symbol`/`name`/`decimals`/`occurrences` were forwarded into `TokenListEntry`.
+  - 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.
+  - `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.
+  - `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.
+
 ### Changed
 
+- **BREAKING:** `AssetsController` now requires two additional messenger events from `NetworkController`: `NetworkController:networkAdded` and `NetworkController:networkRemoved` ([#8727](https://github.com/MetaMask/core/pull/8727))
+  - Consumers building restricted controller messengers must include both events in their allowed event set, otherwise TypeScript/action constraint checks will fail.
 - Bump `@metamask/account-tree-controller` from `^7.2.0` to `^7.3.0` ([#8722](https://github.com/MetaMask/core/pull/8722))
 - Bump `@metamask/keyring-controller` from `^25.4.0` to `^25.5.0` ([#8722](https://github.com/MetaMask/core/pull/8722))
 - Bump `@metamask/permission-controller` from `^13.0.0` to `^13.1.0` ([#8722](https://github.com/MetaMask/core/pull/8722))
 - Bump `@metamask/transaction-controller` from `^65.1.0` to `^65.2.0` ([#8722](https://github.com/MetaMask/core/pull/8722))
 
+### Fixed
+
+- `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))
+  - 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.
+- `RpcDataSource` no longer overwrites richer native token metadata with a minimal stub on every balance refresh ([#8727](https://github.com/MetaMask/core/pull/8727))
+  - 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"`.
+  - 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.
+
 ## [6.4.0]
 
 ### Added

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

@@ -1905,6 +1905,23 @@ describe('AssetsController', () => {
         expect(true).toBe(true);
       });
     });
+
+    it('refreshes assets when a network is added or removed', async () => {
+      await withController(async ({ messenger }) => {
+        (messenger.publish as CallableFunction)(
+          'NetworkController:networkAdded',
+          { chainId: '0x89' },
+        );
+        (messenger.publish as CallableFunction)(
+          'NetworkController:networkRemoved',
+          { chainId: '0x89' },
+        );
+
+        await new Promise(process.nextTick);
+
+        expect(true).toBe(true);
+      });
+    });
   });
 
   describe('account group changes', () => {

packages/assets-controller/src/AssetsController.ts

@@ -30,6 +30,7 @@ import type {
   NetworkControllerGetNetworkClientByIdAction,
   NetworkControllerGetStateAction,
   NetworkControllerNetworkAddedEvent,
+  NetworkControllerNetworkRemovedEvent,
   NetworkControllerStateChangeEvent,
 } from '@metamask/network-controller';
 import type {
@@ -133,6 +134,7 @@ import type {
   TransactionPayLegacyFormat,
 } from './utils';
 import { ZERO_ADDRESS } from './utils/constants';
+import { pickRpcCustomAssetsSupplement } from './utils/customAssetsRpcSupplement';
 
 const NATIVE_ASSETS_QUERY_KEY = ['nativeAssets'];
 
@@ -319,8 +321,11 @@ type AllowedEvents =
   | TransactionControllerUnapprovedTransactionAddedEvent
   // RpcDataSource, StakedBalanceDataSource
   | NetworkControllerStateChangeEvent
-  // AssetsController (default-asset seeding when a new network is added)
+  // AssetsController (default-asset seeding + cross-source asset refresh
+  // whenever a network configuration is added to or removed from
+  // NetworkController)
   | NetworkControllerNetworkAddedEvent
+  | NetworkControllerNetworkRemovedEvent
   | TransactionControllerTransactionConfirmedEvent
   | TransactionControllerIncomingTransactionsReceivedEvent
   // StakedBalanceDataSource
@@ -822,6 +827,11 @@ export class AssetsController extends BaseController<
       getNativeAssetForChain: (chainId: ChainId): Caip19AssetId =>
         this.#getNativeAssetMap()[chainId] ??
         `${chainId}/erc20:${ZERO_ADDRESS}`,
+      // Share the API platform's TanStack Query client so the RPC token
+      // detector caches/dedupes its top-token-list fetches alongside the rest
+      // of the package's API calls. Caller-provided rpcConfig.queryClient
+      // wins via the spread below.
+      queryClient: queryApiClient.queryClient,
       ...rpcConfig,
       isOnboarded: rpcConfig.isOnboarded ?? isOnboarded,
     });
@@ -1011,19 +1021,24 @@ export class AssetsController extends BaseController<
       },
     );
 
-    // When a new network is added (e.g. the user finally adds Monad
-    // to NetworkController), seed the default tracked assets for it
-    // — but only if the chain is in our defaults registry. This is
-    // what makes mUSD show up on Monad the moment the network is
-    // configured, without waiting for it to also be enabled in
-    // NetworkEnablementController.
+    // When a network is added or removed from NetworkController, refresh
+    // assets across every data source so balances, prices, and metadata
+    // stay consistent. On add we also seed default tracked assets (e.g.
+    // mUSD on Monad) when the chain is in our defaults registry, so the
+    // entries appear immediately without waiting for it to also be
+    // enabled in NetworkEnablementController.
     this.messenger.subscribe(
       'NetworkController:networkAdded',
       (networkConfiguration) => {
         this.#handleNetworkAdded(networkConfiguration.chainId);
+        this.#refreshAssetsAfterNetworkChange();
       },
     );
 
+    this.messenger.subscribe('NetworkController:networkRemoved', () => {
+      this.#refreshAssetsAfterNetworkChange();
+    });
+
     // Client + Keyring lifecycle: only run when UI is open AND keyring is unlocked
     this.messenger.subscribe(
       'ClientController:stateChange',
@@ -2605,10 +2620,13 @@ export class AssetsController extends BaseController<
   }
 
   /**
-   * Subscribe RPC to chains where the user has customAssets but another
-   * balance data source already owns the chain in regular handoff. Uses a
-   * separate subscription key (`customAssetsOnly` mode) so the regular RPC
-   * subscription, if any, is unaffected.
+   * Guarantee that customAssets are **always** polled by RPC, even when
+   * AccountsApi or the websocket data source has claimed the chain in the
+   * regular handoff. RPC is the sole balance fetcher for user-imported
+   * tokens (see `pickRpcCustomAssetsSupplement` for the full rationale),
+   * so we run a dedicated subscription in `customAssetsOnly` mode under a
+   * distinct subscription key (`ds:RpcDataSource:custom`) that does not
+   * interfere with the regular RPC subscription.
    *
    * @param accounts - Accounts to consider for customAssets.
    * @param chainToAccounts - Map of chain → accounts (built by caller).
@@ -2620,54 +2638,30 @@ export class AssetsController extends BaseController<
     rpcAssignedChains: Set<ChainId>,
   ): void {
     const rpc = this.#rpcDataSource;
-    const rpcAvailableChains = new Set(rpc.getActiveChainsSync());
+    const supplementalKey = `ds:${rpc.getName()}:custom`;
 
-    // Collect chains that have customAssets for at least one of the given
-    // accounts and are NOT already covered by the regular RPC subscription.
-    const supplementalChainSet = new Set<ChainId>();
-    const accountsWithCustomAssets = new Set<string>();
-    for (const account of accounts) {
-      const customForAccount = this.state.customAssets[account.id] ?? [];
-      if (customForAccount.length === 0) {
-        continue;
-      }
-      accountsWithCustomAssets.add(account.id);
-      for (const assetId of customForAccount) {
-        let chainId: ChainId;
-        try {
-          chainId = extractChainId(assetId);
-        } catch {
-          continue;
-        }
-        if (rpcAssignedChains.has(chainId)) {
-          continue;
-        }
-        if (!rpcAvailableChains.has(chainId)) {
-          continue;
-        }
-        if (!chainToAccounts.has(chainId)) {
-          continue;
-        }
-        supplementalChainSet.add(chainId);
-      }
-    }
+    const decision = pickRpcCustomAssetsSupplement({
+      accountIds: accounts.map((account) => account.id),
+      customAssetsByAccount: this.state.customAssets,
+      rpcAssignedChains,
+      rpcAvailableChains: new Set(rpc.getActiveChainsSync()),
+      enabledChains: new Set(chainToAccounts.keys()),
+    });
 
-    const supplementalKey = `ds:${rpc.getName()}:custom`;
-    if (supplementalChainSet.size === 0) {
+    if (decision.chains.length === 0) {
       this.#unsubscribeBySubscriptionKey(rpc, supplementalKey);
       return;
     }
 
-    const supplementalChains = [...supplementalChainSet];
     const supplementalAccounts = accounts.filter((account) =>
-      accountsWithCustomAssets.has(account.id),
+      decision.accountIds.has(account.id),
     );
     if (supplementalAccounts.length === 0) {
       this.#unsubscribeBySubscriptionKey(rpc, supplementalKey);
       return;
     }
 
-    this.#subscribeDataSource(rpc, supplementalAccounts, supplementalChains, {
+    this.#subscribeDataSource(rpc, supplementalAccounts, decision.chains, {
       subscriptionKey: supplementalKey,
       customAssetsOnly: true,
     });
@@ -3042,6 +3036,22 @@ export class AssetsController extends BaseController<
     this.#ensureDefaultTrackedAssetsSeeded([caipChainId]);
   }
 
+  /**
+   * Refresh assets across every data source after a network configuration
+   * is added to or removed from NetworkController. Mirrors the
+   * `forceUpdate` path used elsewhere (e.g. unapproved tx, account-tree
+   * change), so balances/prices/metadata stay consistent for the user's
+   * currently-enabled chains without us having to maintain bespoke
+   * per-event state surgery.
+   */
+  #refreshAssetsAfterNetworkChange(): void {
+    this.getAssets(this.#getSelectedAccounts(), {
+      forceUpdate: true,
+    }).catch((error) => {
+      log('Failed to refresh assets after network change', { error });
+    });
+  }
+
   /**
    * Handle assets updated from a data source.
    * Called via the onAssetsUpdate callback passed in SubscriptionRequest when the controller subscribes to a data source.

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

@@ -52,6 +52,7 @@ import {
 import type {
   BalancePollingInput,
   DetectionPollingInput,
+  TokenListQueryClient,
 } from './evm-rpc-services';
 import type {
   Address,
@@ -103,6 +104,12 @@ export type RpcDataSourceConfig = {
   /** Function returning whether onboarding is complete. When false, fetch and subscribe are no-ops. Defaults to () => true. */
   isOnboarded?: () => boolean;
   timeout?: number;
+  /**
+   * Optional shared TanStack Query client used by `TokensApiClient` to cache
+   * token-list responses across detector polls. Pass `apiPlatformClient.queryClient`
+   * to share the cache with other API clients in the host app.
+   */
+  queryClient?: TokenListQueryClient;
 };
 
 export type RpcDataSourceOptions = {
@@ -128,6 +135,11 @@ export type RpcDataSourceOptions = {
   useExternalService?: () => boolean;
   /** Function returning whether onboarding is complete. When false, fetch and subscribe are no-ops. Defaults to () => true. */
   isOnboarded?: () => boolean;
+  /**
+   * Optional shared TanStack Query client used by `TokensApiClient` to cache
+   * token-list responses across detector polls.
+   */
+  queryClient?: TokenListQueryClient;
 };
 
 /**
@@ -300,8 +312,13 @@ export class RpcDataSource extends AbstractDataSource<
       }
     });
 
-    // Initialize TokenDetector with polling interval
-    const tokensApiClient = new TokensApiClient();
+    // Initialize TokenDetector with polling interval. The TokensApiClient is
+    // configured with the shared TanStack Query client (when the controller
+    // provides one) so concurrent detector polls/accounts/instances share a
+    // single in-flight request and cached result per chain.
+    const tokensApiClient = new TokensApiClient({
+      queryClient: options.queryClient,
+    });
     this.#tokenDetector = new TokenDetector(
       this.#multicallClient,
       tokensApiClient,
@@ -356,27 +373,32 @@ export class RpcDataSource extends AbstractDataSource<
 
     const nativeAssetId = this.#getNativeAssetForChain(chainId);
     for (const balance of balances) {
+      const existingMeta = existingMetadata[balance.assetId];
       const isNative =
-        existingMetadata[balance.assetId]?.type === 'native' ||
+        existingMeta?.type === 'native' ||
         balance.assetId.toLowerCase() === nativeAssetId?.toLowerCase();
       if (isNative) {
-        const chainStatus = this.#chainStatuses[chainId];
-
-        if (chainStatus) {
-          assetsInfo[balance.assetId] = {
-            type: 'native',
-            symbol: chainStatus.nativeCurrency,
-            name: chainStatus.nativeCurrency,
-            decimals: 18,
-          };
-        }
-      } else {
-        // For ERC20 tokens, use existing metadata from state if available.
-        // Unknown ERC-20s are omitted until TokenDataSource enriches them.
-        const existingMeta = existingMetadata[balance.assetId];
+        // Prefer existing (richer) metadata in state — it may have been
+        // enriched by the price/info API with image, description, etc.
+        // Only emit a minimal stub when there's nothing in state yet,
+        // so we don't clobber that richer metadata on every balance refresh.
         if (existingMeta) {
           assetsInfo[balance.assetId] = existingMeta;
+        } else {
+          const chainStatus = this.#chainStatuses[chainId];
+          if (chainStatus) {
+            assetsInfo[balance.assetId] = {
+              type: 'native',
+              symbol: chainStatus.nativeCurrency,
+              name: chainStatus.nativeCurrency,
+              decimals: 18,
+            };
+          }
         }
+      } else if (existingMeta) {
+        // For ERC20 tokens, use existing metadata from state if available.
+        // Unknown ERC-20s are omitted until TokenDataSource enriches them.
+        assetsInfo[balance.assetId] = existingMeta;
       }
     }
 
@@ -1021,15 +1043,24 @@ export class RpcDataSource extends AbstractDataSource<
           }
           assetsBalance[accountId][nativeAssetId] = { amount: '0' };
 
-          // Even on error, include native token metadata
-          const chainStatus = this.#chainStatuses[chainId];
-          if (chainStatus) {
-            assetsInfo[nativeAssetId] = {
-              type: 'native',
-              symbol: chainStatus.nativeCurrency,
-              name: chainStatus.nativeCurrency,
-              decimals: 18,
-            };
+          // Even on error, include native token metadata. Prefer the richer
+          // metadata already in state (e.g. enriched with image/description
+          // by the price/info API) and fall back to a minimal stub only when
+          // nothing is in state yet, so we don't clobber that richer metadata.
+          const existingNativeMeta =
+            this.#getExistingAssetsMetadata()[nativeAssetId];
+          if (existingNativeMeta) {
+            assetsInfo[nativeAssetId] = existingNativeMeta;
+          } else {
+            const chainStatus = this.#chainStatuses[chainId];
+            if (chainStatus) {
+              assetsInfo[nativeAssetId] = {
+                type: 'native',
+                symbol: chainStatus.nativeCurrency,
+                name: chainStatus.nativeCurrency,
+                decimals: 18,
+              };
+            }
           }
 
           if (!failedChains.includes(chainId)) {