From 4402125496136fed7eb1b0c6f3b5d3ff158abd42 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 5 Nov 2024 14:58:02 +0100 Subject: [PATCH 01/35] wip: Add MultichainRoutingController --- .../multichain/MultichainRoutingController.ts | 165 ++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 packages/snaps-controllers/src/multichain/MultichainRoutingController.ts diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts new file mode 100644 index 0000000000..1a0fc083c2 --- /dev/null +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -0,0 +1,165 @@ +import type { + RestrictedControllerMessenger, + ControllerGetStateAction, + ControllerStateChangeEvent, +} from '@metamask/base-controller'; +import { BaseController } from '@metamask/base-controller'; +import type { + Caveat, + GetPermissions, + ValidPermission, +} from '@metamask/permission-controller'; +import { rpcErrors } from '@metamask/rpc-errors'; +import { SnapEndowments } from '@metamask/snaps-rpc-methods'; +import type { + EmptyObject, + Json, + JsonRpcRequest, + SnapId, +} from '@metamask/snaps-sdk'; +import { HandlerType, type Caip2ChainId } from '@metamask/snaps-utils'; +import { hasProperty } from '@metamask/utils'; + +import { getRunnableSnaps } from '../snaps'; +import type { GetAllSnaps, HandleSnapRequest } from '../snaps'; + +export type MultichainRoutingControllerGetStateAction = + ControllerGetStateAction< + typeof controllerName, + MultichainRoutingControllerState + >; +export type MultichainRoutingControllerStateChangeEvent = + ControllerStateChangeEvent< + typeof controllerName, + MultichainRoutingControllerState + >; + +// Since the AccountsController depends on snaps-controllers we manually type this +type InternalAccount = { + id: string; + type: string; + address: string; + options: Record; + methods: string[]; +}; + +export type AccountsControllerListMultichainAccountsAction = { + type: `AccountsController:listMultichainAccounts`; + handler: (chainId?: Caip2ChainId) => InternalAccount[]; +}; + +export type MultichainRoutingControllerActions = + | GetAllSnaps + | HandleSnapRequest + | GetPermissions + | AccountsControllerListMultichainAccountsAction + | MultichainRoutingControllerGetStateAction; + +export type MultichainRoutingControllerEvents = + MultichainRoutingControllerStateChangeEvent; + +export type MultichainRoutingControllerMessenger = + RestrictedControllerMessenger< + typeof controllerName, + MultichainRoutingControllerActions, + MultichainRoutingControllerEvents, + MultichainRoutingControllerActions['type'], + MultichainRoutingControllerEvents['type'] + >; + +export type MultichainRoutingControllerArgs = { + messenger: MultichainRoutingControllerMessenger; + state?: MultichainRoutingControllerState; +}; + +export type MultichainRoutingControllerState = EmptyObject; + +type SnapWithPermission = { + snapId: SnapId; + permission: ValidPermission>; +}; + +const controllerName = 'MultichainRoutingController'; + +export class MultichainRoutingController extends BaseController< + typeof controllerName, + MultichainRoutingControllerState, + MultichainRoutingControllerMessenger +> { + constructor({ messenger, state }: MultichainRoutingControllerArgs) { + super({ + messenger, + metadata: {}, + name: controllerName, + state: { + ...state, + }, + }); + } + + #getAccountSnapMethods(chainId: Caip2ChainId) { + const accounts = this.messagingSystem.call( + 'AccountsController:listMultichainAccounts', + chainId, + ); + + return accounts.flatMap((account) => account.methods); + } + + #getProtocolSnaps(_chainId: Caip2ChainId, _method: string) { + const allSnaps = this.messagingSystem.call('SnapController:getAll'); + const filteredSnaps = getRunnableSnaps(allSnaps); + + return filteredSnaps.reduce((accumulator, snap) => { + const permissions = this.messagingSystem.call( + 'PermissionController:getPermissions', + snap.id, + ); + // TODO: Protocol Snap export + // TODO: Filter based on chain ID and method + if (permissions && hasProperty(permissions, SnapEndowments.Rpc)) { + accumulator.push({ + snapId: snap.id, + permission: permissions[SnapEndowments.Rpc], + }); + } + + return accumulator; + }, []); + } + + async handleRequest({ + chainId, + request, + }: { + origin: string; + chainId: Caip2ChainId; + request: JsonRpcRequest; + }) { + // TODO: Determine if the request is already validated here? + const { method } = request; + + // If the RPC request can be serviced by an account Snap, route it there. + const accountMethods = this.#getAccountSnapMethods(chainId); + if (accountMethods.includes(method)) { + // TODO: Determine how to call the AccountsRouter + return null; + } + + // If the RPC request cannot be serviced by an account Snap, + // but has a protocol Snap available, route it there. + const protocolSnaps = this.#getProtocolSnaps(chainId, method); + const snapId = protocolSnaps[0]?.snapId; + if (snapId) { + return this.messagingSystem.call('SnapController:handleRequest', { + snapId, + origin: 'metamask', // TODO: Determine origin of these requests? + request, + handler: HandlerType.OnRpcRequest, // TODO: Protocol Snap export + }); + } + + // If no compatible account or protocol Snaps were found, throw. + throw rpcErrors.methodNotFound(); + } +} From 9968b13c45345fe83a1e5946c78c4e3e8ae7c1cd Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 5 Nov 2024 16:43:21 +0100 Subject: [PATCH 02/35] Add onProtocolRequest export --- .../multichain/MultichainRoutingController.ts | 28 ++- .../src/common/commands.ts | 1 + .../snaps-rpc-methods/src/endowments/enum.ts | 1 + .../snaps-rpc-methods/src/endowments/index.ts | 15 ++ .../src/endowments/protocol.ts | 173 ++++++++++++++++++ .../snaps-sdk/src/types/handlers/index.ts | 1 + .../snaps-sdk/src/types/handlers/protocol.ts | 23 +++ packages/snaps-utils/src/caveats.ts | 5 + packages/snaps-utils/src/handler-types.ts | 1 + packages/snaps-utils/src/handlers.ts | 16 +- .../snaps-utils/src/manifest/validation.ts | 9 + 11 files changed, 261 insertions(+), 12 deletions(-) create mode 100644 packages/snaps-rpc-methods/src/endowments/protocol.ts create mode 100644 packages/snaps-sdk/src/types/handlers/protocol.ts diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index 1a0fc083c2..974fd36a2c 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -10,7 +10,11 @@ import type { ValidPermission, } from '@metamask/permission-controller'; import { rpcErrors } from '@metamask/rpc-errors'; -import { SnapEndowments } from '@metamask/snaps-rpc-methods'; +import { + getProtocolCaveatChainIds, + getProtocolCaveatRpcMethods, + SnapEndowments, +} from '@metamask/snaps-rpc-methods'; import type { EmptyObject, Json, @@ -106,7 +110,7 @@ export class MultichainRoutingController extends BaseController< return accounts.flatMap((account) => account.methods); } - #getProtocolSnaps(_chainId: Caip2ChainId, _method: string) { + #getProtocolSnaps(chainId: Caip2ChainId, method: string) { const allSnaps = this.messagingSystem.call('SnapController:getAll'); const filteredSnaps = getRunnableSnaps(allSnaps); @@ -115,13 +119,17 @@ export class MultichainRoutingController extends BaseController< 'PermissionController:getPermissions', snap.id, ); - // TODO: Protocol Snap export - // TODO: Filter based on chain ID and method - if (permissions && hasProperty(permissions, SnapEndowments.Rpc)) { - accumulator.push({ - snapId: snap.id, - permission: permissions[SnapEndowments.Rpc], - }); + if (permissions && hasProperty(permissions, SnapEndowments.Protocol)) { + const permission = permissions[SnapEndowments.Protocol]; + const chains = getProtocolCaveatChainIds(permission); + const methods = getProtocolCaveatRpcMethods(permission); + // TODO: This may need to be more complicated depending on the decided format. + if (chains?.includes(chainId) && methods?.includes(method)) { + accumulator.push({ + snapId: snap.id, + permission, + }); + } } return accumulator; @@ -155,7 +163,7 @@ export class MultichainRoutingController extends BaseController< snapId, origin: 'metamask', // TODO: Determine origin of these requests? request, - handler: HandlerType.OnRpcRequest, // TODO: Protocol Snap export + handler: HandlerType.OnProtocolRequest, }); } diff --git a/packages/snaps-execution-environments/src/common/commands.ts b/packages/snaps-execution-environments/src/common/commands.ts index 8dbfac295f..227b698a0d 100644 --- a/packages/snaps-execution-environments/src/common/commands.ts +++ b/packages/snaps-execution-environments/src/common/commands.ts @@ -88,6 +88,7 @@ export function getHandlerArguments( } case HandlerType.OnRpcRequest: case HandlerType.OnKeyringRequest: + case HandlerType.OnProtocolRequest: // TODO: Decide on origin return { origin, request }; case HandlerType.OnCronjob: diff --git a/packages/snaps-rpc-methods/src/endowments/enum.ts b/packages/snaps-rpc-methods/src/endowments/enum.ts index 96f4179fc6..e3f24b4de5 100644 --- a/packages/snaps-rpc-methods/src/endowments/enum.ts +++ b/packages/snaps-rpc-methods/src/endowments/enum.ts @@ -12,4 +12,5 @@ export enum SnapEndowments { HomePage = 'endowment:page-home', SettingsPage = 'endowment:page-settings', Assets = 'endowment:assets', + Protocol = 'endowment:protocol', } diff --git a/packages/snaps-rpc-methods/src/endowments/index.ts b/packages/snaps-rpc-methods/src/endowments/index.ts index e0767cedd7..7cdd9d06a8 100644 --- a/packages/snaps-rpc-methods/src/endowments/index.ts +++ b/packages/snaps-rpc-methods/src/endowments/index.ts @@ -27,6 +27,11 @@ import { nameLookupEndowmentBuilder, } from './name-lookup'; import { networkAccessEndowmentBuilder } from './network-access'; +import { + getProtocolCaveatMapper, + protocolCaveatSpecifications, + protocolEndowmentBuilder, +} from './protocol'; import { getRpcCaveatMapper, rpcCaveatSpecifications, @@ -58,6 +63,7 @@ export const endowmentPermissionBuilders = { [lifecycleHooksEndowmentBuilder.targetName]: lifecycleHooksEndowmentBuilder, [keyringEndowmentBuilder.targetName]: keyringEndowmentBuilder, [settingsPageEndowmentBuilder.targetName]: settingsPageEndowmentBuilder, + [protocolEndowmentBuilder.targetName]: protocolEndowmentBuilder, [homePageEndowmentBuilder.targetName]: homePageEndowmentBuilder, [signatureInsightEndowmentBuilder.targetName]: signatureInsightEndowmentBuilder, @@ -72,6 +78,7 @@ export const endowmentCaveatSpecifications = { ...keyringCaveatSpecifications, ...signatureInsightCaveatSpecifications, ...maxRequestTimeCaveatSpecifications, + ...protocolCaveatSpecifications, }; export const endowmentCaveatMappers: Record< @@ -92,6 +99,9 @@ export const endowmentCaveatMappers: Record< [keyringEndowmentBuilder.targetName]: createMaxRequestTimeMapper( getKeyringCaveatMapper, ), + [protocolEndowmentBuilder.targetName]: createMaxRequestTimeMapper( + getProtocolCaveatMapper, + ), [signatureInsightEndowmentBuilder.targetName]: createMaxRequestTimeMapper( getSignatureInsightCaveatMapper, ), @@ -115,6 +125,7 @@ export const handlerEndowments: Record = { [HandlerType.OnHomePage]: homePageEndowmentBuilder.targetName, [HandlerType.OnSettingsPage]: settingsPageEndowmentBuilder.targetName, [HandlerType.OnSignature]: signatureInsightEndowmentBuilder.targetName, + [HandlerType.OnProtocolRequest]: protocolEndowmentBuilder.targetName, [HandlerType.OnUserInput]: null, [HandlerType.OnAssetsLookup]: assetsEndowmentBuilder.targetName, [HandlerType.OnAssetsConversion]: assetsEndowmentBuilder.targetName, @@ -128,3 +139,7 @@ export { getChainIdsCaveat, getLookupMatchersCaveat } from './name-lookup'; export { getKeyringCaveatOrigins } from './keyring'; export { getMaxRequestTimeCaveat } from './caveats'; export { getCronjobCaveatJobs } from './cronjob'; +export { + getProtocolCaveatChainIds, + getProtocolCaveatRpcMethods, +} from './protocol'; diff --git a/packages/snaps-rpc-methods/src/endowments/protocol.ts b/packages/snaps-rpc-methods/src/endowments/protocol.ts new file mode 100644 index 0000000000..5cbddd2576 --- /dev/null +++ b/packages/snaps-rpc-methods/src/endowments/protocol.ts @@ -0,0 +1,173 @@ +import type { + Caveat, + CaveatConstraint, + CaveatSpecificationConstraint, + EndowmentGetterParams, + PermissionConstraint, + PermissionSpecificationBuilder, + PermissionValidatorConstraint, + ValidPermissionSpecification, +} from '@metamask/permission-controller'; +import { PermissionType, SubjectType } from '@metamask/permission-controller'; +import { rpcErrors } from '@metamask/rpc-errors'; +import { + ProtocolRpcMethodsStruct, + SnapCaveatType, +} from '@metamask/snaps-utils'; +import type { Json, NonEmptyArray } from '@metamask/utils'; +import { + assertStruct, + hasProperty, + isObject, + isPlainObject, +} from '@metamask/utils'; + +import { createGenericPermissionValidator } from './caveats'; +import { SnapEndowments } from './enum'; + +const permissionName = SnapEndowments.Protocol; + +type ProtocolEndowmentSpecification = ValidPermissionSpecification<{ + permissionType: PermissionType.Endowment; + targetName: typeof permissionName; + endowmentGetter: (_options?: EndowmentGetterParams) => null; + allowedCaveats: Readonly> | null; + validator: PermissionValidatorConstraint; + subjectTypes: readonly SubjectType[]; +}>; + +/** + * `endowment:protocol` returns nothing; it is intended to be used as a flag + * by the client to detect whether the Snap supports the Protocol API. + * + * @param _builderOptions - Optional specification builder options. + * @returns The specification for the accounts chain endowment. + */ +const specificationBuilder: PermissionSpecificationBuilder< + PermissionType.Endowment, + any, + ProtocolEndowmentSpecification +> = (_builderOptions?: unknown) => { + return { + permissionType: PermissionType.Endowment, + targetName: permissionName, + allowedCaveats: [ + SnapCaveatType.KeyringOrigin, + SnapCaveatType.ChainIds, + SnapCaveatType.SnapRpcMethods, + SnapCaveatType.MaxRequestTime, + ], + endowmentGetter: (_getterOptions?: EndowmentGetterParams) => null, + validator: createGenericPermissionValidator([ + { type: SnapCaveatType.ChainIds }, + { type: SnapCaveatType.SnapRpcMethods }, + { type: SnapCaveatType.MaxRequestTime, optional: true }, + ]), + subjectTypes: [SubjectType.Snap], + }; +}; + +export const protocolEndowmentBuilder = Object.freeze({ + targetName: permissionName, + specificationBuilder, +} as const); + +/** + * Map a raw value from the `initialPermissions` to a caveat specification. + * Note that this function does not do any validation, that's handled by the + * PermissionsController when the permission is requested. + * + * @param value - The raw value from the `initialPermissions`. + * @returns The caveat specification. + */ +export function getProtocolCaveatMapper( + value: Json, +): Pick { + if (!value || !isObject(value) || Object.keys(value).length === 0) { + return { caveats: null }; + } + + const caveats = []; + + if (value.chains) { + caveats.push({ + type: SnapCaveatType.ChainIds, + value: value.chains, + }); + } + + if (value.methods) { + caveats.push({ + type: SnapCaveatType.SnapRpcMethods, + value: value.methods, + }); + } + + return { caveats: caveats as NonEmptyArray }; +} + +/** + * Getter function to get the {@link ChainIds} caveat value from a + * permission. + * + * @param permission - The permission to get the caveat value from. + * @returns The caveat value. + */ +export function getProtocolCaveatChainIds( + permission?: PermissionConstraint, +): string[] | null { + const caveat = permission?.caveats?.find( + (permCaveat) => permCaveat.type === SnapCaveatType.ChainIds, + ) as Caveat | undefined; + + return caveat ? caveat.value : null; +} + +/** + * Getter function to get the {@link SnapRpcMethods} caveat value from a + * permission. + * + * @param permission - The permission to get the caveat value from. + * @returns The caveat value. + */ +export function getProtocolCaveatRpcMethods( + permission?: PermissionConstraint, +): string[] | null { + const caveat = permission?.caveats?.find( + (permCaveat) => permCaveat.type === SnapCaveatType.SnapRpcMethods, + ) as Caveat | undefined; + + return caveat ? caveat.value : null; +} + +/** + * Validates the type of the caveat value. + * + * @param caveat - The caveat to validate. + * @throws If the caveat value is invalid. + */ +function validateCaveat(caveat: Caveat): void { + if (!hasProperty(caveat, 'value') || !isPlainObject(caveat)) { + throw rpcErrors.invalidParams({ + message: 'Expected a plain object.', + }); + } + + const { value } = caveat; + assertStruct( + value, + ProtocolRpcMethodsStruct, + 'Invalid RPC methods specified', + rpcErrors.invalidParams, + ); +} + +export const protocolCaveatSpecifications: Record< + SnapCaveatType.SnapRpcMethods, + CaveatSpecificationConstraint +> = { + [SnapCaveatType.SnapRpcMethods]: Object.freeze({ + type: SnapCaveatType.SnapRpcMethods, + validator: (caveat: Caveat) => validateCaveat(caveat), + }), +}; diff --git a/packages/snaps-sdk/src/types/handlers/index.ts b/packages/snaps-sdk/src/types/handlers/index.ts index 33fd11ac75..5961304c08 100644 --- a/packages/snaps-sdk/src/types/handlers/index.ts +++ b/packages/snaps-sdk/src/types/handlers/index.ts @@ -5,6 +5,7 @@ export * from './home-page'; export * from './keyring'; export * from './lifecycle'; export * from './name-lookup'; +export * from './protocol'; export * from './rpc-request'; export * from './transaction'; export * from './signature'; diff --git a/packages/snaps-sdk/src/types/handlers/protocol.ts b/packages/snaps-sdk/src/types/handlers/protocol.ts new file mode 100644 index 0000000000..601b6cf926 --- /dev/null +++ b/packages/snaps-sdk/src/types/handlers/protocol.ts @@ -0,0 +1,23 @@ +import type { Json, JsonRpcParams, JsonRpcRequest } from '@metamask/utils'; + +/** + * The `onProtocolRequest` handler, which is called when a Snap receives a + * protocol request. + * + * Note that using this handler requires the `endowment:protocol` permission. + * + * @param args - The request arguments. + * @param args.origin - The origin of the request. This can be the ID of another + * Snap, or the URL of a website. + * @param args.request - The protocol request sent to the Snap. This includes + * the method name and parameters. + * @returns The response to the protocol request. This must be a + * JSON-serializable value. In order to return an error, throw a `SnapError` + * instead. + */ +export type OnProtocolRequestHandler< + Params extends JsonRpcParams = JsonRpcParams, +> = (args: { + origin: string; + request: JsonRpcRequest; +}) => Promise; diff --git a/packages/snaps-utils/src/caveats.ts b/packages/snaps-utils/src/caveats.ts index 61bd80910e..7599e95b4c 100644 --- a/packages/snaps-utils/src/caveats.ts +++ b/packages/snaps-utils/src/caveats.ts @@ -53,4 +53,9 @@ export enum SnapCaveatType { * Caveat specifying the max request time for a handler endowment. */ MaxRequestTime = 'maxRequestTime', + + /** + * Caveat specifying a list of RPC methods serviced by an endowment. + */ + SnapRpcMethods = 'snapRpcMethods', } diff --git a/packages/snaps-utils/src/handler-types.ts b/packages/snaps-utils/src/handler-types.ts index 392969898b..210ac70115 100644 --- a/packages/snaps-utils/src/handler-types.ts +++ b/packages/snaps-utils/src/handler-types.ts @@ -12,6 +12,7 @@ export enum HandlerType { OnUserInput = 'onUserInput', OnAssetsLookup = 'onAssetsLookup', OnAssetsConversion = 'onAssetsConversion', + OnProtocolRequest = 'onProtocolRequest', } export type SnapHandler = { diff --git a/packages/snaps-utils/src/handlers.ts b/packages/snaps-utils/src/handlers.ts index f2de2b6e2a..52675564cd 100644 --- a/packages/snaps-utils/src/handlers.ts +++ b/packages/snaps-utils/src/handlers.ts @@ -1,9 +1,12 @@ import type { + OnAssetsConversionHandler, + OnAssetsLookupHandler, OnCronjobHandler, OnHomePageHandler, OnInstallHandler, OnKeyringRequestHandler, OnNameLookupHandler, + OnProtocolRequestHandler, OnRpcRequestHandler, OnSettingsPageHandler, OnSignatureHandler, @@ -114,17 +117,26 @@ export const SNAP_EXPORTS = { [HandlerType.OnAssetsLookup]: { type: HandlerType.OnAssetsLookup, required: true, - validator: (snapExport: unknown): snapExport is OnUserInputHandler => { + validator: (snapExport: unknown): snapExport is OnAssetsLookupHandler => { return typeof snapExport === 'function'; }, }, [HandlerType.OnAssetsConversion]: { type: HandlerType.OnAssetsConversion, required: true, - validator: (snapExport: unknown): snapExport is OnUserInputHandler => { + validator: (snapExport: unknown): snapExport is OnAssetsConversionHandler => { return typeof snapExport === 'function'; }, }, + [HandlerType.OnProtocolRequest]: { + type: HandlerType.OnProtocolRequest, + required: false, + validator: ( + snapExport: unknown, + ): snapExport is OnProtocolRequestHandler => { + return typeof snapExport === 'function'; + }, + } } as const; export const OnTransactionSeverityResponseStruct = object({ diff --git a/packages/snaps-utils/src/manifest/validation.ts b/packages/snaps-utils/src/manifest/validation.ts index 2721bb2cc8..1314dbc2a5 100644 --- a/packages/snaps-utils/src/manifest/validation.ts +++ b/packages/snaps-utils/src/manifest/validation.ts @@ -174,6 +174,9 @@ export const MaxRequestTimeStruct = size( MAXIMUM_REQUEST_TIMEOUT, ); +// TODO: Decide on the format for this +export const ProtocolRpcMethodsStruct = array(string()); + // Utility type to union with for all handler structs export const HandlerCaveatsStruct = object({ maxRequestTime: optional(MaxRequestTimeStruct), @@ -201,6 +204,12 @@ export const PermissionsStruct: Describe = type({ 'endowment:keyring': optional( mergeStructs(HandlerCaveatsStruct, KeyringOriginsStruct), ), + 'endowment:protocol': optional( + mergeStructs( + HandlerCaveatsStruct, + object({ chains: ChainIdsStruct, methods: ProtocolRpcMethodsStruct }), + ), + ), 'endowment:lifecycle-hooks': optional(HandlerCaveatsStruct), 'endowment:name-lookup': optional( mergeStructs( From 7586814445ee8337732a13df1f0614908746a215 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 7 Nov 2024 12:23:26 +0100 Subject: [PATCH 03/35] Add address resolving logic --- .../multichain/MultichainRoutingController.ts | 123 +++++++++++++++--- .../snaps-controllers/src/multichain/index.ts | 1 + .../snaps-sdk/src/types/handlers/protocol.ts | 9 +- 3 files changed, 112 insertions(+), 21 deletions(-) create mode 100644 packages/snaps-controllers/src/multichain/index.ts diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index 974fd36a2c..6d7e6a14d6 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -21,7 +21,8 @@ import type { JsonRpcRequest, SnapId, } from '@metamask/snaps-sdk'; -import { HandlerType, type Caip2ChainId } from '@metamask/snaps-utils'; +import { HandlerType } from '@metamask/snaps-utils'; +import type { CaipChainId } from '@metamask/utils'; import { hasProperty } from '@metamask/utils'; import { getRunnableSnaps } from '../snaps'; @@ -45,11 +46,15 @@ type InternalAccount = { address: string; options: Record; methods: string[]; + metadata: { + name: string; + snap?: { id: SnapId; enabled: boolean; name: string }; + }; }; export type AccountsControllerListMultichainAccountsAction = { type: `AccountsController:listMultichainAccounts`; - handler: (chainId?: Caip2ChainId) => InternalAccount[]; + handler: (chainId?: CaipChainId) => InternalAccount[]; }; export type MultichainRoutingControllerActions = @@ -101,16 +106,75 @@ export class MultichainRoutingController extends BaseController< }); } - #getAccountSnapMethods(chainId: Caip2ChainId) { - const accounts = this.messagingSystem.call( - 'AccountsController:listMultichainAccounts', + async #resolveRequestAddress( + snapId: SnapId, + chainId: CaipChainId, + request: JsonRpcRequest, + ) { + const result = (await this.messagingSystem.call( + 'SnapController:handleRequest', + { + snapId, + origin: 'metamask', + request: { + method: '', + params: { + chainId, + request, + }, + }, + handler: HandlerType.OnProtocolRequest, // TODO: Export and request format + }, + )) as { address: string } | null; + return result?.address; + } + + async #getAccountSnap( + protocolSnapId: SnapId, + chainId: CaipChainId, + request: JsonRpcRequest, + ) { + const accounts = this.messagingSystem + .call('AccountsController:listMultichainAccounts', chainId) + .filter( + (account) => + account.metadata.snap?.enabled && + account.methods.includes(request.method), + ); + + // If no accounts can service the request, return null. + if (accounts.length === 0) { + return null; + } + + // Attempt to resolve the address that should be used for signing. + const address = await this.#resolveRequestAddress( + protocolSnapId, chainId, + request, ); - return accounts.flatMap((account) => account.methods); + if (!address) { + throw rpcErrors.invalidParams(); + } + + // TODO: Decide what happens if we have more than one possible account. + const selectedAccount = accounts.find( + (account) => account.address.toLowerCase() === address.toLowerCase(), + ); + + if (!selectedAccount) { + throw rpcErrors.invalidParams(); + } + + return { + address: selectedAccount.address, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + snapId: selectedAccount.metadata.snap!.id, + }; } - #getProtocolSnaps(chainId: Caip2ChainId, method: string) { + #getProtocolSnaps(chainId: CaipChainId) { const allSnaps = this.messagingSystem.call('SnapController:getAll'); const filteredSnaps = getRunnableSnaps(allSnaps); @@ -122,9 +186,7 @@ export class MultichainRoutingController extends BaseController< if (permissions && hasProperty(permissions, SnapEndowments.Protocol)) { const permission = permissions[SnapEndowments.Protocol]; const chains = getProtocolCaveatChainIds(permission); - const methods = getProtocolCaveatRpcMethods(permission); - // TODO: This may need to be more complicated depending on the decided format. - if (chains?.includes(chainId) && methods?.includes(method)) { + if (chains?.includes(chainId)) { accumulator.push({ snapId: snap.id, permission, @@ -141,28 +203,49 @@ export class MultichainRoutingController extends BaseController< request, }: { origin: string; - chainId: Caip2ChainId; + chainId: CaipChainId; request: JsonRpcRequest; }) { // TODO: Determine if the request is already validated here? const { method } = request; + const protocolSnaps = this.#getProtocolSnaps(chainId); + if (protocolSnaps.length === 0) { + throw rpcErrors.methodNotFound(); + } + // If the RPC request can be serviced by an account Snap, route it there. - const accountMethods = this.#getAccountSnapMethods(chainId); - if (accountMethods.includes(method)) { - // TODO: Determine how to call the AccountsRouter - return null; + const accountSnap = await this.#getAccountSnap( + protocolSnaps[0].snapId, + chainId, + request, + ); + if (accountSnap) { + return this.messagingSystem.call('SnapController:handleRequest', { + snapId: accountSnap.snapId, + origin: 'metamask', // TODO: Determine origin of these requests? + request, + handler: HandlerType.OnKeyringRequest, + }); } // If the RPC request cannot be serviced by an account Snap, // but has a protocol Snap available, route it there. - const protocolSnaps = this.#getProtocolSnaps(chainId, method); - const snapId = protocolSnaps[0]?.snapId; - if (snapId) { + // TODO: This may need to be more complicated depending on the decided format. + const protocolSnap = protocolSnaps.find((snap) => + getProtocolCaveatRpcMethods(snap.permission)?.includes(method), + ); + if (protocolSnap) { return this.messagingSystem.call('SnapController:handleRequest', { - snapId, + snapId: protocolSnap.snapId, origin: 'metamask', // TODO: Determine origin of these requests? - request, + request: { + method: '', + params: { + request, + chainId, + }, + }, handler: HandlerType.OnProtocolRequest, }); } diff --git a/packages/snaps-controllers/src/multichain/index.ts b/packages/snaps-controllers/src/multichain/index.ts new file mode 100644 index 0000000000..6c07180225 --- /dev/null +++ b/packages/snaps-controllers/src/multichain/index.ts @@ -0,0 +1 @@ +export * from './MultichainRoutingController'; diff --git a/packages/snaps-sdk/src/types/handlers/protocol.ts b/packages/snaps-sdk/src/types/handlers/protocol.ts index 601b6cf926..4d0c86d992 100644 --- a/packages/snaps-sdk/src/types/handlers/protocol.ts +++ b/packages/snaps-sdk/src/types/handlers/protocol.ts @@ -1,4 +1,9 @@ -import type { Json, JsonRpcParams, JsonRpcRequest } from '@metamask/utils'; +import type { + CaipChainId, + Json, + JsonRpcParams, + JsonRpcRequest, +} from '@metamask/utils'; /** * The `onProtocolRequest` handler, which is called when a Snap receives a @@ -9,6 +14,7 @@ import type { Json, JsonRpcParams, JsonRpcRequest } from '@metamask/utils'; * @param args - The request arguments. * @param args.origin - The origin of the request. This can be the ID of another * Snap, or the URL of a website. + * @param args.chainId - The chain ID of the request. * @param args.request - The protocol request sent to the Snap. This includes * the method name and parameters. * @returns The response to the protocol request. This must be a @@ -19,5 +25,6 @@ export type OnProtocolRequestHandler< Params extends JsonRpcParams = JsonRpcParams, > = (args: { origin: string; + chainId: CaipChainId; request: JsonRpcRequest; }) => Promise; From 37ea7768dcc48f59e6f0873ce03304fa29126a71 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 7 Nov 2024 13:01:18 +0100 Subject: [PATCH 04/35] Rethrow error --- .../multichain/MultichainRoutingController.ts | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index 6d7e6a14d6..5bdc19c2bb 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -111,22 +111,26 @@ export class MultichainRoutingController extends BaseController< chainId: CaipChainId, request: JsonRpcRequest, ) { - const result = (await this.messagingSystem.call( - 'SnapController:handleRequest', - { - snapId, - origin: 'metamask', - request: { - method: '', - params: { - chainId, - request, + try { + const result = (await this.messagingSystem.call( + 'SnapController:handleRequest', + { + snapId, + origin: 'metamask', + request: { + method: '', + params: { + chainId, + request, + }, }, + handler: HandlerType.OnProtocolRequest, // TODO: Export and request format }, - handler: HandlerType.OnProtocolRequest, // TODO: Export and request format - }, - )) as { address: string } | null; - return result?.address; + )) as { address: string } | null; + return result?.address; + } catch { + throw rpcErrors.internal(); + } } async #getAccountSnap( From 223c2c9e21c232bc39a00f574de3ae9dbe0cbd44 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 8 Nov 2024 13:22:22 +0100 Subject: [PATCH 05/35] Use account Snaps for address resolution + consider connected accounts --- .../multichain/MultichainRoutingController.ts | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index 5bdc19c2bb..8afaf2bcbd 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -22,7 +22,11 @@ import type { SnapId, } from '@metamask/snaps-sdk'; import { HandlerType } from '@metamask/snaps-utils'; -import type { CaipChainId } from '@metamask/utils'; +import type { + CaipAccountAddress, + CaipAccountId, + CaipChainId, +} from '@metamask/utils'; import { hasProperty } from '@metamask/utils'; import { getRunnableSnaps } from '../snaps'; @@ -134,7 +138,7 @@ export class MultichainRoutingController extends BaseController< } async #getAccountSnap( - protocolSnapId: SnapId, + connectedAddresses: CaipAccountAddress[], chainId: CaipChainId, request: JsonRpcRequest, ) { @@ -144,27 +148,31 @@ export class MultichainRoutingController extends BaseController< (account) => account.metadata.snap?.enabled && account.methods.includes(request.method), - ); + ) as (InternalAccount & { + metadata: Required; + })[]; // If no accounts can service the request, return null. if (accounts.length === 0) { return null; } + const resolutionSnapId = accounts[0].metadata.snap.id; + // Attempt to resolve the address that should be used for signing. const address = await this.#resolveRequestAddress( - protocolSnapId, + resolutionSnapId, chainId, request, ); - if (!address) { - throw rpcErrors.invalidParams(); - } - - // TODO: Decide what happens if we have more than one possible account. + // If we have a resolved address, try to find the selected account based on that + // otherwise, default to one of the connected accounts. + // TODO: Eventually let the user choose if we have more than one option for the account. const selectedAccount = accounts.find( - (account) => account.address.toLowerCase() === address.toLowerCase(), + (account) => + connectedAddresses.includes(account.address) && + (!address || account.address.toLowerCase() === address.toLowerCase()), ); if (!selectedAccount) { @@ -173,8 +181,7 @@ export class MultichainRoutingController extends BaseController< return { address: selectedAccount.address, - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - snapId: selectedAccount.metadata.snap!.id, + snapId: selectedAccount.metadata.snap.id, }; } @@ -203,9 +210,11 @@ export class MultichainRoutingController extends BaseController< } async handleRequest({ + connectedAddresses, chainId, request, }: { + connectedAddresses: CaipAccountId[]; origin: string; chainId: CaipChainId; request: JsonRpcRequest; @@ -213,14 +222,9 @@ export class MultichainRoutingController extends BaseController< // TODO: Determine if the request is already validated here? const { method } = request; - const protocolSnaps = this.#getProtocolSnaps(chainId); - if (protocolSnaps.length === 0) { - throw rpcErrors.methodNotFound(); - } - // If the RPC request can be serviced by an account Snap, route it there. const accountSnap = await this.#getAccountSnap( - protocolSnaps[0].snapId, + connectedAddresses, chainId, request, ); @@ -235,7 +239,7 @@ export class MultichainRoutingController extends BaseController< // If the RPC request cannot be serviced by an account Snap, // but has a protocol Snap available, route it there. - // TODO: This may need to be more complicated depending on the decided format. + const protocolSnaps = this.#getProtocolSnaps(chainId); const protocolSnap = protocolSnaps.find((snap) => getProtocolCaveatRpcMethods(snap.permission)?.includes(method), ); From a16ea8423f1acaedd67ecd9faecf1c7efcf6a4ad Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 12 Nov 2024 11:48:03 +0100 Subject: [PATCH 06/35] Add basic tests --- .../MultichainRoutingController.test.ts | 187 ++++++++++++++++++ .../multichain/MultichainRoutingController.ts | 50 +++-- .../src/snaps/SnapController.test.tsx | 2 +- .../src/test-utils/controller.ts | 41 ++++ .../snaps-controllers/src/test-utils/index.ts | 1 + .../src/test-utils/multichain.ts | 97 +++++++++ .../src/endowments/protocol.ts | 1 - .../snaps-rpc-methods/src/permissions.test.ts | 14 ++ 8 files changed, 374 insertions(+), 19 deletions(-) create mode 100644 packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts create mode 100644 packages/snaps-controllers/src/test-utils/multichain.ts diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts new file mode 100644 index 0000000000..afe09275ba --- /dev/null +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts @@ -0,0 +1,187 @@ +import { HandlerType } from '@metamask/snaps-utils'; +import { getTruncatedSnap } from '@metamask/snaps-utils/test-utils'; + +import { + getRootMultichainRoutingControllerMessenger, + getRestrictedMultichainRoutingControllerMessenger, + BTC_CAIP2, + BTC_CONNECTED_ACCOUNTS, + MOCK_SOLANA_SNAP_PERMISSIONS, + SOLANA_CONNECTED_ACCOUNTS, + SOLANA_CAIP2, + MOCK_SOLANA_ACCOUNTS, + MOCK_BTC_ACCOUNTS, +} from '../test-utils'; +import { MultichainRoutingController } from './MultichainRoutingController'; + +describe('MultichainRoutingController', () => { + it('can route signing requests to account Snaps without address resolution', async () => { + const rootMessenger = getRootMultichainRoutingControllerMessenger(); + const messenger = + getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRoutingController({ messenger }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_BTC_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'SnapController:handleRequest', + async ({ handler }) => { + // TODO: Use proper handler + if (handler === HandlerType.OnProtocolRequest) { + return null; + } else if (handler === HandlerType.OnKeyringRequest) { + return { + txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', + }; + } + throw new Error('Unmocked request'); + }, + ); + + const result = await messenger.call( + 'MultichainRoutingController:handleRequest', + { + connectedAddresses: BTC_CONNECTED_ACCOUNTS, + chainId: BTC_CAIP2, + request: { + method: 'btc_sendmany', + params: { + message: 'foo', + }, + }, + }, + ); + + expect(result).toStrictEqual({ + txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', + }); + }); + + it('can route signing requests to account Snaps using address resolution', async () => { + const rootMessenger = getRootMultichainRoutingControllerMessenger(); + const messenger = + getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRoutingController({ messenger }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_SOLANA_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + rootMessenger.registerActionHandler( + 'SnapController:handleRequest', + async ({ handler }) => { + // TODO: Use proper handler + if (handler === HandlerType.OnProtocolRequest) { + return { address: SOLANA_CONNECTED_ACCOUNTS[0] }; + } else if (handler === HandlerType.OnKeyringRequest) { + return { signature: '0x' }; + } + throw new Error('Unmocked request'); + }, + ); + + const result = await messenger.call( + 'MultichainRoutingController:handleRequest', + { + connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, + chainId: SOLANA_CAIP2, + request: { + method: 'signAndSendTransaction', + params: { + message: 'foo', + }, + }, + }, + ); + + expect(result).toStrictEqual({ signature: '0x' }); + }); + + it('can route protocol requests to procotol Snaps', async () => { + const rootMessenger = getRootMultichainRoutingControllerMessenger(); + const messenger = + getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRoutingController({ messenger }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => [], + ); + + rootMessenger.registerActionHandler('SnapController:getAll', () => { + return [getTruncatedSnap()]; + }); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + rootMessenger.registerActionHandler( + 'SnapController:handleRequest', + async () => ({ + 'feature-set': 2891131721, + 'solana-core': '1.16.7', + }), + ); + + const result = await messenger.call( + 'MultichainRoutingController:handleRequest', + { + connectedAddresses: [], + chainId: SOLANA_CAIP2, + request: { + method: 'getVersion', + }, + }, + ); + + expect(result).toStrictEqual({ + 'feature-set': 2891131721, + 'solana-core': '1.16.7', + }); + }); + + it('throws if no suitable Snaps are found', async () => { + const rootMessenger = getRootMultichainRoutingControllerMessenger(); + const messenger = + getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRoutingController({ messenger }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => [], + ); + + rootMessenger.registerActionHandler('SnapController:getAll', () => { + return []; + }); + + await expect( + messenger.call('MultichainRoutingController:handleRequest', { + connectedAddresses: [], + chainId: SOLANA_CAIP2, + request: { + method: 'getVersion', + }, + }), + ).rejects.toThrow('The method does not exist / is not available'); + }); +}); diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index 8afaf2bcbd..c3b8558fd1 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -22,12 +22,8 @@ import type { SnapId, } from '@metamask/snaps-sdk'; import { HandlerType } from '@metamask/snaps-utils'; -import type { - CaipAccountAddress, - CaipAccountId, - CaipChainId, -} from '@metamask/utils'; -import { hasProperty } from '@metamask/utils'; +import type { CaipAccountId, CaipChainId } from '@metamask/utils'; +import { hasProperty, parseCaipAccountId } from '@metamask/utils'; import { getRunnableSnaps } from '../snaps'; import type { GetAllSnaps, HandleSnapRequest } from '../snaps'; @@ -37,6 +33,12 @@ export type MultichainRoutingControllerGetStateAction = typeof controllerName, MultichainRoutingControllerState >; + +export type MultichainRoutingControllerHandleRequestAction = { + type: `${typeof controllerName}:handleRequest`; + handler: MultichainRoutingController['handleRequest']; +}; + export type MultichainRoutingControllerStateChangeEvent = ControllerStateChangeEvent< typeof controllerName, @@ -62,11 +64,14 @@ export type AccountsControllerListMultichainAccountsAction = { }; export type MultichainRoutingControllerActions = + | MultichainRoutingControllerGetStateAction + | MultichainRoutingControllerHandleRequestAction; + +export type MultichainRoutingControllerAllowedActions = | GetAllSnaps | HandleSnapRequest | GetPermissions - | AccountsControllerListMultichainAccountsAction - | MultichainRoutingControllerGetStateAction; + | AccountsControllerListMultichainAccountsAction; export type MultichainRoutingControllerEvents = MultichainRoutingControllerStateChangeEvent; @@ -74,9 +79,10 @@ export type MultichainRoutingControllerEvents = export type MultichainRoutingControllerMessenger = RestrictedControllerMessenger< typeof controllerName, - MultichainRoutingControllerActions, - MultichainRoutingControllerEvents, - MultichainRoutingControllerActions['type'], + | MultichainRoutingControllerActions + | MultichainRoutingControllerAllowedActions, + never, + MultichainRoutingControllerAllowedActions['type'], MultichainRoutingControllerEvents['type'] >; @@ -108,6 +114,11 @@ export class MultichainRoutingController extends BaseController< ...state, }, }); + + this.messagingSystem.registerActionHandler( + `${controllerName}:handleRequest`, + async (...args) => this.handleRequest(...args), + ); } async #resolveRequestAddress( @@ -130,22 +141,23 @@ export class MultichainRoutingController extends BaseController< }, handler: HandlerType.OnProtocolRequest, // TODO: Export and request format }, - )) as { address: string } | null; - return result?.address; + )) as { address: CaipAccountId } | null; + const address = result?.address; + return address ? parseCaipAccountId(address).address : null; } catch { throw rpcErrors.internal(); } } async #getAccountSnap( - connectedAddresses: CaipAccountAddress[], + connectedAddresses: CaipAccountId[], chainId: CaipChainId, request: JsonRpcRequest, ) { const accounts = this.messagingSystem .call('AccountsController:listMultichainAccounts', chainId) .filter( - (account) => + (account: InternalAccount) => account.metadata.snap?.enabled && account.methods.includes(request.method), ) as (InternalAccount & { @@ -166,12 +178,16 @@ export class MultichainRoutingController extends BaseController< request, ); + const parsedConnectedAddresses = connectedAddresses.map( + (connectedAddress) => parseCaipAccountId(connectedAddress).address, + ); + // If we have a resolved address, try to find the selected account based on that // otherwise, default to one of the connected accounts. // TODO: Eventually let the user choose if we have more than one option for the account. const selectedAccount = accounts.find( (account) => - connectedAddresses.includes(account.address) && + parsedConnectedAddresses.includes(account.address) && (!address || account.address.toLowerCase() === address.toLowerCase()), ); @@ -218,7 +234,7 @@ export class MultichainRoutingController extends BaseController< origin: string; chainId: CaipChainId; request: JsonRpcRequest; - }) { + }): Promise { // TODO: Determine if the request is already validated here? const { method } = request; diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index 7c31cf10f1..09b698954a 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -6108,7 +6108,7 @@ describe('SnapController', () => { [MOCK_SNAP_ID]: {}, }), ).rejects.toThrow( - 'A snap must request at least one of the following permissions: endowment:rpc, endowment:transaction-insight, endowment:cronjob, endowment:name-lookup, endowment:lifecycle-hooks, endowment:keyring, endowment:page-home, endowment:page-settings, endowment:signature-insight, endowment:assets.', + 'A snap must request at least one of the following permissions: endowment:rpc, endowment:transaction-insight, endowment:cronjob, endowment:name-lookup, endowment:lifecycle-hooks, endowment:keyring, endowment:page-home, endowment:page-settings, endowment:signature-insight, endowment:assets, endowment:protocol.', ); controller.destroy(); diff --git a/packages/snaps-controllers/src/test-utils/controller.ts b/packages/snaps-controllers/src/test-utils/controller.ts index f71b8b072a..a72310adb4 100644 --- a/packages/snaps-controllers/src/test-utils/controller.ts +++ b/packages/snaps-controllers/src/test-utils/controller.ts @@ -53,6 +53,11 @@ import type { StoredInterface, } from '../interface/SnapInterfaceController'; import { SnapController } from '../snaps'; +import type { + MultichainRoutingControllerActions, + MultichainRoutingControllerAllowedActions, + MultichainRoutingControllerEvents, +} from '../multichain'; import type { AllowedActions, AllowedEvents, @@ -861,3 +866,39 @@ export async function waitForStateChange( }); }); } + +// Mock controller messenger for Multichain Routing Controller +export const getRootMultichainRoutingControllerMessenger = () => { + const messenger = new MockControllerMessenger< + | MultichainRoutingControllerActions + | MultichainRoutingControllerAllowedActions, + MultichainRoutingControllerEvents + >(); + + jest.spyOn(messenger, 'call'); + + return messenger; +}; + +export const getRestrictedMultichainRoutingControllerMessenger = ( + messenger: ReturnType< + typeof getRootMultichainRoutingControllerMessenger + > = getRootMultichainRoutingControllerMessenger(), +) => { + const controllerMessenger = messenger.getRestricted< + 'MultichainRoutingController', + MultichainRoutingControllerAllowedActions['type'], + never + >({ + name: 'MultichainRoutingController', + allowedEvents: [], + allowedActions: [ + 'PermissionController:getPermissions', + 'SnapController:getAll', + 'SnapController:handleRequest', + 'AccountsController:listMultichainAccounts', + ], + }); + + return controllerMessenger; +}; diff --git a/packages/snaps-controllers/src/test-utils/index.ts b/packages/snaps-controllers/src/test-utils/index.ts index 91b9d1585a..eb5301eacf 100644 --- a/packages/snaps-controllers/src/test-utils/index.ts +++ b/packages/snaps-controllers/src/test-utils/index.ts @@ -4,4 +4,5 @@ export * from './execution-environment'; export * from './service'; export * from './sleep'; export * from './location'; +export * from './multichain'; export * from './registry'; diff --git a/packages/snaps-controllers/src/test-utils/multichain.ts b/packages/snaps-controllers/src/test-utils/multichain.ts new file mode 100644 index 0000000000..f736354a8c --- /dev/null +++ b/packages/snaps-controllers/src/test-utils/multichain.ts @@ -0,0 +1,97 @@ +import type { PermissionConstraint } from '@metamask/permission-controller'; +import { SnapEndowments } from '@metamask/snaps-rpc-methods'; +import { SnapCaveatType } from '@metamask/snaps-utils'; +import { MOCK_SNAP_ID } from '@metamask/snaps-utils/test-utils'; +import type { CaipAccountId, CaipChainId } from '@metamask/utils'; + +export const BTC_CAIP2 = + 'bip122:000000000019d6689c085ae165831e93' as CaipChainId; +export const BTC_CONNECTED_ACCOUNTS = [ + 'bip122:000000000019d6689c085ae165831e93:128Lkh3S7CkDTBZ8W7BbpsN3YYizJMp8p6', +] as CaipAccountId[]; + +export const MOCK_BTC_ACCOUNTS = [ + { + address: '128Lkh3S7CkDTBZ8W7BbpsN3YYizJMp8p6', + id: '408eb023-8678-4b53-8885-f1e50b8b5bc3', + metadata: { + importTime: 1729154128900, + keyring: { type: 'Snap Keyring' }, + lastSelected: 1729154128902, + name: 'Bitcoin Account', + snap: { + enabled: true, + id: MOCK_SNAP_ID, + name: 'Bitcoin', + }, + }, + methods: ['btc_sendmany'], + options: { index: 0, scope: BTC_CAIP2 }, + type: 'bip122:p2wpkh', + }, +]; + +export const SOLANA_CAIP2 = + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp' as CaipChainId; +export const SOLANA_CONNECTED_ACCOUNTS = [ + `solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKv`, +] as CaipAccountId[]; + +export const MOCK_SOLANA_ACCOUNTS = [ + { + address: '7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKv', + id: '408eb023-8678-4b53-8885-f1e50b8b5bc3', + metadata: { + importTime: 1729154128900, + keyring: { type: 'Snap Keyring' }, + lastSelected: 1729154128902, + name: 'Solana Account', + snap: { + enabled: true, + id: MOCK_SNAP_ID, + name: 'Solana', + }, + }, + methods: ['signAndSendTransaction'], + options: { index: 0, scope: SOLANA_CAIP2 }, + type: 'solana:data-account', + }, +]; + +export const MOCK_SOLANA_SNAP_PERMISSIONS: Record< + string, + PermissionConstraint +> = { + [SnapEndowments.Keyring]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: [SOLANA_CAIP2], + }, + { + type: SnapCaveatType.SnapRpcMethods, + value: ['getVersion'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Keyring, + }, + [SnapEndowments.Protocol]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: [SOLANA_CAIP2], + }, + { + type: SnapCaveatType.SnapRpcMethods, + value: ['getVersion'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Protocol, + }, +}; diff --git a/packages/snaps-rpc-methods/src/endowments/protocol.ts b/packages/snaps-rpc-methods/src/endowments/protocol.ts index 5cbddd2576..cdfd02c85b 100644 --- a/packages/snaps-rpc-methods/src/endowments/protocol.ts +++ b/packages/snaps-rpc-methods/src/endowments/protocol.ts @@ -52,7 +52,6 @@ const specificationBuilder: PermissionSpecificationBuilder< permissionType: PermissionType.Endowment, targetName: permissionName, allowedCaveats: [ - SnapCaveatType.KeyringOrigin, SnapCaveatType.ChainIds, SnapCaveatType.SnapRpcMethods, SnapCaveatType.MaxRequestTime, diff --git a/packages/snaps-rpc-methods/src/permissions.test.ts b/packages/snaps-rpc-methods/src/permissions.test.ts index 9b9486d2e4..cea6759949 100644 --- a/packages/snaps-rpc-methods/src/permissions.test.ts +++ b/packages/snaps-rpc-methods/src/permissions.test.ts @@ -104,6 +104,20 @@ describe('buildSnapEndowmentSpecifications', () => { ], "targetName": "endowment:page-settings", }, + "endowment:protocol": { + "allowedCaveats": [ + "chainIds", + "snapRpcMethods", + "maxRequestTime", + ], + "endowmentGetter": [Function], + "permissionType": "Endowment", + "subjectTypes": [ + "snap", + ], + "targetName": "endowment:protocol", + "validator": [Function], + }, "endowment:rpc": { "allowedCaveats": [ "rpcOrigin", From 9b73af4bcfe65b00949d271868a9a92ea9393675 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 12 Nov 2024 13:15:47 +0100 Subject: [PATCH 07/35] Call SnapKeyring instead of the Snap directly --- .../MultichainRoutingController.test.ts | 33 +++++++++++++------ .../multichain/MultichainRoutingController.ts | 33 +++++++++++++++---- .../src/test-utils/multichain.ts | 7 ++++ 3 files changed, 56 insertions(+), 17 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts index afe09275ba..697568fd02 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts @@ -11,6 +11,7 @@ import { SOLANA_CAIP2, MOCK_SOLANA_ACCOUNTS, MOCK_BTC_ACCOUNTS, + getMockSnapKeyring, } from '../test-utils'; import { MultichainRoutingController } from './MultichainRoutingController'; @@ -21,7 +22,14 @@ describe('MultichainRoutingController', () => { getRestrictedMultichainRoutingControllerMessenger(rootMessenger); /* eslint-disable-next-line no-new */ - new MultichainRoutingController({ messenger }); + new MultichainRoutingController({ + messenger, + getSnapKeyring: getMockSnapKeyring( + jest.fn().mockResolvedValue({ + txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', + }), + ), + }); rootMessenger.registerActionHandler( 'AccountsController:listMultichainAccounts', @@ -34,10 +42,6 @@ describe('MultichainRoutingController', () => { // TODO: Use proper handler if (handler === HandlerType.OnProtocolRequest) { return null; - } else if (handler === HandlerType.OnKeyringRequest) { - return { - txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', - }; } throw new Error('Unmocked request'); }, @@ -68,7 +72,12 @@ describe('MultichainRoutingController', () => { getRestrictedMultichainRoutingControllerMessenger(rootMessenger); /* eslint-disable-next-line no-new */ - new MultichainRoutingController({ messenger }); + new MultichainRoutingController({ + messenger, + getSnapKeyring: getMockSnapKeyring( + jest.fn().mockResolvedValue({ signature: '0x' }), + ), + }); rootMessenger.registerActionHandler( 'AccountsController:listMultichainAccounts', @@ -86,8 +95,6 @@ describe('MultichainRoutingController', () => { // TODO: Use proper handler if (handler === HandlerType.OnProtocolRequest) { return { address: SOLANA_CONNECTED_ACCOUNTS[0] }; - } else if (handler === HandlerType.OnKeyringRequest) { - return { signature: '0x' }; } throw new Error('Unmocked request'); }, @@ -116,7 +123,10 @@ describe('MultichainRoutingController', () => { getRestrictedMultichainRoutingControllerMessenger(rootMessenger); /* eslint-disable-next-line no-new */ - new MultichainRoutingController({ messenger }); + new MultichainRoutingController({ + messenger, + getSnapKeyring: getMockSnapKeyring(), + }); rootMessenger.registerActionHandler( 'AccountsController:listMultichainAccounts', @@ -163,7 +173,10 @@ describe('MultichainRoutingController', () => { getRestrictedMultichainRoutingControllerMessenger(rootMessenger); /* eslint-disable-next-line no-new */ - new MultichainRoutingController({ messenger }); + new MultichainRoutingController({ + messenger, + getSnapKeyring: getMockSnapKeyring(), + }); rootMessenger.registerActionHandler( 'AccountsController:listMultichainAccounts', diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index c3b8558fd1..70d68cee39 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -86,9 +86,19 @@ export type MultichainRoutingControllerMessenger = MultichainRoutingControllerEvents['type'] >; +export type SnapKeyring = { + submitNonEvmRequest: (args: { + address: string; + method: string; + params?: Json[] | Record; + chainId: CaipChainId; + }) => Promise; +}; + export type MultichainRoutingControllerArgs = { messenger: MultichainRoutingControllerMessenger; state?: MultichainRoutingControllerState; + getSnapKeyring: () => Promise; }; export type MultichainRoutingControllerState = EmptyObject; @@ -105,7 +115,13 @@ export class MultichainRoutingController extends BaseController< MultichainRoutingControllerState, MultichainRoutingControllerMessenger > { - constructor({ messenger, state }: MultichainRoutingControllerArgs) { + #getSnapKeyring: () => Promise; + + constructor({ + messenger, + state, + getSnapKeyring, + }: MultichainRoutingControllerArgs) { super({ messenger, metadata: {}, @@ -115,6 +131,8 @@ export class MultichainRoutingController extends BaseController< }, }); + this.#getSnapKeyring = getSnapKeyring; + this.messagingSystem.registerActionHandler( `${controllerName}:handleRequest`, async (...args) => this.handleRequest(...args), @@ -236,7 +254,7 @@ export class MultichainRoutingController extends BaseController< request: JsonRpcRequest; }): Promise { // TODO: Determine if the request is already validated here? - const { method } = request; + const { method, params } = request; // If the RPC request can be serviced by an account Snap, route it there. const accountSnap = await this.#getAccountSnap( @@ -245,11 +263,12 @@ export class MultichainRoutingController extends BaseController< request, ); if (accountSnap) { - return this.messagingSystem.call('SnapController:handleRequest', { - snapId: accountSnap.snapId, - origin: 'metamask', // TODO: Determine origin of these requests? - request, - handler: HandlerType.OnKeyringRequest, + const keyring = await this.#getSnapKeyring(); + return keyring.submitNonEvmRequest({ + address: accountSnap.address, + method, + params, + chainId, }); } diff --git a/packages/snaps-controllers/src/test-utils/multichain.ts b/packages/snaps-controllers/src/test-utils/multichain.ts index f736354a8c..168479e5d0 100644 --- a/packages/snaps-controllers/src/test-utils/multichain.ts +++ b/packages/snaps-controllers/src/test-utils/multichain.ts @@ -95,3 +95,10 @@ export const MOCK_SOLANA_SNAP_PERMISSIONS: Record< parentCapability: SnapEndowments.Protocol, }, }; + +export const getMockSnapKeyring = (implementation = jest.fn()) => { + return async () => + Promise.resolve({ + submitNonEvmRequest: implementation, + }); +}; From 679f95ca932787522433bf86fa37aaeb894ee3a5 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 13 Nov 2024 15:03:13 +0100 Subject: [PATCH 08/35] Refactor protocol caveats --- .../multichain/MultichainRoutingController.ts | 23 +++---- .../src/test-utils/multichain.ts | 16 ++--- .../snaps-rpc-methods/src/endowments/index.ts | 5 +- .../src/endowments/protocol.ts | 61 +++++-------------- packages/snaps-utils/src/caveats.ts | 2 +- .../snaps-utils/src/manifest/validation.ts | 9 ++- 6 files changed, 37 insertions(+), 79 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index 70d68cee39..1754915754 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -4,15 +4,10 @@ import type { ControllerStateChangeEvent, } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; -import type { - Caveat, - GetPermissions, - ValidPermission, -} from '@metamask/permission-controller'; +import type { GetPermissions } from '@metamask/permission-controller'; import { rpcErrors } from '@metamask/rpc-errors'; import { - getProtocolCaveatChainIds, - getProtocolCaveatRpcMethods, + getProtocolCaveatChains, SnapEndowments, } from '@metamask/snaps-rpc-methods'; import type { @@ -103,9 +98,9 @@ export type MultichainRoutingControllerArgs = { export type MultichainRoutingControllerState = EmptyObject; -type SnapWithPermission = { +type ProtocolSnap = { snapId: SnapId; - permission: ValidPermission>; + methods: string[]; }; const controllerName = 'MultichainRoutingController'; @@ -223,18 +218,18 @@ export class MultichainRoutingController extends BaseController< const allSnaps = this.messagingSystem.call('SnapController:getAll'); const filteredSnaps = getRunnableSnaps(allSnaps); - return filteredSnaps.reduce((accumulator, snap) => { + return filteredSnaps.reduce((accumulator, snap) => { const permissions = this.messagingSystem.call( 'PermissionController:getPermissions', snap.id, ); if (permissions && hasProperty(permissions, SnapEndowments.Protocol)) { const permission = permissions[SnapEndowments.Protocol]; - const chains = getProtocolCaveatChainIds(permission); - if (chains?.includes(chainId)) { + const chains = getProtocolCaveatChains(permission); + if (chains && hasProperty(chains, chainId)) { accumulator.push({ snapId: snap.id, - permission, + methods: chains[chainId].methods, }); } } @@ -276,7 +271,7 @@ export class MultichainRoutingController extends BaseController< // but has a protocol Snap available, route it there. const protocolSnaps = this.#getProtocolSnaps(chainId); const protocolSnap = protocolSnaps.find((snap) => - getProtocolCaveatRpcMethods(snap.permission)?.includes(method), + snap.methods.includes(method), ); if (protocolSnap) { return this.messagingSystem.call('SnapController:handleRequest', { diff --git a/packages/snaps-controllers/src/test-utils/multichain.ts b/packages/snaps-controllers/src/test-utils/multichain.ts index 168479e5d0..c3238bcc57 100644 --- a/packages/snaps-controllers/src/test-utils/multichain.ts +++ b/packages/snaps-controllers/src/test-utils/multichain.ts @@ -65,12 +65,8 @@ export const MOCK_SOLANA_SNAP_PERMISSIONS: Record< [SnapEndowments.Keyring]: { caveats: [ { - type: SnapCaveatType.ChainIds, - value: [SOLANA_CAIP2], - }, - { - type: SnapCaveatType.SnapRpcMethods, - value: ['getVersion'], + type: SnapCaveatType.KeyringOrigin, + value: { allowedOrigins: [] }, }, ], date: 1664187844588, @@ -81,12 +77,8 @@ export const MOCK_SOLANA_SNAP_PERMISSIONS: Record< [SnapEndowments.Protocol]: { caveats: [ { - type: SnapCaveatType.ChainIds, - value: [SOLANA_CAIP2], - }, - { - type: SnapCaveatType.SnapRpcMethods, - value: ['getVersion'], + type: SnapCaveatType.ProtocolSnapChains, + value: { [SOLANA_CAIP2]: { methods: ['getVersion'] } }, }, ], date: 1664187844588, diff --git a/packages/snaps-rpc-methods/src/endowments/index.ts b/packages/snaps-rpc-methods/src/endowments/index.ts index 7cdd9d06a8..5f50e7550b 100644 --- a/packages/snaps-rpc-methods/src/endowments/index.ts +++ b/packages/snaps-rpc-methods/src/endowments/index.ts @@ -139,7 +139,4 @@ export { getChainIdsCaveat, getLookupMatchersCaveat } from './name-lookup'; export { getKeyringCaveatOrigins } from './keyring'; export { getMaxRequestTimeCaveat } from './caveats'; export { getCronjobCaveatJobs } from './cronjob'; -export { - getProtocolCaveatChainIds, - getProtocolCaveatRpcMethods, -} from './protocol'; +export { getProtocolCaveatChains } from './protocol'; diff --git a/packages/snaps-rpc-methods/src/endowments/protocol.ts b/packages/snaps-rpc-methods/src/endowments/protocol.ts index cdfd02c85b..7dc2028abc 100644 --- a/packages/snaps-rpc-methods/src/endowments/protocol.ts +++ b/packages/snaps-rpc-methods/src/endowments/protocol.ts @@ -10,10 +10,8 @@ import type { } from '@metamask/permission-controller'; import { PermissionType, SubjectType } from '@metamask/permission-controller'; import { rpcErrors } from '@metamask/rpc-errors'; -import { - ProtocolRpcMethodsStruct, - SnapCaveatType, -} from '@metamask/snaps-utils'; +import { ProtocolChainsStruct, SnapCaveatType } from '@metamask/snaps-utils'; +import type { Infer } from '@metamask/superstruct'; import type { Json, NonEmptyArray } from '@metamask/utils'; import { assertStruct, @@ -51,15 +49,10 @@ const specificationBuilder: PermissionSpecificationBuilder< return { permissionType: PermissionType.Endowment, targetName: permissionName, - allowedCaveats: [ - SnapCaveatType.ChainIds, - SnapCaveatType.SnapRpcMethods, - SnapCaveatType.MaxRequestTime, - ], + allowedCaveats: [SnapCaveatType.ChainIds, SnapCaveatType.MaxRequestTime], endowmentGetter: (_getterOptions?: EndowmentGetterParams) => null, validator: createGenericPermissionValidator([ - { type: SnapCaveatType.ChainIds }, - { type: SnapCaveatType.SnapRpcMethods }, + { type: SnapCaveatType.ProtocolSnapChains }, { type: SnapCaveatType.MaxRequestTime, optional: true }, ]), subjectTypes: [SubjectType.Snap], @@ -90,51 +83,29 @@ export function getProtocolCaveatMapper( if (value.chains) { caveats.push({ - type: SnapCaveatType.ChainIds, + type: SnapCaveatType.ProtocolSnapChains, value: value.chains, }); } - if (value.methods) { - caveats.push({ - type: SnapCaveatType.SnapRpcMethods, - value: value.methods, - }); - } - return { caveats: caveats as NonEmptyArray }; } -/** - * Getter function to get the {@link ChainIds} caveat value from a - * permission. - * - * @param permission - The permission to get the caveat value from. - * @returns The caveat value. - */ -export function getProtocolCaveatChainIds( - permission?: PermissionConstraint, -): string[] | null { - const caveat = permission?.caveats?.find( - (permCaveat) => permCaveat.type === SnapCaveatType.ChainIds, - ) as Caveat | undefined; - - return caveat ? caveat.value : null; -} +export type ProtocolChains = Infer; /** - * Getter function to get the {@link SnapRpcMethods} caveat value from a + * Getter function to get the {@link ProtocolSnapChains} caveat value from a * permission. * * @param permission - The permission to get the caveat value from. * @returns The caveat value. */ -export function getProtocolCaveatRpcMethods( +export function getProtocolCaveatChains( permission?: PermissionConstraint, -): string[] | null { +): ProtocolChains | null { const caveat = permission?.caveats?.find( - (permCaveat) => permCaveat.type === SnapCaveatType.SnapRpcMethods, - ) as Caveat | undefined; + (permCaveat) => permCaveat.type === SnapCaveatType.ProtocolSnapChains, + ) as Caveat | undefined; return caveat ? caveat.value : null; } @@ -155,18 +126,18 @@ function validateCaveat(caveat: Caveat): void { const { value } = caveat; assertStruct( value, - ProtocolRpcMethodsStruct, - 'Invalid RPC methods specified', + ProtocolChainsStruct, + 'Invalid chains specified', rpcErrors.invalidParams, ); } export const protocolCaveatSpecifications: Record< - SnapCaveatType.SnapRpcMethods, + SnapCaveatType.ProtocolSnapChains, CaveatSpecificationConstraint > = { - [SnapCaveatType.SnapRpcMethods]: Object.freeze({ - type: SnapCaveatType.SnapRpcMethods, + [SnapCaveatType.ProtocolSnapChains]: Object.freeze({ + type: SnapCaveatType.ProtocolSnapChains, validator: (caveat: Caveat) => validateCaveat(caveat), }), }; diff --git a/packages/snaps-utils/src/caveats.ts b/packages/snaps-utils/src/caveats.ts index 7599e95b4c..9aa0a4e370 100644 --- a/packages/snaps-utils/src/caveats.ts +++ b/packages/snaps-utils/src/caveats.ts @@ -57,5 +57,5 @@ export enum SnapCaveatType { /** * Caveat specifying a list of RPC methods serviced by an endowment. */ - SnapRpcMethods = 'snapRpcMethods', + ProtocolSnapChains = 'protocolSnapChains', } diff --git a/packages/snaps-utils/src/manifest/validation.ts b/packages/snaps-utils/src/manifest/validation.ts index 1314dbc2a5..5c5bc9b286 100644 --- a/packages/snaps-utils/src/manifest/validation.ts +++ b/packages/snaps-utils/src/manifest/validation.ts @@ -27,6 +27,7 @@ import { isValidSemVerRange, inMilliseconds, Duration, + CaipChainIdStruct, } from '@metamask/utils'; import { isEqual } from '../array'; @@ -174,8 +175,10 @@ export const MaxRequestTimeStruct = size( MAXIMUM_REQUEST_TIMEOUT, ); -// TODO: Decide on the format for this -export const ProtocolRpcMethodsStruct = array(string()); +export const ProtocolChainsStruct = record( + CaipChainIdStruct, + object({ methods: array(string()) }), +); // Utility type to union with for all handler structs export const HandlerCaveatsStruct = object({ @@ -207,7 +210,7 @@ export const PermissionsStruct: Describe = type({ 'endowment:protocol': optional( mergeStructs( HandlerCaveatsStruct, - object({ chains: ChainIdsStruct, methods: ProtocolRpcMethodsStruct }), + object({ chains: ProtocolChainsStruct }), ), ), 'endowment:lifecycle-hooks': optional(HandlerCaveatsStruct), From 6879f9f7e39bde8354abea4e9ecf14eda954e144 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 14 Nov 2024 10:55:20 +0100 Subject: [PATCH 09/35] Chain ID -> Scope --- .../MultichainRoutingController.test.ts | 8 ++--- .../multichain/MultichainRoutingController.ts | 32 +++++++++---------- .../src/test-utils/multichain.ts | 2 +- .../snaps-rpc-methods/src/endowments/index.ts | 2 +- .../src/endowments/protocol.ts | 26 +++++++-------- .../snaps-sdk/src/types/handlers/protocol.ts | 4 +-- packages/snaps-utils/src/caveats.ts | 4 +-- .../snaps-utils/src/manifest/validation.ts | 4 +-- 8 files changed, 41 insertions(+), 41 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts index 697568fd02..19883e7926 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts @@ -51,7 +51,7 @@ describe('MultichainRoutingController', () => { 'MultichainRoutingController:handleRequest', { connectedAddresses: BTC_CONNECTED_ACCOUNTS, - chainId: BTC_CAIP2, + scope: BTC_CAIP2, request: { method: 'btc_sendmany', params: { @@ -104,7 +104,7 @@ describe('MultichainRoutingController', () => { 'MultichainRoutingController:handleRequest', { connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, - chainId: SOLANA_CAIP2, + scope: SOLANA_CAIP2, request: { method: 'signAndSendTransaction', params: { @@ -154,7 +154,7 @@ describe('MultichainRoutingController', () => { 'MultichainRoutingController:handleRequest', { connectedAddresses: [], - chainId: SOLANA_CAIP2, + scope: SOLANA_CAIP2, request: { method: 'getVersion', }, @@ -190,7 +190,7 @@ describe('MultichainRoutingController', () => { await expect( messenger.call('MultichainRoutingController:handleRequest', { connectedAddresses: [], - chainId: SOLANA_CAIP2, + scope: SOLANA_CAIP2, request: { method: 'getVersion', }, diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index 1754915754..9d13fa1f86 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -7,7 +7,7 @@ import { BaseController } from '@metamask/base-controller'; import type { GetPermissions } from '@metamask/permission-controller'; import { rpcErrors } from '@metamask/rpc-errors'; import { - getProtocolCaveatChains, + getProtocolCaveatScopes, SnapEndowments, } from '@metamask/snaps-rpc-methods'; import type { @@ -136,7 +136,7 @@ export class MultichainRoutingController extends BaseController< async #resolveRequestAddress( snapId: SnapId, - chainId: CaipChainId, + scope: CaipChainId, request: JsonRpcRequest, ) { try { @@ -148,7 +148,7 @@ export class MultichainRoutingController extends BaseController< request: { method: '', params: { - chainId, + scope, request, }, }, @@ -164,11 +164,11 @@ export class MultichainRoutingController extends BaseController< async #getAccountSnap( connectedAddresses: CaipAccountId[], - chainId: CaipChainId, + scope: CaipChainId, request: JsonRpcRequest, ) { const accounts = this.messagingSystem - .call('AccountsController:listMultichainAccounts', chainId) + .call('AccountsController:listMultichainAccounts', scope) .filter( (account: InternalAccount) => account.metadata.snap?.enabled && @@ -187,7 +187,7 @@ export class MultichainRoutingController extends BaseController< // Attempt to resolve the address that should be used for signing. const address = await this.#resolveRequestAddress( resolutionSnapId, - chainId, + scope, request, ); @@ -214,7 +214,7 @@ export class MultichainRoutingController extends BaseController< }; } - #getProtocolSnaps(chainId: CaipChainId) { + #getProtocolSnaps(scope: CaipChainId) { const allSnaps = this.messagingSystem.call('SnapController:getAll'); const filteredSnaps = getRunnableSnaps(allSnaps); @@ -225,11 +225,11 @@ export class MultichainRoutingController extends BaseController< ); if (permissions && hasProperty(permissions, SnapEndowments.Protocol)) { const permission = permissions[SnapEndowments.Protocol]; - const chains = getProtocolCaveatChains(permission); - if (chains && hasProperty(chains, chainId)) { + const scopes = getProtocolCaveatScopes(permission); + if (scopes && hasProperty(scopes, scope)) { accumulator.push({ snapId: snap.id, - methods: chains[chainId].methods, + methods: scopes[scope].methods, }); } } @@ -240,12 +240,12 @@ export class MultichainRoutingController extends BaseController< async handleRequest({ connectedAddresses, - chainId, + scope, request, }: { connectedAddresses: CaipAccountId[]; origin: string; - chainId: CaipChainId; + scope: CaipChainId; request: JsonRpcRequest; }): Promise { // TODO: Determine if the request is already validated here? @@ -254,7 +254,7 @@ export class MultichainRoutingController extends BaseController< // If the RPC request can be serviced by an account Snap, route it there. const accountSnap = await this.#getAccountSnap( connectedAddresses, - chainId, + scope, request, ); if (accountSnap) { @@ -263,13 +263,13 @@ export class MultichainRoutingController extends BaseController< address: accountSnap.address, method, params, - chainId, + chainId: scope, }); } // If the RPC request cannot be serviced by an account Snap, // but has a protocol Snap available, route it there. - const protocolSnaps = this.#getProtocolSnaps(chainId); + const protocolSnaps = this.#getProtocolSnaps(scope); const protocolSnap = protocolSnaps.find((snap) => snap.methods.includes(method), ); @@ -281,7 +281,7 @@ export class MultichainRoutingController extends BaseController< method: '', params: { request, - chainId, + scope, }, }, handler: HandlerType.OnProtocolRequest, diff --git a/packages/snaps-controllers/src/test-utils/multichain.ts b/packages/snaps-controllers/src/test-utils/multichain.ts index c3238bcc57..bd938fecfa 100644 --- a/packages/snaps-controllers/src/test-utils/multichain.ts +++ b/packages/snaps-controllers/src/test-utils/multichain.ts @@ -77,7 +77,7 @@ export const MOCK_SOLANA_SNAP_PERMISSIONS: Record< [SnapEndowments.Protocol]: { caveats: [ { - type: SnapCaveatType.ProtocolSnapChains, + type: SnapCaveatType.ProtocolSnapScopes, value: { [SOLANA_CAIP2]: { methods: ['getVersion'] } }, }, ], diff --git a/packages/snaps-rpc-methods/src/endowments/index.ts b/packages/snaps-rpc-methods/src/endowments/index.ts index 5f50e7550b..4b4d96e4b3 100644 --- a/packages/snaps-rpc-methods/src/endowments/index.ts +++ b/packages/snaps-rpc-methods/src/endowments/index.ts @@ -139,4 +139,4 @@ export { getChainIdsCaveat, getLookupMatchersCaveat } from './name-lookup'; export { getKeyringCaveatOrigins } from './keyring'; export { getMaxRequestTimeCaveat } from './caveats'; export { getCronjobCaveatJobs } from './cronjob'; -export { getProtocolCaveatChains } from './protocol'; +export { getProtocolCaveatScopes } from './protocol'; diff --git a/packages/snaps-rpc-methods/src/endowments/protocol.ts b/packages/snaps-rpc-methods/src/endowments/protocol.ts index 7dc2028abc..e182ad755b 100644 --- a/packages/snaps-rpc-methods/src/endowments/protocol.ts +++ b/packages/snaps-rpc-methods/src/endowments/protocol.ts @@ -10,7 +10,7 @@ import type { } from '@metamask/permission-controller'; import { PermissionType, SubjectType } from '@metamask/permission-controller'; import { rpcErrors } from '@metamask/rpc-errors'; -import { ProtocolChainsStruct, SnapCaveatType } from '@metamask/snaps-utils'; +import { ProtocolScopesStruct, SnapCaveatType } from '@metamask/snaps-utils'; import type { Infer } from '@metamask/superstruct'; import type { Json, NonEmptyArray } from '@metamask/utils'; import { @@ -52,7 +52,7 @@ const specificationBuilder: PermissionSpecificationBuilder< allowedCaveats: [SnapCaveatType.ChainIds, SnapCaveatType.MaxRequestTime], endowmentGetter: (_getterOptions?: EndowmentGetterParams) => null, validator: createGenericPermissionValidator([ - { type: SnapCaveatType.ProtocolSnapChains }, + { type: SnapCaveatType.ProtocolSnapScopes }, { type: SnapCaveatType.MaxRequestTime, optional: true }, ]), subjectTypes: [SubjectType.Snap], @@ -83,7 +83,7 @@ export function getProtocolCaveatMapper( if (value.chains) { caveats.push({ - type: SnapCaveatType.ProtocolSnapChains, + type: SnapCaveatType.ProtocolSnapScopes, value: value.chains, }); } @@ -91,21 +91,21 @@ export function getProtocolCaveatMapper( return { caveats: caveats as NonEmptyArray }; } -export type ProtocolChains = Infer; +export type ProtocolScopes = Infer; /** - * Getter function to get the {@link ProtocolSnapChains} caveat value from a + * Getter function to get the {@link ProtocolSnapScopes} caveat value from a * permission. * * @param permission - The permission to get the caveat value from. * @returns The caveat value. */ -export function getProtocolCaveatChains( +export function getProtocolCaveatScopes( permission?: PermissionConstraint, -): ProtocolChains | null { +): ProtocolScopes | null { const caveat = permission?.caveats?.find( - (permCaveat) => permCaveat.type === SnapCaveatType.ProtocolSnapChains, - ) as Caveat | undefined; + (permCaveat) => permCaveat.type === SnapCaveatType.ProtocolSnapScopes, + ) as Caveat | undefined; return caveat ? caveat.value : null; } @@ -126,18 +126,18 @@ function validateCaveat(caveat: Caveat): void { const { value } = caveat; assertStruct( value, - ProtocolChainsStruct, + ProtocolScopesStruct, 'Invalid chains specified', rpcErrors.invalidParams, ); } export const protocolCaveatSpecifications: Record< - SnapCaveatType.ProtocolSnapChains, + SnapCaveatType.ProtocolSnapScopes, CaveatSpecificationConstraint > = { - [SnapCaveatType.ProtocolSnapChains]: Object.freeze({ - type: SnapCaveatType.ProtocolSnapChains, + [SnapCaveatType.ProtocolSnapScopes]: Object.freeze({ + type: SnapCaveatType.ProtocolSnapScopes, validator: (caveat: Caveat) => validateCaveat(caveat), }), }; diff --git a/packages/snaps-sdk/src/types/handlers/protocol.ts b/packages/snaps-sdk/src/types/handlers/protocol.ts index 4d0c86d992..085d6c1b2c 100644 --- a/packages/snaps-sdk/src/types/handlers/protocol.ts +++ b/packages/snaps-sdk/src/types/handlers/protocol.ts @@ -14,7 +14,7 @@ import type { * @param args - The request arguments. * @param args.origin - The origin of the request. This can be the ID of another * Snap, or the URL of a website. - * @param args.chainId - The chain ID of the request. + * @param args.scope - The scope of the request. * @param args.request - The protocol request sent to the Snap. This includes * the method name and parameters. * @returns The response to the protocol request. This must be a @@ -25,6 +25,6 @@ export type OnProtocolRequestHandler< Params extends JsonRpcParams = JsonRpcParams, > = (args: { origin: string; - chainId: CaipChainId; + scope: CaipChainId; request: JsonRpcRequest; }) => Promise; diff --git a/packages/snaps-utils/src/caveats.ts b/packages/snaps-utils/src/caveats.ts index 9aa0a4e370..8272be9bc8 100644 --- a/packages/snaps-utils/src/caveats.ts +++ b/packages/snaps-utils/src/caveats.ts @@ -55,7 +55,7 @@ export enum SnapCaveatType { MaxRequestTime = 'maxRequestTime', /** - * Caveat specifying a list of RPC methods serviced by an endowment. + * Caveat specifying a list of scopes serviced by an endowment. */ - ProtocolSnapChains = 'protocolSnapChains', + ProtocolSnapScopes = 'protocolSnapScopes', } diff --git a/packages/snaps-utils/src/manifest/validation.ts b/packages/snaps-utils/src/manifest/validation.ts index 5c5bc9b286..02b7208d78 100644 --- a/packages/snaps-utils/src/manifest/validation.ts +++ b/packages/snaps-utils/src/manifest/validation.ts @@ -175,7 +175,7 @@ export const MaxRequestTimeStruct = size( MAXIMUM_REQUEST_TIMEOUT, ); -export const ProtocolChainsStruct = record( +export const ProtocolScopesStruct = record( CaipChainIdStruct, object({ methods: array(string()) }), ); @@ -210,7 +210,7 @@ export const PermissionsStruct: Describe = type({ 'endowment:protocol': optional( mergeStructs( HandlerCaveatsStruct, - object({ chains: ProtocolChainsStruct }), + object({ chains: ProtocolScopesStruct }), ), ), 'endowment:lifecycle-hooks': optional(HandlerCaveatsStruct), From 638b724b2c933d3513895d2195607202044b29f5 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 14 Nov 2024 15:12:43 +0100 Subject: [PATCH 10/35] Missing renames --- packages/snaps-rpc-methods/src/endowments/protocol.ts | 4 ++-- packages/snaps-utils/src/manifest/validation.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/snaps-rpc-methods/src/endowments/protocol.ts b/packages/snaps-rpc-methods/src/endowments/protocol.ts index e182ad755b..c9b0364101 100644 --- a/packages/snaps-rpc-methods/src/endowments/protocol.ts +++ b/packages/snaps-rpc-methods/src/endowments/protocol.ts @@ -81,10 +81,10 @@ export function getProtocolCaveatMapper( const caveats = []; - if (value.chains) { + if (value.scopes) { caveats.push({ type: SnapCaveatType.ProtocolSnapScopes, - value: value.chains, + value: value.scopes, }); } diff --git a/packages/snaps-utils/src/manifest/validation.ts b/packages/snaps-utils/src/manifest/validation.ts index 02b7208d78..2eabc2a769 100644 --- a/packages/snaps-utils/src/manifest/validation.ts +++ b/packages/snaps-utils/src/manifest/validation.ts @@ -210,7 +210,7 @@ export const PermissionsStruct: Describe = type({ 'endowment:protocol': optional( mergeStructs( HandlerCaveatsStruct, - object({ chains: ProtocolScopesStruct }), + object({ scopes: ProtocolScopesStruct }), ), ), 'endowment:lifecycle-hooks': optional(HandlerCaveatsStruct), From 01e1c83ee7bf375d5e240db452732af18ac10800 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 15 Nov 2024 11:08:36 +0100 Subject: [PATCH 11/35] Use KeyringController --- .../MultichainRoutingController.test.ts | 25 ++++++---- .../multichain/MultichainRoutingController.ts | 50 +++++++++---------- .../src/test-utils/controller.ts | 1 + .../src/test-utils/multichain.ts | 7 --- 4 files changed, 38 insertions(+), 45 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts index 19883e7926..77fd54e787 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts @@ -11,7 +11,6 @@ import { SOLANA_CAIP2, MOCK_SOLANA_ACCOUNTS, MOCK_BTC_ACCOUNTS, - getMockSnapKeyring, } from '../test-utils'; import { MultichainRoutingController } from './MultichainRoutingController'; @@ -24,11 +23,6 @@ describe('MultichainRoutingController', () => { /* eslint-disable-next-line no-new */ new MultichainRoutingController({ messenger, - getSnapKeyring: getMockSnapKeyring( - jest.fn().mockResolvedValue({ - txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', - }), - ), }); rootMessenger.registerActionHandler( @@ -36,6 +30,13 @@ describe('MultichainRoutingController', () => { () => MOCK_BTC_ACCOUNTS, ); + rootMessenger.registerActionHandler( + 'KeyringController:submitNonEvmRequest', + async () => ({ + txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', + }), + ); + rootMessenger.registerActionHandler( 'SnapController:handleRequest', async ({ handler }) => { @@ -74,9 +75,6 @@ describe('MultichainRoutingController', () => { /* eslint-disable-next-line no-new */ new MultichainRoutingController({ messenger, - getSnapKeyring: getMockSnapKeyring( - jest.fn().mockResolvedValue({ signature: '0x' }), - ), }); rootMessenger.registerActionHandler( @@ -84,6 +82,13 @@ describe('MultichainRoutingController', () => { () => MOCK_SOLANA_ACCOUNTS, ); + rootMessenger.registerActionHandler( + 'KeyringController:submitNonEvmRequest', + async () => ({ + signature: '0x', + }), + ); + rootMessenger.registerActionHandler( 'PermissionController:getPermissions', () => MOCK_SOLANA_SNAP_PERMISSIONS, @@ -125,7 +130,6 @@ describe('MultichainRoutingController', () => { /* eslint-disable-next-line no-new */ new MultichainRoutingController({ messenger, - getSnapKeyring: getMockSnapKeyring(), }); rootMessenger.registerActionHandler( @@ -175,7 +179,6 @@ describe('MultichainRoutingController', () => { /* eslint-disable-next-line no-new */ new MultichainRoutingController({ messenger, - getSnapKeyring: getMockSnapKeyring(), }); rootMessenger.registerActionHandler( diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index 9d13fa1f86..0e3205a1be 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -58,6 +58,16 @@ export type AccountsControllerListMultichainAccountsAction = { handler: (chainId?: CaipChainId) => InternalAccount[]; }; +export type KeyringControllerSubmitNonEvmRequestAction = { + type: `KeyringController:submitNonEvmRequest`; + handler: (args: { + address: string; + method: string; + params?: Json[] | Record; + chainId: CaipChainId; + }) => Promise; +}; + export type MultichainRoutingControllerActions = | MultichainRoutingControllerGetStateAction | MultichainRoutingControllerHandleRequestAction; @@ -66,7 +76,8 @@ export type MultichainRoutingControllerAllowedActions = | GetAllSnaps | HandleSnapRequest | GetPermissions - | AccountsControllerListMultichainAccountsAction; + | AccountsControllerListMultichainAccountsAction + | KeyringControllerSubmitNonEvmRequestAction; export type MultichainRoutingControllerEvents = MultichainRoutingControllerStateChangeEvent; @@ -81,19 +92,9 @@ export type MultichainRoutingControllerMessenger = MultichainRoutingControllerEvents['type'] >; -export type SnapKeyring = { - submitNonEvmRequest: (args: { - address: string; - method: string; - params?: Json[] | Record; - chainId: CaipChainId; - }) => Promise; -}; - export type MultichainRoutingControllerArgs = { messenger: MultichainRoutingControllerMessenger; state?: MultichainRoutingControllerState; - getSnapKeyring: () => Promise; }; export type MultichainRoutingControllerState = EmptyObject; @@ -110,13 +111,7 @@ export class MultichainRoutingController extends BaseController< MultichainRoutingControllerState, MultichainRoutingControllerMessenger > { - #getSnapKeyring: () => Promise; - - constructor({ - messenger, - state, - getSnapKeyring, - }: MultichainRoutingControllerArgs) { + constructor({ messenger, state }: MultichainRoutingControllerArgs) { super({ messenger, metadata: {}, @@ -126,8 +121,6 @@ export class MultichainRoutingController extends BaseController< }, }); - this.#getSnapKeyring = getSnapKeyring; - this.messagingSystem.registerActionHandler( `${controllerName}:handleRequest`, async (...args) => this.handleRequest(...args), @@ -258,13 +251,16 @@ export class MultichainRoutingController extends BaseController< request, ); if (accountSnap) { - const keyring = await this.#getSnapKeyring(); - return keyring.submitNonEvmRequest({ - address: accountSnap.address, - method, - params, - chainId: scope, - }); + // TODO: Decide on API for this. + return this.messagingSystem.call( + 'KeyringController:submitNonEvmRequest', + { + address: accountSnap.address, + method, + params, + chainId: scope, + }, + ); } // If the RPC request cannot be serviced by an account Snap, diff --git a/packages/snaps-controllers/src/test-utils/controller.ts b/packages/snaps-controllers/src/test-utils/controller.ts index a72310adb4..8eb4b532c8 100644 --- a/packages/snaps-controllers/src/test-utils/controller.ts +++ b/packages/snaps-controllers/src/test-utils/controller.ts @@ -897,6 +897,7 @@ export const getRestrictedMultichainRoutingControllerMessenger = ( 'SnapController:getAll', 'SnapController:handleRequest', 'AccountsController:listMultichainAccounts', + 'KeyringController:submitNonEvmRequest', ], }); diff --git a/packages/snaps-controllers/src/test-utils/multichain.ts b/packages/snaps-controllers/src/test-utils/multichain.ts index bd938fecfa..91c710bf28 100644 --- a/packages/snaps-controllers/src/test-utils/multichain.ts +++ b/packages/snaps-controllers/src/test-utils/multichain.ts @@ -87,10 +87,3 @@ export const MOCK_SOLANA_SNAP_PERMISSIONS: Record< parentCapability: SnapEndowments.Protocol, }, }; - -export const getMockSnapKeyring = (implementation = jest.fn()) => { - return async () => - Promise.resolve({ - submitNonEvmRequest: implementation, - }); -}; From c5d7291efac0cc98e731f9dda5498efb4f88b50d Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 15 Nov 2024 11:38:42 +0100 Subject: [PATCH 12/35] Handle origins and use proper method for address resolution --- .../MultichainRoutingController.test.ts | 6 ++-- .../multichain/MultichainRoutingController.ts | 11 +++++-- .../src/common/commands.ts | 15 +++++++++- .../src/common/validation.ts | 30 +++++++++++++++++++ 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts index 77fd54e787..1950f52b8b 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts @@ -40,8 +40,7 @@ describe('MultichainRoutingController', () => { rootMessenger.registerActionHandler( 'SnapController:handleRequest', async ({ handler }) => { - // TODO: Use proper handler - if (handler === HandlerType.OnProtocolRequest) { + if (handler === HandlerType.OnKeyringRequest) { return null; } throw new Error('Unmocked request'); @@ -97,8 +96,7 @@ describe('MultichainRoutingController', () => { rootMessenger.registerActionHandler( 'SnapController:handleRequest', async ({ handler }) => { - // TODO: Use proper handler - if (handler === HandlerType.OnProtocolRequest) { + if (handler === HandlerType.OnKeyringRequest) { return { address: SOLANA_CONNECTED_ACCOUNTS[0] }; } throw new Error('Unmocked request'); diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index 0e3205a1be..c759db8deb 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -133,19 +133,20 @@ export class MultichainRoutingController extends BaseController< request: JsonRpcRequest, ) { try { + // TODO: Decide if we should call this using another abstraction. const result = (await this.messagingSystem.call( 'SnapController:handleRequest', { snapId, origin: 'metamask', request: { - method: '', + method: 'keyring_resolveAccountAddress', params: { scope, request, }, }, - handler: HandlerType.OnProtocolRequest, // TODO: Export and request format + handler: HandlerType.OnKeyringRequest, }, )) as { address: CaipAccountId } | null; const address = result?.address; @@ -233,6 +234,7 @@ export class MultichainRoutingController extends BaseController< async handleRequest({ connectedAddresses, + origin, scope, request, }: { @@ -272,10 +274,13 @@ export class MultichainRoutingController extends BaseController< if (protocolSnap) { return this.messagingSystem.call('SnapController:handleRequest', { snapId: protocolSnap.snapId, - origin: 'metamask', // TODO: Determine origin of these requests? + origin: 'metamask', request: { method: '', params: { + // We are overriding the origin here, so that the Snap gets the proper origin + // while the permissions check is skipped due to the requesting origin being metamask. + origin, request, scope, }, diff --git a/packages/snaps-execution-environments/src/common/commands.ts b/packages/snaps-execution-environments/src/common/commands.ts index 227b698a0d..75a75e5700 100644 --- a/packages/snaps-execution-environments/src/common/commands.ts +++ b/packages/snaps-execution-environments/src/common/commands.ts @@ -17,6 +17,7 @@ import { assertIsOnUserInputRequestArguments, assertIsOnAssetsLookupRequestArguments, assertIsOnAssetsConversionRequestArguments, + assertIsOnProtocolRequestArguments, } from './validation'; export type CommandMethodsMapping = { @@ -86,9 +87,21 @@ export function getHandlerArguments( address, }; } + + case HandlerType.OnProtocolRequest: { + assertIsOnProtocolRequestArguments(request.params); + + // For this specific handler we extract the origin from the parameters. + const { + origin: nestedOrigin, + request: nestedRequest, + scope, + } = request.params; + return { origin: nestedOrigin, request: nestedRequest, scope }; + } + case HandlerType.OnRpcRequest: case HandlerType.OnKeyringRequest: - case HandlerType.OnProtocolRequest: // TODO: Decide on origin return { origin, request }; case HandlerType.OnCronjob: diff --git a/packages/snaps-execution-environments/src/common/validation.ts b/packages/snaps-execution-environments/src/common/validation.ts index 22a5e68033..409792dc1d 100644 --- a/packages/snaps-execution-environments/src/common/validation.ts +++ b/packages/snaps-execution-environments/src/common/validation.ts @@ -27,6 +27,7 @@ import { CaipAssetTypeStruct, JsonRpcIdStruct, JsonRpcParamsStruct, + JsonRpcRequestStruct, JsonRpcSuccessStruct, JsonRpcVersionStruct, JsonStruct, @@ -308,6 +309,35 @@ export function assertIsOnUserInputRequestArguments( ); } +export const OnProtocolRequestArgumentsStruct = object({ + origin: string(), + scope: ChainIdStruct, + request: JsonRpcRequestStruct, +}); + +export type OnProtocolRequestArguments = Infer< + typeof OnProtocolRequestArgumentsStruct +>; + +/** + * Asserts that the given value is a valid {@link OnProtocolRequestArguments} + * object. + * + * @param value - The value to validate. + * @throws If the value is not a valid {@link OnProtocolRequestArguments} + * object. + */ +export function assertIsOnProtocolRequestArguments( + value: unknown, +): asserts value is OnProtocolRequestArguments { + assertStruct( + value, + OnProtocolRequestArgumentsStruct, + 'Invalid request params', + rpcErrors.invalidParams, + ); +} + const OkResponseStruct = object({ id: JsonRpcIdStruct, jsonrpc: JsonRpcVersionStruct, From 20f6f74f36e2011eee07c0633432454b30101b25 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 15 Nov 2024 11:42:44 +0100 Subject: [PATCH 13/35] Simplify origin handling --- .../src/multichain/MultichainRoutingController.ts | 5 +---- .../snaps-execution-environments/src/common/commands.ts | 9 ++------- .../src/common/validation.ts | 1 - 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts index c759db8deb..106d249892 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts @@ -274,13 +274,10 @@ export class MultichainRoutingController extends BaseController< if (protocolSnap) { return this.messagingSystem.call('SnapController:handleRequest', { snapId: protocolSnap.snapId, - origin: 'metamask', + origin, request: { method: '', params: { - // We are overriding the origin here, so that the Snap gets the proper origin - // while the permissions check is skipped due to the requesting origin being metamask. - origin, request, scope, }, diff --git a/packages/snaps-execution-environments/src/common/commands.ts b/packages/snaps-execution-environments/src/common/commands.ts index 75a75e5700..345f604c30 100644 --- a/packages/snaps-execution-environments/src/common/commands.ts +++ b/packages/snaps-execution-environments/src/common/commands.ts @@ -91,13 +91,8 @@ export function getHandlerArguments( case HandlerType.OnProtocolRequest: { assertIsOnProtocolRequestArguments(request.params); - // For this specific handler we extract the origin from the parameters. - const { - origin: nestedOrigin, - request: nestedRequest, - scope, - } = request.params; - return { origin: nestedOrigin, request: nestedRequest, scope }; + const { request: nestedRequest, scope } = request.params; + return { request: nestedRequest, scope }; } case HandlerType.OnRpcRequest: diff --git a/packages/snaps-execution-environments/src/common/validation.ts b/packages/snaps-execution-environments/src/common/validation.ts index 409792dc1d..7b48d39207 100644 --- a/packages/snaps-execution-environments/src/common/validation.ts +++ b/packages/snaps-execution-environments/src/common/validation.ts @@ -310,7 +310,6 @@ export function assertIsOnUserInputRequestArguments( } export const OnProtocolRequestArgumentsStruct = object({ - origin: string(), scope: ChainIdStruct, request: JsonRpcRequestStruct, }); From 6a9679a22cf924d6c0f5e7c45465f007bc619858 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 15 Nov 2024 11:48:27 +0100 Subject: [PATCH 14/35] Fix typo --- packages/snaps-execution-environments/src/common/commands.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snaps-execution-environments/src/common/commands.ts b/packages/snaps-execution-environments/src/common/commands.ts index 345f604c30..db7b2f34d0 100644 --- a/packages/snaps-execution-environments/src/common/commands.ts +++ b/packages/snaps-execution-environments/src/common/commands.ts @@ -92,7 +92,7 @@ export function getHandlerArguments( assertIsOnProtocolRequestArguments(request.params); const { request: nestedRequest, scope } = request.params; - return { request: nestedRequest, scope }; + return { origin, request: nestedRequest, scope }; } case HandlerType.OnRpcRequest: From 3b932ae5fa46ba0d6eab9d01baac6934e30007cd Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 20 Nov 2024 11:56:28 +0100 Subject: [PATCH 15/35] Fix struct type --- .../src/common/validation.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/snaps-execution-environments/src/common/validation.ts b/packages/snaps-execution-environments/src/common/validation.ts index 7b48d39207..806e78dedf 100644 --- a/packages/snaps-execution-environments/src/common/validation.ts +++ b/packages/snaps-execution-environments/src/common/validation.ts @@ -4,7 +4,7 @@ import { UserInputEventStruct, } from '@metamask/snaps-sdk'; import { ChainIdStruct, HandlerType } from '@metamask/snaps-utils'; -import type { Infer } from '@metamask/superstruct'; +import type { Infer, Struct } from '@metamask/superstruct'; import { any, array, @@ -21,10 +21,16 @@ import { tuple, union, } from '@metamask/superstruct'; -import type { Json, JsonRpcSuccess } from '@metamask/utils'; +import type { + CaipChainId, + Json, + JsonRpcRequest, + JsonRpcSuccess, +} from '@metamask/utils'; import { assertStruct, CaipAssetTypeStruct, + CaipChainIdStruct, JsonRpcIdStruct, JsonRpcParamsStruct, JsonRpcRequestStruct, @@ -310,9 +316,9 @@ export function assertIsOnUserInputRequestArguments( } export const OnProtocolRequestArgumentsStruct = object({ - scope: ChainIdStruct, + scope: CaipChainIdStruct, request: JsonRpcRequestStruct, -}); +}) as unknown as Struct<{ scope: CaipChainId; request: JsonRpcRequest }, null>; export type OnProtocolRequestArguments = Infer< typeof OnProtocolRequestArgumentsStruct From be054faa0aed66bb625ee8a040e61790ff7418cc Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 20 Nov 2024 12:46:36 +0100 Subject: [PATCH 16/35] Add tests for protocol endowment --- .../src/endowments/keyring.test.ts | 2 + .../src/endowments/protocol.test.ts | 153 ++++++++++++++++++ .../src/endowments/protocol.ts | 7 +- .../snaps-rpc-methods/src/permissions.test.ts | 3 +- 4 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 packages/snaps-rpc-methods/src/endowments/protocol.test.ts diff --git a/packages/snaps-rpc-methods/src/endowments/keyring.test.ts b/packages/snaps-rpc-methods/src/endowments/keyring.test.ts index e591189cfb..569b221778 100644 --- a/packages/snaps-rpc-methods/src/endowments/keyring.test.ts +++ b/packages/snaps-rpc-methods/src/endowments/keyring.test.ts @@ -23,6 +23,8 @@ describe('endowment:keyring', () => { subjectTypes: [SubjectType.Snap], validator: expect.any(Function), }); + + expect(specification.endowmentGetter()).toBeNull(); }); describe('validator', () => { diff --git a/packages/snaps-rpc-methods/src/endowments/protocol.test.ts b/packages/snaps-rpc-methods/src/endowments/protocol.test.ts new file mode 100644 index 0000000000..8d3862fa4b --- /dev/null +++ b/packages/snaps-rpc-methods/src/endowments/protocol.test.ts @@ -0,0 +1,153 @@ +import { PermissionType, SubjectType } from '@metamask/permission-controller'; +import { SnapCaveatType } from '@metamask/snaps-utils'; + +import { SnapEndowments } from './enum'; +import { + getProtocolCaveatMapper, + getProtocolCaveatScopes, + protocolCaveatSpecifications, + protocolEndowmentBuilder, +} from './protocol'; + +describe('endowment:protocol', () => { + it('builds the expected permission specification', () => { + const specification = protocolEndowmentBuilder.specificationBuilder({}); + expect(specification).toStrictEqual({ + permissionType: PermissionType.Endowment, + targetName: SnapEndowments.Protocol, + endowmentGetter: expect.any(Function), + allowedCaveats: [ + SnapCaveatType.ProtocolSnapScopes, + SnapCaveatType.MaxRequestTime, + ], + subjectTypes: [SubjectType.Snap], + validator: expect.any(Function), + }); + + expect(specification.endowmentGetter()).toBeNull(); + }); + + describe('validator', () => { + it('throws if the caveat is not a single "protocolSnapScopes"', () => { + const specification = protocolEndowmentBuilder.specificationBuilder({}); + + expect(() => + specification.validator({ + // @ts-expect-error Missing other required permission types. + caveats: undefined, + }), + ).toThrow('Expected the following caveats: "protocolSnapScopes".'); + + expect(() => + // @ts-expect-error Missing other required permission types. + specification.validator({ + caveats: [{ type: 'foo', value: 'bar' }], + }), + ).toThrow( + 'Expected the following caveats: "protocolSnapScopes", "maxRequestTime", received "foo".', + ); + + expect(() => + // @ts-expect-error Missing other required permission types. + specification.validator({ + caveats: [ + { type: SnapCaveatType.ProtocolSnapScopes, value: 'bar' }, + { type: SnapCaveatType.ProtocolSnapScopes, value: 'bar' }, + ], + }), + ).toThrow('Duplicate caveats are not allowed.'); + }); + }); +}); + +describe('getProtocolCaveatMapper', () => { + it('maps a value to a caveat', () => { + expect( + getProtocolCaveatMapper({ + scopes: { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp': { + methods: ['getVersion'], + }, + }, + }), + ).toStrictEqual({ + caveats: [ + { + type: SnapCaveatType.ProtocolSnapScopes, + value: { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp': { + methods: ['getVersion'], + }, + }, + }, + ], + }); + }); + + it('returns null if value is empty', () => { + expect(getProtocolCaveatMapper({})).toStrictEqual({ caveats: null }); + }); +}); + +describe('getProtocolCaveatScopes', () => { + it('returns the scopes from the caveat', () => { + expect( + // @ts-expect-error Missing other required permission types. + getProtocolCaveatScopes({ + caveats: [ + { + type: SnapCaveatType.ProtocolSnapScopes, + value: { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp': { + methods: ['getVersion'], + }, + }, + }, + ], + }), + ).toStrictEqual({ + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp': { + methods: ['getVersion'], + }, + }); + }); + + it('returns null if no caveat found', () => { + expect( + // @ts-expect-error Missing other required permission types. + getProtocolCaveatScopes({ + caveats: null, + }), + ).toBeNull(); + }); +}); + +describe('protocolCaveatSpecifications', () => { + describe('validator', () => { + it('throws if the caveat values are invalid', () => { + expect(() => + protocolCaveatSpecifications[ + SnapCaveatType.ProtocolSnapScopes + ].validator?.( + // @ts-expect-error Missing value type. + { + type: SnapCaveatType.ProtocolSnapScopes, + }, + ), + ).toThrow('Expected a plain object.'); + + expect(() => + protocolCaveatSpecifications[ + SnapCaveatType.ProtocolSnapScopes + ].validator?.({ + type: SnapCaveatType.ProtocolSnapScopes, + value: { + foo: 'bar', + }, + }), + ).toThrow( + 'Invalid scopes specified: At path: foo -- Expected a string matching `/^(?[-a-z0-9]{3,8}):(?[-_a-zA-Z0-9]{1,32})$/` but received "foo".', + ); + }); + }); +}); diff --git a/packages/snaps-rpc-methods/src/endowments/protocol.ts b/packages/snaps-rpc-methods/src/endowments/protocol.ts index c9b0364101..9c4a529c91 100644 --- a/packages/snaps-rpc-methods/src/endowments/protocol.ts +++ b/packages/snaps-rpc-methods/src/endowments/protocol.ts @@ -49,7 +49,10 @@ const specificationBuilder: PermissionSpecificationBuilder< return { permissionType: PermissionType.Endowment, targetName: permissionName, - allowedCaveats: [SnapCaveatType.ChainIds, SnapCaveatType.MaxRequestTime], + allowedCaveats: [ + SnapCaveatType.ProtocolSnapScopes, + SnapCaveatType.MaxRequestTime, + ], endowmentGetter: (_getterOptions?: EndowmentGetterParams) => null, validator: createGenericPermissionValidator([ { type: SnapCaveatType.ProtocolSnapScopes }, @@ -127,7 +130,7 @@ function validateCaveat(caveat: Caveat): void { assertStruct( value, ProtocolScopesStruct, - 'Invalid chains specified', + 'Invalid scopes specified', rpcErrors.invalidParams, ); } diff --git a/packages/snaps-rpc-methods/src/permissions.test.ts b/packages/snaps-rpc-methods/src/permissions.test.ts index cea6759949..ba39525e4d 100644 --- a/packages/snaps-rpc-methods/src/permissions.test.ts +++ b/packages/snaps-rpc-methods/src/permissions.test.ts @@ -106,8 +106,7 @@ describe('buildSnapEndowmentSpecifications', () => { }, "endowment:protocol": { "allowedCaveats": [ - "chainIds", - "snapRpcMethods", + "protocolSnapScopes", "maxRequestTime", ], "endowmentGetter": [Function], From ab0f00b57af30582b6e79cc3ad3945c590b7f6e8 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 20 Nov 2024 13:07:45 +0100 Subject: [PATCH 17/35] Add more tests --- .../MultichainRoutingController.test.ts | 93 +++++++++++++++++++ .../common/BaseSnapExecutor.test.browser.ts | 42 +++++++++ .../src/common/validation.test.ts | 66 +++++++++++++ .../src/methods/specifications.test.ts | 13 +++ 4 files changed, 214 insertions(+) diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts index 1950f52b8b..cc08ae0b64 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts @@ -198,4 +198,97 @@ describe('MultichainRoutingController', () => { }), ).rejects.toThrow('The method does not exist / is not available'); }); + + it('throws if address resolution fails', async () => { + const rootMessenger = getRootMultichainRoutingControllerMessenger(); + const messenger = + getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRoutingController({ + messenger, + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_SOLANA_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + rootMessenger.registerActionHandler( + 'SnapController:handleRequest', + async ({ handler }) => { + if (handler === HandlerType.OnKeyringRequest) { + // Simulate the Snap returning a bogus address + return { address: 'foo' }; + } + throw new Error('Unmocked request'); + }, + ); + + await expect( + messenger.call('MultichainRoutingController:handleRequest', { + connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, + scope: SOLANA_CAIP2, + request: { + method: 'signAndSendTransaction', + params: { + message: 'foo', + }, + }, + }), + ).rejects.toThrow('Internal JSON-RPC error'); + }); + + it('throws if address resolution returns an address that isnt available', async () => { + const rootMessenger = getRootMultichainRoutingControllerMessenger(); + const messenger = + getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRoutingController({ + messenger, + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_SOLANA_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + rootMessenger.registerActionHandler( + 'SnapController:handleRequest', + async ({ handler }) => { + if (handler === HandlerType.OnKeyringRequest) { + // Simulate the Snap returning an unconnected address + return { + address: + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKa', + }; + } + throw new Error('Unmocked request'); + }, + ); + + await expect( + messenger.call('MultichainRoutingController:handleRequest', { + connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, + scope: SOLANA_CAIP2, + request: { + method: 'signAndSendTransaction', + params: { + message: 'foo', + }, + }, + }), + ).rejects.toThrow('Invalid method parameter(s)'); + }); }); diff --git a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts index d803c3ce66..1fb0576804 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts @@ -1683,6 +1683,48 @@ describe('BaseSnapExecutor', () => { }); }); + it('supports onProtocolRequest export', async () => { + const CODE = ` + module.exports.onProtocolRequest = ({ origin, scope, request }) => ({ origin, scope, request }) + `; + + const executor = new TestSnapExecutor(); + await executor.executeSnap(1, MOCK_SNAP_ID, CODE, []); + + expect(await executor.readCommand()).toStrictEqual({ + jsonrpc: '2.0', + id: 1, + result: 'OK', + }); + + const params = { + scope: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + request: { + jsonrpc: '2.0', + id: 'foo', + method: 'getVersion', + }, + }; + + await executor.writeCommand({ + jsonrpc: '2.0', + id: 2, + method: 'snapRpc', + params: [ + MOCK_SNAP_ID, + HandlerType.OnProtocolRequest, + MOCK_ORIGIN, + { jsonrpc: '2.0', method: '', params }, + ], + }); + + expect(await executor.readCommand()).toStrictEqual({ + id: 2, + jsonrpc: '2.0', + result: { origin: MOCK_ORIGIN, ...params }, + }); + }); + describe('lifecycle hooks', () => { const LIFECYCLE_HOOKS = [HandlerType.OnInstall, HandlerType.OnUpdate]; diff --git a/packages/snaps-execution-environments/src/common/validation.test.ts b/packages/snaps-execution-environments/src/common/validation.test.ts index eef0294d2a..c3cec65b34 100644 --- a/packages/snaps-execution-environments/src/common/validation.test.ts +++ b/packages/snaps-execution-environments/src/common/validation.test.ts @@ -4,6 +4,7 @@ import { assertIsOnAssetsConversionRequestArguments, assertIsOnAssetsLookupRequestArguments, assertIsOnNameLookupRequestArguments, + assertIsOnProtocolRequestArguments, assertIsOnSignatureRequestArguments, assertIsOnTransactionRequestArguments, assertIsOnUserInputRequestArguments, @@ -312,3 +313,68 @@ describe('assertIsOnAssetsConversionRequestArguments', () => { }, ); }); + +describe('assertIsOnProtocolRequestArguments', () => { + it.each([ + { + scope: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + request: { + jsonrpc: '2.0', + id: 'foo', + method: 'getVersion', + }, + }, + { + scope: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp', + request: { + jsonrpc: '2.0', + id: 'foo', + method: 'getVersion', + params: { + foo: 'bar', + }, + }, + }, + ])('does not throw for a valid protocol request param object', (value) => { + expect(() => assertIsOnProtocolRequestArguments(value)).not.toThrow(); + }); + + it.each([ + true, + false, + null, + undefined, + 0, + 1, + '', + 'foo', + [], + {}, + { request: { foo: 'bar' } }, + { + request: { + jsonrpc: '2.0', + id: 'foo', + method: 'getVersion', + params: { + foo: 'bar', + }, + }, + }, + { + scope: 'foo', + request: { + jsonrpc: '2.0', + id: 'foo', + method: 'getVersion', + }, + }, + ])( + 'throws if the value is not a valid protocol request params object', + (value) => { + expect(() => assertIsOnProtocolRequestArguments(value as any)).toThrow( + 'Invalid request params:', + ); + }, + ); +}); diff --git a/packages/snaps-simulation/src/methods/specifications.test.ts b/packages/snaps-simulation/src/methods/specifications.test.ts index a1be97a8d5..55007e456c 100644 --- a/packages/snaps-simulation/src/methods/specifications.test.ts +++ b/packages/snaps-simulation/src/methods/specifications.test.ts @@ -145,6 +145,19 @@ describe('getPermissionSpecifications', () => { ], "targetName": "endowment:page-settings", }, + "endowment:protocol": { + "allowedCaveats": [ + "protocolSnapScopes", + "maxRequestTime", + ], + "endowmentGetter": [Function], + "permissionType": "Endowment", + "subjectTypes": [ + "snap", + ], + "targetName": "endowment:protocol", + "validator": [Function], + }, "endowment:rpc": { "allowedCaveats": [ "rpcOrigin", From e8df59d03e88b17914dec5e24b24a20a0197dc5e Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 21 Nov 2024 11:45:08 +0100 Subject: [PATCH 18/35] MultichainRoutingController -> MultichainRouter --- ...oller.test.ts => MultichainRouter.test.ts} | 109 ++++++++--------- ...utingController.ts => MultichainRouter.ts} | 114 ++++++------------ .../snaps-controllers/src/multichain/index.ts | 2 +- .../src/test-utils/controller.ts | 27 ++--- 4 files changed, 97 insertions(+), 155 deletions(-) rename packages/snaps-controllers/src/multichain/{MultichainRoutingController.test.ts => MultichainRouter.test.ts} (69%) rename packages/snaps-controllers/src/multichain/{MultichainRoutingController.ts => MultichainRouter.ts} (68%) diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts similarity index 69% rename from packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts rename to packages/snaps-controllers/src/multichain/MultichainRouter.test.ts index cc08ae0b64..6b2b7df8e4 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts @@ -2,8 +2,8 @@ import { HandlerType } from '@metamask/snaps-utils'; import { getTruncatedSnap } from '@metamask/snaps-utils/test-utils'; import { - getRootMultichainRoutingControllerMessenger, - getRestrictedMultichainRoutingControllerMessenger, + getRootMultichainRouterMessenger, + getRestrictedMultichainRouterMessenger, BTC_CAIP2, BTC_CONNECTED_ACCOUNTS, MOCK_SOLANA_SNAP_PERMISSIONS, @@ -12,16 +12,15 @@ import { MOCK_SOLANA_ACCOUNTS, MOCK_BTC_ACCOUNTS, } from '../test-utils'; -import { MultichainRoutingController } from './MultichainRoutingController'; +import { MultichainRouter } from './MultichainRouter'; -describe('MultichainRoutingController', () => { +describe('MultichainRouter', () => { it('can route signing requests to account Snaps without address resolution', async () => { - const rootMessenger = getRootMultichainRoutingControllerMessenger(); - const messenger = - getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); /* eslint-disable-next-line no-new */ - new MultichainRoutingController({ + new MultichainRouter({ messenger, }); @@ -47,19 +46,16 @@ describe('MultichainRoutingController', () => { }, ); - const result = await messenger.call( - 'MultichainRoutingController:handleRequest', - { - connectedAddresses: BTC_CONNECTED_ACCOUNTS, - scope: BTC_CAIP2, - request: { - method: 'btc_sendmany', - params: { - message: 'foo', - }, + const result = await messenger.call('MultichainRouter:handleRequest', { + connectedAddresses: BTC_CONNECTED_ACCOUNTS, + scope: BTC_CAIP2, + request: { + method: 'btc_sendmany', + params: { + message: 'foo', }, }, - ); + }); expect(result).toStrictEqual({ txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', @@ -67,12 +63,11 @@ describe('MultichainRoutingController', () => { }); it('can route signing requests to account Snaps using address resolution', async () => { - const rootMessenger = getRootMultichainRoutingControllerMessenger(); - const messenger = - getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); /* eslint-disable-next-line no-new */ - new MultichainRoutingController({ + new MultichainRouter({ messenger, }); @@ -103,30 +98,26 @@ describe('MultichainRoutingController', () => { }, ); - const result = await messenger.call( - 'MultichainRoutingController:handleRequest', - { - connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, - scope: SOLANA_CAIP2, - request: { - method: 'signAndSendTransaction', - params: { - message: 'foo', - }, + const result = await messenger.call('MultichainRouter:handleRequest', { + connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, + scope: SOLANA_CAIP2, + request: { + method: 'signAndSendTransaction', + params: { + message: 'foo', }, }, - ); + }); expect(result).toStrictEqual({ signature: '0x' }); }); it('can route protocol requests to procotol Snaps', async () => { - const rootMessenger = getRootMultichainRoutingControllerMessenger(); - const messenger = - getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); /* eslint-disable-next-line no-new */ - new MultichainRoutingController({ + new MultichainRouter({ messenger, }); @@ -152,16 +143,13 @@ describe('MultichainRoutingController', () => { }), ); - const result = await messenger.call( - 'MultichainRoutingController:handleRequest', - { - connectedAddresses: [], - scope: SOLANA_CAIP2, - request: { - method: 'getVersion', - }, + const result = await messenger.call('MultichainRouter:handleRequest', { + connectedAddresses: [], + scope: SOLANA_CAIP2, + request: { + method: 'getVersion', }, - ); + }); expect(result).toStrictEqual({ 'feature-set': 2891131721, @@ -170,12 +158,11 @@ describe('MultichainRoutingController', () => { }); it('throws if no suitable Snaps are found', async () => { - const rootMessenger = getRootMultichainRoutingControllerMessenger(); - const messenger = - getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); /* eslint-disable-next-line no-new */ - new MultichainRoutingController({ + new MultichainRouter({ messenger, }); @@ -189,7 +176,7 @@ describe('MultichainRoutingController', () => { }); await expect( - messenger.call('MultichainRoutingController:handleRequest', { + messenger.call('MultichainRouter:handleRequest', { connectedAddresses: [], scope: SOLANA_CAIP2, request: { @@ -200,12 +187,11 @@ describe('MultichainRoutingController', () => { }); it('throws if address resolution fails', async () => { - const rootMessenger = getRootMultichainRoutingControllerMessenger(); - const messenger = - getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); /* eslint-disable-next-line no-new */ - new MultichainRoutingController({ + new MultichainRouter({ messenger, }); @@ -231,7 +217,7 @@ describe('MultichainRoutingController', () => { ); await expect( - messenger.call('MultichainRoutingController:handleRequest', { + messenger.call('MultichainRouter:handleRequest', { connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, scope: SOLANA_CAIP2, request: { @@ -245,12 +231,11 @@ describe('MultichainRoutingController', () => { }); it('throws if address resolution returns an address that isnt available', async () => { - const rootMessenger = getRootMultichainRoutingControllerMessenger(); - const messenger = - getRestrictedMultichainRoutingControllerMessenger(rootMessenger); + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); /* eslint-disable-next-line no-new */ - new MultichainRoutingController({ + new MultichainRouter({ messenger, }); @@ -279,7 +264,7 @@ describe('MultichainRoutingController', () => { ); await expect( - messenger.call('MultichainRoutingController:handleRequest', { + messenger.call('MultichainRouter:handleRequest', { connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, scope: SOLANA_CAIP2, request: { diff --git a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts similarity index 68% rename from packages/snaps-controllers/src/multichain/MultichainRoutingController.ts rename to packages/snaps-controllers/src/multichain/MultichainRouter.ts index 106d249892..77d0e8f343 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRoutingController.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -1,21 +1,11 @@ -import type { - RestrictedControllerMessenger, - ControllerGetStateAction, - ControllerStateChangeEvent, -} from '@metamask/base-controller'; -import { BaseController } from '@metamask/base-controller'; +import type { RestrictedControllerMessenger } from '@metamask/base-controller'; import type { GetPermissions } from '@metamask/permission-controller'; import { rpcErrors } from '@metamask/rpc-errors'; import { getProtocolCaveatScopes, SnapEndowments, } from '@metamask/snaps-rpc-methods'; -import type { - EmptyObject, - Json, - JsonRpcRequest, - SnapId, -} from '@metamask/snaps-sdk'; +import type { Json, JsonRpcRequest, SnapId } from '@metamask/snaps-sdk'; import { HandlerType } from '@metamask/snaps-utils'; import type { CaipAccountId, CaipChainId } from '@metamask/utils'; import { hasProperty, parseCaipAccountId } from '@metamask/utils'; @@ -23,23 +13,11 @@ import { hasProperty, parseCaipAccountId } from '@metamask/utils'; import { getRunnableSnaps } from '../snaps'; import type { GetAllSnaps, HandleSnapRequest } from '../snaps'; -export type MultichainRoutingControllerGetStateAction = - ControllerGetStateAction< - typeof controllerName, - MultichainRoutingControllerState - >; - -export type MultichainRoutingControllerHandleRequestAction = { - type: `${typeof controllerName}:handleRequest`; - handler: MultichainRoutingController['handleRequest']; +export type MultichainRouterHandleRequestAction = { + type: `${typeof name}:handleRequest`; + handler: MultichainRouter['handleRequest']; }; -export type MultichainRoutingControllerStateChangeEvent = - ControllerStateChangeEvent< - typeof controllerName, - MultichainRoutingControllerState - >; - // Since the AccountsController depends on snaps-controllers we manually type this type InternalAccount = { id: string; @@ -68,61 +46,44 @@ export type KeyringControllerSubmitNonEvmRequestAction = { }) => Promise; }; -export type MultichainRoutingControllerActions = - | MultichainRoutingControllerGetStateAction - | MultichainRoutingControllerHandleRequestAction; +export type MultichainRouterActions = MultichainRouterHandleRequestAction; -export type MultichainRoutingControllerAllowedActions = +export type MultichainRouterAllowedActions = | GetAllSnaps | HandleSnapRequest | GetPermissions | AccountsControllerListMultichainAccountsAction | KeyringControllerSubmitNonEvmRequestAction; -export type MultichainRoutingControllerEvents = - MultichainRoutingControllerStateChangeEvent; +export type MultichainRouterEvents = never; -export type MultichainRoutingControllerMessenger = - RestrictedControllerMessenger< - typeof controllerName, - | MultichainRoutingControllerActions - | MultichainRoutingControllerAllowedActions, - never, - MultichainRoutingControllerAllowedActions['type'], - MultichainRoutingControllerEvents['type'] - >; +export type MultichainRouterMessenger = RestrictedControllerMessenger< + typeof name, + MultichainRouterActions | MultichainRouterAllowedActions, + never, + MultichainRouterAllowedActions['type'], + MultichainRouterEvents['type'] +>; -export type MultichainRoutingControllerArgs = { - messenger: MultichainRoutingControllerMessenger; - state?: MultichainRoutingControllerState; +export type MultichainRouterArgs = { + messenger: MultichainRouterMessenger; }; -export type MultichainRoutingControllerState = EmptyObject; - type ProtocolSnap = { snapId: SnapId; methods: string[]; }; -const controllerName = 'MultichainRoutingController'; +const name = 'MultichainRouter'; + +export class MultichainRouter { + #messenger: MultichainRouterMessenger; -export class MultichainRoutingController extends BaseController< - typeof controllerName, - MultichainRoutingControllerState, - MultichainRoutingControllerMessenger -> { - constructor({ messenger, state }: MultichainRoutingControllerArgs) { - super({ - messenger, - metadata: {}, - name: controllerName, - state: { - ...state, - }, - }); + constructor({ messenger }: MultichainRouterArgs) { + this.#messenger = messenger; - this.messagingSystem.registerActionHandler( - `${controllerName}:handleRequest`, + this.#messenger.registerActionHandler( + `${name}:handleRequest`, async (...args) => this.handleRequest(...args), ); } @@ -134,7 +95,7 @@ export class MultichainRoutingController extends BaseController< ) { try { // TODO: Decide if we should call this using another abstraction. - const result = (await this.messagingSystem.call( + const result = (await this.#messenger.call( 'SnapController:handleRequest', { snapId, @@ -161,7 +122,7 @@ export class MultichainRoutingController extends BaseController< scope: CaipChainId, request: JsonRpcRequest, ) { - const accounts = this.messagingSystem + const accounts = this.#messenger .call('AccountsController:listMultichainAccounts', scope) .filter( (account: InternalAccount) => @@ -209,11 +170,11 @@ export class MultichainRoutingController extends BaseController< } #getProtocolSnaps(scope: CaipChainId) { - const allSnaps = this.messagingSystem.call('SnapController:getAll'); + const allSnaps = this.#messenger.call('SnapController:getAll'); const filteredSnaps = getRunnableSnaps(allSnaps); return filteredSnaps.reduce((accumulator, snap) => { - const permissions = this.messagingSystem.call( + const permissions = this.#messenger.call( 'PermissionController:getPermissions', snap.id, ); @@ -254,15 +215,12 @@ export class MultichainRoutingController extends BaseController< ); if (accountSnap) { // TODO: Decide on API for this. - return this.messagingSystem.call( - 'KeyringController:submitNonEvmRequest', - { - address: accountSnap.address, - method, - params, - chainId: scope, - }, - ); + return this.#messenger.call('KeyringController:submitNonEvmRequest', { + address: accountSnap.address, + method, + params, + chainId: scope, + }); } // If the RPC request cannot be serviced by an account Snap, @@ -272,7 +230,7 @@ export class MultichainRoutingController extends BaseController< snap.methods.includes(method), ); if (protocolSnap) { - return this.messagingSystem.call('SnapController:handleRequest', { + return this.#messenger.call('SnapController:handleRequest', { snapId: protocolSnap.snapId, origin, request: { diff --git a/packages/snaps-controllers/src/multichain/index.ts b/packages/snaps-controllers/src/multichain/index.ts index 6c07180225..8c696c379e 100644 --- a/packages/snaps-controllers/src/multichain/index.ts +++ b/packages/snaps-controllers/src/multichain/index.ts @@ -1 +1 @@ -export * from './MultichainRoutingController'; +export * from './MultichainRouter'; diff --git a/packages/snaps-controllers/src/test-utils/controller.ts b/packages/snaps-controllers/src/test-utils/controller.ts index 8eb4b532c8..173e132b62 100644 --- a/packages/snaps-controllers/src/test-utils/controller.ts +++ b/packages/snaps-controllers/src/test-utils/controller.ts @@ -54,9 +54,9 @@ import type { } from '../interface/SnapInterfaceController'; import { SnapController } from '../snaps'; import type { - MultichainRoutingControllerActions, - MultichainRoutingControllerAllowedActions, - MultichainRoutingControllerEvents, + MultichainRouterActions, + MultichainRouterAllowedActions, + MultichainRouterEvents, } from '../multichain'; import type { AllowedActions, @@ -867,12 +867,11 @@ export async function waitForStateChange( }); } -// Mock controller messenger for Multichain Routing Controller -export const getRootMultichainRoutingControllerMessenger = () => { +// Mock controller messenger for Multichain Router +export const getRootMultichainRouterMessenger = () => { const messenger = new MockControllerMessenger< - | MultichainRoutingControllerActions - | MultichainRoutingControllerAllowedActions, - MultichainRoutingControllerEvents + MultichainRouterActions | MultichainRouterAllowedActions, + MultichainRouterEvents >(); jest.spyOn(messenger, 'call'); @@ -880,17 +879,17 @@ export const getRootMultichainRoutingControllerMessenger = () => { return messenger; }; -export const getRestrictedMultichainRoutingControllerMessenger = ( +export const getRestrictedMultichainRouterMessenger = ( messenger: ReturnType< - typeof getRootMultichainRoutingControllerMessenger - > = getRootMultichainRoutingControllerMessenger(), + typeof getRootMultichainRouterMessenger + > = getRootMultichainRouterMessenger(), ) => { const controllerMessenger = messenger.getRestricted< - 'MultichainRoutingController', - MultichainRoutingControllerAllowedActions['type'], + 'MultichainRouter', + MultichainRouterAllowedActions['type'], never >({ - name: 'MultichainRoutingController', + name: 'MultichainRouter', allowedEvents: [], allowedActions: [ 'PermissionController:getPermissions', From 89e6421b612109572df23dbe345779ab40b0a3ca Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 5 Dec 2024 10:44:56 +0100 Subject: [PATCH 19/35] Add getSupportedMethods --- .../src/multichain/MultichainRouter.test.ts | 562 ++++++++++-------- .../src/multichain/MultichainRouter.ts | 47 +- 2 files changed, 374 insertions(+), 235 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts index 6b2b7df8e4..3e2750f36f 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts @@ -15,265 +15,359 @@ import { import { MultichainRouter } from './MultichainRouter'; describe('MultichainRouter', () => { - it('can route signing requests to account Snaps without address resolution', async () => { - const rootMessenger = getRootMultichainRouterMessenger(); - const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); - - /* eslint-disable-next-line no-new */ - new MultichainRouter({ - messenger, - }); - - rootMessenger.registerActionHandler( - 'AccountsController:listMultichainAccounts', - () => MOCK_BTC_ACCOUNTS, - ); - - rootMessenger.registerActionHandler( - 'KeyringController:submitNonEvmRequest', - async () => ({ - txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', - }), - ); - - rootMessenger.registerActionHandler( - 'SnapController:handleRequest', - async ({ handler }) => { - if (handler === HandlerType.OnKeyringRequest) { - return null; - } - throw new Error('Unmocked request'); - }, - ); - - const result = await messenger.call('MultichainRouter:handleRequest', { - connectedAddresses: BTC_CONNECTED_ACCOUNTS, - scope: BTC_CAIP2, - request: { - method: 'btc_sendmany', - params: { - message: 'foo', + describe('handleRequest', () => { + it('can route signing requests to account Snaps without address resolution', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_BTC_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'KeyringController:submitNonEvmRequest', + async () => ({ + txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', + }), + ); + + rootMessenger.registerActionHandler( + 'SnapController:handleRequest', + async ({ handler }) => { + if (handler === HandlerType.OnKeyringRequest) { + return null; + } + throw new Error('Unmocked request'); }, - }, - }); - - expect(result).toStrictEqual({ - txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', - }); - }); + ); - it('can route signing requests to account Snaps using address resolution', async () => { - const rootMessenger = getRootMultichainRouterMessenger(); - const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); - - /* eslint-disable-next-line no-new */ - new MultichainRouter({ - messenger, - }); - - rootMessenger.registerActionHandler( - 'AccountsController:listMultichainAccounts', - () => MOCK_SOLANA_ACCOUNTS, - ); - - rootMessenger.registerActionHandler( - 'KeyringController:submitNonEvmRequest', - async () => ({ - signature: '0x', - }), - ); - - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => MOCK_SOLANA_SNAP_PERMISSIONS, - ); - - rootMessenger.registerActionHandler( - 'SnapController:handleRequest', - async ({ handler }) => { - if (handler === HandlerType.OnKeyringRequest) { - return { address: SOLANA_CONNECTED_ACCOUNTS[0] }; - } - throw new Error('Unmocked request'); - }, - ); - - const result = await messenger.call('MultichainRouter:handleRequest', { - connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, - scope: SOLANA_CAIP2, - request: { - method: 'signAndSendTransaction', - params: { - message: 'foo', + const result = await messenger.call('MultichainRouter:handleRequest', { + connectedAddresses: BTC_CONNECTED_ACCOUNTS, + scope: BTC_CAIP2, + request: { + method: 'btc_sendmany', + params: { + message: 'foo', + }, }, - }, - }); + }); - expect(result).toStrictEqual({ signature: '0x' }); - }); - - it('can route protocol requests to procotol Snaps', async () => { - const rootMessenger = getRootMultichainRouterMessenger(); - const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); - - /* eslint-disable-next-line no-new */ - new MultichainRouter({ - messenger, - }); - - rootMessenger.registerActionHandler( - 'AccountsController:listMultichainAccounts', - () => [], - ); - - rootMessenger.registerActionHandler('SnapController:getAll', () => { - return [getTruncatedSnap()]; - }); - - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => MOCK_SOLANA_SNAP_PERMISSIONS, - ); - - rootMessenger.registerActionHandler( - 'SnapController:handleRequest', - async () => ({ - 'feature-set': 2891131721, - 'solana-core': '1.16.7', - }), - ); - - const result = await messenger.call('MultichainRouter:handleRequest', { - connectedAddresses: [], - scope: SOLANA_CAIP2, - request: { - method: 'getVersion', - }, - }); - - expect(result).toStrictEqual({ - 'feature-set': 2891131721, - 'solana-core': '1.16.7', + expect(result).toStrictEqual({ + txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', + }); }); - }); - - it('throws if no suitable Snaps are found', async () => { - const rootMessenger = getRootMultichainRouterMessenger(); - const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); - /* eslint-disable-next-line no-new */ - new MultichainRouter({ - messenger, - }); + it('can route signing requests to account Snaps using address resolution', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_SOLANA_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'KeyringController:submitNonEvmRequest', + async () => ({ + signature: '0x', + }), + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + rootMessenger.registerActionHandler( + 'SnapController:handleRequest', + async ({ handler }) => { + if (handler === HandlerType.OnKeyringRequest) { + return { address: SOLANA_CONNECTED_ACCOUNTS[0] }; + } + throw new Error('Unmocked request'); + }, + ); - rootMessenger.registerActionHandler( - 'AccountsController:listMultichainAccounts', - () => [], - ); + const result = await messenger.call('MultichainRouter:handleRequest', { + connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, + scope: SOLANA_CAIP2, + request: { + method: 'signAndSendTransaction', + params: { + message: 'foo', + }, + }, + }); - rootMessenger.registerActionHandler('SnapController:getAll', () => { - return []; + expect(result).toStrictEqual({ signature: '0x' }); }); - await expect( - messenger.call('MultichainRouter:handleRequest', { + it('can route protocol requests to procotol Snaps', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => [], + ); + + rootMessenger.registerActionHandler('SnapController:getAll', () => { + return [getTruncatedSnap()]; + }); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + rootMessenger.registerActionHandler( + 'SnapController:handleRequest', + async () => ({ + 'feature-set': 2891131721, + 'solana-core': '1.16.7', + }), + ); + + const result = await messenger.call('MultichainRouter:handleRequest', { connectedAddresses: [], scope: SOLANA_CAIP2, request: { method: 'getVersion', }, - }), - ).rejects.toThrow('The method does not exist / is not available'); - }); + }); - it('throws if address resolution fails', async () => { - const rootMessenger = getRootMultichainRouterMessenger(); - const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + expect(result).toStrictEqual({ + 'feature-set': 2891131721, + 'solana-core': '1.16.7', + }); + }); - /* eslint-disable-next-line no-new */ - new MultichainRouter({ - messenger, + it('throws if no suitable Snaps are found', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => [], + ); + + rootMessenger.registerActionHandler('SnapController:getAll', () => { + return []; + }); + + await expect( + messenger.call('MultichainRouter:handleRequest', { + connectedAddresses: [], + scope: SOLANA_CAIP2, + request: { + method: 'getVersion', + }, + }), + ).rejects.toThrow('The method does not exist / is not available'); }); - rootMessenger.registerActionHandler( - 'AccountsController:listMultichainAccounts', - () => MOCK_SOLANA_ACCOUNTS, - ); - - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => MOCK_SOLANA_SNAP_PERMISSIONS, - ); - - rootMessenger.registerActionHandler( - 'SnapController:handleRequest', - async ({ handler }) => { - if (handler === HandlerType.OnKeyringRequest) { - // Simulate the Snap returning a bogus address - return { address: 'foo' }; - } - throw new Error('Unmocked request'); - }, - ); - - await expect( - messenger.call('MultichainRouter:handleRequest', { - connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, - scope: SOLANA_CAIP2, - request: { - method: 'signAndSendTransaction', - params: { - message: 'foo', + it('throws if address resolution fails', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_SOLANA_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + rootMessenger.registerActionHandler( + 'SnapController:handleRequest', + async ({ handler }) => { + if (handler === HandlerType.OnKeyringRequest) { + // Simulate the Snap returning a bogus address + return { address: 'foo' }; + } + throw new Error('Unmocked request'); + }, + ); + + await expect( + messenger.call('MultichainRouter:handleRequest', { + connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, + scope: SOLANA_CAIP2, + request: { + method: 'signAndSendTransaction', + params: { + message: 'foo', + }, }, + }), + ).rejects.toThrow('Internal JSON-RPC error'); + }); + + it('throws if address resolution returns an address that isnt available', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_SOLANA_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + rootMessenger.registerActionHandler( + 'SnapController:handleRequest', + async ({ handler }) => { + if (handler === HandlerType.OnKeyringRequest) { + // Simulate the Snap returning an unconnected address + return { + address: + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKa', + }; + } + throw new Error('Unmocked request'); }, - }), - ).rejects.toThrow('Internal JSON-RPC error'); + ); + + await expect( + messenger.call('MultichainRouter:handleRequest', { + connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, + scope: SOLANA_CAIP2, + request: { + method: 'signAndSendTransaction', + params: { + message: 'foo', + }, + }, + }), + ).rejects.toThrow('Invalid method parameter(s)'); + }); }); - it('throws if address resolution returns an address that isnt available', async () => { - const rootMessenger = getRootMultichainRouterMessenger(); - const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + describe('getSupportedMethods', () => { + it('returns a set of both protocol and account Snap methods', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + }); + + rootMessenger.registerActionHandler('SnapController:getAll', () => { + return [getTruncatedSnap()]; + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_SOLANA_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + expect( + messenger.call('MultichainRouter:getSupportedMethods', { + scope: SOLANA_CAIP2, + }), + ).toStrictEqual(['signAndSendTransaction', 'getVersion']); + }); - /* eslint-disable-next-line no-new */ - new MultichainRouter({ - messenger, + it('handles lack of protocol Snaps', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + }); + + rootMessenger.registerActionHandler('SnapController:getAll', () => { + return [getTruncatedSnap()]; + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_SOLANA_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({}), + ); + + expect( + messenger.call('MultichainRouter:getSupportedMethods', { + scope: SOLANA_CAIP2, + }), + ).toStrictEqual(['signAndSendTransaction']); }); - rootMessenger.registerActionHandler( - 'AccountsController:listMultichainAccounts', - () => MOCK_SOLANA_ACCOUNTS, - ); - - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => MOCK_SOLANA_SNAP_PERMISSIONS, - ); - - rootMessenger.registerActionHandler( - 'SnapController:handleRequest', - async ({ handler }) => { - if (handler === HandlerType.OnKeyringRequest) { - // Simulate the Snap returning an unconnected address - return { - address: - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKa', - }; - } - throw new Error('Unmocked request'); - }, - ); - - await expect( - messenger.call('MultichainRouter:handleRequest', { - connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, - scope: SOLANA_CAIP2, - request: { - method: 'signAndSendTransaction', - params: { - message: 'foo', - }, - }, - }), - ).rejects.toThrow('Invalid method parameter(s)'); + it('handles lack of account Snaps', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + }); + + rootMessenger.registerActionHandler('SnapController:getAll', () => { + return [getTruncatedSnap()]; + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => [], + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + expect( + messenger.call('MultichainRouter:getSupportedMethods', { + scope: SOLANA_CAIP2, + }), + ).toStrictEqual(['getVersion']); + }); }); }); diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index 77d0e8f343..114a5fd0a3 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -18,6 +18,11 @@ export type MultichainRouterHandleRequestAction = { handler: MultichainRouter['handleRequest']; }; +export type MultichainRouterGetSupportedMethodsAction = { + type: `${typeof name}:getSupportedMethods`; + handler: MultichainRouter['getSupportedMethods']; +}; + // Since the AccountsController depends on snaps-controllers we manually type this type InternalAccount = { id: string; @@ -46,7 +51,9 @@ export type KeyringControllerSubmitNonEvmRequestAction = { }) => Promise; }; -export type MultichainRouterActions = MultichainRouterHandleRequestAction; +export type MultichainRouterActions = + | MultichainRouterHandleRequestAction + | MultichainRouterGetSupportedMethodsAction; export type MultichainRouterAllowedActions = | GetAllSnaps @@ -86,6 +93,11 @@ export class MultichainRouter { `${name}:handleRequest`, async (...args) => this.handleRequest(...args), ); + + this.#messenger.registerActionHandler( + `${name}:getSupportedMethods`, + (...args) => this.getSupportedMethods(...args), + ); } async #resolveRequestAddress( @@ -193,6 +205,18 @@ export class MultichainRouter { }, []); } + /** + * Handle an incoming JSON-RPC request tied to a specific scope by routing + * to either a procotol Snap or an account Snap. + * + * @param options - An options bag. + * @param options.connectedAddresses - Addresses currently connected to the origin. + * @param options.origin - The origin of the RPC request. + * @param options.request - The JSON-RPC request. + * @param options.scope - The CAIP-2 scope for the request. + * @returns The response from the chosen Snap. + * @throws If no handler was found. + */ async handleRequest({ connectedAddresses, origin, @@ -247,4 +271,25 @@ export class MultichainRouter { // If no compatible account or protocol Snaps were found, throw. throw rpcErrors.methodNotFound(); } + + /** + * Get a list of supported methods for a given scope. + * This combines both protocol and account Snaps supported methods. + * + * @param options - An options bag. + * @param options.scope - The CAIP-2 scope. + * @returns A list of supported methods. + */ + getSupportedMethods({ scope }: { scope: CaipChainId }): string[] { + const accountMethods = this.#messenger + .call('AccountsController:listMultichainAccounts', scope) + .filter((account: InternalAccount) => account.metadata.snap?.enabled) + .flatMap((account) => account.methods); + + const protocolMethods = this.#getProtocolSnaps(scope).flatMap( + (snap) => snap.methods, + ); + + return Array.from(new Set([...accountMethods, ...protocolMethods])); + } } From 300c434b7e51e2097a733fc01e2fdde339a3e988 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 13 Dec 2024 13:51:32 +0100 Subject: [PATCH 20/35] Use withKeyring --- .../src/multichain/MultichainRouter.test.ts | 77 +++++++++---------- .../src/multichain/MultichainRouter.ts | 70 +++++++++-------- .../src/test-utils/controller.ts | 1 - .../src/test-utils/multichain.ts | 22 ++++++ 4 files changed, 96 insertions(+), 74 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts index 3e2750f36f..ef8343ba1a 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts @@ -11,6 +11,7 @@ import { SOLANA_CAIP2, MOCK_SOLANA_ACCOUNTS, MOCK_BTC_ACCOUNTS, + getMockWithSnapKeyring, } from '../test-utils'; import { MultichainRouter } from './MultichainRouter'; @@ -19,10 +20,16 @@ describe('MultichainRouter', () => { it('can route signing requests to account Snaps without address resolution', async () => { const rootMessenger = getRootMultichainRouterMessenger(); const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring({ + submitRequest: jest.fn().mockResolvedValue({ + txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', + }), + }); /* eslint-disable-next-line no-new */ new MultichainRouter({ messenger, + withSnapKeyring, }); rootMessenger.registerActionHandler( @@ -30,13 +37,6 @@ describe('MultichainRouter', () => { () => MOCK_BTC_ACCOUNTS, ); - rootMessenger.registerActionHandler( - 'KeyringController:submitNonEvmRequest', - async () => ({ - txid: '53de51e2fa75c3cfa51132865f7d430138b1cd92a8f5267ec836ec565b422969', - }), - ); - rootMessenger.registerActionHandler( 'SnapController:handleRequest', async ({ handler }) => { @@ -66,10 +66,16 @@ describe('MultichainRouter', () => { it('can route signing requests to account Snaps using address resolution', async () => { const rootMessenger = getRootMultichainRouterMessenger(); const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring({ + submitRequest: jest.fn().mockResolvedValue({ + signature: '0x', + }), + }); /* eslint-disable-next-line no-new */ new MultichainRouter({ messenger, + withSnapKeyring, }); rootMessenger.registerActionHandler( @@ -77,13 +83,6 @@ describe('MultichainRouter', () => { () => MOCK_SOLANA_ACCOUNTS, ); - rootMessenger.registerActionHandler( - 'KeyringController:submitNonEvmRequest', - async () => ({ - signature: '0x', - }), - ); - rootMessenger.registerActionHandler( 'PermissionController:getPermissions', () => MOCK_SOLANA_SNAP_PERMISSIONS, @@ -113,13 +112,15 @@ describe('MultichainRouter', () => { expect(result).toStrictEqual({ signature: '0x' }); }); - it('can route protocol requests to procotol Snaps', async () => { + it('can route protocol requests to protocol Snaps', async () => { const rootMessenger = getRootMultichainRouterMessenger(); const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring(); /* eslint-disable-next-line no-new */ new MultichainRouter({ messenger, + withSnapKeyring, }); rootMessenger.registerActionHandler( @@ -161,10 +162,12 @@ describe('MultichainRouter', () => { it('throws if no suitable Snaps are found', async () => { const rootMessenger = getRootMultichainRouterMessenger(); const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring(); /* eslint-disable-next-line no-new */ new MultichainRouter({ messenger, + withSnapKeyring, }); rootMessenger.registerActionHandler( @@ -190,10 +193,15 @@ describe('MultichainRouter', () => { it('throws if address resolution fails', async () => { const rootMessenger = getRootMultichainRouterMessenger(); const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring({ + // Simulate the Snap returning a bogus address + resolveAccountAddress: jest.fn().mockResolvedValue({ address: 'foo' }), + }); /* eslint-disable-next-line no-new */ new MultichainRouter({ messenger, + withSnapKeyring, }); rootMessenger.registerActionHandler( @@ -206,17 +214,6 @@ describe('MultichainRouter', () => { () => MOCK_SOLANA_SNAP_PERMISSIONS, ); - rootMessenger.registerActionHandler( - 'SnapController:handleRequest', - async ({ handler }) => { - if (handler === HandlerType.OnKeyringRequest) { - // Simulate the Snap returning a bogus address - return { address: 'foo' }; - } - throw new Error('Unmocked request'); - }, - ); - await expect( messenger.call('MultichainRouter:handleRequest', { connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, @@ -234,10 +231,18 @@ describe('MultichainRouter', () => { it('throws if address resolution returns an address that isnt available', async () => { const rootMessenger = getRootMultichainRouterMessenger(); const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring({ + // Simulate the Snap returning an unconnected address + resolveAccountAddress: jest.fn().mockResolvedValue({ + address: + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKa', + }), + }); /* eslint-disable-next-line no-new */ new MultichainRouter({ messenger, + withSnapKeyring, }); rootMessenger.registerActionHandler( @@ -250,20 +255,6 @@ describe('MultichainRouter', () => { () => MOCK_SOLANA_SNAP_PERMISSIONS, ); - rootMessenger.registerActionHandler( - 'SnapController:handleRequest', - async ({ handler }) => { - if (handler === HandlerType.OnKeyringRequest) { - // Simulate the Snap returning an unconnected address - return { - address: - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKa', - }; - } - throw new Error('Unmocked request'); - }, - ); - await expect( messenger.call('MultichainRouter:handleRequest', { connectedAddresses: SOLANA_CONNECTED_ACCOUNTS, @@ -283,10 +274,12 @@ describe('MultichainRouter', () => { it('returns a set of both protocol and account Snap methods', async () => { const rootMessenger = getRootMultichainRouterMessenger(); const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring(); /* eslint-disable-next-line no-new */ new MultichainRouter({ messenger, + withSnapKeyring, }); rootMessenger.registerActionHandler('SnapController:getAll', () => { @@ -313,10 +306,12 @@ describe('MultichainRouter', () => { it('handles lack of protocol Snaps', async () => { const rootMessenger = getRootMultichainRouterMessenger(); const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring(); /* eslint-disable-next-line no-new */ new MultichainRouter({ messenger, + withSnapKeyring, }); rootMessenger.registerActionHandler('SnapController:getAll', () => { @@ -343,10 +338,12 @@ describe('MultichainRouter', () => { it('handles lack of account Snaps', async () => { const rootMessenger = getRootMultichainRouterMessenger(); const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring(); /* eslint-disable-next-line no-new */ new MultichainRouter({ messenger, + withSnapKeyring, }); rootMessenger.registerActionHandler('SnapController:getAll', () => { diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index 114a5fd0a3..702bd6b13b 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -6,6 +6,7 @@ import { SnapEndowments, } from '@metamask/snaps-rpc-methods'; import type { Json, JsonRpcRequest, SnapId } from '@metamask/snaps-sdk'; +import type { Caip2ChainId } from '@metamask/snaps-utils'; import { HandlerType } from '@metamask/snaps-utils'; import type { CaipAccountId, CaipChainId } from '@metamask/utils'; import { hasProperty, parseCaipAccountId } from '@metamask/utils'; @@ -36,21 +37,25 @@ type InternalAccount = { }; }; +type SnapKeyring = { + submitRequest: (request: Record) => Promise; + resolveAccountAddress: (options: { + snapId: SnapId; + scope: Caip2ChainId; + request: Json; + }) => Promise<{ address: CaipAccountId } | null>; +}; + +// Expecting a bound function that calls KeyringController.withKeyring selecting the Snap keyring +type WithSnapKeyringFunction = ( + operation: (keyring: SnapKeyring) => Promise, +) => Promise; + export type AccountsControllerListMultichainAccountsAction = { type: `AccountsController:listMultichainAccounts`; handler: (chainId?: CaipChainId) => InternalAccount[]; }; -export type KeyringControllerSubmitNonEvmRequestAction = { - type: `KeyringController:submitNonEvmRequest`; - handler: (args: { - address: string; - method: string; - params?: Json[] | Record; - chainId: CaipChainId; - }) => Promise; -}; - export type MultichainRouterActions = | MultichainRouterHandleRequestAction | MultichainRouterGetSupportedMethodsAction; @@ -59,8 +64,7 @@ export type MultichainRouterAllowedActions = | GetAllSnaps | HandleSnapRequest | GetPermissions - | AccountsControllerListMultichainAccountsAction - | KeyringControllerSubmitNonEvmRequestAction; + | AccountsControllerListMultichainAccountsAction; export type MultichainRouterEvents = never; @@ -74,6 +78,7 @@ export type MultichainRouterMessenger = RestrictedControllerMessenger< export type MultichainRouterArgs = { messenger: MultichainRouterMessenger; + withSnapKeyring: WithSnapKeyringFunction; }; type ProtocolSnap = { @@ -86,8 +91,11 @@ const name = 'MultichainRouter'; export class MultichainRouter { #messenger: MultichainRouterMessenger; - constructor({ messenger }: MultichainRouterArgs) { + #withSnapKeyring: WithSnapKeyringFunction; + + constructor({ messenger, withSnapKeyring }: MultichainRouterArgs) { this.#messenger = messenger; + this.#withSnapKeyring = withSnapKeyring; this.#messenger.registerActionHandler( `${name}:handleRequest`, @@ -107,20 +115,12 @@ export class MultichainRouter { ) { try { // TODO: Decide if we should call this using another abstraction. - const result = (await this.#messenger.call( - 'SnapController:handleRequest', - { + const result = (await this.#withSnapKeyring(async (keyring) => + keyring.resolveAccountAddress({ snapId, - origin: 'metamask', - request: { - method: 'keyring_resolveAccountAddress', - params: { - scope, - request, - }, - }, - handler: HandlerType.OnKeyringRequest, - }, + request, + scope, + }), )) as { address: CaipAccountId } | null; const address = result?.address; return address ? parseCaipAccountId(address).address : null; @@ -176,7 +176,7 @@ export class MultichainRouter { } return { - address: selectedAccount.address, + accountId: selectedAccount.id, snapId: selectedAccount.metadata.snap.id, }; } @@ -239,12 +239,16 @@ export class MultichainRouter { ); if (accountSnap) { // TODO: Decide on API for this. - return this.#messenger.call('KeyringController:submitNonEvmRequest', { - address: accountSnap.address, - method, - params, - chainId: scope, - }); + return this.#withSnapKeyring(async (keyring) => + keyring.submitRequest({ + id: accountSnap.accountId, + scope, + request: { + method, + params, + }, + }), + ); } // If the RPC request cannot be serviced by an account Snap, diff --git a/packages/snaps-controllers/src/test-utils/controller.ts b/packages/snaps-controllers/src/test-utils/controller.ts index 173e132b62..ba40b50425 100644 --- a/packages/snaps-controllers/src/test-utils/controller.ts +++ b/packages/snaps-controllers/src/test-utils/controller.ts @@ -896,7 +896,6 @@ export const getRestrictedMultichainRouterMessenger = ( 'SnapController:getAll', 'SnapController:handleRequest', 'AccountsController:listMultichainAccounts', - 'KeyringController:submitNonEvmRequest', ], }); diff --git a/packages/snaps-controllers/src/test-utils/multichain.ts b/packages/snaps-controllers/src/test-utils/multichain.ts index 91c710bf28..c4f7a39269 100644 --- a/packages/snaps-controllers/src/test-utils/multichain.ts +++ b/packages/snaps-controllers/src/test-utils/multichain.ts @@ -87,3 +87,25 @@ export const MOCK_SOLANA_SNAP_PERMISSIONS: Record< parentCapability: SnapEndowments.Protocol, }, }; + +type MockSnapKeyring = { + submitRequest: (request: unknown) => Promise; + resolveAccountAddress: (options: unknown) => Promise; +}; + +type MockOperationCallback = ( + keyring: MockSnapKeyring, +) => Promise; + +export const getMockWithSnapKeyring = ( + { submitRequest = jest.fn(), resolveAccountAddress = jest.fn() } = { + submitRequest: jest.fn(), + resolveAccountAddress: jest.fn(), + }, +) => { + return async (callback: MockOperationCallback) => + callback({ + submitRequest, + resolveAccountAddress, + }); +}; From 81f20ea5debdd872c81165f2f28808b99ec30393 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 13 Dec 2024 14:03:09 +0100 Subject: [PATCH 21/35] Improve typing --- .../src/multichain/MultichainRouter.ts | 8 ++++++-- .../snaps-controllers/src/test-utils/multichain.ts | 12 ++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index 702bd6b13b..917c43ec1f 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -8,7 +8,11 @@ import { import type { Json, JsonRpcRequest, SnapId } from '@metamask/snaps-sdk'; import type { Caip2ChainId } from '@metamask/snaps-utils'; import { HandlerType } from '@metamask/snaps-utils'; -import type { CaipAccountId, CaipChainId } from '@metamask/utils'; +import type { + CaipAccountId, + CaipChainId, + JsonRpcParams, +} from '@metamask/utils'; import { hasProperty, parseCaipAccountId } from '@metamask/utils'; import { getRunnableSnaps } from '../snaps'; @@ -245,7 +249,7 @@ export class MultichainRouter { scope, request: { method, - params, + params: params as JsonRpcParams, }, }), ); diff --git a/packages/snaps-controllers/src/test-utils/multichain.ts b/packages/snaps-controllers/src/test-utils/multichain.ts index c4f7a39269..6a28f1513a 100644 --- a/packages/snaps-controllers/src/test-utils/multichain.ts +++ b/packages/snaps-controllers/src/test-utils/multichain.ts @@ -2,7 +2,7 @@ import type { PermissionConstraint } from '@metamask/permission-controller'; import { SnapEndowments } from '@metamask/snaps-rpc-methods'; import { SnapCaveatType } from '@metamask/snaps-utils'; import { MOCK_SNAP_ID } from '@metamask/snaps-utils/test-utils'; -import type { CaipAccountId, CaipChainId } from '@metamask/utils'; +import type { CaipAccountId, CaipChainId, Json } from '@metamask/utils'; export const BTC_CAIP2 = 'bip122:000000000019d6689c085ae165831e93' as CaipChainId; @@ -89,13 +89,13 @@ export const MOCK_SOLANA_SNAP_PERMISSIONS: Record< }; type MockSnapKeyring = { - submitRequest: (request: unknown) => Promise; - resolveAccountAddress: (options: unknown) => Promise; + submitRequest: (request: unknown) => Promise; + resolveAccountAddress: ( + options: unknown, + ) => Promise<{ address: CaipAccountId } | null>; }; -type MockOperationCallback = ( - keyring: MockSnapKeyring, -) => Promise; +type MockOperationCallback = (keyring: MockSnapKeyring) => Promise; export const getMockWithSnapKeyring = ( { submitRequest = jest.fn(), resolveAccountAddress = jest.fn() } = { From 58c855d92ccbddc2a7e2a8833aced8a4e825b13d Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 16 Jan 2025 13:11:40 +0100 Subject: [PATCH 22/35] Add getSupportedAccounts --- .../src/multichain/MultichainRouter.test.ts | 36 +++++++++++++++++++ .../src/multichain/MultichainRouter.ts | 27 +++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts index ef8343ba1a..49b1117336 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts @@ -367,4 +367,40 @@ describe('MultichainRouter', () => { ).toStrictEqual(['getVersion']); }); }); + + describe('getSupportedAccounts', () => { + it('returns a set of both protocol and account Snap methods', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring(); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + withSnapKeyring, + }); + + rootMessenger.registerActionHandler('SnapController:getAll', () => { + return [getTruncatedSnap()]; + }); + + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_SOLANA_ACCOUNTS, + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => MOCK_SOLANA_SNAP_PERMISSIONS, + ); + + expect( + messenger.call('MultichainRouter:getSupportedAccounts', { + scope: SOLANA_CAIP2, + }), + ).toStrictEqual([ + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKv', + ]); + }); + }); }); diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index 917c43ec1f..48aff22fe6 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -28,6 +28,11 @@ export type MultichainRouterGetSupportedMethodsAction = { handler: MultichainRouter['getSupportedMethods']; }; +export type MultichainRouterGetSupportedAccountsAction = { + type: `${typeof name}:getSupportedAccounts`; + handler: MultichainRouter['getSupportedAccounts']; +}; + // Since the AccountsController depends on snaps-controllers we manually type this type InternalAccount = { id: string; @@ -62,7 +67,8 @@ export type AccountsControllerListMultichainAccountsAction = { export type MultichainRouterActions = | MultichainRouterHandleRequestAction - | MultichainRouterGetSupportedMethodsAction; + | MultichainRouterGetSupportedMethodsAction + | MultichainRouterGetSupportedAccountsAction; export type MultichainRouterAllowedActions = | GetAllSnaps @@ -110,6 +116,11 @@ export class MultichainRouter { `${name}:getSupportedMethods`, (...args) => this.getSupportedMethods(...args), ); + + this.#messenger.registerActionHandler( + `${name}:getSupportedAccounts`, + (...args) => this.getSupportedAccounts(...args), + ); } async #resolveRequestAddress( @@ -300,4 +311,18 @@ export class MultichainRouter { return Array.from(new Set([...accountMethods, ...protocolMethods])); } + + /** + * Get a list of supported accounts for a given scope. + * + * @param options - An options bag. + * @param options.scope - The CAIP-2 scope. + * @returns A list of CAIP-10 addresses. + */ + getSupportedAccounts({ scope }: { scope: CaipChainId }): string[] { + return this.#messenger + .call('AccountsController:listMultichainAccounts', scope) + .filter((account: InternalAccount) => account.metadata.snap?.enabled) + .map((account) => `${scope}:${account.address}`); + } } From 8fcf325c1c84083aaadb49b25d55e32daa41f2de Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 16 Jan 2025 13:25:35 +0100 Subject: [PATCH 23/35] More fixes --- .../snaps-controllers/src/multichain/MultichainRouter.ts | 1 - packages/snaps-controllers/src/test-utils/controller.ts | 2 +- packages/snaps-rpc-methods/jest.config.js | 8 ++++---- packages/snaps-utils/coverage.json | 6 +++--- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index 48aff22fe6..bc1b66ca44 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -243,7 +243,6 @@ export class MultichainRouter { scope: CaipChainId; request: JsonRpcRequest; }): Promise { - // TODO: Determine if the request is already validated here? const { method, params } = request; // If the RPC request can be serviced by an account Snap, route it there. diff --git a/packages/snaps-controllers/src/test-utils/controller.ts b/packages/snaps-controllers/src/test-utils/controller.ts index ba40b50425..fab6d0c937 100644 --- a/packages/snaps-controllers/src/test-utils/controller.ts +++ b/packages/snaps-controllers/src/test-utils/controller.ts @@ -52,12 +52,12 @@ import type { SnapInterfaceControllerEvents, StoredInterface, } from '../interface/SnapInterfaceController'; -import { SnapController } from '../snaps'; import type { MultichainRouterActions, MultichainRouterAllowedActions, MultichainRouterEvents, } from '../multichain'; +import { SnapController } from '../snaps'; import type { AllowedActions, AllowedEvents, diff --git a/packages/snaps-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js index d7bfe3cbd4..95c324bc1f 100644 --- a/packages/snaps-rpc-methods/jest.config.js +++ b/packages/snaps-rpc-methods/jest.config.js @@ -10,10 +10,10 @@ module.exports = deepmerge(baseConfig, { ], coverageThreshold: { global: { - branches: 94.93, - functions: 98.08, - lines: 98.69, - statements: 98.36, + branches: 95.09, + functions: 98.61, + lines: 98.8, + statements: 98.47, }, }, }); diff --git a/packages/snaps-utils/coverage.json b/packages/snaps-utils/coverage.json index 701516c7b8..16cb70ee5a 100644 --- a/packages/snaps-utils/coverage.json +++ b/packages/snaps-utils/coverage.json @@ -1,6 +1,6 @@ { "branches": 99.74, - "functions": 98.94, - "lines": 99.46, - "statements": 96.32 + "functions": 98.95, + "lines": 99.47, + "statements": 96.27 } From 11118388140ad81cc157498d9f57480f94347879 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 20 Jan 2025 12:28:35 +0100 Subject: [PATCH 24/35] Add isSupportedScope --- .../src/multichain/MultichainRouter.test.ts | 67 +++++++++++++------ .../src/multichain/MultichainRouter.ts | 36 ++++++++-- .../snaps-rpc-methods/src/endowments/index.ts | 2 +- 3 files changed, 77 insertions(+), 28 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts index 49b1117336..6c7d247ced 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts @@ -297,9 +297,7 @@ describe('MultichainRouter', () => { ); expect( - messenger.call('MultichainRouter:getSupportedMethods', { - scope: SOLANA_CAIP2, - }), + messenger.call('MultichainRouter:getSupportedMethods', SOLANA_CAIP2), ).toStrictEqual(['signAndSendTransaction', 'getVersion']); }); @@ -329,9 +327,7 @@ describe('MultichainRouter', () => { ); expect( - messenger.call('MultichainRouter:getSupportedMethods', { - scope: SOLANA_CAIP2, - }), + messenger.call('MultichainRouter:getSupportedMethods', SOLANA_CAIP2), ).toStrictEqual(['signAndSendTransaction']); }); @@ -361,15 +357,13 @@ describe('MultichainRouter', () => { ); expect( - messenger.call('MultichainRouter:getSupportedMethods', { - scope: SOLANA_CAIP2, - }), + messenger.call('MultichainRouter:getSupportedMethods', SOLANA_CAIP2), ).toStrictEqual(['getVersion']); }); }); describe('getSupportedAccounts', () => { - it('returns a set of both protocol and account Snap methods', async () => { + it('returns a set of accounts for the requested scope', async () => { const rootMessenger = getRootMultichainRouterMessenger(); const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); const withSnapKeyring = getMockWithSnapKeyring(); @@ -380,8 +374,29 @@ describe('MultichainRouter', () => { withSnapKeyring, }); - rootMessenger.registerActionHandler('SnapController:getAll', () => { - return [getTruncatedSnap()]; + rootMessenger.registerActionHandler( + 'AccountsController:listMultichainAccounts', + () => MOCK_SOLANA_ACCOUNTS, + ); + + expect( + messenger.call('MultichainRouter:getSupportedAccounts', SOLANA_CAIP2), + ).toStrictEqual([ + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKv', + ]); + }); + }); + + describe('isSupportedScope', () => { + it('returns true if an account Snap exists', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring(); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + withSnapKeyring, }); rootMessenger.registerActionHandler( @@ -389,18 +404,30 @@ describe('MultichainRouter', () => { () => MOCK_SOLANA_ACCOUNTS, ); + expect( + messenger.call('MultichainRouter:isSupportedScope', SOLANA_CAIP2), + ).toBe(true); + }); + + it('returns false if no account Snap is found', async () => { + const rootMessenger = getRootMultichainRouterMessenger(); + const messenger = getRestrictedMultichainRouterMessenger(rootMessenger); + const withSnapKeyring = getMockWithSnapKeyring(); + + /* eslint-disable-next-line no-new */ + new MultichainRouter({ + messenger, + withSnapKeyring, + }); + rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => MOCK_SOLANA_SNAP_PERMISSIONS, + 'AccountsController:listMultichainAccounts', + () => [], ); expect( - messenger.call('MultichainRouter:getSupportedAccounts', { - scope: SOLANA_CAIP2, - }), - ).toStrictEqual([ - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKv', - ]); + messenger.call('MultichainRouter:isSupportedScope', SOLANA_CAIP2), + ).toBe(false); }); }); }); diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index bc1b66ca44..aa675b6536 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -33,6 +33,11 @@ export type MultichainRouterGetSupportedAccountsAction = { handler: MultichainRouter['getSupportedAccounts']; }; +export type MultichainRouterIsSupportedScopeAction = { + type: `${typeof name}:isSupportedScope`; + handler: MultichainRouter['isSupportedScope']; +}; + // Since the AccountsController depends on snaps-controllers we manually type this type InternalAccount = { id: string; @@ -68,7 +73,8 @@ export type AccountsControllerListMultichainAccountsAction = { export type MultichainRouterActions = | MultichainRouterHandleRequestAction | MultichainRouterGetSupportedMethodsAction - | MultichainRouterGetSupportedAccountsAction; + | MultichainRouterGetSupportedAccountsAction + | MultichainRouterIsSupportedScopeAction; export type MultichainRouterAllowedActions = | GetAllSnaps @@ -121,6 +127,11 @@ export class MultichainRouter { `${name}:getSupportedAccounts`, (...args) => this.getSupportedAccounts(...args), ); + + this.#messenger.registerActionHandler( + `${name}:isSupportedScope`, + (...args) => this.isSupportedScope(...args), + ); } async #resolveRequestAddress( @@ -294,11 +305,10 @@ export class MultichainRouter { * Get a list of supported methods for a given scope. * This combines both protocol and account Snaps supported methods. * - * @param options - An options bag. - * @param options.scope - The CAIP-2 scope. + * @param scope - The CAIP-2 scope. * @returns A list of supported methods. */ - getSupportedMethods({ scope }: { scope: CaipChainId }): string[] { + getSupportedMethods(scope: CaipChainId): string[] { const accountMethods = this.#messenger .call('AccountsController:listMultichainAccounts', scope) .filter((account: InternalAccount) => account.metadata.snap?.enabled) @@ -314,14 +324,26 @@ export class MultichainRouter { /** * Get a list of supported accounts for a given scope. * - * @param options - An options bag. - * @param options.scope - The CAIP-2 scope. + * @param scope - The CAIP-2 scope. * @returns A list of CAIP-10 addresses. */ - getSupportedAccounts({ scope }: { scope: CaipChainId }): string[] { + getSupportedAccounts(scope: CaipChainId): string[] { return this.#messenger .call('AccountsController:listMultichainAccounts', scope) .filter((account: InternalAccount) => account.metadata.snap?.enabled) .map((account) => `${scope}:${account.address}`); } + + /** + * Determine whether a given CAIP-2 scope is supported by the router. + * + * @param scope - The CAIP-2 scope. + * @returns True if the router can service the scope, otherwise false. + */ + isSupportedScope(scope: CaipChainId): boolean { + // We currently assume here that if one Snap exists that service the scope, we can service the scope generally. + return this.#messenger + .call('AccountsController:listMultichainAccounts', scope) + .some((account: InternalAccount) => account.metadata.snap?.enabled); + } } diff --git a/packages/snaps-rpc-methods/src/endowments/index.ts b/packages/snaps-rpc-methods/src/endowments/index.ts index 4b4d96e4b3..c4d435d31e 100644 --- a/packages/snaps-rpc-methods/src/endowments/index.ts +++ b/packages/snaps-rpc-methods/src/endowments/index.ts @@ -125,10 +125,10 @@ export const handlerEndowments: Record = { [HandlerType.OnHomePage]: homePageEndowmentBuilder.targetName, [HandlerType.OnSettingsPage]: settingsPageEndowmentBuilder.targetName, [HandlerType.OnSignature]: signatureInsightEndowmentBuilder.targetName, - [HandlerType.OnProtocolRequest]: protocolEndowmentBuilder.targetName, [HandlerType.OnUserInput]: null, [HandlerType.OnAssetsLookup]: assetsEndowmentBuilder.targetName, [HandlerType.OnAssetsConversion]: assetsEndowmentBuilder.targetName, + [HandlerType.OnProtocolRequest]: protocolEndowmentBuilder.targetName, }; export * from './enum'; From 560e5550326424a1d6b0cb217eae6a3f7dda0b75 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 24 Jan 2025 15:16:52 +0100 Subject: [PATCH 25/35] Add export --- packages/snaps-controllers/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/snaps-controllers/src/index.ts b/packages/snaps-controllers/src/index.ts index 46f68a7381..ea8a94de5d 100644 --- a/packages/snaps-controllers/src/index.ts +++ b/packages/snaps-controllers/src/index.ts @@ -5,3 +5,4 @@ export * from './utils'; export * from './cronjob'; export * from './interface'; export * from './insights'; +export * from './multichain'; From edcb5d6a6470589fa5ec27159b0273186ca185fc Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 28 Jan 2025 18:54:15 +0100 Subject: [PATCH 26/35] Update API for SnapKeyring --- .../src/multichain/MultichainRouter.ts | 29 +++++++++---------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index aa675b6536..671ae96fe1 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -52,12 +52,17 @@ type InternalAccount = { }; type SnapKeyring = { - submitRequest: (request: Record) => Promise; - resolveAccountAddress: (options: { - snapId: SnapId; + submitRequest: (request: { + id: string; + method: string; + params?: Json[] | Record; scope: Caip2ChainId; - request: Json; - }) => Promise<{ address: CaipAccountId } | null>; + }) => Promise; + resolveAccountAddress: ( + snapId: SnapId, + scope: Caip2ChainId, + request: Json, + ) => Promise<{ address: CaipAccountId } | null>; }; // Expecting a bound function that calls KeyringController.withKeyring selecting the Snap keyring @@ -140,13 +145,8 @@ export class MultichainRouter { request: JsonRpcRequest, ) { try { - // TODO: Decide if we should call this using another abstraction. const result = (await this.#withSnapKeyring(async (keyring) => - keyring.resolveAccountAddress({ - snapId, - request, - scope, - }), + keyring.resolveAccountAddress(snapId, scope, request), )) as { address: CaipAccountId } | null; const address = result?.address; return address ? parseCaipAccountId(address).address : null; @@ -263,15 +263,12 @@ export class MultichainRouter { request, ); if (accountSnap) { - // TODO: Decide on API for this. return this.#withSnapKeyring(async (keyring) => keyring.submitRequest({ id: accountSnap.accountId, scope, - request: { - method, - params: params as JsonRpcParams, - }, + method, + params: params as JsonRpcParams, }), ); } From 8442e864f32b5120fdc6d6a6b75c97dc82f8f487 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 28 Jan 2025 19:20:50 +0100 Subject: [PATCH 27/35] Fix lint --- packages/snaps-utils/src/handlers.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/snaps-utils/src/handlers.ts b/packages/snaps-utils/src/handlers.ts index 52675564cd..ef24aa4da7 100644 --- a/packages/snaps-utils/src/handlers.ts +++ b/packages/snaps-utils/src/handlers.ts @@ -124,19 +124,21 @@ export const SNAP_EXPORTS = { [HandlerType.OnAssetsConversion]: { type: HandlerType.OnAssetsConversion, required: true, - validator: (snapExport: unknown): snapExport is OnAssetsConversionHandler => { + validator: ( + snapExport: unknown, + ): snapExport is OnAssetsConversionHandler => { return typeof snapExport === 'function'; }, }, [HandlerType.OnProtocolRequest]: { type: HandlerType.OnProtocolRequest, - required: false, + required: true, validator: ( snapExport: unknown, ): snapExport is OnProtocolRequestHandler => { return typeof snapExport === 'function'; }, - } + }, } as const; export const OnTransactionSeverityResponseStruct = object({ From ca7dc696898164456546f7150114200f11853591 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 28 Jan 2025 19:21:27 +0100 Subject: [PATCH 28/35] Update coverage --- packages/snaps-execution-environments/coverage.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index f8b47f1792..1bcdf58f69 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -2,5 +2,5 @@ "branches": 80.95, "functions": 89.47, "lines": 90.84, - "statements": 90 + "statements": 89.95 } From 8048e81d8296b5a5dbcda9e4de7025cc46eba44a Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 29 Jan 2025 10:37:25 +0100 Subject: [PATCH 29/35] Update manifests and coverage --- .../examples/packages/browserify-plugin/snap.manifest.json | 2 +- packages/examples/packages/browserify/snap.manifest.json | 2 +- packages/snaps-execution-environments/coverage.json | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/examples/packages/browserify-plugin/snap.manifest.json b/packages/examples/packages/browserify-plugin/snap.manifest.json index fc111c2e52..70799b3553 100644 --- a/packages/examples/packages/browserify-plugin/snap.manifest.json +++ b/packages/examples/packages/browserify-plugin/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snaps.git" }, "source": { - "shasum": "R4WjwqkDLNMUtU07n8AGq0WZKjsqjTjQXlASF++J4ws=", + "shasum": "Yzt/aRJTbRwAn3zbbK7W3MjgVVt2f6jLMGSc6pe2oyg=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/examples/packages/browserify/snap.manifest.json b/packages/examples/packages/browserify/snap.manifest.json index d42908c907..5cde77ba66 100644 --- a/packages/examples/packages/browserify/snap.manifest.json +++ b/packages/examples/packages/browserify/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snaps.git" }, "source": { - "shasum": "06xWu+ehUlNMpbJQxTZi2mlnAJId3cLHEK6fWD2Z9rc=", + "shasum": "J+fHvBGBrcoSZ5R1dEiIO3PegqNGFElb1i4h9Zw4zxg=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index 1bcdf58f69..9625ffb66a 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { - "branches": 80.95, - "functions": 89.47, - "lines": 90.84, + "branches": 81.08, + "functions": 89.54, + "lines": 90.92, "statements": 89.95 } From 5e7b78e3766bc6aac0da4bac8496931cfc32d24a Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 29 Jan 2025 11:36:04 +0100 Subject: [PATCH 30/35] Apply suggestions from code review Co-authored-by: Maarten Zuidhoorn --- packages/snaps-controllers/src/multichain/MultichainRouter.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index 671ae96fe1..c4143dbb5a 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -216,6 +216,7 @@ export class MultichainRouter { 'PermissionController:getPermissions', snap.id, ); + if (permissions && hasProperty(permissions, SnapEndowments.Protocol)) { const permission = permissions[SnapEndowments.Protocol]; const scopes = getProtocolCaveatScopes(permission); @@ -262,6 +263,7 @@ export class MultichainRouter { scope, request, ); + if (accountSnap) { return this.#withSnapKeyring(async (keyring) => keyring.submitRequest({ @@ -279,6 +281,7 @@ export class MultichainRouter { const protocolSnap = protocolSnaps.find((snap) => snap.methods.includes(method), ); + if (protocolSnap) { return this.#messenger.call('SnapController:handleRequest', { snapId: protocolSnap.snapId, From eb3964622c4830414bd7b1b97a9eaf0f39672cd8 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 29 Jan 2025 12:01:46 +0100 Subject: [PATCH 31/35] Address PR comments --- .../src/multichain/MultichainRouter.ts | 59 +++++++++++++++---- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index c4143dbb5a..31b646099c 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -139,6 +139,17 @@ export class MultichainRouter { ); } + /** + * Attempts to resolve the account address to use for a given request by inspecting the request itself. + * + * The request is sent to to an account Snap via the SnapKeyring that will attempt this resolution. + * + * @param snapId - The ID of the Snap to send the request to. + * @param scope - The CAIP-2 scope for the request. + * @param request - The JSON-RPC request. + * @returns The resolved address if found, otherwise null. + * @throws If the invocation of the SnapKeyring fails. + */ async #resolveRequestAddress( snapId: SnapId, scope: CaipChainId, @@ -155,7 +166,21 @@ export class MultichainRouter { } } - async #getAccountSnap( + /** + * Get the account ID of the account that should service the RPC request via an account Snap. + * + * This function checks whether any accounts exist that can service a given request by + * using a combination of the resolveAccountAddress functionality and the connected accounts. + * + * If an account is expected to service this request but none is found, the function will throw. + * + * @param connectedAddresses - The CAIP-10 addresses connected to the requesting origin. + * @param scope - The CAIP-2 scope for the request. + * @param request - The JSON-RPC request. + * @returns An account ID if found, otherwise null. + * @throws If no account is found, but the accounts exist that could service the request. + */ + async #getSnapAccountId( connectedAddresses: CaipAccountId[], scope: CaipChainId, request: JsonRpcRequest, @@ -163,12 +188,14 @@ export class MultichainRouter { const accounts = this.#messenger .call('AccountsController:listMultichainAccounts', scope) .filter( - (account: InternalAccount) => - account.metadata.snap?.enabled && + ( + account: InternalAccount, + ): account is InternalAccount & { + metadata: Required; + } => + Boolean(account.metadata.snap?.enabled) && account.methods.includes(request.method), - ) as (InternalAccount & { - metadata: Required; - })[]; + ); // If no accounts can service the request, return null. if (accounts.length === 0) { @@ -201,12 +228,18 @@ export class MultichainRouter { throw rpcErrors.invalidParams(); } - return { - accountId: selectedAccount.id, - snapId: selectedAccount.metadata.snap.id, - }; + return selectedAccount.id; } + /** + * Get all protocol Snaps that can service a given CAIP-2 scope. + * + * Protocol Snaps are deemed fit to service a scope if they are runnable + * and have the proper permissions set for the scope. + * + * @param scope - A CAIP-2 scope. + * @returns A list of all the protocol Snaps available and their RPC methods. + */ #getProtocolSnaps(scope: CaipChainId) { const allSnaps = this.#messenger.call('SnapController:getAll'); const filteredSnaps = getRunnableSnaps(allSnaps); @@ -258,16 +291,16 @@ export class MultichainRouter { const { method, params } = request; // If the RPC request can be serviced by an account Snap, route it there. - const accountSnap = await this.#getAccountSnap( + const accountId = await this.#getSnapAccountId( connectedAddresses, scope, request, ); - if (accountSnap) { + if (accountId) { return this.#withSnapKeyring(async (keyring) => keyring.submitRequest({ - id: accountSnap.accountId, + id: accountId, scope, method, params: params as JsonRpcParams, From 27ffe7c2d247db8beb0aacec5b874474f126ec89 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 29 Jan 2025 14:11:02 +0100 Subject: [PATCH 32/35] Update error message and add assertion --- packages/snaps-controllers/coverage.json | 8 ++++---- .../src/multichain/MultichainRouter.test.ts | 2 +- .../snaps-controllers/src/multichain/MultichainRouter.ts | 9 +++++++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json index 813ee1e36b..486cbc8f4d 100644 --- a/packages/snaps-controllers/coverage.json +++ b/packages/snaps-controllers/coverage.json @@ -1,6 +1,6 @@ { - "branches": 93.06, - "functions": 96.61, - "lines": 98.08, - "statements": 97.8 + "branches": 93.28, + "functions": 96.8, + "lines": 98.15, + "statements": 97.88 } diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts index 6c7d247ced..8c858a4816 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts @@ -266,7 +266,7 @@ describe('MultichainRouter', () => { }, }, }), - ).rejects.toThrow('Invalid method parameter(s)'); + ).rejects.toThrow('No available account found for request.'); }); }); diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index 31b646099c..3cdafbdc41 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -13,7 +13,7 @@ import type { CaipChainId, JsonRpcParams, } from '@metamask/utils'; -import { hasProperty, parseCaipAccountId } from '@metamask/utils'; +import { assert, hasProperty, parseCaipAccountId } from '@metamask/utils'; import { getRunnableSnaps } from '../snaps'; import type { GetAllSnaps, HandleSnapRequest } from '../snaps'; @@ -225,7 +225,9 @@ export class MultichainRouter { ); if (!selectedAccount) { - throw rpcErrors.invalidParams(); + throw rpcErrors.invalidParams({ + message: 'No available account found for request.', + }); } return selectedAccount.id; @@ -288,6 +290,9 @@ export class MultichainRouter { scope: CaipChainId; request: JsonRpcRequest; }): Promise { + // Explicitly block EVM scopes, just in case. + assert(!scope.startsWith('eip155') && !scope.startsWith('wallet:eip155')); + const { method, params } = request; // If the RPC request can be serviced by an account Snap, route it there. From f86cb098253527f8129a05ea840c9a8c18039309 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 29 Jan 2025 18:35:30 +0100 Subject: [PATCH 33/35] Fix option name --- packages/snaps-controllers/src/multichain/MultichainRouter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index 3cdafbdc41..688543ac31 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -53,7 +53,7 @@ type InternalAccount = { type SnapKeyring = { submitRequest: (request: { - id: string; + account: string; method: string; params?: Json[] | Record; scope: Caip2ChainId; @@ -305,7 +305,7 @@ export class MultichainRouter { if (accountId) { return this.#withSnapKeyring(async (keyring) => keyring.submitRequest({ - id: accountId, + account: accountId, scope, method, params: params as JsonRpcParams, From 2425dcda143ab45d4c487db0addfb95a0754c38c Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 30 Jan 2025 13:38:55 +0100 Subject: [PATCH 34/35] Use Json as the return type for handleRequest --- packages/snaps-controllers/src/multichain/MultichainRouter.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index 688543ac31..5d40bfdd65 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -289,7 +289,7 @@ export class MultichainRouter { origin: string; scope: CaipChainId; request: JsonRpcRequest; - }): Promise { + }): Promise { // Explicitly block EVM scopes, just in case. assert(!scope.startsWith('eip155') && !scope.startsWith('wallet:eip155')); @@ -332,7 +332,7 @@ export class MultichainRouter { }, }, handler: HandlerType.OnProtocolRequest, - }); + }) as Promise; } // If no compatible account or protocol Snaps were found, throw. From 2ac23966236a8ab63cc217dcc57c20413725cc7b Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 31 Jan 2025 13:37:56 +0100 Subject: [PATCH 35/35] Address PR comments --- .../src/multichain/MultichainRouter.test.ts | 2 +- .../src/multichain/MultichainRouter.ts | 47 +++++++++++++------ .../src/test-utils/multichain.ts | 2 +- 3 files changed, 34 insertions(+), 17 deletions(-) diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts index 8c858a4816..a17b6d0fc7 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.test.ts @@ -51,7 +51,7 @@ describe('MultichainRouter', () => { connectedAddresses: BTC_CONNECTED_ACCOUNTS, scope: BTC_CAIP2, request: { - method: 'btc_sendmany', + method: 'sendBitcoin', params: { message: 'foo', }, diff --git a/packages/snaps-controllers/src/multichain/MultichainRouter.ts b/packages/snaps-controllers/src/multichain/MultichainRouter.ts index 5d40bfdd65..4d23f8ef1b 100644 --- a/packages/snaps-controllers/src/multichain/MultichainRouter.ts +++ b/packages/snaps-controllers/src/multichain/MultichainRouter.ts @@ -6,14 +6,18 @@ import { SnapEndowments, } from '@metamask/snaps-rpc-methods'; import type { Json, JsonRpcRequest, SnapId } from '@metamask/snaps-sdk'; -import type { Caip2ChainId } from '@metamask/snaps-utils'; import { HandlerType } from '@metamask/snaps-utils'; import type { CaipAccountId, CaipChainId, JsonRpcParams, } from '@metamask/utils'; -import { assert, hasProperty, parseCaipAccountId } from '@metamask/utils'; +import { + assert, + hasProperty, + KnownCaipNamespace, + parseCaipAccountId, +} from '@metamask/utils'; import { getRunnableSnaps } from '../snaps'; import type { GetAllSnaps, HandleSnapRequest } from '../snaps'; @@ -56,11 +60,11 @@ type SnapKeyring = { account: string; method: string; params?: Json[] | Record; - scope: Caip2ChainId; + scope: CaipChainId; }) => Promise; resolveAccountAddress: ( snapId: SnapId, - scope: Caip2ChainId, + scope: CaipChainId, request: Json, ) => Promise<{ address: CaipAccountId } | null>; }; @@ -156,9 +160,9 @@ export class MultichainRouter { request: JsonRpcRequest, ) { try { - const result = (await this.#withSnapKeyring(async (keyring) => + const result = await this.#withSnapKeyring(async (keyring) => keyring.resolveAccountAddress(snapId, scope, request), - )) as { address: CaipAccountId } | null; + ); const address = result?.address; return address ? parseCaipAccountId(address).address : null; } catch { @@ -291,7 +295,10 @@ export class MultichainRouter { request: JsonRpcRequest; }): Promise { // Explicitly block EVM scopes, just in case. - assert(!scope.startsWith('eip155') && !scope.startsWith('wallet:eip155')); + assert( + !scope.startsWith(KnownCaipNamespace.Eip155) && + !scope.startsWith('wallet:eip155'), + ); const { method, params } = request; @@ -339,6 +346,18 @@ export class MultichainRouter { throw rpcErrors.methodNotFound(); } + /** + * Get a list of metadata for supported accounts for a given scope from the client. + * + * @param scope - The CAIP-2 scope. + * @returns A list of metadata for the supported accounts. + */ + #getSupportedAccountsMetadata(scope: CaipChainId): InternalAccount[] { + return this.#messenger + .call('AccountsController:listMultichainAccounts', scope) + .filter((account: InternalAccount) => account.metadata.snap?.enabled); + } + /** * Get a list of supported methods for a given scope. * This combines both protocol and account Snaps supported methods. @@ -347,10 +366,9 @@ export class MultichainRouter { * @returns A list of supported methods. */ getSupportedMethods(scope: CaipChainId): string[] { - const accountMethods = this.#messenger - .call('AccountsController:listMultichainAccounts', scope) - .filter((account: InternalAccount) => account.metadata.snap?.enabled) - .flatMap((account) => account.methods); + const accountMethods = this.#getSupportedAccountsMetadata(scope).flatMap( + (account) => account.methods, + ); const protocolMethods = this.#getProtocolSnaps(scope).flatMap( (snap) => snap.methods, @@ -366,10 +384,9 @@ export class MultichainRouter { * @returns A list of CAIP-10 addresses. */ getSupportedAccounts(scope: CaipChainId): string[] { - return this.#messenger - .call('AccountsController:listMultichainAccounts', scope) - .filter((account: InternalAccount) => account.metadata.snap?.enabled) - .map((account) => `${scope}:${account.address}`); + return this.#getSupportedAccountsMetadata(scope).map( + (account) => `${scope}:${account.address}`, + ); } /** diff --git a/packages/snaps-controllers/src/test-utils/multichain.ts b/packages/snaps-controllers/src/test-utils/multichain.ts index 6a28f1513a..361190786a 100644 --- a/packages/snaps-controllers/src/test-utils/multichain.ts +++ b/packages/snaps-controllers/src/test-utils/multichain.ts @@ -25,7 +25,7 @@ export const MOCK_BTC_ACCOUNTS = [ name: 'Bitcoin', }, }, - methods: ['btc_sendmany'], + methods: ['sendBitcoin'], options: { index: 0, scope: BTC_CAIP2 }, type: 'bip122:p2wpkh', },