fix(accounts): fix account id indexing when there an index gap

Author: ccharly

packages/accounts-controller/src/AccountsController.ts

@@ -652,17 +652,15 @@ export class AccountsController extends BaseController<
   }
 
   /**
-   * Returns the next account number for a given keyring type.
+   * Returns the list of accounts for a given keyring type.
    * @param keyringType - The type of keyring.
-   * @returns An object containing the account prefix and index to use.
+   * @returns The list of accounts associcated with this keyring type.
    */
-  #getNextAccountNumber(keyringType: string): {
-    accountPrefix: string;
-    indexToUse: number;
-  } {
-    const keyringName = keyringTypeToName(keyringType);
-    const previousKeyringAccounts = this.listAccounts().filter(
+  #getAccountsByKeyringType(keyringType: string) {
+    return this.listAccounts().filter(
       (internalAccount) => {
+        // We do consider `hd` and `simple` keyrings to be of same type. So we check those 2 types
+        // to group those accounts together!
         if (
           keyringType === KeyringTypes.hd ||
           keyringType === KeyringTypes.simple
@@ -672,24 +670,47 @@ export class AccountsController extends BaseController<
             internalAccount.metadata.keyring.type === KeyringTypes.simple
           );
         }
+
         return internalAccount.metadata.keyring.type === keyringType;
       },
     );
+  }
+
+  /**
+   * Returns the next account number for a given keyring type.
+   * @param keyringType - The type of keyring.
+   * @returns An object containing the account prefix and index to use.
+   */
+  #getNextAccountNumber(keyringType: string): {
+    accountPrefix: string;
+    indexToUse: number;
+  } {
+    const keyringName = keyringTypeToName(keyringType);
+    const keyringAccounts = this.#getAccountsByKeyringType(keyringType);
     const lastDefaultIndexUsedForKeyringType =
-      previousKeyringAccounts
-        .filter((internalAccount) =>
-          new RegExp(`${keyringName} \\d+$`, 'u').test(
+      keyringAccounts
+        .reduce((maxInternalAccountIndex, internalAccount) => {
+          // We **DO NOT USE** `\d+` here to only consider valid "human"
+          // number (rounded decimal number)
+          const match = (new RegExp(`${keyringName} ([0-9]+)$`, 'u')).exec(
             internalAccount.metadata.name,
-          ),
-        )
-        .map((internalAccount) => {
-          const nameToWords = internalAccount.metadata.name.split(' '); // get the index of a default account name
-          return parseInt(nameToWords[nameToWords.length], 10);
-        })
-        .sort((a, b) => b - a)[0] || 0;
+          );
+
+          if (match) {
+            // Quoting `RegExp.exec` documentation:
+            // > The returned array has the matched text as the first item, and then one item for
+            // > each capturing group of the matched text.
+            // So use `match[1]` to get the captured value
+            const internalAccountIndex = parseInt(match[1], 10);
+            return Math.max(maxInternalAccountIndex, internalAccountIndex);
+          }
+
+          return maxInternalAccountIndex;
+
+        }, 0);
 
     const indexToUse = Math.max(
-      previousKeyringAccounts.length + 1,
+      keyringAccounts.length + 1,
       lastDefaultIndexUsedForKeyringType + 1,
     );