[pull] main from MetaMask:main

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@metamask/core-monorepo",
-  "version": "381.0.0",
+  "version": "382.0.0",
   "private": true,
   "description": "Monorepo for packages shared between MetaMask clients",
   "repository": {

packages/accounts-controller/CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - Add new `setAccountNameAndSelectAccount` action ([#5714](https://github.com/MetaMask/core/pull/5714))
+- Add `entropySource` and `derivationPath` to EVM HD account options ([#5618](https://github.com/MetaMask/core/pull/5618))
 
 ### Changed
 
@@ -18,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **BREAKING:** Bump `@metamask/providers` peer dependency from ^18.1.0 to ^21.0.0 ([#5639](https://github.com/MetaMask/core/pull/5639))
 - Bump `@metamask/snaps-sdk` from ^6.17.1 to ^6.22.0 ([#5639](https://github.com/MetaMask/core/pull/5639))
 - Bump `@metamask/snaps-utils` from ^8.10.0 to ^9.2.0 ([#5639](https://github.com/MetaMask/core/pull/5639))
+- Prevent unnecasary state updates when updating `InternalAccount.metadata.snap` ([#5735](https://github.com/MetaMask/core/pull/5735))
 
 ## [27.0.0]
 

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

@@ -17,7 +17,7 @@
 import type { SnapControllerState } from '@metamask/snaps-controllers';
 import { SnapStatus } from '@metamask/snaps-utils';
 import type { CaipChainId } from '@metamask/utils';
 import * as uuid from 'uuid';
 import type { V4Options } from 'uuid';
 
 import type {
@@ -31,6 +31,7 @@
 import {
   createExpectedInternalAccount,
   createMockInternalAccount,
+  createMockInternalAccountOptions,
   ETH_EOA_METHODS,
 } from './tests/mocks';
 import {
@@ -279,7 +280,7 @@
   });
 
   describe('onSnapStateChange', () => {
-    it('be used enable an account if the Snap is enabled and not blocked', async () => {
+    it('enables an account if the Snap is enabled and not blocked', async () => {
       const messenger = buildMessenger();
       const mockSnapAccount = createMockInternalAccount({
         id: 'mock-id',
@@ -325,7 +326,7 @@
       expect(updatedAccount.metadata.snap?.enabled).toBe(true);
     });
 
-    it('be used disable an account if the Snap is disabled', async () => {
+    it('disables an account if the Snap is disabled', async () => {
       const messenger = buildMessenger();
       const mockSnapAccount = createMockInternalAccount({
         id: 'mock-id',
@@ -371,7 +372,7 @@
       expect(updatedAccount.metadata.snap?.enabled).toBe(false);
     });
 
-    it('be used disable an account if the Snap is blocked', async () => {
+    it('disables an account if the Snap is blocked', async () => {
       const messenger = buildMessenger();
       const mockSnapAccount = createMockInternalAccount({
         id: 'mock-id',
@@ -416,6 +417,61 @@
 
       expect(updatedAccount.metadata.snap?.enabled).toBe(false);
     });
+
+    it('does not trigger any unnecessary updates', async () => {
+      const messenger = buildMessenger();
+      const mockSnapAccount = createMockInternalAccount({
+        id: 'mock-id',
+        name: 'Snap Account 1',
+        address: '0x0',
+        keyringType: KeyringTypes.snap,
+        snap: {
+          id: 'mock-snap',
+          name: 'mock-snap-name',
+          enabled: false, // Will be enabled later by a `SnapController:stateChange`.
+        },
+      });
+      const mockSnapChangeState = {
+        snaps: {
+          'mock-snap': {
+            enabled: true,
+            id: 'mock-snap',
+            blocked: false,
+            status: SnapStatus.Running,
+          },
+        },
+        // TODO: Replace `any` with type
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      } as any as SnapControllerState;
+      const mockStateChange = jest.fn();
+      const { accountsController } = setupAccountsController({
+        initialState: {
+          internalAccounts: {
+            accounts: {
+              [mockSnapAccount.id]: mockSnapAccount,
+            },
+            selectedAccount: mockSnapAccount.id,
+          },
+        },
+        messenger,
+      });
+
+      messenger.subscribe('AccountsController:stateChange', mockStateChange);
+
+      // First update will update the account's metadata, thus triggering a `AccountsController:stateChange`.
+      messenger.publish('SnapController:stateChange', mockSnapChangeState, []);
+      const updatedAccount = accountsController.getAccountExpect(
+        mockSnapAccount.id,
+      );
+      expect(updatedAccount.metadata.snap?.enabled).toBe(true);
+      expect(mockStateChange).toHaveBeenCalled();
+
+      // Second update is the same, thus the account does not need any update, and SHOULD NOT trigger a `AccountsController:stateChange`.
+      mockStateChange.mockReset();
+      messenger.publish('SnapController:stateChange', mockSnapChangeState, []);
+      expect(updatedAccount.metadata.snap?.enabled).toBe(true);
+      expect(mockStateChange).not.toHaveBeenCalled();
+    });
   });
 
   describe('onKeyringStateChange', () => {
@@ -901,6 +957,7 @@
             name: 'Account 3',
             address: mockAccount3.address,
             keyringType: KeyringTypes.hd,
+            options: {},
           }),
         ]);
       });
@@ -1744,6 +1801,12 @@
           keyrings: [
             { type: KeyringTypes.hd, accounts: [mockAddress1, mockAddress2] },
           ],
+          keyringsMetadata: [
+            {
+              id: 'mock-keyring-id-0',
+              name: 'mock-keyring-id-name',
+            },
+          ],
         }),
       );
 
@@ -1772,12 +1835,14 @@
           id: 'mock-id',
           address: mockAddress1,
           keyringType: KeyringTypes.hd,
+          options: createMockInternalAccountOptions(0, KeyringTypes.hd, 0),
         }),
         createExpectedInternalAccount({
           name: 'Account 2',
           id: 'mock-id2',
           address: mockAddress2,
           keyringType: KeyringTypes.hd,
+          options: createMockInternalAccountOptions(0, KeyringTypes.hd, 1),
         }),
       ];
       mockUUIDWithNormalAccounts(expectedAccounts);
@@ -1800,6 +1865,12 @@
               accounts: [mockSnapAccount, mockSnapAccount2],
             },
           ],
+          keyringsMetadata: [
+            {
+              id: 'mock-keyring-id-1',
+              name: 'mock-keyring-id-name',
+            },
+          ],
         }),
       );
 
@@ -1893,6 +1964,12 @@
           keyrings: [
             { type: KeyringTypes.hd, accounts: [mockAddress1, mockAddress2] },
           ],
+          keyringsMetadata: [
+            {
+              id: 'mock-keyring-id-0',
+              name: 'mock-keyring-id-name',
+            },
+          ],
         }),
       );
 
@@ -1918,12 +1995,16 @@
         messenger,
       });
       const expectedAccounts = [
-        mockAccount,
+        {
+          ...mockAccount,
+          options: createMockInternalAccountOptions(0, KeyringTypes.hd, 0),
+        },
         createExpectedInternalAccount({
           name: 'Account 2',
           id: 'mock-id2',
           address: mockAddress2,
           keyringType: KeyringTypes.hd,
+          options: createMockInternalAccountOptions(0, KeyringTypes.hd, 1),
         }),
       ];
       mockUUIDWithNormalAccounts(expectedAccounts);
@@ -1957,6 +2038,16 @@
             { type: KeyringTypes.hd, accounts: [mockAddress1] },
             { type: KeyringTypes.snap, accounts: ['0x1234'] },
           ],
+          keyringsMetadata: [
+            {
+              id: 'mock-keyring-id-0',
+              name: 'mock-keyring-id-name',
+            },
+            {
+              id: 'mock-keyring-id-1',
+              name: 'mock-keyring-id-name2',
+            },
+          ],
         }),
       );
 
@@ -1975,6 +2066,7 @@
           id: 'mock-id',
           address: mockAddress1,
           keyringType: KeyringTypes.hd,
+          options: createMockInternalAccountOptions(0, KeyringTypes.hd, 0),
         }),
         createExpectedInternalAccount({
           name: 'Snap Account 1', // it is Snap Account 1 because it is the only snap account
@@ -2014,6 +2106,16 @@
             { type: KeyringTypes.snap, accounts: ['0x1234'] },
             { type: KeyringTypes.hd, accounts: [mockAddress1] },
           ],
+          keyringsMetadata: [
+            {
+              id: 'mock-keyring-id-0',
+              name: 'mock-keyring-id-name',
+            },
+            {
+              id: 'mock-keyring-id-1',
+              name: 'mock-keyring-id-name2',
+            },
+          ],
         }),
       );
 
