diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 45acf6cd067..74cddb0d078 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/transaction-controller` from `^69.6.1` to `^69.7.0` ([#10046](https://github.com/MetaMask/core/pull/10046)) +### Fixed + +- Fix the unlock-time spam cleanup sweep (`useUnlockCleanup`) deleting mUSD holdings on chains outside its 3-chain seeding registry, and treating a token entirely absent from the Token API's response as spam instead of unjudgeable ([#10066](https://github.com/MetaMask/core/pull/10066)) + ## [14.0.3] ### Changed diff --git a/packages/assets-controller/src/__fixtures__/mockTokenApi.ts b/packages/assets-controller/src/__fixtures__/mockTokenApi.ts index aace581815b..d6300931d96 100644 --- a/packages/assets-controller/src/__fixtures__/mockTokenApi.ts +++ b/packages/assets-controller/src/__fixtures__/mockTokenApi.ts @@ -6,6 +6,7 @@ import { ARBITRUM_GMX, BASE_FARTCOIN, BASE_USDC, + BNB_MUSD, MAINNET_MUSD, MAINNET_USDT, MONAD_WMON, @@ -51,6 +52,7 @@ export const TOKEN_API_OCCURRENCES: Record = { [SOLANA_USDC]: 4, [OPTIMISM_SPAM]: 2, // a scam token that talked its way onto two lists [MONAD_WMON]: undefined, // as the API answers for every Monad token today + [BNB_MUSD]: 1, // real, low real-world count — matches the live Token API }; export function createTestApiClient(): ApiPlatformClient { diff --git a/packages/assets-controller/src/__fixtures__/spamWalletState.ts b/packages/assets-controller/src/__fixtures__/spamWalletState.ts index 73ac89fa4b3..8bf3cd9939a 100644 --- a/packages/assets-controller/src/__fixtures__/spamWalletState.ts +++ b/packages/assets-controller/src/__fixtures__/spamWalletState.ts @@ -60,6 +60,16 @@ export const FLARE_SFLR = export const MONAD_WMON = 'eip155:143/erc20:0x3bd359C1119dA7Da1D913D1C4D2B7c461115433A' as Caip19AssetId; +/** + * mUSD on BNB Smart Chain (56) — a chain covered by the Accounts API but + * absent from `DEFAULT_TRACKED_ASSETS_BY_CHAIN`'s mUSD entries (which only + * seed Ethereum, Linea, and Monad). Real deployment, low real occurrence + * count (matching the live Token API) — exactly the shape that used to lose + * its spam-filter exemption on any chain outside that seeding list. + */ +export const BNB_MUSD = + 'eip155:56/erc20:0xacA92E438df0B2401fF60dA7E4337B687a2435DA' as Caip19AssetId; + export const MAINNET_SPAM = 'eip155:1/erc20:0xB5f0e1b64a4a1a2A6cbf0E8f9d0c4e7A1b2C3D4E' as Caip19AssetId; export const OPTIMISM_SPAM = @@ -186,6 +196,12 @@ const TOKEN_METADATA: Record = { name: 'Wrapped MON', decimals: 18, }, + [BNB_MUSD]: { + type: 'erc20', + symbol: 'MUSD', + name: 'MetaMask USD', + decimals: 6, + }, [ARBITRUM_GMX]: { type: 'erc20', symbol: 'GMX', diff --git a/packages/assets-controller/src/data-sources/TokenDataSource.ts b/packages/assets-controller/src/data-sources/TokenDataSource.ts index fffed2b9461..de5b604c4eb 100644 --- a/packages/assets-controller/src/data-sources/TokenDataSource.ts +++ b/packages/assets-controller/src/data-sources/TokenDataSource.ts @@ -9,6 +9,7 @@ import { KnownCaipNamespace, parseCaipAssetType } from '@metamask/utils'; import type { CaipAssetType } from '@metamask/utils'; import type { AssetsControllerMessenger } from '../AssetsController.js'; +import { isMusdAssetId } from '../defaults.js'; import { projectLogger, createModuleLogger } from '../logger.js'; import { forDataTypes } from '../types.js'; import type { @@ -52,8 +53,6 @@ export enum CaipAssetNamespace { Token = 'token', } -const MUSD_ADDRESS_LOWERCASE = '0xaca92e438df0b2401ff60da7e4337b687a2435da'; - // ============================================================================ // OPTIONS // ============================================================================ @@ -402,7 +401,7 @@ export class TokenDataSource { knownBalanceIds.has(lowerId) || knownMetadataIds.has(lowerId) || customAssetIds.has(lowerId) || - lowerId.includes(`/erc20:${MUSD_ADDRESS_LOWERCASE}`) + isMusdAssetId(lowerId) ) { continue; } @@ -679,7 +678,7 @@ export class TokenDataSource { balanceHealAssetIds.has(id.toLowerCase()) || (occurrencesByAssetId.get(id) ?? 0) >= getOccurrenceFloorForAsset(id, suggestedOccurrenceFloors) || - id.includes(`/erc20:${MUSD_ADDRESS_LOWERCASE}`), + isMusdAssetId(id), ), ); diff --git a/packages/assets-controller/src/defaults.ts b/packages/assets-controller/src/defaults.ts index 613819c7002..167d00a099e 100644 --- a/packages/assets-controller/src/defaults.ts +++ b/packages/assets-controller/src/defaults.ts @@ -19,6 +19,31 @@ import type { */ const MUSD_ADDRESS = '0xacA92E438df0B2401fF60dA7E4337B687a2435DA'; +/** + * Lowercase form of {@link MUSD_ADDRESS}, for case-insensitive comparisons + * against asset IDs sourced from APIs that don't checksum (e.g. the Token + * API's V3 assets response). This is the single source of truth for "is + * this asset mUSD" — every spam/occurrence filter in this package must + * exempt mUSD chain-agnostically by address, not by enumerating chains in + * {@link DEFAULT_TRACKED_ASSETS_BY_CHAIN} (that map is a *seeding* registry + * for a handful of chains, not an exhaustive list of every chain mUSD is + * deployed to — using it as an exemption list left mUSD holdings on other + * chains exposed to the occurrence-floor spam filter). + */ +export const MUSD_ADDRESS_LOWERCASE = MUSD_ADDRESS.toLowerCase(); + +/** + * Whether a CAIP-19 asset ID refers to MetaMask USD (mUSD), on any chain. + * Case-insensitive so it matches both checksummed (state) and lowercase + * (some API responses) asset ID forms. + * + * @param assetId - The CAIP-19 asset ID to check (or any string). + * @returns `true` if the asset ID's ERC-20 address is mUSD's. + */ +export function isMusdAssetId(assetId: string): boolean { + return assetId.toLowerCase().includes(`/erc20:${MUSD_ADDRESS_LOWERCASE}`); +} + /** * Hardcoded metadata for MetaMask USD. Pre-seeding this in default * state makes the token immediately renderable in the UI before any diff --git a/packages/assets-controller/src/migrations/healAssetsInfoMetadata.test.ts b/packages/assets-controller/src/migrations/healAssetsInfoMetadata.test.ts index dcaf4dfacf9..7caa45794ca 100644 --- a/packages/assets-controller/src/migrations/healAssetsInfoMetadata.test.ts +++ b/packages/assets-controller/src/migrations/healAssetsInfoMetadata.test.ts @@ -12,6 +12,7 @@ import { BASE_FARTCOIN, BASE_SPAM, BASE_USDC, + BNB_MUSD, MAINNET_NATIVE, MAINNET_USDT, MONAD_WMON, @@ -778,10 +779,16 @@ describe('cleanSpamAssets', () => { removed: [MONAD_WMON], }, { - description: 'drops a token the API leaves out of its response', + // An asset entirely absent from the response array (as opposed to + // present with no occurrence count, like MONAD_WMON above) is one the + // API has never indexed at all — unjudgeable, so it must be kept, not + // deleted. MAINNET_USDT is a genuine, high-volume token; a chain the + // Token API doesn't serve for this asset is not evidence of spam. + description: + 'keeps a token the API leaves out of its response entirely', held: [MAINNET_USDT, OPTIMISM_USDC], omittedFromResponse: [MAINNET_USDT], - removed: [MAINNET_USDT], + removed: [], }, { // State keys are EIP-55 checksummed; the API answers in lowercase. @@ -797,6 +804,16 @@ describe('cleanSpamAssets', () => { removed: [], casing: 'checksum', }, + { + // BNB is Accounts-API-covered but absent from + // `DEFAULT_TRACKED_ASSETS_BY_CHAIN`'s mUSD entries (only Ethereum, + // Linea, and Monad are seeded there) — mUSD's real, below-floor + // occurrence count on this chain used to make the sweep delete it. + // mUSD must be exempt chain-agnostically, by address, everywhere. + description: 'never drops mUSD, even on a chain it is not default-tracked on', + held: [BNB_MUSD, OPTIMISM_SPAM], + removed: [OPTIMISM_SPAM], + }, ]; it.each(classificationCases)( diff --git a/packages/assets-controller/src/migrations/healAssetsInfoMetadata.ts b/packages/assets-controller/src/migrations/healAssetsInfoMetadata.ts index 6845d275511..b29246c276b 100644 --- a/packages/assets-controller/src/migrations/healAssetsInfoMetadata.ts +++ b/packages/assets-controller/src/migrations/healAssetsInfoMetadata.ts @@ -11,7 +11,7 @@ import { import { cloneDeep } from 'lodash'; import { divideIntoBatches } from '../data-sources/evm-rpc-services/utils/batch.js'; -import { DEFAULT_TRACKED_ASSETS_BY_CHAIN } from '../defaults.js'; +import { DEFAULT_TRACKED_ASSETS_BY_CHAIN, isMusdAssetId } from '../defaults.js'; import { createModuleLogger, projectLogger } from '../logger.js'; import type { AccountId, @@ -724,7 +724,15 @@ function collectSpamCleanupCandidates({ const [chainId, asset] = lowerId.split('/'); const isERC20 = Boolean(asset?.startsWith('erc20:')); - const isNotExcluded = !excludedAssets.includes(lowerId); + // `DEFAULT_TRACKED_ASSETS_BY_CHAIN` is a *seeding* registry that only + // lists mUSD on the chains it's been added as a default tracked asset — + // not every chain mUSD is actually deployed to. Exempting by that list + // alone left mUSD holdings on every other chain exposed to the + // occurrence-floor filter below, where mUSD's real (low) aggregator + // count falls under the floor and this sweep deletes the holding. Exempt + // mUSD chain-agnostically by address instead, matching how the sibling + // spam filters in `TokenDataSource.ts` already do it. + const isNotExcluded = !excludedAssets.includes(lowerId) && !isMusdAssetId(lowerId); const isNotCustomAsset = !customAssetIds.has(lowerId); const isOnAccountAPICoveredChain = ACCOUNT_API_SUPPORTED_CHAIN_IDS.has(chainId); @@ -760,14 +768,33 @@ async function findBelowFloorAssetIds( FETCH_TIMEOUT_MS, ); + // Assets absent from the response array altogether are ones the API has + // never indexed at all (e.g. a chain it doesn't serve) — distinct from + // an asset the API *did* respond about but scored with no occurrence + // count. Only the former is unjudgeable; matches the sibling + // `TokenDataSource.occurrenceFilterMiddleware`'s "only assets the API + // knows can be judged; missing ones are kept" contract, which likewise + // never iterates past what the response array actually contains. Unlike + // that sibling — whose worst case for "unjudgeable" is not adding a + // token — this function DELETES existing holdings, so defaulting an + // asset entirely missing from the response to a confirmed-zero + // occurrence count turned "the API has never indexed this asset" into + // "delete the user's holding of it". + const respondedLowerIds = new Set( + assets.map((asset) => asset.assetId.toLowerCase()), + ); const occurrencesByLowerId = new Map( assets.map((asset) => [asset.assetId.toLowerCase(), asset.occurrences]), ); return assetIds.filter((assetId) => { + const lowerId = assetId.toLowerCase(); + if (!respondedLowerIds.has(lowerId)) { + return false; + } const chainReference = assetId.split(':')[1]?.split('/')[0] ?? ''; const floor = floors[chainReference] ?? DEFAULT_OCCURRENCE_FLOOR; - return (occurrencesByLowerId.get(assetId.toLowerCase()) ?? 0) < floor; + return (occurrencesByLowerId.get(lowerId) ?? 0) < floor; }); } catch (error) { cleanupLog('Failed to fetch assets', error);