From 2d8a2a1d1570779aa0120734a8173ded8c793f25 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 6 Aug 2025 14:23:24 +0200 Subject: [PATCH 1/3] fix: Prevent double scheduling events and ensure long-running events work correctly --- .../src/cronjob/CronjobController.test.ts | 62 ++----------------- .../src/cronjob/CronjobController.ts | 7 +++ 2 files changed, 11 insertions(+), 58 deletions(-) diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.test.ts b/packages/snaps-controllers/src/cronjob/CronjobController.test.ts index 3cf36a6ca5..4d5efa5e7d 100644 --- a/packages/snaps-controllers/src/cronjob/CronjobController.test.ts +++ b/packages/snaps-controllers/src/cronjob/CronjobController.test.ts @@ -275,19 +275,14 @@ describe('CronjobController', () => { }, ); - const secondCronjobController = new CronjobController({ - messenger: controllerMessenger, - state: cronjobController.state, - stateManager: getMockStateManager(), - }); + expect(handleRequest).toHaveBeenCalledTimes(1); - secondCronjobController.init(); + jest.advanceTimersByTime(inMilliseconds(26, Duration.Hour)); await new Promise((resolve) => originalProcessNextTick(resolve)); - expect(handleRequest).toHaveBeenCalledTimes(1); + expect(handleRequest).toHaveBeenCalledTimes(2); cronjobController.destroy(); - secondCronjobController.destroy(); }); it('does not schedule cronjob that is too far in the future', () => { @@ -473,55 +468,6 @@ describe('CronjobController', () => { cronjobController.destroy(); }); - it('handles scheduled event close to current time gracefully', () => { - const rootMessenger = getRootCronjobControllerMessenger(); - const controllerMessenger = - getRestrictedCronjobControllerMessenger(rootMessenger); - - const cronjobController = new CronjobController({ - messenger: controllerMessenger, - stateManager: getMockStateManager(), - state: { - events: { - foo: { - id: 'foo', - recurring: false, - date: '2022-01-01T00:00:01.000Z', - schedule: '2022-01-01T00:00:01.000Z', - scheduledAt: new Date().toISOString(), - snapId: MOCK_SNAP_ID, - request: { - method: 'handleEvent', - params: ['p1'], - }, - }, - }, - }, - }); - - rootMessenger.subscribe('CronjobController:stateChange', () => { - jest.advanceTimersByTime(inMilliseconds(2, Duration.Second)); - }); - - cronjobController.init(); - - expect(rootMessenger.call).toHaveBeenNthCalledWith( - 1, - 'SnapController:handleRequest', - { - snapId: MOCK_SNAP_ID, - origin: METAMASK_ORIGIN, - handler: HandlerType.OnCronjob, - request: { - method: 'handleEvent', - params: ['p1'], - }, - }, - ); - - cronjobController.destroy(); - }); - it('handles the `snapInstalled` event', () => { const rootMessenger = getRootCronjobControllerMessenger(); const controllerMessenger = @@ -663,7 +609,7 @@ describe('CronjobController', () => { foo: { id: 'foo', recurring: false, - date: '2022-01-01T01:00:00Z', + date: '2022-01-01T01:00Z', schedule: '2022-01-01T01:00Z', scheduledAt: new Date().toISOString(), snapId: MOCK_SNAP_ID, diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.ts b/packages/snaps-controllers/src/cronjob/CronjobController.ts index a34db56d25..bd9f2d5524 100644 --- a/packages/snaps-controllers/src/cronjob/CronjobController.ts +++ b/packages/snaps-controllers/src/cronjob/CronjobController.ts @@ -621,6 +621,13 @@ export class CronjobController extends BaseController< // immediately. if (event.recurring && eventDate <= now) { this.#execute(event); + return; + } + + // If the existing event has not passed, start the timer. + if (eventDate >= now) { + this.#startTimer(event); + return; } this.#schedule(event); From b8c8087bf9c707b9ff614369c991eb8baa2952c1 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 6 Aug 2025 14:30:26 +0200 Subject: [PATCH 2/3] return -> continue --- packages/snaps-controllers/src/cronjob/CronjobController.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.ts b/packages/snaps-controllers/src/cronjob/CronjobController.ts index bd9f2d5524..6eebf24aff 100644 --- a/packages/snaps-controllers/src/cronjob/CronjobController.ts +++ b/packages/snaps-controllers/src/cronjob/CronjobController.ts @@ -621,13 +621,13 @@ export class CronjobController extends BaseController< // immediately. if (event.recurring && eventDate <= now) { this.#execute(event); - return; + continue; } // If the existing event has not passed, start the timer. if (eventDate >= now) { this.#startTimer(event); - return; + continue; } this.#schedule(event); From 3f011dbb4ae309c903227f6b9b0f865318af9a56 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Wed, 6 Aug 2025 15:15:52 +0200 Subject: [PATCH 3/3] Simplify --- .../src/cronjob/CronjobController.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/packages/snaps-controllers/src/cronjob/CronjobController.ts b/packages/snaps-controllers/src/cronjob/CronjobController.ts index 6eebf24aff..c98416064a 100644 --- a/packages/snaps-controllers/src/cronjob/CronjobController.ts +++ b/packages/snaps-controllers/src/cronjob/CronjobController.ts @@ -406,8 +406,15 @@ export class CronjobController extends BaseController< * Get the next execution date for a given event and start a timer for it. * * @param event - The event to schedule. + * @param next - Whether to schedule to the next date, otherwise will + * schedule for existing date. */ - #schedule(event: InternalBackgroundEvent) { + #schedule(event: InternalBackgroundEvent, next = true) { + if (!next) { + this.#startTimer(event); + return; + } + const date = getExecutionDate(event.schedule); const { nextState } = this.update((state) => { state.events[event.id].date = date; @@ -624,13 +631,7 @@ export class CronjobController extends BaseController< continue; } - // If the existing event has not passed, start the timer. - if (eventDate >= now) { - this.#startTimer(event); - continue; - } - - this.#schedule(event); + this.#schedule(event, false); } }