From 329baf8415b7b83c9694b59580489d1095a7f2b3 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Thu, 21 Jul 2022 12:58:34 +0200 Subject: [PATCH 1/6] Add CronjobService and permission Fix build error Fix test Add part of cronjob caveat implementation Register cronjob caveat Add more caveat validations and tests Fix imports after rebase Fix conflict-merge issue after rebase Fix validation functions build issue Add configuration for caveat mapper Update CronjobService and add tests Add some more unit tests for CronjobService Update CronjobService to become CronjobController Add some refactoring and new features for the CronjobController Refactor and clean up More refactor and tests Add missing docstring Improve test coverage for CronjobController Improve test coverage for utils Improve test coverage for CronjobController Fix execution-environments commands Fix jest threshold after rebase Refactor directory structure Add support for object-like cron expressions Fix and rework a couple of things for structural expressions Add some refactoring & test improvements Code refactoring Add way to unregister all snaps on controller destroy method call Change timers and snapIds to be private with getters Remove eslint ignore instructions where they are not needed anymore Redesign the test with daily timeout check (schedule skip) Refactor unit tests --- packages/controllers/package.json | 1 + .../src/cronjob/CronjobController.test.ts | 423 ++++++++++++++++++ .../src/cronjob/CronjobController.ts | 360 +++++++++++++++ .../src/snaps/endowments/cronjob.test.ts | 223 +++++++++ .../src/snaps/endowments/cronjob.ts | 146 ++++++ .../controllers/src/snaps/endowments/enum.ts | 1 + .../controllers/src/snaps/endowments/index.ts | 8 + .../controllers/src/test-utils/controller.ts | 63 +++ .../src/common/commands.ts | 1 + .../src/common/validation.ts | 1 + packages/types/src/types.d.ts | 3 + packages/utils/package.json | 1 + packages/utils/src/caveats.test.ts | 15 + packages/utils/src/caveats.ts | 5 + packages/utils/src/cronjob.test.ts | 173 +++++++ packages/utils/src/cronjob.ts | 101 +++++ packages/utils/src/index.ts | 1 + packages/utils/src/types.ts | 1 + yarn.lock | 18 + 19 files changed, 1545 insertions(+) create mode 100644 packages/controllers/src/cronjob/CronjobController.test.ts create mode 100644 packages/controllers/src/cronjob/CronjobController.ts create mode 100644 packages/controllers/src/snaps/endowments/cronjob.test.ts create mode 100644 packages/controllers/src/snaps/endowments/cronjob.ts create mode 100644 packages/utils/src/caveats.test.ts create mode 100644 packages/utils/src/cronjob.test.ts create mode 100644 packages/utils/src/cronjob.ts diff --git a/packages/controllers/package.json b/packages/controllers/package.json index 1d1c94c1bc..b298b8e682 100644 --- a/packages/controllers/package.json +++ b/packages/controllers/package.json @@ -42,6 +42,7 @@ "@metamask/utils": "^3.1.0", "@xstate/fsm": "^2.0.0", "concat-stream": "^2.0.0", + "cron-parser": "^4.5.0", "eth-rpc-errors": "^4.0.2", "gunzip-maybe": "^1.4.2", "immer": "^9.0.6", diff --git a/packages/controllers/src/cronjob/CronjobController.test.ts b/packages/controllers/src/cronjob/CronjobController.test.ts new file mode 100644 index 0000000000..67039d1b94 --- /dev/null +++ b/packages/controllers/src/cronjob/CronjobController.test.ts @@ -0,0 +1,423 @@ +import { + SnapCaveatType, + HandlerType, + deepClone, + TruncatedSnap, +} from '@metamask/snap-utils'; +import { + MOCK_ORIGIN, + MOCK_SNAP_ID, + getTruncatedSnap, +} from '@metamask/snap-utils/test-utils'; +import { Duration, inMilliseconds } from '@metamask/utils'; +import { parseExpression } from 'cron-parser'; +import { SnapEndowments } from '../snaps'; +import { + getRestrictedCronjobControllerMessenger, + getRootCronjobControllerMessenger, +} from '../test-utils'; +import { CronjobController, DAILY_TIMEOUT } from './CronjobController'; + +const MOCK_CRONJOB_PERMISSION = { + caveats: [ + { + type: SnapCaveatType.SnapCronjob, + value: { + jobs: [ + { + expression: { + minute: '*', + hour: '*', + dayOfMonth: '*', + month: '*', + dayOfWeek: '*', + }, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + { + expression: '* * * * *', + request: { + method: 'exampleMethodTwo', + params: ['p1'], + }, + }, + ], + }, + }, + ], + date: 1664187844588, + id: 'izn0WGUO8cvq_jqvLQuQP', + invoker: MOCK_ORIGIN, + parentCapability: SnapEndowments.Cronjob, +}; + +describe('CronjobController', () => { + beforeAll(() => { + jest.useFakeTimers(); + }); + + it('registers a cronjob', async () => { + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + const callActionMock = jest + .spyOn(controllerMessenger, 'call') + .mockImplementation((method) => { + if (method === 'SnapController:getAll') { + return [getTruncatedSnap()]; + } else if (method === 'PermissionController:getPermissions') { + return { [SnapEndowments.Cronjob]: MOCK_CRONJOB_PERMISSION } as any; + } + return false; + }); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + await cronjobController.register(MOCK_SNAP_ID); + + expect(callActionMock).toHaveBeenCalledWith( + 'PermissionController:getPermissions', + MOCK_SNAP_ID, + ); + + jest.advanceTimersByTime(inMilliseconds(1, Duration.Minute)); + + expect(callActionMock).toHaveBeenNthCalledWith( + 4, + 'SnapController:handleRequest', + { + snapId: MOCK_SNAP_ID, + origin: '', + handler: HandlerType.OnCronjob, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ); + + cronjobController.destroy(); + }); + + it('unregisters a cronjob', async () => { + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + const callActionMock = jest + .spyOn(controllerMessenger, 'call') + .mockImplementation((method) => { + if (method === 'SnapController:getAll') { + return [getTruncatedSnap()]; + } else if (method === 'PermissionController:getPermissions') { + return { [SnapEndowments.Cronjob]: MOCK_CRONJOB_PERMISSION } as any; + } + return false; + }); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + await cronjobController.register(MOCK_SNAP_ID); + await cronjobController.unregister(MOCK_SNAP_ID); + + jest.advanceTimersByTime(inMilliseconds(1, Duration.Minute)); + + expect(callActionMock).not.toHaveBeenNthCalledWith( + 4, + 'SnapController:handleRequest', + { + snapId: MOCK_SNAP_ID, + origin: '', + handler: HandlerType.OnCronjob, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ); + + cronjobController.destroy(); + }); + + it('executes cronjobs that were missed during daily check in', async () => { + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + const callActionMock = jest + .spyOn(controllerMessenger, 'call') + .mockImplementation((method) => { + if (method === 'SnapController:getAll') { + return [getTruncatedSnap()]; + } else if (method === 'PermissionController:getPermissions') { + return { [SnapEndowments.Cronjob]: MOCK_CRONJOB_PERMISSION } as any; + } + return false; + }); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + // Update state manually for test + // eslint-disable-next-line dot-notation + cronjobController['update'](() => { + return { + jobs: { + [`${MOCK_SNAP_ID}-0`]: { lastRun: 0 }, + }, + }; + }); + + await cronjobController.dailyCheckIn(); + + jest.advanceTimersByTime(inMilliseconds(24, Duration.Hour)); + + expect(callActionMock).toHaveBeenNthCalledWith( + 5, + 'SnapController:handleRequest', + { + snapId: MOCK_SNAP_ID, + origin: '', + handler: HandlerType.OnCronjob, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ); + + cronjobController.destroy(); + }); + + it('does not schedule cronjob that is too far in the future', async () => { + // Ensure that Cronjob will not yet be scheduled if it reaches DAILY_TIMEOUT + // Make expression so the schedule is on some complex date + let cronExpression = '59 23 29 2 *'; // At 11:59pm on February 29th + // But also ensure that it's not very close so the test doesn't fail + const parsed = parseExpression(cronExpression); + const next = parsed.next(); + const now = new Date(); + const ms = next.getTime() - now.getTime(); + // So, if the scheduled date is within the range of a daily timeout, + // jump over to some other far date by redefining cron expression + if (ms < DAILY_TIMEOUT) { + cronExpression = '59 23 1 1 *'; // At 11:59pm on January 1st + } + + const MOCK_TOO_FAR_CRONJOB_PERMISSION = deepClone(MOCK_CRONJOB_PERMISSION); + MOCK_TOO_FAR_CRONJOB_PERMISSION.caveats[0].value = { + jobs: [ + { + expression: cronExpression, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ], + }; + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + const callActionMock = jest + .spyOn(controllerMessenger, 'call') + .mockImplementation((method) => { + if (method === 'SnapController:getAll') { + return [getTruncatedSnap()]; + } else if (method === 'PermissionController:getPermissions') { + return { + [SnapEndowments.Cronjob]: MOCK_TOO_FAR_CRONJOB_PERMISSION, + } as any; + } + return false; + }); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + await cronjobController.register(MOCK_SNAP_ID); + + expect(callActionMock).toHaveBeenCalledWith( + 'PermissionController:getPermissions', + MOCK_SNAP_ID, + ); + + expect(cronjobController.timers.size).toBe(0); + + cronjobController.destroy(); + }); + + it('handles SnapInstalled event', async () => { + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + jest.spyOn(controllerMessenger, 'call').mockImplementation((method) => { + if (method === 'SnapController:getAll') { + return [getTruncatedSnap()]; + } else if (method === 'PermissionController:getPermissions') { + return { [SnapEndowments.Cronjob]: MOCK_CRONJOB_PERMISSION } as any; + } + return false; + }); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + const snapInfo: TruncatedSnap = { + blocked: false, + enabled: true, + id: MOCK_SNAP_ID, + initialPermissions: {}, + permissionName: '', + version: '', + }; + // eslint-disable-next-line dot-notation + await cronjobController['_handleEventSnapInstalled'](snapInfo); + + expect(cronjobController.snapIds.size).toBe(2); + + cronjobController.destroy(); + }); + + it('handles SnapRemoved event', async () => { + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + jest.spyOn(controllerMessenger, 'call').mockImplementation((method) => { + if (method === 'SnapController:getAll') { + return [getTruncatedSnap()]; + } else if (method === 'PermissionController:getPermissions') { + return { [SnapEndowments.Cronjob]: MOCK_CRONJOB_PERMISSION } as any; + } + return false; + }); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + await cronjobController.register(MOCK_SNAP_ID); + + const snapInfo: TruncatedSnap = { + blocked: false, + enabled: true, + id: MOCK_SNAP_ID, + initialPermissions: {}, + permissionName: '', + version: '', + }; + // eslint-disable-next-line dot-notation + cronjobController['_handleEventSnapRemoved'](snapInfo); + + expect(cronjobController.snapIds.size).toBe(0); + + cronjobController.destroy(); + }); + + it('handles SnapUpdated event', async () => { + const MOCK_ANOTHER_CRONJOB_PERMISSION = deepClone(MOCK_CRONJOB_PERMISSION); + MOCK_ANOTHER_CRONJOB_PERMISSION.caveats[0].value = { + jobs: [ + { + expression: '*/15 * * * *', + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ], + }; + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + jest + .spyOn(controllerMessenger, 'call') + .mockResolvedValueOnce([getTruncatedSnap()]) + .mockResolvedValueOnce({ + [SnapEndowments.Cronjob]: MOCK_CRONJOB_PERMISSION, + }) + .mockResolvedValueOnce({ + [SnapEndowments.Cronjob]: MOCK_ANOTHER_CRONJOB_PERMISSION, + }); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + await cronjobController.register(MOCK_SNAP_ID); + + expect(cronjobController.snapIds.size).toBe(2); + + expect(cronjobController.timers.size).toBe(2); + + const snapInfo: TruncatedSnap = { + blocked: false, + enabled: true, + id: MOCK_SNAP_ID, + initialPermissions: {}, + permissionName: '', + version: '', + }; + // eslint-disable-next-line dot-notation + await cronjobController['_handleEventSnapUpdated'](snapInfo); + + expect(cronjobController.snapIds.size).toBe(1); + + expect(cronjobController.timers.size).toBe(1); + + cronjobController.destroy(); + }); + + it('removes all jobs and schedules after controller destroy is called', async () => { + const rootMessenger = getRootCronjobControllerMessenger(); + const controllerMessenger = + getRestrictedCronjobControllerMessenger(rootMessenger); + + const callActionMock = jest + .spyOn(controllerMessenger, 'call') + .mockImplementation((method) => { + if (method === 'SnapController:getAll') { + return [getTruncatedSnap()]; + } else if (method === 'PermissionController:getPermissions') { + return { [SnapEndowments.Cronjob]: MOCK_CRONJOB_PERMISSION } as any; + } + return false; + }); + + const cronjobController = new CronjobController({ + messenger: controllerMessenger, + }); + + await cronjobController.register(MOCK_SNAP_ID); + + expect(callActionMock).toHaveBeenCalledWith( + 'PermissionController:getPermissions', + MOCK_SNAP_ID, + ); + + expect(cronjobController.snapIds.size).toBe(2); + + expect(cronjobController.timers.size).toBe(2); + + cronjobController.destroy(); + + expect(cronjobController.snapIds.size).toBe(0); + + expect(cronjobController.timers.size).toBe(0); + }); +}); diff --git a/packages/controllers/src/cronjob/CronjobController.ts b/packages/controllers/src/cronjob/CronjobController.ts new file mode 100644 index 0000000000..ff9269ff60 --- /dev/null +++ b/packages/controllers/src/cronjob/CronjobController.ts @@ -0,0 +1,360 @@ +import { + BaseControllerV2 as BaseController, + RestrictedControllerMessenger, + HasPermission, + GetPermissions, +} from '@metamask/controllers'; +import { + HandlerType, + SnapId, + TruncatedSnap, + CronjobSpecification, + flatten, + parseCronExpression, +} from '@metamask/snap-utils'; +import { Duration, inMilliseconds } from '@metamask/utils'; +import { + GetAllSnaps, + getRunnableSnaps, + GetSnap, + HandleSnapRequest, + SnapAdded, + SnapBlocked, + SnapEndowments, + SnapInstalled, + SnapRemoved, + SnapTerminated, + SnapUnblocked, + SnapUpdated, +} from '..'; +import { getCronjobCaveatJobs } from '../snaps/endowments/cronjob'; +import { Timer } from '../snaps/Timer'; + +export type CronjobControllerActions = + | GetAllSnaps + | GetSnap + | HandleSnapRequest + | HasPermission + | GetPermissions; + +export type CronjobControllerEvents = + | SnapAdded + | SnapBlocked + | SnapInstalled + | SnapRemoved + | SnapUnblocked + | SnapUpdated + | SnapTerminated; + +export type CronjobControllerMessenger = RestrictedControllerMessenger< + 'CronjobController', + CronjobControllerActions, + CronjobControllerEvents, + CronjobControllerActions['type'], + CronjobControllerEvents['type'] +>; + +export const DAILY_TIMEOUT = inMilliseconds(24, Duration.Hour); + +export type CronjobControllerArgs = { + messenger: CronjobControllerMessenger; + /** + * Persisted state that will be used for rehydration. + */ + state?: CronjobControllerState; +}; + +export type Cronjob = { + timer?: Timer; + id: string; + snapId: SnapId; +} & CronjobSpecification; + +export type StoredJobInformation = { + lastRun: number; +}; + +export type CronjobControllerState = { + jobs: Record; +}; + +const controllerName = 'CronjobController'; + +/** + * Use this controller to register and schedule periodically executed jobs + * using RPC method hooks. + */ +export class CronjobController extends BaseController< + typeof controllerName, + CronjobControllerState, + CronjobControllerMessenger +> { + #messenger: CronjobControllerMessenger; + + #dailyTimer!: Timer; + + #timers: Map; + + // Mapping from jobId to snapId + #snapIds: Map; + + constructor({ messenger, state }: CronjobControllerArgs) { + super({ + messenger, + metadata: { + jobs: { persist: true, anonymous: false }, + }, + name: controllerName, + state: { + jobs: {}, + ...state, + }, + }); + this.#timers = new Map(); + this.#snapIds = new Map(); + this.#messenger = messenger; + this.dailyCheckIn(); + + this._handleEventSnapInstalled = this._handleEventSnapInstalled.bind(this); + this._handleEventSnapRemoved = this._handleEventSnapRemoved.bind(this); + this._handleEventSnapUpdated = this._handleEventSnapUpdated.bind(this); + + // Subscribe to Snap events + this.messagingSystem.subscribe( + 'SnapController:snapInstalled', + this._handleEventSnapInstalled, + ); + + this.messagingSystem.subscribe( + 'SnapController:snapRemoved', + this._handleEventSnapRemoved, + ); + + this.messagingSystem.subscribe( + 'SnapController:snapUpdated', + this._handleEventSnapUpdated, + ); + } + + public get timers() { + return this.#timers; + } + + public get snapIds() { + return this.#snapIds; + } + + /** + * Retrieve all cronjob specifications for all runnable snaps. + * + * @returns Array of Cronjob specifications. + */ + private async getAllJobs(): Promise { + const snaps = await this.messagingSystem.call('SnapController:getAll'); + const filteredSnaps = getRunnableSnaps(snaps); + + const jobs = await Promise.all( + filteredSnaps.map((snap) => this.getSnapJobs(snap.id)), + ); + return flatten(jobs).filter((job) => job !== undefined) as Cronjob[]; + } + + /** + * Retrieve all Cronjob specifications for a Snap. + * + * @param snapId - ID of a Snap. + * @returns Array of Cronjob specifications. + */ + private async getSnapJobs(snapId: SnapId): Promise { + const permissions = await this.#messenger.call( + 'PermissionController:getPermissions', + snapId, + ); + + const permission = permissions?.[SnapEndowments.Cronjob]; + const definitions = getCronjobCaveatJobs(permission); + + return definitions?.map((definition, idx) => { + return { ...definition, id: `${snapId}-${idx}`, snapId }; + }); + } + + /** + * Register cron jobs for a given snap by getting specification from a permission caveats. + * Once registered, each job will be scheduled. + * + * @param snapId - ID of a snap. + */ + async register(snapId: SnapId) { + const jobs = await this.getSnapJobs(snapId); + jobs?.forEach((job) => this.schedule(job)); + } + + /** + * Schedule a new job. + * This will interpret the cron expression and tell the timer to execute the job + * at the next suitable point in time. + * Job last run state will be initialized afterwards. + * + * Note: Schedule will be skipped if the job's execution time is too far in the future and + * will be revisited on a daily check. + * + * @param job - Cronjob specification. + */ + private schedule(job: Cronjob) { + if (this.#timers.has(job.id)) { + return; + } + const parsed = parseCronExpression(job.expression); + const next = parsed.next(); + const now = new Date(); + const ms = next.getTime() - now.getTime(); + + // Don't schedule this job yet as it is too far in the future + if (ms > DAILY_TIMEOUT) { + return; + } + + const timer = new Timer(ms); + timer.start(() => { + this.executeCronjob(job); + this.#timers.delete(job.id); + this.schedule(job); + }); + + this.updateJobLastRunState(job.id, 0); // 0 for init, never ran actually + this.#timers.set(job.id, timer); + this.#snapIds.set(job.id, job.snapId); + } + + /** + * Execute job. + * + * @param job - Cronjob specification. + */ + private executeCronjob(job: Cronjob) { + this.updateJobLastRunState(job.id, Date.now()); + this.#messenger.call('SnapController:handleRequest', { + snapId: job.snapId, + origin: '', + handler: HandlerType.OnCronjob, + request: job.request, + }); + } + + /** + * Unregister all jobs related to the given snapId. + * + * @param snapId - ID of a snap. + */ + unregister(snapId: SnapId) { + const jobs = [...this.#snapIds.entries()].filter( + ([_, jobSnapId]) => jobSnapId === snapId, + ); + + if (jobs.length) { + jobs.forEach(([id]) => { + const timer = this.#timers.get(id); + if (timer) { + timer.cancel(); + this.#timers.delete(id); + this.#snapIds.delete(id); + } + }); + } + } + + /** + * Update time of a last run for the Cronjob specified by ID. + * + * @param jobId - ID of a cron job. + * @param lastRun - Unix timestamp when the job was last ran. + */ + private updateJobLastRunState(jobId: string, lastRun: number) { + this.update((state) => { + state.jobs[jobId] = { + lastRun, + }; + }); + } + + /** + * Runs every 24 hours to check if new jobs need to be scheduled. + * + * This is necesary for longer running jobs that execute with more than 24 hours between them. + */ + async dailyCheckIn() { + const jobs = await this.getAllJobs(); + jobs.forEach((job) => { + const parsed = parseCronExpression(job.expression); + const lastRun = this.state.jobs[job.id]?.lastRun; + // If a job was supposed to run while we were shut down but wasn't we run it now + if ( + lastRun !== undefined && + parsed.hasPrev() && + parsed.prev().getTime() > lastRun + ) { + this.executeCronjob(job); + } + + // Try scheduling, will fail if an existing scheduled job is found + this.schedule(job); + }); + this.#dailyTimer = new Timer(DAILY_TIMEOUT); + this.#dailyTimer.start(() => this.dailyCheckIn()); + } + + /** + * Run controller teardown process and unsubscribe from Snap events. + */ + destroy() { + super.destroy(); + + this.messagingSystem.unsubscribe( + 'SnapController:snapInstalled', + this._handleEventSnapInstalled, + ); + + this.messagingSystem.unsubscribe( + 'SnapController:snapRemoved', + this._handleEventSnapRemoved, + ); + + this.messagingSystem.unsubscribe( + 'SnapController:snapUpdated', + this._handleEventSnapUpdated, + ); + + this.#snapIds.forEach((snapId) => { + this.unregister(snapId); + }); + } + + /** + * Handle cron jobs on 'snapInstalled' event. + * + * @param snap - Basic Snap information. + */ + private async _handleEventSnapInstalled(snap: TruncatedSnap) { + await this.register(snap.id); + } + + /** + * Handle cron jobs on 'snapRemoved' event. + * + * @param snap - Basic Snap information. + */ + private _handleEventSnapRemoved(snap: TruncatedSnap) { + this.unregister(snap.id); + } + + /** + * Handle cron jobs on 'snapUpdated' event. + * + * @param snap - Basic Snap information. + */ + private async _handleEventSnapUpdated(snap: TruncatedSnap) { + this.unregister(snap.id); + await this.register(snap.id); + } +} diff --git a/packages/controllers/src/snaps/endowments/cronjob.test.ts b/packages/controllers/src/snaps/endowments/cronjob.test.ts new file mode 100644 index 0000000000..8f3a96d286 --- /dev/null +++ b/packages/controllers/src/snaps/endowments/cronjob.test.ts @@ -0,0 +1,223 @@ +import { Caveat, PermissionType } from '@metamask/controllers'; +import { SnapCaveatType } from '@metamask/snap-utils'; +import { + getCronjobCaveatMapper, + cronjobEndowmentBuilder, + validateCronjobCaveat, + cronjobCaveatSpecifications, +} from './cronjob'; +import { SnapEndowments } from '.'; + +describe('endowment:cronjob', () => { + describe('specificationBuilder', () => { + it('builds the expected permission specification', () => { + const specification = cronjobEndowmentBuilder.specificationBuilder({}); + expect(specification).toStrictEqual({ + permissionType: PermissionType.Endowment, + targetKey: SnapEndowments.Cronjob, + endowmentGetter: expect.any(Function), + allowedCaveats: [SnapCaveatType.SnapCronjob], + }); + + expect(specification.endowmentGetter()).toBeUndefined(); + }); + }); + + describe('cronjobCaveatMapper', () => { + it('returns a caveat value for the objects of cronjob specification', () => { + expect( + getCronjobCaveatMapper({ + jobs: [ + { + expression: '* * * * *', + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + { + expression: { + minute: '*', + hour: '*', + dayOfMonth: '*', + month: '*', + dayOfWeek: '*', + }, + request: { + method: 'exampleMethodTwo', + params: ['p1', 'p2', 'p3'], + }, + }, + ], + }), + ).toStrictEqual({ + caveats: [ + { + type: SnapCaveatType.SnapCronjob, + value: { + jobs: [ + { + expression: '* * * * *', + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + { + expression: { + minute: '*', + hour: '*', + dayOfMonth: '*', + month: '*', + dayOfWeek: '*', + }, + request: { + method: 'exampleMethodTwo', + params: ['p1', 'p2', 'p3'], + }, + }, + ], + }, + }, + ], + }); + }); + }); +}); + +describe('validateCronjobCaveat', () => { + it('should not throw an error when provided specification is valid', () => { + const caveat: Caveat = { + type: SnapCaveatType.SnapCronjob, + value: { + jobs: [ + { + expression: { + minute: '*', + hour: '*', + dayOfMonth: '*', + month: '*', + dayOfWeek: '*', + }, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + { + expression: '* * * * *', + request: { + method: 'snapMethod', + params: [], + }, + }, + ], + }, + }; + + expect(() => validateCronjobCaveat(caveat)).not.toThrow(); + }); + + it('should throw if caveat has no proper value', () => { + const caveat: Caveat = { + type: SnapCaveatType.SnapCronjob, + value: {}, + }; + + expect(() => validateCronjobCaveat(caveat)).toThrow( + `Expected a plain object.`, + ); + }); + + it('should throw an error when cron specification is missing', () => { + const caveat: Caveat = { + type: SnapCaveatType.SnapCronjob, + value: { + jobs: [ + { + expression: '* * * * *', + request: undefined, + }, + ], + }, + }; + + expect(() => validateCronjobCaveat(caveat)).toThrow( + 'Expected a valid cronjob specification array.', + ); + }); +}); + +describe('CronjobCaveatSpecifications', () => { + describe('validator', () => { + it('does not throw when parameters are valid', () => { + const params = { + jobs: [ + { + expression: { + minute: '*', + hour: '*', + dayOfMonth: '*', + month: '*', + dayOfWeek: '*', + }, + request: { method: 'snapMethod', params: [] }, + }, + { + expression: '*/2 * * * *', + request: { method: 'anotherSnapMethod', params: ['a', 'b', 'c'] }, + }, + ], + }; + + expect(() => + cronjobCaveatSpecifications[SnapCaveatType.SnapCronjob].validator?.({ + type: SnapCaveatType.SnapCronjob, + value: params, + }), + ).not.toThrow(); + }); + + it('throws if the expression parameter value is invalid', () => { + const invalidParams = { + jobs: [ + { + expression: '* * * * * * * * *', + request: { method: 'snapMethod', params: [] }, + }, + ], + }; + + expect(() => + cronjobCaveatSpecifications[SnapCaveatType.SnapCronjob].validator?.({ + type: SnapCaveatType.SnapCronjob, + value: invalidParams, + }), + ).toThrow('Expected a valid cronjob specification array.'); + }); + + it('throws if the expression parameter value is invalid and provided in object format', () => { + const invalidParams = { + jobs: [ + { + expression: { + minute: 'aaa', + hour: 'b', + dayOfMonth: 'c', + month: 'd', + dayOfWeek: 'e', + }, + request: { method: 'snapMethod', params: [] }, + }, + ], + }; + + expect(() => + cronjobCaveatSpecifications[SnapCaveatType.SnapCronjob].validator?.({ + type: SnapCaveatType.SnapCronjob, + value: invalidParams, + }), + ).toThrow('Expected a valid cronjob specification array.'); + }); + }); +}); diff --git a/packages/controllers/src/snaps/endowments/cronjob.ts b/packages/controllers/src/snaps/endowments/cronjob.ts new file mode 100644 index 0000000000..e8ac158b19 --- /dev/null +++ b/packages/controllers/src/snaps/endowments/cronjob.ts @@ -0,0 +1,146 @@ +import { + PermissionSpecificationBuilder, + PermissionType, + EndowmentGetterParams, + ValidPermissionSpecification, + PermissionConstraint, + Caveat, + CaveatSpecificationConstraint, +} from '@metamask/controllers'; +import { + assert, + hasProperty, + isPlainObject, + Json, + NonEmptyArray, +} from '@metamask/utils'; +import { + SnapCaveatType, + CronjobSpecification, + isCronjobSpecificationArray, +} from '@metamask/snap-utils'; +import { ethErrors } from 'eth-rpc-errors'; +import { SnapEndowments } from './enum'; + +const permissionName = SnapEndowments.Cronjob; + +type CronjobEndowmentSpecification = ValidPermissionSpecification<{ + permissionType: PermissionType.Endowment; + targetKey: typeof permissionName; + endowmentGetter: (_options?: any) => undefined; + allowedCaveats: Readonly> | null; +}>; + +/** + * `endowment:cronjob` returns nothing; it is intended to be used as a flag to determine whether the snap wants to run cronjobs. + * + * @param _builderOptions - Optional specification builder options. + * @returns The specification for the cronjob endowment. + */ +const specificationBuilder: PermissionSpecificationBuilder< + PermissionType.Endowment, + any, + CronjobEndowmentSpecification +> = (_builderOptions?: any) => { + return { + permissionType: PermissionType.Endowment, + targetKey: permissionName, + allowedCaveats: [SnapCaveatType.SnapCronjob], + endowmentGetter: (_getterOptions?: EndowmentGetterParams) => undefined, + }; +}; + +export const cronjobEndowmentBuilder = Object.freeze({ + targetKey: 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 getCronjobCaveatMapper( + value: Json, +): Pick { + return { + caveats: [ + { + type: SnapCaveatType.SnapCronjob, + value, + }, + ], + }; +} + +/** + * Getter function to get the cronjobs from a permission. + * + * This does basic validation of the caveat, but does not validate the type or + * value of the namespaces object itself, as this is handled by the + * `PermissionsController` when the permission is requested. + * + * @param permission - The permission to get the keyring namespaces from. + * @returns The cronjobs, or `null` if the permission does not have a + * cronjob caveat. + */ +export function getCronjobCaveatJobs( + permission?: PermissionConstraint, +): CronjobSpecification[] | null { + if (!permission?.caveats) { + return null; + } + + assert(permission.caveats.length === 1); + assert(permission.caveats[0].type === SnapCaveatType.SnapCronjob); + + const caveat = permission.caveats[0] as Caveat; + + return (caveat.value?.jobs as CronjobSpecification[]) ?? null; +} + +/** + * Validate the cronjob specification values associated with a caveat. + * This validates that the value is a non-empty array with valid + * cronjob expression and request object. + * + * @param caveat - The caveat to validate. + * @throws If the value is invalid. + */ +export function validateCronjobCaveat(caveat: Caveat) { + if (!hasProperty(caveat, 'value') || !isPlainObject(caveat.value)) { + throw ethErrors.rpc.invalidParams({ + message: 'Expected a plain object.', + }); + } + + const { value } = caveat; + + if (!hasProperty(value, 'jobs') || !isPlainObject(value)) { + throw ethErrors.rpc.invalidParams({ + message: 'Expected a plain object.', + }); + } + + if (!isCronjobSpecificationArray(value.jobs)) { + throw ethErrors.rpc.invalidParams({ + message: 'Expected a valid cronjob specification array.', + }); + } +} + +/** + * Caveat specification for the Cronjob. + */ +export const cronjobCaveatSpecifications: Record< + SnapCaveatType.SnapCronjob, + CaveatSpecificationConstraint +> = { + [SnapCaveatType.SnapCronjob]: Object.freeze({ + type: SnapCaveatType.SnapCronjob, + validator: (caveat) => validateCronjobCaveat(caveat), + }), +}; diff --git a/packages/controllers/src/snaps/endowments/enum.ts b/packages/controllers/src/snaps/endowments/enum.ts index 312389a22c..fd9f5619ec 100644 --- a/packages/controllers/src/snaps/endowments/enum.ts +++ b/packages/controllers/src/snaps/endowments/enum.ts @@ -3,4 +3,5 @@ export enum SnapEndowments { LongRunning = 'endowment:long-running', TransactionInsight = 'endowment:transaction-insight', Keyring = 'endowment:keyring', + Cronjob = 'endowment:cronjob', } diff --git a/packages/controllers/src/snaps/endowments/index.ts b/packages/controllers/src/snaps/endowments/index.ts index ec9e88e32a..eeb0a1b727 100644 --- a/packages/controllers/src/snaps/endowments/index.ts +++ b/packages/controllers/src/snaps/endowments/index.ts @@ -1,5 +1,10 @@ import { PermissionConstraint } from '@metamask/controllers'; import { Json } from '@metamask/utils'; +import { + cronjobCaveatSpecifications, + cronjobEndowmentBuilder, + getCronjobCaveatMapper, +} from './cronjob'; import { longRunningEndowmentBuilder } from './long-running'; import { networkAccessEndowmentBuilder } from './network-access'; import { transactionInsightEndowmentBuilder } from './transaction-insight'; @@ -15,10 +20,12 @@ export const endowmentPermissionBuilders = { [transactionInsightEndowmentBuilder.targetKey]: transactionInsightEndowmentBuilder, [keyringEndowmentBuilder.targetKey]: keyringEndowmentBuilder, + [cronjobEndowmentBuilder.targetKey]: cronjobEndowmentBuilder, } as const; export const endowmentCaveatSpecifications = { ...keyringCaveatSpecifications, + ...cronjobCaveatSpecifications, }; export const endowmentCaveatMappers: Record< @@ -26,6 +33,7 @@ export const endowmentCaveatMappers: Record< (value: Json) => Pick > = { [keyringEndowmentBuilder.targetKey]: getKeyringCaveatMapper, + [cronjobEndowmentBuilder.targetKey]: getCronjobCaveatMapper, }; export * from './enum'; diff --git a/packages/controllers/src/test-utils/controller.ts b/packages/controllers/src/test-utils/controller.ts index 10d737ce8d..de873a4dda 100644 --- a/packages/controllers/src/test-utils/controller.ts +++ b/packages/controllers/src/test-utils/controller.ts @@ -10,6 +10,10 @@ import { SnapControllerEvents, SnapEndowments, } from '../snaps'; +import { + CronjobControllerActions, + CronjobControllerEvents, +} from '../cronjob/CronjobController'; import { getNodeEES, getNodeEESMessenger } from './execution-environment'; export const getControllerMessenger = () => @@ -201,3 +205,62 @@ export const getPersistedSnapsState = ( {} as PersistedSnapControllerState['snaps'], ); }; + +// Mock controller messenger for Cronjob Controller +export const getRootCronjobControllerMessenger = () => + new ControllerMessenger< + CronjobControllerActions | AllowedActions, + CronjobControllerEvents | AllowedEvents + >(); + +export const getRestrictedCronjobControllerMessenger = ( + messenger: ReturnType< + typeof getRootCronjobControllerMessenger + > = getRootCronjobControllerMessenger(), + mocked = true, +) => { + const m = messenger.getRestricted< + 'CronjobController', + CronjobControllerActions['type'] | AllowedActions['type'], + CronjobControllerEvents['type'] | AllowedEvents['type'] + >({ + name: 'CronjobController', + allowedEvents: [ + 'ExecutionService:unhandledError', + 'ExecutionService:outboundRequest', + 'ExecutionService:outboundResponse', + 'SnapController:snapAdded', + 'SnapController:snapBlocked', + 'SnapController:snapInstalled', + 'SnapController:snapUnblocked', + 'SnapController:snapUpdated', + 'SnapController:snapRemoved', + ], + allowedActions: [ + 'ApprovalController:addRequest', + 'ExecutionService:executeSnap', + 'ExecutionService:terminateAllSnaps', + 'ExecutionService:terminateSnap', + 'ExecutionService:handleRpcRequest', + 'PermissionController:getEndowments', + 'PermissionController:hasPermission', + 'PermissionController:hasPermissions', + 'PermissionController:getPermissions', + 'SnapController:handleRequest', + ], + }); + + if (mocked) { + jest.spyOn(m, 'call').mockImplementation((method, ...args) => { + // Return false for long-running by default, and true for everything else. + if ( + method === 'PermissionController:hasPermission' && + args[1] === SnapEndowments.LongRunning + ) { + return false; + } + return true; + }); + } + return m; +}; diff --git a/packages/execution-environments/src/common/commands.ts b/packages/execution-environments/src/common/commands.ts index c5b607faaa..bd5fd78207 100644 --- a/packages/execution-environments/src/common/commands.ts +++ b/packages/execution-environments/src/common/commands.ts @@ -44,6 +44,7 @@ export function getHandlerArguments( } case HandlerType.OnRpcRequest: + case HandlerType.OnCronjob: case HandlerType.SnapKeyring: return { origin, request }; diff --git a/packages/execution-environments/src/common/validation.ts b/packages/execution-environments/src/common/validation.ts index b3d393bf17..f77e132267 100644 --- a/packages/execution-environments/src/common/validation.ts +++ b/packages/execution-environments/src/common/validation.ts @@ -29,6 +29,7 @@ const VALIDATION_FUNCTIONS = { [HandlerType.OnRpcRequest]: validateFunctionExport, [HandlerType.OnTransaction]: validateFunctionExport, [HandlerType.SnapKeyring]: validateKeyringExport, + [HandlerType.OnCronjob]: validateFunctionExport, }; /** diff --git a/packages/types/src/types.d.ts b/packages/types/src/types.d.ts index 746f9c8806..80a9009f88 100644 --- a/packages/types/src/types.d.ts +++ b/packages/types/src/types.d.ts @@ -18,6 +18,8 @@ export type OnTransactionHandler = (args: { chainId: string; }) => Promise; +export type OnCronjobHandler = SnapRpcHandler; + export type SnapProvider = MetaMaskInpageProvider; // CAIP2 - https://github.com/ChainAgnostic/CAIPs/blob/master/CAIPs/caip-2.md @@ -57,6 +59,7 @@ export interface SnapKeyring { export type SnapFunctionExports = { onRpcRequest?: OnRpcRequestHandler; onTransaction?: OnTransactionHandler; + onCronjob?: OnCronjobHandler; }; export type SnapExports = SnapFunctionExports & { diff --git a/packages/utils/package.json b/packages/utils/package.json index 77e419be79..7743d88a17 100644 --- a/packages/utils/package.json +++ b/packages/utils/package.json @@ -56,6 +56,7 @@ "@metamask/utils": "^3.1.0", "@noble/hashes": "^1.1.3", "@scure/base": "^1.1.1", + "cron-parser": "^4.5.0", "eth-rpc-errors": "^4.0.3", "fast-deep-equal": "^3.1.3", "rfdc": "^1.3.0", diff --git a/packages/utils/src/caveats.test.ts b/packages/utils/src/caveats.test.ts new file mode 100644 index 0000000000..7a2158d592 --- /dev/null +++ b/packages/utils/src/caveats.test.ts @@ -0,0 +1,15 @@ +import { SnapCaveatType } from './caveats'; + +describe('Caveat utilities', () => { + it('exports expected caveats', () => { + expect(SnapCaveatType.PermittedDerivationPaths).toBe( + 'permittedDerivationPaths', + ); + + expect(SnapCaveatType.PermittedCoinTypes).toBe('permittedCoinTypes'); + + expect(SnapCaveatType.SnapKeyring).toBe('snapKeyring'); + + expect(SnapCaveatType.SnapCronjob).toBe('snapCronjob'); + }); +}); diff --git a/packages/utils/src/caveats.ts b/packages/utils/src/caveats.ts index 19a90941ae..ec04ccf6af 100644 --- a/packages/utils/src/caveats.ts +++ b/packages/utils/src/caveats.ts @@ -13,4 +13,9 @@ export enum SnapCaveatType { * Permission to use the Snap keyring API. */ SnapKeyring = 'snapKeyring', + + /** + * Caveat specifying a snap cronjob. + */ + SnapCronjob = 'snapCronjob', } diff --git a/packages/utils/src/cronjob.test.ts b/packages/utils/src/cronjob.test.ts new file mode 100644 index 0000000000..35011de050 --- /dev/null +++ b/packages/utils/src/cronjob.test.ts @@ -0,0 +1,173 @@ +import { + isCronjobSpecification, + isCronjobSpecificationArray, + parseCronExpression, +} from './cronjob'; + +describe('Cronjob Utilities', () => { + describe('isCronjobSpecification', () => { + it('returns true for a valid cronjob specification', () => { + const cronjobSpecification = { + expression: '* * * * *', + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }; + expect(isCronjobSpecification(cronjobSpecification)).toBe(true); + }); + + it('returns true for a valid cronjob specification when object is provided', () => { + const cronjobSpecification = { + expression: { + minute: '*/15', + hour: '1', + dayOfMonth: '3', + month: '6', + dayOfWeek: '*', + }, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }; + expect(isCronjobSpecification(cronjobSpecification)).toBe(true); + }); + + it('returns false for an invalid cronjob specification', () => { + const cronjobSpecification = { + expression: '* * * * * * * * * * *', + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }; + expect(isCronjobSpecification(cronjobSpecification)).toBe(false); + }); + + it('returns false for an invalid cronjob specification when object is provided', () => { + const cronjobSpecification = { + expression: { + minute: 'aaaa', + hour: 'bbbb', + dayOfMonth: 'cccc', + month: 'dddd', + dayOfWeek: 'eeee', + }, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }; + expect(isCronjobSpecification(cronjobSpecification)).toBe(false); + }); + + it('returns true for cronjob specification when object is provided with missing values', () => { + // This case should use '*' by default + const cronjobSpecification = { + expression: { + minute: undefined, + hour: undefined, + dayOfMonth: undefined, + month: undefined, + dayOfWeek: undefined, + }, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }; + expect(isCronjobSpecification(cronjobSpecification)).toBe(true); + }); + }); + + describe('isCronjobSpecificationArray', () => { + it('returns true for a valid cronjob specification array', () => { + const cronjobSpecificationArray = [ + { + expression: '* * * * *', + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ]; + expect(isCronjobSpecificationArray(cronjobSpecificationArray)).toBe(true); + }); + + it('returns true for a valid cronjob specification array when object specification is provided', () => { + const cronjobSpecificationArray = [ + { + expression: { + minute: '*/15', + hour: '1', + dayOfMonth: '3', + month: '6', + dayOfWeek: '*', + }, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ]; + expect(isCronjobSpecificationArray(cronjobSpecificationArray)).toBe(true); + }); + + it('returns false for an invalid cronjob specification array', () => { + const cronjobSpecificationArray = { + expression: '* * * * *', + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }; + expect(isCronjobSpecificationArray(cronjobSpecificationArray)).toBe( + false, + ); + }); + + it('returns false for an invalid cronjob specification array when object specification is provided', () => { + const cronjobSpecificationArray = { + expression: { + minute: 'aaaa', + hour: 'bbbb', + dayOfMonth: 'cccc', + month: 'dddd', + dayOfWeek: 'eeee', + }, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }; + expect(isCronjobSpecificationArray(cronjobSpecificationArray)).toBe( + false, + ); + }); + }); + + describe('parseCronExpression', () => { + it('successfully parses cronjob expression that is provided as an object', () => { + const cronjobExpression = { + minute: '*', + hour: '*', + dayOfMonth: '*', + month: '*', + dayOfWeek: '*', + }; + + const parsedExpression = parseCronExpression(cronjobExpression); + expect(parsedExpression.next()).toBeDefined(); + expect(typeof parsedExpression.next().getTime()).toBe('number'); + }); + + it('successfully parses cronjob expression that is provided as a string', () => { + const cronjobExpression = '* * * * *'; + + const parsedExpression = parseCronExpression(cronjobExpression); + expect(parsedExpression.next()).toBeDefined(); + expect(typeof parsedExpression.next().getTime()).toBe('number'); + }); + }); +}); diff --git a/packages/utils/src/cronjob.ts b/packages/utils/src/cronjob.ts new file mode 100644 index 0000000000..1febd114a3 --- /dev/null +++ b/packages/utils/src/cronjob.ts @@ -0,0 +1,101 @@ +import { JsonRpcRequestStruct } from '@metamask/utils'; +import { + array, + assign, + coerce, + create, + Infer, + object, + omit, + optional, + partial, + pick, + refine, + string, +} from 'superstruct'; +import { parseExpression } from 'cron-parser'; + +export const CronjobRpcRequestStruct = assign( + partial(pick(JsonRpcRequestStruct, ['id', 'jsonrpc'])), + omit(JsonRpcRequestStruct, ['id', 'jsonrpc']), +); +export type CronjobRpcRequest = Infer; + +export const CronExpressionStruct = refine( + coerce( + string(), + object({ + minute: optional(string()), + hour: optional(string()), + dayOfMonth: optional(string()), + month: optional(string()), + dayOfWeek: optional(string()), + }), + (value) => + `${value.minute ?? '*'} ${value.hour ?? '*'} ${value.dayOfMonth ?? '*'} ${ + value.month ?? '*' + } ${value.dayOfWeek ?? '*'}`, + ), + 'CronExpression', + (value) => { + try { + parseExpression(value); + return true; + } catch { + return false; + } + }, +); + +export type CronExpression = Infer; + +/** + * Parses a cron expression. + * + * @param expression - Expression to parse. + * @returns A CronExpression class instance. + */ +export function parseCronExpression(expression: string | object) { + const ensureStringExpression = create(expression, CronExpressionStruct); + return parseExpression(ensureStringExpression); +} + +export const CronjobSpecificationStruct = object({ + expression: CronExpressionStruct, + request: CronjobRpcRequestStruct, +}); +export type CronjobSpecification = Infer; + +/** + * Check if the given value is a {@link CronjobSpecification} object. + * + * @param value - The value to check. + * @returns Whether the value is a valid {@link CronjobSpecification} object. + */ +export function isCronjobSpecification(value: unknown): boolean { + try { + create(value, CronjobSpecificationStruct); + return true; + } catch { + return false; + } +} + +export const CronjobSpecificationArrayStruct = array( + CronjobSpecificationStruct, +); + +/** + * Check if the given value is an array of {@link CronjobSpecification} objects. + * + * @param value - The value to check. + * @returns Whether the value is a valid array of {@link CronjobSpecification} objects. + */ +export function isCronjobSpecificationArray(value: unknown): boolean { + try { + create(value, CronjobSpecificationArrayStruct); + return true; + } catch { + return false; + } +} diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 55fbedda3f..57ee0791af 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -1,5 +1,6 @@ export * from './assert'; export * from './caveats'; +export * from './cronjob'; export * from './deep-clone'; export * from './default-endowments'; export * from './eval'; diff --git a/packages/utils/src/types.ts b/packages/utils/src/types.ts index 941ad6568c..8a5da03de2 100644 --- a/packages/utils/src/types.ts +++ b/packages/utils/src/types.ts @@ -208,6 +208,7 @@ export enum HandlerType { OnRpcRequest = 'onRpcRequest', OnTransaction = 'onTransaction', SnapKeyring = 'keyring', + OnCronjob = 'onCronjob', } export const SNAP_EXPORT_NAMES = Object.values(HandlerType); diff --git a/yarn.lock b/yarn.lock index d1ebfdb1f2..e8f132470e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3018,6 +3018,7 @@ __metadata: "@types/tar-stream": ^2.2.2 "@xstate/fsm": ^2.0.0 concat-stream: ^2.0.0 + cron-parser: ^4.5.0 eslint: ^7.30.0 eslint-config-prettier: ^8.3.0 eslint-plugin-import: ^2.23.4 @@ -3096,6 +3097,7 @@ __metadata: "@scure/base": ^1.1.1 "@types/jest": ^27.5.1 "@types/semver": ^7.3.10 + cron-parser: ^4.5.0 eslint: ^7.30.0 eslint-config-prettier: ^8.3.0 eslint-plugin-import: ^2.23.4 @@ -6964,6 +6966,15 @@ __metadata: languageName: node linkType: hard +"cron-parser@npm:^4.5.0": + version: 4.5.0 + resolution: "cron-parser@npm:4.5.0" + dependencies: + luxon: ^2.4.0 + checksum: 9e5a6d07c2d86fb27b5701067018776aaf9ad4bf9f57a0f02e5f7c33d3d46dd804802ed74c54f001b18db540293633f1904632efdab3466e1f5630b953de26eb + languageName: node + linkType: hard + "cross-fetch@npm:^2.1.0": version: 2.2.6 resolution: "cross-fetch@npm:2.2.6" @@ -12285,6 +12296,13 @@ __metadata: languageName: node linkType: hard +"luxon@npm:^2.4.0": + version: 2.5.0 + resolution: "luxon@npm:2.5.0" + checksum: 2fccce6bbdfc8f13c5a8c148ff045ab3b10f4f80cac28dd92575588fffce9b2d7197096d7fedcc61a6245b59f4233507797f530e63f22b9ae4c425dff2909ae3 + languageName: node + linkType: hard + "magic-string@npm:0.25.1": version: 0.25.1 resolution: "magic-string@npm:0.25.1" From a93f74b3bf4eb1c4bea3418b7328fe2f9b07575c Mon Sep 17 00:00:00 2001 From: david0xd Date: Thu, 27 Oct 2022 18:50:47 +0200 Subject: [PATCH 2/6] Jest configuration updates after rebase & test run --- .../plugin-webpack/src/__snapshots__/plugin.test.ts.snap | 7 +++++++ packages/utils/jest.config.js | 8 ++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/plugin-webpack/src/__snapshots__/plugin.test.ts.snap b/packages/plugin-webpack/src/__snapshots__/plugin.test.ts.snap index 5769a0d5ea..1eab36c650 100644 --- a/packages/plugin-webpack/src/__snapshots__/plugin.test.ts.snap +++ b/packages/plugin-webpack/src/__snapshots__/plugin.test.ts.snap @@ -7,6 +7,13 @@ exports[`SnapsWebpackPlugin applies a transform 1`] = ` })();" `; +exports[`SnapsWebpackPlugin applies a transform 2`] = ` +"(() => { + var __webpack_exports__ = {}; + const foo = 'bar'; +})();" +`; + exports[`SnapsWebpackPlugin forwards the options 1`] = ` "/******/ (() => { diff --git a/packages/utils/jest.config.js b/packages/utils/jest.config.js index 7885fd5e88..b0657f1033 100644 --- a/packages/utils/jest.config.js +++ b/packages/utils/jest.config.js @@ -15,10 +15,10 @@ module.exports = { coverageReporters: ['clover', 'json', 'lcov', 'text', 'json-summary'], coverageThreshold: { global: { - branches: 86.54, - functions: 97.19, - lines: 96.82, - statements: 96.89, + branches: 88.47, + functions: 98.21, + lines: 97.8, + statements: 97.84, }, }, moduleFileExtensions: ['js', 'json', 'jsx', 'ts', 'tsx', 'node'], From a1b7d72fc725cecfe0317b18332b101d9831ef70 Mon Sep 17 00:00:00 2001 From: david0xd Date: Thu, 27 Oct 2022 19:51:03 +0200 Subject: [PATCH 3/6] Remove problematic test snapshot --- .../plugin-webpack/src/__snapshots__/plugin.test.ts.snap | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/plugin-webpack/src/__snapshots__/plugin.test.ts.snap b/packages/plugin-webpack/src/__snapshots__/plugin.test.ts.snap index 1eab36c650..5769a0d5ea 100644 --- a/packages/plugin-webpack/src/__snapshots__/plugin.test.ts.snap +++ b/packages/plugin-webpack/src/__snapshots__/plugin.test.ts.snap @@ -7,13 +7,6 @@ exports[`SnapsWebpackPlugin applies a transform 1`] = ` })();" `; -exports[`SnapsWebpackPlugin applies a transform 2`] = ` -"(() => { - var __webpack_exports__ = {}; - const foo = 'bar'; -})();" -`; - exports[`SnapsWebpackPlugin forwards the options 1`] = ` "/******/ (() => { From a3ccb1011e29aaaca0802628aa61146dc5837d71 Mon Sep 17 00:00:00 2001 From: david0xd Date: Fri, 28 Oct 2022 12:18:14 +0200 Subject: [PATCH 4/6] Change way of calling private methods in tests --- .../src/cronjob/CronjobController.test.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/controllers/src/cronjob/CronjobController.test.ts b/packages/controllers/src/cronjob/CronjobController.test.ts index 67039d1b94..4b455b3855 100644 --- a/packages/controllers/src/cronjob/CronjobController.test.ts +++ b/packages/controllers/src/cronjob/CronjobController.test.ts @@ -284,8 +284,9 @@ describe('CronjobController', () => { permissionName: '', version: '', }; - // eslint-disable-next-line dot-notation - await cronjobController['_handleEventSnapInstalled'](snapInfo); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-expect-error + await cronjobController._handleEventSnapInstalled(snapInfo); expect(cronjobController.snapIds.size).toBe(2); @@ -320,8 +321,9 @@ describe('CronjobController', () => { permissionName: '', version: '', }; - // eslint-disable-next-line dot-notation - cronjobController['_handleEventSnapRemoved'](snapInfo); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-expect-error + cronjobController._handleEventSnapRemoved(snapInfo); expect(cronjobController.snapIds.size).toBe(0); @@ -373,8 +375,9 @@ describe('CronjobController', () => { permissionName: '', version: '', }; - // eslint-disable-next-line dot-notation - await cronjobController['_handleEventSnapUpdated'](snapInfo); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-expect-error + await cronjobController._handleEventSnapUpdated(snapInfo); expect(cronjobController.snapIds.size).toBe(1); From f5f0f855c3246582e9e2dbf6d0e7e500b6ba9504 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 31 Oct 2022 11:12:32 +0100 Subject: [PATCH 5/6] Improve tests --- .../src/cronjob/CronjobController.test.ts | 113 +++++++++++++----- .../src/cronjob/CronjobController.ts | 8 -- 2 files changed, 86 insertions(+), 35 deletions(-) diff --git a/packages/controllers/src/cronjob/CronjobController.test.ts b/packages/controllers/src/cronjob/CronjobController.test.ts index 4b455b3855..f4fbe189f2 100644 --- a/packages/controllers/src/cronjob/CronjobController.test.ts +++ b/packages/controllers/src/cronjob/CronjobController.test.ts @@ -168,8 +168,8 @@ describe('CronjobController', () => { }); // Update state manually for test - // eslint-disable-next-line dot-notation - cronjobController['update'](() => { + // @ts-expect-error Accessing private property + cronjobController.update(() => { return { jobs: { [`${MOCK_SNAP_ID}-0`]: { lastRun: 0 }, @@ -253,7 +253,20 @@ describe('CronjobController', () => { MOCK_SNAP_ID, ); - expect(cronjobController.timers.size).toBe(0); + jest.runOnlyPendingTimers(); + + expect(callActionMock).not.toHaveBeenCalledWith( + 'SnapController:handleRequest', + { + snapId: MOCK_SNAP_ID, + origin: '', + handler: HandlerType.OnCronjob, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ); cronjobController.destroy(); }); @@ -263,7 +276,9 @@ describe('CronjobController', () => { const controllerMessenger = getRestrictedCronjobControllerMessenger(rootMessenger); - jest.spyOn(controllerMessenger, 'call').mockImplementation((method) => { + const callActionMock = jest.spyOn(controllerMessenger, 'call'); + + callActionMock.mockImplementation((method) => { if (method === 'SnapController:getAll') { return [getTruncatedSnap()]; } else if (method === 'PermissionController:getPermissions') { @@ -284,11 +299,24 @@ describe('CronjobController', () => { permissionName: '', version: '', }; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + // @ts-expect-error Accessing private property await cronjobController._handleEventSnapInstalled(snapInfo); - expect(cronjobController.snapIds.size).toBe(2); + jest.advanceTimersByTime(inMilliseconds(1, Duration.Minute)); + + expect(callActionMock).toHaveBeenNthCalledWith( + 4, + 'SnapController:handleRequest', + { + snapId: MOCK_SNAP_ID, + origin: '', + handler: HandlerType.OnCronjob, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ); cronjobController.destroy(); }); @@ -298,7 +326,9 @@ describe('CronjobController', () => { const controllerMessenger = getRestrictedCronjobControllerMessenger(rootMessenger); - jest.spyOn(controllerMessenger, 'call').mockImplementation((method) => { + const callActionMock = jest.spyOn(controllerMessenger, 'call'); + + callActionMock.mockImplementation((method) => { if (method === 'SnapController:getAll') { return [getTruncatedSnap()]; } else if (method === 'PermissionController:getPermissions') { @@ -321,11 +351,24 @@ describe('CronjobController', () => { permissionName: '', version: '', }; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + + // @ts-expect-error Accessing private property cronjobController._handleEventSnapRemoved(snapInfo); - expect(cronjobController.snapIds.size).toBe(0); + jest.advanceTimersByTime(inMilliseconds(1, Duration.Minute)); + + expect(callActionMock).not.toHaveBeenCalledWith( + 'SnapController:handleRequest', + { + snapId: MOCK_SNAP_ID, + origin: '', + handler: HandlerType.OnCronjob, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ); cronjobController.destroy(); }); @@ -347,8 +390,9 @@ describe('CronjobController', () => { const controllerMessenger = getRestrictedCronjobControllerMessenger(rootMessenger); - jest - .spyOn(controllerMessenger, 'call') + const callActionMock = jest.spyOn(controllerMessenger, 'call'); + + callActionMock .mockResolvedValueOnce([getTruncatedSnap()]) .mockResolvedValueOnce({ [SnapEndowments.Cronjob]: MOCK_CRONJOB_PERMISSION, @@ -363,10 +407,6 @@ describe('CronjobController', () => { await cronjobController.register(MOCK_SNAP_ID); - expect(cronjobController.snapIds.size).toBe(2); - - expect(cronjobController.timers.size).toBe(2); - const snapInfo: TruncatedSnap = { blocked: false, enabled: true, @@ -375,13 +415,25 @@ describe('CronjobController', () => { permissionName: '', version: '', }; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-expect-error + + // @ts-expect-error Accessing private property await cronjobController._handleEventSnapUpdated(snapInfo); - expect(cronjobController.snapIds.size).toBe(1); + jest.advanceTimersByTime(inMilliseconds(15, Duration.Minute)); - expect(cronjobController.timers.size).toBe(1); + expect(callActionMock).toHaveBeenNthCalledWith( + 5, + 'SnapController:handleRequest', + { + snapId: MOCK_SNAP_ID, + origin: '', + handler: HandlerType.OnCronjob, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ); cronjobController.destroy(); }); @@ -413,14 +465,21 @@ describe('CronjobController', () => { MOCK_SNAP_ID, ); - expect(cronjobController.snapIds.size).toBe(2); - - expect(cronjobController.timers.size).toBe(2); - cronjobController.destroy(); - expect(cronjobController.snapIds.size).toBe(0); + jest.advanceTimersByTime(inMilliseconds(1, Duration.Minute)); - expect(cronjobController.timers.size).toBe(0); + expect(callActionMock).not.toHaveBeenCalledWith( + 'SnapController:handleRequest', + { + snapId: MOCK_SNAP_ID, + origin: '', + handler: HandlerType.OnCronjob, + request: { + method: 'exampleMethodOne', + params: ['p1'], + }, + }, + ); }); }); diff --git a/packages/controllers/src/cronjob/CronjobController.ts b/packages/controllers/src/cronjob/CronjobController.ts index ff9269ff60..f7043a6700 100644 --- a/packages/controllers/src/cronjob/CronjobController.ts +++ b/packages/controllers/src/cronjob/CronjobController.ts @@ -136,14 +136,6 @@ export class CronjobController extends BaseController< ); } - public get timers() { - return this.#timers; - } - - public get snapIds() { - return this.#snapIds; - } - /** * Retrieve all cronjob specifications for all runnable snaps. * From b5ab1ba62d39e1d37c462f30780766594efc9da2 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 31 Oct 2022 11:35:15 +0100 Subject: [PATCH 6/6] Test onCronjob handler and fix type --- packages/controllers/jest.config.js | 8 ++-- .../execution-environments/jest.config.js | 6 +-- .../src/common/BaseSnapExecutor.test.ts | 38 +++++++++++++++++++ .../src/common/commands.ts | 4 +- packages/types/src/types.d.ts | 4 +- 5 files changed, 51 insertions(+), 9 deletions(-) diff --git a/packages/controllers/jest.config.js b/packages/controllers/jest.config.js index 4626f24b09..605de47e7c 100644 --- a/packages/controllers/jest.config.js +++ b/packages/controllers/jest.config.js @@ -8,10 +8,10 @@ module.exports = { coveragePathIgnorePatterns: ['/node_modules/', '/mocks/', '/test/'], coverageThreshold: { global: { - branches: 84.47, - functions: 95.34, - lines: 94.57, - statements: 94.67, + branches: 84.63, + functions: 95.8, + lines: 94.92, + statements: 95.02, }, }, projects: [ diff --git a/packages/execution-environments/jest.config.js b/packages/execution-environments/jest.config.js index b842133215..ed51358a23 100644 --- a/packages/execution-environments/jest.config.js +++ b/packages/execution-environments/jest.config.js @@ -6,10 +6,10 @@ module.exports = { coverageReporters: ['clover', 'json', 'lcov', 'text', 'json-summary'], coverageThreshold: { global: { - branches: 83.79, + branches: 83.88, functions: 92.48, - lines: 86.53, - statements: 86.7, + lines: 86.55, + statements: 86.72, }, }, moduleFileExtensions: ['js', 'json', 'jsx', 'ts', 'tsx', 'node'], diff --git a/packages/execution-environments/src/common/BaseSnapExecutor.test.ts b/packages/execution-environments/src/common/BaseSnapExecutor.test.ts index 7a63bc1444..a7ee05ebe3 100644 --- a/packages/execution-environments/src/common/BaseSnapExecutor.test.ts +++ b/packages/execution-environments/src/common/BaseSnapExecutor.test.ts @@ -1098,6 +1098,44 @@ describe('BaseSnapExecutor', () => { }); }); + it('supports onCronjob export', async () => { + const CODE = ` + module.exports.onCronjob = ({ request }) => request.params[0]; + `; + const executor = new TestSnapExecutor(); + + await executor.writeCommand({ + jsonrpc: '2.0', + id: 1, + method: 'executeSnap', + params: [FAKE_SNAP_NAME, CODE, []], + }); + + expect(await executor.readCommand()).toStrictEqual({ + jsonrpc: '2.0', + id: 1, + result: 'OK', + }); + + await executor.writeCommand({ + jsonrpc: '2.0', + id: 2, + method: 'snapRpc', + params: [ + FAKE_SNAP_NAME, + HandlerType.OnCronjob, + FAKE_ORIGIN, + { jsonrpc: '2.0', method: 'foo', params: ['bar'] }, + ], + }); + + expect(await executor.readCommand()).toStrictEqual({ + id: 2, + jsonrpc: '2.0', + result: 'bar', + }); + }); + it('blocks Snaps from escaping confinement by using unbound this', async () => { const PAYLOAD = ` console.error("Hack the planet"); diff --git a/packages/execution-environments/src/common/commands.ts b/packages/execution-environments/src/common/commands.ts index bd5fd78207..d2441351e8 100644 --- a/packages/execution-environments/src/common/commands.ts +++ b/packages/execution-environments/src/common/commands.ts @@ -44,10 +44,12 @@ export function getHandlerArguments( } case HandlerType.OnRpcRequest: - case HandlerType.OnCronjob: case HandlerType.SnapKeyring: return { origin, request }; + case HandlerType.OnCronjob: + return { request }; + default: return assertExhaustive(handler); } diff --git a/packages/types/src/types.d.ts b/packages/types/src/types.d.ts index 80a9009f88..c880ff2c8c 100644 --- a/packages/types/src/types.d.ts +++ b/packages/types/src/types.d.ts @@ -18,7 +18,9 @@ export type OnTransactionHandler = (args: { chainId: string; }) => Promise; -export type OnCronjobHandler = SnapRpcHandler; +export type OnCronjobHandler = (args: { + request: JsonRpcRequest; +}) => Promise; export type SnapProvider = MetaMaskInpageProvider;