From 570ba35b67c7d66eaa3ac639022c936ea45aa12b Mon Sep 17 00:00:00 2001 From: gabrieledm Date: Wed, 15 Apr 2026 09:50:53 +0200 Subject: [PATCH 01/10] fix: Managed `removed` property from `MultichainAssetsController:accountAssetListUpdated` event body to manage stale balances --- packages/assets-controllers/CHANGELOG.md | 1 + .../MultichainBalancesController.test.ts | 142 ++++++++++++++++++ .../MultichainBalancesController.ts | 81 +++++----- 3 files changed, 187 insertions(+), 37 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index b1fed9259c5..057eae50412 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Restore `checkAndUpdateSingleNftOwnershipStatus` to `NftController` to fix a regression where consumers (e.g. the MetaMask extension) that call this method individually were broken by its removal in [#8281](https://github.com/MetaMask/core/pull/8281) ([#8435](https://github.com/MetaMask/core/pull/8435)) +- Used the `removed` property coming from `MultichainAssetsController:accountAssetListUpdated` event body to manage stale balances ([#]()) ## [103.1.1] diff --git a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts index 1dfd40564c9..84fec30388a 100644 --- a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts +++ b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts @@ -732,6 +732,148 @@ describe('MultichainBalancesController', () => { }, }); }); + + it('removes stale balances that are no longer present in MultichainAssetsController state', async () => { + const mockSolanaAccountId1 = mockListSolanaAccounts[0].id; + + const existingBalancesState = { + [mockSolanaAccountId1]: { + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:removedToken': { + amount: '5.00000000', + unit: 'SOL', + }, + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken': { + amount: '6.00000000', + unit: 'SOL', + }, + }, + }; + + const { + controller, + messenger, + mockGetAssetsState, + mockSnapHandleRequest, + mockListMultichainAccounts, + } = setupController({ + state: { + balances: existingBalancesState, + }, + mocks: { + handleMockGetAssetsState: { + accountsAssets: { + [mockSolanaAccountId1]: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:removedToken', + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken', + ], + }, + }, + handleRequestReturnValue: { + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken': { + amount: '6.00000000', + unit: 'SOL', + }, + }, + listMultichainAccounts: [], + }, + }); + + mockSnapHandleRequest.mockReset(); + mockListMultichainAccounts.mockReset(); + + mockListMultichainAccounts.mockReturnValue(mockListSolanaAccounts); + mockGetAssetsState.mockReturnValue({ + accountsAssets: { + [mockSolanaAccountId1]: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken', + ], + }, + }); + mockSnapHandleRequest.mockResolvedValueOnce({ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken': { + amount: '6.00000000', + unit: 'SOL', + }, + }); + + messenger.publish('MultichainAssetsController:accountAssetListUpdated', { + assets: { + [mockSolanaAccountId1]: { + added: ['solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken'], + removed: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:removedToken', + ], + }, + }, + }); + + await waitForAllPromises(); + + expect(controller.state.balances).toStrictEqual({ + [mockSolanaAccountId1]: { + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken': { + amount: '6.00000000', + unit: 'SOL', + }, + }, + }); + }); + + it('clears balances when an account no longer has any assets after the update', async () => { + const mockSolanaAccountId1 = mockListSolanaAccounts[0].id; + + const { + controller, + messenger, + mockGetAssetsState, + mockListMultichainAccounts, + } = setupController({ + state: { + balances: { + [mockSolanaAccountId1]: { + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:removedToken': { + amount: '5.00000000', + unit: 'SOL', + }, + }, + }, + }, + mocks: { + listMultichainAccounts: [], + handleMockGetAssetsState: { + accountsAssets: { + [mockSolanaAccountId1]: [], + }, + }, + handleRequestReturnValue: {}, + }, + }); + + mockGetAssetsState.mockReturnValue({ + accountsAssets: { + [mockSolanaAccountId1]: [], + }, + }); + mockListMultichainAccounts.mockReset(); + mockListMultichainAccounts.mockReturnValue(mockListSolanaAccounts); + + messenger.publish('MultichainAssetsController:accountAssetListUpdated', { + assets: { + [mockSolanaAccountId1]: { + added: [], + removed: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:removedToken', + ], + }, + }, + }); + + await waitForAllPromises(); + + expect(controller.state.balances).toStrictEqual({ + [mockSolanaAccountId1]: {}, + }); + }); }); it('resumes updating balances after unlocking KeyringController', async () => { diff --git a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts index 88356c68bd3..9a977b92d66 100644 --- a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts +++ b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts @@ -178,26 +178,35 @@ export class MultichainBalancesController extends BaseController< this.messenger.subscribe( 'MultichainAssetsController:accountAssetListUpdated', async ({ assets }) => { - const newAccountAssets = Object.entries(assets).map( - ([accountId, { added }]) => ({ + const updatedAccountAssets = Object.entries(assets).map( + ([accountId, { added, removed }]) => ({ accountId, - assets: [...added], + added: [...added], + removed: [...removed], }), ); - await this.#handleOnAccountAssetListUpdated(newAccountAssets); + + await this.#handleOnAccountAssetListUpdated(updatedAccountAssets); }, ); } /** - * Updates the balances for the given accounts. + * Reconciles cached balances after a multichain asset-list update event. + * + * The event payload is treated as a delta: + * - balances for `removed` assets are deleted so stale entries cannot remain + * - balances for `added` assets are fetched from the snap and merged in + * - if an added asset is not returned by the snap, a zero placeholder is stored + * so the asset can still be represented in state * - * @param accounts - The accounts to update the balances for. + * @param accounts - The per-account asset deltas from the asset-list update event. */ async #handleOnAccountAssetListUpdated( accounts: { accountId: string; - assets: CaipAssetType[]; + added: CaipAssetType[]; + removed: CaipAssetType[]; }[], ): Promise { const { isUnlocked } = this.messenger.call('KeyringController:getState'); @@ -205,49 +214,47 @@ export class MultichainBalancesController extends BaseController< if (!isUnlocked) { return; } - const balancesToUpdate: MultichainBalancesControllerState['balances'] = {}; + const balancesToAdd: MultichainBalancesControllerState['balances'] = {}; + + for (const { accountId, added } of accounts) { + if (added.length === 0) { + continue; + } - for (const { accountId, assets } of accounts) { const account = this.#getAccount(accountId); if (account.metadata.snap) { const accountBalance = await this.#getBalances( account.id, account.metadata.snap.id, - assets, + added, ); - balancesToUpdate[accountId] = accountBalance; + + balancesToAdd[accountId] = accountBalance; } } - if (Object.keys(balancesToUpdate).length === 0) { - return; - } + this.update((state: Draft) => { + for (const { accountId, added, removed } of accounts) { + const accountBalances = state.balances[accountId] ?? {}; + const addedBalances = balancesToAdd[accountId] ?? {}; - const accountsMap = new Map(accounts.map((acc) => [acc.accountId, acc])); + state.balances[accountId] = accountBalances; - this.update((state: Draft) => { - for (const [accountId, accountBalances] of Object.entries( - balancesToUpdate, - )) { - if ( - !state.balances[accountId] || - Object.keys(state.balances[accountId]).length === 0 - ) { - state.balances[accountId] = accountBalances; - } else { - const acc = accountsMap.get(accountId); - - const assetsWithoutBalance = new Set(acc?.assets || []); - - for (const assetId of Object.keys(accountBalances)) { - if (!state.balances[accountId][assetId]) { - state.balances[accountId][assetId] = accountBalances[assetId]; - } - assetsWithoutBalance.delete(assetId as CaipAssetType); - } + // Remove balances for assets that disappeared from the account asset list + // so stale entries cannot remain in state. + for (const assetId of removed) { + delete state.balances[accountId][assetId]; + } + + // Merge the balances returned by the snap for the newly added assets. + for (const [assetId, balance] of Object.entries(addedBalances)) { + state.balances[accountId][assetId] = balance; + } - // Triggered when an asset is added to the accountAssets list manually - for (const assetId of assetsWithoutBalance) { + // If the asset list was updated but the snap did not return a balance for + // one of the added assets, keep the asset visible with an explicit zero. + for (const assetId of added) { + if (!state.balances[accountId][assetId]) { state.balances[accountId][assetId] = { amount: '0', unit: '' }; } } From 273821cc7ed200f1f096f736d36e59e9477909c3 Mon Sep 17 00:00:00 2001 From: gabrieledm Date: Wed, 15 Apr 2026 09:50:53 +0200 Subject: [PATCH 02/10] fix: Managed `removed` property from `MultichainAssetsController:accountAssetListUpdated` event body to manage stale balances --- packages/assets-controllers/CHANGELOG.md | 1 + .../MultichainBalancesController.test.ts | 142 ++++++++++++++++++ .../MultichainBalancesController.ts | 81 +++++----- 3 files changed, 187 insertions(+), 37 deletions(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index b1fed9259c5..b74c8916a17 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Restore `checkAndUpdateSingleNftOwnershipStatus` to `NftController` to fix a regression where consumers (e.g. the MetaMask extension) that call this method individually were broken by its removal in [#8281](https://github.com/MetaMask/core/pull/8281) ([#8435](https://github.com/MetaMask/core/pull/8435)) +- Used the `removed` property coming from `MultichainAssetsController:accountAssetListUpdated` event body to manage stale balances ([#8461](https://github.com/MetaMask/core/pull/8461)) ## [103.1.1] diff --git a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts index 1dfd40564c9..84fec30388a 100644 --- a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts +++ b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.test.ts @@ -732,6 +732,148 @@ describe('MultichainBalancesController', () => { }, }); }); + + it('removes stale balances that are no longer present in MultichainAssetsController state', async () => { + const mockSolanaAccountId1 = mockListSolanaAccounts[0].id; + + const existingBalancesState = { + [mockSolanaAccountId1]: { + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:removedToken': { + amount: '5.00000000', + unit: 'SOL', + }, + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken': { + amount: '6.00000000', + unit: 'SOL', + }, + }, + }; + + const { + controller, + messenger, + mockGetAssetsState, + mockSnapHandleRequest, + mockListMultichainAccounts, + } = setupController({ + state: { + balances: existingBalancesState, + }, + mocks: { + handleMockGetAssetsState: { + accountsAssets: { + [mockSolanaAccountId1]: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:removedToken', + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken', + ], + }, + }, + handleRequestReturnValue: { + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken': { + amount: '6.00000000', + unit: 'SOL', + }, + }, + listMultichainAccounts: [], + }, + }); + + mockSnapHandleRequest.mockReset(); + mockListMultichainAccounts.mockReset(); + + mockListMultichainAccounts.mockReturnValue(mockListSolanaAccounts); + mockGetAssetsState.mockReturnValue({ + accountsAssets: { + [mockSolanaAccountId1]: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken', + ], + }, + }); + mockSnapHandleRequest.mockResolvedValueOnce({ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken': { + amount: '6.00000000', + unit: 'SOL', + }, + }); + + messenger.publish('MultichainAssetsController:accountAssetListUpdated', { + assets: { + [mockSolanaAccountId1]: { + added: ['solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken'], + removed: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:removedToken', + ], + }, + }, + }); + + await waitForAllPromises(); + + expect(controller.state.balances).toStrictEqual({ + [mockSolanaAccountId1]: { + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:keptToken': { + amount: '6.00000000', + unit: 'SOL', + }, + }, + }); + }); + + it('clears balances when an account no longer has any assets after the update', async () => { + const mockSolanaAccountId1 = mockListSolanaAccounts[0].id; + + const { + controller, + messenger, + mockGetAssetsState, + mockListMultichainAccounts, + } = setupController({ + state: { + balances: { + [mockSolanaAccountId1]: { + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:removedToken': { + amount: '5.00000000', + unit: 'SOL', + }, + }, + }, + }, + mocks: { + listMultichainAccounts: [], + handleMockGetAssetsState: { + accountsAssets: { + [mockSolanaAccountId1]: [], + }, + }, + handleRequestReturnValue: {}, + }, + }); + + mockGetAssetsState.mockReturnValue({ + accountsAssets: { + [mockSolanaAccountId1]: [], + }, + }); + mockListMultichainAccounts.mockReset(); + mockListMultichainAccounts.mockReturnValue(mockListSolanaAccounts); + + messenger.publish('MultichainAssetsController:accountAssetListUpdated', { + assets: { + [mockSolanaAccountId1]: { + added: [], + removed: [ + 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1/token:removedToken', + ], + }, + }, + }); + + await waitForAllPromises(); + + expect(controller.state.balances).toStrictEqual({ + [mockSolanaAccountId1]: {}, + }); + }); }); it('resumes updating balances after unlocking KeyringController', async () => { diff --git a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts index 88356c68bd3..9a977b92d66 100644 --- a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts +++ b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts @@ -178,26 +178,35 @@ export class MultichainBalancesController extends BaseController< this.messenger.subscribe( 'MultichainAssetsController:accountAssetListUpdated', async ({ assets }) => { - const newAccountAssets = Object.entries(assets).map( - ([accountId, { added }]) => ({ + const updatedAccountAssets = Object.entries(assets).map( + ([accountId, { added, removed }]) => ({ accountId, - assets: [...added], + added: [...added], + removed: [...removed], }), ); - await this.#handleOnAccountAssetListUpdated(newAccountAssets); + + await this.#handleOnAccountAssetListUpdated(updatedAccountAssets); }, ); } /** - * Updates the balances for the given accounts. + * Reconciles cached balances after a multichain asset-list update event. + * + * The event payload is treated as a delta: + * - balances for `removed` assets are deleted so stale entries cannot remain + * - balances for `added` assets are fetched from the snap and merged in + * - if an added asset is not returned by the snap, a zero placeholder is stored + * so the asset can still be represented in state * - * @param accounts - The accounts to update the balances for. + * @param accounts - The per-account asset deltas from the asset-list update event. */ async #handleOnAccountAssetListUpdated( accounts: { accountId: string; - assets: CaipAssetType[]; + added: CaipAssetType[]; + removed: CaipAssetType[]; }[], ): Promise { const { isUnlocked } = this.messenger.call('KeyringController:getState'); @@ -205,49 +214,47 @@ export class MultichainBalancesController extends BaseController< if (!isUnlocked) { return; } - const balancesToUpdate: MultichainBalancesControllerState['balances'] = {}; + const balancesToAdd: MultichainBalancesControllerState['balances'] = {}; + + for (const { accountId, added } of accounts) { + if (added.length === 0) { + continue; + } - for (const { accountId, assets } of accounts) { const account = this.#getAccount(accountId); if (account.metadata.snap) { const accountBalance = await this.#getBalances( account.id, account.metadata.snap.id, - assets, + added, ); - balancesToUpdate[accountId] = accountBalance; + + balancesToAdd[accountId] = accountBalance; } } - if (Object.keys(balancesToUpdate).length === 0) { - return; - } + this.update((state: Draft) => { + for (const { accountId, added, removed } of accounts) { + const accountBalances = state.balances[accountId] ?? {}; + const addedBalances = balancesToAdd[accountId] ?? {}; - const accountsMap = new Map(accounts.map((acc) => [acc.accountId, acc])); + state.balances[accountId] = accountBalances; - this.update((state: Draft) => { - for (const [accountId, accountBalances] of Object.entries( - balancesToUpdate, - )) { - if ( - !state.balances[accountId] || - Object.keys(state.balances[accountId]).length === 0 - ) { - state.balances[accountId] = accountBalances; - } else { - const acc = accountsMap.get(accountId); - - const assetsWithoutBalance = new Set(acc?.assets || []); - - for (const assetId of Object.keys(accountBalances)) { - if (!state.balances[accountId][assetId]) { - state.balances[accountId][assetId] = accountBalances[assetId]; - } - assetsWithoutBalance.delete(assetId as CaipAssetType); - } + // Remove balances for assets that disappeared from the account asset list + // so stale entries cannot remain in state. + for (const assetId of removed) { + delete state.balances[accountId][assetId]; + } + + // Merge the balances returned by the snap for the newly added assets. + for (const [assetId, balance] of Object.entries(addedBalances)) { + state.balances[accountId][assetId] = balance; + } - // Triggered when an asset is added to the accountAssets list manually - for (const assetId of assetsWithoutBalance) { + // If the asset list was updated but the snap did not return a balance for + // one of the added assets, keep the asset visible with an explicit zero. + for (const assetId of added) { + if (!state.balances[accountId][assetId]) { state.balances[accountId][assetId] = { amount: '0', unit: '' }; } } From bc7bc40f86905e712b040d6b8023374453da30a9 Mon Sep 17 00:00:00 2001 From: gabrieledm Date: Wed, 15 Apr 2026 11:25:12 +0200 Subject: [PATCH 03/10] fix: run `yarn lint:eslint --prune-suppressions` --- eslint-suppressions.json | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 13e48bc3e85..c001500084b 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -320,9 +320,6 @@ "@typescript-eslint/no-misused-promises": { "count": 2 }, - "@typescript-eslint/prefer-nullish-coalescing": { - "count": 1 - }, "no-restricted-syntax": { "count": 2 } @@ -2387,4 +2384,4 @@ "count": 10 } } -} +} \ No newline at end of file From 0ecdfea625e25ea83eb5cb3ca72a59efc89dcdf3 Mon Sep 17 00:00:00 2001 From: gabrieledm Date: Wed, 15 Apr 2026 11:45:47 +0200 Subject: [PATCH 04/10] fix: format `eslint-suppressions.json` Signed-off-by: gabrieledm --- eslint-suppressions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index c001500084b..01c6e368336 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -2384,4 +2384,4 @@ "count": 10 } } -} \ No newline at end of file +} From e5bc4d4dae36650b96aa150ef9d22f72c7403964 Mon Sep 17 00:00:00 2001 From: gabrieledm Date: Wed, 15 Apr 2026 15:50:00 +0200 Subject: [PATCH 05/10] fix: CHANGELOG structure for assets-controllers --- packages/assets-controllers/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 90915e3b0c3..4c8672fb1ac 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Used the `removed` property coming from `MultichainAssetsController:accountAssetListUpdated` event body to manage stale balances ([#8461](https://github.com/MetaMask/core/pull/8461)) + ## [104.0.0] ### Changed @@ -27,7 +31,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Restore `checkAndUpdateSingleNftOwnershipStatus` to `NftController` to fix a regression where consumers (e.g. the MetaMask extension) that call this method individually were broken by its removal in [#8281](https://github.com/MetaMask/core/pull/8281) ([#8435](https://github.com/MetaMask/core/pull/8435)) -- Used the `removed` property coming from `MultichainAssetsController:accountAssetListUpdated` event body to manage stale balances ([#8461](https://github.com/MetaMask/core/pull/8461)) ## [103.1.1] From ef10345091e1dd4be760d0e0017a37fdba41e0e1 Mon Sep 17 00:00:00 2001 From: gabrieledm Date: Thu, 16 Apr 2026 15:57:28 +0200 Subject: [PATCH 06/10] fix: added clean up of stale assets also in MultichainAssetsRatesController --- .../MultichainAssetsRatesController.test.ts | 200 +++++++++++++++--- .../MultichainAssetsRatesController.ts | 82 +++++-- .../MultichainBalancesController.ts | 1 + 3 files changed, 242 insertions(+), 41 deletions(-) diff --git a/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.test.ts b/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.test.ts index e976f15651c..0921f36761d 100644 --- a/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.test.ts +++ b/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.test.ts @@ -150,41 +150,43 @@ const setupController = ({ controller: MultichainAssetsRatesController; messenger: RootMessenger; updateSpy: jest.SpyInstance; + mockGetAssetsState: jest.Mock; } => { const messenger: RootMessenger = new Messenger({ namespace: MOCK_ANY_NAMESPACE, }); - messenger.registerActionHandler( - 'MultichainAssetsController:getState', - () => ({ - accountsAssets: { - account1: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'], - account2: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'], - account3: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'], - account5: [ - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v', - ], + const mockGetAssetsState = jest.fn().mockImplementation(() => ({ + accountsAssets: { + account1: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'], + account2: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'], + account3: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'], + account5: [ + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v', + ], + }, + assetsMetadata: { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { + name: 'Solana', + symbol: 'SOL', + fungible: true, + iconUrl: 'https://example.com/solana.png', + units: [{ symbol: 'SOL', name: 'Solana', decimals: 9 }], }, - assetsMetadata: { - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { - name: 'Solana', - symbol: 'SOL', + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v': + { + name: 'USDC', + symbol: 'USDC', fungible: true, - iconUrl: 'https://example.com/solana.png', - units: [{ symbol: 'SOL', name: 'Solana', decimals: 9 }], + iconUrl: 'https://example.com/usdc.png', + units: [{ symbol: 'USDC', name: 'USDC', decimals: 2 }], }, - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v': - { - name: 'USDC', - symbol: 'USDC', - fungible: true, - iconUrl: 'https://example.com/usdc.png', - units: [{ symbol: 'USDC', name: 'USDC', decimals: 2 }], - }, - }, - allIgnoredAssets: {}, - }), + }, + allIgnoredAssets: {}, + })); + messenger.registerActionHandler( + 'MultichainAssetsController:getState', + mockGetAssetsState, ); messenger.registerActionHandler( @@ -240,6 +242,7 @@ const setupController = ({ controller, messenger, updateSpy, + mockGetAssetsState, }; }; @@ -524,6 +527,149 @@ describe('MultichainAssetsRatesController', () => { }); }); + it('removes stale rates and historical prices for assets no longer tracked by any account', async () => { + const removedAsset = + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:removed-token' as CaipAssetType; + const keptAsset = + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501' as CaipAssetType; + const { controller, messenger, mockGetAssetsState } = setupController({ + config: { + state: { + conversionRates: { + [removedAsset]: { + rate: '10', + conversionTime: 1738539923277, + currency: 'swift:0/iso4217:USD', + }, + [keptAsset]: { + rate: '202.11', + conversionTime: 1738539923277, + currency: 'swift:0/iso4217:USD', + }, + }, + historicalPrices: { + [removedAsset]: { + USD: { + intervals: fakeHistoricalPrices.historicalPrice.intervals, + updateTime: 1737542312, + expirationTime: 1737542312, + }, + }, + [keptAsset]: { + USD: { + intervals: fakeHistoricalPrices.historicalPrice.intervals, + updateTime: 1737542312, + expirationTime: 1737542312, + }, + }, + }, + }, + }, + }); + + mockGetAssetsState.mockReturnValue({ + accountsAssets: { + account1: [keptAsset], + }, + assetsMetadata: {}, + allIgnoredAssets: {}, + }); + + messenger.publish('MultichainAssetsController:accountAssetListUpdated', { + assets: { + account1: { + added: [], + removed: [removedAsset], + }, + }, + }); + + await Promise.resolve(); + await jestAdvanceTime({ duration: 10 }); + + expect(controller.state.conversionRates).toStrictEqual({ + [keptAsset]: { + rate: '202.11', + conversionTime: 1738539923277, + currency: 'swift:0/iso4217:USD', + }, + }); + expect(controller.state.historicalPrices).toStrictEqual({ + [keptAsset]: { + USD: { + intervals: fakeHistoricalPrices.historicalPrice.intervals, + updateTime: 1737542312, + expirationTime: 1737542312, + }, + }, + }); + }); + + it('keeps rates for removed assets that are still tracked by another account', async () => { + const sharedAsset = + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/token:shared-token' as CaipAssetType; + const { controller, messenger, mockGetAssetsState } = setupController({ + config: { + state: { + conversionRates: { + [sharedAsset]: { + rate: '77', + conversionTime: 1738539923277, + currency: 'swift:0/iso4217:USD', + }, + }, + historicalPrices: { + [sharedAsset]: { + USD: { + intervals: fakeHistoricalPrices.historicalPrice.intervals, + updateTime: 1737542312, + expirationTime: 1737542312, + }, + }, + }, + }, + }, + }); + + mockGetAssetsState.mockReturnValue({ + accountsAssets: { + account1: [], + account2: [sharedAsset], + }, + assetsMetadata: {}, + allIgnoredAssets: {}, + }); + + messenger.publish('MultichainAssetsController:accountAssetListUpdated', { + assets: { + account1: { + added: [], + removed: [sharedAsset], + }, + }, + }); + + await Promise.resolve(); + await jestAdvanceTime({ duration: 10 }); + + expect(controller.state.conversionRates).toStrictEqual({ + [sharedAsset]: { + rate: '77', + conversionTime: 1738539923277, + currency: 'swift:0/iso4217:USD', + }, + }); + expect(controller.state.historicalPrices).toStrictEqual({ + [sharedAsset]: { + USD: { + intervals: fakeHistoricalPrices.historicalPrice.intervals, + updateTime: 1737542312, + expirationTime: 1737542312, + }, + }, + }); + }); + it('handles partial or empty Snap responses gracefully', async () => { const { controller, messenger } = setupController(); diff --git a/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts b/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts index 8cfe2762b24..b373c99fda6 100644 --- a/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts +++ b/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts @@ -260,14 +260,16 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro 'MultichainAssetsController:accountAssetListUpdated', // eslint-disable-next-line @typescript-eslint/no-misused-promises async ({ assets }) => { - const newAccountAssets = Object.entries(assets).map( - ([accountId, { added }]) => ({ + // Treat the event payload as a per-account delta so we can fetch only + // newly added assets and independently clean up removed ones. + const updatedAccountAssets = Object.entries(assets).map( + ([accountId, { added, removed }]) => ({ accountId, - assets: [...added], + added: [...added], + removed: [...removed], }), ); - // TODO; removed can be used in future for further cleanup - await this.#updateAssetsRatesForNewAssets(newAccountAssets); + await this.#updateAssetsRatesForNewAssets(updatedAccountAssets); }, ); } @@ -622,15 +624,23 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro } /** - * Updates the conversion rates for new assets. + * Reconciles cached rates after an account asset-list update event. * - * @param accounts - The accounts to update the conversion rates for. + * The event payload is treated as a delta: + * - `added` assets are batched by Snap and fetched for fresh rates + * - `removed` assets are deleted from cached state only if they are no longer tracked by any account + * + * This global check is required because rate state is keyed by asset rather + * than by account, so the same asset may still be shared by another account. + * + * @param accounts - The per-account asset deltas from the asset-list update event. * @returns A promise that resolves when the rates are updated. */ async #updateAssetsRatesForNewAssets( accounts: { accountId: string; - assets: CaipAssetType[]; + added: CaipAssetType[]; + removed: CaipAssetType[]; }[], ): Promise { const releaseLock = await this.#mutex.acquire(); @@ -643,16 +653,31 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro // First build a map containing all assets that need to be updated per // Snap ID, this will be used to batch the requests. const snapIdToAssets = new Map>(); + const removedAssets = new Set(); - for (const { accountId, assets } of accounts) { + for (const { accountId, added, removed } of accounts) { + // Only newly added assets need fresh rate requests. this.#addAssetsToSnapIdMap( snapIdToAssets, this.#getAccount(accountId), - assets, + added, ); + + // Collect removed assets separately so we can decide later whether they + // are truly stale or still referenced by another account. + for (const asset of removed) { + removedAssets.add(asset); + } } - this.#applyUpdatedRates(await this.#getUpdatedRatesFor(snapIdToAssets)); + const updatedRates = await this.#getUpdatedRatesFor(snapIdToAssets); + // Rates are stored globally by asset, so delete only assets that no + // longer exist in any account asset list. + const assetsToDelete = Array.from(removedAssets).filter( + (asset) => !this.#isAssetTracked(asset), + ); + + this.#applyUpdatedRates(updatedRates, assetsToDelete); })().finally(() => { releaseLock(); }); @@ -693,20 +718,30 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro } /** - * Merges the new rates into the controller's state. + * Applies fresh rates and removes stale asset-rate state in one update. * - * @param updatedRates - The new rates to merge. + * @param updatedRates - The latest conversion rates fetched for added assets. + * @param removedAssets - Assets that should be purged because they are no longer tracked by any account. */ #applyUpdatedRates( updatedRates: Record< CaipAssetType, UnifiedAssetConversion & { currency: CaipAssetType } >, + removedAssets: CaipAssetType[] = [], ): void { - if (Object.keys(updatedRates).length === 0) { + if (Object.keys(updatedRates).length === 0 && removedAssets.length === 0) { return; } this.update((state: Draft) => { + // Drop both current rates and historical prices for assets that are no + // longer referenced anywhere in MultichainAssetsController state. + for (const asset of removedAssets) { + delete state.conversionRates[asset]; + delete state.historicalPrices[asset]; + } + + // Merge the freshly fetched rates after cleanup. state.conversionRates = { ...state.conversionRates, ...updatedRates, @@ -714,6 +749,25 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro }); } + /** + * Checks whether an asset is still tracked by any account in + * MultichainAssetsController state. + * + * @param asset - The asset to check. + * @returns True if the asset still exists in any account asset list. + */ + #isAssetTracked(asset: CaipAssetType): boolean { + const { accountsAssets } = this.messenger.call( + 'MultichainAssetsController:getState', + ); + + // Rate state is global per asset, so inspect all account asset lists before + // deciding whether a removed asset should be purged from cache. + return Object.values(accountsAssets ?? {}).some((accountAssets) => + accountAssets.includes(asset), + ); + } + /** * Forwards a Snap request to the SnapController. * diff --git a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts index 9a977b92d66..a8c58ed4a61 100644 --- a/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts +++ b/packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts @@ -177,6 +177,7 @@ export class MultichainBalancesController extends BaseController< this.messenger.subscribe( 'MultichainAssetsController:accountAssetListUpdated', + // eslint-disable-next-line @typescript-eslint/no-misused-promises async ({ assets }) => { const updatedAccountAssets = Object.entries(assets).map( ([accountId, { added, removed }]) => ({ From c5fa690db259324f628265c284f5682618c1335c Mon Sep 17 00:00:00 2001 From: gabrieledm Date: Thu, 16 Apr 2026 16:05:51 +0200 Subject: [PATCH 07/10] fix: run eslint:fix --- eslint-suppressions.json | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 31573dc17a5..7e981682450 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -305,7 +305,7 @@ }, "packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts": { "@typescript-eslint/no-misused-promises": { - "count": 2 + "count": 1 }, "no-restricted-syntax": { "count": 2 @@ -2245,11 +2245,6 @@ "count": 1 } }, - "packages/transaction-pay-controller/src/utils/source-amounts.ts": { - "import-x/no-relative-packages": { - "count": 1 - } - }, "packages/transaction-pay-controller/src/utils/transaction.ts": { "no-restricted-syntax": { "count": 2 From 53ec52ea66c7382bde8da342729bd5525d3bf463 Mon Sep 17 00:00:00 2001 From: gabrieledm Date: Thu, 16 Apr 2026 16:05:51 +0200 Subject: [PATCH 08/10] fix: run eslint:fix --- eslint-suppressions.json | 7 +------ .../transaction-pay-controller/src/utils/source-amounts.ts | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 31573dc17a5..7e981682450 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -305,7 +305,7 @@ }, "packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts": { "@typescript-eslint/no-misused-promises": { - "count": 2 + "count": 1 }, "no-restricted-syntax": { "count": 2 @@ -2245,11 +2245,6 @@ "count": 1 } }, - "packages/transaction-pay-controller/src/utils/source-amounts.ts": { - "import-x/no-relative-packages": { - "count": 1 - } - }, "packages/transaction-pay-controller/src/utils/transaction.ts": { "no-restricted-syntax": { "count": 2 diff --git a/packages/transaction-pay-controller/src/utils/source-amounts.ts b/packages/transaction-pay-controller/src/utils/source-amounts.ts index a8dcd1e302b..311fccd1c46 100644 --- a/packages/transaction-pay-controller/src/utils/source-amounts.ts +++ b/packages/transaction-pay-controller/src/utils/source-amounts.ts @@ -1,4 +1,5 @@ import { TransactionType } from '@metamask/transaction-controller'; +import type { TransactionMeta } from '@metamask/transaction-controller/src/src'; import { createModuleLogger } from '@metamask/utils'; import { BigNumber } from 'bignumber.js'; @@ -7,7 +8,6 @@ import type { TransactionPaymentToken, } from '..'; import { TransactionPayStrategy } from '..'; -import type { TransactionMeta } from '../../../transaction-controller/src'; import { ARBITRUM_USDC_ADDRESS, CHAIN_ID_ARBITRUM } from '../constants'; import { projectLogger } from '../logger'; import type { From c395c1677e32a308938d4b459165e634b2a17df8 Mon Sep 17 00:00:00 2001 From: gabrieledm Date: Thu, 16 Apr 2026 18:06:49 +0200 Subject: [PATCH 09/10] fix: TransactionMeta import Signed-off-by: gabrieledm --- packages/transaction-pay-controller/src/utils/source-amounts.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/transaction-pay-controller/src/utils/source-amounts.ts b/packages/transaction-pay-controller/src/utils/source-amounts.ts index 311fccd1c46..3054d60587f 100644 --- a/packages/transaction-pay-controller/src/utils/source-amounts.ts +++ b/packages/transaction-pay-controller/src/utils/source-amounts.ts @@ -1,5 +1,5 @@ import { TransactionType } from '@metamask/transaction-controller'; -import type { TransactionMeta } from '@metamask/transaction-controller/src/src'; +import type { TransactionMeta } from '@metamask/transaction-controller'; import { createModuleLogger } from '@metamask/utils'; import { BigNumber } from 'bignumber.js'; From 891fa43c2784341b77ad5e3dbcd8e077a321e551 Mon Sep 17 00:00:00 2001 From: gabrieledm Date: Thu, 16 Apr 2026 18:19:12 +0200 Subject: [PATCH 10/10] fix: guard against empty `added` assets Signed-off-by: gabrieledm --- .../MultichainAssetsRatesController.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts b/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts index b373c99fda6..cec100709ea 100644 --- a/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts +++ b/packages/assets-controllers/src/MultichainAssetsRatesController/MultichainAssetsRatesController.ts @@ -656,12 +656,14 @@ export class MultichainAssetsRatesController extends StaticIntervalPollingContro const removedAssets = new Set(); for (const { accountId, added, removed } of accounts) { - // Only newly added assets need fresh rate requests. - this.#addAssetsToSnapIdMap( - snapIdToAssets, - this.#getAccount(accountId), - added, - ); + if (added.length !== 0) { + // Only newly added assets need fresh rate requests. + this.#addAssetsToSnapIdMap( + snapIdToAssets, + this.#getAccount(accountId), + added, + ); + } // Collect removed assets separately so we can decide later whether they // are truly stale or still referenced by another account.