From 5d88a74876187ea9740fa2bb55933fd7d3c61e24 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 21 Jan 2025 11:24:16 +0100 Subject: [PATCH 01/17] feat: Implement SIP-29 permission and handlers --- .../src/endowments/assets.test.ts | 23 ++++++ .../src/endowments/assets.ts | 78 +++++++++++++++++++ .../snaps-rpc-methods/src/endowments/enum.ts | 1 + .../snaps-rpc-methods/src/endowments/index.ts | 7 ++ .../src/types/handlers/assets-conversion.ts | 24 ++++++ .../src/types/handlers/assets-lookup.ts | 24 ++++++ .../snaps-sdk/src/types/handlers/index.ts | 2 + packages/snaps-utils/src/handler-types.ts | 2 + packages/snaps-utils/src/handlers.ts | 14 ++++ 9 files changed, 175 insertions(+) create mode 100644 packages/snaps-rpc-methods/src/endowments/assets.test.ts create mode 100644 packages/snaps-rpc-methods/src/endowments/assets.ts create mode 100644 packages/snaps-sdk/src/types/handlers/assets-conversion.ts create mode 100644 packages/snaps-sdk/src/types/handlers/assets-lookup.ts diff --git a/packages/snaps-rpc-methods/src/endowments/assets.test.ts b/packages/snaps-rpc-methods/src/endowments/assets.test.ts new file mode 100644 index 0000000000..a47645b13f --- /dev/null +++ b/packages/snaps-rpc-methods/src/endowments/assets.test.ts @@ -0,0 +1,23 @@ +import { PermissionType, SubjectType } from '@metamask/permission-controller'; +import { SnapCaveatType } from '@metamask/snaps-utils'; + +import { assetsEndowmentBuilder } from './assets'; +import { SnapEndowments } from './enum'; + +describe('endowment:assets', () => { + describe('specificationBuilder', () => { + it('builds the expected permission specification', () => { + const specification = assetsEndowmentBuilder.specificationBuilder({}); + expect(specification).toStrictEqual({ + permissionType: PermissionType.Endowment, + targetName: SnapEndowments.Assets, + endowmentGetter: expect.any(Function), + allowedCaveats: [SnapCaveatType.ChainIds], + subjectTypes: [SubjectType.Snap], + validator: expect.any(Function), + }); + + expect(specification.endowmentGetter()).toBeNull(); + }); + }); +}); diff --git a/packages/snaps-rpc-methods/src/endowments/assets.ts b/packages/snaps-rpc-methods/src/endowments/assets.ts new file mode 100644 index 0000000000..57b0eb0998 --- /dev/null +++ b/packages/snaps-rpc-methods/src/endowments/assets.ts @@ -0,0 +1,78 @@ +import type { + PermissionSpecificationBuilder, + EndowmentGetterParams, + ValidPermissionSpecification, + PermissionConstraint, +} from '@metamask/permission-controller'; +import { PermissionType, SubjectType } from '@metamask/permission-controller'; +import { SnapCaveatType } from '@metamask/snaps-utils'; +import type { Json, NonEmptyArray } from '@metamask/utils'; +import { assert, isObject } from '@metamask/utils'; + +import { createGenericPermissionValidator } from './caveats'; +import { SnapEndowments } from './enum'; + +const permissionName = SnapEndowments.Assets; + +type AssetsEndowmentSpecification = ValidPermissionSpecification<{ + permissionType: PermissionType.Endowment; + targetName: typeof permissionName; + endowmentGetter: (_options?: any) => null; + allowedCaveats: Readonly> | null; +}>; + +/** + * `endowment:assets` returns nothing; it is intended to be used as a flag to determine whether the Snap can run asset queries. + * + * @param _builderOptions - Optional specification builder options. + * @returns The specification for the assets endowment. + */ +const specificationBuilder: PermissionSpecificationBuilder< + PermissionType.Endowment, + any, + AssetsEndowmentSpecification +> = (_builderOptions?: any) => { + return { + permissionType: PermissionType.Endowment, + targetName: permissionName, + allowedCaveats: [SnapCaveatType.ChainIds], + endowmentGetter: (_getterOptions?: EndowmentGetterParams) => null, + subjectTypes: [SubjectType.Snap], + validator: createGenericPermissionValidator([ + { type: SnapCaveatType.ChainIds }, + { type: SnapCaveatType.MaxRequestTime, optional: true }, + ]), + }; +}; + +export const assetsEndowmentBuilder = 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 getAssetsCaveatMapper( + value: Json, +): Pick { + if (!value || !isObject(value) || Object.keys(value).length === 0) { + return { caveats: null }; + } + + assert(value.scopes); + + return { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: value.scopes, + }, + ], + }; +} diff --git a/packages/snaps-rpc-methods/src/endowments/enum.ts b/packages/snaps-rpc-methods/src/endowments/enum.ts index cdd13a6746..96f4179fc6 100644 --- a/packages/snaps-rpc-methods/src/endowments/enum.ts +++ b/packages/snaps-rpc-methods/src/endowments/enum.ts @@ -11,4 +11,5 @@ export enum SnapEndowments { Keyring = 'endowment:keyring', HomePage = 'endowment:page-home', SettingsPage = 'endowment:page-settings', + Assets = 'endowment:assets', } diff --git a/packages/snaps-rpc-methods/src/endowments/index.ts b/packages/snaps-rpc-methods/src/endowments/index.ts index 7d82ac72d3..e0767cedd7 100644 --- a/packages/snaps-rpc-methods/src/endowments/index.ts +++ b/packages/snaps-rpc-methods/src/endowments/index.ts @@ -2,6 +2,7 @@ import type { PermissionConstraint } from '@metamask/permission-controller'; import { HandlerType } from '@metamask/snaps-utils'; import type { Json } from '@metamask/utils'; +import { assetsEndowmentBuilder, getAssetsCaveatMapper } from './assets'; import { createMaxRequestTimeMapper, getMaxRequestTimeCaveatMapper, @@ -60,6 +61,7 @@ export const endowmentPermissionBuilders = { [homePageEndowmentBuilder.targetName]: homePageEndowmentBuilder, [signatureInsightEndowmentBuilder.targetName]: signatureInsightEndowmentBuilder, + [assetsEndowmentBuilder.targetName]: assetsEndowmentBuilder, } as const; export const endowmentCaveatSpecifications = { @@ -96,6 +98,9 @@ export const endowmentCaveatMappers: Record< [lifecycleHooksEndowmentBuilder.targetName]: getMaxRequestTimeCaveatMapper, [homePageEndowmentBuilder.targetName]: getMaxRequestTimeCaveatMapper, [settingsPageEndowmentBuilder.targetName]: getMaxRequestTimeCaveatMapper, + [assetsEndowmentBuilder.targetName]: createMaxRequestTimeMapper( + getAssetsCaveatMapper, + ), }; // We allow null because a permitted handler does not have an endowment @@ -111,6 +116,8 @@ export const handlerEndowments: Record = { [HandlerType.OnSettingsPage]: settingsPageEndowmentBuilder.targetName, [HandlerType.OnSignature]: signatureInsightEndowmentBuilder.targetName, [HandlerType.OnUserInput]: null, + [HandlerType.OnAssetsLookup]: assetsEndowmentBuilder.targetName, + [HandlerType.OnAssetsConversion]: assetsEndowmentBuilder.targetName, }; export * from './enum'; diff --git a/packages/snaps-sdk/src/types/handlers/assets-conversion.ts b/packages/snaps-sdk/src/types/handlers/assets-conversion.ts new file mode 100644 index 0000000000..a98d5f193a --- /dev/null +++ b/packages/snaps-sdk/src/types/handlers/assets-conversion.ts @@ -0,0 +1,24 @@ +import type { CaipAssetType } from '@metamask/utils'; + +export type OnAssetsConversionArguments = { + conversions: { from: CaipAssetType; to: CaipAssetType }[]; +}; + +/** + * The `onAssetsConversion` handler. This is called by MetaMask when querying about asset conversion on specific chains. + * + * @returns The conversion for each asset. See + * {@link OnAssetsConversionResponse}. + */ +export type OnAssetsConversionHandler = ( + args: OnAssetsConversionArguments, +) => Promise; + +/** + * The response from the conversion query, containing rates about each requested asset pair. + * + * @property conversionRates - A nested object with two CAIP-19 keys that contains a conversion rate between the two keys. + */ +export type OnAssetsConversionResponse = { + conversionRates: Record>; +}; diff --git a/packages/snaps-sdk/src/types/handlers/assets-lookup.ts b/packages/snaps-sdk/src/types/handlers/assets-lookup.ts new file mode 100644 index 0000000000..f3bc61839d --- /dev/null +++ b/packages/snaps-sdk/src/types/handlers/assets-lookup.ts @@ -0,0 +1,24 @@ +import type { CaipAssetType } from '@metamask/utils'; + +export type OnAssetsLookupArguments = { + assets: CaipAssetType[]; +}; + +/** + * The `onAssetsLookup` handler. This is called by MetaMask when querying about specific assets on specific chains. + * + * @returns The metadata about each asset. See + * {@link OnAssetsLookupResponse}. + */ +export type OnAssetsLookupHandler = ( + args: OnAssetsLookupArguments, +) => Promise; + +/** + * The response from the query, containing metadata about each requested asset. + * + * @property assets - An object containing a mapping between the CAIP-19 key and a metadata object. + */ +export type OnAssetsLookupResponse = { + assets: Record; +}; diff --git a/packages/snaps-sdk/src/types/handlers/index.ts b/packages/snaps-sdk/src/types/handlers/index.ts index ab6a86d833..33fd11ac75 100644 --- a/packages/snaps-sdk/src/types/handlers/index.ts +++ b/packages/snaps-sdk/src/types/handlers/index.ts @@ -1,3 +1,5 @@ +export * from './assets-conversion'; +export * from './assets-lookup'; export * from './cronjob'; export * from './home-page'; export * from './keyring'; diff --git a/packages/snaps-utils/src/handler-types.ts b/packages/snaps-utils/src/handler-types.ts index 230486a95e..392969898b 100644 --- a/packages/snaps-utils/src/handler-types.ts +++ b/packages/snaps-utils/src/handler-types.ts @@ -10,6 +10,8 @@ export enum HandlerType { OnHomePage = 'onHomePage', OnSettingsPage = 'onSettingsPage', OnUserInput = 'onUserInput', + OnAssetsLookup = 'onAssetsLookup', + OnAssetsConversion = 'onAssetsConversion', } export type SnapHandler = { diff --git a/packages/snaps-utils/src/handlers.ts b/packages/snaps-utils/src/handlers.ts index a20a48b842..f2de2b6e2a 100644 --- a/packages/snaps-utils/src/handlers.ts +++ b/packages/snaps-utils/src/handlers.ts @@ -111,6 +111,20 @@ export const SNAP_EXPORTS = { return typeof snapExport === 'function'; }, }, + [HandlerType.OnAssetsLookup]: { + type: HandlerType.OnAssetsLookup, + required: true, + validator: (snapExport: unknown): snapExport is OnUserInputHandler => { + return typeof snapExport === 'function'; + }, + }, + [HandlerType.OnAssetsConversion]: { + type: HandlerType.OnAssetsConversion, + required: true, + validator: (snapExport: unknown): snapExport is OnUserInputHandler => { + return typeof snapExport === 'function'; + }, + }, } as const; export const OnTransactionSeverityResponseStruct = object({ From 071d8aaa1faddb23935d3547d6ba6708e1a623bf Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 21 Jan 2025 11:32:53 +0100 Subject: [PATCH 02/17] Add test for execution handlers --- .../common/BaseSnapExecutor.test.browser.ts | 66 +++++++++++++++++++ .../src/common/commands.ts | 10 +++ 2 files changed, 76 insertions(+) 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 9b0ece4e9a..68ef9279af 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts @@ -1474,6 +1474,72 @@ describe('BaseSnapExecutor', () => { }); }); + it('supports onAssetsLookup export', async () => { + const CODE = ` + module.exports.onAssetsLookup = () => ({ assets: {} }); + `; + + const executor = new TestSnapExecutor(); + await executor.executeSnap(1, MOCK_SNAP_ID, CODE, []); + + expect(await executor.readCommand()).toStrictEqual({ + jsonrpc: '2.0', + id: 1, + result: 'OK', + }); + + await executor.writeCommand({ + jsonrpc: '2.0', + id: 2, + method: 'snapRpc', + params: [ + MOCK_SNAP_ID, + HandlerType.OnAssetsLookup, + MOCK_ORIGIN, + { jsonrpc: '2.0', method: '', params: { assets: [] } }, + ], + }); + + expect(await executor.readCommand()).toStrictEqual({ + id: 2, + jsonrpc: '2.0', + result: { assets: {} }, + }); + }); + + it('supports onAssetsConversion export', async () => { + const CODE = ` + module.exports.onAssetsConversion = () => ({ conversionRates: {} }); + `; + + const executor = new TestSnapExecutor(); + await executor.executeSnap(1, MOCK_SNAP_ID, CODE, []); + + expect(await executor.readCommand()).toStrictEqual({ + jsonrpc: '2.0', + id: 1, + result: 'OK', + }); + + await executor.writeCommand({ + jsonrpc: '2.0', + id: 2, + method: 'snapRpc', + params: [ + MOCK_SNAP_ID, + HandlerType.OnAssetsConversion, + MOCK_ORIGIN, + { jsonrpc: '2.0', method: '', params: { conversions: [] } }, + ], + }); + + expect(await executor.readCommand()).toStrictEqual({ + id: 2, + jsonrpc: '2.0', + result: { conversionRates: {} }, + }); + }); + it('supports onSignature export', async () => { const CODE = ` module.exports.onSignature = ({ signature, signatureOrigin }) => diff --git a/packages/snaps-execution-environments/src/common/commands.ts b/packages/snaps-execution-environments/src/common/commands.ts index b03a32d23d..0a4ab76843 100644 --- a/packages/snaps-execution-environments/src/common/commands.ts +++ b/packages/snaps-execution-environments/src/common/commands.ts @@ -56,6 +56,16 @@ export function getHandlerArguments( const { signature, signatureOrigin } = request.params; return { signature, signatureOrigin }; } + case HandlerType.OnAssetsLookup: { + // TODO: Assert once we have structs + const { assets } = request.params; + return { assets }; + } + case HandlerType.OnAssetsConversion: { + // TODO: Assert once we have structs + const { conversions } = request.params; + return { conversions }; + } case HandlerType.OnNameLookup: { assertIsOnNameLookupRequestArguments(request.params); From bf6737aeaadd5113b4ce80d78e053b3e4f476ddd Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 21 Jan 2025 18:41:34 +0100 Subject: [PATCH 03/17] Add basic structs/types --- .../src/types/handlers/assets-conversion.ts | 15 ++++++- .../src/types/handlers/assets-lookup.ts | 43 ++++++++++++++++++- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/packages/snaps-sdk/src/types/handlers/assets-conversion.ts b/packages/snaps-sdk/src/types/handlers/assets-conversion.ts index a98d5f193a..742c0b7fe5 100644 --- a/packages/snaps-sdk/src/types/handlers/assets-conversion.ts +++ b/packages/snaps-sdk/src/types/handlers/assets-conversion.ts @@ -1,5 +1,15 @@ +import type { Infer } from '@metamask/superstruct'; +import { number, object, string } from '@metamask/superstruct'; import type { CaipAssetType } from '@metamask/utils'; +export const AssetConversionStruct = object({ + rate: string(), + conversionTime: number(), + expirationTime: number(), +}); + +export type AssetConversion = Infer; + export type OnAssetsConversionArguments = { conversions: { from: CaipAssetType; to: CaipAssetType }[]; }; @@ -20,5 +30,8 @@ export type OnAssetsConversionHandler = ( * @property conversionRates - A nested object with two CAIP-19 keys that contains a conversion rate between the two keys. */ export type OnAssetsConversionResponse = { - conversionRates: Record>; + conversionRates: Record< + CaipAssetType, + Record + >; }; diff --git a/packages/snaps-sdk/src/types/handlers/assets-lookup.ts b/packages/snaps-sdk/src/types/handlers/assets-lookup.ts index f3bc61839d..61cbd4fe08 100644 --- a/packages/snaps-sdk/src/types/handlers/assets-lookup.ts +++ b/packages/snaps-sdk/src/types/handlers/assets-lookup.ts @@ -1,4 +1,43 @@ -import type { CaipAssetType } from '@metamask/utils'; +import type { Infer } from '@metamask/superstruct'; +import { + array, + boolean, + literal, + number, + object, + refine, + string, +} from '@metamask/superstruct'; +import { assert, type CaipAssetType } from '@metamask/utils'; + +export const FungibleAssetUnitStruct = object({ + name: string(), + symbol: string(), + decimals: number(), +}); + +export type FungibleAssetUnit = Infer; + +export const AssetIconUrlStruct = refine(string(), 'Asset URL', (value) => { + try { + const url = new URL(value); + assert(url.protocol === 'https:'); + return true; + } catch { + return 'Invalid URL'; + } +}); + +export const FungibleAssetMetadataStruct = object({ + name: string(), + symbol: string(), + native: boolean(), + fungible: literal(true), + iconUrl: AssetIconUrlStruct, + units: array(FungibleAssetUnitStruct), +}); + +export type FungibleAssetMetadata = Infer; export type OnAssetsLookupArguments = { assets: CaipAssetType[]; @@ -20,5 +59,5 @@ export type OnAssetsLookupHandler = ( * @property assets - An object containing a mapping between the CAIP-19 key and a metadata object. */ export type OnAssetsLookupResponse = { - assets: Record; + assets: Record; }; From 80f5142f3f85b02d0586290c8a1f2e862500dfba Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 12:03:00 +0100 Subject: [PATCH 04/17] Add struct tests --- .../types/handlers/assets-conversion.test.ts | 24 ++++++ .../src/types/handlers/assets-conversion.ts | 4 +- .../src/types/handlers/assets-lookup.test.ts | 83 +++++++++++++++++++ .../src/types/handlers/assets-lookup.ts | 11 ++- 4 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 packages/snaps-sdk/src/types/handlers/assets-conversion.test.ts create mode 100644 packages/snaps-sdk/src/types/handlers/assets-lookup.test.ts diff --git a/packages/snaps-sdk/src/types/handlers/assets-conversion.test.ts b/packages/snaps-sdk/src/types/handlers/assets-conversion.test.ts new file mode 100644 index 0000000000..81eb3e7594 --- /dev/null +++ b/packages/snaps-sdk/src/types/handlers/assets-conversion.test.ts @@ -0,0 +1,24 @@ +import { is } from '@metamask/superstruct'; + +import { AssetConversionStruct } from './assets-conversion'; + +describe('AssetConversionStruct', () => { + it.each([ + { rate: '1.23', conversionTime: 1737542312, expirationTime: 1737542312 }, + { rate: '1.23', conversionTime: 1737542312 }, + ])('validates an object', (value) => { + expect(is(value, AssetConversionStruct)).toBe(true); + }); + + it.each([ + 'foo', + 42, + null, + undefined, + {}, + [], + { rate: 123, conversionTime: 123 }, + ])('does not validate "%p"', (value) => { + expect(is(value, AssetConversionStruct)).toBe(false); + }); +}); diff --git a/packages/snaps-sdk/src/types/handlers/assets-conversion.ts b/packages/snaps-sdk/src/types/handlers/assets-conversion.ts index 742c0b7fe5..f4509bc4e2 100644 --- a/packages/snaps-sdk/src/types/handlers/assets-conversion.ts +++ b/packages/snaps-sdk/src/types/handlers/assets-conversion.ts @@ -1,11 +1,11 @@ import type { Infer } from '@metamask/superstruct'; -import { number, object, string } from '@metamask/superstruct'; +import { number, object, string, optional } from '@metamask/superstruct'; import type { CaipAssetType } from '@metamask/utils'; export const AssetConversionStruct = object({ rate: string(), conversionTime: number(), - expirationTime: number(), + expirationTime: optional(number()), }); export type AssetConversion = Infer; diff --git a/packages/snaps-sdk/src/types/handlers/assets-lookup.test.ts b/packages/snaps-sdk/src/types/handlers/assets-lookup.test.ts new file mode 100644 index 0000000000..b1dec9559c --- /dev/null +++ b/packages/snaps-sdk/src/types/handlers/assets-lookup.test.ts @@ -0,0 +1,83 @@ +import { is } from '@metamask/superstruct'; + +import { FungibleAssetMetadataStruct } from './assets-lookup'; + +const BTC_ICON_BASE64 = + 'PHN2ZyB3aWR0aD0iNDAiIGhlaWdodD0iNDAiIHZpZXdCb3g9IjAgMCA0MCA0MCIgZmlsbD0ibm9uZSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj4KPGNpcmNsZSBjeD0iMjAiIGN5PSIyMC4wMDAyIiByPSIxOC44ODg5IiBmaWxsPSJ1cmwoI3BhaW50MF9saW5lYXJfNjlfODQxKSIvPgo8cGF0aCBmaWxsLXJ1bGU9ImV2ZW5vZGQiIGNsaXAtcnVsZT0iZXZlbm9kZCIgZD0iTTI0LjgxNTIgMTIuMTcxNkMyNy40MzQyIDEzLjEzMjUgMjkuMzIwNiAxNC41NTYxIDI4Ljg4ODIgMTcuMjg3OUMyOC41NjA5IDE5LjI3NjUgMjcuNTE4OCAyMC4yNDg1IDI2LjExMDYgMjAuNTkxNUMyNy45ODQ1IDIxLjY0MDQgMjguODg2NSAyMy4yMzI1IDI3LjkyMTYgMjYuMDI1MkMyNi43MjE3IDI5LjUyNzggMjQuMDM1OSAyOS44NDU1IDIwLjQ3ODQgMjkuMTY3TDE5LjU2MjUgMzIuODYzNkwxNy40OTU1IDMyLjM1MTNMMTguNDExMyAyOC42NTQ4QzE4LjE4NjQgMjguNTk0OSAxNy45NDk0IDI4LjUzOTcgMTcuNzA2MyAyOC40ODMxQzE3LjM4MTUgMjguNDA3NSAxNy4wNDU4IDI4LjMyOTMgMTYuNzEzNiAyOC4yMzM5TDE1Ljc5NzcgMzEuOTMwN0wxMy43MzQ1IDMxLjQxOTNMMTQuNjUwMyAyNy43MjI2TDEwLjU0MDMgMjYuNjAzMkwxMS41NjE5IDIzLjk4OTRDMTEuNTYxOSAyMy45ODk0IDEzLjExMzIgMjQuNDE2MSAxMy4wODkxIDI0LjM4OTNDMTMuNjY0NSAyNC41MjkyIDEzLjk0NTkgMjQuMTI3MyAxNC4wNjExIDIzLjg0NThMMTUuNTI3OCAxNy45MTk3TDE2LjU5NTEgMTMuNzA3N0MxNi42NDEzIDEzLjI1MjQgMTYuNDk4NyAxMi42NTY4IDE1LjY1OCAxMi40MzAyQzE1LjcxNTIgMTIuMzk2NiAxNC4xNDQ1IDEyLjA1NTEgMTQuMTQ0NSAxMi4wNTUxTDE0Ljc1NjYgOS41Nzc5N0wxOC45OTI2IDEwLjYyNzhMMTkuODg5NyA3LjAwNjg0TDIyLjAyMzcgNy41MzU3M0wyMS4xMjY2IDExLjE1NjdDMjEuNTQxNSAxMS4yNDY5IDIxLjk0NyAxMS4zNTE4IDIyLjM1NjggMTEuNDU3OEwyMi4zNTcgMTEuNDU3OEMyMi40OTE1IDExLjQ5MjYgMjIuNjI2NSAxMS41Mjc1IDIyLjc2MjQgMTEuNTYyMUwyMy42NTk1IDcuOTQxMTJMMjUuNzM1OSA4LjQ1NTcxTDI0LjgxNTIgMTIuMTcxNlpNMTkuMTUyNSAxNy45OTRDMTkuMTg0OCAxOC4wMDM2IDE5LjIxOTQgMTguMDE0IDE5LjI1NjEgMTguMDI1QzIwLjQ5NyAxOC4zOTggMjQuMTc2NiAxOS41MDM3IDI0Ljc5NjQgMTcuMDQxN0MyNS4zNzM1IDE0LjcwMTQgMjIuMTg1NyAxMy45ODY2IDIwLjcwNDUgMTMuNjU0NEMyMC41Mjk2IDEzLjYxNTIgMjAuMzc4NCAxMy41ODEzIDIwLjI2MDEgMTMuNTUwN0wxOS4xNTI1IDE3Ljk5NFpNMTcuNTE5NiAyNS4yOTM5QzE3LjQ1NDQgMjUuMjc0NCAxNy4zOTQzIDI1LjI1NjcgMTcuMzM5OCAyNS4yNDA2TDE4LjQ0NzQgMjAuNzk3NEMxOC41NzgzIDIwLjgzMTQgMTguNzQzOCAyMC44NzAzIDE4LjkzNTIgMjAuOTE1MkMyMC42ODEzIDIxLjMyNTUgMjQuNTgxMyAyMi4yNDIgMjMuOTc1MSAyNC41OTU0QzIzLjM4NjggMjcuMDM5IDE5LjA0ODQgMjUuNzQ4NyAxNy41MTk2IDI1LjI5MzlaIiBmaWxsPSJ3aGl0ZSIvPgo8ZGVmcz4KPGxpbmVhckdyYWRpZW50IGlkPSJwYWludDBfbGluZWFyXzY5Xzg0MSIgeDE9IjIwIiB5MT0iMS4xMTEzMyIgeDI9IjIwIiB5Mj0iMzguODg5MSIgZ3JhZGllbnRVbml0cz0idXNlclNwYWNlT25Vc2UiPgo8c3RvcCBzdG9wLWNvbG9yPSIjRkZCNjBBIi8+CjxzdG9wIG9mZnNldD0iMSIgc3RvcC1jb2xvcj0iI0Y1ODMwMCIvPgo8L2xpbmVhckdyYWRpZW50Pgo8L2RlZnM+Cjwvc3ZnPgo='; + +describe('FungibleAssetMetadataStruct', () => { + it.each([ + { + name: 'Bitcoin', + symbol: 'BTC', + fungible: true, + iconUrl: `data:image/svg+xml;base64,${BTC_ICON_BASE64}`, + units: [ + { + name: 'Bitcoin', + symbol: 'BTC', + decimals: 8, + }, + ], + }, + { + name: 'Solana', + symbol: 'SOL', + fungible: true, + iconUrl: 'https://metamask.io/sol.svg', + units: [ + { + name: 'Solana', + symbol: 'SOL', + decimals: 9, + }, + ], + }, + ])('validates an object', (value) => { + expect(is(value, FungibleAssetMetadataStruct)).toBe(true); + }); + + it.each([ + 'foo', + 42, + null, + undefined, + {}, + [], + { + name: 'Bitcoin', + symbol: 'BTC', + fungible: true, + iconUrl: 'https://metamask.io/btc.svg', + units: [], + }, + { + name: 'Bitcoin', + symbol: 'BTC', + fungible: true, + iconUrl: 'http://metamask.io/btc.svg', + units: [ + { + name: 'Bitcoin', + symbol: 'BTC', + decimals: 8, + }, + ], + }, + { + name: 'Bitcoin', + symbol: 'BTC', + fungible: true, + iconUrl: 'data:image/png;base64,', + units: [ + { + name: 'Bitcoin', + symbol: 'BTC', + decimals: 8, + }, + ], + }, + ])('does not validate "%p"', (value) => { + expect(is(value, FungibleAssetMetadataStruct)).toBe(false); + }); +}); diff --git a/packages/snaps-sdk/src/types/handlers/assets-lookup.ts b/packages/snaps-sdk/src/types/handlers/assets-lookup.ts index 61cbd4fe08..d576d0b618 100644 --- a/packages/snaps-sdk/src/types/handlers/assets-lookup.ts +++ b/packages/snaps-sdk/src/types/handlers/assets-lookup.ts @@ -1,7 +1,7 @@ import type { Infer } from '@metamask/superstruct'; import { array, - boolean, + size, literal, number, object, @@ -21,7 +21,11 @@ export type FungibleAssetUnit = Infer; export const AssetIconUrlStruct = refine(string(), 'Asset URL', (value) => { try { const url = new URL(value); - assert(url.protocol === 'https:'); + // For now, we require asset URLs to either be base64 SVGs or remote HTTPS URLs + assert( + url.protocol === 'https:' || + value.startsWith('data:image/svg+xml;base64,'), + ); return true; } catch { return 'Invalid URL'; @@ -31,10 +35,9 @@ export const AssetIconUrlStruct = refine(string(), 'Asset URL', (value) => { export const FungibleAssetMetadataStruct = object({ name: string(), symbol: string(), - native: boolean(), fungible: literal(true), iconUrl: AssetIconUrlStruct, - units: array(FungibleAssetUnitStruct), + units: size(array(FungibleAssetUnitStruct), 1, Infinity), }); export type FungibleAssetMetadata = Infer; From 111f2009674a8c9c2ae42921f5f1da4350c93fcf Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 12:10:45 +0100 Subject: [PATCH 05/17] Assert response types --- .../src/snaps/SnapController.ts | 13 ++++++++++++- .../src/types/handlers/assets-conversion.ts | 17 +++++++++++++++-- .../src/types/handlers/assets-lookup.ts | 11 ++++++++++- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 655c1bcdec..20a6564f22 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -49,7 +49,12 @@ import type { ComponentOrElement, ContentType, } from '@metamask/snaps-sdk'; -import { AuxiliaryFileEncoding, getErrorMessage } from '@metamask/snaps-sdk'; +import { + AuxiliaryFileEncoding, + getErrorMessage, + OnAssetsConversionResponseStruct, + OnAssetsLookupResponseStruct, +} from '@metamask/snaps-sdk'; import type { FetchedSnapFiles, InitialConnections, @@ -3654,6 +3659,12 @@ export class SnapController extends BaseController< case HandlerType.OnNameLookup: assertStruct(result, OnNameLookupResponseStruct); break; + case HandlerType.OnAssetsLookup: + assertStruct(result, OnAssetsLookupResponseStruct); + break; + case HandlerType.OnAssetsConversion: + assertStruct(result, OnAssetsConversionResponseStruct); + break; default: break; } diff --git a/packages/snaps-sdk/src/types/handlers/assets-conversion.ts b/packages/snaps-sdk/src/types/handlers/assets-conversion.ts index f4509bc4e2..f5d33de5de 100644 --- a/packages/snaps-sdk/src/types/handlers/assets-conversion.ts +++ b/packages/snaps-sdk/src/types/handlers/assets-conversion.ts @@ -1,6 +1,12 @@ import type { Infer } from '@metamask/superstruct'; -import { number, object, string, optional } from '@metamask/superstruct'; -import type { CaipAssetType } from '@metamask/utils'; +import { + number, + object, + string, + optional, + record, +} from '@metamask/superstruct'; +import { CaipAssetTypeStruct, type CaipAssetType } from '@metamask/utils'; export const AssetConversionStruct = object({ rate: string(), @@ -8,6 +14,13 @@ export const AssetConversionStruct = object({ expirationTime: optional(number()), }); +export const OnAssetsConversionResponseStruct = object({ + assets: record( + CaipAssetTypeStruct, + record(CaipAssetTypeStruct, AssetConversionStruct), + ), +}); + export type AssetConversion = Infer; export type OnAssetsConversionArguments = { diff --git a/packages/snaps-sdk/src/types/handlers/assets-lookup.ts b/packages/snaps-sdk/src/types/handlers/assets-lookup.ts index d576d0b618..62aca05f4b 100644 --- a/packages/snaps-sdk/src/types/handlers/assets-lookup.ts +++ b/packages/snaps-sdk/src/types/handlers/assets-lookup.ts @@ -7,8 +7,13 @@ import { object, refine, string, + record, } from '@metamask/superstruct'; -import { assert, type CaipAssetType } from '@metamask/utils'; +import { + assert, + CaipAssetTypeStruct, + type CaipAssetType, +} from '@metamask/utils'; export const FungibleAssetUnitStruct = object({ name: string(), @@ -40,6 +45,10 @@ export const FungibleAssetMetadataStruct = object({ units: size(array(FungibleAssetUnitStruct), 1, Infinity), }); +export const OnAssetsLookupResponseStruct = object({ + assets: record(CaipAssetTypeStruct, FungibleAssetMetadataStruct), +}); + export type FungibleAssetMetadata = Infer; export type OnAssetsLookupArguments = { From 87889f57ac5dd9574a7170e1a1f960894e953150 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 12:20:26 +0100 Subject: [PATCH 06/17] Add parameter validation --- .../common/BaseSnapExecutor.test.browser.ts | 21 +++++- .../src/common/commands.ts | 6 +- .../src/common/validation.ts | 65 +++++++++++++++++++ 3 files changed, 88 insertions(+), 4 deletions(-) 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 68ef9279af..396549adf6 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts @@ -1496,7 +1496,13 @@ describe('BaseSnapExecutor', () => { MOCK_SNAP_ID, HandlerType.OnAssetsLookup, MOCK_ORIGIN, - { jsonrpc: '2.0', method: '', params: { assets: [] } }, + { + jsonrpc: '2.0', + method: '', + params: { + assets: ['bip122:000000000019d6689c085ae165831e93/slip44:0'], + }, + }, ], }); @@ -1529,7 +1535,18 @@ describe('BaseSnapExecutor', () => { MOCK_SNAP_ID, HandlerType.OnAssetsConversion, MOCK_ORIGIN, - { jsonrpc: '2.0', method: '', params: { conversions: [] } }, + { + jsonrpc: '2.0', + method: '', + params: { + conversions: [ + { + from: 'bip122:000000000019d6689c085ae165831e93/slip44:0', + to: 'eip155:1/slip44:60', + }, + ], + }, + }, ], }); diff --git a/packages/snaps-execution-environments/src/common/commands.ts b/packages/snaps-execution-environments/src/common/commands.ts index 0a4ab76843..8dbfac295f 100644 --- a/packages/snaps-execution-environments/src/common/commands.ts +++ b/packages/snaps-execution-environments/src/common/commands.ts @@ -15,6 +15,8 @@ import { assertIsOnSignatureRequestArguments, assertIsOnNameLookupRequestArguments, assertIsOnUserInputRequestArguments, + assertIsOnAssetsLookupRequestArguments, + assertIsOnAssetsConversionRequestArguments, } from './validation'; export type CommandMethodsMapping = { @@ -57,12 +59,12 @@ export function getHandlerArguments( return { signature, signatureOrigin }; } case HandlerType.OnAssetsLookup: { - // TODO: Assert once we have structs + assertIsOnAssetsLookupRequestArguments(request.params); const { assets } = request.params; return { assets }; } case HandlerType.OnAssetsConversion: { - // TODO: Assert once we have structs + assertIsOnAssetsConversionRequestArguments(request.params); const { conversions } = request.params; return { conversions }; } diff --git a/packages/snaps-execution-environments/src/common/validation.ts b/packages/snaps-execution-environments/src/common/validation.ts index 166c9df8c7..22a5e68033 100644 --- a/packages/snaps-execution-environments/src/common/validation.ts +++ b/packages/snaps-execution-environments/src/common/validation.ts @@ -16,6 +16,7 @@ import { object, optional, record, + size, string, tuple, union, @@ -23,6 +24,7 @@ import { import type { Json, JsonRpcSuccess } from '@metamask/utils'; import { assertStruct, + CaipAssetTypeStruct, JsonRpcIdStruct, JsonRpcParamsStruct, JsonRpcSuccessStruct, @@ -216,6 +218,69 @@ export function assertIsOnNameLookupRequestArguments( ); } +export const OnAssetsLookupRequestArgumentsStruct = object({ + assets: size(array(CaipAssetTypeStruct), 1, Infinity), +}); + +export type OnAssetsLookupRequestArguments = Infer< + typeof OnAssetsLookupRequestArgumentsStruct +>; + +/** + * Asserts that the given value is a valid {@link OnAssetsLookupRequestArguments} + * object. + * + * @param value - The value to validate. + * @throws If the value is not a valid {@link OnAssetsLookupRequestArguments} + * object. + */ +export function assertIsOnAssetsLookupRequestArguments( + value: unknown, +): asserts value is OnAssetsLookupRequestArguments { + assertStruct( + value, + OnAssetsLookupRequestArgumentsStruct, + 'Invalid request params', + rpcErrors.invalidParams, + ); +} + +export const OnAssetsConversionRequestArgumentsStruct = object({ + conversions: size( + array( + object({ + from: CaipAssetTypeStruct, + to: CaipAssetTypeStruct, + }), + ), + 1, + Infinity, + ), +}); + +export type OnAssetsConversionRequestArguments = Infer< + typeof OnAssetsConversionRequestArgumentsStruct +>; + +/** + * Asserts that the given value is a valid {@link OnAssetsConversionRequestArguments} + * object. + * + * @param value - The value to validate. + * @throws If the value is not a valid {@link OnNameLookupRequestArguments} + * object. + */ +export function assertIsOnAssetsConversionRequestArguments( + value: unknown, +): asserts value is OnAssetsConversionRequestArguments { + assertStruct( + value, + OnAssetsConversionRequestArgumentsStruct, + 'Invalid request params', + rpcErrors.invalidParams, + ); +} + export const OnUserInputArgumentsStruct = object({ id: string(), event: UserInputEventStruct, From 027a2becb88f68475e762fb8a17a212d85461f65 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 12:50:35 +0100 Subject: [PATCH 07/17] Add result transformers --- .../src/snaps/SnapController.ts | 65 ++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 20a6564f22..46769d2c64 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -41,6 +41,7 @@ import { getRpcCaveatOrigins, processSnapPermissions, getEncryptionEntropy, + getChainIdsCaveat, } from '@metamask/snaps-rpc-methods'; import type { RequestSnapsParams, @@ -48,6 +49,11 @@ import type { SnapId, ComponentOrElement, ContentType, + OnAssetsLookupResponse, + FungibleAssetMetadata, + OnAssetsConversionResponse, + OnAssetsConversionArguments, + AssetConversion, } from '@metamask/snaps-sdk'; import { AuxiliaryFileEncoding, @@ -97,7 +103,12 @@ import { MAX_FILE_SIZE, OnSettingsPageResponseStruct, } from '@metamask/snaps-utils'; -import type { Json, NonEmptyArray, SemVerRange } from '@metamask/utils'; +import type { + Json, + NonEmptyArray, + SemVerRange, + CaipAssetType, +} from '@metamask/utils'; import { assert, assertIsJsonRpcRequest, @@ -3513,6 +3524,7 @@ export class SnapController extends BaseController< const transformedResult = await this.#transformSnapRpcRequestResult( snapId, handlerType, + request, result, ); @@ -3574,12 +3586,14 @@ export class SnapController extends BaseController< * * @param snapId - The snap ID of the snap that produced the result. * @param handlerType - The handler type that produced the result. + * @param request - The request that returned the result. * @param result - The result. * @returns The transformed result if applicable, otherwise the original result. */ async #transformSnapRpcRequestResult( snapId: SnapId, handlerType: HandlerType, + request: Record, result: unknown, ) { switch (handlerType) { @@ -3602,6 +3616,55 @@ export class SnapController extends BaseController< } return result; } + case HandlerType.OnAssetsLookup: { + // We know the permissions are guaranteed to be set here. + const permissions = this.messagingSystem.call( + 'PermissionController:getPermissions', + snapId, + ) as SubjectPermissions; + + const permission = permissions[SnapEndowments.Assets]; + const scopes = getChainIdsCaveat(permission) as string[]; + + // We can cast since the result has already been validated. + const { assets } = result as OnAssetsLookupResponse; + const filteredAssets = Object.keys(assets).reduce< + Record + >((accumulator, assetType) => { + const castAssetType = assetType as CaipAssetType; + const isValid = scopes.some((scope) => + castAssetType.startsWith(scope), + ); + // Filter out assets for scopes the Snap hasn't registered for. + if (isValid) { + accumulator[castAssetType] = assets[castAssetType]; + } + return accumulator; + }, {}); + return { assets: filteredAssets }; + } + case HandlerType.OnAssetsConversion: { + // We can cast since the request and result have already been validated. + const { params: requestedParams } = request as { + params: OnAssetsConversionArguments; + }; + const { conversions: requestedConversions } = requestedParams; + + const { conversionRates } = result as OnAssetsConversionResponse; + + const filteredConversionRates = requestedConversions.reduce< + Record> + >((accumulator, conversion) => { + const rate = conversionRates[conversion.from]?.[conversion.to]; + // Only include rates that were actually requested. + if (rate) { + accumulator[conversion.from] ??= {}; + accumulator[conversion.from][conversion.to] = rate; + } + return accumulator; + }, {}); + return { conversionRates: filteredConversionRates }; + } default: return result; } From cffea47cad60a92abc2dae2f5e497b686614aaca Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 13:34:05 +0100 Subject: [PATCH 08/17] Add additional tests, update manifest and LM policy --- .../browserify-plugin/snap.manifest.json | 2 +- .../packages/browserify/snap.manifest.json | 2 +- packages/snaps-controllers/coverage.json | 8 +- .../src/snaps/SnapController.test.tsx | 451 +++++++++++++++++- .../src/snaps/SnapController.ts | 15 +- .../lavamoat/browserify/iframe/policy.json | 1 + .../browserify/node-process/policy.json | 1 + .../browserify/node-thread/policy.json | 1 + .../lavamoat/browserify/webview/policy.json | 1 + .../browserify/worker-executor/policy.json | 1 + .../browserify/worker-pool/policy.json | 1 + packages/snaps-rpc-methods/jest.config.js | 8 +- .../src/endowments/assets.test.ts | 19 +- .../src/endowments/assets.ts | 6 +- .../snaps-rpc-methods/src/permissions.test.ts | 12 + .../src/types/handlers/assets-conversion.ts | 2 +- .../src/methods/specifications.test.ts | 12 + packages/snaps-utils/coverage.json | 4 +- .../snaps-utils/src/manifest/validation.ts | 6 + 19 files changed, 528 insertions(+), 25 deletions(-) diff --git a/packages/examples/packages/browserify-plugin/snap.manifest.json b/packages/examples/packages/browserify-plugin/snap.manifest.json index 37e782f977..161d2a1b1a 100644 --- a/packages/examples/packages/browserify-plugin/snap.manifest.json +++ b/packages/examples/packages/browserify-plugin/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snaps.git" }, "source": { - "shasum": "B0senywfM+w5lQ+iMvK+bVcKJ6VeLDj7HiUVYR5Cuag=", + "shasum": "Q3egujSHQAQz2qqugqHiX/JverP9N9X1uazmZLaWhBQ=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/examples/packages/browserify/snap.manifest.json b/packages/examples/packages/browserify/snap.manifest.json index 96083306bf..b0779355a1 100644 --- a/packages/examples/packages/browserify/snap.manifest.json +++ b/packages/examples/packages/browserify/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snaps.git" }, "source": { - "shasum": "PS0U7SHYXWpFhO8QMtArHKU1rFzMkwtTLFlc3/g1HQ4=", + "shasum": "8deQsIjPZ7MNy/EY6XgyxslyqGKGMOGa0oOGgkow3SM=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snaps-controllers/coverage.json b/packages/snaps-controllers/coverage.json index 7a462547c5..fffcd4a239 100644 --- a/packages/snaps-controllers/coverage.json +++ b/packages/snaps-controllers/coverage.json @@ -1,6 +1,6 @@ { - "branches": 92.96, - "functions": 96.56, - "lines": 98.05, - "statements": 97.77 + "branches": 93.06, + "functions": 96.59, + "lines": 98.08, + "statements": 97.8 } diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index aaa463bfed..f884f73fdb 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -3898,6 +3898,455 @@ describe('SnapController', () => { snapController.destroy(); }); + it('throws if OnAssetsLookup handler returns an invalid response', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, + }), + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, + }, + }), + ); + + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); + + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + assets: { foo: {} }, + }), + ); + + await expect( + snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsLookup, + request: { + jsonrpc: '2.0', + method: ' ', + params: {}, + id: 1, + }, + }), + ).rejects.toThrow( + `Assertion failed: At path: assets.foo -- Expected a string matching`, + ); + + snapController.destroy(); + }); + + it('filters out assets that are out of scope for OnAssetsLookup', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, + }), + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, + }, + }), + ); + + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); + + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + assets: { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { + name: 'Solana', + symbol: 'SOL', + fungible: true, + iconUrl: 'https://metamask.io/sol.svg', + units: [ + { + name: 'Solana', + symbol: 'SOL', + decimals: 9, + }, + ], + }, + }, + }), + ); + + expect( + await snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsLookup, + request: { + jsonrpc: '2.0', + method: ' ', + params: { + assets: ['bip122:000000000019d6689c085ae165831e93/slip44:0'], + }, + id: 1, + }, + }), + ).toStrictEqual({ assets: {} }); + + snapController.destroy(); + }); + + it('returns the value when OnAssetsLookup returns a valid response', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, + }), + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, + }, + }), + ); + + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); + + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + assets: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + name: 'Bitcoin', + symbol: 'BTC', + fungible: true, + iconUrl: 'https://metamask.io/btc.svg', + units: [ + { + name: 'Bitcoin', + symbol: 'BTC', + decimals: 8, + }, + ], + }, + }, + }), + ); + + expect( + await snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsLookup, + request: { + jsonrpc: '2.0', + method: ' ', + params: { + assets: ['bip122:000000000019d6689c085ae165831e93/slip44:0'], + }, + id: 1, + }, + }), + ).toStrictEqual({ + assets: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + name: 'Bitcoin', + symbol: 'BTC', + fungible: true, + iconUrl: 'https://metamask.io/btc.svg', + units: [ + { + name: 'Bitcoin', + symbol: 'BTC', + decimals: 8, + }, + ], + }, + }, + }); + + snapController.destroy(); + }); + + it('throws if OnAssetsConversion handler returns an invalid response', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, + }), + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, + }, + }), + ); + + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); + + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + conversionRates: { foo: {} }, + }), + ); + + await expect( + snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsConversion, + request: { + jsonrpc: '2.0', + method: ' ', + params: {}, + id: 1, + }, + }), + ).rejects.toThrow( + `Assertion failed: At path: conversionRates.foo -- Expected a string matching`, + ); + + snapController.destroy(); + }); + + it('filters out assets that are out of scope for OnAssetsConversion', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, + }), + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, + }, + }), + ); + + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); + + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + conversionRates: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + 'eip155:1/slip44:60': { + rate: '33', + conversionTime: 1737548790, + }, + }, + }, + }), + ); + + expect( + await snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsConversion, + request: { + jsonrpc: '2.0', + method: ' ', + params: { + conversions: [ + { + from: 'bip122:000000000019d6689c085ae165831e93/slip44:0', + to: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + }, + ], + }, + id: 1, + }, + }), + ).toStrictEqual({ conversionRates: {} }); + + snapController.destroy(); + }); + + it('returns the value when OnAssetsConversion returns a valid response', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, + }), + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, + }, + }), + ); + + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); + + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + conversionRates: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { + rate: '400', + conversionTime: 1737548790, + }, + }, + }, + }), + ); + + expect( + await snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsConversion, + request: { + jsonrpc: '2.0', + method: ' ', + params: { + conversions: [ + { + from: 'bip122:000000000019d6689c085ae165831e93/slip44:0', + to: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + }, + ], + }, + id: 1, + }, + }), + ).toStrictEqual({ + conversionRates: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { + rate: '400', + conversionTime: 1737548790, + }, + }, + }, + }); + + snapController.destroy(); + }); + describe('getRpcRequestHandler', () => { it('handlers populate the "jsonrpc" property if missing', async () => { const rootMessenger = getControllerMessenger(); @@ -5655,7 +6104,7 @@ describe('SnapController', () => { [MOCK_SNAP_ID]: {}, }), ).rejects.toThrow( - 'A snap must request at least one of the following permissions: endowment:rpc, endowment:transaction-insight, endowment:cronjob, endowment:name-lookup, endowment:lifecycle-hooks, endowment:keyring, endowment:page-home, endowment:page-settings, endowment:signature-insight.', + 'A snap must request at least one of the following permissions: endowment:rpc, endowment:transaction-insight, endowment:cronjob, endowment:name-lookup, endowment:lifecycle-hooks, endowment:keyring, endowment:page-home, endowment:page-settings, endowment:signature-insight, endowment:assets.', ); controller.destroy(); diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 46769d2c64..eae3a11601 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -54,6 +54,7 @@ import type { OnAssetsConversionResponse, OnAssetsConversionArguments, AssetConversion, + OnAssetsLookupArguments, } from '@metamask/snaps-sdk'; import { AuxiliaryFileEncoding, @@ -3626,16 +3627,20 @@ export class SnapController extends BaseController< const permission = permissions[SnapEndowments.Assets]; const scopes = getChainIdsCaveat(permission) as string[]; - // We can cast since the result has already been validated. + // We can cast since the request and result have already been validated. + const { params: requestedParams } = request as { + params: OnAssetsLookupArguments; + }; + const { assets: requestedAssets } = requestedParams; const { assets } = result as OnAssetsLookupResponse; const filteredAssets = Object.keys(assets).reduce< Record >((accumulator, assetType) => { const castAssetType = assetType as CaipAssetType; - const isValid = scopes.some((scope) => - castAssetType.startsWith(scope), - ); - // Filter out assets for scopes the Snap hasn't registered for. + const isValid = + scopes.some((scope) => castAssetType.startsWith(scope)) && + requestedAssets.includes(castAssetType); + // Filter out unrequested assets and assets for scopes the Snap hasn't registered for. if (isValid) { accumulator[castAssetType] = assets[castAssetType]; } diff --git a/packages/snaps-execution-environments/lavamoat/browserify/iframe/policy.json b/packages/snaps-execution-environments/lavamoat/browserify/iframe/policy.json index 5f53e91c6f..e386897747 100644 --- a/packages/snaps-execution-environments/lavamoat/browserify/iframe/policy.json +++ b/packages/snaps-execution-environments/lavamoat/browserify/iframe/policy.json @@ -79,6 +79,7 @@ }, "@metamask/snaps-sdk": { "globals": { + "URL": true, "fetch": true }, "packages": { diff --git a/packages/snaps-execution-environments/lavamoat/browserify/node-process/policy.json b/packages/snaps-execution-environments/lavamoat/browserify/node-process/policy.json index 1c4b1c0425..57d9fbbc20 100644 --- a/packages/snaps-execution-environments/lavamoat/browserify/node-process/policy.json +++ b/packages/snaps-execution-environments/lavamoat/browserify/node-process/policy.json @@ -89,6 +89,7 @@ }, "@metamask/snaps-sdk": { "globals": { + "URL": true, "fetch": true }, "packages": { diff --git a/packages/snaps-execution-environments/lavamoat/browserify/node-thread/policy.json b/packages/snaps-execution-environments/lavamoat/browserify/node-thread/policy.json index 1c4b1c0425..57d9fbbc20 100644 --- a/packages/snaps-execution-environments/lavamoat/browserify/node-thread/policy.json +++ b/packages/snaps-execution-environments/lavamoat/browserify/node-thread/policy.json @@ -89,6 +89,7 @@ }, "@metamask/snaps-sdk": { "globals": { + "URL": true, "fetch": true }, "packages": { diff --git a/packages/snaps-execution-environments/lavamoat/browserify/webview/policy.json b/packages/snaps-execution-environments/lavamoat/browserify/webview/policy.json index 7417631724..df5877fd7f 100644 --- a/packages/snaps-execution-environments/lavamoat/browserify/webview/policy.json +++ b/packages/snaps-execution-environments/lavamoat/browserify/webview/policy.json @@ -24,6 +24,7 @@ }, "@metamask/snaps-sdk": { "globals": { + "URL": true, "fetch": true }, "packages": { diff --git a/packages/snaps-execution-environments/lavamoat/browserify/worker-executor/policy.json b/packages/snaps-execution-environments/lavamoat/browserify/worker-executor/policy.json index 5f53e91c6f..e386897747 100644 --- a/packages/snaps-execution-environments/lavamoat/browserify/worker-executor/policy.json +++ b/packages/snaps-execution-environments/lavamoat/browserify/worker-executor/policy.json @@ -79,6 +79,7 @@ }, "@metamask/snaps-sdk": { "globals": { + "URL": true, "fetch": true }, "packages": { diff --git a/packages/snaps-execution-environments/lavamoat/browserify/worker-pool/policy.json b/packages/snaps-execution-environments/lavamoat/browserify/worker-pool/policy.json index 7417631724..df5877fd7f 100644 --- a/packages/snaps-execution-environments/lavamoat/browserify/worker-pool/policy.json +++ b/packages/snaps-execution-environments/lavamoat/browserify/worker-pool/policy.json @@ -24,6 +24,7 @@ }, "@metamask/snaps-sdk": { "globals": { + "URL": true, "fetch": true }, "packages": { diff --git a/packages/snaps-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js index 0b93d167c8..d7bfe3cbd4 100644 --- a/packages/snaps-rpc-methods/jest.config.js +++ b/packages/snaps-rpc-methods/jest.config.js @@ -10,10 +10,10 @@ module.exports = deepmerge(baseConfig, { ], coverageThreshold: { global: { - branches: 94.89, - functions: 98.05, - lines: 98.67, - statements: 98.34, + branches: 94.93, + functions: 98.08, + lines: 98.69, + statements: 98.36, }, }, }); diff --git a/packages/snaps-rpc-methods/src/endowments/assets.test.ts b/packages/snaps-rpc-methods/src/endowments/assets.test.ts index a47645b13f..223ad031a4 100644 --- a/packages/snaps-rpc-methods/src/endowments/assets.test.ts +++ b/packages/snaps-rpc-methods/src/endowments/assets.test.ts @@ -1,7 +1,7 @@ import { PermissionType, SubjectType } from '@metamask/permission-controller'; import { SnapCaveatType } from '@metamask/snaps-utils'; -import { assetsEndowmentBuilder } from './assets'; +import { assetsEndowmentBuilder, getAssetsCaveatMapper } from './assets'; import { SnapEndowments } from './enum'; describe('endowment:assets', () => { @@ -20,4 +20,21 @@ describe('endowment:assets', () => { expect(specification.endowmentGetter()).toBeNull(); }); }); + + describe('getAssetsCaveatMapper', () => { + it('maps a value to a caveat', () => { + expect( + getAssetsCaveatMapper({ + scopes: ['bip122:000000000019d6689c085ae165831e93'], + }), + ).toStrictEqual({ + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + }); + }); + }); }); diff --git a/packages/snaps-rpc-methods/src/endowments/assets.ts b/packages/snaps-rpc-methods/src/endowments/assets.ts index 57b0eb0998..392912127e 100644 --- a/packages/snaps-rpc-methods/src/endowments/assets.ts +++ b/packages/snaps-rpc-methods/src/endowments/assets.ts @@ -61,11 +61,7 @@ export const assetsEndowmentBuilder = Object.freeze({ export function getAssetsCaveatMapper( value: Json, ): Pick { - if (!value || !isObject(value) || Object.keys(value).length === 0) { - return { caveats: null }; - } - - assert(value.scopes); + assert(isObject(value) && value.scopes); return { caveats: [ diff --git a/packages/snaps-rpc-methods/src/permissions.test.ts b/packages/snaps-rpc-methods/src/permissions.test.ts index de38781944..9b9486d2e4 100644 --- a/packages/snaps-rpc-methods/src/permissions.test.ts +++ b/packages/snaps-rpc-methods/src/permissions.test.ts @@ -8,6 +8,18 @@ describe('buildSnapEndowmentSpecifications', () => { const specifications = buildSnapEndowmentSpecifications([]); expect(specifications).toMatchInlineSnapshot(` { + "endowment:assets": { + "allowedCaveats": [ + "chainIds", + ], + "endowmentGetter": [Function], + "permissionType": "Endowment", + "subjectTypes": [ + "snap", + ], + "targetName": "endowment:assets", + "validator": [Function], + }, "endowment:cronjob": { "allowedCaveats": [ "snapCronjob", diff --git a/packages/snaps-sdk/src/types/handlers/assets-conversion.ts b/packages/snaps-sdk/src/types/handlers/assets-conversion.ts index f5d33de5de..7d5139057f 100644 --- a/packages/snaps-sdk/src/types/handlers/assets-conversion.ts +++ b/packages/snaps-sdk/src/types/handlers/assets-conversion.ts @@ -15,7 +15,7 @@ export const AssetConversionStruct = object({ }); export const OnAssetsConversionResponseStruct = object({ - assets: record( + conversionRates: record( CaipAssetTypeStruct, record(CaipAssetTypeStruct, AssetConversionStruct), ), diff --git a/packages/snaps-simulation/src/methods/specifications.test.ts b/packages/snaps-simulation/src/methods/specifications.test.ts index 2690485057..6b23e2789b 100644 --- a/packages/snaps-simulation/src/methods/specifications.test.ts +++ b/packages/snaps-simulation/src/methods/specifications.test.ts @@ -49,6 +49,18 @@ describe('getPermissionSpecifications', () => { }), ).toMatchInlineSnapshot(` { + "endowment:assets": { + "allowedCaveats": [ + "chainIds", + ], + "endowmentGetter": [Function], + "permissionType": "Endowment", + "subjectTypes": [ + "snap", + ], + "targetName": "endowment:assets", + "validator": [Function], + }, "endowment:cronjob": { "allowedCaveats": [ "snapCronjob", diff --git a/packages/snaps-utils/coverage.json b/packages/snaps-utils/coverage.json index a9248b7438..701516c7b8 100644 --- a/packages/snaps-utils/coverage.json +++ b/packages/snaps-utils/coverage.json @@ -1,6 +1,6 @@ { "branches": 99.74, - "functions": 98.93, + "functions": 98.94, "lines": 99.46, - "statements": 96.31 + "statements": 96.32 } diff --git a/packages/snaps-utils/src/manifest/validation.ts b/packages/snaps-utils/src/manifest/validation.ts index ad14afc916..becb3073b0 100644 --- a/packages/snaps-utils/src/manifest/validation.ts +++ b/packages/snaps-utils/src/manifest/validation.ts @@ -188,6 +188,12 @@ export const EmptyObjectStruct = object({}) as unknown as Struct< /* eslint-disable @typescript-eslint/naming-convention */ export const PermissionsStruct: Describe = type({ + 'endowment:assets': optional( + mergeStructs( + HandlerCaveatsStruct, + object({ scopes: size(array(ChainIdsStruct), 1, Infinity) }), + ), + ), 'endowment:cronjob': optional( mergeStructs( HandlerCaveatsStruct, From ef721fd2ab16894b6197186950a7466ac052be53 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 13:44:21 +0100 Subject: [PATCH 09/17] Update manifests and coverage again --- .../examples/packages/browserify-plugin/snap.manifest.json | 2 +- packages/examples/packages/browserify/snap.manifest.json | 2 +- packages/snaps-execution-environments/coverage.json | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/examples/packages/browserify-plugin/snap.manifest.json b/packages/examples/packages/browserify-plugin/snap.manifest.json index 161d2a1b1a..61d9fb2146 100644 --- a/packages/examples/packages/browserify-plugin/snap.manifest.json +++ b/packages/examples/packages/browserify-plugin/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snaps.git" }, "source": { - "shasum": "Q3egujSHQAQz2qqugqHiX/JverP9N9X1uazmZLaWhBQ=", + "shasum": "PU8/QaQOlO6/ShRIM+jofaiQFUAprfuUX9RV6G5xRJo=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/examples/packages/browserify/snap.manifest.json b/packages/examples/packages/browserify/snap.manifest.json index b0779355a1..4fc06d52a3 100644 --- a/packages/examples/packages/browserify/snap.manifest.json +++ b/packages/examples/packages/browserify/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/MetaMask/snaps.git" }, "source": { - "shasum": "8deQsIjPZ7MNy/EY6XgyxslyqGKGMOGa0oOGgkow3SM=", + "shasum": "5vUCvHpbE8BnQv9R8QorYcvyKPZk0s+Fuh/MFUZ7LH4=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index 926fe3031b..6e151d69bd 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { "branches": 80.68, - "functions": 89.33, - "lines": 90.68, - "statements": 90.08 + "functions": 89.15, + "lines": 90.28, + "statements": 89.45 } From aa396c77eb737ba66e11967e5905c36ab689d54e Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 13:53:53 +0100 Subject: [PATCH 10/17] Update coverage --- packages/snaps-execution-environments/coverage.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index 6e151d69bd..7a077f3293 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { "branches": 80.68, - "functions": 89.15, + "functions": 88.15, "lines": 90.28, "statements": 89.45 } From fdf7934518ffbe08357a0c8b15201787c2b1b592 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 14:03:42 +0100 Subject: [PATCH 11/17] Another coverage tweak --- packages/snaps-execution-environments/coverage.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index 7a077f3293..59b385b8e1 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,5 +1,5 @@ { - "branches": 80.68, + "branches": 80.95, "functions": 88.15, "lines": 90.28, "statements": 89.45 From 8a1f5d56aef2eae1f55e27e51469be96501a6848 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 14:25:24 +0100 Subject: [PATCH 12/17] Apply suggestions from code review Co-authored-by: Maarten Zuidhoorn --- .../src/snaps/SnapController.test.tsx | 12 ++++++------ .../src/common/BaseSnapExecutor.test.browser.ts | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index f884f73fdb..f20e2a229a 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -3898,7 +3898,7 @@ describe('SnapController', () => { snapController.destroy(); }); - it('throws if OnAssetsLookup handler returns an invalid response', async () => { + it('throws if `onAssetsLookup` handler returns an invalid response', async () => { const rootMessenger = getControllerMessenger(); const messenger = getSnapControllerMessenger(rootMessenger); const snapController = getSnapController( @@ -3960,7 +3960,7 @@ describe('SnapController', () => { snapController.destroy(); }); - it('filters out assets that are out of scope for OnAssetsLookup', async () => { + it('filters out assets that are out of scope for `onAssetsLookup`', async () => { const rootMessenger = getControllerMessenger(); const messenger = getSnapControllerMessenger(rootMessenger); const snapController = getSnapController( @@ -4036,7 +4036,7 @@ describe('SnapController', () => { snapController.destroy(); }); - it('returns the value when OnAssetsLookup returns a valid response', async () => { + it('returns the value when `onAssetsLookup` returns a valid response', async () => { const rootMessenger = getControllerMessenger(); const messenger = getSnapControllerMessenger(rootMessenger); const snapController = getSnapController( @@ -4128,7 +4128,7 @@ describe('SnapController', () => { snapController.destroy(); }); - it('throws if OnAssetsConversion handler returns an invalid response', async () => { + it('throws if `onAssetsConversion` handler returns an invalid response', async () => { const rootMessenger = getControllerMessenger(); const messenger = getSnapControllerMessenger(rootMessenger); const snapController = getSnapController( @@ -4190,7 +4190,7 @@ describe('SnapController', () => { snapController.destroy(); }); - it('filters out assets that are out of scope for OnAssetsConversion', async () => { + it('filters out assets that are out of scope for `onAssetsConversion`', async () => { const rootMessenger = getControllerMessenger(); const messenger = getSnapControllerMessenger(rootMessenger); const snapController = getSnapController( @@ -4264,7 +4264,7 @@ describe('SnapController', () => { snapController.destroy(); }); - it('returns the value when OnAssetsConversion returns a valid response', async () => { + it('returns the value when `onAssetsConversion` returns a valid response', async () => { const rootMessenger = getControllerMessenger(); const messenger = getSnapControllerMessenger(rootMessenger); const snapController = getSnapController( 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 396549adf6..d803c3ce66 100644 --- a/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts +++ b/packages/snaps-execution-environments/src/common/BaseSnapExecutor.test.browser.ts @@ -1474,7 +1474,7 @@ describe('BaseSnapExecutor', () => { }); }); - it('supports onAssetsLookup export', async () => { + it('supports `onAssetsLookup` export', async () => { const CODE = ` module.exports.onAssetsLookup = () => ({ assets: {} }); `; @@ -1513,7 +1513,7 @@ describe('BaseSnapExecutor', () => { }); }); - it('supports onAssetsConversion export', async () => { + it('supports `onAssetsConversion` export', async () => { const CODE = ` module.exports.onAssetsConversion = () => ({ conversionRates: {} }); `; From eb19f8c8273e4f3b656cc046a4a9fbebf8bc7cb2 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 14:28:14 +0100 Subject: [PATCH 13/17] Address PR review comments --- .../src/snaps/SnapController.test.tsx | 788 +++++++++--------- .../src/snaps/SnapController.ts | 8 +- 2 files changed, 401 insertions(+), 395 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.test.tsx b/packages/snaps-controllers/src/snaps/SnapController.test.tsx index f20e2a229a..9bc0b487ca 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.test.tsx +++ b/packages/snaps-controllers/src/snaps/SnapController.test.tsx @@ -3898,453 +3898,457 @@ describe('SnapController', () => { snapController.destroy(); }); - it('throws if `onAssetsLookup` handler returns an invalid response', async () => { - const rootMessenger = getControllerMessenger(); - const messenger = getSnapControllerMessenger(rootMessenger); - const snapController = getSnapController( - getSnapControllerOptions({ - messenger, - state: { - snaps: getPersistedSnapsState(), - }, - }), - ); - - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => ({ - [SnapEndowments.Assets]: { - caveats: [ - { - type: SnapCaveatType.ChainIds, - value: ['bip122:000000000019d6689c085ae165831e93'], - }, - ], - date: 1664187844588, - id: 'izn0WGUO8cvq_jqvLQuQP', - invoker: MOCK_SNAP_ID, - parentCapability: SnapEndowments.Assets, - }, - }), - ); - - rootMessenger.registerActionHandler( - 'SubjectMetadataController:getSubjectMetadata', - () => MOCK_SNAP_SUBJECT_METADATA, - ); - - rootMessenger.registerActionHandler( - 'ExecutionService:handleRpcRequest', - async () => - Promise.resolve({ - assets: { foo: {} }, + describe('onAssetsLookup', () => { + it('throws if `onAssetsLookup` handler returns an invalid response', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, }), - ); + ); - await expect( - snapController.handleRequest({ - snapId: MOCK_SNAP_ID, - origin: 'foo.com', - handler: HandlerType.OnAssetsLookup, - request: { - jsonrpc: '2.0', - method: ' ', - params: {}, - id: 1, - }, - }), - ).rejects.toThrow( - `Assertion failed: At path: assets.foo -- Expected a string matching`, - ); + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, + }, + }), + ); - snapController.destroy(); - }); + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); - it('filters out assets that are out of scope for `onAssetsLookup`', async () => { - const rootMessenger = getControllerMessenger(); - const messenger = getSnapControllerMessenger(rootMessenger); - const snapController = getSnapController( - getSnapControllerOptions({ - messenger, - state: { - snaps: getPersistedSnapsState(), - }, - }), - ); + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + assets: { foo: {} }, + }), + ); - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => ({ - [SnapEndowments.Assets]: { - caveats: [ - { - type: SnapCaveatType.ChainIds, - value: ['bip122:000000000019d6689c085ae165831e93'], - }, - ], - date: 1664187844588, - id: 'izn0WGUO8cvq_jqvLQuQP', - invoker: MOCK_SNAP_ID, - parentCapability: SnapEndowments.Assets, - }, - }), - ); + await expect( + snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsLookup, + request: { + jsonrpc: '2.0', + method: ' ', + params: {}, + id: 1, + }, + }), + ).rejects.toThrow( + `Assertion failed: At path: assets.foo -- Expected a string matching`, + ); - rootMessenger.registerActionHandler( - 'SubjectMetadataController:getSubjectMetadata', - () => MOCK_SNAP_SUBJECT_METADATA, - ); + snapController.destroy(); + }); - rootMessenger.registerActionHandler( - 'ExecutionService:handleRpcRequest', - async () => - Promise.resolve({ - assets: { - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { - name: 'Solana', - symbol: 'SOL', - fungible: true, - iconUrl: 'https://metamask.io/sol.svg', - units: [ - { - name: 'Solana', - symbol: 'SOL', - decimals: 9, - }, - ], - }, + it('filters out assets that are out of scope for `onAssetsLookup`', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), }, }), - ); + ); - expect( - await snapController.handleRequest({ - snapId: MOCK_SNAP_ID, - origin: 'foo.com', - handler: HandlerType.OnAssetsLookup, - request: { - jsonrpc: '2.0', - method: ' ', - params: { - assets: ['bip122:000000000019d6689c085ae165831e93/slip44:0'], + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, }, - id: 1, - }, - }), - ).toStrictEqual({ assets: {} }); - - snapController.destroy(); - }); + }), + ); - it('returns the value when `onAssetsLookup` returns a valid response', async () => { - const rootMessenger = getControllerMessenger(); - const messenger = getSnapControllerMessenger(rootMessenger); - const snapController = getSnapController( - getSnapControllerOptions({ - messenger, - state: { - snaps: getPersistedSnapsState(), - }, - }), - ); + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => ({ - [SnapEndowments.Assets]: { - caveats: [ - { - type: SnapCaveatType.ChainIds, - value: ['bip122:000000000019d6689c085ae165831e93'], + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + assets: { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { + name: 'Solana', + symbol: 'SOL', + fungible: true, + iconUrl: 'https://metamask.io/sol.svg', + units: [ + { + name: 'Solana', + symbol: 'SOL', + decimals: 9, + }, + ], + }, }, - ], - date: 1664187844588, - id: 'izn0WGUO8cvq_jqvLQuQP', - invoker: MOCK_SNAP_ID, - parentCapability: SnapEndowments.Assets, - }, - }), - ); - - rootMessenger.registerActionHandler( - 'SubjectMetadataController:getSubjectMetadata', - () => MOCK_SNAP_SUBJECT_METADATA, - ); + }), + ); - rootMessenger.registerActionHandler( - 'ExecutionService:handleRpcRequest', - async () => - Promise.resolve({ - assets: { - 'bip122:000000000019d6689c085ae165831e93/slip44:0': { - name: 'Bitcoin', - symbol: 'BTC', - fungible: true, - iconUrl: 'https://metamask.io/btc.svg', - units: [ - { - name: 'Bitcoin', - symbol: 'BTC', - decimals: 8, - }, - ], + expect( + await snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsLookup, + request: { + jsonrpc: '2.0', + method: ' ', + params: { + assets: ['bip122:000000000019d6689c085ae165831e93/slip44:0'], }, + id: 1, }, }), - ); + ).toStrictEqual({ assets: {} }); - expect( - await snapController.handleRequest({ - snapId: MOCK_SNAP_ID, - origin: 'foo.com', - handler: HandlerType.OnAssetsLookup, - request: { - jsonrpc: '2.0', - method: ' ', - params: { - assets: ['bip122:000000000019d6689c085ae165831e93/slip44:0'], - }, - id: 1, - }, - }), - ).toStrictEqual({ - assets: { - 'bip122:000000000019d6689c085ae165831e93/slip44:0': { - name: 'Bitcoin', - symbol: 'BTC', - fungible: true, - iconUrl: 'https://metamask.io/btc.svg', - units: [ - { - name: 'Bitcoin', - symbol: 'BTC', - decimals: 8, - }, - ], - }, - }, + snapController.destroy(); }); - snapController.destroy(); - }); - - it('throws if `onAssetsConversion` handler returns an invalid response', async () => { - const rootMessenger = getControllerMessenger(); - const messenger = getSnapControllerMessenger(rootMessenger); - const snapController = getSnapController( - getSnapControllerOptions({ - messenger, - state: { - snaps: getPersistedSnapsState(), - }, - }), - ); + it('returns the value when `onAssetsLookup` returns a valid response', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, + }), + ); - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => ({ - [SnapEndowments.Assets]: { - caveats: [ - { - type: SnapCaveatType.ChainIds, - value: ['bip122:000000000019d6689c085ae165831e93'], - }, - ], - date: 1664187844588, - id: 'izn0WGUO8cvq_jqvLQuQP', - invoker: MOCK_SNAP_ID, - parentCapability: SnapEndowments.Assets, - }, - }), - ); + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, + }, + }), + ); - rootMessenger.registerActionHandler( - 'SubjectMetadataController:getSubjectMetadata', - () => MOCK_SNAP_SUBJECT_METADATA, - ); + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); - rootMessenger.registerActionHandler( - 'ExecutionService:handleRpcRequest', - async () => - Promise.resolve({ - conversionRates: { foo: {} }, - }), - ); + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + assets: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + name: 'Bitcoin', + symbol: 'BTC', + fungible: true, + iconUrl: 'https://metamask.io/btc.svg', + units: [ + { + name: 'Bitcoin', + symbol: 'BTC', + decimals: 8, + }, + ], + }, + }, + }), + ); - await expect( - snapController.handleRequest({ - snapId: MOCK_SNAP_ID, - origin: 'foo.com', - handler: HandlerType.OnAssetsConversion, - request: { - jsonrpc: '2.0', - method: ' ', - params: {}, - id: 1, + expect( + await snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsLookup, + request: { + jsonrpc: '2.0', + method: ' ', + params: { + assets: ['bip122:000000000019d6689c085ae165831e93/slip44:0'], + }, + id: 1, + }, + }), + ).toStrictEqual({ + assets: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + name: 'Bitcoin', + symbol: 'BTC', + fungible: true, + iconUrl: 'https://metamask.io/btc.svg', + units: [ + { + name: 'Bitcoin', + symbol: 'BTC', + decimals: 8, + }, + ], + }, }, - }), - ).rejects.toThrow( - `Assertion failed: At path: conversionRates.foo -- Expected a string matching`, - ); + }); - snapController.destroy(); + snapController.destroy(); + }); }); - it('filters out assets that are out of scope for `onAssetsConversion`', async () => { - const rootMessenger = getControllerMessenger(); - const messenger = getSnapControllerMessenger(rootMessenger); - const snapController = getSnapController( - getSnapControllerOptions({ - messenger, - state: { - snaps: getPersistedSnapsState(), - }, - }), - ); + describe('onAssetsConversion', () => { + it('throws if `onAssetsConversion` handler returns an invalid response', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, + }), + ); - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => ({ - [SnapEndowments.Assets]: { - caveats: [ - { - type: SnapCaveatType.ChainIds, - value: ['bip122:000000000019d6689c085ae165831e93'], - }, - ], - date: 1664187844588, - id: 'izn0WGUO8cvq_jqvLQuQP', - invoker: MOCK_SNAP_ID, - parentCapability: SnapEndowments.Assets, - }, - }), - ); + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ + { + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, + }, + }), + ); - rootMessenger.registerActionHandler( - 'SubjectMetadataController:getSubjectMetadata', - () => MOCK_SNAP_SUBJECT_METADATA, - ); + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); - rootMessenger.registerActionHandler( - 'ExecutionService:handleRpcRequest', - async () => - Promise.resolve({ - conversionRates: { - 'bip122:000000000019d6689c085ae165831e93/slip44:0': { - 'eip155:1/slip44:60': { - rate: '33', - conversionTime: 1737548790, - }, - }, + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + conversionRates: { foo: {} }, + }), + ); + + await expect( + snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsConversion, + request: { + jsonrpc: '2.0', + method: ' ', + params: {}, + id: 1, }, }), - ); + ).rejects.toThrow( + `Assertion failed: At path: conversionRates.foo -- Expected a string matching`, + ); - expect( - await snapController.handleRequest({ - snapId: MOCK_SNAP_ID, - origin: 'foo.com', - handler: HandlerType.OnAssetsConversion, - request: { - jsonrpc: '2.0', - method: ' ', - params: { - conversions: [ + snapController.destroy(); + }); + + it('filters out assets that are out of scope for `onAssetsConversion`', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), + }, + }), + ); + + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ { - from: 'bip122:000000000019d6689c085ae165831e93/slip44:0', - to: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], }, ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, }, - id: 1, - }, - }), - ).toStrictEqual({ conversionRates: {} }); + }), + ); - snapController.destroy(); - }); + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); - it('returns the value when `onAssetsConversion` returns a valid response', async () => { - const rootMessenger = getControllerMessenger(); - const messenger = getSnapControllerMessenger(rootMessenger); - const snapController = getSnapController( - getSnapControllerOptions({ - messenger, - state: { - snaps: getPersistedSnapsState(), - }, - }), - ); + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + conversionRates: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + 'eip155:1/slip44:60': { + rate: '33', + conversionTime: 1737548790, + }, + }, + }, + }), + ); - rootMessenger.registerActionHandler( - 'PermissionController:getPermissions', - () => ({ - [SnapEndowments.Assets]: { - caveats: [ - { - type: SnapCaveatType.ChainIds, - value: ['bip122:000000000019d6689c085ae165831e93'], + expect( + await snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsConversion, + request: { + jsonrpc: '2.0', + method: ' ', + params: { + conversions: [ + { + from: 'bip122:000000000019d6689c085ae165831e93/slip44:0', + to: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + }, + ], }, - ], - date: 1664187844588, - id: 'izn0WGUO8cvq_jqvLQuQP', - invoker: MOCK_SNAP_ID, - parentCapability: SnapEndowments.Assets, - }, - }), - ); + id: 1, + }, + }), + ).toStrictEqual({ conversionRates: {} }); - rootMessenger.registerActionHandler( - 'SubjectMetadataController:getSubjectMetadata', - () => MOCK_SNAP_SUBJECT_METADATA, - ); + snapController.destroy(); + }); - rootMessenger.registerActionHandler( - 'ExecutionService:handleRpcRequest', - async () => - Promise.resolve({ - conversionRates: { - 'bip122:000000000019d6689c085ae165831e93/slip44:0': { - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { - rate: '400', - conversionTime: 1737548790, - }, - }, + it('returns the value when `onAssetsConversion` returns a valid response', async () => { + const rootMessenger = getControllerMessenger(); + const messenger = getSnapControllerMessenger(rootMessenger); + const snapController = getSnapController( + getSnapControllerOptions({ + messenger, + state: { + snaps: getPersistedSnapsState(), }, }), - ); + ); - expect( - await snapController.handleRequest({ - snapId: MOCK_SNAP_ID, - origin: 'foo.com', - handler: HandlerType.OnAssetsConversion, - request: { - jsonrpc: '2.0', - method: ' ', - params: { - conversions: [ + rootMessenger.registerActionHandler( + 'PermissionController:getPermissions', + () => ({ + [SnapEndowments.Assets]: { + caveats: [ { - from: 'bip122:000000000019d6689c085ae165831e93/slip44:0', - to: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + type: SnapCaveatType.ChainIds, + value: ['bip122:000000000019d6689c085ae165831e93'], }, ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_SNAP_ID, + parentCapability: SnapEndowments.Assets, }, - id: 1, - }, - }), - ).toStrictEqual({ - conversionRates: { - 'bip122:000000000019d6689c085ae165831e93/slip44:0': { - 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { - rate: '400', - conversionTime: 1737548790, + }), + ); + + rootMessenger.registerActionHandler( + 'SubjectMetadataController:getSubjectMetadata', + () => MOCK_SNAP_SUBJECT_METADATA, + ); + + rootMessenger.registerActionHandler( + 'ExecutionService:handleRpcRequest', + async () => + Promise.resolve({ + conversionRates: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { + rate: '400', + conversionTime: 1737548790, + }, + }, + }, + }), + ); + + expect( + await snapController.handleRequest({ + snapId: MOCK_SNAP_ID, + origin: 'foo.com', + handler: HandlerType.OnAssetsConversion, + request: { + jsonrpc: '2.0', + method: ' ', + params: { + conversions: [ + { + from: 'bip122:000000000019d6689c085ae165831e93/slip44:0', + to: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + }, + ], + }, + id: 1, + }, + }), + ).toStrictEqual({ + conversionRates: { + 'bip122:000000000019d6689c085ae165831e93/slip44:0': { + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501': { + rate: '400', + conversionTime: 1737548790, + }, }, }, - }, - }); + }); - snapController.destroy(); + snapController.destroy(); + }); }); describe('getRpcRequestHandler', () => { diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index eae3a11601..b2a68a39b5 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -3618,14 +3618,16 @@ export class SnapController extends BaseController< return result; } case HandlerType.OnAssetsLookup: { - // We know the permissions are guaranteed to be set here. const permissions = this.messagingSystem.call( 'PermissionController:getPermissions', snapId, - ) as SubjectPermissions; + ); + // We know the permissions are guaranteed to be set here. + assert(permissions); const permission = permissions[SnapEndowments.Assets]; - const scopes = getChainIdsCaveat(permission) as string[]; + const scopes = getChainIdsCaveat(permission); + assert(scopes); // We can cast since the request and result have already been validated. const { params: requestedParams } = request as { From 1696fb18a2c1660de1270c78b79b2e44ee5c6f66 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 22 Jan 2025 16:36:53 +0100 Subject: [PATCH 14/17] Refactor slightly --- .../src/snaps/SnapController.ts | 117 ++++++++++-------- 1 file changed, 66 insertions(+), 51 deletions(-) diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index b2a68a39b5..0808ddbe7a 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -3617,66 +3617,81 @@ export class SnapController extends BaseController< } return result; } - case HandlerType.OnAssetsLookup: { - const permissions = this.messagingSystem.call( - 'PermissionController:getPermissions', + case HandlerType.OnAssetsLookup: + // We can cast since the request and result have already been validated. + return this.#transformOnAssetsLookupResult( snapId, + request as { params: OnAssetsLookupArguments }, + result as OnAssetsLookupResponse, ); - // We know the permissions are guaranteed to be set here. - assert(permissions); - - const permission = permissions[SnapEndowments.Assets]; - const scopes = getChainIdsCaveat(permission); - assert(scopes); + case HandlerType.OnAssetsConversion: // We can cast since the request and result have already been validated. - const { params: requestedParams } = request as { - params: OnAssetsLookupArguments; - }; - const { assets: requestedAssets } = requestedParams; - const { assets } = result as OnAssetsLookupResponse; - const filteredAssets = Object.keys(assets).reduce< - Record - >((accumulator, assetType) => { - const castAssetType = assetType as CaipAssetType; - const isValid = - scopes.some((scope) => castAssetType.startsWith(scope)) && - requestedAssets.includes(castAssetType); - // Filter out unrequested assets and assets for scopes the Snap hasn't registered for. - if (isValid) { - accumulator[castAssetType] = assets[castAssetType]; - } - return accumulator; - }, {}); - return { assets: filteredAssets }; - } - case HandlerType.OnAssetsConversion: { - // We can cast since the request and result have already been validated. - const { params: requestedParams } = request as { - params: OnAssetsConversionArguments; - }; - const { conversions: requestedConversions } = requestedParams; - - const { conversionRates } = result as OnAssetsConversionResponse; - - const filteredConversionRates = requestedConversions.reduce< - Record> - >((accumulator, conversion) => { - const rate = conversionRates[conversion.from]?.[conversion.to]; - // Only include rates that were actually requested. - if (rate) { - accumulator[conversion.from] ??= {}; - accumulator[conversion.from][conversion.to] = rate; - } - return accumulator; - }, {}); - return { conversionRates: filteredConversionRates }; - } + return this.#transformOnAssetsConversionResult( + request as { + params: OnAssetsConversionArguments; + }, + result as OnAssetsConversionResponse, + ); default: return result; } } + #transformOnAssetsLookupResult( + snapId: SnapId, + { params: requestedParams }: { params: OnAssetsLookupArguments }, + { assets }: OnAssetsLookupResponse, + ) { + const permissions = this.messagingSystem.call( + 'PermissionController:getPermissions', + snapId, + ); + // We know the permissions are guaranteed to be set here. + assert(permissions); + + const permission = permissions[SnapEndowments.Assets]; + const scopes = getChainIdsCaveat(permission); + assert(scopes); + + const { assets: requestedAssets } = requestedParams; + + const filteredAssets = Object.keys(assets).reduce< + Record + >((accumulator, assetType) => { + const castAssetType = assetType as CaipAssetType; + const isValid = + scopes.some((scope) => castAssetType.startsWith(scope)) && + requestedAssets.includes(castAssetType); + // Filter out unrequested assets and assets for scopes the Snap hasn't registered for. + if (isValid) { + accumulator[castAssetType] = assets[castAssetType]; + } + return accumulator; + }, {}); + return { assets: filteredAssets }; + } + + #transformOnAssetsConversionResult( + { params: requestedParams }: { params: OnAssetsConversionArguments }, + { conversionRates }: OnAssetsConversionResponse, + ) { + const { conversions: requestedConversions } = requestedParams; + + const filteredConversionRates = requestedConversions.reduce< + Record> + >((accumulator, conversion) => { + const rate = conversionRates[conversion.from]?.[conversion.to]; + // Only include rates that were actually requested. + if (rate) { + accumulator[conversion.from] ??= {}; + accumulator[conversion.from][conversion.to] = rate; + } + return accumulator; + }, {}); + return { conversionRates: filteredConversionRates }; + } + /** * Assert that the returned result of a Snap RPC call is the expected shape. * From ee86bd75d43f479225f19838c1e1151bd530f58b Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 23 Jan 2025 10:52:30 +0100 Subject: [PATCH 15/17] Add JSDoc --- .../src/snaps/SnapController.ts | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/snaps-controllers/src/snaps/SnapController.ts b/packages/snaps-controllers/src/snaps/SnapController.ts index 0808ddbe7a..f93d78d01c 100644 --- a/packages/snaps-controllers/src/snaps/SnapController.ts +++ b/packages/snaps-controllers/src/snaps/SnapController.ts @@ -3638,6 +3638,19 @@ export class SnapController extends BaseController< } } + /** + * Transform an RPC response coming from the `onAssetsLookup` handler. + * + * This filters out responses that are out of scope for the Snap based on + * its permissions and the incoming request. + * + * @param snapId - The snap ID of the snap that produced the result. + * @param request - The request that returned the result. + * @param request.params - The parameters for the request. + * @param result - The result. + * @param result.assets - The assets returned by the Snap. + * @returns The transformed result. + */ #transformOnAssetsLookupResult( snapId: SnapId, { params: requestedParams }: { params: OnAssetsLookupArguments }, @@ -3672,6 +3685,18 @@ export class SnapController extends BaseController< return { assets: filteredAssets }; } + /** + * Transform an RPC response coming from the `onAssetsConversion` handler. + * + * This filters out responses that are out of scope for the Snap based on + * the incoming request. + * + * @param request - The request that returned the result. + * @param request.params - The parameters for the request. + * @param result - The result. + * @param result.conversionRates - The conversion rates returned by the Snap. + * @returns The transformed result. + */ #transformOnAssetsConversionResult( { params: requestedParams }: { params: OnAssetsConversionArguments }, { conversionRates }: OnAssetsConversionResponse, From 5b6149728581296922bf49aaadc5b41c5ef126ee Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 23 Jan 2025 10:59:36 +0100 Subject: [PATCH 16/17] Add test for uncovered validation function --- .../coverage.json | 8 +- .../src/common/validation.test.ts | 80 +++++++++++++++++++ 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index 59b385b8e1..5aae79e628 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { - "branches": 80.95, - "functions": 88.15, - "lines": 90.28, - "statements": 89.45 + "branches": 80.27, + "functions": 89.47, + "lines": 90.7, + "statements": 89.86 } diff --git a/packages/snaps-execution-environments/src/common/validation.test.ts b/packages/snaps-execution-environments/src/common/validation.test.ts index 87a6b8ab5e..eef0294d2a 100644 --- a/packages/snaps-execution-environments/src/common/validation.test.ts +++ b/packages/snaps-execution-environments/src/common/validation.test.ts @@ -1,6 +1,8 @@ import { UserInputEventType } from '@metamask/snaps-sdk'; import { + assertIsOnAssetsConversionRequestArguments, + assertIsOnAssetsLookupRequestArguments, assertIsOnNameLookupRequestArguments, assertIsOnSignatureRequestArguments, assertIsOnTransactionRequestArguments, @@ -232,3 +234,81 @@ describe('assertIsOnUserInputRequestArguments', () => { ); }); }); + +describe('assertIsOnAssetsLookupRequestArguments', () => { + it.each([ + { assets: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501'] }, + { + assets: [ + 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + 'bip122:000000000019d6689c085ae165831e93/slip44:0', + ], + }, + ])('does not throw for a valid assets lookup param object', (value) => { + expect(() => assertIsOnAssetsLookupRequestArguments(value)).not.toThrow(); + }); + + it.each([ + true, + false, + null, + undefined, + 0, + 1, + '', + 'foo', + [], + {}, + { assets: [] }, + { assets: ['foo'] }, + ])( + 'throws if the value is not a valid assets lookup params object', + (value) => { + expect(() => + assertIsOnAssetsLookupRequestArguments(value as any), + ).toThrow('Invalid request params:'); + }, + ); +}); + +describe('assertIsOnAssetsConversionRequestArguments', () => { + it.each([ + { + conversions: [ + { + from: 'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp/slip44:501', + to: 'bip122:000000000019d6689c085ae165831e93/slip44:0', + }, + ], + }, + ])('does not throw for a valid assets conversion param object', (value) => { + expect(() => + assertIsOnAssetsConversionRequestArguments(value), + ).not.toThrow(); + }); + + it.each([ + true, + false, + null, + undefined, + 0, + 1, + '', + 'foo', + [], + {}, + { conversions: [] }, + { conversions: ['foo'] }, + { conversions: [{}] }, + { conversions: [{ from: 'foo' }] }, + { conversions: [{ from: 'foo', to: 'foo' }] }, + ])( + 'throws if the value is not a valid assets conversion params object', + (value) => { + expect(() => + assertIsOnAssetsConversionRequestArguments(value as any), + ).toThrow('Invalid request params:'); + }, + ); +}); From cb67728fbe50ff893b70a583ea0cf9cd2e226cac Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 23 Jan 2025 11:08:05 +0100 Subject: [PATCH 17/17] Update coverage --- packages/snaps-execution-environments/coverage.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/snaps-execution-environments/coverage.json b/packages/snaps-execution-environments/coverage.json index 5aae79e628..f8b47f1792 100644 --- a/packages/snaps-execution-environments/coverage.json +++ b/packages/snaps-execution-environments/coverage.json @@ -1,6 +1,6 @@ { - "branches": 80.27, + "branches": 80.95, "functions": 89.47, - "lines": 90.7, - "statements": 89.86 + "lines": 90.84, + "statements": 90 }