@@ -2032,6 +2134,7 @@
           id: 'mock-id',
           address: mockAddress1,
           keyringType: KeyringTypes.hd,
+          options: createMockInternalAccountOptions(1, KeyringTypes.hd, 0),
         }),
         createExpectedInternalAccount({
           name: 'Snap Account 1', // it is Snap Account 1 because it is the only snap account
@@ -2057,7 +2160,6 @@
       KeyringTypes.ledger,
       KeyringTypes.lattice,
       KeyringTypes.qr,
-      'Custody - JSON - RPC',
     ])('should add accounts for %s type', async (keyringType) => {
       mockUUIDWithNormalAccounts([mockAccount]);
 
@@ -2067,6 +2169,12 @@
         'KeyringController:getState',
         mockGetState.mockReturnValue({
           keyrings: [{ type: keyringType, accounts: [mockAddress1] }],
+          keyringsMetadata: [
+            {
+              id: 'mock-keyring-id-0',
+              name: 'mock-keyring-id-name',
+            },
+          ],
         }),
       );
 
@@ -2096,6 +2204,7 @@
           id: 'mock-id',
           address: mockAddress1,
           keyringType: keyringType as KeyringTypes,
+          options: createMockInternalAccountOptions(0, keyringType, 0),
         }),
       ];
 
