From a3e4dbd41a60ee98f231df64c26f99a33493f74c Mon Sep 17 00:00:00 2001 From: Charly Chevalier Date: Wed, 18 Dec 2024 17:25:16 +0100 Subject: [PATCH] feat(keyring-api)!: add `scopes` field to `KeyringAccount` (#101) See: - https://github.com/MetaMask/accounts-planning/issues/627 --------- Co-authored-by: Gustavo Antunes <17601467+gantunesr@users.noreply.github.com> Co-authored-by: Dario Anongba Varela --- README.md | 1 + packages/keyring-api/src/api/account.test.ts | 24 ++- packages/keyring-api/src/api/account.ts | 20 +- packages/keyring-api/src/btc/constants.ts | 13 ++ packages/keyring-api/src/btc/index.ts | 1 + packages/keyring-api/src/eth/constants.ts | 9 + packages/keyring-api/src/eth/index.ts | 1 + packages/keyring-api/src/eth/types.ts | 8 +- packages/keyring-api/src/events.test.ts | 6 + packages/keyring-api/src/sol/constants.ts | 11 ++ packages/keyring-api/src/sol/index.ts | 1 + .../keyring-internal-api/src/types.test.ts | 36 +++- .../src/KeyringSnapControllerClient.test.ts | 1 + packages/keyring-snap-bridge/package.json | 1 + .../src/SnapKeyring.test.ts | 182 +++++++++++++++--- .../keyring-snap-bridge/src/SnapKeyring.ts | 78 ++++++-- packages/keyring-snap-bridge/src/account.ts | 19 ++ packages/keyring-snap-bridge/src/events.ts | 36 ++++ packages/keyring-snap-bridge/src/util.ts | 22 --- .../keyring-snap-bridge/tsconfig.build.json | 3 +- packages/keyring-snap-bridge/tsconfig.json | 3 +- .../src/KeyringClient.test.ts | 8 +- .../src/KeyringSnapRpcClient.test.ts | 1 + .../keyring-snap-sdk/src/rpc-handler.test.ts | 2 + yarn.lock | 1 + 25 files changed, 416 insertions(+), 72 deletions(-) create mode 100644 packages/keyring-api/src/btc/constants.ts create mode 100644 packages/keyring-api/src/eth/constants.ts create mode 100644 packages/keyring-api/src/sol/constants.ts create mode 100644 packages/keyring-snap-bridge/src/account.ts create mode 100644 packages/keyring-snap-bridge/src/events.ts diff --git a/README.md b/README.md index a9a17cff..50c7d3a9 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ linkStyle default opacity:0.5 eth_snap_keyring --> keyring_api; eth_snap_keyring --> keyring_internal_api; eth_snap_keyring --> keyring_internal_snap_client; + eth_snap_keyring --> keyring_utils; keyring_snap_client --> keyring_api; keyring_snap_client --> keyring_utils; keyring_snap_sdk --> keyring_utils; diff --git a/packages/keyring-api/src/api/account.test.ts b/packages/keyring-api/src/api/account.test.ts index f1e3b455..cb07e8d7 100644 --- a/packages/keyring-api/src/api/account.test.ts +++ b/packages/keyring-api/src/api/account.test.ts @@ -1,6 +1,6 @@ import { assert } from '@metamask/superstruct'; -import { KeyringAccountStruct } from './account'; +import { EthAccountType, KeyringAccountStruct } from './account'; const supportedKeyringAccountTypes = Object.keys( KeyringAccountStruct.schema.type.schema, @@ -12,6 +12,7 @@ describe('api', () => { const baseAccount = { id: '606a7759-b0fb-48e4-9874-bab62ff8e7eb', address: '0x000', + scopes: [], options: {}, methods: [], }; @@ -33,5 +34,26 @@ describe('api', () => { ); }, ); + + it.each([ + // Namespace too short (< 3): + '', + 'a', + 'ei', + 'bi', + 'bi:p122something', + // Namespace too long (> 8): + 'eip11155111', + 'eip11155111:11155111', + ])('throws an error if account scopes is: %s', (scope: string) => { + const account = { + ...baseAccount, + type: EthAccountType.Eoa, + scopes: [scope], + }; + expect(() => assert(account, KeyringAccountStruct)).toThrow( + `At path: scopes.0 -- Expected the value to satisfy a union of \`string | string\`, but received: "${scope}"`, + ); + }); }); }); diff --git a/packages/keyring-api/src/api/account.ts b/packages/keyring-api/src/api/account.ts index 52f465e9..647909a2 100644 --- a/packages/keyring-api/src/api/account.ts +++ b/packages/keyring-api/src/api/account.ts @@ -1,7 +1,18 @@ import { object, UuidStruct } from '@metamask/keyring-utils'; import type { Infer } from '@metamask/superstruct'; -import { array, enums, record, string } from '@metamask/superstruct'; -import { JsonStruct } from '@metamask/utils'; +import { + nonempty, + array, + enums, + record, + string, + union, +} from '@metamask/superstruct'; +import { + CaipChainIdStruct, + CaipNamespaceStruct, + JsonStruct, +} from '@metamask/utils'; /** * Supported Ethereum account types. @@ -62,6 +73,11 @@ export const KeyringAccountStruct = object({ */ address: string(), + /** + * Account supported scopes (CAIP-2 chain IDs). + */ + scopes: nonempty(array(union([CaipNamespaceStruct, CaipChainIdStruct]))), + /** * Account options. */ diff --git a/packages/keyring-api/src/btc/constants.ts b/packages/keyring-api/src/btc/constants.ts new file mode 100644 index 00000000..954097ea --- /dev/null +++ b/packages/keyring-api/src/btc/constants.ts @@ -0,0 +1,13 @@ +/* istanbul ignore file */ + +/** + * Scopes for Bitcoin account type. See {@link KeyringAccount.scopes}. + */ +export enum BtcScopes { + Namespace = 'bip122', + Mainnet = 'bip122:000000000019d6689c085ae165831e93', + Testnet = 'bip122:000000000933ea01ad0ee984209779ba', + Testnet4 = 'bip122:00000000da84f2bafbbc53dee25a72ae', + Signet = 'bip122:00000008819873e925422c1ff0f99f7c', + Regtest = 'bip122:regtest', +} diff --git a/packages/keyring-api/src/btc/index.ts b/packages/keyring-api/src/btc/index.ts index fcb073fe..4fbce956 100644 --- a/packages/keyring-api/src/btc/index.ts +++ b/packages/keyring-api/src/btc/index.ts @@ -1 +1,2 @@ +export * from './constants'; export * from './types'; diff --git a/packages/keyring-api/src/eth/constants.ts b/packages/keyring-api/src/eth/constants.ts new file mode 100644 index 00000000..2ba369d7 --- /dev/null +++ b/packages/keyring-api/src/eth/constants.ts @@ -0,0 +1,9 @@ +/* istanbul ignore file */ + +/** + * Scopes for EVM account type. See {@link KeyringAccount.scopes}. + */ +export enum EthScopes { + Namespace = 'eip155', + Mainnet = 'eip155:1', +} diff --git a/packages/keyring-api/src/eth/index.ts b/packages/keyring-api/src/eth/index.ts index 0e8ad032..53f7c9bf 100644 --- a/packages/keyring-api/src/eth/index.ts +++ b/packages/keyring-api/src/eth/index.ts @@ -1,3 +1,4 @@ +export * from './constants'; export * from './erc4337'; export * from './types'; export * from './utils'; diff --git a/packages/keyring-api/src/eth/types.ts b/packages/keyring-api/src/eth/types.ts index 892bcc5e..e52993fa 100644 --- a/packages/keyring-api/src/eth/types.ts +++ b/packages/keyring-api/src/eth/types.ts @@ -1,7 +1,8 @@ import { object, definePattern } from '@metamask/keyring-utils'; import type { Infer } from '@metamask/superstruct'; -import { array, enums, literal } from '@metamask/superstruct'; +import { nonempty, array, enums, literal } from '@metamask/superstruct'; +import { EthScopes } from '.'; import { EthAccountType, KeyringAccountStruct } from '../api'; export const EthBytesStruct = definePattern('EthBytes', /^0x[0-9a-f]*$/iu); @@ -46,6 +47,11 @@ export const EthEoaAccountStruct = object({ */ type: literal(`${EthAccountType.Eoa}`), + /** + * Account scopes (must be ['eip155']). + */ + scopes: nonempty(array(literal(EthScopes.Namespace))), + /** * Account supported methods. */ diff --git a/packages/keyring-api/src/events.test.ts b/packages/keyring-api/src/events.test.ts index 8748aa5a..4934f644 100644 --- a/packages/keyring-api/src/events.test.ts +++ b/packages/keyring-api/src/events.test.ts @@ -1,6 +1,7 @@ import { is } from '@metamask/superstruct'; import { EthAccountType } from './api'; +import { EthScopes } from './eth/constants'; import { AccountCreatedEventStruct, AccountDeletedEventStruct, @@ -21,6 +22,7 @@ describe('events', () => { address: '0x0123', methods: [], options: {}, + scopes: [EthScopes.Namespace], type: EthAccountType.Eoa, }, }, @@ -38,6 +40,7 @@ describe('events', () => { address: '0x0123', methods: [], options: {}, + scopes: [EthScopes.Namespace], type: EthAccountType.Eoa, }, }, @@ -55,6 +58,7 @@ describe('events', () => { address: '0x0123', methods: [], options: {}, + scopes: [EthScopes.Namespace], type: EthAccountType.Eoa, }, displayConfirmation: true, @@ -75,6 +79,7 @@ describe('events', () => { address: '0x0123', methods: [], options: {}, + scopes: [EthScopes.Namespace], type: EthAccountType.Eoa, }, }, @@ -92,6 +97,7 @@ describe('events', () => { address: '0x0123', methods: [], options: {}, + scopes: [EthScopes.Namespace], type: EthAccountType.Eoa, }, }, diff --git a/packages/keyring-api/src/sol/constants.ts b/packages/keyring-api/src/sol/constants.ts new file mode 100644 index 00000000..db6078cd --- /dev/null +++ b/packages/keyring-api/src/sol/constants.ts @@ -0,0 +1,11 @@ +/* istanbul ignore file */ + +/** + * Scopes for Solana account type. See {@link KeyringAccount.scopes}. + */ +export enum SolScopes { + Namespace = 'solana', + Devnet = 'solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1', + Mainnet = 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + Testnet = 'solana:4uhcVJyU9pJkvQyS88uRDiswHXSCkY3z', +} diff --git a/packages/keyring-api/src/sol/index.ts b/packages/keyring-api/src/sol/index.ts index fcb073fe..4fbce956 100644 --- a/packages/keyring-api/src/sol/index.ts +++ b/packages/keyring-api/src/sol/index.ts @@ -1 +1,2 @@ +export * from './constants'; export * from './types'; diff --git a/packages/keyring-internal-api/src/types.test.ts b/packages/keyring-internal-api/src/types.test.ts index 7d8ae838..505697c0 100644 --- a/packages/keyring-internal-api/src/types.test.ts +++ b/packages/keyring-internal-api/src/types.test.ts @@ -1,20 +1,23 @@ import { assert } from '@metamask/superstruct'; +import type { InternalAccount } from '.'; import { InternalAccountStruct } from '.'; describe('InternalAccount', () => { it.each([ - { type: 'eip155:eoa', address: '0x000' }, + { type: 'eip155:eoa', address: '0x000', scopes: ['eip155'] }, { type: 'bip122:p2wpkh', address: 'bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4', + scopes: ['bip122:000000000019d6689c085ae165831e93'], }, - ])('should have the correct structure: %s', ({ type, address }) => { + ])('should have the correct structure: %s', ({ type, address, scopes }) => { const account = { id: '606a7759-b0fb-48e4-9874-bab62ff8e7eb', address, options: {}, methods: [], + scopes, type, metadata: { keyring: { @@ -34,6 +37,7 @@ describe('InternalAccount', () => { address: '0x000', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', metadata: { keyring: {}, @@ -53,6 +57,7 @@ describe('InternalAccount', () => { address: '0x000', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', metadata: { name: 'Account 1', @@ -71,6 +76,7 @@ describe('InternalAccount', () => { address: '0x000', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', }; @@ -79,12 +85,27 @@ describe('InternalAccount', () => { ); }); + it('should throw if scopes is not set', () => { + const account = { + id: '606a7759-b0fb-48e4-9874-bab62ff8e7eb', + address: '0x000', + options: {}, + methods: [], + type: 'eip155:eoa', + }; + + expect(() => assert(account, InternalAccountStruct)).toThrow( + 'At path: scopes -- Expected an array value, but received: undefined', + ); + }); + it('should throw if there are extra fields', () => { const account = { id: '606a7759-b0fb-48e4-9874-bab62ff8e7eb', address: '0x000', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', metadata: { keyring: { @@ -102,12 +123,13 @@ describe('InternalAccount', () => { }); it('should contain snap name, id and enabled if the snap metadata exists', () => { - const account = { + const account: InternalAccount = { id: '606a7759-b0fb-48e4-9874-bab62ff8e7eb', address: '0x000', options: {}, methods: [], type: 'eip155:eoa', + scopes: ['eip155'], metadata: { keyring: { type: 'Test Keyring', @@ -126,13 +148,14 @@ describe('InternalAccount', () => { }); it.each([['name', 'enabled', 'id']])( - 'should throw if snap.%i is not set', + 'should throw if snap.%s is not set', (key: string) => { - const account = { + const account: InternalAccount = { id: '606a7759-b0fb-48e4-9874-bab62ff8e7eb', address: '0x000', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', metadata: { keyring: { @@ -148,7 +171,8 @@ describe('InternalAccount', () => { }, }; - delete account.metadata.snap[key as keyof typeof account.metadata.snap]; + // On `InternalAccount` the `metadata.snap` is optional, hence the `?.` here. + delete account.metadata.snap?.[key as keyof typeof account.metadata.snap]; const regex = new RegExp(`At path: metadata.snap.${key}`, 'u'); diff --git a/packages/keyring-internal-snap-client/src/KeyringSnapControllerClient.test.ts b/packages/keyring-internal-snap-client/src/KeyringSnapControllerClient.test.ts index 9c16fc94..a9d73773 100644 --- a/packages/keyring-internal-snap-client/src/KeyringSnapControllerClient.test.ts +++ b/packages/keyring-internal-snap-client/src/KeyringSnapControllerClient.test.ts @@ -13,6 +13,7 @@ describe('KeyringSnapControllerClient', () => { address: '0xE9A74AACd7df8112911ca93260fC5a046f8a64Ae', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', }, ]; diff --git a/packages/keyring-snap-bridge/package.json b/packages/keyring-snap-bridge/package.json index 5856d441..f1584b86 100644 --- a/packages/keyring-snap-bridge/package.json +++ b/packages/keyring-snap-bridge/package.json @@ -42,6 +42,7 @@ "@metamask/keyring-api": "workspace:^", "@metamask/keyring-internal-api": "workspace:^", "@metamask/keyring-internal-snap-client": "workspace:^", + "@metamask/keyring-utils": "workspace:^", "@metamask/snaps-controllers": "^9.10.0", "@metamask/snaps-sdk": "^6.7.0", "@metamask/snaps-utils": "^8.3.0", diff --git a/packages/keyring-snap-bridge/src/SnapKeyring.test.ts b/packages/keyring-snap-bridge/src/SnapKeyring.test.ts index a4552236..ec99bc42 100644 --- a/packages/keyring-snap-bridge/src/SnapKeyring.test.ts +++ b/packages/keyring-snap-bridge/src/SnapKeyring.test.ts @@ -8,6 +8,7 @@ import type { EthUserOperationPatch, } from '@metamask/keyring-api'; import { + EthScopes, BtcAccountType, EthAccountType, SolAccountType, @@ -18,7 +19,7 @@ import { } from '@metamask/keyring-api'; import type { SnapController } from '@metamask/snaps-controllers'; import type { SnapId } from '@metamask/snaps-sdk'; -import { KnownCaipNamespace, toCaipChainId } from '@metamask/utils'; +import { toCaipChainId } from '@metamask/utils'; import type { KeyringState } from '.'; import { SnapKeyring } from '.'; @@ -79,6 +80,7 @@ describe('SnapKeyring', () => { address: '0xC728514Df8A7F9271f4B7a4dd2Aa6d2D723d3eE3'.toLowerCase(), options: {}, methods: ETH_EOA_METHODS, + scopes: [EthScopes.Namespace], type: EthAccountType.Eoa, }; const ethEoaAccount2 = { @@ -86,6 +88,7 @@ describe('SnapKeyring', () => { address: '0x34b13912eAc00152bE0Cb409A301Ab8E55739e63'.toLowerCase(), options: {}, methods: ETH_EOA_METHODS, + scopes: [EthScopes.Namespace], type: EthAccountType.Eoa, }; const ethEoaAccount3 = { @@ -93,6 +96,7 @@ describe('SnapKeyring', () => { address: '0xab1G3q98V7C67T9103g30C0417610237A137d763'.toLowerCase(), options: {}, methods: ETH_EOA_METHODS, + scopes: [EthScopes.Namespace], type: EthAccountType.Eoa, }; const ethErc4337Account = { @@ -100,6 +104,7 @@ describe('SnapKeyring', () => { address: '0x2f15b30952aebe0ed5fdbfe5bf16fb9ecdb31d9a'.toLowerCase(), options: {}, methods: ETH_4337_METHODS, + scopes: [EthScopes.Namespace], type: EthAccountType.Erc4337, }; const btcP2wpkhAccount = { @@ -107,6 +112,7 @@ describe('SnapKeyring', () => { address: 'bc1qxy2kgdygjrsqtzq2n0yrf2493p83kkfjhx0wlh', options: {}, methods: [...Object.values(BtcMethod)], + scopes: ['bip122:000000000019d6689c085ae165831e93'], type: BtcAccountType.P2wpkh, }; const solDataAccount = { @@ -114,6 +120,7 @@ describe('SnapKeyring', () => { address: '3d4v35MRK57xM2Nte3E3rTQU1zyXGVrkXJ6FuEjVoKzM', options: {}, methods: [...Object.values(SolMethod)], + scopes: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp'], type: SolAccountType.DataAccount, }; @@ -162,6 +169,7 @@ describe('SnapKeyring', () => { id: 'b05d918a-b37c-497a-bb28-3d15c0d56b7a', options: {}, methods: ETH_EOA_METHODS, + scopes: [EthScopes.Namespace], type: EthAccountType.Eoa, // Even checksummed address will be lower-cased by the bridge. address: '0x6431726EEE67570BF6f0Cf892aE0a3988F03903F', @@ -190,6 +198,7 @@ describe('SnapKeyring', () => { options: {}, methods: [...Object.values(SolMethod)], type: SolAccountType.DataAccount, + scopes: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp'], address: '4k3s6XreQwU9Jht6FzZt8c5yDGrNo8tZ9pGE6S5YjowM', }; await keyring.handleKeyringSnapMessage(snapId, { @@ -314,6 +323,100 @@ describe('SnapKeyring', () => { }, ); }); + + it('creates an account and registers it properly', async () => { + // Reset the keyring so it's empty. + keyring = new SnapKeyring( + mockSnapController as unknown as SnapController, + mockCallbacks, + ); + + const account = ethEoaAccount1; + await keyring.handleKeyringSnapMessage(snapId, { + method: KeyringEvent.AccountCreated, + params: { + account, + }, + }); + + const keyringAccounts = keyring.listAccounts(); + expect(keyringAccounts.length).toBeGreaterThan(0); + expect(keyringAccounts[0]).toStrictEqual({ + ...account, + metadata: expect.any(Object), + }); + }); + + it('creates an EOA account and set a default scopes if not provided', async () => { + // Reset the keyring so it's empty. + keyring = new SnapKeyring( + mockSnapController as unknown as SnapController, + mockCallbacks, + ); + + // Omit `scopes` from `account`. + const { scopes: _, ...account } = ethEoaAccount1; + await keyring.handleKeyringSnapMessage(snapId, { + method: KeyringEvent.AccountCreated, + params: { + account: account as unknown as KeyringAccount, + }, + }); + + const keyringAccounts = keyring.listAccounts(); + expect(keyringAccounts.length).toBeGreaterThan(0); + expect(keyringAccounts[0]).toStrictEqual({ + ...account, + metadata: expect.any(Object), + // By default, new EVM accounts will have this scopes if it not provided + // during the account creation flow. + scopes: [EthScopes.Namespace], + }); + }); + + it('creating an EOA account with the wrong scopes will throw an error', async () => { + // Reset the keyring so it's empty. + keyring = new SnapKeyring( + mockSnapController as unknown as SnapController, + mockCallbacks, + ); + + // Force `scopes` to something else + const account: KeyringAccount = { + ...ethEoaAccount1, + + // EOA accounts are compatible on every EVM chains and MUST USE ['eip155']. Here + // were using a more specific scopes, which is now allowed by the Snap keyring. + scopes: [EthScopes.Mainnet], + }; + await expect( + keyring.handleKeyringSnapMessage(snapId, { + method: KeyringEvent.AccountCreated, + params: { + account, + }, + }), + ).rejects.toThrow('EVM EOA accounts must use scopes: [eip155]'); + }); + + it('creating a non-EVM account with the no scope will throw an error', async () => { + // Reset the keyring so it's empty. + keyring = new SnapKeyring( + mockSnapController as unknown as SnapController, + mockCallbacks, + ); + + // Omit `scopes` from non-EVM `account`. + const { scopes: _, ...account } = btcP2wpkhAccount; + await expect( + keyring.handleKeyringSnapMessage(snapId, { + method: KeyringEvent.AccountCreated, + params: { + account, + }, + }), + ).rejects.toThrow('Account scopes is required for non-EVM accounts'); + }); }); describe('#handleAccountUpdated', () => { @@ -451,6 +554,54 @@ describe('SnapKeyring', () => { `Method '${EthMethod.SignTransaction}' not supported for account ${ethEoaAccount1.address}`, ); }); + + it('updates an EOA account and set a default scopes if not provided', async () => { + // Omit `scopes` from `account`. + const { scopes: _, ...account } = ethEoaAccount1; + + // Return the updated list of accounts when the keyring requests it. + mockSnapController.handleRequest.mockResolvedValue([{ ...account }]); + + expect( + await keyring.handleKeyringSnapMessage(snapId, { + method: KeyringEvent.AccountUpdated, + params: { account }, + }), + ).toBeNull(); + + const keyringAccounts = keyring.listAccounts(); + expect(keyringAccounts.length).toBeGreaterThan(0); + expect(keyringAccounts[0]?.scopes).toStrictEqual([EthScopes.Namespace]); + }); + + it('updates an EOA account with a wrong scope will throw an error', async () => { + const account = { ...ethEoaAccount1, scopes: [EthScopes.Mainnet] }; + + // Return the updated list of accounts when the keyring requests it. + mockSnapController.handleRequest.mockResolvedValue([{ ...account }]); + + await expect( + keyring.handleKeyringSnapMessage(snapId, { + method: KeyringEvent.AccountUpdated, + params: { account }, + }), + ).rejects.toThrow('EVM EOA accounts must use scopes: [eip155]'); + }); + + it('updates a non-EVM account with the no scope will throw an error', async () => { + // Omit `scopes` from non-EVM `account`. + const { scopes: _, ...account } = btcP2wpkhAccount; + + // Return the updated list of accounts when the keyring requests it. + mockSnapController.handleRequest.mockResolvedValue([{ ...account }]); + + await expect( + keyring.handleKeyringSnapMessage(snapId, { + method: KeyringEvent.AccountUpdated, + params: { account }, + }), + ).rejects.toThrow('Account scopes is required for non-EVM accounts'); + }); }); describe('#handleAccountDeleted', () => { @@ -869,7 +1020,7 @@ describe('SnapKeyring', () => { }; const tx = TransactionFactory.fromTxData(mockTx); const expectedSignedTx = TransactionFactory.fromTxData(mockSignedTx); - const expectedScope = 'eip155:1'; + const expectedScope = EthScopes.Mainnet; mockSnapController.handleRequest.mockResolvedValue({ pending: false, @@ -947,7 +1098,7 @@ describe('SnapKeyring', () => { }, }; - const expectedScope = 'eip155:1'; + const expectedScope = EthScopes.Mainnet; const expectedSignature = '0x4355c47d63924e8a72e509b65029052eb6c299d53a04e167c5775fd466751c9d07299936d304c153f6443dfa05f40ff007d72911b6f72307f996231605b915621c'; @@ -1145,10 +1296,7 @@ describe('SnapKeyring', () => { method: 'keyring_submitRequest', params: { id: expect.any(String), - scope: toCaipChainId( - KnownCaipNamespace.Eip155, - executionContext.chainId, - ), + scope: toCaipChainId(EthScopes.Namespace, executionContext.chainId), account: ethErc4337Account.id, request: { method: 'eth_prepareUserOperation', @@ -1201,10 +1349,7 @@ describe('SnapKeyring', () => { method: 'keyring_submitRequest', params: { id: expect.any(String), - scope: toCaipChainId( - KnownCaipNamespace.Eip155, - executionContext.chainId, - ), + scope: toCaipChainId(EthScopes.Namespace, executionContext.chainId), account: ethErc4337Account.id, request: { method: 'eth_patchUserOperation', @@ -1253,10 +1398,7 @@ describe('SnapKeyring', () => { method: 'keyring_submitRequest', params: { id: expect.any(String), - scope: toCaipChainId( - KnownCaipNamespace.Eip155, - executionContext.chainId, - ), + scope: toCaipChainId(EthScopes.Namespace, executionContext.chainId), account: ethErc4337Account.id, request: { method: 'eth_signUserOperation', @@ -1481,10 +1623,7 @@ describe('SnapKeyring', () => { method: 'keyring_submitRequest', params: { id: expect.any(String), - scope: toCaipChainId( - KnownCaipNamespace.Eip155, - executionContext.chainId, - ), + scope: toCaipChainId(EthScopes.Namespace, executionContext.chainId), account: ethErc4337Account.id, request: { method: 'eth_prepareUserOperation', @@ -1551,10 +1690,7 @@ describe('SnapKeyring', () => { method: 'keyring_submitRequest', params: { id: expect.any(String), - scope: toCaipChainId( - KnownCaipNamespace.Eip155, - executionContext.chainId, - ), + scope: toCaipChainId(EthScopes.Namespace, executionContext.chainId), account: ethErc4337Account.id, request: { method: 'eth_patchUserOperation', diff --git a/packages/keyring-snap-bridge/src/SnapKeyring.ts b/packages/keyring-snap-bridge/src/SnapKeyring.ts index a0d0d0c4..3889548d 100644 --- a/packages/keyring-snap-bridge/src/SnapKeyring.ts +++ b/packages/keyring-snap-bridge/src/SnapKeyring.ts @@ -7,17 +7,14 @@ import { TransactionFactory } from '@ethereumjs/tx'; import type { TypedDataV1, TypedMessage } from '@metamask/eth-sig-util'; import { SignTypedDataVersion } from '@metamask/eth-sig-util'; import { - AccountCreatedEventStruct, - AccountDeletedEventStruct, - AccountUpdatedEventStruct, - EthBaseUserOperationStruct, EthBytesStruct, EthMethod, + EthScopes, + EthBaseUserOperationStruct, EthUserOperationPatchStruct, isEvmAccountType, KeyringEvent, - RequestApprovedEventStruct, - RequestRejectedEventStruct, + EthAccountType, } from '@metamask/keyring-api'; import type { KeyringAccount, @@ -31,9 +28,10 @@ import type { } from '@metamask/keyring-api'; import type { InternalAccount } from '@metamask/keyring-internal-api'; import { KeyringSnapControllerClient } from '@metamask/keyring-internal-snap-client'; +import { strictMask } from '@metamask/keyring-utils'; import type { SnapController } from '@metamask/snaps-controllers'; import type { SnapId } from '@metamask/snaps-sdk'; -import type { Snap } from '@metamask/snaps-utils'; +import { isEqual, type Snap } from '@metamask/snaps-utils'; import { assert, mask, object, string } from '@metamask/superstruct'; import type { Json } from '@metamask/utils'; import { @@ -44,7 +42,15 @@ import { import { EventEmitter } from 'events'; import { v4 as uuid } from 'uuid'; +import type { KeyringAccountFromEvent } from './account'; import { DeferredPromise } from './DeferredPromise'; +import { + AccountCreatedEventStruct, + AccountUpdatedEventStruct, + AccountDeletedEventStruct, + RequestApprovedEventStruct, + RequestRejectedEventStruct, +} from './events'; import { projectLogger as log } from './logger'; import { SnapIdMap } from './SnapIdMap'; import type { SnapMessage } from './types'; @@ -52,7 +58,6 @@ import { SnapMessageStruct } from './types'; import { equalsIgnoreCase, sanitizeUrl, - strictMask, throwError, toJson, unique, @@ -167,6 +172,42 @@ export class SnapKeyring extends EventEmitter { this.#callbacks = callbacks; } + /** + * Transform an account coming from events. This account might have optional fields that + * are required by the Keyring API. This function will automatically provides the missing + * fields with some default values. + * + * @param account - The account (coming from keyring events) to transform. + * @returns A valid KeyringAccount. + */ + #transformAccountFromEvent(account: KeyringAccountFromEvent): KeyringAccount { + const { type, scopes } = account; + + if (type === EthAccountType.Eoa) { + // EVM EOA account are compatible with any EVM networks, and we use CAIP-2 + // namespaces when the scope relates to ALL chains (from that namespace). + const namespace = EthScopes.Namespace; + + if (scopes && !isEqual(scopes, [namespace])) { + // Those accounts must use `['eip155']` for their `scopes`. + throw new Error(`EVM EOA accounts must use scopes: [${namespace}]`); + } + + // If `scopes` is not defined, then we will end-up here and inject it. + return { + ...account, + scopes: [namespace], + }; + } + + // For all other accounts (non-EVM and ERC4337), the Snap is expected to provide its scopes! + if (!scopes) { + throw new Error(`Account scopes is required for non-EVM accounts`); + } + + return account as KeyringAccount; + } + /** * Handle an Account Created event from a Snap. * @@ -179,8 +220,15 @@ export class SnapKeyring extends EventEmitter { message: SnapMessage, ): Promise { assert(message, AccountCreatedEventStruct); - const { account, accountNameSuggestion, displayConfirmation } = - message.params; + const { + account: newAccountFromEvent, + accountNameSuggestion, + displayConfirmation, + } = message.params; + + // To keep the retro-compatibility with older keyring-api versions, we mark some fields + // as optional and provide them here if they are missing. + const account = this.#transformAccountFromEvent(newAccountFromEvent); // The UI still uses the account address to identify accounts, so we need // to block the creation of duplicate accounts for now to prevent accounts @@ -223,10 +271,14 @@ export class SnapKeyring extends EventEmitter { message: SnapMessage, ): Promise { assert(message, AccountUpdatedEventStruct); - const { account: newAccount } = message.params; + const { account: newAccountFromEvent } = message.params; const { account: oldAccount } = - this.#accounts.get(snapId, newAccount.id) ?? - throwError(`Account '${newAccount.id}' not found`); + this.#accounts.get(snapId, newAccountFromEvent.id) ?? + throwError(`Account '${newAccountFromEvent.id}' not found`); + + // To keep the retro-compatibility with older keyring-api versions, we mark some fields + // as optional and provide them here if they are missing. + const newAccount = this.#transformAccountFromEvent(newAccountFromEvent); // The address of the account cannot be changed. In the future, we will // support changing the address of an account since it will be required to diff --git a/packages/keyring-snap-bridge/src/account.ts b/packages/keyring-snap-bridge/src/account.ts new file mode 100644 index 00000000..f47ecf19 --- /dev/null +++ b/packages/keyring-snap-bridge/src/account.ts @@ -0,0 +1,19 @@ +import { KeyringAccountStruct } from '@metamask/keyring-api'; +import { object, exactOptional } from '@metamask/keyring-utils'; +import type { Infer } from '@metamask/superstruct'; + +/** + * A `KeyringAccount` with some optional fields which can be used to keep + * the retro-compatility with older version of keyring events. + */ +export const KeyringAccountFromEventStruct = object({ + // Derive from a `KeyringAccount`. + ...KeyringAccountStruct.schema, + + // This has been introduced since: @metamask/keyring-api@>11 + scopes: exactOptional(KeyringAccountStruct.schema.scopes), +}); + +export type KeyringAccountFromEvent = Infer< + typeof KeyringAccountFromEventStruct +>; diff --git a/packages/keyring-snap-bridge/src/events.ts b/packages/keyring-snap-bridge/src/events.ts new file mode 100644 index 00000000..e45e7006 --- /dev/null +++ b/packages/keyring-snap-bridge/src/events.ts @@ -0,0 +1,36 @@ +import { + AccountCreatedEventStruct as OriginalAccountCreatedEventStruct, + AccountUpdatedEventStruct as OriginalAccountUpdatedEventStruct, + AccountDeletedEventStruct, + RequestApprovedEventStruct, + RequestRejectedEventStruct, +} from '@metamask/keyring-api'; +import { object } from '@metamask/keyring-utils'; + +import { KeyringAccountFromEventStruct } from './account'; + +export const AccountCreatedEventStruct = object({ + ...OriginalAccountCreatedEventStruct.schema, + + params: object({ + ...OriginalAccountCreatedEventStruct.schema.params.schema, + account: KeyringAccountFromEventStruct, + }), +}); + +export const AccountUpdatedEventStruct = object({ + ...OriginalAccountUpdatedEventStruct.schema, + + params: object({ + ...OriginalAccountUpdatedEventStruct.schema.params.schema, + account: KeyringAccountFromEventStruct, + }), +}); + +// Re-export the all keyring events, so we don't have to mix between events coming +// from `@metamask/keyring-api` and the local ones: +export { + AccountDeletedEventStruct, + RequestApprovedEventStruct, + RequestRejectedEventStruct, +}; diff --git a/packages/keyring-snap-bridge/src/util.ts b/packages/keyring-snap-bridge/src/util.ts index 8b4ed5f6..512cba0c 100644 --- a/packages/keyring-snap-bridge/src/util.ts +++ b/packages/keyring-snap-bridge/src/util.ts @@ -1,27 +1,5 @@ -import type { Struct } from '@metamask/superstruct'; -import { assert } from '@metamask/superstruct'; import type { Json } from '@metamask/utils'; -/** - * Assert that a value is valid according to a struct. - * - * It is similar to superstruct's mask function, but it does not ignore extra - * properties. - * - * @param value - Value to check. - * @param struct - Struct to validate the value against. - * @param message - Error message to throw if the value is not valid. - * @returns The value if it is valid. - */ -export function strictMask( - value: unknown, - struct: Struct, - message?: string, -): Type { - assert(value, struct, message); - return value; -} - /** * Remove duplicate entries from an array. * diff --git a/packages/keyring-snap-bridge/tsconfig.build.json b/packages/keyring-snap-bridge/tsconfig.build.json index 33492d59..afc49e8e 100644 --- a/packages/keyring-snap-bridge/tsconfig.build.json +++ b/packages/keyring-snap-bridge/tsconfig.build.json @@ -13,7 +13,8 @@ "references": [ { "path": "../keyring-api/tsconfig.build.json" }, { "path": "../keyring-internal-api/tsconfig.build.json" }, - { "path": "../keyring-internal-snap-client/tsconfig.build.json" } + { "path": "../keyring-internal-snap-client/tsconfig.build.json" }, + { "path": "../keyring-utils/tsconfig.build.json" } ], "include": ["./src/**/*.ts"], "exclude": ["./src/**/*.test.ts"] diff --git a/packages/keyring-snap-bridge/tsconfig.json b/packages/keyring-snap-bridge/tsconfig.json index b4ab7b13..60f1bf4a 100644 --- a/packages/keyring-snap-bridge/tsconfig.json +++ b/packages/keyring-snap-bridge/tsconfig.json @@ -9,7 +9,8 @@ "references": [ { "path": "../keyring-api/tsconfig.build.json" }, { "path": "../keyring-internal-api/tsconfig.build.json" }, - { "path": "../keyring-internal-snap-client/tsconfig.build.json" } + { "path": "../keyring-internal-snap-client/tsconfig.build.json" }, + { "path": "../keyring-utils/tsconfig.build.json" } ], "include": ["./src"], "exclude": ["./dist/**/*"] diff --git a/packages/keyring-snap-client/src/KeyringClient.test.ts b/packages/keyring-snap-client/src/KeyringClient.test.ts index 99da8230..0187b95e 100644 --- a/packages/keyring-snap-client/src/KeyringClient.test.ts +++ b/packages/keyring-snap-client/src/KeyringClient.test.ts @@ -26,6 +26,7 @@ describe('KeyringClient', () => { address: '0xE9A74AACd7df8112911ca93260fC5a046f8a64Ae', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', }, ]; @@ -44,11 +45,12 @@ describe('KeyringClient', () => { describe('getAccount', () => { it('should send a request to get an account by ID and return the response', async () => { const id = '49116980-0712-4fa5-b045-e4294f1d440e'; - const expectedResponse = { + const expectedResponse: KeyringAccount = { id: '49116980-0712-4fa5-b045-e4294f1d440e', address: '0xE9A74AACd7df8112911ca93260fC5a046f8a64Ae', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', }; @@ -66,11 +68,12 @@ describe('KeyringClient', () => { describe('createAccount', () => { it('should send a request to create an account and return the response', async () => { - const expectedResponse = { + const expectedResponse: KeyringAccount = { id: '49116980-0712-4fa5-b045-e4294f1d440e', address: '0xE9A74AACd7df8112911ca93260fC5a046f8a64Ae', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', }; @@ -374,6 +377,7 @@ describe('KeyringClient', () => { address: '0xE9A74AACd7df8112911ca93260fC5a046f8a64Ae', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', }; diff --git a/packages/keyring-snap-client/src/KeyringSnapRpcClient.test.ts b/packages/keyring-snap-client/src/KeyringSnapRpcClient.test.ts index b1b471ea..736274c4 100644 --- a/packages/keyring-snap-client/src/KeyringSnapRpcClient.test.ts +++ b/packages/keyring-snap-client/src/KeyringSnapRpcClient.test.ts @@ -12,6 +12,7 @@ describe('KeyringSnapRpcClient', () => { address: '0xE9A74AACd7df8112911ca93260fC5a046f8a64Ae', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', }, ]; diff --git a/packages/keyring-snap-sdk/src/rpc-handler.test.ts b/packages/keyring-snap-sdk/src/rpc-handler.test.ts index 980e85ae..dc0f30eb 100644 --- a/packages/keyring-snap-sdk/src/rpc-handler.test.ts +++ b/packages/keyring-snap-sdk/src/rpc-handler.test.ts @@ -188,6 +188,7 @@ describe('handleKeyringRequest', () => { address: '0x0', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', }, }, @@ -201,6 +202,7 @@ describe('handleKeyringRequest', () => { address: '0x0', options: {}, methods: [], + scopes: ['eip155'], type: 'eip155:eoa', }); expect(result).toBe('UpdateAccount result'); diff --git a/yarn.lock b/yarn.lock index fdee2dff..e8430b2e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2001,6 +2001,7 @@ __metadata: "@metamask/keyring-api": "workspace:^" "@metamask/keyring-internal-api": "workspace:^" "@metamask/keyring-internal-snap-client": "workspace:^" + "@metamask/keyring-utils": "workspace:^" "@metamask/providers": "npm:^18.1.0" "@metamask/snaps-controllers": "npm:^9.10.0" "@metamask/snaps-sdk": "npm:^6.7.0"