From bc77e27d9c01a6f93e2919cf71147726a20ddb23 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 12 Nov 2025 12:16:21 -0500 Subject: [PATCH 1/4] feat: remove call to getNextAvailableAccountName --- packages/accounts-controller/src/AccountsController.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 4526ec9e98b..7d2eccdf8f0 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -868,12 +868,6 @@ export class AccountsController extends BaseController< internalAccounts.accounts, ) as InternalAccount[]; - // Get next account name available for this given keyring. - const name = this.getNextAvailableAccountName( - account.metadata.keyring.type, - accounts, - ); - // If it's the first account, we need to select it. const lastSelected = accounts.length === 0 ? this.#getLastSelectedIndex() : 0; @@ -882,7 +876,6 @@ export class AccountsController extends BaseController< ...account, metadata: { ...account.metadata, - name, importTime: Date.now(), lastSelected, }, From a679ac75b91de7e1f2f7ffc39452d36ca609b4d5 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 12 Nov 2025 12:45:30 -0500 Subject: [PATCH 2/4] chore: update changelog --- packages/accounts-controller/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/accounts-controller/CHANGELOG.md b/packages/accounts-controller/CHANGELOG.md index e266de88f3c..fcfca714efa 100644 --- a/packages/accounts-controller/CHANGELOG.md +++ b/packages/accounts-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Remove usage of `getNextAvailableAccountName` in `handleOnKeyringStateChange` ([#7137](https://github.com/MetaMask/core/pull/7137)) + - The function call was decided to be removed since it was taking a lot of time, we compute account names in the `AccountTreeController` regardless. + ## [34.0.0] ### Changed From ec0e9040d6b9b8675f678154aebc533e90c741b6 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 12 Nov 2025 13:42:43 -0500 Subject: [PATCH 3/4] fix: update tests --- .../src/AccountsController.test.ts | 59 ++++--------------- 1 file changed, 10 insertions(+), 49 deletions(-) diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index d560b73f841..c2863a7d3b9 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -78,7 +78,7 @@ const mockAccount: InternalAccount = { type: EthAccountType.Eoa, scopes: [EthScope.Eoa], metadata: { - name: 'Account 1', + name: '', keyring: { type: KeyringTypes.hd }, importTime: 1691565967600, lastSelected: 1691565967656, @@ -94,7 +94,7 @@ const mockAccount2: InternalAccount = { type: EthAccountType.Eoa, scopes: [EthScope.Eoa], metadata: { - name: 'Account 2', + name: '', keyring: { type: KeyringTypes.hd }, importTime: 1691565967600, lastSelected: 1955565967656, @@ -786,7 +786,7 @@ describe('AccountsController', () => { setExpectedLastSelectedAsAny( createExpectedInternalAccount({ id: 'mock-id3', - name: 'Snap Account 2', + name: '', address: mockAccount3.address, keyringType: mockAccount3.metadata.keyring.type as KeyringTypes, snap: mockAccount3.metadata.snap, @@ -970,7 +970,7 @@ describe('AccountsController', () => { MockExpectedInternalAccountBuilder.from( createExpectedInternalAccount({ id: 'mock-id3', - name: 'Account 3', + name: '', address: mockAccount3.address, keyringType: KeyringTypes.hd, }), @@ -1039,7 +1039,7 @@ describe('AccountsController', () => { MockExpectedInternalAccountBuilder.from( createExpectedInternalAccount({ id: 'mock-id3', - name: 'Account 3', + name: '', address: mockAccount3.address, keyringType: KeyringTypes.hd, }), @@ -1510,13 +1510,13 @@ describe('AccountsController', () => { const messenger = buildMessenger(); const mockInitialAccount = createMockInternalAccount({ id: 'mock-id', - name: 'Account 1', + name: '', address: '0x123', keyringType: KeyringTypes.hd, }); const mockReinitialisedAccount = createMockInternalAccount({ id: 'mock-id2', - name: 'Account 1', + name: '', address: '0x456', keyringType: KeyringTypes.hd, // Entropy options are added automatically by the controller. @@ -3309,27 +3309,6 @@ describe('AccountsController', () => { accountsController.getAccountExpect(mockAccount.id), ); }); - - it('throw an error if the account name already exists', () => { - const { accountsController } = setupAccountsController({ - initialState: { - internalAccounts: { - accounts: { - [mockAccount.id]: mockAccount, - [mockAccount2.id]: mockAccount2, - }, - selectedAccount: mockAccount.id, - }, - }, - }); - - expect(() => - accountsController.setAccountNameAndSelectAccount( - mockAccount.id, - mockAccount2.metadata.name, - ), - ).toThrow('Account name already exists'); - }); }); describe('setAccountName', () => { @@ -3391,24 +3370,6 @@ describe('AccountsController', () => { accountsController.getAccountExpect(mockAccount.id), ); }); - - it('throw an error if the account name already exists', () => { - const { accountsController } = setupAccountsController({ - initialState: { - internalAccounts: { - accounts: { - [mockAccount.id]: mockAccount, - [mockAccount2.id]: mockAccount2, - }, - selectedAccount: mockAccount.id, - }, - }, - }); - - expect(() => - accountsController.setAccountName(mockAccount.id, 'Account 2'), - ).toThrow('Account name already exists'); - }); }); describe('updateAccountMetadata', () => { @@ -3437,19 +3398,19 @@ describe('AccountsController', () => { // those keyring types are "grouped" together) const mockSimpleKeyring1 = createMockInternalAccount({ id: 'mock-id2', - name: 'Account 2', + name: '', address: '0x555', keyringType: KeyringTypes.simple, }); const mockSimpleKeyring2 = createMockInternalAccount({ id: 'mock-id3', - name: 'Account 3', + name: '', address: '0x666', keyringType: KeyringTypes.simple, }); const mockSimpleKeyring3 = createMockInternalAccount({ id: 'mock-id4', - name: 'Account 4', + name: '', address: '0x777', keyringType: KeyringTypes.simple, }); From d27ff28464396573f6407707ec5394330497de99 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Wed, 12 Nov 2025 14:14:27 -0500 Subject: [PATCH 4/4] test: add tests to cover uncovered lines --- .../src/AccountsController.test.ts | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index c2863a7d3b9..03495a473c7 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -3370,6 +3370,32 @@ describe('AccountsController', () => { accountsController.getAccountExpect(mockAccount.id), ); }); + + it('throws when renaming to an existing account name', () => { + const existingName = 'Existing Name'; + const mockAccountWithName = createMockInternalAccount({ + id: 'mock-id-2', + name: existingName, + address: '0xabc', + keyringType: KeyringTypes.hd, + }); + + const { accountsController } = setupAccountsController({ + initialState: { + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + [mockAccountWithName.id]: mockAccountWithName, + }, + selectedAccount: mockAccount.id, + }, + }, + }); + + expect(() => + accountsController.setAccountName(mockAccount.id, existingName), + ).toThrow('Account name already exists'); + }); }); describe('updateAccountMetadata', () => { @@ -3901,6 +3927,42 @@ describe('AccountsController', () => { ); expect(accountName).toBe('Account 4'); }); + + it('computes next name for non-HD keyring types (Ledger branch)', () => { + const { accountsController } = setupAccountsController({}); + + const ledger1 = createMockInternalAccount({ + id: 'ledger-1', + name: 'Ledger 1', + address: '0xdeadbeef1', + keyringType: KeyringTypes.ledger, + }); + + const next = accountsController.getNextAvailableAccountName( + KeyringTypes.ledger, + [ledger1], + ); + expect(next).toBe('Ledger 2'); + }); + + it('treats Simple and HD as one group (invokes simple-type check)', () => { + const { accountsController } = setupAccountsController({}); + + const simple1 = createMockInternalAccount({ + id: 'simple-1', + name: 'Account 1', + address: '0xfacefeed1', + keyringType: KeyringTypes.simple, + }); + + // Asking for next HD name with only a Simple account forces the + // filter to evaluate the Simple branch, covering isSimpleKeyringType. + const next = accountsController.getNextAvailableAccountName( + KeyringTypes.hd, + [simple1], + ); + expect(next).toBe('Account 2'); + }); }); }); });