@@ -2206,6 +2315,16 @@
               { type: KeyringTypes.snap, accounts: ['0x1234'] },
               { type: KeyringTypes.hd, accounts: [mockAddress1] },
             ],
+            keyringsMetadata: [
+              {
+                id: 'mock-keyring-id-1',
+                name: 'mock-keyring-id-name',
+              },
+              {
+                id: 'mock-keyring-id-2',
+                name: 'mock-keyring-id-name2',
+              },
+            ],
           }),
         );
 

packages/accounts-controller/src/AccountsController.ts

@@ -38,7 +38,9 @@ import type { WritableDraft } from 'immer/dist/internal.js';
 
 import type { MultichainNetworkControllerNetworkDidChangeEvent } from './types';
 import {
+  getDerivationPathForIndex,
   getUUIDFromAddressOfNormalAccount,
+  isHdKeyringType,
   isNormalKeyringType,
   keyringTypeToName,
 } from './utils';
@@ -682,19 +684,32 @@ export class AccountsController extends BaseController<
    */
   async #listNormalAccounts(): Promise<InternalAccount[]> {
     const internalAccounts: InternalAccount[] = [];
-    const { keyrings } = await this.messagingSystem.call(
+    const { keyrings, keyringsMetadata } = this.messagingSystem.call(
       'KeyringController:getState',
     );
-    for (const keyring of keyrings) {
+
+    for (const [keyringIndex, keyring] of keyrings.entries()) {
       const keyringType = keyring.type;
       if (!isNormalKeyringType(keyringType as KeyringTypes)) {
         // We only consider "normal accounts" here, so keep looping
         continue;
       }
 
-      for (const address of keyring.accounts) {
+      for (const [accountIndex, address] of keyring.accounts.entries()) {
         const id = getUUIDFromAddressOfNormalAccount(address);
 
+        let options = {};
+
+        if (isHdKeyringType(keyring.type as KeyringTypes)) {
+          options = {
+            entropySource: keyringsMetadata[keyringIndex].id,
+            // NOTE: We are not using the `hdPath` from the associated keyring here and
+            // getting the keyring instance here feels a bit overkill.
+            // This will be naturally fixed once every keyring start using `KeyringAccount` and implement the keyring API.
+            derivationPath: getDerivationPathForIndex(accountIndex),
+          };
+        }
+
         const nameLastUpdatedAt = this.#populateExistingMetadata(
           id,
           'nameLastUpdatedAt',
@@ -703,7 +718,7 @@ export class AccountsController extends BaseController<
         internalAccounts.push({
           id,
           address,
-          options: {},
+          options,
           methods: [
             EthMethod.PersonalSign,
             EthMethod.Sign,
@@ -973,24 +988,31 @@ export class AccountsController extends BaseController<
   #handleOnSnapStateChange(snapState: SnapControllerState) {
     // only check if snaps changed in status
     const { snaps } = snapState;
-    const accounts = this.listMultichainAccounts().filter(
-      (account) => account.metadata.snap,
-    );
 
-    this.update((currentState) => {
-      accounts.forEach((account) => {
-        const currentAccount =
-          currentState.internalAccounts.accounts[account.id];
-        if (currentAccount.metadata.snap) {
-          const snapId = currentAccount.metadata.snap.id;
-          const storedSnap: Snap = snaps[snapId as SnapId];
-          if (storedSnap) {
-            currentAccount.metadata.snap.enabled =
-              storedSnap.enabled && !storedSnap.blocked;
+    const accounts: { id: string; enabled: boolean }[] = [];
+    for (const account of this.listMultichainAccounts()) {
+      if (account.metadata.snap) {
+        const snap: Snap = snaps[account.metadata.snap.id as SnapId];
+        const enabled = snap.enabled && !snap.blocked;
+        const metadata = account.metadata.snap;
+
+        if (metadata.enabled !== enabled) {
+          accounts.push({ id: account.id, enabled });
+        }
+      }
+    }
+
+    if (accounts.length > 0) {
+      this.update((state) => {
+        for (const { id, enabled } of accounts) {
+          const account = state.internalAccounts.accounts[id];
+
+          if (account.metadata.snap) {
+            account.metadata.snap.enabled = enabled;
           }
         }
       });
-    });
+    }
   }
 
   /**