fix: Managed removed property from MultichainAssetsController:accountAssetListUpdated event body

eslint-suppressions.json

@@ -307,9 +307,6 @@
     "@typescript-eslint/no-misused-promises": {
       "count": 2
     },
-    "@typescript-eslint/prefer-nullish-coalescing": {
-      "count": 1
-    },
     "no-restricted-syntax": {
       "count": 2
     }

packages/assets-controllers/CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Used the `removed` property coming from `MultichainAssetsController:accountAssetListUpdated` event body to manage stale balances ([#8461](https://github.com/MetaMask/core/pull/8461))
 - `MultichainAssetsController`: fungible `token:` assets from automatic detection are no longer added when Blockaid bulk scan fails, returns empty, or omits that address (previously fail open); an explicit non-malicious per-token result from `PhishingController:bulkScanTokens` is now required before add. ([#8400](https://github.com/MetaMask/core/pull/8400))
 
 ## [104.0.0]

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 () => {

packages/assets-controllers/src/MultichainBalancesController/MultichainBalancesController.ts

@@ -178,76 +178,83 @@ 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<void> {
     const { isUnlocked } = this.messenger.call('KeyringController:getState');
 
     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<MultichainBalancesControllerState>) => {
+      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<MultichainBalancesControllerState>) => {
-      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: '' };
           }
         }