Skip to content

Commit 680b813

Browse files
authored
fix: update start index for proposed account name for pk/hardware imports (#6677)
## Explanation Currently, PK/Hardware imports were being started at `Account 2` as opposed to `Account 1`. When we apply group metadata we already have that group in state, the logic in `#applyGroupMetadata` doesn't account for the fact that the group we're applying metadata shouldn't be counted for in the following line: https://github.com/MetaMask/core/blob/main/packages/account-tree-controller/src/AccountTreeController.ts#L461. Due to the way we apply group metadata (after we've added the groups into state), we need to take the min between `proposedNameIndex` and `highestAccountNameIndex` NOT the max. This change in logic accounts for group metadata being applied in the scenario when the controller is being rehydrated through `init` and when a new pk/hardware account is added in an active session (and handled through `accountAdded`). ## Before https://github.com/user-attachments/assets/8ab1b3c6-574f-45bc-bf3a-3a3de7b45836 ## After https://github.com/user-attachments/assets/66e53362-e587-45d3-b7ae-43e09da17a09 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent b1a43cf commit 680b813

File tree

3 files changed

+16
-11
lines changed

3 files changed

+16
-11
lines changed

packages/account-tree-controller/CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Fix group naming for PK/Hardware accounts ([#6677](https://github.com/MetaMask/core/pull/6677))
13+
- 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.
14+
1015
## [1.0.0]
1116

1217
### Changed

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ describe('AccountTreeController', () => {
587587
type: AccountGroupType.SingleAccount,
588588
accounts: [MOCK_SNAP_ACCOUNT_2.id],
589589
metadata: {
590-
name: 'Account 2', // Updated: per-wallet numbering (different wallet)
590+
name: 'Account 1', // Updated: per-wallet numbering (different wallet)
591591
pinned: false,
592592
hidden: false,
593593
},
@@ -610,7 +610,7 @@ describe('AccountTreeController', () => {
610610
type: AccountGroupType.SingleAccount,
611611
accounts: [MOCK_HARDWARE_ACCOUNT_1.id],
612612
metadata: {
613-
name: 'Account 2', // Updated: per-wallet numbering (different wallet)
613+
name: 'Account 1', // Updated: per-wallet numbering (different wallet)
614614
pinned: false,
615615
hidden: false,
616616
},
@@ -652,13 +652,13 @@ describe('AccountTreeController', () => {
652652
},
653653
[expectedKeyringWalletIdGroup]: {
654654
name: {
655-
value: 'Account 2', // Updated: per-wallet numbering (different wallet)
655+
value: 'Account 1', // Updated: per-wallet numbering (different wallet)
656656
lastUpdatedAt: expect.any(Number),
657657
},
658658
},
659659
[expectedSnapWalletIdGroup]: {
660660
name: {
661-
value: 'Account 2', // Updated: per-wallet numbering (different wallet)
661+
value: 'Account 1', // Updated: per-wallet numbering (different wallet)
662662
lastUpdatedAt: expect.any(Number),
663663
},
664664
},
@@ -2430,8 +2430,8 @@ describe('AccountTreeController', () => {
24302430

24312431
// Groups should use consistent default naming regardless of import time
24322432
// Updated expectations based on per-wallet sequential naming logic
2433-
expect(group1?.metadata.name).toBe('Account 3'); // Updated: reflects actual naming logic
2434-
expect(group2?.metadata.name).toBe('Account 2'); // Updated: reflects actual naming logic
2433+
expect(group1?.metadata.name).toBe('Account 2'); // Updated: reflects actual naming logic
2434+
expect(group2?.metadata.name).toBe('Account 1'); // Updated: reflects actual naming logic
24352435
});
24362436

24372437
it('uses fallback naming when rule-based naming returns empty string', () => {
@@ -2902,9 +2902,9 @@ describe('AccountTreeController', () => {
29022902
// Critical assertion: should have 2 unique names (no duplicates)
29032903
expect(uniqueNames.size).toBe(2);
29042904

2905-
// Due to optimization, names start at wallet.length, so we get "Account 3" and "Account 4"
2906-
expect(allNames).toContain('Account 3');
2907-
expect(allNames).toContain('Account 4');
2905+
// Due to optimization, names start at wallet.length, so we get "Account 1" and "Account 2"
2906+
expect(allNames).toContain('Account 1');
2907+
expect(allNames).toContain('Account 2');
29082908

29092909
// Verify they're actually different
29102910
expect(group1.metadata.name).not.toBe(group2.metadata.name);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,11 +458,11 @@ export class AccountTreeController extends BaseController<
458458
} else {
459459
// For other wallet types, start with the number of existing groups
460460
// This gives us the next logical sequential number
461-
proposedNameIndex = Object.keys(wallet.groups).length;
461+
proposedNameIndex = Object.keys(wallet.groups).length - 1;
462462
}
463463

464464
// Use the higher of the two: highest parsed index or computed index
465-
proposedNameIndex = Math.max(highestAccountNameIndex, proposedNameIndex);
465+
proposedNameIndex = Math.min(highestAccountNameIndex, proposedNameIndex);
466466

467467
// Find a unique name by checking for conflicts and incrementing if needed
468468
let nameExists: boolean;

0 commit comments

Comments
 (0)