From c00690470d1073249938d6647b80691ec4461693 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 22 Sep 2025 12:34:41 -0400 Subject: [PATCH 1/5] fix: update start index for proposed account name for pk imports --- packages/account-tree-controller/src/AccountTreeController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-tree-controller/src/AccountTreeController.ts b/packages/account-tree-controller/src/AccountTreeController.ts index 1b7ab99549d..723bfa6c976 100644 --- a/packages/account-tree-controller/src/AccountTreeController.ts +++ b/packages/account-tree-controller/src/AccountTreeController.ts @@ -458,7 +458,7 @@ export class AccountTreeController extends BaseController< } else { // For other wallet types, start with the number of existing groups // This gives us the next logical sequential number - proposedNameIndex = Object.keys(wallet.groups).length; + proposedNameIndex = Object.keys(wallet.groups).length - 1; } // Use the higher of the two: highest parsed index or computed index From 6eaf6d0bc421b6fb932b4ad1985246db4d710b0a Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 22 Sep 2025 14:21:01 -0400 Subject: [PATCH 2/5] fix: update logic to use the min out of the highestAccountNameIndex and proposedNameIndex --- packages/account-tree-controller/src/AccountTreeController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-tree-controller/src/AccountTreeController.ts b/packages/account-tree-controller/src/AccountTreeController.ts index 723bfa6c976..e9db61c8a15 100644 --- a/packages/account-tree-controller/src/AccountTreeController.ts +++ b/packages/account-tree-controller/src/AccountTreeController.ts @@ -462,7 +462,7 @@ export class AccountTreeController extends BaseController< } // Use the higher of the two: highest parsed index or computed index - proposedNameIndex = Math.max(highestAccountNameIndex, proposedNameIndex); + proposedNameIndex = Math.min(highestAccountNameIndex, proposedNameIndex); // Find a unique name by checking for conflicts and incrementing if needed let nameExists: boolean; From 5380a69ebb38cf52f4119b025277cf3d913b6e83 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 22 Sep 2025 14:21:16 -0400 Subject: [PATCH 3/5] fix: update tests --- .../src/AccountTreeController.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/account-tree-controller/src/AccountTreeController.test.ts b/packages/account-tree-controller/src/AccountTreeController.test.ts index 801c47c4cb2..70c208ffe8a 100644 --- a/packages/account-tree-controller/src/AccountTreeController.test.ts +++ b/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: '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: '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: 'Account 1', // Updated: per-wallet numbering (different wallet) lastUpdatedAt: expect.any(Number), }, }, [expectedSnapWalletIdGroup]: { name: { - value: 'Account 2', // Updated: per-wallet numbering (different wallet) + value: '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('Account 1'); + expect(allNames).toContain('Account 2'); // Verify they're actually different expect(group1.metadata.name).not.toBe(group2.metadata.name); From c25811c5d6db8bdfb7f709a27facb3c2a43b4175 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 22 Sep 2025 14:36:19 -0400 Subject: [PATCH 4/5] chore: update changelog --- packages/account-tree-controller/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/account-tree-controller/CHANGELOG.md b/packages/account-tree-controller/CHANGELOG.md index 0ab49e590c4..e946e1c24b5 100644 --- a/packages/account-tree-controller/CHANGELOG.md +++ b/packages/account-tree-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fix group naming for PK/Hardware accounts ([#6677](https://github.com/MetaMask/core/pull/6677)) + - Previously, the first PK/Hardware account would start as `Account 2` as opposed to `Account 1` and thus subsequent group names were off as well. + ## [1.0.0] ### Changed From c9569d31f88e229a668c4255d6bfca1c7da80e2f Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 23 Sep 2025 05:07:01 -0400 Subject: [PATCH 5/5] chore: fix comment --- .../account-tree-controller/src/AccountTreeController.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-tree-controller/src/AccountTreeController.test.ts b/packages/account-tree-controller/src/AccountTreeController.test.ts index 70c208ffe8a..cd7df67be00 100644 --- a/packages/account-tree-controller/src/AccountTreeController.test.ts +++ b/packages/account-tree-controller/src/AccountTreeController.test.ts @@ -2902,7 +2902,7 @@ describe('AccountTreeController', () => { // Critical assertion: should have 2 unique names (no duplicates) expect(uniqueNames.size).toBe(2); - // Due to optimization, names start at wallet.length, so we get "Account 3" and "Account 4" + // Due to optimization, names start at wallet.length, so we get "Account 1" and "Account 2" expect(allNames).toContain('Account 1'); expect(allNames).toContain('Account 2');