fix!: bug 4157, use getAccounts on HD Keyring when calling addNewAccount

packages/keyring-controller/src/KeyringController.test.ts

@@ -176,6 +176,32 @@ describe('KeyringController', () => {
         },
       );
     });
+
+    // Testing fix for bug #4157 {@link https://github.com/MetaMask/core/issues/4157}
+    it('should return an existing HD account if the accountCount is lower than oldAccounts', async () => {
+      const mockAddress = '0x123';
+      stubKeyringClassWithAccount(MockKeyring, mockAddress);
+      await withController(
+        { keyringBuilders: [keyringBuilderFactory(MockKeyring)] },
+        async ({ controller, initialState }) => {
+          await controller.addNewKeyring(MockKeyring.type);
+
+          // expect there to be two accounts, 1 from HD and 1 from MockKeyring
+          expect(await controller.getAccounts()).toHaveLength(2);
+
+          const accountCount = initialState.keyrings[0].accounts.length;
+          const { addedAccountAddress: firstAccountAdded } =
+            await controller.addNewAccount(accountCount);
+          const { addedAccountAddress: secondAccountAdded } =
+            await controller.addNewAccount(accountCount);
+          expect(firstAccountAdded).toBe(secondAccountAdded);
+          expect(controller.state.keyrings[0].accounts).toHaveLength(
+            accountCount + 1,
+          );
+          expect(await controller.getAccounts()).toHaveLength(3);
+        },
+      );
+    });
   });
 
   describe('addNewAccountForKeyring', () => {

packages/keyring-controller/src/KeyringController.ts

@@ -573,17 +573,22 @@ export class KeyringController extends BaseController<
     if (!primaryKeyring) {
       throw new Error('No HD keyring found');
     }
-    const oldAccounts = await this.getAccounts();
+    const oldAccounts = await primaryKeyring.getAccounts();
 
     if (accountCount && oldAccounts.length !== accountCount) {
       if (accountCount > oldAccounts.length) {
         throw new Error('Account out of sequence');
       }
       // we return the account already existing at index `accountCount`
-      const primaryKeyringAccounts = await primaryKeyring.getAccounts();
+      const existingAccount = oldAccounts[accountCount];
+
+      if (!existingAccount) {
+        throw new Error(`Can't find account at index ${accountCount}`);
+      }
+
       return {
         keyringState: this.#getMemState(),
-        addedAccountAddress: primaryKeyringAccounts[accountCount],
+        addedAccountAddress: existingAccount,
       };
     }
 
@@ -797,7 +802,7 @@ export class KeyringController extends BaseController<
   }
 
   /**
-   * Returns the public addresses of all accounts for the current keyring.
+   * Returns the public addresses of all accounts from every keyring.
    *
    * @returns A promise resolving to an array of addresses.
    */