Merge branch 'main' into jc/WAPI-728

Author: adonesky1

eslint-warning-thresholds.json

@@ -349,21 +349,10 @@
     "jest/no-conditional-in-test": 1
   },
   "packages/signature-controller/src/SignatureController.ts": {
-    "@typescript-eslint/no-unsafe-enum-comparison": 4
-  },
-  "packages/signature-controller/src/utils/decoding-api.test.ts": {
-    "import-x/order": 1,
-    "jsdoc/tag-lines": 1
-  },
-  "packages/signature-controller/src/utils/decoding-api.ts": {
-    "import-x/order": 1
-  },
-  "packages/signature-controller/src/utils/normalize.test.ts": {
-    "import-x/order": 1
+    "@typescript-eslint/no-unsafe-enum-comparison": 3
   },
   "packages/signature-controller/src/utils/normalize.ts": {
-    "@typescript-eslint/no-unused-vars": 1,
-    "jsdoc/tag-lines": 2
+    "@typescript-eslint/no-unused-vars": 1
   },
   "packages/signature-controller/src/utils/validation.ts": {
     "@typescript-eslint/no-base-to-string": 1,

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@metamask/core-monorepo",
-  "version": "569.0.0",
+  "version": "574.0.0",
   "private": true,
   "description": "Monorepo for packages shared between MetaMask clients",
   "repository": {
@@ -61,8 +61,8 @@
     "@metamask/eslint-config-nodejs": "^14.0.0",
     "@metamask/eslint-config-typescript": "^14.0.0",
     "@metamask/eth-block-tracker": "^12.0.1",
-    "@metamask/eth-json-rpc-provider": "^4.1.8",
-    "@metamask/json-rpc-engine": "^10.0.3",
+    "@metamask/eth-json-rpc-provider": "^5.0.0",
+    "@metamask/json-rpc-engine": "^10.1.0",
     "@metamask/utils": "^11.8.0",
     "@ts-bridge/cli": "^0.6.1",
     "@types/jest": "^27.4.1",

packages/account-tree-controller/CHANGELOG.md

@@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Set the `setAccountGroupName`'s option `autoHandleConflict` to `true` for all backup & sync operations ([#6697](https://github.com/MetaMask/core/pull/6697))
+- Add new group naming for non-HD keyring accounts ([#6679](https://github.com/MetaMask/core/pull/6679)), ([#6696](https://github.com/MetaMask/core/pull/6696))
+  - Hardware-wallet account groups are now named: "Ledger|Trezor|QR|Lattice|OneKey Account N".
+  - Private key account groups are now named: "Imported Account N".
+  - Snap account groups are now named: "Snap Account N".
+- Account group names now use natural indexing as a fallback ([#6677](https://github.com/MetaMask/core/pull/6677)), ([#6679](https://github.com/MetaMask/core/pull/6679)), ([#6696](https://github.com/MetaMask/core/pull/6696))
+  - If a user names his accounts without any indexes, we would just use the number of accounts to compute the next available index.
+
+### Fixed
+
+- Fix group naming for non-HD keyring accounts ([#6677](https://github.com/MetaMask/core/pull/6677)), ([#6679](https://github.com/MetaMask/core/pull/6679))
+  - Previously, the first non-HD keyring account would start as `Account 2` as opposed to `Account 1` and thus subsequent group names were off as well.
+
+## [1.0.0]
+
+### Changed
+
+- **BREAKING:** Bump peer dependency `@metamask/multichain-account-service` from `^0.8.0` to `^1.0.0` ([#6652](https://github.com/MetaMask/core/pull/6652), [#6676](https://github.com/MetaMask/core/pull/6676))
+
+## [0.18.1]
+
+### Fixed
+
+- Set `lastUpdatedAt` to `0` when generating default account group names ([#6672](https://github.com/MetaMask/core/pull/6672))
+  - This created conflicts with backup and sync, where newly created local groups' names were taking precedence over user-defined backed up names.
+
 ## [0.18.0]
 
 ### Added
@@ -276,7 +304,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Initial release ([#5847](https://github.com/MetaMask/core/pull/5847))
   - Grouping accounts into 3 main categories: Entropy source, Snap ID, keyring types.
 
-[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/account-tree-controller@0.18.0...HEAD
+[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/account-tree-controller@1.0.0...HEAD
+[1.0.0]: https://github.com/MetaMask/core/compare/@metamask/account-tree-controller@0.18.1...@metamask/account-tree-controller@1.0.0
+[0.18.1]: https://github.com/MetaMask/core/compare/@metamask/account-tree-controller@0.18.0...@metamask/account-tree-controller@0.18.1
 [0.18.0]: https://github.com/MetaMask/core/compare/@metamask/account-tree-controller@0.17.0...@metamask/account-tree-controller@0.18.0
 [0.17.0]: https://github.com/MetaMask/core/compare/@metamask/account-tree-controller@0.16.1...@metamask/account-tree-controller@0.17.0
 [0.16.1]: https://github.com/MetaMask/core/compare/@metamask/account-tree-controller@0.16.0...@metamask/account-tree-controller@0.16.1

packages/account-tree-controller/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@metamask/account-tree-controller",
-  "version": "0.18.0",
+  "version": "1.0.0",
   "description": "Controller to group account together based on some pre-defined rules",
   "keywords": [
     "MetaMask",
@@ -61,7 +61,7 @@
     "@metamask/auto-changelog": "^3.4.4",
     "@metamask/keyring-api": "^21.0.0",
     "@metamask/keyring-controller": "^23.1.0",
-    "@metamask/multichain-account-service": "^0.11.0",
+    "@metamask/multichain-account-service": "^1.0.0",
     "@metamask/profile-sync-controller": "^25.0.0",
     "@metamask/providers": "^22.1.0",
     "@metamask/snaps-controllers": "^14.0.1",
@@ -78,7 +78,7 @@
     "@metamask/account-api": "^0.12.0",
     "@metamask/accounts-controller": "^33.0.0",
     "@metamask/keyring-controller": "^23.0.0",
-    "@metamask/multichain-account-service": "^0.8.0",
+    "@metamask/multichain-account-service": "^1.0.0",
     "@metamask/profile-sync-controller": "^25.0.0",
     "@metamask/providers": "^22.0.0",
     "@metamask/snaps-controllers": "^14.0.0",

packages/account-tree-controller/src/AccountTreeController.test.ts

@@ -587,7 +587,7 @@ describe('AccountTreeController', () => {
                   type: AccountGroupType.SingleAccount,
                   accounts: [MOCK_SNAP_ACCOUNT_2.id],
                   metadata: {
-                    name: 'Account 2', // Updated: per-wallet numbering (different wallet)
+                    name: 'Snap Account 1', // Updated: per-wallet numbering (different wallet)
                     pinned: false,
                     hidden: false,
                   },
@@ -610,7 +610,7 @@ describe('AccountTreeController', () => {
                   type: AccountGroupType.SingleAccount,
                   accounts: [MOCK_HARDWARE_ACCOUNT_1.id],
                   metadata: {
-                    name: 'Account 2', // Updated: per-wallet numbering (different wallet)
+                    name: 'Ledger Account 1', // Updated: per-wallet numbering (different wallet)
                     pinned: false,
                     hidden: false,
                   },
@@ -652,13 +652,13 @@ describe('AccountTreeController', () => {
           },
           [expectedKeyringWalletIdGroup]: {
             name: {
-              value: 'Account 2', // Updated: per-wallet numbering (different wallet)
+              value: 'Ledger Account 1', // Updated: per-wallet numbering (different wallet)
               lastUpdatedAt: expect.any(Number),
             },
           },
           [expectedSnapWalletIdGroup]: {
             name: {
-              value: 'Account 2', // Updated: per-wallet numbering (different wallet)
+              value: 'Snap Account 1', // Updated: per-wallet numbering (different wallet)
               lastUpdatedAt: expect.any(Number),
             },
           },
@@ -2430,8 +2430,8 @@ describe('AccountTreeController', () => {
 
       // Groups should use consistent default naming regardless of import time
       // Updated expectations based on per-wallet sequential naming logic
-      expect(group1?.metadata.name).toBe('Account 3'); // Updated: reflects actual naming logic
-      expect(group2?.metadata.name).toBe('Account 2'); // Updated: reflects actual naming logic
+      expect(group1?.metadata.name).toBe('Account 2'); // Updated: reflects actual naming logic
+      expect(group2?.metadata.name).toBe('Account 1'); // Updated: reflects actual naming logic
     });
 
     it('uses fallback naming when rule-based naming returns empty string', () => {
@@ -2903,8 +2903,8 @@ describe('AccountTreeController', () => {
       expect(uniqueNames.size).toBe(2);
 
       // Due to optimization, names start at wallet.length, so we get "Account 3" and "Account 4"
-      expect(allNames).toContain('Account 3');
-      expect(allNames).toContain('Account 4');
+      expect(allNames).toContain('Ledger Account 1');
+      expect(allNames).toContain('Ledger Account 2');
 
       // Verify they're actually different
       expect(group1.metadata.name).not.toBe(group2.metadata.name);
@@ -4033,4 +4033,219 @@ describe('AccountTreeController', () => {
       ).toBe('Conflict Name (2)');
     });
   });
+
+  describe('naming', () => {
+    const mockAccount1 = {
+      ...MOCK_HARDWARE_ACCOUNT_1,
+      id: 'mock-id-1',
+      address: '0x123',
+    };
+    const mockAccount2 = {
+      ...MOCK_HARDWARE_ACCOUNT_1,
+      id: 'mock-id-2',
+      address: '0x456',
+    };
+    const mockAccount3 = {
+      ...MOCK_HARDWARE_ACCOUNT_1,
+      id: 'mock-id-3',
+      address: '0x789',
+    };
+    const mockAccount4 = {
+      ...MOCK_HARDWARE_ACCOUNT_1,
+      id: 'mock-id-4',
+      address: '0xabc',
+    };
+
+    const mockWalletId = toAccountWalletId(
+      AccountWalletType.Keyring,
+      KeyringTypes.ledger,
+    );
+
+    const getAccountGroupFromAccount = (
+      controller: AccountTreeController,
+      mockAccount: InternalAccount,
+    ) => {
+      const groupId = toAccountGroupId(mockWalletId, mockAccount.address);
+      return controller.state.accountTree.wallets[mockWalletId].groups[groupId];
+    };
+
+    it('names non-HD keyrings accounts properly', () => {
+      const { controller, messenger } = setup();
+
+      // Add all 3 accounts.
+      [mockAccount1, mockAccount2, mockAccount3].forEach(
+        (mockAccount, index) => {
+          messenger.publish('AccountsController:accountAdded', mockAccount);
+
+          const mockGroup = getAccountGroupFromAccount(controller, mockAccount);
+          expect(mockGroup).toBeDefined();
+          expect(mockGroup.metadata.name).toBe(`Ledger Account ${index + 1}`);
+        },
+      );
+
+      // Remove account 2, should still create account 4 afterward.
+      messenger.publish('AccountsController:accountRemoved', mockAccount2.id);
+
+      expect(
+        getAccountGroupFromAccount(controller, mockAccount4),
+      ).toBeUndefined();
+      messenger.publish('AccountsController:accountAdded', mockAccount4);
+
+      const mockGroup4 = getAccountGroupFromAccount(controller, mockAccount4);
+      expect(mockGroup4).toBeDefined();
+      expect(mockGroup4.metadata.name).toBe('Ledger Account 4');
+
+      // Now, removing account 3 and 4, should defaults to an index of "2" (since only
+      // account 1 remains), thus, re-inserting account 2, should be named "* Account 2".
+      messenger.publish('AccountsController:accountRemoved', mockAccount4.id);
+      messenger.publish('AccountsController:accountRemoved', mockAccount3.id);
+
+      expect(
+        getAccountGroupFromAccount(controller, mockAccount2),
+      ).toBeUndefined();
+      messenger.publish('AccountsController:accountAdded', mockAccount2);
+
+      const mockGroup2 = getAccountGroupFromAccount(controller, mockAccount2);
+      expect(mockGroup2).toBeDefined();
+      expect(mockGroup2.metadata.name).toBe('Ledger Account 2');
+    });
+
+    it('ignores bad account group name pattern and fallback to natural indexing', () => {
+      const { controller, messenger } = setup({
+        accounts: [mockAccount1],
+      });
+
+      controller.init();
+
+      const mockGroup1 = getAccountGroupFromAccount(controller, mockAccount1);
+      expect(mockGroup1).toBeDefined();
+
+      const mockIndex = 90;
+      controller.setAccountGroupName(
+        mockGroup1.id,
+        `Account${mockIndex}`, // No space, so this should fallback to natural indexing
+      );
+
+      // The first account has a non-matching pattern, thus we should fallback to the next
+      // natural index.
+      messenger.publish('AccountsController:accountAdded', mockAccount2);
+      const mockGroup2 = getAccountGroupFromAccount(controller, mockAccount2);
+      expect(mockGroup2).toBeDefined();
+      expect(mockGroup2.metadata.name).toBe(`Ledger Account 2`); // Natural indexing.
+    });
+
+    it.each([
+      ['Account', 'account'],
+      ['Account', 'aCCount'],
+      ['Account', 'accOunT'],
+      [' ', '  '],
+      [' ', '\t'],
+      [' ', ' \t'],
+      [' ', '\t '],
+    ])(
+      'ignores case (case-insensitive) and spaces when extracting highest index: "$0" -> "$1"',
+      (toReplace, replaced) => {
+        const { controller, messenger } = setup({
+          accounts: [mockAccount1],
+        });
+
+        controller.init();
+
+        const mockGroup1 = getAccountGroupFromAccount(controller, mockAccount1);
+        expect(mockGroup1).toBeDefined();
+
+        const mockIndex = 90;
+        controller.setAccountGroupName(
+          mockGroup1.id,
+          mockGroup1.metadata.name
+            .replace(toReplace, replaced)
+            .replace('1', `${mockIndex}`), // Use index different than 1.
+        );
+
+        // Even if the account is not strictly named "Ledger Account 90", we should be able
+        // to compute the next index from there.
+        messenger.publish('AccountsController:accountAdded', mockAccount2);
+        const mockGroup2 = getAccountGroupFromAccount(controller, mockAccount2);
+        expect(mockGroup2).toBeDefined();
+        expect(mockGroup2.metadata.name).toBe(
+          `Ledger Account ${mockIndex + 1}`,
+        );
+      },
+    );
+
+    it.each([' ', '  ', '\t', ' \t'])(
+      'extract name indexes and ignore multiple spaces: "%s"',
+      (space) => {
+        const { controller, messenger } = setup({
+          accounts: [mockAccount1],
+        });
+
+        controller.init();
+
+        const mockGroup1 = getAccountGroupFromAccount(controller, mockAccount1);
+        expect(mockGroup1).toBeDefined();
+
+        const mockIndex = 90;
+        controller.setAccountGroupName(
+          mockGroup1.id,
+          mockGroup1.metadata.name
+            .replace(' ', space)
+            .replace('1', `${mockIndex}`), // Use index different than 1.
+        );
+
+        // Even if the account is not strictly named "Ledger Account 90", we should be able
+        // to compute the next index from there.
+        messenger.publish('AccountsController:accountAdded', mockAccount2);
+        const mockGroup2 = getAccountGroupFromAccount(controller, mockAccount2);
+        expect(mockGroup2).toBeDefined();
+        expect(mockGroup2.metadata.name).toBe(
+          `Ledger Account ${mockIndex + 1}`,
+        );
+      },
+    );
+
+    it('uses natural indexing for pre-existing accounts', () => {
+      const { controller } = setup({
+        accounts: [mockAccount1, mockAccount2, mockAccount3],
+      });
+
+      controller.init();
+
+      // After initializing the controller, all accounts should be named appropriately.
+      [mockAccount1, mockAccount2, mockAccount3].forEach(
+        (mockAccount, index) => {
+          const mockGroup = getAccountGroupFromAccount(controller, mockAccount);
+          expect(mockGroup).toBeDefined();
+          expect(mockGroup.metadata.name).toBe(`Ledger Account ${index + 1}`);
+        },
+      );
+    });
+
+    it('fallbacks to natural indexing if group names are not using our default name pattern', () => {
+      const { controller, messenger } = setup();
+
+      [mockAccount1, mockAccount2, mockAccount3].forEach((mockAccount) =>
+        messenger.publish('AccountsController:accountAdded', mockAccount),
+      );
+
+      const mockGroup1 = getAccountGroupFromAccount(controller, mockAccount1);
+      const mockGroup2 = getAccountGroupFromAccount(controller, mockAccount2);
+      const mockGroup3 = getAccountGroupFromAccount(controller, mockAccount3);
+      expect(mockGroup1).toBeDefined();
+      expect(mockGroup2).toBeDefined();
+      expect(mockGroup3).toBeDefined();
+
+      // Rename all accounts to something different than "* Account <index>".
+      controller.setAccountGroupName(mockGroup1.id, 'Account A');
+      controller.setAccountGroupName(mockGroup2.id, 'The next account');
+      controller.setAccountGroupName(mockGroup3.id, 'Best account so far');
+
+      // Adding a new account should not reset back to "Account 1", but it should
+      // use the next natural index, here, "Account 4".
+      messenger.publish('AccountsController:accountAdded', mockAccount4);
+      const mockGroup4 = getAccountGroupFromAccount(controller, mockAccount4);
+      expect(mockGroup4).toBeDefined();
+      expect(mockGroup4.metadata.name).toBe('Ledger Account 4');
+    });
+  });
 });