Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/assets-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
62 changes: 62 additions & 0 deletions packages/assets-controller/src/AssetsController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 2 additions & 0 deletions packages/assets-controller/src/AssetsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down
262 changes: 262 additions & 0 deletions packages/assets-controller/src/utils/cleanupUnusedMetadata.test.ts
Original file line number Diff line number Diff line change
@@ -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> = {},
): 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<AssetsControllerState>;
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());
});
});
42 changes: 42 additions & 0 deletions packages/assets-controller/src/utils/cleanupUnusedMetadata.ts
Comment thread
Prithpal-Sooriya marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -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<string, unknown>;

export type CleanupUnusedMetadataState = {
assetsInfo: AssetIdKeyedRecord;
assetsBalance: Record<string, AssetIdKeyedRecord>;
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]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
for (const slice of [state.assetsInfo, state.assetsPrice]) {
for (const assetId of Object.keys(slice)) {
if (isUnused(assetId)) {
delete slice[assetId];
}
}

for (const assetId of Object.keys(slice).filter(isUnused)) {
delete slice[assetId];
}
}
}
Loading