From e3abb086de1595bf7c34ec8efabeb9d22772bead Mon Sep 17 00:00:00 2001 From: salimtb Date: Wed, 6 May 2026 12:39:10 +0200 Subject: [PATCH 01/11] fix: preserve custom asset metadata when V3 Tokens API returns lower-cased asset IDs `TokenDataSource` stores `customAssets` checksummed (via `normalizeAssetId`) but the V3 Tokens API can echo the asset ID lower-cased. The case-sensitive `customAssetIds.has()` bypass missed in that case, dropping user-imported EVM tokens through the `MIN_TOKEN_OCCURRENCES` spam filter and wiping their metadata from `assetsInfo`. Compare lower-cased on both sides so customs reliably bypass the occurrence filter (and the non-EVM Blockaid bypass). --- packages/assets-controller/CHANGELOG.md | 5 +++ .../src/data-sources/TokenDataSource.test.ts | 41 +++++++++++++++++++ .../src/data-sources/TokenDataSource.ts | 17 +++++--- 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 9527c165ce..39096ec5a9 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -17,6 +17,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/accounts-controller` from `^37.2.0` to `^38.0.0` ([#8665](https://github.com/MetaMask/core/pull/8665)) - Bump `@metamask/account-tree-controller` from `^7.1.0` to `^7.2.0` ([#8665](https://github.com/MetaMask/core/pull/8665)) +### Fixed + +- `TokenDataSource` no longer drops user-imported EVM custom asset metadata when the V3 Tokens API returns the asset ID lower-cased + - 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. + ## [6.3.0] ### Added diff --git a/packages/assets-controller/src/data-sources/TokenDataSource.test.ts b/packages/assets-controller/src/data-sources/TokenDataSource.test.ts index f3d82fa432..15932f3936 100644 --- a/packages/assets-controller/src/data-sources/TokenDataSource.test.ts +++ b/packages/assets-controller/src/data-sources/TokenDataSource.test.ts @@ -780,6 +780,47 @@ describe('TokenDataSource', () => { ); }); + it('middleware bypasses occurrence filter for custom assets when state-stored ID is checksummed and API returns lower-case', async () => { + // State stores customs in normalized (checksummed) form; the V3 Tokens + // API can echo the same asset ID lower-cased. Without a case-insensitive + // bypass, the user's imported asset would fall through to the + // occurrence filter and be stripped from `response.assetsInfo`. + const checksummedCustomAsset = + 'eip155:1/erc20:0xA0b86991c6218b36c1D19D4a2e9Eb0cE3606eB48' as Caip19AssetId; + const lowercaseCustomAsset = + 'eip155:1/erc20:0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48' as Caip19AssetId; + + const { controller } = setupController({ + messenger: createTestMessenger(), + supportedNetworks: ['eip155:1'], + assetsResponse: [ + createMockAssetResponse(lowercaseCustomAsset, { occurrences: 1 }), + ], + }); + + const next = jest.fn().mockResolvedValue(undefined); + const context: Context = { + request: createDataRequest(), + response: { + detectedAssets: { + 'mock-account-id': [checksummedCustomAsset], + }, + }, + getAssetsState: jest.fn().mockReturnValue({ + assetsInfo: {}, + customAssets: { + 'mock-account-id': [checksummedCustomAsset], + }, + }), + }; + + await controller.assetsMiddleware(context, next); + + // Custom asset is preserved despite occurrences=1 because the + // user-imported bypass is case-insensitive. + expect(context.response.assetsInfo?.[lowercaseCustomAsset]).toBeDefined(); + }); + it.each([ { occurrences: 1, label: 'low occurrences' }, { occurrences: undefined, label: 'no occurrences' }, diff --git a/packages/assets-controller/src/data-sources/TokenDataSource.ts b/packages/assets-controller/src/data-sources/TokenDataSource.ts index 3fe5b9fb70..c5ffc245be 100644 --- a/packages/assets-controller/src/data-sources/TokenDataSource.ts +++ b/packages/assets-controller/src/data-sources/TokenDataSource.ts @@ -325,8 +325,13 @@ export class TokenDataSource { const assetIdsNeedingMetadata = new Set(); // Custom assets are user-imported — exempt from spam filtering. + // State stores asset IDs in their normalized (checksummed) form, but the + // V3 Tokens API can return them lower-cased. Lowercase both sides so the + // bypass is robust to address-case differences across data sources. const customAssetIds = new Set( - Object.values(customAssets ?? {}).flat(), + Object.values(customAssets ?? {}) + .flat() + .map((id) => id.toLowerCase()), ); // Always include native asset IDs from NetworkEnablementController @@ -433,11 +438,13 @@ export class TokenDataSource { // EVM: require minimum occurrence count to suppress low-signal tokens. // Tokens with no occurrence data (undefined) are treated the same as // zero occurrences and filtered out. - // Custom assets (user-imported) bypass the occurrence filter. + // Custom assets (user-imported) bypass the occurrence filter — users + // can import whatever they want and we must keep their metadata even + // if the API has fewer than `MIN_TOKEN_OCCURRENCES` aggregator hits. const allowedEvmIds = new Set( evmErc20Ids.filter( (id) => - customAssetIds.has(id) || + customAssetIds.has(id.toLowerCase()) || (occurrencesByAssetId.get(id) ?? 0) >= MIN_TOKEN_OCCURRENCES || id.includes(`/erc20:${MUSD_ADDRESS_LOWERCASE}`), ), @@ -446,10 +453,10 @@ export class TokenDataSource { // Non-EVM: Blockaid bulk scan. // Custom assets (user-imported) bypass Blockaid filtering. const nonEvmToScan = nonEvmTokenIds.filter( - (id) => !customAssetIds.has(id), + (id) => !customAssetIds.has(id.toLowerCase()), ); const allowedNonEvmIds = new Set([ - ...nonEvmTokenIds.filter((id) => customAssetIds.has(id)), + ...nonEvmTokenIds.filter((id) => customAssetIds.has(id.toLowerCase())), ...(await this.#filterBlockaidSpamTokens(nonEvmToScan)), ]); From 2b491d2a6e72eee8ff76139a5425bf97c78c0f33 Mon Sep 17 00:00:00 2001 From: salimtb Date: Wed, 6 May 2026 16:28:34 +0200 Subject: [PATCH 02/11] fix: clean up --- packages/assets-controller/CHANGELOG.md | 6 + packages/assets-controller/package.json | 2 +- .../assets-controller/src/AssetsController.ts | 5 + .../src/data-sources/RpcDataSource.ts | 21 +- .../clients/TokensApiClient.test.ts | 319 ++++++++++++++---- .../clients/TokensApiClient.ts | 204 +++++++++-- .../evm-rpc-services/clients/index.ts | 6 +- .../data-sources/evm-rpc-services/index.ts | 1 + .../services/TokenDetector.ts | 3 +- 9 files changed, 464 insertions(+), 103 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 39096ec5a9..a44fa9a919 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Expose missing `AssetsController:setSelectedCurrency` action through its messenger ([#8719](https://github.com/MetaMask/core/pull/8719)) - Corresponding action type is available as well. +- 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 + - 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. - Bump `@metamask/transaction-controller` from `^65.0.0` to `^65.1.0` ([#8691](https://github.com/MetaMask/core/pull/8691)) - Bump `@metamask/network-enablement-controller` from `^5.0.2` to `^5.1.0` ([#8665](https://github.com/MetaMask/core/pull/8665)) - Bump `@metamask/keyring-controller` from `^25.3.0` to `^25.4.0` ([#8665](https://github.com/MetaMask/core/pull/8665)) diff --git a/packages/assets-controller/package.json b/packages/assets-controller/package.json index 4cd6586553..2c85b69e1f 100644 --- a/packages/assets-controller/package.json +++ b/packages/assets-controller/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/assets-controller", - "version": "6.3.0", + "version": "6.3.1", "description": "Tracks assets balances/prices and handles token detection across all digital assets", "keywords": [ "Ethereum", diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 7a8d0a5416..28d499cb02 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -822,6 +822,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, }); diff --git a/packages/assets-controller/src/data-sources/RpcDataSource.ts b/packages/assets-controller/src/data-sources/RpcDataSource.ts index 941e89e78d..e1417d6b3d 100644 --- a/packages/assets-controller/src/data-sources/RpcDataSource.ts +++ b/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, diff --git a/packages/assets-controller/src/data-sources/evm-rpc-services/clients/TokensApiClient.test.ts b/packages/assets-controller/src/data-sources/evm-rpc-services/clients/TokensApiClient.test.ts index 073268ebde..0500e715e7 100644 --- a/packages/assets-controller/src/data-sources/evm-rpc-services/clients/TokensApiClient.test.ts +++ b/packages/assets-controller/src/data-sources/evm-rpc-services/clients/TokensApiClient.test.ts @@ -1,6 +1,9 @@ import type { ChainId } from '../types'; import { TokensApiClient } from './TokensApiClient'; -import type { TokensApiClientConfig } from './TokensApiClient'; +import type { + TokensApiClientConfig, + TokenListQueryClient, +} from './TokensApiClient'; // ============================================================================= // CONSTANTS @@ -8,19 +11,28 @@ import type { TokensApiClientConfig } from './TokensApiClient'; const MAINNET_CHAIN_ID = '0x1' as ChainId; const POLYGON_CHAIN_ID = '0x89' as ChainId; +const LINEA_MAINNET_CHAIN_ID = '0xe708' as ChainId; +const MEGAETH_MAINNET_CHAIN_ID = '0x10e6' as ChainId; +const TEMPO_MAINNET_CHAIN_ID = '0x1079' as ChainId; -const EXPECTED_BASE_URL = 'https://tokens.api.cx.metamask.io/v3/chains'; +const EXPECTED_BASE_URL = 'https://token.api.cx.metamask.io/tokens'; // ============================================================================= // HELPERS // ============================================================================= +/** + * Mirrors the response shape returned by `token.api.cx.metamask.io/tokens/{chainId}`, + * matching what `TokenListController` consumes via `fetchTokenListByChainId`. + */ type MockApiToken = { - assetId: string; + address: string; symbol?: string; name?: string; decimals?: number; occurrences?: number; + aggregators?: string[]; + iconUrl?: string; }; function createMockResponse( @@ -30,7 +42,7 @@ function createMockResponse( return { ok: status >= 200 && status < 300, status, - json: jest.fn().mockResolvedValue({ data }), + json: jest.fn().mockResolvedValue(data), } as unknown as jest.Mocked; } @@ -74,40 +86,71 @@ describe('TokensApiClient', () => { describe('fetchTokenList', () => { describe('URL construction', () => { - it('converts hex chain ID to CAIP chain ID in the URL', async () => { + it('hits the same `token.api.cx.metamask.io/tokens/{decimalChainId}` endpoint as TokenListController', async () => { const mockFetch = createMockFetch(createMockResponse([])); const client = buildClient({ fetch: mockFetch }); await client.fetchTokenList(MAINNET_CHAIN_ID); const [url] = mockFetch.mock.calls[0] as [string]; - expect(url).toContain(`${EXPECTED_BASE_URL}/eip155:1/assets`); + expect(url).toContain(`${EXPECTED_BASE_URL}/1?`); }); - it('correctly converts a non-mainnet hex chain ID', async () => { + it('converts non-mainnet hex chain IDs to their decimal form in the URL', async () => { const mockFetch = createMockFetch(createMockResponse([])); const client = buildClient({ fetch: mockFetch }); await client.fetchTokenList(POLYGON_CHAIN_ID); const [url] = mockFetch.mock.calls[0] as [string]; - expect(url).toContain(`${EXPECTED_BASE_URL}/eip155:137/assets`); + expect(url).toContain(`${EXPECTED_BASE_URL}/137?`); }); - it('includes all required query parameters', async () => { + it('includes the same query parameters as TokenListController', async () => { const mockFetch = createMockFetch(createMockResponse([])); const client = buildClient({ fetch: mockFetch }); await client.fetchTokenList(MAINNET_CHAIN_ID); const [url] = mockFetch.mock.calls[0] as [string]; - expect(url).toContain('first=25'); - expect(url).toContain('includeOccurrences=true'); - expect(url).toContain('includeMetadata=true'); expect(url).toContain('occurrenceFloor=3'); + expect(url).toContain('includeNativeAssets=false'); + expect(url).toContain('includeTokenFees=false'); + expect(url).toContain('includeAssetType=false'); + expect(url).toContain('includeERC20Permit=false'); + expect(url).toContain('includeStorage=false'); expect(url).toContain('includeRwaData=true'); - expect(url).toContain('excludeDescription=true'); }); + + it('does not pass a `first` cap so the API returns the full per-chain list', async () => { + // Detection deliberately scans the entire occurrenceFloor list (no + // top-N slice) — guard against a client-side cap creeping back in. + const mockFetch = createMockFetch(createMockResponse([])); + const client = buildClient({ fetch: mockFetch }); + + await client.fetchTokenList(MAINNET_CHAIN_ID); + + const [url] = mockFetch.mock.calls[0] as [string]; + expect(url).not.toMatch(/[?&]first=/u); + }); + + it.each([ + ['Linea mainnet', LINEA_MAINNET_CHAIN_ID], + ['MegaETH mainnet', MEGAETH_MAINNET_CHAIN_ID], + ['Tempo mainnet', TEMPO_MAINNET_CHAIN_ID], + ])( + 'lowers occurrenceFloor to 1 on %s (matching TokenListController)', + async (_label, chainId) => { + const mockFetch = createMockFetch(createMockResponse([])); + const client = buildClient({ fetch: mockFetch }); + + await client.fetchTokenList(chainId); + + const [url] = mockFetch.mock.calls[0] as [string]; + expect(url).toContain('occurrenceFloor=1'); + expect(url).not.toContain('occurrenceFloor=3'); + }, + ); }); describe('successful responses', () => { @@ -120,16 +163,17 @@ describe('TokensApiClient', () => { expect(result).toStrictEqual([]); }); - it('maps a valid erc20 token entry to a TokenListEntry', async () => { + it('maps a valid token entry to a TokenListEntry preserving aggregators and iconUrl', async () => { const mockFetch = createMockFetch( createMockResponse([ { - assetId: - 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', + address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', symbol: 'USDC', name: 'USD Coin', decimals: 6, occurrences: 10, + aggregators: ['coinGecko', 'oneInch', 'sushiSwap'], + iconUrl: 'https://example.com/usdc.png', }, ]), ); @@ -144,24 +188,24 @@ describe('TokensApiClient', () => { name: 'USD Coin', decimals: 6, occurrences: 10, + aggregators: ['coinGecko', 'oneInch', 'sushiSwap'], + iconUrl: 'https://example.com/usdc.png', }, ]); }); - it('returns multiple entries when the API returns multiple erc20 tokens', async () => { + it('returns multiple entries when the API returns multiple tokens', async () => { const mockFetch = createMockFetch( createMockResponse([ { - assetId: - 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', + address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', symbol: 'USDC', name: 'USD Coin', decimals: 6, occurrences: 10, }, { - assetId: - 'eip155:1/erc20:0xdAC17F958D2ee523a2206206994597C13D831ec7', + address: '0xdAC17F958D2ee523a2206206994597C13D831ec7', symbol: 'USDT', name: 'Tether USD', decimals: 6, @@ -178,12 +222,12 @@ describe('TokensApiClient', () => { expect(result[1].symbol).toBe('USDT'); }); - it('extracts the contract address from the assetId', async () => { + it('preserves the address as returned by the API', async () => { const tokenAddress = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48'; const mockFetch = createMockFetch( createMockResponse([ { - assetId: `eip155:1/erc20:${tokenAddress}`, + address: tokenAddress, symbol: 'USDC', name: 'USD Coin', decimals: 6, @@ -203,8 +247,7 @@ describe('TokensApiClient', () => { const mockFetch = createMockFetch( createMockResponse([ { - assetId: - 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', + address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', name: 'USD Coin', decimals: 6, }, @@ -221,8 +264,7 @@ describe('TokensApiClient', () => { const mockFetch = createMockFetch( createMockResponse([ { - assetId: - 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', + address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', symbol: 'USDC', decimals: 6, }, @@ -239,8 +281,7 @@ describe('TokensApiClient', () => { const mockFetch = createMockFetch( createMockResponse([ { - assetId: - 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', + address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', symbol: 'USDC', name: 'USD Coin', }, @@ -257,8 +298,7 @@ describe('TokensApiClient', () => { const mockFetch = createMockFetch( createMockResponse([ { - assetId: - 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', + address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', symbol: 'USDC', name: 'USD Coin', decimals: 6, @@ -273,67 +313,94 @@ describe('TokensApiClient', () => { }); }); - describe('filtering non-erc20 assets', () => { - it('filters out native slip44 assets', async () => { + describe('Linea mainnet aggregator filter', () => { + // Mirrors the filter applied in `fetchTokenListByChainId` + // (assets-controllers/src/token-service.ts) so the RPC token detector + // sees the same Linea token set as TokenListController. + it('keeps entries flagged by `lineaTeam` regardless of aggregator count', async () => { const mockFetch = createMockFetch( createMockResponse([ { - assetId: 'eip155:1/slip44:60', - symbol: 'ETH', - name: 'Ether', + address: '0x1111111111111111111111111111111111111111', + symbol: 'A', + name: 'A', decimals: 18, + aggregators: ['lineaTeam'], }, + ]), + ); + const client = buildClient({ fetch: mockFetch }); + + const result = await client.fetchTokenList(LINEA_MAINNET_CHAIN_ID); + + expect(result).toHaveLength(1); + }); + + it('keeps entries with at least 3 aggregators even without `lineaTeam`', async () => { + const mockFetch = createMockFetch( + createMockResponse([ { - assetId: - 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', - symbol: 'USDC', - name: 'USD Coin', - decimals: 6, + address: '0x2222222222222222222222222222222222222222', + symbol: 'B', + name: 'B', + decimals: 18, + aggregators: ['agg1', 'agg2', 'agg3'], }, ]), ); const client = buildClient({ fetch: mockFetch }); - const result = await client.fetchTokenList(MAINNET_CHAIN_ID); + const result = await client.fetchTokenList(LINEA_MAINNET_CHAIN_ID); expect(result).toHaveLength(1); - expect(result[0].symbol).toBe('USDC'); }); - it('filters out erc721 assets', async () => { + it('drops entries with fewer than 3 aggregators and no `lineaTeam` flag', async () => { const mockFetch = createMockFetch( createMockResponse([ { - assetId: - 'eip155:1/erc721:0xBC4CA0EdA7647A8aB7C2061c2E118A18a936f13D', - symbol: 'BAYC', - name: 'Bored Ape Yacht Club', - decimals: 0, + address: '0x3333333333333333333333333333333333333333', + symbol: 'C', + name: 'C', + decimals: 18, + aggregators: ['agg1', 'agg2'], }, ]), ); const client = buildClient({ fetch: mockFetch }); - const result = await client.fetchTokenList(MAINNET_CHAIN_ID); + const result = await client.fetchTokenList(LINEA_MAINNET_CHAIN_ID); expect(result).toStrictEqual([]); }); - it('returns an empty array when all items are non-erc20', async () => { + it('drops entries missing the aggregators field on Linea', async () => { const mockFetch = createMockFetch( createMockResponse([ { - assetId: 'eip155:1/slip44:60', - symbol: 'ETH', - name: 'Ether', + address: '0x4444444444444444444444444444444444444444', + symbol: 'D', + name: 'D', decimals: 18, }, + ]), + ); + const client = buildClient({ fetch: mockFetch }); + + const result = await client.fetchTokenList(LINEA_MAINNET_CHAIN_ID); + + expect(result).toStrictEqual([]); + }); + + it('does not apply the Linea filter on other chains', async () => { + const mockFetch = createMockFetch( + createMockResponse([ { - assetId: - 'eip155:1/erc721:0xBC4CA0EdA7647A8aB7C2061c2E118A18a936f13D', - symbol: 'BAYC', - name: 'BAYC', - decimals: 0, + address: '0x5555555555555555555555555555555555555555', + symbol: 'E', + name: 'E', + decimals: 18, + aggregators: ['agg1'], }, ]), ); @@ -341,7 +408,7 @@ describe('TokensApiClient', () => { const result = await client.fetchTokenList(MAINNET_CHAIN_ID); - expect(result).toStrictEqual([]); + expect(result).toHaveLength(1); }); }); @@ -351,7 +418,7 @@ describe('TokensApiClient', () => { const client = buildClient({ fetch: mockFetch }); await expect(client.fetchTokenList(MAINNET_CHAIN_ID)).rejects.toThrow( - 'Tokens API responded with 404 for eip155:1', + 'Tokens API responded with 404 for chain 0x1', ); }); @@ -360,16 +427,16 @@ describe('TokensApiClient', () => { const client = buildClient({ fetch: mockFetch }); await expect(client.fetchTokenList(MAINNET_CHAIN_ID)).rejects.toThrow( - 'Tokens API responded with 500 for eip155:1', + 'Tokens API responded with 500 for chain 0x1', ); }); - it('includes the CAIP chain ID in the error message', async () => { + it('includes the hex chain ID in the error message', async () => { const mockFetch = createMockFetch(createMockResponse([], 503)); const client = buildClient({ fetch: mockFetch }); await expect(client.fetchTokenList(POLYGON_CHAIN_ID)).rejects.toThrow( - 'eip155:137', + '0x89', ); }); @@ -384,6 +451,130 @@ describe('TokensApiClient', () => { 'Network failure', ); }); + + it('returns an empty array if the response is not a JSON array', async () => { + const malformed = { + ok: true, + status: 200, + json: jest.fn().mockResolvedValue({ unexpected: 'shape' }), + } as unknown as jest.Mocked; + const mockFetch = jest.fn().mockResolvedValue(malformed); + const client = buildClient({ + fetch: mockFetch as unknown as typeof globalThis.fetch, + }); + + const result = await client.fetchTokenList(MAINNET_CHAIN_ID); + + expect(result).toStrictEqual([]); + }); + }); + + describe('TanStack Query caching', () => { + // Minimal in-memory cache that mimics the surface of TanStack Query's + // `QueryClient.fetchQuery`: dedupes by serialized queryKey and never + // expires within a test (sufficient for asserting cache hits). + const buildFakeQueryClient = (): TokenListQueryClient => { + const cache = new Map>(); + return { + fetchQuery: async ({ + queryKey, + queryFn, + }: { + queryKey: readonly unknown[]; + queryFn: () => Promise; + }): Promise => { + const key = JSON.stringify(queryKey); + const cached = cache.get(key) as Promise | undefined; + if (cached) { + return cached; + } + const pending = queryFn(); + cache.set(key, pending); + return pending; + }, + }; + }; + + it('reuses the cached response on the second call for the same chain', async () => { + const mockFetch = createMockFetch( + createMockResponse([ + { + address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', + symbol: 'USDC', + name: 'USD Coin', + decimals: 6, + occurrences: 10, + }, + ]), + ); + const queryClient = buildFakeQueryClient(); + const client = buildClient({ fetch: mockFetch, queryClient }); + + const first = await client.fetchTokenList(MAINNET_CHAIN_ID); + const second = await client.fetchTokenList(MAINNET_CHAIN_ID); + + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(second).toStrictEqual(first); + }); + + it('refetches when called with a different chain id', async () => { + const mockFetch = createMockFetch(createMockResponse([])); + const queryClient = buildFakeQueryClient(); + const client = buildClient({ fetch: mockFetch, queryClient }); + + await client.fetchTokenList(MAINNET_CHAIN_ID); + await client.fetchTokenList(POLYGON_CHAIN_ID); + + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + + it('dedupes concurrent in-flight requests for the same chain', async () => { + // The fake QueryClient stores the in-flight Promise on the first call + // and returns it for subsequent callers — same behaviour as TanStack + // Query, and the property the detector relies on across accounts. + let resolveFetch: ((value: Response) => void) | undefined; + const mockFetch = jest.fn( + () => + new Promise((resolve) => { + resolveFetch = resolve; + }), + ); + const queryClient = buildFakeQueryClient(); + const client = buildClient({ + fetch: mockFetch as unknown as typeof globalThis.fetch, + queryClient, + }); + + const a = client.fetchTokenList(MAINNET_CHAIN_ID); + const b = client.fetchTokenList(MAINNET_CHAIN_ID); + + resolveFetch?.(createMockResponse([])); + await Promise.all([a, b]); + + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('does not cap the page size in the cached path either', async () => { + const mockFetch = createMockFetch(createMockResponse([])); + const queryClient = buildFakeQueryClient(); + const client = buildClient({ fetch: mockFetch, queryClient }); + + await client.fetchTokenList(MAINNET_CHAIN_ID); + + const [url] = mockFetch.mock.calls[0] as [string]; + expect(url).not.toMatch(/[?&]first=/u); + }); + + it('falls back to direct fetch when no queryClient is provided', async () => { + const mockFetch = createMockFetch(createMockResponse([])); + const client = buildClient({ fetch: mockFetch }); + + await client.fetchTokenList(MAINNET_CHAIN_ID); + await client.fetchTokenList(MAINNET_CHAIN_ID); + + // Without a queryClient there is no cache, so each call hits the network. + expect(mockFetch).toHaveBeenCalledTimes(2); + }); }); }); }); diff --git a/packages/assets-controller/src/data-sources/evm-rpc-services/clients/TokensApiClient.ts b/packages/assets-controller/src/data-sources/evm-rpc-services/clients/TokensApiClient.ts index 77879ad1ee..006fb604d1 100644 --- a/packages/assets-controller/src/data-sources/evm-rpc-services/clients/TokensApiClient.ts +++ b/packages/assets-controller/src/data-sources/evm-rpc-services/clients/TokensApiClient.ts @@ -1,76 +1,214 @@ +import { + ChainId as ControllerChainId, + convertHexToDecimal, +} from '@metamask/controller-utils'; + import type { ChainId, TokenListEntry } from '../types'; -const TOKENS_API_BASE_URL = 'https://tokens.api.cx.metamask.io/v3/chains'; +/** + * Same host + path that `TokenListController` (assets-controllers) uses + * (`token-service.ts` → `getTokensURL`). Sharing this endpoint keeps the + * RPC token detector aligned with the rest of the wallet's token listing + * (occurrence floors, aggregator filters, icon URLs, etc.). + */ +const TOKEN_END_POINT_API = 'https://token.api.cx.metamask.io'; -/** How many tokens to request from the API per chain. */ -const TOKENS_API_FIRST = 25; +/** + * Tempo Mainnet — not yet present in `@metamask/controller-utils`'s `ChainId` + * map at the time of writing, so it's spelled out as a literal here exactly as + * `TokenListController` does (see `token-service.ts:getTokensURL`). + */ +const TEMPO_MAINNET_CHAIN_ID = '0x1079' as const; -/** Shape of a single item in the Tokens API response `data` array. */ -type ApiTokenData = { - assetId: string; +/** + * Per-chain occurrence floor, mirroring `TokenListController.getTokensURL`: + * Linea mainnet, MegaETH mainnet, and Tempo mainnet have thinner aggregator + * coverage so we lower the floor; everything else uses the default 3. + * + * @param hexChainId - Hex chain ID. + * @returns The occurrence floor to send to the Tokens API. + */ +function getOccurrenceFloor(hexChainId: ChainId): number { + if ( + hexChainId === ControllerChainId['linea-mainnet'] || + hexChainId === ControllerChainId['megaeth-mainnet'] || + hexChainId === TEMPO_MAINNET_CHAIN_ID + ) { + return 1; + } + return 3; +} + +/** + * TanStack-Query cache config for the cached `fetchTokenList` path. + * + * The Tokens API per-chain list barely changes between polls, so we keep + * results fresh for a few minutes (`staleTime`) and retain them in cache for + * an hour (`gcTime`) so re-detections across accounts/chains hit the cache. + * These tunings only apply when a `queryClient` is provided to the client; + * the uncached fallback path (used in standalone tests) is unaffected. + */ +const TOKEN_LIST_STALE_TIME_MS = 5 * 60_000; +const TOKEN_LIST_GC_TIME_MS = 60 * 60_000; + +/** + * Shape of a single item returned by the Tokens API `/tokens/{chainId}` + * endpoint. Mirrors `TokenListToken` in + * `packages/assets-controllers/src/TokenListController.ts` and the response + * parsed by `fetchTokenListByChainId` in `token-service.ts`. + */ +type ApiTokenListItem = { + address: string; symbol?: string; name?: string; decimals?: number; occurrences?: number; + aggregators?: string[]; + iconUrl?: string; +}; + +/** + * Minimal structural type for the TanStack Query client method we use. + * Avoids a direct dependency on `@tanstack/query-core` while still being + * fully compatible with the shared `QueryClient` exposed by + * `ApiPlatformClient.queryClient` (`@metamask/core-backend`). + */ +export type TokenListQueryClient = { + fetchQuery(options: { + queryKey: readonly unknown[]; + queryFn: () => Promise; + staleTime?: number; + gcTime?: number; + }): Promise; }; export type TokensApiClientConfig = { /** Fetch function (defaults to globalThis.fetch). */ fetch?: typeof globalThis.fetch; + /** + * Optional TanStack-Query client used to cache token-list responses across + * detector polls / accounts / instances. When omitted, every call hits the + * network directly (preserves prior behaviour for tests and standalone use). + */ + queryClient?: TokenListQueryClient; }; /** * Client for the MetaMask Tokens API. - * Fetches the top ERC-20 tokens for a given chain (occurrenceFloor=3, first=25). + * + * Fetches the per-chain ERC-20 token list from the same endpoint that + * `TokenListController` uses (`token.api.cx.metamask.io/tokens/{chainId}`), + * with the same query parameters and the same per-chain occurrence floor / + * Linea aggregator filter. This keeps RPC token detection in lockstep with + * the wallet's primary token list. + * + * When constructed with a `queryClient`, results are cached and deduped via + * TanStack Query (5 min staleTime, 1 h gcTime), so concurrent detection cycles + * for the same chain coalesce into a single network request. */ export class TokensApiClient { readonly #fetch: typeof globalThis.fetch; + readonly #queryClient: TokenListQueryClient | undefined; + constructor(config?: TokensApiClientConfig) { this.#fetch = config?.fetch ?? globalThis.fetch.bind(globalThis); + this.#queryClient = config?.queryClient; } /** - * Fetch the list of top ERC-20 tokens for a chain from the Tokens API. - * Only `erc20` assets are returned; native (`slip44`) entries are skipped. + * Fetch the list of ERC-20 tokens for a chain from the Tokens API. * * @param hexChainId - Chain ID in hex format (e.g. `'0x1'` for Ethereum mainnet). * @returns Array of token list entries with address and metadata. * @throws If the API responds with a non-2xx status. */ async fetchTokenList(hexChainId: ChainId): Promise { - const chainIdDecimal = parseInt(hexChainId, 16); - const caipChainId = `eip155:${chainIdDecimal}`; + const queryClient = this.#queryClient; + if (queryClient === undefined) { + return this.#fetchTokenListUncached(hexChainId); + } + + return queryClient.fetchQuery({ + // Namespacing keeps this key from colliding with other clients that + // share the same QueryClient (e.g. core-backend's ApiPlatformClient). + queryKey: [ + 'assets-controller', + 'rpc-detection', + 'token-list', + { chainId: hexChainId }, + ], + queryFn: () => this.#fetchTokenListUncached(hexChainId), + staleTime: TOKEN_LIST_STALE_TIME_MS, + gcTime: TOKEN_LIST_GC_TIME_MS, + }); + } + async #fetchTokenListUncached( + hexChainId: ChainId, + ): Promise { + const decimalChainId = convertHexToDecimal(hexChainId); + const occurrenceFloor = getOccurrenceFloor(hexChainId); + + // Mirrors `TokenListController.getTokensURL` exactly (token-service.ts). + // No `first=...` cap — the API returns the full per-chain list bounded + // server-side by `occurrenceFloor`. const url = - `${TOKENS_API_BASE_URL}/${caipChainId}/assets` + - `?first=${TOKENS_API_FIRST}` + - `&includeOccurrences=true` + - `&includeMetadata=true` + - `&occurrenceFloor=3` + - `&includeRwaData=true` + - `&excludeDescription=true`; + `${TOKEN_END_POINT_API}/tokens/${decimalChainId}` + + `?occurrenceFloor=${occurrenceFloor}` + + `&includeNativeAssets=false` + + `&includeTokenFees=false` + + `&includeAssetType=false` + + `&includeERC20Permit=false` + + `&includeStorage=false` + + `&includeRwaData=true`; const response = await this.#fetch(url); if (!response.ok) { throw new Error( - `Tokens API responded with ${response.status} for ${caipChainId}`, + `Tokens API responded with ${response.status} for chain ${hexChainId}`, ); } - const { data } = (await response.json()) as { data: ApiTokenData[] }; - - return data - .filter((item) => item.assetId.includes('/erc20:')) - .map((item) => { - const address = item.assetId.split('/erc20:')[1]; - return { - address, - symbol: item.symbol ?? '', - name: item.name ?? '', - decimals: item.decimals ?? 18, - occurrences: item.occurrences, - }; - }); + const raw = (await response.json()) as unknown; + const items: ApiTokenListItem[] = Array.isArray(raw) + ? (raw as ApiTokenListItem[]) + : []; + + const filtered = applyChainSpecificFilters(hexChainId, items); + + return filtered.map((item) => ({ + address: item.address, + symbol: item.symbol ?? '', + name: item.name ?? '', + decimals: item.decimals ?? 18, + iconUrl: item.iconUrl, + aggregators: item.aggregators, + occurrences: item.occurrences, + })); + } +} + +/** + * Apply chain-specific filters to a raw token list response, mirroring + * `fetchTokenListByChainId` in `assets-controllers/src/token-service.ts`. + * + * For Linea mainnet, the API returns extras with low aggregator coverage, so + * we keep only entries flagged by Linea's own team or seen by ≥3 aggregators. + * + * @param hexChainId - Hex chain ID. + * @param items - Raw items from the API response. + * @returns Items after chain-specific filtering. + */ +function applyChainSpecificFilters( + hexChainId: ChainId, + items: ApiTokenListItem[], +): ApiTokenListItem[] { + if (hexChainId === ControllerChainId['linea-mainnet']) { + return items.filter((item) => { + const aggregators = item.aggregators ?? []; + return aggregators.includes('lineaTeam') || aggregators.length >= 3; + }); } + return items; } diff --git a/packages/assets-controller/src/data-sources/evm-rpc-services/clients/index.ts b/packages/assets-controller/src/data-sources/evm-rpc-services/clients/index.ts index 53d8d1f66e..59afaabeb0 100644 --- a/packages/assets-controller/src/data-sources/evm-rpc-services/clients/index.ts +++ b/packages/assets-controller/src/data-sources/evm-rpc-services/clients/index.ts @@ -5,7 +5,11 @@ export { type MulticallClientConfig, } from './MulticallClient'; -export { TokensApiClient, type TokensApiClientConfig } from './TokensApiClient'; +export { + TokensApiClient, + type TokensApiClientConfig, + type TokenListQueryClient, +} from './TokensApiClient'; // Re-export provider types from types module export type { GetProviderFunction, Provider } from '../types'; diff --git a/packages/assets-controller/src/data-sources/evm-rpc-services/index.ts b/packages/assets-controller/src/data-sources/evm-rpc-services/index.ts index 2e3f571136..f193260d18 100644 --- a/packages/assets-controller/src/data-sources/evm-rpc-services/index.ts +++ b/packages/assets-controller/src/data-sources/evm-rpc-services/index.ts @@ -16,6 +16,7 @@ export { type MulticallClientConfig, TokensApiClient, type TokensApiClientConfig, + type TokenListQueryClient, } from './clients'; export { BalanceFetcher, diff --git a/packages/assets-controller/src/data-sources/evm-rpc-services/services/TokenDetector.ts b/packages/assets-controller/src/data-sources/evm-rpc-services/services/TokenDetector.ts index 558f16e96e..5c5c4e4f96 100644 --- a/packages/assets-controller/src/data-sources/evm-rpc-services/services/TokenDetector.ts +++ b/packages/assets-controller/src/data-sources/evm-rpc-services/services/TokenDetector.ts @@ -212,7 +212,7 @@ export class TokenDetector extends StaticIntervalPollingControllerOnly Date: Wed, 6 May 2026 16:41:36 +0200 Subject: [PATCH 03/11] fix: fix changelog --- packages/assets-controller/CHANGELOG.md | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 0ee77f32b1..47fbdf750e 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -7,19 +7,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [6.4.0] - ### Added -- Expose missing `AssetsController:setSelectedCurrency` action through its messenger ([#8719](https://github.com/MetaMask/core/pull/8719)) - - Corresponding action type is available as well. -- 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 +- 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. +### 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. + +## [6.4.0] + +### Added + +- Expose missing `AssetsController:setSelectedCurrency` action through its messenger ([#8719](https://github.com/MetaMask/core/pull/8719)) + - Corresponding action type is available as well. + ### Changed - Bump `@metamask/transaction-controller` from `^65.0.0` to `^65.1.0` ([#8691](https://github.com/MetaMask/core/pull/8691)) @@ -29,11 +37,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/account-tree-controller` from `^7.1.0` to `^7.2.0` ([#8665](https://github.com/MetaMask/core/pull/8665)) - Bump `@metamask/assets-controllers` from `^105.1.0` to `^106.0.0` ([#8721](https://github.com/MetaMask/core/pull/8721)) -### Fixed - -- `TokenDataSource` no longer drops user-imported EVM custom asset metadata when the V3 Tokens API returns the asset ID lower-cased - - 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. - ## [6.3.0] ### Added From f60a0b769dbd21d168f8f8a87f0d1adb86499f41 Mon Sep 17 00:00:00 2001 From: salimtb Date: Wed, 6 May 2026 16:45:52 +0200 Subject: [PATCH 04/11] fix: fix linter --- .../assets-controller/src/data-sources/TokenDataSource.ts | 4 +++- .../data-sources/evm-rpc-services/services/TokenDetector.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/assets-controller/src/data-sources/TokenDataSource.ts b/packages/assets-controller/src/data-sources/TokenDataSource.ts index c5ffc245be..110831664f 100644 --- a/packages/assets-controller/src/data-sources/TokenDataSource.ts +++ b/packages/assets-controller/src/data-sources/TokenDataSource.ts @@ -456,7 +456,9 @@ export class TokenDataSource { (id) => !customAssetIds.has(id.toLowerCase()), ); const allowedNonEvmIds = new Set([ - ...nonEvmTokenIds.filter((id) => customAssetIds.has(id.toLowerCase())), + ...nonEvmTokenIds.filter((id) => + customAssetIds.has(id.toLowerCase()), + ), ...(await this.#filterBlockaidSpamTokens(nonEvmToScan)), ]); diff --git a/packages/assets-controller/src/data-sources/evm-rpc-services/services/TokenDetector.ts b/packages/assets-controller/src/data-sources/evm-rpc-services/services/TokenDetector.ts index 5c5c4e4f96..e63a907f37 100644 --- a/packages/assets-controller/src/data-sources/evm-rpc-services/services/TokenDetector.ts +++ b/packages/assets-controller/src/data-sources/evm-rpc-services/services/TokenDetector.ts @@ -212,7 +212,7 @@ export class TokenDetector extends StaticIntervalPollingControllerOnly Date: Wed, 6 May 2026 17:24:50 +0200 Subject: [PATCH 05/11] fix: fix changelog --- packages/assets-controller/CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 6599143af3..dba46055f0 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -16,11 +16,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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. -### 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. - ### Changed - Bump `@metamask/account-tree-controller` from `^7.2.0` to `^7.3.0` ([#8722](https://github.com/MetaMask/core/pull/8722)) @@ -28,6 +23,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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. + ## [6.4.0] ### Added From 2b316ced3efa68e56873ff4137faceb081cb7ffd Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 7 May 2026 11:52:24 +0200 Subject: [PATCH 06/11] fix: preserve richer native token metadata in RpcDataSource `RpcDataSource` previously emitted a minimal stub for native asset metadata (`{ type: 'native', symbol: chainStatus.nativeCurrency, name: chainStatus.nativeCurrency, decimals: 18 }`) on every balance fetch. Because the merge logic in `AssetsController#updateState` only preserves existing metadata when the incoming payload has no `symbol` and no `name`, this stub clobbered enrichment from the price/info API (image, description, occurrences, aggregators, ...) and renamed "Avalanche" to "AVAX" on every refresh. Mirror the existing ERC-20 behavior for native assets: prefer the existing metadata in state when present and only emit the stub when nothing is in state yet (e.g. first sighting of the asset). The fallback in the balance-fetch error path is gated the same way. --- packages/assets-controller/CHANGELOG.md | 3 + .../src/data-sources/RpcDataSource.ts | 62 ++++++++++++------- 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index dba46055f0..4f9ef2961f 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -27,6 +27,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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] diff --git a/packages/assets-controller/src/data-sources/RpcDataSource.ts b/packages/assets-controller/src/data-sources/RpcDataSource.ts index e1417d6b3d..3656221936 100644 --- a/packages/assets-controller/src/data-sources/RpcDataSource.ts +++ b/packages/assets-controller/src/data-sources/RpcDataSource.ts @@ -373,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; } } @@ -1038,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)) { From 4ec060eb88351fc5324aaf5ec8f3f832b527db00 Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 7 May 2026 16:37:13 +0200 Subject: [PATCH 07/11] fix: only graduate custom assets when AccountsAPI returns a positive balance AccountsAPI can include zero-balance entries for tokens it indexes but the user no longer holds. Graduating those would remove them from `customAssets` and stop the supplemental RPC poll, leaving incoming transfers undetected until the next milestone fetch. Skip graduation unless the reported amount is strictly greater than zero. --- .../CustomAssetGraduationMiddleware.test.ts | 109 ++++++++++++++++++ .../CustomAssetGraduationMiddleware.ts | 44 ++++++- 2 files changed, 148 insertions(+), 5 deletions(-) diff --git a/packages/assets-controller/src/middlewares/CustomAssetGraduationMiddleware.test.ts b/packages/assets-controller/src/middlewares/CustomAssetGraduationMiddleware.test.ts index 843fbe0589..02331161e9 100644 --- a/packages/assets-controller/src/middlewares/CustomAssetGraduationMiddleware.test.ts +++ b/packages/assets-controller/src/middlewares/CustomAssetGraduationMiddleware.test.ts @@ -156,6 +156,115 @@ describe('CustomAssetGraduationMiddleware', () => { ); }); + it('does not graduate when AccountsAPI returns a zero balance', async () => { + // The API may include zero entries for tokens it indexes but the user + // no longer holds. Keeping them in customAssets ensures RPC keeps + // polling so a future incoming transfer is reflected immediately. + const { middleware, context, removeCustomAsset } = setup({ + [MOCK_ACCOUNT_ID]: [EVM_CUSTOM_ASSET], + }); + context.response = { + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [EVM_CUSTOM_ASSET]: { amount: '0' }, + }, + }, + }; + const next = jest.fn().mockImplementation((ctx) => Promise.resolve(ctx)); + + await middleware.assetsMiddleware(context, next); + + expect(removeCustomAsset).not.toHaveBeenCalled(); + }); + + it('does not graduate when the decimal amount is zero', async () => { + // Both AccountsApi and BackendWebsocketDataSource emit human-readable + // decimal strings, so "0.0" must be treated the same as "0". + const { middleware, context, removeCustomAsset } = setup({ + [MOCK_ACCOUNT_ID]: [EVM_CUSTOM_ASSET], + }); + context.response = { + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [EVM_CUSTOM_ASSET]: { amount: '0.0' }, + }, + }, + }; + const next = jest.fn().mockImplementation((ctx) => Promise.resolve(ctx)); + + await middleware.assetsMiddleware(context, next); + + expect(removeCustomAsset).not.toHaveBeenCalled(); + }); + + it('graduates when AccountsAPI returns a small positive decimal balance (V5 format)', async () => { + // V5 returns amounts already divided by decimals, e.g. WETH: + // { balance: "0.283549083429656057", decimals: 18 }. The middleware + // must recognise these as positive without any further unit handling. + const { middleware, context, removeCustomAsset } = setup({ + [MOCK_ACCOUNT_ID]: [EVM_CUSTOM_ASSET], + }); + context.response = { + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [EVM_CUSTOM_ASSET]: { amount: '0.283549083429656057' }, + }, + }, + }; + const next = jest.fn().mockImplementation((ctx) => Promise.resolve(ctx)); + + await middleware.assetsMiddleware(context, next); + + expect(removeCustomAsset).toHaveBeenCalledTimes(1); + expect(removeCustomAsset).toHaveBeenCalledWith( + MOCK_ACCOUNT_ID, + EVM_CUSTOM_ASSET, + ); + }); + + it('graduates only custom assets whose balance is positive', async () => { + // Mixed response: one asset has zero balance, another has a real + // balance. Only the latter should graduate. + const { middleware, context, removeCustomAsset } = setup({ + [MOCK_ACCOUNT_ID]: [EVM_CUSTOM_ASSET, EVM_OTHER_ASSET], + }); + context.response = { + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [EVM_CUSTOM_ASSET]: { amount: '0' }, + [EVM_OTHER_ASSET]: { amount: '500' }, + }, + }, + }; + const next = jest.fn().mockImplementation((ctx) => Promise.resolve(ctx)); + + await middleware.assetsMiddleware(context, next); + + expect(removeCustomAsset).toHaveBeenCalledTimes(1); + expect(removeCustomAsset).toHaveBeenCalledWith( + MOCK_ACCOUNT_ID, + EVM_OTHER_ASSET, + ); + }); + + it('does not graduate when the balance amount is malformed', async () => { + const { middleware, context, removeCustomAsset } = setup({ + [MOCK_ACCOUNT_ID]: [EVM_CUSTOM_ASSET], + }); + context.response = { + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [EVM_CUSTOM_ASSET]: { amount: 'not-a-number' }, + }, + }, + }; + const next = jest.fn().mockImplementation((ctx) => Promise.resolve(ctx)); + + await middleware.assetsMiddleware(context, next); + + expect(removeCustomAsset).not.toHaveBeenCalled(); + }); + it('does not graduate non-EVM (Solana) custom assets', async () => { const { middleware, context, removeCustomAsset } = setup({ [MOCK_ACCOUNT_ID]: [SOLANA_CUSTOM_ASSET], diff --git a/packages/assets-controller/src/middlewares/CustomAssetGraduationMiddleware.ts b/packages/assets-controller/src/middlewares/CustomAssetGraduationMiddleware.ts index db5b4375ab..3fec59bcce 100644 --- a/packages/assets-controller/src/middlewares/CustomAssetGraduationMiddleware.ts +++ b/packages/assets-controller/src/middlewares/CustomAssetGraduationMiddleware.ts @@ -2,7 +2,12 @@ import { KnownCaipNamespace } from '@metamask/utils'; import { projectLogger, createModuleLogger } from '../logger'; import { forDataTypes } from '../types'; -import type { AccountId, Caip19AssetId, Middleware } from '../types'; +import type { + AccountId, + AssetBalance, + Caip19AssetId, + Middleware, +} from '../types'; import { normalizeAssetId } from '../utils'; const CONTROLLER_NAME = 'CustomAssetGraduationMiddleware'; @@ -16,15 +21,22 @@ export type CustomAssetGraduationMiddlewareOptions = { /** * CustomAssetGraduationMiddleware removes EVM assets from `customAssets` when - * an upstream balance source (AccountsAPI / Websocket) reports a balance for - * them. Once a detector sees the asset, it no longer needs to be tracked as - * "custom" — the regular detection flow will keep it fresh. + * an upstream balance source (AccountsAPI / Websocket) reports a non-zero + * balance for them. Once a detector sees the asset with a real balance, it + * no longer needs to be tracked as "custom" — the regular detection flow + * will keep it fresh. * * Rules: - * - Only the selected account's custom assets are considered. + * - Only the selected account's custom assets are considered. Switching the + * selected account triggers a fresh fetch, which re-runs graduation + * against the new account's balances. * - Only EVM (CAIP-2 namespace `eip155`) assets graduate. Non-EVM custom * assets (Solana, BTC, Tron, etc. — served by Snap data sources) are left * alone. + * - Only positive balances graduate. A zero balance from AccountsAPI means + * the API knows about the token but the user does not currently hold it; + * keeping it in `customAssets` ensures RPC keeps polling so a future + * incoming transfer is reflected promptly. */ export class CustomAssetGraduationMiddleware { readonly name = CONTROLLER_NAME; @@ -79,6 +91,9 @@ export class CustomAssetGraduationMiddleware { if (!isEvmAssetId(rawAssetId)) { continue; } + if (!hasPositiveBalance(returnedBalances[rawAssetId])) { + continue; + } const normalizedAssetId = safeNormalize(rawAssetId); if (!customSet.has(normalizedAssetId)) { continue; @@ -124,3 +139,22 @@ function safeNormalize(assetId: Caip19AssetId): Caip19AssetId { return assetId; } } + +/** + * Whether a balance entry reports a strictly positive amount. AccountsAPI + * may return zero for tokens it indexes but the user no longer holds; we + * treat those as non-graduating so RPC keeps polling and surfaces any + * future incoming transfer immediately. + * + * `AssetBalance.amount` is already a human-readable decimal string from + * both AccountsApi (e.g. "0.283549083429656057") and the websocket data + * source (which divides by `decimals` before emitting), so a `Number()` + * sign check is safe: `NaN`, `undefined`, empty strings, and zero all + * fail the comparison. + * + * @param balance - The balance entry from the response. + * @returns `true` when the balance amount represents a value greater than 0. + */ +function hasPositiveBalance(balance: AssetBalance | undefined): boolean { + return Number(balance?.amount) > 0; +} From 76d0e20b13b2bffbe8eee9b0fbada4e846328b2a Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 7 May 2026 16:56:32 +0200 Subject: [PATCH 08/11] feat: purge per-chain assets state on NetworkController:networkRemoved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a network is permanently removed from NetworkController the user has no way to navigate back to it, so leftover entries in `assetsBalance`, `assetsInfo`, `assetsPrice`, `customAssets`, and `assetPreferences` just bloat persisted state forever. Subscribe to the existing `NetworkController:networkRemoved` event and drop every CAIP-19-keyed slice for the removed chain. Per-account containers that become empty after the purge are pruned. Subscription teardown is unchanged — it still cascades through NetworkEnablementController. Disabling a chain still preserves data (handled separately in `#handleEnabledNetworksChanged`). --- .../src/AssetsController.test.ts | 181 ++++++++++++++++++ .../assets-controller/src/AssetsController.ts | 92 ++++++++- 2 files changed, 272 insertions(+), 1 deletion(-) diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index d965ca0464..86a25b4692 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -1905,6 +1905,187 @@ describe('AssetsController', () => { expect(true).toBe(true); }); }); + + describe('NetworkController:networkRemoved', () => { + // Asset IDs spanning two chains so the test verifies both that the + // removed chain's entries are purged and the surviving chain's are kept. + const MAINNET_ASSET_ID = + 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48' as Caip19AssetId; + const POLYGON_ASSET_ID = + 'eip155:137/erc20:0xc2132D05D31c914a87C6611C10748AEb04B58e8F' as Caip19AssetId; + const POLYGON_NATIVE_ID = + 'eip155:137/slip44:966' as Caip19AssetId; + + const seededState: Partial = { + assetsInfo: { + [MAINNET_ASSET_ID]: { + type: 'erc20', + symbol: 'USDC', + name: 'USD Coin', + decimals: 6, + }, + [POLYGON_ASSET_ID]: { + type: 'erc20', + symbol: 'USDT', + name: 'Tether', + decimals: 6, + }, + [POLYGON_NATIVE_ID]: { + type: 'native', + symbol: 'POL', + name: 'Polygon', + decimals: 18, + }, + }, + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [MAINNET_ASSET_ID]: { amount: '1' }, + [POLYGON_ASSET_ID]: { amount: '2' }, + [POLYGON_NATIVE_ID]: { amount: '3' }, + }, + }, + assetsPrice: { + [MAINNET_ASSET_ID]: { + assetPriceType: 'fungible', + price: 1, + lastUpdated: 0, + usdPrice: 1, + }, + [POLYGON_ASSET_ID]: { + assetPriceType: 'fungible', + price: 1, + lastUpdated: 0, + usdPrice: 1, + }, + }, + customAssets: { + [MOCK_ACCOUNT_ID]: [MAINNET_ASSET_ID, POLYGON_ASSET_ID], + }, + assetPreferences: { + [MAINNET_ASSET_ID]: { hidden: true }, + [POLYGON_ASSET_ID]: { hidden: true }, + }, + }; + + it('purges every per-chain state slice for the removed chain', async () => { + await withController( + { state: seededState }, + async ({ controller, messenger }) => { + (messenger.publish as CallableFunction)( + 'NetworkController:networkRemoved', + { chainId: '0x89' }, + ); + + await new Promise(process.nextTick); + + // Mainnet state remains intact. + expect(controller.state.assetsInfo[MAINNET_ASSET_ID]).toBeDefined(); + expect( + controller.state.assetsBalance[MOCK_ACCOUNT_ID][MAINNET_ASSET_ID], + ).toBeDefined(); + expect( + controller.state.assetsPrice[MAINNET_ASSET_ID], + ).toBeDefined(); + expect( + controller.state.customAssets[MOCK_ACCOUNT_ID], + ).toContain(MAINNET_ASSET_ID); + expect( + controller.state.assetPreferences[MAINNET_ASSET_ID], + ).toBeDefined(); + + // Polygon state purged across every slice. + expect( + controller.state.assetsInfo[POLYGON_ASSET_ID], + ).toBeUndefined(); + expect( + controller.state.assetsInfo[POLYGON_NATIVE_ID], + ).toBeUndefined(); + expect( + controller.state.assetsBalance[MOCK_ACCOUNT_ID][POLYGON_ASSET_ID], + ).toBeUndefined(); + expect( + controller.state.assetsBalance[MOCK_ACCOUNT_ID][ + POLYGON_NATIVE_ID + ], + ).toBeUndefined(); + expect( + controller.state.assetsPrice[POLYGON_ASSET_ID], + ).toBeUndefined(); + expect( + controller.state.customAssets[MOCK_ACCOUNT_ID], + ).not.toContain(POLYGON_ASSET_ID); + expect( + controller.state.assetPreferences[POLYGON_ASSET_ID], + ).toBeUndefined(); + }, + ); + }); + + it('removes empty per-account containers after purge', async () => { + const onlyPolygon: Partial = { + assetsBalance: { + [MOCK_ACCOUNT_ID]: { + [POLYGON_ASSET_ID]: { amount: '2' }, + }, + }, + customAssets: { + [MOCK_ACCOUNT_ID]: [POLYGON_ASSET_ID], + }, + }; + + await withController( + { state: onlyPolygon }, + async ({ controller, messenger }) => { + (messenger.publish as CallableFunction)( + 'NetworkController:networkRemoved', + { chainId: '0x89' }, + ); + + await new Promise(process.nextTick); + + expect( + controller.state.assetsBalance[MOCK_ACCOUNT_ID], + ).toBeUndefined(); + expect( + controller.state.customAssets[MOCK_ACCOUNT_ID], + ).toBeUndefined(); + }, + ); + }); + + it('is a no-op when no state matches the removed chain', async () => { + const mainnetOnly: Partial = { + assetsInfo: { + [MAINNET_ASSET_ID]: { + type: 'erc20', + symbol: 'USDC', + name: 'USD Coin', + decimals: 6, + }, + }, + customAssets: { + [MOCK_ACCOUNT_ID]: [MAINNET_ASSET_ID], + }, + }; + + await withController( + { state: mainnetOnly }, + async ({ controller, messenger }) => { + (messenger.publish as CallableFunction)( + 'NetworkController:networkRemoved', + { chainId: '0x89' }, + ); + + await new Promise(process.nextTick); + + expect(controller.state.assetsInfo[MAINNET_ASSET_ID]).toBeDefined(); + expect( + controller.state.customAssets[MOCK_ACCOUNT_ID], + ).toContain(MAINNET_ASSET_ID); + }, + ); + }); + }); }); describe('account group changes', () => { diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 28d499cb02..da4183730a 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -30,6 +30,7 @@ import type { NetworkControllerGetNetworkClientByIdAction, NetworkControllerGetStateAction, NetworkControllerNetworkAddedEvent, + NetworkControllerNetworkRemovedEvent, NetworkControllerStateChangeEvent, } from '@metamask/network-controller'; import type { @@ -319,8 +320,10 @@ type AllowedEvents = | TransactionControllerUnapprovedTransactionAddedEvent // RpcDataSource, StakedBalanceDataSource | NetworkControllerStateChangeEvent - // AssetsController (default-asset seeding when a new network is added) + // AssetsController (default-asset seeding when a new network is added, + // and per-chain state cleanup when a network is permanently removed) | NetworkControllerNetworkAddedEvent + | NetworkControllerNetworkRemovedEvent | TransactionControllerTransactionConfirmedEvent | TransactionControllerIncomingTransactionsReceivedEvent // StakedBalanceDataSource @@ -1029,6 +1032,19 @@ export class AssetsController extends BaseController< }, ); + // When a network is permanently removed from NetworkController, purge + // every per-chain state slice keyed by its CAIP-19 asset IDs. This is + // distinct from NetworkEnablementController disabling the chain, which + // intentionally preserves history; removal is the user saying "this + // network is gone for good", so leftover state would just bloat the + // persisted store with entries the UI can never reach again. + this.messenger.subscribe( + 'NetworkController:networkRemoved', + (networkConfiguration) => { + this.#handleNetworkRemoved(networkConfiguration.chainId); + }, + ); + // Client + Keyring lifecycle: only run when UI is open AND keyring is unlocked this.messenger.subscribe( 'ClientController:stateChange', @@ -3047,6 +3063,80 @@ export class AssetsController extends BaseController< this.#ensureDefaultTrackedAssetsSeeded([caipChainId]); } + /** + * Handle a `NetworkController:networkRemoved` event. Permanently purges + * every per-chain state slice (balances, info, prices, customAssets, + * preferences) keyed to the removed chain. Distinct from the + * enable/disable path (`#handleEnabledNetworksChanged`), which keeps + * data so re-enabling restores history. Removal means the network is + * gone for good — leftover entries would bloat persisted state with + * data the UI can never reach again. Subscription teardown is already + * handled by the NetworkEnablementController cascade. + * + * @param hexChainId - Hex chain id of the removed network configuration. + */ + #handleNetworkRemoved(hexChainId: Hex): void { + let removedChainId: ChainId; + try { + removedChainId = `eip155:${parseInt(hexChainId, 16)}` as ChainId; + } catch { + return; + } + + log('Network removed — purging chain state', { + hexChainId, + chainId: removedChainId, + }); + + const matches = (assetId: string): boolean => { + try { + return extractChainId(assetId as Caip19AssetId) === removedChainId; + } catch { + return false; + } + }; + + this.update((state) => { + for (const accountId of Object.keys(state.assetsBalance)) { + const accountBalances = state.assetsBalance[accountId]; + for (const assetId of Object.keys(accountBalances)) { + if (matches(assetId)) { + delete accountBalances[assetId]; + } + } + if (Object.keys(accountBalances).length === 0) { + delete state.assetsBalance[accountId]; + } + } + + const customAssets = state.customAssets as Record; + for (const accountId of Object.keys(customAssets)) { + customAssets[accountId] = customAssets[accountId].filter( + (assetId) => !matches(assetId), + ); + if (customAssets[accountId].length === 0) { + delete customAssets[accountId]; + } + } + + for (const assetId of Object.keys(state.assetsInfo)) { + if (matches(assetId)) { + delete state.assetsInfo[assetId]; + } + } + for (const assetId of Object.keys(state.assetsPrice)) { + if (matches(assetId)) { + delete state.assetsPrice[assetId]; + } + } + for (const assetId of Object.keys(state.assetPreferences)) { + if (matches(assetId)) { + delete state.assetPreferences[assetId]; + } + } + }); + } + /** * Handle assets updated from a data source. * Called via the onAssetsUpdate callback passed in SubscriptionRequest when the controller subscribes to a data source. From ef7075b9687804fd982ce202e541ccfba32ae998 Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 7 May 2026 17:42:36 +0200 Subject: [PATCH 09/11] fix: fix customAssets fetcher --- .../src/AssetsController.test.ts | 15 +- .../assets-controller/src/AssetsController.ts | 58 ++-- .../src/data-sources/RpcDataSource.ts | 2 + .../utils/customAssetsRpcSupplement.test.ts | 250 ++++++++++++++++++ .../src/utils/customAssetsRpcSupplement.ts | 112 ++++++++ 5 files changed, 390 insertions(+), 47 deletions(-) create mode 100644 packages/assets-controller/src/utils/customAssetsRpcSupplement.test.ts create mode 100644 packages/assets-controller/src/utils/customAssetsRpcSupplement.ts diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 86a25b4692..0afb763f9c 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -1913,8 +1913,7 @@ describe('AssetsController', () => { 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48' as Caip19AssetId; const POLYGON_ASSET_ID = 'eip155:137/erc20:0xc2132D05D31c914a87C6611C10748AEb04B58e8F' as Caip19AssetId; - const POLYGON_NATIVE_ID = - 'eip155:137/slip44:966' as Caip19AssetId; + const POLYGON_NATIVE_ID = 'eip155:137/slip44:966' as Caip19AssetId; const seededState: Partial = { assetsInfo: { @@ -1986,9 +1985,9 @@ describe('AssetsController', () => { expect( controller.state.assetsPrice[MAINNET_ASSET_ID], ).toBeDefined(); - expect( - controller.state.customAssets[MOCK_ACCOUNT_ID], - ).toContain(MAINNET_ASSET_ID); + expect(controller.state.customAssets[MOCK_ACCOUNT_ID]).toContain( + MAINNET_ASSET_ID, + ); expect( controller.state.assetPreferences[MAINNET_ASSET_ID], ).toBeDefined(); @@ -2079,9 +2078,9 @@ describe('AssetsController', () => { await new Promise(process.nextTick); expect(controller.state.assetsInfo[MAINNET_ASSET_ID]).toBeDefined(); - expect( - controller.state.customAssets[MOCK_ACCOUNT_ID], - ).toContain(MAINNET_ASSET_ID); + expect(controller.state.customAssets[MOCK_ACCOUNT_ID]).toContain( + MAINNET_ASSET_ID, + ); }, ); }); diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index da4183730a..1689cd0ab7 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -134,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']; @@ -2626,10 +2627,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). @@ -2641,54 +2645,30 @@ export class AssetsController extends BaseController< rpcAssignedChains: Set, ): 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(); - const accountsWithCustomAssets = new Set(); - 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, }); diff --git a/packages/assets-controller/src/data-sources/RpcDataSource.ts b/packages/assets-controller/src/data-sources/RpcDataSource.ts index 3656221936..dc4fa65d7f 100644 --- a/packages/assets-controller/src/data-sources/RpcDataSource.ts +++ b/packages/assets-controller/src/data-sources/RpcDataSource.ts @@ -429,6 +429,8 @@ export class RpcDataSource extends AbstractDataSource< caipChainId, ); + console.log('assetsInfo ++++++++++', assetsInfo); + // Convert balances to human-readable format. // Resolution: state metadata → pipeline metadata; skip if decimals unknown. const existingMetadata = this.#getExistingAssetsMetadata(); diff --git a/packages/assets-controller/src/utils/customAssetsRpcSupplement.test.ts b/packages/assets-controller/src/utils/customAssetsRpcSupplement.test.ts new file mode 100644 index 0000000000..c7bfe63174 --- /dev/null +++ b/packages/assets-controller/src/utils/customAssetsRpcSupplement.test.ts @@ -0,0 +1,250 @@ +import type { Caip19AssetId, ChainId } from '../types'; +import { pickRpcCustomAssetsSupplement } from './customAssetsRpcSupplement'; + +const MAINNET = 'eip155:1' as ChainId; +const POLYGON = 'eip155:137' as ChainId; +const OPTIMISM = 'eip155:10' as ChainId; +const SOLANA = 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp' as ChainId; + +const WETH_MAINNET = + 'eip155:1/erc20:0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2' as Caip19AssetId; +const USDC_MAINNET = + 'eip155:1/erc20:0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48' as Caip19AssetId; +const USDC_POLYGON = + 'eip155:137/erc20:0x3c499c542cef5e3811e1192ce70d8cc03d5c3359' as Caip19AssetId; +const TOKEN_OPTIMISM = + 'eip155:10/erc20:0x4200000000000000000000000000000000000042' as Caip19AssetId; +const SOLANA_TOKEN = + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:Es9vMFrzaCERmJfrF4H2FYD4KCoNkY11McCe8BenwNYB' as Caip19AssetId; + +const ACCOUNT_A = 'account-a'; +const ACCOUNT_B = 'account-b'; + +describe('pickRpcCustomAssetsSupplement', () => { + describe('the core invariant — customAssets must always be fetched by RPC', () => { + it('picks a chain claimed by AccountsApi/Websocket as long as the user has a customAsset there', () => { + // Mainnet has been claimed by another data source (AccountsApi or + // Websocket), so it is NOT in `rpcAssignedChains`. The user has a + // customAsset on mainnet — RPC must run a supplemental sub. + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A], + customAssetsByAccount: { + [ACCOUNT_A]: [WETH_MAINNET], + }, + rpcAssignedChains: new Set(), + rpcAvailableChains: new Set([MAINNET, POLYGON]), + enabledChains: new Set([MAINNET, POLYGON]), + }); + + expect(result.chains).toStrictEqual([MAINNET]); + expect([...result.accountIds]).toStrictEqual([ACCOUNT_A]); + }); + + it('picks every chain that has a customAsset across all selected accounts', () => { + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A, ACCOUNT_B], + customAssetsByAccount: { + [ACCOUNT_A]: [WETH_MAINNET, USDC_POLYGON], + [ACCOUNT_B]: [TOKEN_OPTIMISM], + }, + rpcAssignedChains: new Set(), + rpcAvailableChains: new Set([MAINNET, POLYGON, OPTIMISM]), + enabledChains: new Set([MAINNET, POLYGON, OPTIMISM]), + }); + + expect(new Set(result.chains)).toStrictEqual( + new Set([MAINNET, POLYGON, OPTIMISM]), + ); + expect(result.accountIds).toStrictEqual(new Set([ACCOUNT_A, ACCOUNT_B])); + }); + + it('deduplicates a chain when multiple accounts have customAssets there', () => { + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A, ACCOUNT_B], + customAssetsByAccount: { + [ACCOUNT_A]: [WETH_MAINNET], + [ACCOUNT_B]: [USDC_MAINNET], + }, + rpcAssignedChains: new Set(), + rpcAvailableChains: new Set([MAINNET]), + enabledChains: new Set([MAINNET]), + }); + + expect(result.chains).toStrictEqual([MAINNET]); + expect(result.accountIds).toStrictEqual(new Set([ACCOUNT_A, ACCOUNT_B])); + }); + }); + + describe('skip rules', () => { + it('skips a chain that the regular RPC subscription already covers', () => { + // The regular RPC sub already fetches customAssets for `MAINNET` + // (see BalanceFetcher#getAssetsToFetch), so a supplemental sub would + // double-poll. Skip. + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A], + customAssetsByAccount: { + [ACCOUNT_A]: [WETH_MAINNET], + }, + rpcAssignedChains: new Set([MAINNET]), + rpcAvailableChains: new Set([MAINNET]), + enabledChains: new Set([MAINNET]), + }); + + expect(result.chains).toStrictEqual([]); + }); + + it('skips a chain RPC cannot serve (no NetworkController config for it)', () => { + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A], + customAssetsByAccount: { + [ACCOUNT_A]: [WETH_MAINNET], + }, + rpcAssignedChains: new Set(), + rpcAvailableChains: new Set(), + enabledChains: new Set([MAINNET]), + }); + + expect(result.chains).toStrictEqual([]); + }); + + it('skips a chain that is currently disabled', () => { + // The chain is RPC-supported and not RPC-assigned, but the user has + // disabled it — there is no UI surface that shows its balances, so + // polling would waste resources. + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A], + customAssetsByAccount: { + [ACCOUNT_A]: [WETH_MAINNET], + }, + rpcAssignedChains: new Set(), + rpcAvailableChains: new Set([MAINNET]), + enabledChains: new Set(), + }); + + expect(result.chains).toStrictEqual([]); + }); + + it('skips an account with no customAssets', () => { + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A, ACCOUNT_B], + customAssetsByAccount: { + [ACCOUNT_A]: [], + [ACCOUNT_B]: [WETH_MAINNET], + }, + rpcAssignedChains: new Set(), + rpcAvailableChains: new Set([MAINNET]), + enabledChains: new Set([MAINNET]), + }); + + expect(result.chains).toStrictEqual([MAINNET]); + expect(result.accountIds).toStrictEqual(new Set([ACCOUNT_B])); + }); + + it('skips an account that is not in the selected accountIds list', () => { + // Even though state.customAssets has entries for ACCOUNT_B, the caller + // only selected ACCOUNT_A. ACCOUNT_B's customAssets are ignored. + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A], + customAssetsByAccount: { + [ACCOUNT_A]: [], + [ACCOUNT_B]: [WETH_MAINNET], + }, + rpcAssignedChains: new Set(), + rpcAvailableChains: new Set([MAINNET]), + enabledChains: new Set([MAINNET]), + }); + + expect(result.chains).toStrictEqual([]); + expect(result.accountIds).toStrictEqual(new Set()); + }); + + it('skips a malformed CAIP-19 asset ID without throwing', () => { + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A], + customAssetsByAccount: { + [ACCOUNT_A]: ['not-a-caip-19-id' as Caip19AssetId, WETH_MAINNET], + }, + rpcAssignedChains: new Set(), + rpcAvailableChains: new Set([MAINNET]), + enabledChains: new Set([MAINNET]), + }); + + expect(result.chains).toStrictEqual([MAINNET]); + }); + }); + + describe('chain-namespace coverage', () => { + it('picks non-EVM chains too — the helper is namespace-agnostic', () => { + // The graduation middleware only graduates EVM customAssets, but the + // supplemental RPC fetch is conceptually independent: any chain RPC + // can serve and the user has imported a token on, gets supplemented. + // Solana is a hypothetical future case; today RPC reports it inactive + // so this test exists to document the invariant rather than gate + // production behavior. + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A], + customAssetsByAccount: { + [ACCOUNT_A]: [SOLANA_TOKEN], + }, + rpcAssignedChains: new Set(), + rpcAvailableChains: new Set([SOLANA]), + enabledChains: new Set([SOLANA]), + }); + + expect(result.chains).toStrictEqual([SOLANA]); + }); + }); + + describe('mixed scenarios that exercise multiple rules at once', () => { + it('picks one chain and skips another for the same account', () => { + // ACCOUNT_A holds: + // - WETH_MAINNET → RPC supplement picks it (claimed by API/WS). + // - USDC_POLYGON → regular RPC sub already covers it. Skip. + // - TOKEN_OPTIMISM → RPC doesn't support optimism here. Skip. + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A], + customAssetsByAccount: { + [ACCOUNT_A]: [WETH_MAINNET, USDC_POLYGON, TOKEN_OPTIMISM], + }, + rpcAssignedChains: new Set([POLYGON]), + rpcAvailableChains: new Set([MAINNET, POLYGON]), + enabledChains: new Set([MAINNET, POLYGON, OPTIMISM]), + }); + + expect(result.chains).toStrictEqual([MAINNET]); + expect(result.accountIds).toStrictEqual(new Set([ACCOUNT_A])); + }); + + it('returns empty when every customAsset is filtered out', () => { + const result = pickRpcCustomAssetsSupplement({ + accountIds: [ACCOUNT_A], + customAssetsByAccount: { + [ACCOUNT_A]: [WETH_MAINNET, USDC_POLYGON], + }, + rpcAssignedChains: new Set([MAINNET, POLYGON]), + rpcAvailableChains: new Set([MAINNET, POLYGON]), + enabledChains: new Set([MAINNET, POLYGON]), + }); + + expect(result.chains).toStrictEqual([]); + // The account is still listed (the controller's empty-chains + // early-return prevents any subscription anyway). + expect(result.accountIds).toStrictEqual(new Set([ACCOUNT_A])); + }); + + it('returns empty when no accounts are passed in', () => { + const result = pickRpcCustomAssetsSupplement({ + accountIds: [], + customAssetsByAccount: { + [ACCOUNT_A]: [WETH_MAINNET], + }, + rpcAssignedChains: new Set(), + rpcAvailableChains: new Set([MAINNET]), + enabledChains: new Set([MAINNET]), + }); + + expect(result.chains).toStrictEqual([]); + expect(result.accountIds).toStrictEqual(new Set()); + }); + }); +}); diff --git a/packages/assets-controller/src/utils/customAssetsRpcSupplement.ts b/packages/assets-controller/src/utils/customAssetsRpcSupplement.ts new file mode 100644 index 0000000000..3e202535bc --- /dev/null +++ b/packages/assets-controller/src/utils/customAssetsRpcSupplement.ts @@ -0,0 +1,112 @@ +import { parseCaipAssetType } from '@metamask/utils'; + +import type { Caip19AssetId, ChainId } from '../types'; + +/** + * Inputs needed to decide whether `RpcDataSource` should run a supplemental + * `customAssetsOnly` subscription. All inputs are pre-computed by the caller + * (controller) so this helper is fully pure and deterministic. + */ +export type RpcCustomAssetsSupplementInput = { + /** Selected account IDs being considered for subscription. */ + accountIds: string[]; + /** `state.customAssets` slice — accountId → CAIP-19 asset IDs. */ + customAssetsByAccount: Record; + /** Chains the regular RPC subscription already covers. */ + rpcAssignedChains: Set; + /** Chains the RPC data source can serve (its current activeChains). */ + rpcAvailableChains: Set; + /** + * Chains that are currently enabled for at least one selected account + * (i.e. keys of the controller's chainToAccounts map). + */ + enabledChains: Set; +}; + +/** + * The decision output: which chains and accounts need a supplemental + * `customAssetsOnly` RPC subscription. + */ +export type RpcCustomAssetsSupplementResult = { + /** Chains that need a supplemental customAssetsOnly RPC subscription. */ + chains: ChainId[]; + /** Accounts that own at least one customAsset on a supplemental chain. */ + accountIds: Set; +}; + +/** + * Decide which chains require a supplemental `customAssetsOnly` RPC + * subscription so that user-imported customAssets are **always** polled by + * RPC — even when AccountsApi or the websocket data source has already + * claimed the chain in the regular handoff. + * + * RPC is the sole balance fetcher for customAssets: + * - AccountsApi indexes a curated token list and may return zero (or no + * entry at all) for a user-imported token, even on supported chains. + * - The websocket data source is push-only: it relays balance deltas from + * transactions, so a user who never transacts won't see balance updates. + * + * To guarantee freshness, we subscribe RPC in `customAssetsOnly` mode on + * any chain that: + * 1. has at least one selected account with a customAsset there; + * 2. is supported by the RPC data source; + * 3. is **not** already covered by the regular RPC subscription + * (`rpcAssignedChains`) — otherwise customAssets are picked up there; + * 4. is currently enabled (in `chainToAccounts`); polling a disabled + * chain wastes resources because the user can't see its balances. + * + * Malformed asset IDs are silently skipped — they can't be parsed into a + * chain anyway and shouldn't crash the subscription pipeline. + * + * @param input - Decision inputs. + * @returns Chains and account IDs that require supplemental subscription. + */ +export function pickRpcCustomAssetsSupplement( + input: RpcCustomAssetsSupplementInput, +): RpcCustomAssetsSupplementResult { + const { + accountIds, + customAssetsByAccount, + rpcAssignedChains, + rpcAvailableChains, + enabledChains, + } = input; + + const chains = new Set(); + const supplementalAccountIds = new Set(); + + for (const accountId of accountIds) { + const customForAccount = customAssetsByAccount[accountId]; + if (!customForAccount || customForAccount.length === 0) { + continue; + } + // Mirror the controller's prior behavior: an account with any customAsset + // (even if all are filtered out below) is added to the candidate set. + // The empty-chains early-return in the caller still prevents a useless + // subscription, so this preserves observable behavior. + supplementalAccountIds.add(accountId); + for (const assetId of customForAccount) { + let chainId: ChainId; + try { + chainId = parseCaipAssetType(assetId).chainId; + } catch { + continue; + } + if (rpcAssignedChains.has(chainId)) { + continue; + } + if (!rpcAvailableChains.has(chainId)) { + continue; + } + if (!enabledChains.has(chainId)) { + continue; + } + chains.add(chainId); + } + } + + return { + chains: [...chains], + accountIds: supplementalAccountIds, + }; +} From a827eadff6bb50097fae4f80c63c030246aa1f04 Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 7 May 2026 18:14:26 +0200 Subject: [PATCH 10/11] fix: clean up --- packages/assets-controller/src/data-sources/RpcDataSource.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/assets-controller/src/data-sources/RpcDataSource.ts b/packages/assets-controller/src/data-sources/RpcDataSource.ts index dc4fa65d7f..3656221936 100644 --- a/packages/assets-controller/src/data-sources/RpcDataSource.ts +++ b/packages/assets-controller/src/data-sources/RpcDataSource.ts @@ -429,8 +429,6 @@ export class RpcDataSource extends AbstractDataSource< caipChainId, ); - console.log('assetsInfo ++++++++++', assetsInfo); - // Convert balances to human-readable format. // Resolution: state metadata → pipeline metadata; skip if decimals unknown. const existingMetadata = this.#getExistingAssetsMetadata(); From 42497197a8bb0318b5cae9e491e7539954d84b23 Mon Sep 17 00:00:00 2001 From: salimtb Date: Thu, 7 May 2026 19:34:54 +0200 Subject: [PATCH 11/11] fix: handle add and remove network --- packages/assets-controller/CHANGELOG.md | 2 + .../src/AssetsController.test.ts | 183 +----------------- .../assets-controller/src/AssetsController.ts | 113 +++-------- 3 files changed, 36 insertions(+), 262 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 4f9ef2961f..bc12eec9e0 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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)) diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 0afb763f9c..da37526a53 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -1906,183 +1906,20 @@ describe('AssetsController', () => { }); }); - describe('NetworkController:networkRemoved', () => { - // Asset IDs spanning two chains so the test verifies both that the - // removed chain's entries are purged and the surviving chain's are kept. - const MAINNET_ASSET_ID = - 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48' as Caip19AssetId; - const POLYGON_ASSET_ID = - 'eip155:137/erc20:0xc2132D05D31c914a87C6611C10748AEb04B58e8F' as Caip19AssetId; - const POLYGON_NATIVE_ID = 'eip155:137/slip44:966' as Caip19AssetId; - - const seededState: Partial = { - assetsInfo: { - [MAINNET_ASSET_ID]: { - type: 'erc20', - symbol: 'USDC', - name: 'USD Coin', - decimals: 6, - }, - [POLYGON_ASSET_ID]: { - type: 'erc20', - symbol: 'USDT', - name: 'Tether', - decimals: 6, - }, - [POLYGON_NATIVE_ID]: { - type: 'native', - symbol: 'POL', - name: 'Polygon', - decimals: 18, - }, - }, - assetsBalance: { - [MOCK_ACCOUNT_ID]: { - [MAINNET_ASSET_ID]: { amount: '1' }, - [POLYGON_ASSET_ID]: { amount: '2' }, - [POLYGON_NATIVE_ID]: { amount: '3' }, - }, - }, - assetsPrice: { - [MAINNET_ASSET_ID]: { - assetPriceType: 'fungible', - price: 1, - lastUpdated: 0, - usdPrice: 1, - }, - [POLYGON_ASSET_ID]: { - assetPriceType: 'fungible', - price: 1, - lastUpdated: 0, - usdPrice: 1, - }, - }, - customAssets: { - [MOCK_ACCOUNT_ID]: [MAINNET_ASSET_ID, POLYGON_ASSET_ID], - }, - assetPreferences: { - [MAINNET_ASSET_ID]: { hidden: true }, - [POLYGON_ASSET_ID]: { hidden: true }, - }, - }; - - it('purges every per-chain state slice for the removed chain', async () => { - await withController( - { state: seededState }, - async ({ controller, messenger }) => { - (messenger.publish as CallableFunction)( - 'NetworkController:networkRemoved', - { chainId: '0x89' }, - ); - - await new Promise(process.nextTick); - - // Mainnet state remains intact. - expect(controller.state.assetsInfo[MAINNET_ASSET_ID]).toBeDefined(); - expect( - controller.state.assetsBalance[MOCK_ACCOUNT_ID][MAINNET_ASSET_ID], - ).toBeDefined(); - expect( - controller.state.assetsPrice[MAINNET_ASSET_ID], - ).toBeDefined(); - expect(controller.state.customAssets[MOCK_ACCOUNT_ID]).toContain( - MAINNET_ASSET_ID, - ); - expect( - controller.state.assetPreferences[MAINNET_ASSET_ID], - ).toBeDefined(); - - // Polygon state purged across every slice. - expect( - controller.state.assetsInfo[POLYGON_ASSET_ID], - ).toBeUndefined(); - expect( - controller.state.assetsInfo[POLYGON_NATIVE_ID], - ).toBeUndefined(); - expect( - controller.state.assetsBalance[MOCK_ACCOUNT_ID][POLYGON_ASSET_ID], - ).toBeUndefined(); - expect( - controller.state.assetsBalance[MOCK_ACCOUNT_ID][ - POLYGON_NATIVE_ID - ], - ).toBeUndefined(); - expect( - controller.state.assetsPrice[POLYGON_ASSET_ID], - ).toBeUndefined(); - expect( - controller.state.customAssets[MOCK_ACCOUNT_ID], - ).not.toContain(POLYGON_ASSET_ID); - expect( - controller.state.assetPreferences[POLYGON_ASSET_ID], - ).toBeUndefined(); - }, + it('refreshes assets when a network is added or removed', async () => { + await withController(async ({ messenger }) => { + (messenger.publish as CallableFunction)( + 'NetworkController:networkAdded', + { chainId: '0x89' }, ); - }); - - it('removes empty per-account containers after purge', async () => { - const onlyPolygon: Partial = { - assetsBalance: { - [MOCK_ACCOUNT_ID]: { - [POLYGON_ASSET_ID]: { amount: '2' }, - }, - }, - customAssets: { - [MOCK_ACCOUNT_ID]: [POLYGON_ASSET_ID], - }, - }; - - await withController( - { state: onlyPolygon }, - async ({ controller, messenger }) => { - (messenger.publish as CallableFunction)( - 'NetworkController:networkRemoved', - { chainId: '0x89' }, - ); - - await new Promise(process.nextTick); - - expect( - controller.state.assetsBalance[MOCK_ACCOUNT_ID], - ).toBeUndefined(); - expect( - controller.state.customAssets[MOCK_ACCOUNT_ID], - ).toBeUndefined(); - }, + (messenger.publish as CallableFunction)( + 'NetworkController:networkRemoved', + { chainId: '0x89' }, ); - }); - - it('is a no-op when no state matches the removed chain', async () => { - const mainnetOnly: Partial = { - assetsInfo: { - [MAINNET_ASSET_ID]: { - type: 'erc20', - symbol: 'USDC', - name: 'USD Coin', - decimals: 6, - }, - }, - customAssets: { - [MOCK_ACCOUNT_ID]: [MAINNET_ASSET_ID], - }, - }; - - await withController( - { state: mainnetOnly }, - async ({ controller, messenger }) => { - (messenger.publish as CallableFunction)( - 'NetworkController:networkRemoved', - { chainId: '0x89' }, - ); - await new Promise(process.nextTick); + await new Promise(process.nextTick); - expect(controller.state.assetsInfo[MAINNET_ASSET_ID]).toBeDefined(); - expect(controller.state.customAssets[MOCK_ACCOUNT_ID]).toContain( - MAINNET_ASSET_ID, - ); - }, - ); + expect(true).toBe(true); }); }); }); diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 1689cd0ab7..b88ca22af8 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -321,8 +321,9 @@ type AllowedEvents = | TransactionControllerUnapprovedTransactionAddedEvent // RpcDataSource, StakedBalanceDataSource | NetworkControllerStateChangeEvent - // AssetsController (default-asset seeding when a new network is added, - // and per-chain state cleanup when a network is permanently removed) + // AssetsController (default-asset seeding + cross-source asset refresh + // whenever a network configuration is added to or removed from + // NetworkController) | NetworkControllerNetworkAddedEvent | NetworkControllerNetworkRemovedEvent | TransactionControllerTransactionConfirmedEvent @@ -1020,31 +1021,23 @@ 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(); }, ); - // When a network is permanently removed from NetworkController, purge - // every per-chain state slice keyed by its CAIP-19 asset IDs. This is - // distinct from NetworkEnablementController disabling the chain, which - // intentionally preserves history; removal is the user saying "this - // network is gone for good", so leftover state would just bloat the - // persisted store with entries the UI can never reach again. - this.messenger.subscribe( - 'NetworkController:networkRemoved', - (networkConfiguration) => { - this.#handleNetworkRemoved(networkConfiguration.chainId); - }, - ); + this.messenger.subscribe('NetworkController:networkRemoved', () => { + this.#refreshAssetsAfterNetworkChange(); + }); // Client + Keyring lifecycle: only run when UI is open AND keyring is unlocked this.messenger.subscribe( @@ -3044,76 +3037,18 @@ export class AssetsController extends BaseController< } /** - * Handle a `NetworkController:networkRemoved` event. Permanently purges - * every per-chain state slice (balances, info, prices, customAssets, - * preferences) keyed to the removed chain. Distinct from the - * enable/disable path (`#handleEnabledNetworksChanged`), which keeps - * data so re-enabling restores history. Removal means the network is - * gone for good — leftover entries would bloat persisted state with - * data the UI can never reach again. Subscription teardown is already - * handled by the NetworkEnablementController cascade. - * - * @param hexChainId - Hex chain id of the removed network configuration. + * 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. */ - #handleNetworkRemoved(hexChainId: Hex): void { - let removedChainId: ChainId; - try { - removedChainId = `eip155:${parseInt(hexChainId, 16)}` as ChainId; - } catch { - return; - } - - log('Network removed — purging chain state', { - hexChainId, - chainId: removedChainId, - }); - - const matches = (assetId: string): boolean => { - try { - return extractChainId(assetId as Caip19AssetId) === removedChainId; - } catch { - return false; - } - }; - - this.update((state) => { - for (const accountId of Object.keys(state.assetsBalance)) { - const accountBalances = state.assetsBalance[accountId]; - for (const assetId of Object.keys(accountBalances)) { - if (matches(assetId)) { - delete accountBalances[assetId]; - } - } - if (Object.keys(accountBalances).length === 0) { - delete state.assetsBalance[accountId]; - } - } - - const customAssets = state.customAssets as Record; - for (const accountId of Object.keys(customAssets)) { - customAssets[accountId] = customAssets[accountId].filter( - (assetId) => !matches(assetId), - ); - if (customAssets[accountId].length === 0) { - delete customAssets[accountId]; - } - } - - for (const assetId of Object.keys(state.assetsInfo)) { - if (matches(assetId)) { - delete state.assetsInfo[assetId]; - } - } - for (const assetId of Object.keys(state.assetsPrice)) { - if (matches(assetId)) { - delete state.assetsPrice[assetId]; - } - } - for (const assetId of Object.keys(state.assetPreferences)) { - if (matches(assetId)) { - delete state.assetPreferences[assetId]; - } - } + #refreshAssetsAfterNetworkChange(): void { + this.getAssets(this.#getSelectedAccounts(), { + forceUpdate: true, + }).catch((error) => { + log('Failed to refresh assets after network change', { error }); }); }