Skip to content

Commit

Permalink
feat(keyring-snap-bridge): add displayAccountNameSuggestion flag (#213
Browse files Browse the repository at this point in the history
)

## Background

Some preinstalled snaps (such as the institutional snap, and some non
EVMs) may need to create accounts non-interactively. Additionally, the
profile sync feature might require accounts to be restored without user
input.

Users normally are asked to name their snap accounts, but this isn't
appropriate where a canonical name exists.

This adds `displayAccountNameDialog` similar to `displayConfirmation`

It's intended that similar logic would be used in the client that is
used for displayConfirmation ([extension
code](https://github.com/MetaMask/metamask-extension/blob/2ca6b570a3580fa8b7dd18af492e8c1db702d814/app/scripts/lib/snap-keyring/snap-keyring.ts#L208)),
i.e. it would default to true, and negation would only be allowed in the
case of preinstalled snaps.

## Examples
- Institutional snap gets names from custodians, so users need not be
asked to enter names
- The designs for Multi-SRP have the user pick a name for an account as
they are choosing the SRP, so before the snap invocation


* Fixes: Jira ticket
[https://consensyssoftware.atlassian.net/browse/MMMULTISRP-56](MMMULTISRP-56)

---------

Co-authored-by: Charly Chevalier <[email protected]>
Co-authored-by: Daniel Rocha <[email protected]>
  • Loading branch information
3 people authored Feb 20, 2025
1 parent 0abfd7f commit 925efd8
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 1 deletion.
1 change: 1 addition & 0 deletions packages/keyring-api/src/events.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('events', () => {
type: EthAccountType.Eoa,
},
displayConfirmation: true,
displayAccountNameSuggestion: true,
},
};

Expand Down
10 changes: 10 additions & 0 deletions packages/keyring-api/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ export const AccountCreatedEventStruct = object({
* **Note:** This is not guaranteed to be honored by the MetaMask client.
*/
displayConfirmation: exactOptional(boolean()),

/**
* Instructs MetaMask to display the name confirmation dialog in the UI.
* Otherwise, the account will be added with the suggested name, if it's not
* already in use; if it is, a suffix will be appended to the name to make it
* unique.
*
* **Note:** This is not guaranteed to be honored by the MetaMask client.
*/
displayAccountNameSuggestion: exactOptional(boolean()),
}),
});

Expand Down
28 changes: 27 additions & 1 deletion packages/keyring-snap-bridge/src/SnapKeyring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
} from '@metamask/keyring-api';
import type { JsonRpcRequest } from '@metamask/keyring-utils';
import type { HandleSnapRequest } from '@metamask/snaps-controllers';
import type { SnapId } from '@metamask/snaps-sdk';
import { type SnapId } from '@metamask/snaps-sdk';
import { KnownCaipNamespace, toCaipChainId } from '@metamask/utils';

import type { KeyringState } from '.';
Expand Down Expand Up @@ -128,6 +128,14 @@ describe('SnapKeyring', () => {
scopes: [EthScope.Eoa],
type: EthAccountType.Eoa,
};
const ethEoaAccount4 = {
id: '7e14f1fa-818c-4590-bab5-b19f947559a5',
address: '0xd7eb71598059D0856cd24DcbeF48a0DB5ffDa4D4'.toLowerCase(),
options: {},
methods: ETH_EOA_METHODS,
scopes: [EthScope.Eoa],
type: EthAccountType.Eoa,
};
const ethErc4337Account = {
id: 'fc926fff-f515-4eb5-9952-720bbd9b9849',
address: '0x2f15b30952aebe0ed5fdbfe5bf16fb9ecdb31d9a'.toLowerCase(),
Expand Down Expand Up @@ -296,6 +304,7 @@ describe('SnapKeyring', () => {
expect.any(Promise),
undefined,
undefined,
undefined,
);
});

Expand Down Expand Up @@ -324,6 +333,7 @@ describe('SnapKeyring', () => {
expect.any(Promise),
undefined,
undefined,
undefined,
);
});

Expand Down Expand Up @@ -379,18 +389,28 @@ describe('SnapKeyring', () => {
{ ...ethEoaAccount1 },
'New Account',
undefined,
undefined,
],
[
'handles account creation with displayConfirmation',
{ ...ethEoaAccount2 },
undefined,
false,
undefined,
],
[
'handles account creation with both accountNameSuggestion and displayConfirmation',
{ ...ethEoaAccount3 },
'New Account',
false,
undefined,
],
[
'handles account creation with both accountNameSuggestion and displayAccountNameSuggestion',
{ ...ethEoaAccount4 },
'New Account',
false,
false,
],
])(
'%s',
Expand All @@ -399,6 +419,7 @@ describe('SnapKeyring', () => {
account,
accountNameSuggestion,
displayConfirmation,
displayAccountNameSuggestion,
) => {
// Reset mock
mockCallbacks.addAccount.mockClear();
Expand All @@ -411,6 +432,9 @@ describe('SnapKeyring', () => {
...(accountNameSuggestion !== undefined && {
accountNameSuggestion,
}),
...(displayAccountNameSuggestion !== undefined && {
displayAccountNameSuggestion,
}),
};

await keyring.handleKeyringSnapMessage(snapId, {
Expand All @@ -425,6 +449,7 @@ describe('SnapKeyring', () => {
expect.any(Promise),
accountNameSuggestion,
displayConfirmation,
displayAccountNameSuggestion,
);
},
);
Expand Down Expand Up @@ -677,6 +702,7 @@ describe('SnapKeyring', () => {
expect.any(Promise),
undefined,
undefined,
undefined,
);
expect(mockMessenger.handleRequest).toHaveBeenLastCalledWith({
handler: 'onKeyringRequest',
Expand Down
3 changes: 3 additions & 0 deletions packages/keyring-snap-bridge/src/SnapKeyring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export type SnapKeyringCallbacks = {
onceSaved: Promise<AccountId>,
accountNameSuggestion?: string,
displayConfirmation?: boolean,
displayAccountNameSuggestion?: boolean,
): Promise<void>;

removeAccount(
Expand Down Expand Up @@ -206,6 +207,7 @@ export class SnapKeyring extends EventEmitter {
account: newAccountFromEvent,
accountNameSuggestion,
displayConfirmation,
displayAccountNameSuggestion,
} = message.params;

// READ THIS CAREFULLY:
Expand Down Expand Up @@ -287,6 +289,7 @@ export class SnapKeyring extends EventEmitter {
onceSaved.promise,
accountNameSuggestion,
displayConfirmation,
displayAccountNameSuggestion,
);

return null;
Expand Down

0 comments on commit 925efd8

Please sign in to comment.