Skip to content

Commit 5bc89b3

Browse files
committed
fix: implement per-wallet sequential account naming
- Skip computed names to fix global numbering bug from accounts-controller - All accounts now use proper per-wallet sequential naming (Account 1, 2, 3...) - Maintain 100% test coverage and implement all code review feedback - Phase 1 fix - computed names will be re-added in future PR with proper migration
1 parent 646fc06 commit 5bc89b3

File tree

1 file changed

+65
-84
lines changed

1 file changed

+65
-84
lines changed

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

Lines changed: 65 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -273,15 +273,9 @@ export class AccountTreeController extends BaseController<
273273
state.accountTree.wallets = wallets;
274274

275275
// Apply group metadata within the state update
276-
for (const [walletId, wallet] of Object.entries(
277-
state.accountTree.wallets,
278-
)) {
279-
for (const groupId of Object.keys(wallet.groups)) {
280-
this.#applyAccountGroupMetadata(
281-
state,
282-
walletId as AccountWalletId,
283-
groupId as AccountGroupId,
284-
);
276+
for (const wallet of Object.values(state.accountTree.wallets)) {
277+
for (const group of Object.values(wallet.groups)) {
278+
this.#applyAccountGroupMetadata(state, wallet.id, group.id);
285279
}
286280
}
287281

@@ -425,91 +419,78 @@ export class AccountTreeController extends BaseController<
425419
} else if (!group.metadata.name) {
426420
// Get the appropriate rule for this wallet type
427421
const rule = this.#getRuleForWallet(wallet);
428-
const typedWallet = wallet as AccountWalletObjectOf<typeof wallet.type>;
429-
const typedGroup = typedWallet.groups[group.id] as AccountGroupObjectOf<
430-
typeof group.type
431-
>;
432-
433-
// Use computed name first, then fallback to default naming if empty
434-
const computedName = rule.getComputedAccountGroupName(typedGroup);
435422

436-
if (computedName) {
437-
state.accountTree.wallets[walletId].groups[groupId].metadata.name =
438-
computedName;
439-
} else {
440-
// Generate default name and ensure it's unique within the wallet
441-
let proposedName = '';
442-
let proposedNameIndex: number;
443-
444-
// Parse the highest account index being used (similar to accounts-controller)
445-
let highestAccountNameIndex = 0;
446-
for (const existingGroup of Object.values(
447-
wallet.groups,
448-
) as AccountGroupObject[]) {
449-
// Skip the current group being processed
450-
if (existingGroup.id === group.id) {
451-
continue;
452-
}
453-
// Parse the existing group name to extract the numeric index
454-
// TODO: This regex only matches "Account N" pattern. Hardware wallets (Trezor, Ledger, etc.)
455-
// use different patterns like "Trezor N", "Ledger N" per keyringTypeToName().
456-
// We'll enhance this to handle all keyring types in a future iteration.
457-
const nameMatch =
458-
existingGroup.metadata.name.match(/Account (\d+)$/u);
459-
if (nameMatch) {
460-
const nameIndex = parseInt(nameMatch[1], 10);
461-
if (nameIndex > highestAccountNameIndex) {
462-
highestAccountNameIndex = nameIndex;
463-
}
423+
// Skip computed names for now - use default naming with per-wallet logic
424+
// TODO: Implement computed names in a future iteration
425+
426+
// Generate default name and ensure it's unique within the wallet
427+
let proposedName = '';
428+
let proposedNameIndex: number;
429+
430+
// Parse the highest account index being used (similar to accounts-controller)
431+
let highestAccountNameIndex = 0;
432+
for (const existingGroup of Object.values(
433+
wallet.groups,
434+
) as AccountGroupObject[]) {
435+
// Skip the current group being processed
436+
if (existingGroup.id === group.id) {
437+
continue;
438+
}
439+
// Parse the existing group name to extract the numeric index
440+
// TODO: This regex only matches "Account N" pattern. Hardware wallets (Trezor, Ledger, etc.)
441+
// use different patterns like "Trezor N", "Ledger N" per keyringTypeToName().
442+
// We'll enhance this to handle all keyring types in a future iteration.
443+
const nameMatch = existingGroup.metadata.name.match(/Account (\d+)$/u);
444+
if (nameMatch) {
445+
const nameIndex = parseInt(nameMatch[1], 10);
446+
if (nameIndex > highestAccountNameIndex) {
447+
highestAccountNameIndex = nameIndex;
464448
}
465449
}
450+
}
466451

467-
// For entropy-based multichain groups, start with the actual groupIndex
468-
if (
469-
group.type === AccountGroupType.MultichainAccount &&
470-
group.metadata.entropy
471-
) {
472-
proposedNameIndex = group.metadata.entropy.groupIndex;
473-
} else {
474-
// For other wallet types, start with the number of existing groups
475-
// This gives us the next logical sequential number
476-
proposedNameIndex = Object.keys(wallet.groups).length;
477-
}
452+
// For entropy-based multichain groups, start with the actual groupIndex
453+
if (
454+
group.type === AccountGroupType.MultichainAccount &&
455+
group.metadata.entropy
456+
) {
457+
proposedNameIndex = group.metadata.entropy.groupIndex;
458+
} else {
459+
// For other wallet types, start with the number of existing groups
460+
// This gives us the next logical sequential number
461+
proposedNameIndex = Object.keys(wallet.groups).length;
462+
}
478463

479-
// Use the higher of the two: highest parsed index or computed index
480-
proposedNameIndex = Math.max(
481-
highestAccountNameIndex,
482-
proposedNameIndex,
483-
);
464+
// Use the higher of the two: highest parsed index or computed index
465+
proposedNameIndex = Math.max(highestAccountNameIndex, proposedNameIndex);
484466

485-
// Find a unique name by checking for conflicts and incrementing if needed
486-
let nameExists: boolean;
487-
do {
488-
proposedName = rule.getDefaultAccountGroupName(proposedNameIndex);
467+
// Find a unique name by checking for conflicts and incrementing if needed
468+
let nameExists: boolean;
469+
do {
470+
proposedName = rule.getDefaultAccountGroupName(proposedNameIndex);
489471

490-
// Check if this name already exists in the wallet (excluding current group)
491-
nameExists = !isAccountGroupNameUniqueFromWallet(
492-
wallet,
493-
group.id,
494-
proposedName,
495-
);
472+
// Check if this name already exists in the wallet (excluding current group)
473+
nameExists = !isAccountGroupNameUniqueFromWallet(
474+
wallet,
475+
group.id,
476+
proposedName,
477+
);
496478

497-
/* istanbul ignore next */
498-
if (nameExists) {
499-
proposedNameIndex += 1; // Try next number
500-
}
501-
} while (nameExists);
479+
/* istanbul ignore next */
480+
if (nameExists) {
481+
proposedNameIndex += 1; // Try next number
482+
}
483+
} while (nameExists);
502484

503-
state.accountTree.wallets[walletId].groups[groupId].metadata.name =
504-
proposedName;
485+
state.accountTree.wallets[walletId].groups[groupId].metadata.name =
486+
proposedName;
505487

506-
// Persist the generated name to ensure consistency
507-
state.accountGroupsMetadata[group.id] ??= {};
508-
state.accountGroupsMetadata[group.id].name = {
509-
value: proposedName,
510-
lastUpdatedAt: Date.now(),
511-
};
512-
}
488+
// Persist the generated name to ensure consistency
489+
state.accountGroupsMetadata[group.id] ??= {};
490+
state.accountGroupsMetadata[group.id].name = {
491+
value: proposedName,
492+
lastUpdatedAt: Date.now(),
493+
};
513494
}
514495

515496
// Apply persisted UI states

0 commit comments

Comments
 (0)