diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 5c4942dea43..e8c3ed9ee8d 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Preserve pooled-staking balances across Accounts API chain-slice updates (e.g. network switch / `replaceCoveredChainBalances`): exclude staking contract asset IDs from `AccountsApiDataSource` v5/v6 balance processing, and keep prior staked amounts when a merge replace omits them so Accounts API cannot reset staked ETH to missing/0 ([#9753](https://github.com/MetaMask/core/pull/9753)) +- Clean up unused `assetsInfo` and `assetsPrice` entries after a successful startup refresh so those persisted state slices no longer grow unbounded ([#9806](https://github.com/MetaMask/core/pull/9806)) ## [13.1.1] diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 075b220955f..a5f8fc8e635 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -2917,6 +2917,68 @@ describe('AssetsController', () => { }); }); + it('cleans up unused assetsInfo and assetsPrice entries after the startup refresh', async () => { + const unreferencedAssetId = + 'eip155:1/erc20:0x6B175474E89094C44Da98b954EedeAC495271d0F' as Caip19AssetId; + const zeroBalanceAssetId = + 'eip155:1/erc20:0xdAC17F958D2ee523a2206206994597C13D831ec7' as Caip19AssetId; + // Default tracked asset on a chain that is NOT enabled in this test + // (only eip155:1 is enabled): metadata is pre-seeded but no balance + // entry exists, and it must survive the cleanup. + const musdOnMonadAssetId = + 'eip155:143/erc20:0xacA92E438df0B2401fF60dA7E4337B687a2435DA' as Caip19AssetId; + + await withController( + { + clientControllerState: { isUiOpen: true }, + state: { + assetsInfo: { + ...buildDefaultAssetsInfo(), + [unreferencedAssetId]: { + type: 'erc20', + symbol: 'DAI', + name: 'Dai Stablecoin', + decimals: 18, + }, + [zeroBalanceAssetId]: { + type: 'erc20', + symbol: 'USDT', + name: 'Tether USD', + decimals: 6, + }, + }, + assetsPrice: { + [unreferencedAssetId]: { + assetPriceType: 'fungible', + price: 1, + usdPrice: 1, + lastUpdated: 0, + }, + }, + assetsBalance: { + [MOCK_ACCOUNT_ID]: { [zeroBalanceAssetId]: { amount: '0' } }, + }, + }, + }, + async ({ controller, messenger }) => { + expect( + controller.state.assetsInfo[unreferencedAssetId], + ).toBeDefined(); + + await activateTracking(messenger); + + expect( + controller.state.assetsInfo[unreferencedAssetId], + ).toBeUndefined(); + expect( + controller.state.assetsPrice[unreferencedAssetId], + ).toBeUndefined(); + expect(controller.state.assetsInfo[zeroBalanceAssetId]).toBeDefined(); + expect(controller.state.assetsInfo[musdOnMonadAssetId]).toBeDefined(); + }, + ); + }); + it('stops tracking on keyring lock', async () => { await withController(async ({ messenger }) => { messenger.publish('KeyringController:unlock'); diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index cee1e2daa56..d89f678e5b9 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -139,6 +139,7 @@ import type { SubscriptionResponse, Asset, } from './types.js'; +import { cleanupUnusedMetadata } from './utils/cleanupUnusedMetadata.js'; import { ZERO_ADDRESS } from './utils/constants.js'; import { pickRpcCustomAssetsSupplement } from './utils/customAssetsRpcSupplement.js'; import { @@ -1365,6 +1366,7 @@ export class AssetsController extends BaseController< this.#ensureDefaultTrackedAssetsSeeded(); this.#subscribeAssets(); this.#fetchMissingPricesWithoutCache(accounts, [...this.#enabledChains]); + this.update((state) => cleanupUnusedMetadata(state)); } catch (error) { log('Failed to fetch assets on startup', error); this.#ensureNativeBalancesDefaultZero(); diff --git a/packages/assets-controller/src/utils/cleanupUnusedMetadata.test.ts b/packages/assets-controller/src/utils/cleanupUnusedMetadata.test.ts new file mode 100644 index 00000000000..46185dfd264 --- /dev/null +++ b/packages/assets-controller/src/utils/cleanupUnusedMetadata.test.ts @@ -0,0 +1,262 @@ +import type { AssetsControllerState } from '../AssetsController.js'; +import type { AssetMetadata, AssetPrice, Caip19AssetId } from '../types.js'; +import { cleanupUnusedMetadata } from './cleanupUnusedMetadata.js'; + +const SELECTED_ACCOUNT = 'account-1'; +const OTHER_ACCOUNT = 'account-2'; + +/** USDC on mainnet (checksummed). */ +const HELD_ASSET = 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48'; +/** DAI on mainnet. */ +const UNREFERENCED_ASSET = + 'eip155:1/erc20:0x6B175474E89094C44Da98b954EedeAC495271d0F'; +/** USDT on mainnet. */ +const ZERO_BALANCE_ASSET = + 'eip155:1/erc20:0xdAC17F958D2ee523a2206206994597C13D831ec7'; +/** cbETH on Base. */ +const CUSTOM_ASSET = + 'eip155:8453/erc20:0x2Ae3F1Ec7F1F5012CFEab0185bfc7aa3cf0DEc22'; +/** ETH on mainnet. */ +const NATIVE_SLIP44_ASSET = 'eip155:1/slip44:60'; +/** SOL — slip44 native on a chain outside the hardcoded (EVM-only) registry. */ +const NATIVE_SOLANA_ASSET = + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'; +/** Zero-address ERC-20 native convention on a chain with no registry entry. */ +const NATIVE_ZERO_ADDRESS_ASSET = + 'eip155:424242/erc20:0x0000000000000000000000000000000000000000'; +/** METIS — native only recognizable through the hardcoded registry. */ +const NATIVE_REGISTRY_ASSET = + 'eip155:1088/erc20:0xdeaddeaddeaddeaddeaddeaddeaddeaddead0000'; +/** mUSD on Monad — default tracked, has metadata but no balance until the chain is enabled. */ +const MUSD_ON_MONAD_ASSET = + 'eip155:143/erc20:0xacA92E438df0B2401fF60dA7E4337B687a2435DA'; +/** BAYC #1234 — NFT asset IDs carry a tokenId suffix. */ +const NFT_ASSET = + 'eip155:1/erc721:0xBC4CA0EdA7647A8aB7C2061c2E118A18a936f13D/1234'; + +function buildMetadata(symbol: string): AssetMetadata { + return { type: 'erc20', symbol, name: symbol, decimals: 18 }; +} + +function buildPrice(value: number): AssetPrice { + return { + assetPriceType: 'fungible', + price: value, + usdPrice: value, + lastUpdated: 1700000000000, + }; +} + +function buildState( + overrides: Partial = {}, +): AssetsControllerState { + return { + assetsInfo: {}, + assetsBalance: {}, + assetsPrice: {}, + customAssets: {}, + assetPreferences: {}, + selectedCurrency: 'usd', + ...overrides, + }; +} + +type CleanupCase = { + description: string; + assetId: string; + /** State slices referencing the asset (none means unreferenced). */ + references?: Partial; + expectKept: boolean; +}; + +const cleanupCases: CleanupCase[] = [ + { + description: 'removes an asset that nothing references', + assetId: UNREFERENCED_ASSET, + expectKept: false, + }, + { + description: 'removes a malformed asset ID that nothing references', + assetId: 'not-a-caip-id', + expectKept: false, + }, + { + description: 'removes an unreferenced NFT asset ID', + assetId: NFT_ASSET, + expectKept: false, + }, + { + description: 'keeps an asset with a non-zero balance', + assetId: HELD_ASSET, + references: { + assetsBalance: { [SELECTED_ACCOUNT]: { [HELD_ASSET]: { amount: '5' } } }, + }, + expectKept: true, + }, + { + description: 'keeps an asset whose only balance entry is a zero amount', + assetId: ZERO_BALANCE_ASSET, + references: { + assetsBalance: { + [SELECTED_ACCOUNT]: { [ZERO_BALANCE_ASSET]: { amount: '0' } }, + }, + }, + expectKept: true, + }, + { + description: 'keeps an asset held only by a non-selected account', + assetId: HELD_ASSET, + references: { + assetsBalance: { + [SELECTED_ACCOUNT]: {}, + [OTHER_ACCOUNT]: { [HELD_ASSET]: { amount: '42' } }, + }, + }, + expectKept: true, + }, + { + description: 'keeps an asset that is only referenced by customAssets', + assetId: CUSTOM_ASSET, + references: { customAssets: { [SELECTED_ACCOUNT]: [CUSTOM_ASSET] } }, + expectKept: true, + }, + { + description: 'keeps an asset whose balance key differs in casing', + assetId: HELD_ASSET, + references: { + assetsBalance: { + [SELECTED_ACCOUNT]: { [HELD_ASSET.toLowerCase()]: { amount: '1' } }, + }, + }, + expectKept: true, + }, + { + description: 'keeps an asset whose customAssets entry differs in casing', + assetId: CUSTOM_ASSET, + references: { + customAssets: { + [SELECTED_ACCOUNT]: [CUSTOM_ASSET.toLowerCase() as Caip19AssetId], + }, + }, + expectKept: true, + }, + { + description: 'keeps a malformed asset ID that a balance entry references', + assetId: 'not-a-caip-id', + references: { + assetsBalance: { + [SELECTED_ACCOUNT]: { 'not-a-caip-id': { amount: '1' } }, + }, + }, + expectKept: true, + }, + { + description: 'keeps a slip44 native asset', + assetId: NATIVE_SLIP44_ASSET, + expectKept: true, + }, + { + description: 'keeps a slip44 native on a chain outside the native registry', + assetId: NATIVE_SOLANA_ASSET, + expectKept: true, + }, + { + description: 'keeps a zero-address ERC-20 native', + assetId: NATIVE_ZERO_ADDRESS_ASSET, + expectKept: true, + }, + { + description: 'keeps a registry-only native (METIS dead address)', + assetId: NATIVE_REGISTRY_ASSET, + expectKept: true, + }, + { + description: + 'keeps a default tracked asset with no balance (mUSD on a disabled chain)', + assetId: MUSD_ON_MONAD_ASSET, + expectKept: true, + }, +]; + +describe('cleanupUnusedMetadata', () => { + it.each(cleanupCases)( + '$description', + ({ assetId, references = {}, expectKept }) => { + const state = buildState({ + assetsInfo: { [assetId]: buildMetadata('TEST') }, + assetsPrice: { [assetId]: buildPrice(1) }, + ...references, + }); + + cleanupUnusedMetadata(state); + + expect(state.assetsInfo).toStrictEqual( + expectKept ? { [assetId]: buildMetadata('TEST') } : {}, + ); + expect(state.assetsPrice).toStrictEqual( + expectKept ? { [assetId]: buildPrice(1) } : {}, + ); + }, + ); + + it('removes only unreferenced entries, leaving referenced ones in place', () => { + const state = buildState({ + assetsInfo: { + [HELD_ASSET]: buildMetadata('USDC'), + [UNREFERENCED_ASSET]: buildMetadata('DAI'), + }, + assetsPrice: { + [HELD_ASSET]: buildPrice(1), + [UNREFERENCED_ASSET]: buildPrice(1), + }, + assetsBalance: { + [SELECTED_ACCOUNT]: { [HELD_ASSET]: { amount: '5000000' } }, + }, + }); + + cleanupUnusedMetadata(state); + + expect(state.assetsInfo).toStrictEqual({ + [HELD_ASSET]: buildMetadata('USDC'), + }); + expect(state.assetsPrice).toStrictEqual({ [HELD_ASSET]: buildPrice(1) }); + }); + + it('removes an unreferenced price entry even when the asset has no assetsInfo entry', () => { + const state = buildState({ + assetsPrice: { [UNREFERENCED_ASSET]: buildPrice(1) }, + }); + + cleanupUnusedMetadata(state); + + expect(state.assetsPrice).toStrictEqual({}); + }); + + it('leaves assetPreferences untouched, including entries for removed assets', () => { + const state = buildState({ + assetsInfo: { [UNREFERENCED_ASSET]: buildMetadata('DAI') }, + assetsPrice: { [UNREFERENCED_ASSET]: buildPrice(1) }, + assetPreferences: { + [UNREFERENCED_ASSET]: { hidden: true }, + [HELD_ASSET]: { hidden: false }, + }, + }); + + cleanupUnusedMetadata(state); + + expect(state.assetsInfo).toStrictEqual({}); + expect(state.assetsPrice).toStrictEqual({}); + expect(state.assetPreferences).toStrictEqual({ + [UNREFERENCED_ASSET]: { hidden: true }, + [HELD_ASSET]: { hidden: false }, + }); + }); + + it('does nothing on empty state', () => { + const state = buildState(); + + cleanupUnusedMetadata(state); + + expect(state).toStrictEqual(buildState()); + }); +}); diff --git a/packages/assets-controller/src/utils/cleanupUnusedMetadata.ts b/packages/assets-controller/src/utils/cleanupUnusedMetadata.ts new file mode 100644 index 00000000000..85fb2c9187e --- /dev/null +++ b/packages/assets-controller/src/utils/cleanupUnusedMetadata.ts @@ -0,0 +1,42 @@ +import { DEFAULT_TRACKED_ASSETS_BY_CHAIN } from '../defaults.js'; +import type { AssetsControllerStateInternal } from '../types.js'; +import { isNativeAssetId } from './native-assets.js'; + +type AssetIdKeyedRecord = Record; + +export type CleanupUnusedMetadataState = { + assetsInfo: AssetIdKeyedRecord; + assetsBalance: Record; + assetsPrice: AssetIdKeyedRecord; + customAssets: AssetsControllerStateInternal['customAssets']; +}; + +/** + * Delete `assetsInfo` / `assetsPrice` entries for assets that are not held, + * custom, default tracked, or native. + * + * @param state - The controller state to clean up (mutated in place). + */ +export function cleanupUnusedMetadata(state: CleanupUnusedMetadataState): void { + const defaultTrackedAssetIds = [ + ...DEFAULT_TRACKED_ASSETS_BY_CHAIN.values(), + ].flat(); + const heldAssetIds = Object.values(state.assetsBalance).flatMap( + (accountBalances) => Object.keys(accountBalances), + ); + const customAssetIds = Object.values(state.customAssets).flat(); + + const keptAssetIds = new Set( + [...defaultTrackedAssetIds, ...heldAssetIds, ...customAssetIds].map( + (assetId) => assetId.toLowerCase(), + ), + ); + const isUnused = (assetId: string): boolean => + !keptAssetIds.has(assetId.toLowerCase()) && !isNativeAssetId(assetId); + + for (const slice of [state.assetsInfo, state.assetsPrice]) { + for (const assetId of Object.keys(slice).filter(isUnused)) { + delete slice[assetId]; + } + } +} diff --git a/packages/assets-controller/src/utils/native-assets.test.ts b/packages/assets-controller/src/utils/native-assets.test.ts index b82cedfdeb9..44eb3a8011d 100644 --- a/packages/assets-controller/src/utils/native-assets.test.ts +++ b/packages/assets-controller/src/utils/native-assets.test.ts @@ -4,6 +4,7 @@ import { fetchWithErrorHandling } from '@metamask/controller-utils'; import { buildNativeAssetsFromConstant, buildNativeAssetsFromApi, + isNativeAssetId, } from './native-assets.js'; import { normalizeAssetId } from './normalizeAssetId.js'; @@ -25,6 +26,54 @@ describe('buildNativeAssetsFromConstant', () => { }); }); +describe('isNativeAssetId', () => { + it.each([ + { + description: 'a slip44 native on a chain in the hardcoded registry', + assetId: 'eip155:1/slip44:60', + expected: true, + }, + { + description: 'a slip44 native on a chain outside the hardcoded registry', + assetId: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + expected: true, + }, + { + description: 'a zero-address ERC-20 native on an unregistered chain', + assetId: 'eip155:424242/erc20:0x0000000000000000000000000000000000000000', + expected: true, + }, + { + description: 'a registry-only native (METIS dead address)', + assetId: 'eip155:1088/erc20:0xdeaddeaddeaddeaddeaddeaddeaddeaddead0000', + expected: true, + }, + { + description: 'a registry-only native in a different casing', + assetId: 'eip155:1088/erc20:0xDEADDEADDEADDEADDEADDEADDEADDEADDEAD0000', + expected: true, + }, + { + description: 'a regular ERC-20 token', + assetId: 'eip155:1/erc20:0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', + expected: false, + }, + { + description: 'a malformed asset ID', + assetId: 'not-a-caip-id', + expected: false, + }, + { + description: 'an NFT asset ID with a token ID', + assetId: + 'eip155:1/erc721:0xBC4CA0EdA7647A8aB7C2061c2E118A18a936f13D/1234', + expected: false, + }, + ])('returns $expected for $description', ({ assetId, expected }) => { + expect(isNativeAssetId(assetId)).toBe(expected); + }); +}); + describe('buildNativeAssetsFromApi', () => { beforeEach(() => { fetchWithErrorHandlingMock.mockReset(); diff --git a/packages/assets-controller/src/utils/native-assets.ts b/packages/assets-controller/src/utils/native-assets.ts index 5cf251974aa..47347e78a4f 100644 --- a/packages/assets-controller/src/utils/native-assets.ts +++ b/packages/assets-controller/src/utils/native-assets.ts @@ -1,8 +1,9 @@ import { SPOT_PRICES_SUPPORT_INFO } from '@metamask/assets-controllers'; import { fetchWithErrorHandling } from '@metamask/controller-utils'; -import { parseCaipAssetType } from '@metamask/utils'; +import { isCaipAssetType, parseCaipAssetType } from '@metamask/utils'; import type { Caip19AssetId, ChainId } from '../types.js'; +import { ZERO_ADDRESS } from './constants.js'; import { normalizeAssetId } from './normalizeAssetId.js'; const CHAINID_NETWORK_URL = 'https://chainid.network/chains.json'; @@ -29,6 +30,37 @@ export function buildNativeAssetsFromConstant(): Record< return nativeAssetsMap; } +const KNOWN_NATIVE_ASSET_IDS: ReadonlySet = new Set( + Object.values(buildNativeAssetsFromConstant()).map((assetId) => + assetId.toLowerCase(), + ), +); + +/** + * Whether a CAIP-19 asset ID represents a chain's native asset — via the + * `slip44` namespace, the zero-address ERC-20 convention, or the hardcoded + * native asset registry (non-standard natives like METIS/MNT). + * + * @param assetId - The asset ID to check (any casing); malformed IDs are + * reported as non-native. + * @returns True when the asset ID is a native asset. + */ +export function isNativeAssetId(assetId: string): boolean { + if (KNOWN_NATIVE_ASSET_IDS.has(assetId.toLowerCase())) { + return true; + } + if (!isCaipAssetType(assetId)) { + return false; + } + + const { assetNamespace, assetReference } = parseCaipAssetType(assetId); + return ( + assetNamespace === 'slip44' || + (assetNamespace === 'erc20' && + assetReference.toLowerCase() === ZERO_ADDRESS) + ); +} + /** * Fetches chain data from chainid.network and merges it with the seed * native asset map built from {@link buildNativeAssetsFromConstant}.