From 1e54f3c9a6f5f2cdaff312cf28f061b7eeeb9879 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 24 Dec 2024 17:05:18 -0500 Subject: [PATCH 1/7] add logic to allow ISO 8601 duration strings --- .../permitted/scheduleBackgroundEvent.test.ts | 48 +++++++++++++++++++ .../src/permitted/scheduleBackgroundEvent.ts | 25 +++++++--- .../methods/schedule-background-event.ts | 4 +- 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts index a121ef8c1d..dcb487b40b 100644 --- a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts +++ b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts @@ -120,6 +120,54 @@ describe('snap_scheduleBackgroundEvent', () => { }); }); + it('schedules a background event using duration', async () => { + const { implementation } = scheduleBackgroundEventHandler; + + const scheduleBackgroundEvent = jest.fn(); + const hasPermission = jest.fn().mockImplementation(() => true); + + const hooks = { + scheduleBackgroundEvent, + hasPermission, + }; + + const engine = new JsonRpcEngine(); + + engine.push(createOriginMiddleware(MOCK_SNAP_ID)); + engine.push((request, response, next, end) => { + const result = implementation( + request as JsonRpcRequest, + response as PendingJsonRpcResponse, + next, + end, + hooks, + ); + + result?.catch(end); + }); + + await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: 'snap_scheduleBackgroundEvent', + params: { + date: 'PT30S', + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + }); + + expect(scheduleBackgroundEvent).toHaveBeenCalledWith({ + date: expect.any(String), + request: { + method: 'handleExport', + params: ['p1'], + }, + }); + }); + it('throws if a snap does not have the "endowment:cronjob" permission', async () => { const { implementation } = scheduleBackgroundEventHandler; diff --git a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts index 99e7ce2813..9d57c1f396 100644 --- a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts +++ b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts @@ -19,7 +19,7 @@ import { string, } from '@metamask/superstruct'; import { assert, type PendingJsonRpcResponse } from '@metamask/utils'; -import { DateTime } from 'luxon'; +import { DateTime, Duration } from 'luxon'; import { SnapEndowments } from '../endowments'; import type { MethodHooksObject } from '../utils'; @@ -58,12 +58,15 @@ const offsetRegex = /Z|([+-]\d{2}:?\d{2})$/u; const ScheduleBackgroundEventsParametersStruct = object({ date: refine(string(), 'date', (val) => { const date = DateTime.fromISO(val); + const duration = Duration.fromISO(val); if (date.isValid) { // Luxon doesn't have a reliable way to check if timezone info was not provided if (!offsetRegex.test(val)) { return 'ISO 8601 string must have timezone information'; } return true; + } else if (duration.isValid) { + return true; } return 'Not a valid ISO 8601 string'; }), @@ -109,12 +112,22 @@ async function getScheduleBackgroundEventImplementation( const { date, request } = validatedParams; + let truncatedDate; + + const duration = Duration.fromISO(date); + + // We have to check if the string is a duration or not + if (duration.isValid) { + truncatedDate = DateTime.fromJSDate(new Date()).toUTC().plus(duration); + } else { + truncatedDate = DateTime.fromISO(date, { setZone: true }); + } + // Make sure any millisecond precision is removed. - const truncatedDate = DateTime.fromISO(date, { setZone: true }) - .startOf('second') - .toISO({ - suppressMilliseconds: true, - }); + + truncatedDate = truncatedDate.startOf('second').toISO({ + suppressMilliseconds: true, + }); assert(truncatedDate); diff --git a/packages/snaps-sdk/src/types/methods/schedule-background-event.ts b/packages/snaps-sdk/src/types/methods/schedule-background-event.ts index caa58e3390..d4d1a218b6 100644 --- a/packages/snaps-sdk/src/types/methods/schedule-background-event.ts +++ b/packages/snaps-sdk/src/types/methods/schedule-background-event.ts @@ -3,7 +3,9 @@ import type { Cronjob } from '../permissions'; /** * The request parameters for the `snap_scheduleBackgroundEvent` method. * - * @property date - The ISO8601 date of when to fire the background event. + * Note: The date generated from a duration will be represented in UTC. + * + * @property date - The ISO 8601 date/duration of when to fire the background event. * @property request - The request to be called when the event fires. */ export type ScheduleBackgroundEventParams = { From b9050408dbbda543ae60f17ef17839e79ecf9620 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 24 Dec 2024 17:40:56 -0500 Subject: [PATCH 2/7] update coverage --- packages/snaps-rpc-methods/jest.config.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/snaps-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js index fa993ad89b..269725354e 100644 --- a/packages/snaps-rpc-methods/jest.config.js +++ b/packages/snaps-rpc-methods/jest.config.js @@ -10,9 +10,9 @@ module.exports = deepmerge(baseConfig, { ], coverageThreshold: { global: { - branches: 93.91, + branches: 94, functions: 98.02, - lines: 98.65, + lines: 98.66, statements: 98.24, }, }, From 17975fe890635b152a86cfc55297b276f6aef662 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Mon, 6 Jan 2025 15:29:07 -0500 Subject: [PATCH 3/7] changes per review --- packages/snaps-rpc-methods/jest.config.js | 6 +- .../permitted/scheduleBackgroundEvent.test.ts | 59 ++++++++++++++++-- .../src/permitted/scheduleBackgroundEvent.ts | 60 ++++++++++++------- .../types/methods/get-background-events.ts | 2 + .../methods/schedule-background-event.ts | 13 ++-- 5 files changed, 108 insertions(+), 32 deletions(-) diff --git a/packages/snaps-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js index 269725354e..5bc19e96aa 100644 --- a/packages/snaps-rpc-methods/jest.config.js +++ b/packages/snaps-rpc-methods/jest.config.js @@ -11,9 +11,9 @@ module.exports = deepmerge(baseConfig, { coverageThreshold: { global: { branches: 94, - functions: 98.02, - lines: 98.66, - statements: 98.24, + functions: 98.04, + lines: 98.67, + statements: 98.25, }, }, }); diff --git a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts index dcb487b40b..09848b9524 100644 --- a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts +++ b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.test.ts @@ -151,7 +151,7 @@ describe('snap_scheduleBackgroundEvent', () => { id: 1, method: 'snap_scheduleBackgroundEvent', params: { - date: 'PT30S', + duration: 'PT30S', request: { method: 'handleExport', params: ['p1'], @@ -168,6 +168,57 @@ describe('snap_scheduleBackgroundEvent', () => { }); }); + it('throws on an invalid duration', async () => { + const { implementation } = scheduleBackgroundEventHandler; + + const scheduleBackgroundEvent = jest.fn(); + const hasPermission = jest.fn().mockImplementation(() => true); + + const hooks = { + scheduleBackgroundEvent, + hasPermission, + }; + + const engine = new JsonRpcEngine(); + + engine.push(createOriginMiddleware(MOCK_SNAP_ID)); + engine.push((request, response, next, end) => { + const result = implementation( + request as JsonRpcRequest, + response as PendingJsonRpcResponse, + next, + end, + hooks, + ); + + result?.catch(end); + }); + + const response = await engine.handle({ + jsonrpc: '2.0', + id: 1, + method: 'snap_scheduleBackgroundEvent', + params: { + duration: 'PQ30S', + request: { + method: 'handleExport', + params: ['p1'], + }, + }, + }); + + expect(response).toStrictEqual({ + error: { + code: -32602, + message: + 'Invalid params: At path: duration -- Not a valid ISO 8601 duration.', + stack: expect.any(String), + }, + id: 1, + jsonrpc: '2.0', + }); + }); + it('throws if a snap does not have the "endowment:cronjob" permission', async () => { const { implementation } = scheduleBackgroundEventHandler; @@ -219,7 +270,7 @@ describe('snap_scheduleBackgroundEvent', () => { }); }); - it('throws if no timezone information is provided in the ISO8601 string', async () => { + it('throws if no timezone information is provided in the ISO 8601 date', async () => { const { implementation } = scheduleBackgroundEventHandler; const scheduleBackgroundEvent = jest.fn(); @@ -262,7 +313,7 @@ describe('snap_scheduleBackgroundEvent', () => { error: { code: -32602, message: - 'Invalid params: At path: date -- ISO 8601 string must have timezone information.', + 'Invalid params: At path: date -- ISO 8601 date must have timezone information.', stack: expect.any(String), }, id: 1, @@ -313,7 +364,7 @@ describe('snap_scheduleBackgroundEvent', () => { error: { code: -32602, message: - 'Invalid params: At path: date -- Not a valid ISO 8601 string.', + 'Invalid params: At path: date -- Not a valid ISO 8601 date.', stack: expect.any(String), }, id: 1, diff --git a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts index 9d57c1f396..597e464d0e 100644 --- a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts +++ b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts @@ -1,10 +1,11 @@ import type { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine'; import type { PermittedHandlerExport } from '@metamask/permission-controller'; import { providerErrors, rpcErrors } from '@metamask/rpc-errors'; -import type { - JsonRpcRequest, - ScheduleBackgroundEventParams, - ScheduleBackgroundEventResult, +import { + selectiveUnion, + type JsonRpcRequest, + type ScheduleBackgroundEventParams, + type ScheduleBackgroundEventResult, } from '@metamask/snaps-sdk'; import type { CronjobRpcRequest } from '@metamask/snaps-utils'; import { @@ -18,7 +19,11 @@ import { refine, string, } from '@metamask/superstruct'; -import { assert, type PendingJsonRpcResponse } from '@metamask/utils'; +import { + assert, + hasProperty, + type PendingJsonRpcResponse, +} from '@metamask/utils'; import { DateTime, Duration } from 'luxon'; import { SnapEndowments } from '../endowments'; @@ -55,26 +60,42 @@ export const scheduleBackgroundEventHandler: PermittedHandlerExport< }; const offsetRegex = /Z|([+-]\d{2}:?\d{2})$/u; -const ScheduleBackgroundEventsParametersStruct = object({ + +const ScheduleBackgroundEventParametersWithDateStruct = object({ date: refine(string(), 'date', (val) => { const date = DateTime.fromISO(val); - const duration = Duration.fromISO(val); if (date.isValid) { // Luxon doesn't have a reliable way to check if timezone info was not provided if (!offsetRegex.test(val)) { - return 'ISO 8601 string must have timezone information'; + return 'ISO 8601 date must have timezone information'; } return true; - } else if (duration.isValid) { - return true; } - return 'Not a valid ISO 8601 string'; + return 'Not a valid ISO 8601 date'; }), request: CronjobRpcRequestStruct, }); +const ScheduleBackgroundEventParametersWithDurationStruct = object({ + duration: refine(string(), 'duration', (val) => { + const duration = Duration.fromISO(val); + if (!duration.isValid) { + return 'Not a valid ISO 8601 duration'; + } + return true; + }), + request: CronjobRpcRequestStruct, +}); + +const ScheduleBackgroundEventParametersStruct = selectiveUnion((val) => { + if (hasProperty(val, 'date')) { + return ScheduleBackgroundEventParametersWithDateStruct; + } + return ScheduleBackgroundEventParametersWithDurationStruct; +}); + export type ScheduleBackgroundEventParameters = InferMatching< - typeof ScheduleBackgroundEventsParametersStruct, + typeof ScheduleBackgroundEventParametersStruct, ScheduleBackgroundEventParams >; @@ -110,17 +131,16 @@ async function getScheduleBackgroundEventImplementation( try { const validatedParams = getValidatedParams(params); - const { date, request } = validatedParams; + const { request } = validatedParams; let truncatedDate; - const duration = Duration.fromISO(date); - - // We have to check if the string is a duration or not - if (duration.isValid) { - truncatedDate = DateTime.fromJSDate(new Date()).toUTC().plus(duration); + if ('duration' in validatedParams) { + truncatedDate = DateTime.fromJSDate(new Date()) + .toUTC() + .plus(Duration.fromISO(validatedParams.duration)); } else { - truncatedDate = DateTime.fromISO(date, { setZone: true }); + truncatedDate = DateTime.fromISO(validatedParams.date, { setZone: true }); } // Make sure any millisecond precision is removed. @@ -151,7 +171,7 @@ function getValidatedParams( params: unknown, ): ScheduleBackgroundEventParameters { try { - return create(params, ScheduleBackgroundEventsParametersStruct); + return create(params, ScheduleBackgroundEventParametersStruct); } catch (error) { if (error instanceof StructError) { throw rpcErrors.invalidParams({ diff --git a/packages/snaps-sdk/src/types/methods/get-background-events.ts b/packages/snaps-sdk/src/types/methods/get-background-events.ts index 84fd6dfcff..b2f322c038 100644 --- a/packages/snaps-sdk/src/types/methods/get-background-events.ts +++ b/packages/snaps-sdk/src/types/methods/get-background-events.ts @@ -5,6 +5,8 @@ import type { SnapId } from '../snap'; /** * Background event type * + * Note: The date generated when scheduling an event with a duration will be represented in UTC. + * * @property id - The unique id representing the event. * @property scheduledAt - The ISO 8601 time stamp of when the event was scheduled. * @property snapId - The id of the snap that scheduled the event. diff --git a/packages/snaps-sdk/src/types/methods/schedule-background-event.ts b/packages/snaps-sdk/src/types/methods/schedule-background-event.ts index d4d1a218b6..19f121f933 100644 --- a/packages/snaps-sdk/src/types/methods/schedule-background-event.ts +++ b/packages/snaps-sdk/src/types/methods/schedule-background-event.ts @@ -5,13 +5,16 @@ import type { Cronjob } from '../permissions'; * * Note: The date generated from a duration will be represented in UTC. * - * @property date - The ISO 8601 date/duration of when to fire the background event. + * @property date - The ISO 8601 date of when to fire the background event. + * @property duration - The ISO 8601 duration of when to fire the background event. * @property request - The request to be called when the event fires. */ -export type ScheduleBackgroundEventParams = { - date: string; - request: Cronjob['request']; -}; +export type ScheduleBackgroundEventParams = + | { + date: string; + request: Cronjob['request']; + } + | { duration: string; request: Cronjob['request'] }; /** * The result returned by the `snap_scheduleBackgroundEvent` method, which is the ID of the scheduled event. From 0a504ad9e7f80bd20c752fb6e679d0c79b02e4ec Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 7 Jan 2025 09:26:58 -0500 Subject: [PATCH 4/7] changes per review --- .../src/permitted/scheduleBackgroundEvent.ts | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts index 597e464d0e..eef94a7759 100644 --- a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts +++ b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts @@ -99,6 +99,29 @@ export type ScheduleBackgroundEventParameters = InferMatching< ScheduleBackgroundEventParams >; +/** + * Generates an ISO 8601 date based on if a duration or date is provided. + * + * @param params - The validated params from the `snap_scheduleBackgroundEvent` call. + * @returns An ISO 8601 date string. + */ +function getTruncatedDate(params: ScheduleBackgroundEventParams) { + let date; + + if ('duration' in params) { + date = DateTime.fromJSDate(new Date()) + .toUTC() + .plus(Duration.fromISO(params.duration)); + } else { + date = DateTime.fromISO(params.date, { setZone: true }); + } + + // Make sure any millisecond precision is removed. + return date.startOf('second').toISO({ + suppressMilliseconds: true, + }); +} + /** * The `snap_scheduleBackgroundEvent` method implementation. * @@ -133,25 +156,11 @@ async function getScheduleBackgroundEventImplementation( const { request } = validatedParams; - let truncatedDate; - - if ('duration' in validatedParams) { - truncatedDate = DateTime.fromJSDate(new Date()) - .toUTC() - .plus(Duration.fromISO(validatedParams.duration)); - } else { - truncatedDate = DateTime.fromISO(validatedParams.date, { setZone: true }); - } - - // Make sure any millisecond precision is removed. - - truncatedDate = truncatedDate.startOf('second').toISO({ - suppressMilliseconds: true, - }); + const date = getTruncatedDate(validatedParams); - assert(truncatedDate); + assert(date); - const id = scheduleBackgroundEvent({ date: truncatedDate, request }); + const id = scheduleBackgroundEvent({ date, request }); res.result = id; } catch (error) { return end(error); From 0d690e563e9d9ec87128895df1515a8cae3619a9 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 7 Jan 2025 09:35:06 -0500 Subject: [PATCH 5/7] update coverage --- packages/snaps-rpc-methods/jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snaps-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js index 5bc19e96aa..bd82f628a5 100644 --- a/packages/snaps-rpc-methods/jest.config.js +++ b/packages/snaps-rpc-methods/jest.config.js @@ -11,7 +11,7 @@ module.exports = deepmerge(baseConfig, { coverageThreshold: { global: { branches: 94, - functions: 98.04, + functions: 98.05, lines: 98.67, statements: 98.25, }, From 3bed845d97f6e72ca0e82b42c6f14ffd5e489c63 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 7 Jan 2025 12:59:59 -0500 Subject: [PATCH 6/7] changes per review --- .../src/permitted/scheduleBackgroundEvent.ts | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts index eef94a7759..acdfeebde1 100644 --- a/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts +++ b/packages/snaps-rpc-methods/src/permitted/scheduleBackgroundEvent.ts @@ -100,26 +100,19 @@ export type ScheduleBackgroundEventParameters = InferMatching< >; /** - * Generates an ISO 8601 date based on if a duration or date is provided. + * Generates a `DateTime` object based on if a duration or date is provided. * * @param params - The validated params from the `snap_scheduleBackgroundEvent` call. - * @returns An ISO 8601 date string. + * @returns A `DateTime` object. */ -function getTruncatedDate(params: ScheduleBackgroundEventParams) { - let date; - +function getStartDate(params: ScheduleBackgroundEventParams) { if ('duration' in params) { - date = DateTime.fromJSDate(new Date()) + return DateTime.fromJSDate(new Date()) .toUTC() .plus(Duration.fromISO(params.duration)); - } else { - date = DateTime.fromISO(params.date, { setZone: true }); } - // Make sure any millisecond precision is removed. - return date.startOf('second').toISO({ - suppressMilliseconds: true, - }); + return DateTime.fromISO(params.date, { setZone: true }); } /** @@ -156,11 +149,16 @@ async function getScheduleBackgroundEventImplementation( const { request } = validatedParams; - const date = getTruncatedDate(validatedParams); + const date = getStartDate(validatedParams); + + // Make sure any millisecond precision is removed. + const truncatedDate = date.startOf('second').toISO({ + suppressMilliseconds: true, + }); - assert(date); + assert(truncatedDate); - const id = scheduleBackgroundEvent({ date, request }); + const id = scheduleBackgroundEvent({ date: truncatedDate, request }); res.result = id; } catch (error) { return end(error); From 28b5e8a784c4c224b50ac5b55e291ebe10989295 Mon Sep 17 00:00:00 2001 From: Hassan Malik Date: Tue, 7 Jan 2025 14:24:33 -0500 Subject: [PATCH 7/7] fix coverage --- packages/snaps-rpc-methods/jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/snaps-rpc-methods/jest.config.js b/packages/snaps-rpc-methods/jest.config.js index bd82f628a5..fafe368501 100644 --- a/packages/snaps-rpc-methods/jest.config.js +++ b/packages/snaps-rpc-methods/jest.config.js @@ -10,7 +10,7 @@ module.exports = deepmerge(baseConfig, { ], coverageThreshold: { global: { - branches: 94, + branches: 93.97, functions: 98.05, lines: 98.67, statements: 98.25,