From 3556501b44f81d01c1e6a511f32148ff044a9f0d Mon Sep 17 00:00:00 2001 From: Kriys94 Date: Tue, 29 Sep 2026 13:14:20 +0200 Subject: [PATCH 1/2] fix(assets-controller): clean v5 native balance state update --- packages/assets-controller/CHANGELOG.md | 4 + .../src/AssetsController.test.ts | 98 +++++++++++++++ .../assets-controller/src/AssetsController.ts | 115 +++++++++++++----- 3 files changed, 187 insertions(+), 30 deletions(-) diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index e9a3faee0e7..2cf8db9a431 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `lodash-es` from `^4.17.21` to `^4.18.1` ([#10447](https://github.com/MetaMask/core/pull/10447)) - Bump `@ethersproject/providers` from `^5.7.0` to `^5.8.0` ([#10482](https://github.com/MetaMask/core/pull/10482)) +### Fixed + +- On the v5 balance path, stop seeding every enabled chain's native onto an account when the update arrives after that account is no longer selected. Selecting the account group again drops stored balances whose chain namespace is outside the account's scopes ([#10567](https://github.com/MetaMask/core/pull/10567)) + ## [17.0.0] ### Added diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 9b512f390a9..206b2f5d978 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -3177,6 +3177,35 @@ describe('AssetsController', () => { }); }); + it('does not seed enabled-chain natives onto an account that is no longer selected', async () => { + const bitcoinAccountId = 'bitcoin-account-id'; + const bitcoinNative = + 'bip122:000000000019d6689c085ae165831e93/slip44:0' as Caip19AssetId; + + await withController(async ({ controller, getSelectedAccountsMock }) => { + getSelectedAccountsMock.mockReturnValue([ + createMockInternalAccount({ id: 'imported-account-id' }), + ]); + + await controller.handleAssetsUpdate( + { + assetsBalance: { + [bitcoinAccountId]: { + [bitcoinNative]: { amount: '0.00000404' }, + }, + }, + }, + 'TestSource', + ); + + const balances = controller.state.assetsBalance[bitcoinAccountId] ?? {}; + expect(balances[bitcoinNative]).toStrictEqual({ + amount: '0.00000404', + }); + expect(balances[MOCK_NATIVE_ASSET_ID]).toBeUndefined(); + }); + }); + it('adds default 0 balance for native tokens when missing from response', async () => { await withController(async ({ controller }) => { await controller.handleAssetsUpdate( @@ -5195,6 +5224,75 @@ describe('AssetsController', () => { }); describe('account group changes', () => { + it('drops balances outside the selected account scopes when the group changes', async () => { + const bitcoinAccountId = 'bitcoin-account-id'; + const bitcoinNative = + 'bip122:000000000019d6689c085ae165831e93/slip44:0' as Caip19AssetId; + const solanaNative = + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501' as Caip19AssetId; + const lineaNative = 'eip155:59144/slip44:60' as Caip19AssetId; + + await withController( + { + state: { + assetsBalance: { + [bitcoinAccountId]: { + [bitcoinNative]: { amount: '0.00000404' }, + [solanaNative]: { amount: '0' }, + [MOCK_NATIVE_ASSET_ID]: { amount: '0' }, + }, + [MOCK_ACCOUNT_ID]: { + [MOCK_NATIVE_ASSET_ID]: { amount: '0.02' }, + [lineaNative]: { amount: '0' }, + [solanaNative]: { amount: '0' }, + }, + }, + }, + }, + async ({ controller, messenger, getSelectedAccountsMock }) => { + const getAssetsSpy = jest + .spyOn(controller, 'getAssets') + .mockResolvedValue({}); + + await activateTracking(messenger); + getAssetsSpy.mockClear(); + + getSelectedAccountsMock.mockReturnValue([ + createMockInternalAccount({ + id: bitcoinAccountId, + address: 'bc1qdy9vjsg26rrd8tkqr7f26hlhd9524wyjkynemj', + type: 'bip122:p2wpkh', + scopes: ['bip122:000000000019d6689c085ae165831e93'], + }), + createMockInternalAccount({ + scopes: ['eip155:0'], + }), + ]); + + (messenger as unknown as { publish: LifecyclePublish }).publish( + 'AccountTreeController:selectedAccountGroupChange', + 'entropy:mock-keyring-id-1/0', + 'keyring:Simple Key Pair/0ximported', + ); + await flushPromises(); + + expect( + controller.state.assetsBalance[bitcoinAccountId], + ).toStrictEqual({ + [bitcoinNative]: { amount: '0.00000404' }, + }); + const evmBalances = + controller.state.assetsBalance[MOCK_ACCOUNT_ID] ?? {}; + expect(evmBalances[solanaNative]).toBeUndefined(); + expect(evmBalances[MOCK_NATIVE_ASSET_ID]).toStrictEqual({ + amount: '0.02', + }); + expect(evmBalances[lineaNative]).toStrictEqual({ amount: '0' }); + getAssetsSpy.mockRestore(); + }, + ); + }); + it('refreshes assets when the selected group changes while tracking', async () => { await withController(async ({ controller, messenger }) => { const getAssetsSpy = jest diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 05601020f42..4cc6ddbe6bc 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -1417,14 +1417,14 @@ export class AssetsController extends BaseController< }); // Seed before subscribe so the price poll / update fetch sees natives // and default tracked assets that were never returned by balance APIs. - this.#ensureNativeBalancesDefaultZero(); + this.#reconcileSelectedAccountBalances(); this.#ensureDefaultTrackedAssetsSeeded(); // Balances were just force-fetched — skip AccountsApi's subscribe-time poll. this.#subscribeAssets({ skipInitialFetch: true }); this.#fetchMissingPricesWithoutCache(accounts, [...this.#enabledChains]); } catch (error) { log('Failed to fetch assets on startup', error); - this.#ensureNativeBalancesDefaultZero(); + this.#reconcileSelectedAccountBalances(); this.#ensureDefaultTrackedAssetsSeeded(); this.#subscribeAssets({ skipInitialFetch: true }); this.#fetchMissingPricesWithoutCache(accounts, [...this.#enabledChains]); @@ -2802,11 +2802,17 @@ export class AssetsController extends BaseController< } /** - * Ensures assetsBalance has a 0 balance for each native token (from - * NetworkEnablementController.nativeAssetIdentifiers) for each selected account. - * Only adds natives for chains that the account supports (correct accountId ↔ chain mapping). + * Make each selected account's balances match the chains it can hold. + * + * Adds a 0 balance for each native the account supports when the entry is + * missing, using NetworkEnablementController.nativeAssetIdentifiers. Removes + * balances whose chain namespace is outside the account's scopes. A v5 + * update that arrived after a group switch used to stamp every enabled + * native onto the accounts it was leaving, and later updates never remove + * those keys. The sweep runs here because this is where the account object, + * and therefore its scopes, is available. */ - #ensureNativeBalancesDefaultZero(): void { + #reconcileSelectedAccountBalances(): void { const accounts = this.#getSelectedAccounts(); if (accounts.length === 0) { return; @@ -2817,29 +2823,78 @@ export class AssetsController extends BaseController< Record >; for (const account of accounts) { - const accountId = account.id; - const nativeAssetIds = this.#getNativeAssetIdsForAccount(account); - if (nativeAssetIds.length === 0) { - continue; - } - if (!balances[accountId]) { - balances[accountId] = {}; - } - for (const nativeAssetId of nativeAssetIds) { - if ( - !Object.prototype.hasOwnProperty.call( - balances[accountId], - nativeAssetId, - ) - ) { - balances[accountId][nativeAssetId] = - getDefaultNativeAssetBalance(nativeAssetId); - } - } + this.#seedMissingNativeBalances(balances, account); + this.#dropBalancesOutsideAccountScopes(balances, account); } }); } + /** + * Add a 0 balance for each native the account supports when the entry is missing. + * + * Natives come from NetworkEnablementController.nativeAssetIdentifiers, + * limited to chains in the account's scopes. Existing entries are kept. + * + * @param balances - `assetsBalance` being updated. + * @param account - Account whose supported chains determine the natives to seed. + */ + #seedMissingNativeBalances( + balances: Record>, + account: InternalAccount, + ): void { + const nativeAssetIds = this.#getNativeAssetIdsForAccount(account); + if (nativeAssetIds.length === 0) { + return; + } + const accountId = account.id; + if (!balances[accountId]) { + balances[accountId] = {}; + } + for (const nativeAssetId of nativeAssetIds) { + if ( + !Object.prototype.hasOwnProperty.call( + balances[accountId], + nativeAssetId, + ) + ) { + balances[accountId][nativeAssetId] = + getDefaultNativeAssetBalance(nativeAssetId); + } + } + } + + /** + * Remove balance entries the account cannot own. + * + * Ownership is the chain namespace of the account's scopes, not the + * currently enabled chains. `eip155:0` keeps every EVM chain, including + * ones the user has turned off. An account with no scopes is left untouched. + * + * @param balances - `assetsBalance` being updated. + * @param account - Account whose scopes define the namespaces to keep. + */ + #dropBalancesOutsideAccountScopes( + balances: Record>, + account: InternalAccount, + ): void { + const scopes = account.scopes ?? []; + if (scopes.length === 0) { + return; + } + const accountBalances = balances[account.id]; + if (!accountBalances) { + return; + } + const namespaces = new Set( + scopes.map((scope) => String(scope).split(':')[0]), + ); + for (const assetId of Object.keys(accountBalances)) { + if (!namespaces.has(assetId.split(':')[0])) { + delete accountBalances[assetId]; + } + } + } + /** * Seed selected accounts with zero-balance entries for every * controller-managed default tracked asset (e.g. mUSD on mainnet, @@ -3032,7 +3087,7 @@ export class AssetsController extends BaseController< ); const nativeAssetIdsForAccount = account ? this.#getNativeAssetIdsForAccount(account) - : this.#getNativeAssetIdsForEnabledChains(); + : []; for (const nativeAssetId of nativeAssetIdsForAccount) { if ( !Object.prototype.hasOwnProperty.call( @@ -3963,7 +4018,7 @@ export class AssetsController extends BaseController< }); } - this.#ensureNativeBalancesDefaultZero(); + this.#reconcileSelectedAccountBalances(); this.#ensureDefaultTrackedAssetsSeeded(); this.#subscribeAssets({ skipInitialFetch: true }); this.#fetchMissingPricesWithoutCache(accounts, [...this.#enabledChains]); @@ -4018,7 +4073,7 @@ export class AssetsController extends BaseController< }); } - this.#ensureNativeBalancesDefaultZero(); + this.#reconcileSelectedAccountBalances(); // Seed default tracked assets (mUSD) for any chain the user has // *just* enabled. This is what makes mUSD appear on Monad after // the user finally adds it to NetworkEnablementController. @@ -4133,7 +4188,7 @@ export class AssetsController extends BaseController< dataTypes: ['balance', 'metadata', 'price'], }); - this.#ensureNativeBalancesDefaultZero(); + this.#reconcileSelectedAccountBalances(); this.#fetchMissingPricesWithoutCache(accounts, [selectedChainId]); } finally { releaseLock(); @@ -4180,7 +4235,7 @@ export class AssetsController extends BaseController< forceUpdate: true, dataTypes: ['balance', 'metadata', 'price'], }); - this.#ensureNativeBalancesDefaultZero(); + this.#reconcileSelectedAccountBalances(); this.#fetchMissingPricesWithoutCache(accounts, [caipChainId]); } From 7c5493561af2cd699f1df92467ae5dcd02e91d0d Mon Sep 17 00:00:00 2001 From: Kriys94 Date: Wed, 30 Sep 2026 14:18:01 +0200 Subject: [PATCH 2/2] move drop balance outside of AssetsController --- packages/assets-controller/CHANGELOG.md | 3 +- .../src/AssetsController.test.ts | 98 -------- .../assets-controller/src/AssetsController.ts | 139 +++++------ .../dropBalancesOutsideAccountScopes.test.ts | 216 ++++++++++++++++++ .../dropBalancesOutsideAccountScopes.ts | 84 +++++++ 5 files changed, 357 insertions(+), 183 deletions(-) create mode 100644 packages/assets-controller/src/migrations/dropBalancesOutsideAccountScopes.test.ts create mode 100644 packages/assets-controller/src/migrations/dropBalancesOutsideAccountScopes.ts diff --git a/packages/assets-controller/CHANGELOG.md b/packages/assets-controller/CHANGELOG.md index 2cf8db9a431..ba02904f737 100644 --- a/packages/assets-controller/CHANGELOG.md +++ b/packages/assets-controller/CHANGELOG.md @@ -14,7 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- On the v5 balance path, stop seeding every enabled chain's native onto an account when the update arrives after that account is no longer selected. Selecting the account group again drops stored balances whose chain namespace is outside the account's scopes ([#10567](https://github.com/MetaMask/core/pull/10567)) +- On the v5 balance path, stop seeding every enabled chain's native onto an account when the update arrives after that account is no longer selected ([#10567](https://github.com/MetaMask/core/pull/10567)) +- On unlock, drop stored balances whose chain namespace is outside the selected account's scopes ([#10567](https://github.com/MetaMask/core/pull/10567)) ## [17.0.0] diff --git a/packages/assets-controller/src/AssetsController.test.ts b/packages/assets-controller/src/AssetsController.test.ts index 206b2f5d978..9b512f390a9 100644 --- a/packages/assets-controller/src/AssetsController.test.ts +++ b/packages/assets-controller/src/AssetsController.test.ts @@ -3177,35 +3177,6 @@ describe('AssetsController', () => { }); }); - it('does not seed enabled-chain natives onto an account that is no longer selected', async () => { - const bitcoinAccountId = 'bitcoin-account-id'; - const bitcoinNative = - 'bip122:000000000019d6689c085ae165831e93/slip44:0' as Caip19AssetId; - - await withController(async ({ controller, getSelectedAccountsMock }) => { - getSelectedAccountsMock.mockReturnValue([ - createMockInternalAccount({ id: 'imported-account-id' }), - ]); - - await controller.handleAssetsUpdate( - { - assetsBalance: { - [bitcoinAccountId]: { - [bitcoinNative]: { amount: '0.00000404' }, - }, - }, - }, - 'TestSource', - ); - - const balances = controller.state.assetsBalance[bitcoinAccountId] ?? {}; - expect(balances[bitcoinNative]).toStrictEqual({ - amount: '0.00000404', - }); - expect(balances[MOCK_NATIVE_ASSET_ID]).toBeUndefined(); - }); - }); - it('adds default 0 balance for native tokens when missing from response', async () => { await withController(async ({ controller }) => { await controller.handleAssetsUpdate( @@ -5224,75 +5195,6 @@ describe('AssetsController', () => { }); describe('account group changes', () => { - it('drops balances outside the selected account scopes when the group changes', async () => { - const bitcoinAccountId = 'bitcoin-account-id'; - const bitcoinNative = - 'bip122:000000000019d6689c085ae165831e93/slip44:0' as Caip19AssetId; - const solanaNative = - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501' as Caip19AssetId; - const lineaNative = 'eip155:59144/slip44:60' as Caip19AssetId; - - await withController( - { - state: { - assetsBalance: { - [bitcoinAccountId]: { - [bitcoinNative]: { amount: '0.00000404' }, - [solanaNative]: { amount: '0' }, - [MOCK_NATIVE_ASSET_ID]: { amount: '0' }, - }, - [MOCK_ACCOUNT_ID]: { - [MOCK_NATIVE_ASSET_ID]: { amount: '0.02' }, - [lineaNative]: { amount: '0' }, - [solanaNative]: { amount: '0' }, - }, - }, - }, - }, - async ({ controller, messenger, getSelectedAccountsMock }) => { - const getAssetsSpy = jest - .spyOn(controller, 'getAssets') - .mockResolvedValue({}); - - await activateTracking(messenger); - getAssetsSpy.mockClear(); - - getSelectedAccountsMock.mockReturnValue([ - createMockInternalAccount({ - id: bitcoinAccountId, - address: 'bc1qdy9vjsg26rrd8tkqr7f26hlhd9524wyjkynemj', - type: 'bip122:p2wpkh', - scopes: ['bip122:000000000019d6689c085ae165831e93'], - }), - createMockInternalAccount({ - scopes: ['eip155:0'], - }), - ]); - - (messenger as unknown as { publish: LifecyclePublish }).publish( - 'AccountTreeController:selectedAccountGroupChange', - 'entropy:mock-keyring-id-1/0', - 'keyring:Simple Key Pair/0ximported', - ); - await flushPromises(); - - expect( - controller.state.assetsBalance[bitcoinAccountId], - ).toStrictEqual({ - [bitcoinNative]: { amount: '0.00000404' }, - }); - const evmBalances = - controller.state.assetsBalance[MOCK_ACCOUNT_ID] ?? {}; - expect(evmBalances[solanaNative]).toBeUndefined(); - expect(evmBalances[MOCK_NATIVE_ASSET_ID]).toStrictEqual({ - amount: '0.02', - }); - expect(evmBalances[lineaNative]).toStrictEqual({ amount: '0' }); - getAssetsSpy.mockRestore(); - }, - ); - }); - it('refreshes assets when the selected group changes while tracking', async () => { await withController(async ({ controller, messenger }) => { const getAssetsSpy = jest diff --git a/packages/assets-controller/src/AssetsController.ts b/packages/assets-controller/src/AssetsController.ts index 4cc6ddbe6bc..dddd05bbb05 100644 --- a/packages/assets-controller/src/AssetsController.ts +++ b/packages/assets-controller/src/AssetsController.ts @@ -118,6 +118,7 @@ import { createParallelMiddleware, } from './middlewares/ParallelMiddleware.js'; import { RpcFallbackMiddleware } from './middlewares/RpcFallbackMiddleware.js'; +import { dropBalancesOutsideAccountScopes } from './migrations/dropBalancesOutsideAccountScopes.js'; import type { Assets3346MigrationState } from './migrations/healAssetsInfoMetadata.js'; import { cleanSpamAssets, @@ -1261,6 +1262,11 @@ export class AssetsController extends BaseController< // eslint-disable-next-line @typescript-eslint/no-misused-promises this.messenger.subscribe('KeyringController:unlock', async () => { + try { + this.#runOutOfScopeBalanceCleanup(); + } catch { + /* Do nothing */ + } await this.#runSpamCleanup().catch(() => { /* Do nothing */ }); @@ -1339,6 +1345,26 @@ export class AssetsController extends BaseController< }); } + /** + * One-time cleanup of balances a v5 update stamped onto accounts after a + * group switch. Runs on unlock, the same point as spam cleanup, and only + * touches the selected accounts (their scopes are what define ownership). + */ + #runOutOfScopeBalanceCleanup(): void { + const accounts = this.#getSelectedAccounts(); + if (accounts.length === 0) { + return; + } + + this.update((state) => { + const balances = state.assetsBalance as Record< + string, + Record + >; + dropBalancesOutsideAccountScopes(balances, accounts); + }); + } + async #runSpamCleanup(): Promise { try { const shouldRun = @@ -1417,14 +1443,14 @@ export class AssetsController extends BaseController< }); // Seed before subscribe so the price poll / update fetch sees natives // and default tracked assets that were never returned by balance APIs. - this.#reconcileSelectedAccountBalances(); + this.#ensureNativeBalancesDefaultZero(); this.#ensureDefaultTrackedAssetsSeeded(); // Balances were just force-fetched — skip AccountsApi's subscribe-time poll. this.#subscribeAssets({ skipInitialFetch: true }); this.#fetchMissingPricesWithoutCache(accounts, [...this.#enabledChains]); } catch (error) { log('Failed to fetch assets on startup', error); - this.#reconcileSelectedAccountBalances(); + this.#ensureNativeBalancesDefaultZero(); this.#ensureDefaultTrackedAssetsSeeded(); this.#subscribeAssets({ skipInitialFetch: true }); this.#fetchMissingPricesWithoutCache(accounts, [...this.#enabledChains]); @@ -2802,17 +2828,11 @@ export class AssetsController extends BaseController< } /** - * Make each selected account's balances match the chains it can hold. - * - * Adds a 0 balance for each native the account supports when the entry is - * missing, using NetworkEnablementController.nativeAssetIdentifiers. Removes - * balances whose chain namespace is outside the account's scopes. A v5 - * update that arrived after a group switch used to stamp every enabled - * native onto the accounts it was leaving, and later updates never remove - * those keys. The sweep runs here because this is where the account object, - * and therefore its scopes, is available. + * Ensures assetsBalance has a 0 balance for each native token (from + * NetworkEnablementController.nativeAssetIdentifiers) for each selected account. + * Only adds natives for chains that the account supports (correct accountId ↔ chain mapping). */ - #reconcileSelectedAccountBalances(): void { + #ensureNativeBalancesDefaultZero(): void { const accounts = this.#getSelectedAccounts(); if (accounts.length === 0) { return; @@ -2823,78 +2843,29 @@ export class AssetsController extends BaseController< Record >; for (const account of accounts) { - this.#seedMissingNativeBalances(balances, account); - this.#dropBalancesOutsideAccountScopes(balances, account); + const accountId = account.id; + const nativeAssetIds = this.#getNativeAssetIdsForAccount(account); + if (nativeAssetIds.length === 0) { + continue; + } + if (!balances[accountId]) { + balances[accountId] = {}; + } + for (const nativeAssetId of nativeAssetIds) { + if ( + !Object.prototype.hasOwnProperty.call( + balances[accountId], + nativeAssetId, + ) + ) { + balances[accountId][nativeAssetId] = + getDefaultNativeAssetBalance(nativeAssetId); + } + } } }); } - /** - * Add a 0 balance for each native the account supports when the entry is missing. - * - * Natives come from NetworkEnablementController.nativeAssetIdentifiers, - * limited to chains in the account's scopes. Existing entries are kept. - * - * @param balances - `assetsBalance` being updated. - * @param account - Account whose supported chains determine the natives to seed. - */ - #seedMissingNativeBalances( - balances: Record>, - account: InternalAccount, - ): void { - const nativeAssetIds = this.#getNativeAssetIdsForAccount(account); - if (nativeAssetIds.length === 0) { - return; - } - const accountId = account.id; - if (!balances[accountId]) { - balances[accountId] = {}; - } - for (const nativeAssetId of nativeAssetIds) { - if ( - !Object.prototype.hasOwnProperty.call( - balances[accountId], - nativeAssetId, - ) - ) { - balances[accountId][nativeAssetId] = - getDefaultNativeAssetBalance(nativeAssetId); - } - } - } - - /** - * Remove balance entries the account cannot own. - * - * Ownership is the chain namespace of the account's scopes, not the - * currently enabled chains. `eip155:0` keeps every EVM chain, including - * ones the user has turned off. An account with no scopes is left untouched. - * - * @param balances - `assetsBalance` being updated. - * @param account - Account whose scopes define the namespaces to keep. - */ - #dropBalancesOutsideAccountScopes( - balances: Record>, - account: InternalAccount, - ): void { - const scopes = account.scopes ?? []; - if (scopes.length === 0) { - return; - } - const accountBalances = balances[account.id]; - if (!accountBalances) { - return; - } - const namespaces = new Set( - scopes.map((scope) => String(scope).split(':')[0]), - ); - for (const assetId of Object.keys(accountBalances)) { - if (!namespaces.has(assetId.split(':')[0])) { - delete accountBalances[assetId]; - } - } - } - /** * Seed selected accounts with zero-balance entries for every * controller-managed default tracked asset (e.g. mUSD on mainnet, @@ -4018,7 +3989,7 @@ export class AssetsController extends BaseController< }); } - this.#reconcileSelectedAccountBalances(); + this.#ensureNativeBalancesDefaultZero(); this.#ensureDefaultTrackedAssetsSeeded(); this.#subscribeAssets({ skipInitialFetch: true }); this.#fetchMissingPricesWithoutCache(accounts, [...this.#enabledChains]); @@ -4073,7 +4044,7 @@ export class AssetsController extends BaseController< }); } - this.#reconcileSelectedAccountBalances(); + this.#ensureNativeBalancesDefaultZero(); // Seed default tracked assets (mUSD) for any chain the user has // *just* enabled. This is what makes mUSD appear on Monad after // the user finally adds it to NetworkEnablementController. @@ -4188,7 +4159,7 @@ export class AssetsController extends BaseController< dataTypes: ['balance', 'metadata', 'price'], }); - this.#reconcileSelectedAccountBalances(); + this.#ensureNativeBalancesDefaultZero(); this.#fetchMissingPricesWithoutCache(accounts, [selectedChainId]); } finally { releaseLock(); @@ -4235,7 +4206,7 @@ export class AssetsController extends BaseController< forceUpdate: true, dataTypes: ['balance', 'metadata', 'price'], }); - this.#reconcileSelectedAccountBalances(); + this.#ensureNativeBalancesDefaultZero(); this.#fetchMissingPricesWithoutCache(accounts, [caipChainId]); } diff --git a/packages/assets-controller/src/migrations/dropBalancesOutsideAccountScopes.test.ts b/packages/assets-controller/src/migrations/dropBalancesOutsideAccountScopes.test.ts new file mode 100644 index 00000000000..1a72fb529a6 --- /dev/null +++ b/packages/assets-controller/src/migrations/dropBalancesOutsideAccountScopes.test.ts @@ -0,0 +1,216 @@ +import type { InternalAccount } from '@metamask/keyring-internal-api'; + +import { createMockInternalAccount } from '../__fixtures__/MockAssetControllerMessenger.js'; +import type { Caip19AssetId } from '../types.js'; +import { dropBalancesOutsideAccountScopes } from './dropBalancesOutsideAccountScopes.js'; + +const BITCOIN_ACCOUNT_ID = 'bitcoin-account-id'; +const EVM_ACCOUNT_ID = 'evm-account-id'; +const BITCOIN_NATIVE = + 'bip122:000000000019d6689c085ae165831e93/slip44:0' as Caip19AssetId; +const SOLANA_NATIVE = + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501' as Caip19AssetId; +const MAINNET_NATIVE = 'eip155:1/slip44:60' as Caip19AssetId; +const LINEA_NATIVE = 'eip155:59144/slip44:60' as Caip19AssetId; + +const bitcoinAccount = (scopes: InternalAccount['scopes']): InternalAccount => + createMockInternalAccount({ + id: BITCOIN_ACCOUNT_ID, + address: 'bc1qdy9vjsg26rrd8tkqr7f26hlhd9524wyjkynemj', + type: 'bip122:p2wpkh', + scopes, + }); + +const evmAccount = (scopes: InternalAccount['scopes']): InternalAccount => + createMockInternalAccount({ id: EVM_ACCOUNT_ID, scopes }); + +describe('dropBalancesOutsideAccountScopes', () => { + it('drops balances whose chain namespace is outside the account scopes', () => { + const balances = { + [BITCOIN_ACCOUNT_ID]: { + [BITCOIN_NATIVE]: { amount: '0.00000404' }, + [SOLANA_NATIVE]: { amount: '0' }, + [MAINNET_NATIVE]: { amount: '0' }, + }, + [EVM_ACCOUNT_ID]: { + [MAINNET_NATIVE]: { amount: '0.02' }, + [LINEA_NATIVE]: { amount: '0' }, + [SOLANA_NATIVE]: { amount: '0' }, + }, + }; + + const removed = dropBalancesOutsideAccountScopes(balances, [ + bitcoinAccount(['bip122:000000000019d6689c085ae165831e93']), + evmAccount(['eip155:0']), + ]); + + expect(removed).toBe(true); + expect(balances[BITCOIN_ACCOUNT_ID]).toStrictEqual({ + [BITCOIN_NATIVE]: { amount: '0.00000404' }, + }); + expect(balances[EVM_ACCOUNT_ID]).toStrictEqual({ + [MAINNET_NATIVE]: { amount: '0.02' }, + [LINEA_NATIVE]: { amount: '0' }, + }); + }); + + it('keeps every EVM chain when the account scope is the eip155 wildcard', () => { + const balances = { + [EVM_ACCOUNT_ID]: { + [MAINNET_NATIVE]: { amount: '1' }, + [LINEA_NATIVE]: { amount: '2' }, + }, + }; + + const removed = dropBalancesOutsideAccountScopes(balances, [ + evmAccount(['eip155:0']), + ]); + + expect(removed).toBe(false); + expect(balances[EVM_ACCOUNT_ID]).toStrictEqual({ + [MAINNET_NATIVE]: { amount: '1' }, + [LINEA_NATIVE]: { amount: '2' }, + }); + }); + + it('keeps balances for every namespace the account scopes include', () => { + const balances = { + [EVM_ACCOUNT_ID]: { + [MAINNET_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '2' }, + [BITCOIN_NATIVE]: { amount: '3' }, + }, + }; + + const removed = dropBalancesOutsideAccountScopes(balances, [ + evmAccount(['eip155:1', 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp']), + ]); + + expect(removed).toBe(true); + expect(balances[EVM_ACCOUNT_ID]).toStrictEqual({ + [MAINNET_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '2' }, + }); + }); + + it('leaves malformed scopes and balance keys alone', () => { + const balances = { + [EVM_ACCOUNT_ID]: { + [MAINNET_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '0' }, + 'not-a-caip-id': { amount: '0' }, + }, + } as Record>; + + const removed = dropBalancesOutsideAccountScopes(balances, [ + evmAccount(['eip155:1', 'garbage' as `${string}:${string}`]), + ]); + + expect(removed).toBe(true); + expect(balances[EVM_ACCOUNT_ID]).toStrictEqual({ + [MAINNET_NATIVE]: { amount: '1' }, + 'not-a-caip-id': { amount: '0' }, + }); + }); + + it('leaves an account untouched when none of its scopes are valid CAIP-2 ids', () => { + const balances = { + [EVM_ACCOUNT_ID]: { + [MAINNET_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '0' }, + }, + }; + + const removed = dropBalancesOutsideAccountScopes(balances, [ + evmAccount(['garbage' as `${string}:${string}`]), + ]); + + expect(removed).toBe(false); + expect(balances[EVM_ACCOUNT_ID]).toStrictEqual({ + [MAINNET_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '0' }, + }); + }); + + it('leaves an account whose scopes are missing untouched', () => { + const balances = { + [BITCOIN_ACCOUNT_ID]: { + [BITCOIN_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '0' }, + }, + }; + const { scopes: _scopes, ...accountWithoutScopes } = bitcoinAccount([ + 'bip122:000000000019d6689c085ae165831e93', + ]); + + const removed = dropBalancesOutsideAccountScopes(balances, [ + accountWithoutScopes as InternalAccount, + ]); + + expect(removed).toBe(false); + expect(balances[BITCOIN_ACCOUNT_ID]).toStrictEqual({ + [BITCOIN_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '0' }, + }); + }); + + it('leaves an account with no scopes untouched', () => { + const balances = { + [BITCOIN_ACCOUNT_ID]: { + [BITCOIN_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '0' }, + }, + }; + + const removed = dropBalancesOutsideAccountScopes(balances, [ + bitcoinAccount([]), + ]); + + expect(removed).toBe(false); + expect(balances[BITCOIN_ACCOUNT_ID]).toStrictEqual({ + [BITCOIN_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '0' }, + }); + }); + + it('leaves an account that has no stored balances untouched', () => { + const balances = { + [EVM_ACCOUNT_ID]: { + [MAINNET_NATIVE]: { amount: '1' }, + }, + }; + + const removed = dropBalancesOutsideAccountScopes(balances, [ + createMockInternalAccount({ + id: 'missing-account', + scopes: ['eip155:1'], + }), + ]); + + expect(removed).toBe(false); + expect(balances).toStrictEqual({ + [EVM_ACCOUNT_ID]: { + [MAINNET_NATIVE]: { amount: '1' }, + }, + }); + }); + + it('leaves accounts that were not passed in untouched', () => { + const balances = { + [BITCOIN_ACCOUNT_ID]: { + [BITCOIN_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '0' }, + }, + }; + + const removed = dropBalancesOutsideAccountScopes(balances, [ + evmAccount(['eip155:1']), + ]); + + expect(removed).toBe(false); + expect(balances[BITCOIN_ACCOUNT_ID]).toStrictEqual({ + [BITCOIN_NATIVE]: { amount: '1' }, + [SOLANA_NATIVE]: { amount: '0' }, + }); + }); +}); diff --git a/packages/assets-controller/src/migrations/dropBalancesOutsideAccountScopes.ts b/packages/assets-controller/src/migrations/dropBalancesOutsideAccountScopes.ts new file mode 100644 index 00000000000..e10ba3270fe --- /dev/null +++ b/packages/assets-controller/src/migrations/dropBalancesOutsideAccountScopes.ts @@ -0,0 +1,84 @@ +import type { InternalAccount } from '@metamask/keyring-internal-api'; +import { + isCaipAssetType, + isCaipChainId, + parseCaipAssetType, + parseCaipChainId, +} from '@metamask/utils'; + +import type { AccountId, AssetBalance, Caip19AssetId } from '../types.js'; + +/** + * Remove balance entries the given accounts cannot own. + * + * Ownership is the chain namespace of the account's scopes, not the + * currently enabled chains. `eip155:0` keeps every EVM chain, including + * ones the user has turned off. An account with no scopes is left untouched, + * as is any account that is not in `accounts`. + * + * Mutates `balances`. Intended to run inside an Immer `update` draft, once + * per unlock, the same way spam cleanup is applied. + * + * @param balances - `assetsBalance` being updated. + * @param accounts - Accounts whose scopes define the namespaces to keep. + * @returns True when at least one balance entry was removed. + */ +export function dropBalancesOutsideAccountScopes( + balances: Record>, + accounts: readonly InternalAccount[], +): boolean { + let removed = false; + for (const account of accounts) { + if (dropAccountBalancesOutsideScopes(balances, account)) { + removed = true; + } + } + return removed; +} + +/** + * Remove one account's balances whose chain namespace is outside its scopes. + * + * @param balances - `assetsBalance` being updated. + * @param account - Account whose scopes define the namespaces to keep. + * @returns True when at least one balance entry was removed. + */ +function dropAccountBalancesOutsideScopes( + balances: Record>, + account: InternalAccount, +): boolean { + // Persisted accounts can omit `scopes` even though the type requires it. + const scopes = account.scopes ?? []; + if (scopes.length === 0) { + return false; + } + + const accountBalances = balances[account.id]; + if (!accountBalances) { + return false; + } + + const namespaces = new Set(); + for (const scope of scopes) { + if (isCaipChainId(scope)) { + namespaces.add(parseCaipChainId(scope).namespace); + } + } + if (namespaces.size === 0) { + return false; + } + + let removed = false; + for (const assetId of Object.keys(accountBalances) as Caip19AssetId[]) { + // Malformed keys are left alone; this cleanup only targets valid CAIP-19 + // entries stamped onto the wrong account. + if (!isCaipAssetType(assetId)) { + continue; + } + if (!namespaces.has(parseCaipAssetType(assetId).chain.namespace)) { + delete accountBalances[assetId]; + removed = true; + } + } + return removed; +}