feat(snap-keyring): handle displayAccountNameSuggestion flag (#30531)

## **Description** This PR handles the `displayAccountNameSuggestion` flag that can be set by pre-installed Snaps in the `accountCreated` event.  [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30531?quickstart=1) ## **Related issues** Fixes: MetaMask/accounts-planning#829 ## **Manual testing steps** 1. Update a pre-installed Snap to set `displayAccountNameSuggestion` to false: ```diff diff --git a/packages/snap/src/core/handlers/onKeyringRequest/Keyring.ts b/packages/snap/src/core/handlers/onKeyringRequest/Keyring.ts index 27238e7..6edabc7 100644 --- a/packages/snap/src/core/handlers/onKeyringRequest/Keyring.ts +++ b/packages/snap/src/core/handlers/onKeyringRequest/Keyring.ts @@ -224,7 +224,8 @@ export class SolanaKeyring implements Keyring { * and the snaps sdk does not allow extra properties. */ account: keyringAccount, - accountNameSuggestion: `Solana Account ${index + 1}`, + accountNameSuggestion: `Solana Account`, + displayAccountNameSuggestion: false, }); return keyringAccount; ``` 2. Build the Snap and add a resolution in the extension's `package.json`: ``` "@metamask/solana-wallet-snap@^1.2.0": "file:../snap-solana-wallet/packages/snap" ``` 3. Start the extension: `yarn && yarn start:flask` 4. Create an account 5. The account name dialog should not be shown ## **Screenshots/Recordings**  ### **Before**  ### **After**  ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
MetaMask · Feb 25, 2025 · 32c2077 · 32c2077
1 parent 78c5d2c
commit 32c2077
Show file tree

Hide file tree

Showing 8 changed files with 105 additions and 44 deletions.
diff --git a/app/scripts/lib/snap-keyring/snap-keyring.ts b/app/scripts/lib/snap-keyring/snap-keyring.ts
@@ -14,6 +14,7 @@ import MetamaskController from '../../metamask-controller';
 // eslint-disable-next-line import/no-restricted-paths
 import { IconName } from '../../../../ui/components/component-library/icon';
 import MetaMetricsController from '../../controllers/metametrics-controller';
+import { getUniqueAccountName } from '../../../../shared/lib/accounts';
 import { isBlockedUrl } from './utils/isBlockedUrl';
 import { showError, showSuccess } from './utils/showResult';
 import { SnapKeyringBuilderMessenger } from './types';
@@ -201,23 +202,62 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
     }
   }
 
+  /**
+   * Get the account name from the user through a dialog.
+   *
+   * @param snapId - ID of the Snap that created the account.
+   * @param accountNameSuggestion - Suggested name for the account.
+   * @returns The name that should be used for the account.
+   */
+  async #getAccountNameFromDialog(
+    snapId: SnapId,
+    accountNameSuggestion: string,
+  ): Promise<{ success: boolean; accountName?: string }> {
+    const { success, name: accountName } =
+      await showAccountNameSuggestionDialog(
+        snapId,
+        this.#messenger,
+        accountNameSuggestion,
+      );
+
+    return { success, accountName };
+  }
+
+  /**
+   * Use the account name suggestion to decide the name of the account.
+   *
+   * @param accountNameSuggestion - Suggested name for the account.
+   * @returns The name that should be used for the account.
+   */
+  async #getAccountNameFromSuggestion(
+    accountNameSuggestion: string,
+  ): Promise<{ success: boolean; accountName?: string }> {
+    const accounts = await this.#messenger.call(
+      'AccountsController:listMultichainAccounts',
+    );
+    const accountName = getUniqueAccountName(accounts, accountNameSuggestion);
+    return { success: true, accountName };
+  }
+
   async #addAccountConfirmations({
     snapId,
