Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Feb 25, 2025
86 changes: 64 additions & 22 deletions app/scripts/lib/snap-keyring/snap-keyring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand All @@ -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 };
}
});
Expand All @@ -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;
}) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
});
Expand All @@ -388,7 +430,7 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
void this.#addAccountFinalize({
address,
snapId,
skipConfirmation,
skipConfirmationDialog,
accountName,
onceSaved,
});
Expand Down
2 changes: 2 additions & 0 deletions app/scripts/lib/snap-keyring/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -34,6 +35,7 @@ export type SnapKeyringBuilderAllowActions =
| AccountsControllerSetSelectedAccountAction
| AccountsControllerGetAccountByAddressAction
| AccountsControllerSetAccountNameAction
| AccountsControllerListMultichainAccountsAction
| HandleSnapRequest
| GetSnap
| PreferencesControllerGetStateAction;
Expand Down
1 change: 1 addition & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,7 @@ export default class MetamaskController extends EventEmitter {
'AccountsController:setSelectedAccount',
'AccountsController:getAccountByAddress',
'AccountsController:setAccountName',
'AccountsController:listMultichainAccounts',
'SnapController:handleRequest',
'SnapController:get',
'PreferencesController:getState',
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
27 changes: 27 additions & 0 deletions shared/lib/accounts/accounts.ts
Original file line number Diff line number Diff line change
@@ -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;
}
4 changes: 4 additions & 0 deletions shared/lib/accounts/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export * from './accounts';
export * from './bitcoin-wallet-snap';
export * from './snaps';
export * from './solana-wallet-snap';
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
/**
Expand Down Expand Up @@ -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);
},
[],
);
Expand Down
4 changes: 2 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"
Expand Down
Loading