test(accounts): add test for account name indexing with a gap

Author: ccharly

packages/accounts-controller/src/AccountsController.test.ts

@@ -1799,29 +1799,29 @@ describe('AccountsController', () => {
   });
 
   describe('#getNextAccountNumber', () => {
-    it('should return the next account number', async () => {
-      const messenger = buildMessenger();
-      mockUUID
-        .mockReturnValueOnce('mock-id') // call to check if its a new account
-        .mockReturnValueOnce('mock-id2') // call to check if its a new account
-        .mockReturnValueOnce('mock-id3') // call to check if its a new account
-        .mockReturnValueOnce('mock-id2') // call to add account
-        .mockReturnValueOnce('mock-id3'); // call to add account
-
-      const mockSimpleKeyring1 = createExpectedInternalAccount({
-        id: 'mock-id2',
-        name: 'Account 2',
-        address: '0x555',
-        keyringType: 'Simple Key Pair',
-      });
-      const mockSimpleKeyring2 = createExpectedInternalAccount({
-        id: 'mock-id3',
-        name: 'Account 3',
-        address: '0x666',
-        keyringType: 'Simple Key Pair',
-      });
+    // Account names start at 2 since have 1 HD account + 2 simple keypair accounts (and both
+    // those keyring types are "grouped" together)
+    const mockSimpleKeyring1 = createExpectedInternalAccount({
+      id: 'mock-id2',
+      name: 'Account 2',
+      address: '0x555',
+      keyringType: 'Simple Key Pair',
+    });
+    const mockSimpleKeyring2 = createExpectedInternalAccount({
+      id: 'mock-id3',
+      name: 'Account 3',
+      address: '0x666',
+      keyringType: 'Simple Key Pair',
+    });
+    const mockSimpleKeyring3 = createExpectedInternalAccount({
+      id: 'mock-id4',
+      name: 'Account 4',
+      address: '0x777',
+      keyringType: 'Simple Key Pair',
+    });
 
-      const mockNewKeyringState = {
+    const mockNewKeyringStateWith = (simpleAddressess: string[]) => {
+      return {
         isUnlocked: true,
         keyrings: [
           {
@@ -1830,10 +1830,21 @@ describe('AccountsController', () => {
           },
           {
             type: 'Simple Key Pair',
-            accounts: [mockSimpleKeyring1.address, mockSimpleKeyring2.address],
+            accounts: simpleAddressess,
           },
         ],
       };
+    };
+
+    it('should return the next account number', async () => {
+      const messenger = buildMessenger();
+      mockUUID
+        .mockReturnValueOnce('mock-id') // call to check if its a new account
+        .mockReturnValueOnce('mock-id2') // call to check if its a new account
+        .mockReturnValueOnce('mock-id3') // call to check if its a new account
+        .mockReturnValueOnce('mock-id2') // call to add account
+        .mockReturnValueOnce('mock-id3'); // call to add account
+
       const accountsController = setupAccountsController({
         initialState: {
           internalAccounts: {
@@ -1848,18 +1859,77 @@ describe('AccountsController', () => {
 
       messenger.publish(
         'KeyringController:stateChange',
-        mockNewKeyringState,
+        mockNewKeyringStateWith([
+          mockSimpleKeyring1.address,
+          mockSimpleKeyring2.address,
+        ]),
         [],
       );
 
       const accounts = accountsController.listAccounts();
-
       expect(accounts).toStrictEqual([
         mockAccount,
         setLastSelectedAsAny(mockSimpleKeyring1),
         setLastSelectedAsAny(mockSimpleKeyring2),
       ]);
     });
+
+    it('should return the next account number even with an index gap', async () => {
+      const messenger = buildMessenger();
+      const mockAccountUUIDs = new MockNormalAccountUUID([
+        mockAccount,
+        mockSimpleKeyring1,
+        mockSimpleKeyring2,
+        mockSimpleKeyring3,
+      ]);
+      mockUUID.mockImplementation(mockAccountUUIDs.mock.bind(mockAccountUUIDs));
+
+      const accountsController = setupAccountsController({
+        initialState: {
+          internalAccounts: {
+            accounts: {
+              [mockAccount.id]: mockAccount,
+            },
+            selectedAccount: mockAccount.id,
+          },
+        },
+        messenger,
+      });
+
+      messenger.publish(
+        'KeyringController:stateChange',
+        mockNewKeyringStateWith([
+          mockSimpleKeyring1.address,
+          mockSimpleKeyring2.address,
+        ]),
+        [],
+      );
+
+      // We then remove "Acccount 2" to create a gap
+      messenger.publish(
+        'KeyringController:stateChange',
+        mockNewKeyringStateWith([mockSimpleKeyring2.address]),
+        [],
+      );
+
+      // Finally we add a 3rd account, and it should be named "Account 4" (despite having a gap
+      // since "Account 2" no longer exists)
+      messenger.publish(
+        'KeyringController:stateChange',
+        mockNewKeyringStateWith([
+          mockSimpleKeyring2.address,
+          mockSimpleKeyring3.address,
+        ]),
+        [],
+      );
+
+      const accounts = accountsController.listAccounts();
+      expect(accounts).toStrictEqual([
+        mockAccount,
+        setLastSelectedAsAny(mockSimpleKeyring2),
+        setLastSelectedAsAny(mockSimpleKeyring3),
+      ]);
+    });
   });
 
   describe('getAccountByAddress', () => {