-    skipConfirmation,
+    skipConfirmationDialog,
+    skipAccountNameSuggestionDialog,
     handleUserInput,
     accountNameSuggestion,
   }: {
     snapId: SnapId;
-    skipConfirmation: boolean;
+    skipConfirmationDialog: boolean;
+    skipAccountNameSuggestionDialog: boolean;
     accountNameSuggestion: string;
     handleUserInput: (accepted: boolean) => Promise<void>;
   }): Promise<{ accountName?: string }> {
     return await this.#withApprovalFlow(async (_) => {
-      // 1. Show the account **creation** confirmation dialog.
+      // 1. Show the account CREATION confirmation dialog.
       {
         // If confirmation dialog are skipped, we consider the account creation to be confirmed until the account name dialog is closed
         const success =
-          skipConfirmation ||
+          skipConfirmationDialog ||
           (await showAccountCreationDialog(snapId, this.#messenger));
 
         if (!success) {
@@ -228,24 +268,19 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
         }
       }
 
-      // 2. Show the account **renaming** confirmation dialog.
+      // 2. Show the account RENAMING confirmation dialog. Note that
+      //    pre-installed Snaps can skip this dialog.
       {
-        const { success, name: accountName } =
-          await showAccountNameSuggestionDialog(
-            snapId,
-            this.#messenger,
-            accountNameSuggestion,
-          );
+        const { success, accountName } = skipAccountNameSuggestionDialog
+          ? await this.#getAccountNameFromSuggestion(accountNameSuggestion)
+          : await this.#getAccountNameFromDialog(snapId, accountNameSuggestion);
 
-        if (!success) {
-          // User has cancelled account creation
-          await handleUserInput(success);
+        await handleUserInput(success);
 
+        if (!success) {
           throw new Error('User denied account creation');
         }
 
-        await handleUserInput(success);
-
         return { accountName };
       }
     });
@@ -254,13 +289,13 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
   async #addAccountFinalize({
     address,
     snapId,
-    skipConfirmation,
+    skipConfirmationDialog,
     accountName,
     onceSaved,
   }: {
     address: string;
     snapId: SnapId;
-    skipConfirmation: boolean;
+    skipConfirmationDialog: boolean;
     onceSaved: Promise<string>;
     accountName?: string;
   }) {
@@ -305,7 +340,7 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
           );
         }
 
-        if (!skipConfirmation) {
+        if (!skipConfirmationDialog) {
           // TODO: Add events tracking to the dialog itself, so that events are more
           // "linked" to UI actions
           // User should now see the "Successfuly added account" page
@@ -366,17 +401,24 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
     onceSaved: Promise<string>,
     accountNameSuggestion: string = '',
     displayConfirmation: boolean = false,
+    displayAccountNameSuggestion: boolean = true,
   ) {
     assertIsValidSnapId(snapId);
 
     // If Snap is preinstalled and does not request confirmation, skip the confirmation dialog.
-    const skipConfirmation = isSnapPreinstalled(snapId) && !displayConfirmation;
+    const skipConfirmationDialog =
+      isSnapPreinstalled(snapId) && !displayConfirmation;
+
+    // Only pre-installed Snaps can skip the account name suggestion dialog.
+    const skipAccountNameSuggestionDialog =
+      isSnapPreinstalled(snapId) && !displayAccountNameSuggestion;
 
     // First part of the flow, which includes confirmation dialogs (if not skipped).
     // Once confirmed, we resume the Snap execution.
     const { accountName } = await this.#addAccountConfirmations({
       snapId,
-      skipConfirmation,
+      skipConfirmationDialog,
+      skipAccountNameSuggestionDialog,
       accountNameSuggestion,
       handleUserInput,
     });
@@ -388,7 +430,7 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
     void this.#addAccountFinalize({
       address,
       snapId,
-      skipConfirmation,
+      skipConfirmationDialog,
       accountName,
       onceSaved,
     });

diff --git a/app/scripts/lib/snap-keyring/types.ts b/app/scripts/lib/snap-keyring/types.ts
@@ -4,6 +4,7 @@ import type { KeyringControllerGetAccountsAction } from '@metamask/keyring-contr
 import { GetSubjectMetadata } from '@metamask/permission-controller';
 import {
   AccountsControllerGetAccountByAddressAction,
+  AccountsControllerListMultichainAccountsAction,
   AccountsControllerSetAccountNameAction,
   AccountsControllerSetSelectedAccountAction,
 } from '@metamask/accounts-controller';
@@ -34,6 +35,7 @@ export type SnapKeyringBuilderAllowActions =
   | AccountsControllerSetSelectedAccountAction
   | AccountsControllerGetAccountByAddressAction
   | AccountsControllerSetAccountNameAction
+  | AccountsControllerListMultichainAccountsAction
   | HandleSnapRequest
   | GetSnap
   | PreferencesControllerGetStateAction;

diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js
@@ -1125,6 +1125,7 @@ export default class MetamaskController extends EventEmitter {
         'AccountsController:setSelectedAccount',
         'AccountsController:getAccountByAddress',
         'AccountsController:setAccountName',
+        'AccountsController:listMultichainAccounts',
         'SnapController:handleRequest',
         'SnapController:get',
         'PreferencesController:getState',

diff --git a/package.json b/package.json
@@ -273,7 +273,7 @@
     "@metamask/eth-json-rpc-middleware": "^15.1.2",
     "@metamask/eth-ledger-bridge-keyring": "^8.0.3",
     "@metamask/eth-sig-util": "^7.0.1",
-    "@metamask/eth-snap-keyring": "^11.0.0",
+    "@metamask/eth-snap-keyring": "^11.1.0",
     "@metamask/eth-token-tracker": "^10.0.2",
     "@metamask/eth-trezor-keyring": "^6.0.0",
     "@metamask/etherscan-link": "^3.0.0",

diff --git a/shared/lib/accounts/accounts.ts b/shared/lib/accounts/accounts.ts
@@ -0,0 +1,27 @@
+import { InternalAccount } from '@metamask/keyring-internal-api';
+
+/**
+ * Get the next available account name based on the suggestion and the list of
+ * accounts.
+ *
+ * @param accounts - The list of accounts to check for name availability
+ * @param nameSuggestion - The suggested name for the account
+ * @returns The next available account name based on the suggestion
+ */
+export function getUniqueAccountName(
+  accounts: InternalAccount[],
+  nameSuggestion: string,
+): string {
+  let suffix = 1;
+  let candidateName = nameSuggestion;
+
+  const isNameTaken = (name: string) =>
+    accounts.some((account) => account.metadata.name === name);
+
+  while (isNameTaken(candidateName)) {
+    suffix += 1;
+    candidateName = `${nameSuggestion} ${suffix}`;
+  }
+
+  return candidateName;
+}
diff --git a/shared/lib/accounts/index.ts b/shared/lib/accounts/index.ts
@@ -0,0 +1,4 @@
+export * from './accounts';
+export * from './bitcoin-wallet-snap';
+export * from './snaps';
+export * from './solana-wallet-snap';
diff --git a/ui/components/multichain/create-named-snap-account/create-named-snap-account.tsx b/ui/components/multichain/create-named-snap-account/create-named-snap-account.tsx
@@ -8,6 +8,7 @@ import { Box, ModalHeader } from '../../component-library';
 import { useI18nContext } from '../../../hooks/useI18nContext';
 import { getMostRecentOverviewPage } from '../../../ducks/history/history';
 import { getNextAvailableAccountName } from '../../../store/actions';
+import { getUniqueAccountName } from '../../../../shared/lib/accounts';
 
 export type CreateNamedSnapAccountProps = {
   /**
@@ -44,25 +45,9 @@ export const CreateNamedSnapAccount: React.FC<CreateNamedSnapAccountProps> = ({
   const getNextAccountName = useCallback(
     async (accounts: InternalAccount[]): Promise<string> => {
       // If a snap-suggested account name exists, use it as a base
-      if (snapSuggestedAccountName) {
-        let suffix = 1;
-        let candidateName = snapSuggestedAccountName;
-
-        // Check if the name is already taken
-        const isNameTaken = (name: string) =>
-          accounts.some((account) => account.metadata.name === name);
-
-        // Keep incrementing suffix until we find an available name
-        while (isNameTaken(candidateName)) {
-          suffix += 1;
-          candidateName = `${snapSuggestedAccountName} ${suffix}`;
-        }
-
-        return candidateName;
-      }
-
-      // If no snap-suggested name, use the next available account name
-      return getNextAvailableAccountName(KeyringTypes.snap);
+      return snapSuggestedAccountName
+        ? getUniqueAccountName(accounts, snapSuggestedAccountName)
+        : getNextAvailableAccountName(KeyringTypes.snap);
     },
     [],
   );

diff --git a/yarn.lock b/yarn.lock
@@ -5329,7 +5329,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@metamask/eth-snap-keyring@npm:^11.0.0, @metamask/eth-snap-keyring@npm:^11.1.0":
+"@metamask/eth-snap-keyring@npm:^11.1.0":
   version: 11.1.0
   resolution: "@metamask/eth-snap-keyring@npm:11.1.0"
   dependencies:
@@ -26953,7 +26953,7 @@ __metadata:
     "@metamask/eth-json-rpc-provider": "npm:^4.1.6"
     "@metamask/eth-ledger-bridge-keyring": "npm:^8.0.3"
     "@metamask/eth-sig-util": "npm:^7.0.1"
-    "@metamask/eth-snap-keyring": "npm:^11.0.0"
+    "@metamask/eth-snap-keyring": "npm:^11.1.0"
     "@metamask/eth-token-tracker": "npm:^10.0.2"
     "@metamask/eth-trezor-keyring": "npm:^6.0.0"
     "@metamask/etherscan-link": "npm:^3.0.0"