fix: publish address along with id in AccountsController:accountRemoved

Author: sahar-fehri

packages/accounts-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add account address in payload for `AccountsController:accountRemoved` event ([#5859](https://github.com/MetaMask/core/pull/5859))
+
 ### Fixed
 
 - Populate `.options.entropySource` for new `InternalAccount`s before publishing `:accountAdded` ([#5841](https://github.com/MetaMask/core/pull/5841))

packages/accounts-controller/src/AccountsController.test.ts

@@ -1417,7 +1417,7 @@ describe('AccountsController', () => {
           // 2. AccountsController:stateChange
           3,
           'AccountsController:accountRemoved',
-          mockAccount3.id,
+          { id: mockAccount3.id, address: mockAccount3.address },
         );
       });
     });

packages/accounts-controller/src/AccountsController.ts

@@ -162,7 +162,7 @@ export type AccountsControllerAccountAddedEvent = {
 
 export type AccountsControllerAccountRemovedEvent = {
   type: `${typeof controllerName}:accountRemoved`;
-  payload: [AccountId];
+  payload: [{ id: string; address: string }];
 };
 
 export type AccountsControllerAccountRenamedEvent = {
@@ -862,8 +862,12 @@ export class AccountsController extends BaseController<
     }
 
     // Diff that we will use to publish events afterward.
+    type diffItem = {
+      id: string;
+      address: string;
+    };
     const diff = {
-      removed: [] as string[],
+      removed: [] as diffItem[],
       added: [] as InternalAccount[],
     };
 
@@ -874,7 +878,10 @@ export class AccountsController extends BaseController<
         for (const account of patch.removed) {
           delete internalAccounts.accounts[account.id];
 
-          diff.removed.push(account.id);
+          diff.removed.push({
+            id: account.id,
+            address: account.address,
+          });
         }
 
         for (const added of patch.added) {
@@ -921,8 +928,11 @@ export class AccountsController extends BaseController<
     });
 
     // Now publish events
-    for (const id of diff.removed) {
-      this.messagingSystem.publish('AccountsController:accountRemoved', id);
+    for (const item of diff.removed) {
+      this.messagingSystem.publish('AccountsController:accountRemoved', {
+        id: item.id,
+        address: item.address,
+      });
     }
 
     for (const account of diff.added) {

packages/assets-controllers/src/MultichainAssetsController/MultichainAssetsController.test.ts

@@ -534,7 +534,10 @@ describe('MultichainAssetsController', () => {
       assetsMetadata: mockGetMetadataReturnValue.assets,
     });
     // Remove an EVM account
-    messenger.publish('AccountsController:accountRemoved', mockEthAccount.id);
+    messenger.publish('AccountsController:accountRemoved', {
+      id: mockEthAccount.id,
+      address: mockEthAccount.address,
+    });
 
     await advanceTime({ clock, duration: 1 });
 
@@ -580,10 +583,10 @@ describe('MultichainAssetsController', () => {
       assetsMetadata: mockGetMetadataReturnValue.assets,
     });
     // Remove the added solana account
-    messenger.publish(
-      'AccountsController:accountRemoved',
-      mockSolanaAccount.id,
-    );
+    messenger.publish('AccountsController:accountRemoved', {
+      id: mockSolanaAccount.id,
+      address: mockSolanaAccount.address,
+    });
 
     await advanceTime({ clock, duration: 1 });
 

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

@@ -204,7 +204,7 @@ export class MultichainAssetsController extends BaseController<
     );
     this.messagingSystem.subscribe(
       'AccountsController:accountRemoved',
-      async (account) => await this.#handleOnAccountRemovedEvent(account),
+      async ({ id }) => await this.#handleOnAccountRemovedEvent(id),
     );
     this.messagingSystem.subscribe(
       'AccountsController:accountAssetListUpdated',

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

@@ -272,7 +272,10 @@ describe('MultichainBalancesController', () => {
       },
     });
 
-    messenger.publish('AccountsController:accountRemoved', mockBtcAccount.id);
+    messenger.publish('AccountsController:accountRemoved', {
+      id: mockBtcAccount.id,
+      address: mockBtcAccount.address,
+    });
 
     expect(controller.state).toStrictEqual({
       balances: {},

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

@@ -165,7 +165,7 @@ export class MultichainBalancesController extends BaseController<
 
     this.messagingSystem.subscribe(
       'AccountsController:accountRemoved',
-      (account: string) => this.#handleOnAccountRemoved(account),
+      ({ id }) => this.#handleOnAccountRemoved(id),
     );
     this.messagingSystem.subscribe(
       'AccountsController:accountBalancesUpdated',

packages/assets-controllers/src/TokenBalancesController.test.ts

@@ -747,17 +747,6 @@ describe('TokenBalancesController', () => {
   });
 
   describe('when accountRemoved is published', () => {
-    it('does not update state if account removed is not in the list of accounts', async () => {
-      const { controller, messenger, updateSpy } = setupController();
-
-      messenger.publish(
-        'AccountsController:accountRemoved',
-        '0x0000000000000000000000000000000000000000',
-      );
-
-      expect(controller.state.tokenBalances).toStrictEqual({});
-      expect(updateSpy).toHaveBeenCalledTimes(0);
-    });
     it('removes the balances for the removed account', async () => {
       const chainId = '0x1';
       const accountAddress = '0x0000000000000000000000000000000000000000';
@@ -822,7 +811,10 @@ describe('TokenBalancesController', () => {
         },
       });
 
-      messenger.publish('AccountsController:accountRemoved', account.id);
+      messenger.publish('AccountsController:accountRemoved', {
+        id: account.id,
+        address: account.address,
+      });
 
       await advanceTime({ clock, duration: 1 });
 

packages/assets-controllers/src/TokenBalancesController.ts

@@ -196,7 +196,7 @@ export class TokenBalancesController extends StaticIntervalPollingController<Tok
 
     this.messagingSystem.subscribe(
       'AccountsController:accountRemoved',
-      (accountId: string) => this.#handleOnAccountRemoved(accountId),
+      ({ address }) => this.#handleOnAccountRemoved(address),
     );
   }
 
@@ -286,19 +286,9 @@ export class TokenBalancesController extends StaticIntervalPollingController<Tok
   /**
    * Handles changes when an account has been removed.
    *
-   * @param accountId - The account id being removed.
+   * @param accountAddress - The account address being removed.
    */
-  #handleOnAccountRemoved(accountId: string) {
-    const accounts = this.messagingSystem.call(
-      'AccountsController:listAccounts',
-    );
-    const accountAddress = accounts.find(
-      (account) => account.id === accountId,
-    )?.address;
-    if (!accountAddress) {
-      return;
-    }
-
+  #handleOnAccountRemoved(accountAddress: string) {
     this.update((state) => {
       delete state.tokenBalances[accountAddress as `0x${string}`];
     });

packages/assets-controllers/src/TokensController.test.ts

@@ -3393,7 +3393,7 @@ describe('TokensController', () => {
         ({ controller, triggerAccountRemoved }) => {
           expect(controller.state).toStrictEqual(initialState);
 
-          triggerAccountRemoved(firstAccount.id);
+          triggerAccountRemoved(firstAccount.id, firstAccount.address);
 
           expect(controller.state).toStrictEqual({
             allTokens: {
@@ -3462,7 +3462,7 @@ describe('TokensController', () => {
         ({ controller, triggerAccountRemoved }) => {
           expect(controller.state).toStrictEqual(initialState);
 
-          triggerAccountRemoved(secondAccount.id);
+          triggerAccountRemoved(secondAccount.id, secondAccount.address);
 
           expect(controller.state).toStrictEqual(initialState);
         },
@@ -3486,7 +3486,7 @@ type WithControllerCallback<ReturnValue> = ({
   messenger: UnrestrictedMessenger;
   approvalController: ApprovalController;
   triggerSelectedAccountChange: (internalAccount: InternalAccount) => void;
-  triggerAccountRemoved: (accountId: string) => void;
+  triggerAccountRemoved: (accountId: string, accountAddress: string) => void;
   triggerNetworkStateChange: (
     networkState: NetworkState,
     patches: Patch[],
@@ -3613,8 +3613,11 @@ async function withController<ReturnValue>(
     );
   };
 
-  const triggerAccountRemoved = (accountId: string) => {
-    messenger.publish('AccountsController:accountRemoved', accountId);
+  const triggerAccountRemoved = (accountId: string, accountAddress: string) => {
+    messenger.publish('AccountsController:accountRemoved', {
+      id: accountId,
+      address: accountAddress,
+    });
   };
 
   const changeNetwork = ({

packages/assets-controllers/src/TokensController.ts

@@ -229,8 +229,7 @@ export class TokensController extends BaseController<
 
     this.messagingSystem.subscribe(
       'AccountsController:accountRemoved',
-      (accountAddress: string) =>
-        this.#handleOnAccountRemoved(accountAddress as Hex),
+      ({ address }) => this.#handleOnAccountRemoved(address),
     );
 
     this.messagingSystem.subscribe(
@@ -270,19 +269,8 @@ export class TokensController extends BaseController<
     );
   }
 
-  #handleOnAccountRemoved(accountId: string) {
-    // find the account address in allTokens, allDetectedTokens, allIgnoredTokens
+  #handleOnAccountRemoved(accountAddress: string) {
     const { allTokens, allIgnoredTokens, allDetectedTokens } = this.state;
-    const accounts = this.messagingSystem.call(
-      'AccountsController:listAccounts',
-    );
-    const accountAddress = accounts.find(
-      (account) => account.id === accountId,
-    )?.address;
-
-    if (!accountAddress) {
-      return;
-    }
     const newAllTokens = cloneDeep(allTokens);
     const newAllDetectedTokens = cloneDeep(allDetectedTokens);
     const newAllIgnoredTokens = cloneDeep(allIgnoredTokens);

packages/multichain-transactions-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix getting accountId from payload after subscribing to `AccountsController:accountRemoved` ([#5859](https://github.com/MetaMask/core/pull/5859))
+
 ## [1.0.0]
 
 ### Changed

packages/multichain-transactions-controller/src/MultichainTransactionsController.test.ts

@@ -253,7 +253,10 @@ describe('MultichainTransactionsController', () => {
       lastUpdated: expect.any(Number),
     });
 
-    messenger.publish('AccountsController:accountRemoved', mockBtcAccount.id);
+    messenger.publish('AccountsController:accountRemoved', {
+      id: mockBtcAccount.id,
+      address: mockBtcAccount.address,
+    });
     mockListMultichainAccounts.mockReturnValue([]);
 
     expect(controller.state.nonEvmTransactions).toStrictEqual({});

packages/multichain-transactions-controller/src/MultichainTransactionsController.ts

@@ -205,7 +205,7 @@ export class MultichainTransactionsController extends BaseController<
     );
     this.messagingSystem.subscribe(
       'AccountsController:accountRemoved',
-      (accountId: string) => this.#handleOnAccountRemoved(accountId),
+      ({ id }) => this.#handleOnAccountRemoved(id),
     );
     this.messagingSystem.subscribe(
       'AccountsController:accountTransactionsUpdated',