From 66e5710d283ed55681e3564127b417e34203fb7d Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 5 Nov 2024 14:58:02 +0100 Subject: [PATCH 01/23] 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 76428d52979137625e2f025ce50e357d364507a8 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 5 Nov 2024 16:43:21 +0100 Subject: [PATCH 02/23] 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 | 10 + .../snaps-utils/src/manifest/validation.ts | 9 + 11 files changed, 257 insertions(+), 10 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 32d073f71f..222e5caa13 100644 --- a/packages/snaps-execution-environments/src/common/commands.ts +++ b/packages/snaps-execution-environments/src/common/commands.ts @@ -76,6 +76,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 f0d1577c6f..87e844f4b3 100644 --- a/packages/snaps-rpc-methods/src/endowments/enum.ts +++ b/packages/snaps-rpc-methods/src/endowments/enum.ts @@ -10,4 +10,5 @@ export enum SnapEndowments { LifecycleHooks = 'endowment:lifecycle-hooks', Keyring = 'endowment:keyring', HomePage = 'endowment:page-home', + Protocol = 'endowment:protocol', } diff --git a/packages/snaps-rpc-methods/src/endowments/index.ts b/packages/snaps-rpc-methods/src/endowments/index.ts index faa0c8fe06..2efe293d85 100644 --- a/packages/snaps-rpc-methods/src/endowments/index.ts +++ b/packages/snaps-rpc-methods/src/endowments/index.ts @@ -26,6 +26,11 @@ import { nameLookupEndowmentBuilder, } from './name-lookup'; import { networkAccessEndowmentBuilder } from './network-access'; +import { + getProtocolCaveatMapper, + protocolCaveatSpecifications, + protocolEndowmentBuilder, +} from './protocol'; import { getRpcCaveatMapper, rpcCaveatSpecifications, @@ -55,6 +60,7 @@ export const endowmentPermissionBuilders = { [nameLookupEndowmentBuilder.targetName]: nameLookupEndowmentBuilder, [lifecycleHooksEndowmentBuilder.targetName]: lifecycleHooksEndowmentBuilder, [keyringEndowmentBuilder.targetName]: keyringEndowmentBuilder, + [protocolEndowmentBuilder.targetName]: protocolEndowmentBuilder, [homePageEndowmentBuilder.targetName]: homePageEndowmentBuilder, [signatureInsightEndowmentBuilder.targetName]: signatureInsightEndowmentBuilder, @@ -68,6 +74,7 @@ export const endowmentCaveatSpecifications = { ...keyringCaveatSpecifications, ...signatureInsightCaveatSpecifications, ...maxRequestTimeCaveatSpecifications, + ...protocolCaveatSpecifications, }; export const endowmentCaveatMappers: Record< @@ -88,6 +95,9 @@ export const endowmentCaveatMappers: Record< [keyringEndowmentBuilder.targetName]: createMaxRequestTimeMapper( getKeyringCaveatMapper, ), + [protocolEndowmentBuilder.targetName]: createMaxRequestTimeMapper( + getProtocolCaveatMapper, + ), [signatureInsightEndowmentBuilder.targetName]: createMaxRequestTimeMapper( getSignatureInsightCaveatMapper, ), @@ -106,6 +116,7 @@ export const handlerEndowments: Record = { [HandlerType.OnKeyringRequest]: keyringEndowmentBuilder.targetName, [HandlerType.OnHomePage]: homePageEndowmentBuilder.targetName, [HandlerType.OnSignature]: signatureInsightEndowmentBuilder.targetName, + [HandlerType.OnProtocolRequest]: protocolEndowmentBuilder.targetName, [HandlerType.OnUserInput]: null, }; @@ -117,3 +128,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 1e4fc63634..8065072756 100644 --- a/packages/snaps-sdk/src/types/handlers/index.ts +++ b/packages/snaps-sdk/src/types/handlers/index.ts @@ -3,6 +3,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 642c201fec..d389062003 100644 --- a/packages/snaps-utils/src/handler-types.ts +++ b/packages/snaps-utils/src/handler-types.ts @@ -9,6 +9,7 @@ export enum HandlerType { OnKeyringRequest = 'onKeyringRequest', OnHomePage = 'onHomePage', OnUserInput = 'onUserInput', + OnProtocolRequest = 'onProtocolRequest', } export type SnapHandler = { diff --git a/packages/snaps-utils/src/handlers.ts b/packages/snaps-utils/src/handlers.ts index 6c4e37a9e6..44515aec94 100644 --- a/packages/snaps-utils/src/handlers.ts +++ b/packages/snaps-utils/src/handlers.ts @@ -4,6 +4,7 @@ import type { OnInstallHandler, OnKeyringRequestHandler, OnNameLookupHandler, + OnProtocolRequestHandler, OnRpcRequestHandler, OnSignatureHandler, OnTransactionHandler, @@ -103,6 +104,15 @@ export const SNAP_EXPORTS = { 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 dd65132a10..15dfdd7ac4 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), @@ -198,6 +201,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 53b56944b26878e4a91b120ff61d2ef696f8f87b Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 7 Nov 2024 12:23:26 +0100 Subject: [PATCH 03/23] 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 e6ee34e24115024946f85ab6837da9f514dd7c42 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 7 Nov 2024 13:01:18 +0100 Subject: [PATCH 04/23] 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 572141247a44b18462749d03e327860c85ebd59a Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 8 Nov 2024 13:22:22 +0100 Subject: [PATCH 05/23] 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 0ddc38db9ba31544848bd96d30f4d586decffa4a Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 12 Nov 2024 11:48:03 +0100 Subject: [PATCH 06/23] 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 4c69531223..bf3a6b7ecd 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -5442,7 +5442,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:signature-insight.', + '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:signature-insight, 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 d02ed0ee4a..53c684ea15 100644 --- a/packages/snaps-controllers/src/test-utils/controller.ts +++ b/packages/snaps-controllers/src/test-utils/controller.ts @@ -48,6 +48,11 @@ import type { SnapInterfaceControllerEvents, StoredInterface, } from '../interface/SnapInterfaceController'; +import type { + MultichainRoutingControllerActions, + MultichainRoutingControllerAllowedActions, + MultichainRoutingControllerEvents, +} from '../multichain'; import type { AllowedActions, AllowedEvents, @@ -827,3 +832,39 @@ export const getRestrictedSnapInsightsControllerMessenger = ( return controllerMessenger; }; + +// 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 50f911b100..618bc201ea 100644 --- a/packages/snaps-rpc-methods/src/permissions.test.ts +++ b/packages/snaps-rpc-methods/src/permissions.test.ts @@ -82,6 +82,20 @@ describe('buildSnapEndowmentSpecifications', () => { ], "targetName": "endowment:page-home", }, + "endowment:protocol": { + "allowedCaveats": [ + "chainIds", + "snapRpcMethods", + "maxRequestTime", + ], + "endowmentGetter": [Function], + "permissionType": "Endowment", + "subjectTypes": [ + "snap", + ], + "targetName": "endowment:protocol", + "validator": [Function], + }, "endowment:rpc": { "allowedCaveats": [ "rpcOrigin", From e3e045a30b8b3828fddab37ca8ba35e56ae0d38d Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 12 Nov 2024 13:15:47 +0100 Subject: [PATCH 07/23] 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 c1facd741358a6433bc9a85b394052fb9638c529 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 13 Nov 2024 15:03:13 +0100 Subject: [PATCH 08/23] 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 2efe293d85..f61e8cfbbf 100644 --- a/packages/snaps-rpc-methods/src/endowments/index.ts +++ b/packages/snaps-rpc-methods/src/endowments/index.ts @@ -128,7 +128,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 15dfdd7ac4..0a7c2dce00 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({ @@ -204,7 +207,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 06c99d1243da2c57098f6db52c395dc9330c8d93 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 14 Nov 2024 10:55:20 +0100 Subject: [PATCH 09/23] 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 f61e8cfbbf..1557caca71 100644 --- a/packages/snaps-rpc-methods/src/endowments/index.ts +++ b/packages/snaps-rpc-methods/src/endowments/index.ts @@ -128,4 +128,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 0a7c2dce00..ae732c32b0 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()) }), ); @@ -207,7 +207,7 @@ export const PermissionsStruct: Describe = type({ 'endowment:protocol': optional( mergeStructs( HandlerCaveatsStruct, - object({ chains: ProtocolChainsStruct }), + object({ chains: ProtocolScopesStruct }), ), ), 'endowment:lifecycle-hooks': optional(HandlerCaveatsStruct), From 26bb32154955ef5aee6868ca2d31b557a4fc6be0 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 14 Nov 2024 15:12:43 +0100 Subject: [PATCH 10/23] 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 ae732c32b0..95dbf5c7d2 100644 --- a/packages/snaps-utils/src/manifest/validation.ts +++ b/packages/snaps-utils/src/manifest/validation.ts @@ -207,7 +207,7 @@ export const PermissionsStruct: Describe = type({ 'endowment:protocol': optional( mergeStructs( HandlerCaveatsStruct, - object({ chains: ProtocolScopesStruct }), + object({ scopes: ProtocolScopesStruct }), ), ), 'endowment:lifecycle-hooks': optional(HandlerCaveatsStruct), From c9117f12bc46f48722d34c3ed5fdaa005a168e84 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 15 Nov 2024 11:08:36 +0100 Subject: [PATCH 11/23] 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 53c684ea15..e90951abf2 100644 --- a/packages/snaps-controllers/src/test-utils/controller.ts +++ b/packages/snaps-controllers/src/test-utils/controller.ts @@ -863,6 +863,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 aa0942a87623141c786f4aa735f99027883a094f Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 15 Nov 2024 11:38:42 +0100 Subject: [PATCH 12/23] 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 222e5caa13..0a5a965f0e 100644 --- a/packages/snaps-execution-environments/src/common/commands.ts +++ b/packages/snaps-execution-environments/src/common/commands.ts @@ -15,6 +15,7 @@ import { assertIsOnSignatureRequestArguments, assertIsOnNameLookupRequestArguments, assertIsOnUserInputRequestArguments, + assertIsOnProtocolRequestArguments, } from './validation'; export type CommandMethodsMapping = { @@ -74,9 +75,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 166c9df8c7..0fabe6d4cb 100644 --- a/packages/snaps-execution-environments/src/common/validation.ts +++ b/packages/snaps-execution-environments/src/common/validation.ts @@ -25,6 +25,7 @@ import { assertStruct, JsonRpcIdStruct, JsonRpcParamsStruct, + JsonRpcRequestStruct, JsonRpcSuccessStruct, JsonRpcVersionStruct, JsonStruct, @@ -243,6 +244,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 77f40e907e1b66ae852f17a2305ee1271e39e150 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 15 Nov 2024 11:42:44 +0100 Subject: [PATCH 13/23] 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 0a5a965f0e..d05ac67feb 100644 --- a/packages/snaps-execution-environments/src/common/commands.ts +++ b/packages/snaps-execution-environments/src/common/commands.ts @@ -79,13 +79,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 0fabe6d4cb..4d2f0e855e 100644 --- a/packages/snaps-execution-environments/src/common/validation.ts +++ b/packages/snaps-execution-environments/src/common/validation.ts @@ -245,7 +245,6 @@ export function assertIsOnUserInputRequestArguments( } export const OnProtocolRequestArgumentsStruct = object({ - origin: string(), scope: ChainIdStruct, request: JsonRpcRequestStruct, }); From 45ab1f361906040f3e7604fac0b72de8d9cc3829 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 15 Nov 2024 11:48:27 +0100 Subject: [PATCH 14/23] 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 d05ac67feb..88a10fe888 100644 --- a/packages/snaps-execution-environments/src/common/commands.ts +++ b/packages/snaps-execution-environments/src/common/commands.ts @@ -80,7 +80,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 4b7fb9133deeb34e643cb83d50a5cbdcf6c255da Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 20 Nov 2024 11:56:28 +0100 Subject: [PATCH 15/23] 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 4d2f0e855e..dcf3a7c67d 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, @@ -20,9 +20,15 @@ import { tuple, union, } from '@metamask/superstruct'; -import type { Json, JsonRpcSuccess } from '@metamask/utils'; +import type { + CaipChainId, + Json, + JsonRpcRequest, + JsonRpcSuccess, +} from '@metamask/utils'; import { assertStruct, + CaipChainIdStruct, JsonRpcIdStruct, JsonRpcParamsStruct, JsonRpcRequestStruct, @@ -245,9 +251,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 49936c39356cb631a4876116e83f1707ce484b17 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 20 Nov 2024 12:46:36 +0100 Subject: [PATCH 16/23] 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 618bc201ea..f6b0f65f6a 100644 --- a/packages/snaps-rpc-methods/src/permissions.test.ts +++ b/packages/snaps-rpc-methods/src/permissions.test.ts @@ -84,8 +84,7 @@ describe('buildSnapEndowmentSpecifications', () => { }, "endowment:protocol": { "allowedCaveats": [ - "chainIds", - "snapRpcMethods", + "protocolSnapScopes", "maxRequestTime", ], "endowmentGetter": [Function], From 1fd2bd370e9be396c23d0ed20fe05fc90621abc3 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 20 Nov 2024 13:07:45 +0100 Subject: [PATCH 17/23] Add more tests --- packages/snaps-controllers/coverage.json | 8 +- .../MultichainRoutingController.test.ts | 93 +++++++++++++++++++ .../common/BaseSnapExecutor.test.browser.ts | 42 +++++++++ .../src/common/validation.test.ts | 66 +++++++++++++ packages/snaps-rpc-methods/jest.config.js | 8 +- .../src/methods/specifications.test.ts | 13 +++ packages/snaps-utils/coverage.json | 2 +- 7 files changed, 223 insertions(+), 9 deletions(-) diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json index 28873f6536..a5f0e9447e 100644 --- a/packages/snaps-controllers/coverage.json +++ b/packages/snaps-controllers/coverage.json @@ -1,6 +1,6 @@ { - "branches": 92.89, - "functions": 96.71, - "lines": 98, - "statements": 97.71 + "branches": 93.11, + "functions": 96.79, + "lines": 98.06, + "statements": 97.77 } 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 88208997cb..a4ddd17770 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts @@ -1567,6 +1567,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 87a6b8ab5e..9e24e931d9 100644 --- a/packages/snaps-execution-environments/src/common/validation.test.ts +++ b/packages/snaps-execution-environments/src/common/validation.test.ts @@ -2,6 +2,7 @@ import { UserInputEventType } from '@metamask/snaps-sdk'; import { assertIsOnNameLookupRequestArguments, + assertIsOnProtocolRequestArguments, assertIsOnSignatureRequestArguments, assertIsOnTransactionRequestArguments, assertIsOnUserInputRequestArguments, @@ -232,3 +233,68 @@ describe('assertIsOnUserInputRequestArguments', () => { ); }); }); + +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-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js index 077ed1c65d..a4b68df916 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: 92.94, - functions: 97.26, - lines: 97.87, - statements: 97.39, + branches: 93.2, + functions: 97.89, + lines: 98.03, + statements: 97.56, }, }, }); diff --git a/packages/snaps-simulation/src/methods/specifications.test.ts b/packages/snaps-simulation/src/methods/specifications.test.ts index 487eda7235..cf09b4d8f8 100644 --- a/packages/snaps-simulation/src/methods/specifications.test.ts +++ b/packages/snaps-simulation/src/methods/specifications.test.ts @@ -123,6 +123,19 @@ describe('getPermissionSpecifications', () => { ], "targetName": "endowment:page-home", }, + "endowment:protocol": { + "allowedCaveats": [ + "protocolSnapScopes", + "maxRequestTime", + ], + "endowmentGetter": [Function], + "permissionType": "Endowment", + "subjectTypes": [ + "snap", + ], + "targetName": "endowment:protocol", + "validator": [Function], + }, "endowment:rpc": { "allowedCaveats": [ "rpcOrigin", diff --git a/packages/snaps-utils/coverage.json b/packages/snaps-utils/coverage.json index 102bd1c6df..a9248b7438 100644 --- a/packages/snaps-utils/coverage.json +++ b/packages/snaps-utils/coverage.json @@ -2,5 +2,5 @@ "branches": 99.74, "functions": 98.93, "lines": 99.46, - "statements": 96.37 + "statements": 96.31 } From 3bb3d58685c7bb53d95a3c6c8d24e4de0a94c0cb Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 21 Nov 2024 11:45:08 +0100 Subject: [PATCH 18/23] MultichainRoutingController -> MultichainRouter --- packages/snaps-controllers/coverage.json | 2 +- ...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 ++--- 5 files changed, 98 insertions(+), 156 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/coverage.json b/packages/snaps-controllers/coverage.json index a5f0e9447e..adeadc7aaa 100644 --- a/packages/snaps-controllers/coverage.json +++ b/packages/snaps-controllers/coverage.json @@ -2,5 +2,5 @@ "branches": 93.11, "functions": 96.79, "lines": 98.06, - "statements": 97.77 + "statements": 97.76 } 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 e90951abf2..21f9a1c1ad 100644 --- a/packages/snaps-controllers/src/test-utils/controller.ts +++ b/packages/snaps-controllers/src/test-utils/controller.ts @@ -49,9 +49,9 @@ import type { StoredInterface, } from '../interface/SnapInterfaceController'; import type { - MultichainRoutingControllerActions, - MultichainRoutingControllerAllowedActions, - MultichainRoutingControllerEvents, + MultichainRouterActions, + MultichainRouterAllowedActions, + MultichainRouterEvents, } from '../multichain'; import type { AllowedActions, @@ -833,12 +833,11 @@ export const getRestrictedSnapInsightsControllerMessenger = ( return controllerMessenger; }; -// 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'); @@ -846,17 +845,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 5cc2c6d0777a3058a019b84e5af24bccd362b74b Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 5 Dec 2024 10:44:56 +0100 Subject: [PATCH 19/23] 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 f12465ca086025cabb5e7cfdf67bdcac365f9f59 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 5 Dec 2024 10:47:57 +0100 Subject: [PATCH 20/23] Bump coverage --- packages/snaps-controllers/coverage.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json index adeadc7aaa..3a5d31b3b6 100644 --- a/packages/snaps-controllers/coverage.json +++ b/packages/snaps-controllers/coverage.json @@ -1,6 +1,6 @@ { "branches": 93.11, - "functions": 96.79, + "functions": 96.83, "lines": 98.06, - "statements": 97.76 + "statements": 97.78 } From f4b8ec18f7759d485b254fb9153ded0cc0cd9c36 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 13 Dec 2024 13:51:32 +0100 Subject: [PATCH 21/23] Use withKeyring --- packages/snaps-controllers/coverage.json | 4 +- .../src/multichain/MultichainRouter.test.ts | 77 +++++++++---------- .../src/multichain/MultichainRouter.ts | 70 +++++++++-------- .../src/test-utils/controller.ts | 1 - .../src/test-utils/multichain.ts | 22 ++++++ 5 files changed, 98 insertions(+), 76 deletions(-) diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json index 3a5d31b3b6..699238a079 100644 --- a/packages/snaps-controllers/coverage.json +++ b/packages/snaps-controllers/coverage.json @@ -1,6 +1,6 @@ { "branches": 93.11, - "functions": 96.83, - "lines": 98.06, + "functions": 96.85, + "lines": 98.07, "statements": 97.78 } 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 21f9a1c1ad..90c0fadd45 100644 --- a/packages/snaps-controllers/src/test-utils/controller.ts +++ b/packages/snaps-controllers/src/test-utils/controller.ts @@ -862,7 +862,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 5d70af911ce39cc17b4e70e74d19a36b35779d4f Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 13 Dec 2024 13:55:35 +0100 Subject: [PATCH 22/23] Update coverage --- packages/snaps-execution-environments/coverage.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index f958e510ae..9d61111f29 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { - "branches": 80.55, - "functions": 89.26, - "lines": 90.67, - "statements": 90.06 + "branches": 80.68, + "functions": 89.33, + "lines": 90.75, + "statements": 90.02 } From 0c62acf6866dfb76e62c0d167c30b4eda7e963c5 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Fri, 13 Dec 2024 14:03:09 +0100 Subject: [PATCH 23/23] 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() } = {