From 506468951900d674a19e8af14fe1df349fc9b65f Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 6 Dec 2023 12:20:28 -0600 Subject: [PATCH 01/22] PollingController: add option to poll by blockTracker --- .../src/PollingController.ts | 72 +++++++++++++++++-- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/packages/polling-controller/src/PollingController.ts b/packages/polling-controller/src/PollingController.ts index 0327a7806e..5233c333a5 100644 --- a/packages/polling-controller/src/PollingController.ts +++ b/packages/polling-controller/src/PollingController.ts @@ -1,6 +1,8 @@ -import { BaseController, BaseControllerV1 } from '@metamask/base-controller'; -import type { NetworkClientId } from '@metamask/network-controller'; -import type { Json } from '@metamask/utils'; +import { BaseController, BaseControllerV2 } from '@metamask/base-controller'; +import type { + NetworkClientId, + NetworkClient, +} from '@metamask/network-controller'; import stringify from 'fast-json-stable-stringify'; import { v4 as random } from 'uuid'; @@ -38,6 +40,11 @@ function PollingControllerMixin(Base: TBase) { readonly #intervalIds: Record = {}; + readonly #activeListeners: Record< + PollingTokenSetId, + (options: Json) => Promise + > = {}; + #callbacks: Map< NetworkClientId, Set<(networkClientId: NetworkClientId) => void> @@ -45,6 +52,10 @@ function PollingControllerMixin(Base: TBase) { #intervalLength = 1000; + #getNetworkClientById: + | ((networkClientId: NetworkClientId) => NetworkClient) + | undefined; + getIntervalLength() { return this.#intervalLength; } @@ -58,6 +69,17 @@ function PollingControllerMixin(Base: TBase) { this.#intervalLength = length; } + setPollOnNewBlocks( + getNetworkClientById: (networkClientId: NetworkClientId) => NetworkClient, + ) { + if (!getNetworkClientById) { + throw new Error('getNetworkClientById callback required'); + } + + this.#intervalLength = 0; + this.#getNetworkClientById = getNetworkClientById; + } + /** * Starts polling for a networkClientId * @@ -113,6 +135,21 @@ function PollingControllerMixin(Base: TBase) { if (tokenSet.size === 0) { clearTimeout(this.#intervalIds[key]); delete this.#intervalIds[key]; + + // if applicable stop listening for new blocks + if (this.#getNetworkClientById !== undefined) { + const [networkClientId] = key.split(':'); + const { blockTracker } = + this.#getNetworkClientById(networkClientId); + if (blockTracker) { + blockTracker.removeListener( + 'latest', + this.#activeListeners[key], + ); + } + delete this.#activeListeners[key]; + } + this.#pollingTokenSets.delete(key); this.#callbacks.get(key)?.forEach((callback) => { callback(key); @@ -139,6 +176,31 @@ function PollingControllerMixin(Base: TBase) { #poll(networkClientId: NetworkClientId, options: Json) { const key = getKey(networkClientId, options); + + // if #getNetworkClientById is defined, we want to poll on new blocks + if (this.#getNetworkClientById !== undefined) { + // if we're already listening for new blocks for this key, don't add another listener + if (this.#activeListeners[key]) { + return; + } + const blockTracker = + this.#getNetworkClientById(networkClientId)?.blockTracker; + + if (blockTracker) { + const updateOnNewBlock = this._executePoll.bind( + networkClientId, + options, + ); + blockTracker.addListener('latest', updateOnNewBlock); + this.#activeListeners[key] = updateOnNewBlock; + return; + } + + throw new Error(` + Unable to retreive blockTracker for networkClientId ${networkClientId} `); + } + + // if we're not polling on new blocks, use setTimeout const interval = this.#intervalIds[key]; if (interval) { clearTimeout(interval); @@ -190,5 +252,5 @@ function PollingControllerMixin(Base: TBase) { class Empty {} export const PollingControllerOnly = PollingControllerMixin(Empty); -export const PollingController = PollingControllerMixin(BaseController); -export const PollingControllerV1 = PollingControllerMixin(BaseControllerV1); +export const PollingController = PollingControllerMixin(BaseControllerV2); +export const PollingControllerV1 = PollingControllerMixin(BaseController); From 394fcdd40aecca8335f6269916d1d52a3d59a849 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 6 Dec 2023 12:25:00 -0600 Subject: [PATCH 02/22] fix --- packages/polling-controller/src/PollingController.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/polling-controller/src/PollingController.ts b/packages/polling-controller/src/PollingController.ts index 5233c333a5..5468d6f11f 100644 --- a/packages/polling-controller/src/PollingController.ts +++ b/packages/polling-controller/src/PollingController.ts @@ -1,8 +1,9 @@ -import { BaseController, BaseControllerV2 } from '@metamask/base-controller'; +import { BaseController, BaseControllerV1 } from '@metamask/base-controller'; import type { NetworkClientId, NetworkClient, } from '@metamask/network-controller'; +import type { Json } from '@metamask/utils'; import stringify from 'fast-json-stable-stringify'; import { v4 as random } from 'uuid'; @@ -188,6 +189,7 @@ function PollingControllerMixin(Base: TBase) { if (blockTracker) { const updateOnNewBlock = this._executePoll.bind( + this, networkClientId, options, ); @@ -252,5 +254,5 @@ function PollingControllerMixin(Base: TBase) { class Empty {} export const PollingControllerOnly = PollingControllerMixin(Empty); -export const PollingController = PollingControllerMixin(BaseControllerV2); -export const PollingControllerV1 = PollingControllerMixin(BaseController); +export const PollingController = PollingControllerMixin(BaseController); +export const PollingControllerV1 = PollingControllerMixin(BaseControllerV1); From 242252b4dd6c706d393384f8567de0211cc42cd3 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 6 Dec 2023 12:53:36 -0600 Subject: [PATCH 03/22] adding tests --- .../src/PollingController.test.ts | 323 +++++++++++++++--- 1 file changed, 280 insertions(+), 43 deletions(-) diff --git a/packages/polling-controller/src/PollingController.test.ts b/packages/polling-controller/src/PollingController.test.ts index b6a9344de5..11fc29e11a 100644 --- a/packages/polling-controller/src/PollingController.test.ts +++ b/packages/polling-controller/src/PollingController.test.ts @@ -1,4 +1,5 @@ import { ControllerMessenger } from '@metamask/base-controller'; +import EventEmitter from 'events'; import { useFakeTimers } from 'sinon'; import { advanceTime } from '../../../tests/helpers'; @@ -13,6 +14,10 @@ const createExecutePollMock = () => { return executePollMock; }; +class MyGasFeeController extends PollingController { + _executePoll = createExecutePollMock(); +} + describe('PollingController', () => { let clock: sinon.SinonFakeTimers; beforeEach(() => { @@ -23,9 +28,6 @@ describe('PollingController', () => { }); describe('start', () => { it('should start polling if not polling', async () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -44,9 +46,6 @@ describe('PollingController', () => { }); describe('stop', () => { it('should stop polling when called with a valid polling that was the only active pollingToken for a given networkClient', async () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -65,9 +64,6 @@ describe('PollingController', () => { controller.stopAllPolling(); }); it('should not stop polling if called with one of multiple active polling tokens for a given networkClient', async () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -88,9 +84,6 @@ describe('PollingController', () => { controller.stopAllPolling(); }); it('should error if no pollingToken is passed', () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -106,9 +99,6 @@ describe('PollingController', () => { controller.stopAllPolling(); }); it('should error if no matching pollingToken is found', () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -126,9 +116,6 @@ describe('PollingController', () => { }); describe('startPollingByNetworkClientId', () => { it('should call _executePoll immediately and on interval if polling', async () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -144,9 +131,6 @@ describe('PollingController', () => { expect(controller._executePoll).toHaveBeenCalledTimes(3); }); it('should call _executePoll immediately once and continue calling _executePoll on interval when start is called again with the same networkClientId', async () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -169,9 +153,7 @@ describe('PollingController', () => { }); it('should publish "pollingComplete" when stop is called', async () => { const pollingComplete: any = jest.fn(); - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } + const name = 'PollingController'; const mockMessenger = new ControllerMessenger(); @@ -188,9 +170,6 @@ describe('PollingController', () => { expect(pollingComplete).toHaveBeenCalledTimes(1); }); it('should poll at the interval length when set via setIntervalLength', async () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -211,9 +190,6 @@ describe('PollingController', () => { expect(controller._executePoll).toHaveBeenCalledTimes(2); }); it('should start and stop polling sessions for different networkClientIds with the same options', async () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -263,9 +239,6 @@ describe('PollingController', () => { }); describe('multiple networkClientIds', () => { it('should poll for each networkClientId', async () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -277,38 +250,35 @@ describe('PollingController', () => { controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); - controller.startPollingByNetworkClientId('rinkeby'); + controller.startPollingByNetworkClientId('goerli'); await advanceTime({ clock, duration: 0 }); expect(controller._executePoll.mock.calls).toMatchObject([ ['mainnet', {}], - ['rinkeby', {}], + ['goerli', {}], ]); await advanceTime({ clock, duration: TICK_TIME }); expect(controller._executePoll.mock.calls).toMatchObject([ ['mainnet', {}], - ['rinkeby', {}], + ['goerli', {}], ['mainnet', {}], - ['rinkeby', {}], + ['goerli', {}], ]); await advanceTime({ clock, duration: TICK_TIME }); expect(controller._executePoll.mock.calls).toMatchObject([ ['mainnet', {}], - ['rinkeby', {}], + ['goerli', {}], ['mainnet', {}], - ['rinkeby', {}], + ['goerli', {}], ['mainnet', {}], - ['rinkeby', {}], + ['goerli', {}], ]); controller.stopAllPolling(); }); it('should poll multiple networkClientIds when setting interval length', async () => { - class MyGasFeeController extends PollingController { - _executePoll = createExecutePollMock(); - } const mockMessenger = new ControllerMessenger(); const controller = new MyGasFeeController({ @@ -383,4 +353,271 @@ describe('PollingController', () => { expect(c.stopPollingByPollingToken).toBeDefined(); }); }); + describe('startPollingByNetworkClientId after setPollOnNewBlocks', () => { + class TestBlockTracker extends EventEmitter { + private latestBlockNumber: number; + + public interval: number; + + constructor({ interval } = { interval: 1000 }) { + super(); + this.latestBlockNumber = 0; + this.interval = interval; + this.start(interval); + } + + private start(interval: number) { + setInterval(() => { + this.latestBlockNumber += 1; + this.emit('latest', this.latestBlockNumber); + }, interval); + } + } + + let getNetworkClientById: jest.Mock; + let mainnetBlockTracker: TestBlockTracker; + let goerliBlockTracker: TestBlockTracker; + let sepoliaBlockTracker: TestBlockTracker; + beforeEach(() => { + mainnetBlockTracker = new TestBlockTracker({ interval: 5 }); + goerliBlockTracker = new TestBlockTracker({ interval: 10 }); + sepoliaBlockTracker = new TestBlockTracker({ interval: 15 }); + + getNetworkClientById = jest.fn().mockImplementation((networkClientId) => { + switch (networkClientId) { + case 'mainnet': + return { + blockTracker: mainnetBlockTracker, + }; + case 'goerli': + return { + blockTracker: goerliBlockTracker, + }; + case 'sepolia': + return { + blockTracker: sepoliaBlockTracker, + }; + default: + throw new Error(`Unknown networkClientId: ${networkClientId}`); + } + }); + }); + + it('should start polling for the specified networkClientId', async () => { + const mockMessenger = new ControllerMessenger(); + + const controller = new MyGasFeeController({ + messenger: mockMessenger, + metadata: {}, + name: 'PollingController', + state: { foo: 'bar' }, + }); + + // const getNetworkClientById = jest.fn().mockReturnValue({ + // blockTracker: new TestBlockTracker({ interval: 5 }), + // }); + + controller.setPollOnNewBlocks(getNetworkClientById); + + controller.startPollingByNetworkClientId('mainnet'); + + expect(getNetworkClientById).toHaveBeenCalledWith('mainnet'); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + + await advanceTime({ clock, duration: 1 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + + await advanceTime({ clock, duration: 4 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + expect.arrayContaining(['mainnet', {}]), + expect.arrayContaining(['mainnet', {}]), + ]); + + // Stop all polling + controller.stopAllPolling(); + }); + + it('should poll on new block intervals for each networkClientId', async () => { + const mockMessenger = new ControllerMessenger(); + + const controller = new MyGasFeeController({ + messenger: mockMessenger, + metadata: {}, + name: 'PollingController', + state: { foo: 'bar' }, + }); + + controller.setPollOnNewBlocks(getNetworkClientById); + + controller.startPollingByNetworkClientId('mainnet'); + controller.startPollingByNetworkClientId('goerli'); + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + + // Start polling for goerli, 10ms interval + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ]); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ['mainnet', {}, 3], + ]); + + // 15ms have passed + // Start polling for sepolia, 15ms interval + controller.startPollingByNetworkClientId('sepolia'); + + await advanceTime({ clock, duration: 15 }); + + // at 30ms, 6 blocks have passed for mainnet (every 5ms), 3 for goerli (every 10ms), and 2 for sepolia (every 15ms) + // Didn't start listening to sepolia until 15ms had passed, so we only call executePoll on the 2nd block + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ['mainnet', {}, 5], + ['mainnet', {}, 6], + ['goerli', {}, 3], + ['sepolia', {}, 2], + ]); + + // Stop all polling + controller.stopAllPolling(); + }); + + it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { + const mockMessenger = new ControllerMessenger(); + + const controller = new MyGasFeeController({ + messenger: mockMessenger, + metadata: {}, + name: 'PollingController', + state: { foo: 'bar' }, + }); + + controller.setPollOnNewBlocks(getNetworkClientById); + + const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + + const pollingToken2 = controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); + + controller.stopPollingByPollingToken(pollingToken1); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); + + controller.stopPollingByPollingToken(pollingToken2); + + await advanceTime({ clock, duration: 15 }); + + // no further polling should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); + }); + + it('should should stop polling when all polling tokens for a networkClientId are deleted, even if other networkClientIds are still polling', async () => { + const mockMessenger = new ControllerMessenger(); + + const controller = new MyGasFeeController({ + messenger: mockMessenger, + metadata: {}, + name: 'PollingController', + state: { foo: 'bar' }, + }); + + controller.setPollOnNewBlocks(getNetworkClientById); + + const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + + const pollingToken2 = controller.startPollingByNetworkClientId('mainnet'); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); + + controller.startPollingByNetworkClientId('goerli'); + await advanceTime({ clock, duration: 5 }); + + // 3 blocks have passed for mainnet, 1 for goerli but we only started listening to goerli after 5ms + // so the next block will come at 20ms and be the 2nd block for goerli + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); + + controller.stopPollingByPollingToken(pollingToken1); + + await advanceTime({ clock, duration: 5 }); + + // 20ms have passed, 4 blocks for mainnet, 2 for goerli + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ]); + + controller.stopPollingByPollingToken(pollingToken2); + + await advanceTime({ clock, duration: 20 }); + + // no further polling for mainnet should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ['goerli', {}, 3], + ['goerli', {}, 4], + ]); + }); + }); }); From 4a7470460a8dd7e276564a55aea202172f6284b7 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 6 Dec 2023 16:58:37 -0600 Subject: [PATCH 04/22] another test + cleanup --- .../src/PollingController.test.ts | 154 ++---------------- 1 file changed, 13 insertions(+), 141 deletions(-) diff --git a/packages/polling-controller/src/PollingController.test.ts b/packages/polling-controller/src/PollingController.test.ts index 11fc29e11a..4f71f8ddf5 100644 --- a/packages/polling-controller/src/PollingController.test.ts +++ b/packages/polling-controller/src/PollingController.test.ts @@ -20,7 +20,16 @@ class MyGasFeeController extends PollingController { describe('PollingController', () => { let clock: sinon.SinonFakeTimers; + let mockMessenger: any; + let controller: any; beforeEach(() => { + mockMessenger = new ControllerMessenger(); + controller = new MyGasFeeController({ + messenger: mockMessenger, + metadata: {}, + name: 'PollingController', + state: { foo: 'bar' }, + }); clock = useFakeTimers(); }); afterEach(() => { @@ -28,14 +37,6 @@ describe('PollingController', () => { }); describe('start', () => { it('should start polling if not polling', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); expect(controller._executePoll).toHaveBeenCalledTimes(1); @@ -46,14 +47,6 @@ describe('PollingController', () => { }); describe('stop', () => { it('should stop polling when called with a valid polling that was the only active pollingToken for a given networkClient', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); const pollingToken = controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); expect(controller._executePoll).toHaveBeenCalledTimes(1); @@ -64,14 +57,6 @@ describe('PollingController', () => { controller.stopAllPolling(); }); it('should not stop polling if called with one of multiple active polling tokens for a given networkClient', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); @@ -84,14 +69,6 @@ describe('PollingController', () => { controller.stopAllPolling(); }); it('should error if no pollingToken is passed', () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); controller.startPollingByNetworkClientId('mainnet'); expect(() => { controller.stopPollingByPollingToken(undefined as unknown as any); @@ -99,14 +76,6 @@ describe('PollingController', () => { controller.stopAllPolling(); }); it('should error if no matching pollingToken is found', () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); controller.startPollingByNetworkClientId('mainnet'); expect(() => { controller.stopPollingByPollingToken('potato'); @@ -116,14 +85,6 @@ describe('PollingController', () => { }); describe('startPollingByNetworkClientId', () => { it('should call _executePoll immediately and on interval if polling', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); expect(controller._executePoll).toHaveBeenCalledTimes(1); @@ -131,14 +92,6 @@ describe('PollingController', () => { expect(controller._executePoll).toHaveBeenCalledTimes(3); }); it('should call _executePoll immediately once and continue calling _executePoll on interval when start is called again with the same networkClientId', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); @@ -153,31 +106,12 @@ describe('PollingController', () => { }); it('should publish "pollingComplete" when stop is called', async () => { const pollingComplete: any = jest.fn(); - - const name = 'PollingController'; - - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name, - state: { foo: 'bar' }, - }); controller.onPollingCompleteByNetworkClientId('mainnet', pollingComplete); const pollingToken = controller.startPollingByNetworkClientId('mainnet'); controller.stopPollingByPollingToken(pollingToken); expect(pollingComplete).toHaveBeenCalledTimes(1); }); it('should poll at the interval length when set via setIntervalLength', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); controller.setIntervalLength(TICK_TIME); controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); @@ -190,14 +124,6 @@ describe('PollingController', () => { expect(controller._executePoll).toHaveBeenCalledTimes(2); }); it('should start and stop polling sessions for different networkClientIds with the same options', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); const pollToken1 = controller.startPollingByNetworkClientId('mainnet', { address: '0x1', }); @@ -239,14 +165,6 @@ describe('PollingController', () => { }); describe('multiple networkClientIds', () => { it('should poll for each networkClientId', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); @@ -279,14 +197,6 @@ describe('PollingController', () => { }); it('should poll multiple networkClientIds when setting interval length', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); controller.setIntervalLength(TICK_TIME * 2); controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); @@ -404,19 +314,6 @@ describe('PollingController', () => { }); it('should start polling for the specified networkClientId', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); - - // const getNetworkClientById = jest.fn().mockReturnValue({ - // blockTracker: new TestBlockTracker({ interval: 5 }), - // }); - controller.setPollOnNewBlocks(getNetworkClientById); controller.startPollingByNetworkClientId('mainnet'); @@ -438,20 +335,10 @@ describe('PollingController', () => { expect.arrayContaining(['mainnet', {}]), ]); - // Stop all polling controller.stopAllPolling(); }); it('should poll on new block intervals for each networkClientId', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); - controller.setPollOnNewBlocks(getNetworkClientById); controller.startPollingByNetworkClientId('mainnet'); @@ -500,20 +387,10 @@ describe('PollingController', () => { ['sepolia', {}, 2], ]); - // Stop all polling controller.stopAllPolling(); }); it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); - controller.setPollOnNewBlocks(getNetworkClientById); const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); @@ -551,18 +428,11 @@ describe('PollingController', () => { ['mainnet', {}, 2], ['mainnet', {}, 3], ]); + + controller.stopAllPolling(); }); it('should should stop polling when all polling tokens for a networkClientId are deleted, even if other networkClientIds are still polling', async () => { - const mockMessenger = new ControllerMessenger(); - - const controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); - controller.setPollOnNewBlocks(getNetworkClientById); const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); @@ -618,6 +488,8 @@ describe('PollingController', () => { ['goerli', {}, 3], ['goerli', {}, 4], ]); + + controller.stopAllPolling(); }); }); }); From 6698ebdd96aca7dda2886d920e8646908c07d200 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 6 Dec 2023 17:17:07 -0600 Subject: [PATCH 05/22] add protection against multiple sources of polling interval --- .../src/PollingController.test.ts | 409 ++++++++++-------- .../src/PollingController.ts | 13 +- 2 files changed, 228 insertions(+), 194 deletions(-) diff --git a/packages/polling-controller/src/PollingController.test.ts b/packages/polling-controller/src/PollingController.test.ts index 4f71f8ddf5..b7571c8f2c 100644 --- a/packages/polling-controller/src/PollingController.test.ts +++ b/packages/polling-controller/src/PollingController.test.ts @@ -83,6 +83,18 @@ describe('PollingController', () => { controller.stopAllPolling(); }); }); + + describe('setIntervalLength', () => { + it('should set getNetworkClientById if previously set to undefined when setting interval length', async () => { + controller.setPollWithBlockTracker(() => { + throw new Error('should not be called'); + }); + expect(controller.getPollingWithBlockTracker()).toBe(true); + controller.setIntervalLength(1000); + expect(controller.getPollingWithBlockTracker()).toBe(false); + }); + }); + describe('startPollingByNetworkClientId', () => { it('should call _executePoll immediately and on interval if polling', async () => { controller.startPollingByNetworkClientId('mainnet'); @@ -248,248 +260,261 @@ describe('PollingController', () => { ['sepolia', {}], ]); }); - }); - describe('PollingControllerOnly', () => { - it('can be extended from and constructed', async () => { - class MyClass extends PollingControllerOnly { - _executePoll = createExecutePollMock(); - } - const c = new MyClass(); - expect(c._executePoll).toBeDefined(); - expect(c.getIntervalLength).toBeDefined(); - expect(c.setIntervalLength).toBeDefined(); - expect(c.stopAllPolling).toBeDefined(); - expect(c.startPollingByNetworkClientId).toBeDefined(); - expect(c.stopPollingByPollingToken).toBeDefined(); - }); - }); - describe('startPollingByNetworkClientId after setPollOnNewBlocks', () => { - class TestBlockTracker extends EventEmitter { - private latestBlockNumber: number; - public interval: number; - - constructor({ interval } = { interval: 1000 }) { - super(); - this.latestBlockNumber = 0; - this.interval = interval; - this.start(interval); - } - - private start(interval: number) { - setInterval(() => { - this.latestBlockNumber += 1; - this.emit('latest', this.latestBlockNumber); - }, interval); - } - } - - let getNetworkClientById: jest.Mock; - let mainnetBlockTracker: TestBlockTracker; - let goerliBlockTracker: TestBlockTracker; - let sepoliaBlockTracker: TestBlockTracker; - beforeEach(() => { - mainnetBlockTracker = new TestBlockTracker({ interval: 5 }); - goerliBlockTracker = new TestBlockTracker({ interval: 10 }); - sepoliaBlockTracker = new TestBlockTracker({ interval: 15 }); - - getNetworkClientById = jest.fn().mockImplementation((networkClientId) => { - switch (networkClientId) { - case 'mainnet': - return { - blockTracker: mainnetBlockTracker, - }; - case 'goerli': - return { - blockTracker: goerliBlockTracker, - }; - case 'sepolia': - return { - blockTracker: sepoliaBlockTracker, - }; - default: - throw new Error(`Unknown networkClientId: ${networkClientId}`); + describe('PollingControllerOnly', () => { + it('can be extended from and constructed', async () => { + class MyClass extends PollingControllerOnly { + _executePoll = createExecutePollMock(); } + const c = new MyClass(); + expect(c._executePoll).toBeDefined(); + expect(c.getIntervalLength).toBeDefined(); + expect(c.setIntervalLength).toBeDefined(); + expect(c.stopAllPolling).toBeDefined(); + expect(c.startPollingByNetworkClientId).toBeDefined(); + expect(c.stopPollingByPollingToken).toBeDefined(); }); }); + describe('startPollingByNetworkClientId after setPollWithBlockTracker', () => { + class TestBlockTracker extends EventEmitter { + private latestBlockNumber: number; - it('should start polling for the specified networkClientId', async () => { - controller.setPollOnNewBlocks(getNetworkClientById); - - controller.startPollingByNetworkClientId('mainnet'); - - expect(getNetworkClientById).toHaveBeenCalledWith('mainnet'); - - await advanceTime({ clock, duration: 5 }); + public interval: number; - expect(controller._executePoll).toHaveBeenCalledTimes(1); - - await advanceTime({ clock, duration: 1 }); + constructor({ interval } = { interval: 1000 }) { + super(); + this.latestBlockNumber = 0; + this.interval = interval; + this.start(interval); + } - expect(controller._executePoll).toHaveBeenCalledTimes(1); + private start(interval: number) { + setInterval(() => { + this.latestBlockNumber += 1; + this.emit('latest', this.latestBlockNumber); + }, interval); + } + } - await advanceTime({ clock, duration: 4 }); + let getNetworkClientById: jest.Mock; + let mainnetBlockTracker: TestBlockTracker; + let goerliBlockTracker: TestBlockTracker; + let sepoliaBlockTracker: TestBlockTracker; + beforeEach(() => { + mainnetBlockTracker = new TestBlockTracker({ interval: 5 }); + goerliBlockTracker = new TestBlockTracker({ interval: 10 }); + sepoliaBlockTracker = new TestBlockTracker({ interval: 15 }); + + getNetworkClientById = jest + .fn() + .mockImplementation((networkClientId) => { + switch (networkClientId) { + case 'mainnet': + return { + blockTracker: mainnetBlockTracker, + }; + case 'goerli': + return { + blockTracker: goerliBlockTracker, + }; + case 'sepolia': + return { + blockTracker: sepoliaBlockTracker, + }; + default: + throw new Error(`Unknown networkClientId: ${networkClientId}`); + } + }); + }); - expect(controller._executePoll.mock.calls).toMatchObject([ - expect.arrayContaining(['mainnet', {}]), - expect.arrayContaining(['mainnet', {}]), - ]); + it('should set the interval length to 0', () => { + controller.setPollWithBlockTracker(getNetworkClientById); - controller.stopAllPolling(); - }); + expect(controller.getIntervalLength()).toBe(0); + }); - it('should poll on new block intervals for each networkClientId', async () => { - controller.setPollOnNewBlocks(getNetworkClientById); + it('should start polling for the specified networkClientId', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); - controller.startPollingByNetworkClientId('mainnet'); - controller.startPollingByNetworkClientId('goerli'); - await advanceTime({ clock, duration: 5 }); + controller.startPollingByNetworkClientId('mainnet'); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + expect(getNetworkClientById).toHaveBeenCalledWith('mainnet'); - // Start polling for goerli, 10ms interval - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ]); + expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 1 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ['mainnet', {}, 3], - ]); + expect(controller._executePoll).toHaveBeenCalledTimes(1); - // 15ms have passed - // Start polling for sepolia, 15ms interval - controller.startPollingByNetworkClientId('sepolia'); + await advanceTime({ clock, duration: 4 }); - await advanceTime({ clock, duration: 15 }); + expect(controller._executePoll.mock.calls).toMatchObject([ + expect.arrayContaining(['mainnet', {}]), + expect.arrayContaining(['mainnet', {}]), + ]); - // at 30ms, 6 blocks have passed for mainnet (every 5ms), 3 for goerli (every 10ms), and 2 for sepolia (every 15ms) - // Didn't start listening to sepolia until 15ms had passed, so we only call executePoll on the 2nd block - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ['mainnet', {}, 5], - ['mainnet', {}, 6], - ['goerli', {}, 3], - ['sepolia', {}, 2], - ]); + controller.stopAllPolling(); + }); - controller.stopAllPolling(); - }); + it('should poll on new block intervals for each networkClientId', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); + + controller.startPollingByNetworkClientId('mainnet'); + controller.startPollingByNetworkClientId('goerli'); + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + + // Start polling for goerli, 10ms interval + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ]); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ['mainnet', {}, 3], + ]); + + // 15ms have passed + // Start polling for sepolia, 15ms interval + controller.startPollingByNetworkClientId('sepolia'); + + await advanceTime({ clock, duration: 15 }); + + // at 30ms, 6 blocks have passed for mainnet (every 5ms), 3 for goerli (every 10ms), and 2 for sepolia (every 15ms) + // Didn't start listening to sepolia until 15ms had passed, so we only call executePoll on the 2nd block + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ['mainnet', {}, 5], + ['mainnet', {}, 6], + ['goerli', {}, 3], + ['sepolia', {}, 2], + ]); + + controller.stopAllPolling(); + }); - it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { - controller.setPollOnNewBlocks(getNetworkClientById); + it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); - const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); + const pollingToken1 = + controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - const pollingToken2 = controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + const pollingToken2 = + controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ]); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); - controller.stopPollingByPollingToken(pollingToken1); + controller.stopPollingByPollingToken(pollingToken1); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); - controller.stopPollingByPollingToken(pollingToken2); + controller.stopPollingByPollingToken(pollingToken2); - await advanceTime({ clock, duration: 15 }); + await advanceTime({ clock, duration: 15 }); - // no further polling should occur - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); + // no further polling should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); - controller.stopAllPolling(); - }); + controller.stopAllPolling(); + }); - it('should should stop polling when all polling tokens for a networkClientId are deleted, even if other networkClientIds are still polling', async () => { - controller.setPollOnNewBlocks(getNetworkClientById); + it('should should stop polling when all polling tokens for a networkClientId are deleted, even if other networkClientIds are still polling', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); - const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); + const pollingToken1 = + controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - const pollingToken2 = controller.startPollingByNetworkClientId('mainnet'); + const pollingToken2 = + controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ]); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); - controller.startPollingByNetworkClientId('goerli'); - await advanceTime({ clock, duration: 5 }); + controller.startPollingByNetworkClientId('goerli'); + await advanceTime({ clock, duration: 5 }); - // 3 blocks have passed for mainnet, 1 for goerli but we only started listening to goerli after 5ms - // so the next block will come at 20ms and be the 2nd block for goerli - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); + // 3 blocks have passed for mainnet, 1 for goerli but we only started listening to goerli after 5ms + // so the next block will come at 20ms and be the 2nd block for goerli + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); - controller.stopPollingByPollingToken(pollingToken1); + controller.stopPollingByPollingToken(pollingToken1); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - // 20ms have passed, 4 blocks for mainnet, 2 for goerli - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ]); + // 20ms have passed, 4 blocks for mainnet, 2 for goerli + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ]); - controller.stopPollingByPollingToken(pollingToken2); + controller.stopPollingByPollingToken(pollingToken2); - await advanceTime({ clock, duration: 20 }); + await advanceTime({ clock, duration: 20 }); - // no further polling for mainnet should occur - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ['goerli', {}, 3], - ['goerli', {}, 4], - ]); + // no further polling for mainnet should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ['goerli', {}, 3], + ['goerli', {}, 4], + ]); - controller.stopAllPolling(); + controller.stopAllPolling(); + }); }); }); }); diff --git a/packages/polling-controller/src/PollingController.ts b/packages/polling-controller/src/PollingController.ts index 5468d6f11f..4f524e01ef 100644 --- a/packages/polling-controller/src/PollingController.ts +++ b/packages/polling-controller/src/PollingController.ts @@ -61,6 +61,10 @@ function PollingControllerMixin(Base: TBase) { return this.#intervalLength; } + getPollingWithBlockTracker() { + return this.#getNetworkClientById !== undefined; + } + /** * Sets the length of the polling interval * @@ -68,17 +72,22 @@ function PollingControllerMixin(Base: TBase) { */ setIntervalLength(length: number) { this.#intervalLength = length; + + // setting and using an interval is mutually exclusive with polling on new blocks + this.#getNetworkClientById = undefined; } - setPollOnNewBlocks( + setPollWithBlockTracker( getNetworkClientById: (networkClientId: NetworkClientId) => NetworkClient, ) { if (!getNetworkClientById) { throw new Error('getNetworkClientById callback required'); } - this.#intervalLength = 0; this.#getNetworkClientById = getNetworkClientById; + + // using block times is mutually exclusive with polling on a static interval + this.#intervalLength = 0; } /** From 8906fb8795268c2966074e9c1551d9ea63ce6b2e Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 6 Dec 2023 17:27:33 -0600 Subject: [PATCH 06/22] cleanup --- packages/polling-controller/src/PollingController.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/polling-controller/src/PollingController.ts b/packages/polling-controller/src/PollingController.ts index 4f524e01ef..0080387f1f 100644 --- a/packages/polling-controller/src/PollingController.ts +++ b/packages/polling-controller/src/PollingController.ts @@ -143,8 +143,10 @@ function PollingControllerMixin(Base: TBase) { found = true; tokenSet.delete(pollingToken); if (tokenSet.size === 0) { - clearTimeout(this.#intervalIds[key]); - delete this.#intervalIds[key]; + if (this.#intervalIds[key]) { + clearTimeout(this.#intervalIds[key]); + delete this.#intervalIds[key]; + } // if applicable stop listening for new blocks if (this.#getNetworkClientById !== undefined) { From 8edcf25567752dae5685bca6cab924f0cd41eedd Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 7 Dec 2023 09:36:23 -0600 Subject: [PATCH 07/22] small cleanups --- .../src/PollingController.test.ts | 26 +++++++++---------- .../src/PollingController.ts | 16 +++++++----- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/polling-controller/src/PollingController.test.ts b/packages/polling-controller/src/PollingController.test.ts index b7571c8f2c..693edf028e 100644 --- a/packages/polling-controller/src/PollingController.test.ts +++ b/packages/polling-controller/src/PollingController.test.ts @@ -85,7 +85,7 @@ describe('PollingController', () => { }); describe('setIntervalLength', () => { - it('should set getNetworkClientById if previously set to undefined when setting interval length', async () => { + it('should set getNetworkClientById (if previously set by setPollWithBlockTracker) to undefined when setting interval length', async () => { controller.setPollWithBlockTracker(() => { throw new Error('should not be called'); }); @@ -136,6 +136,7 @@ describe('PollingController', () => { expect(controller._executePoll).toHaveBeenCalledTimes(2); }); it('should start and stop polling sessions for different networkClientIds with the same options', async () => { + controller.setIntervalLength(TICK_TIME); const pollToken1 = controller.startPollingByNetworkClientId('mainnet', { address: '0x1', }); @@ -180,30 +181,30 @@ describe('PollingController', () => { controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); - controller.startPollingByNetworkClientId('goerli'); + controller.startPollingByNetworkClientId('rinkeby'); await advanceTime({ clock, duration: 0 }); expect(controller._executePoll.mock.calls).toMatchObject([ ['mainnet', {}], - ['goerli', {}], + ['rinkeby', {}], ]); await advanceTime({ clock, duration: TICK_TIME }); expect(controller._executePoll.mock.calls).toMatchObject([ ['mainnet', {}], - ['goerli', {}], + ['rinkeby', {}], ['mainnet', {}], - ['goerli', {}], + ['rinkeby', {}], ]); await advanceTime({ clock, duration: TICK_TIME }); expect(controller._executePoll.mock.calls).toMatchObject([ ['mainnet', {}], - ['goerli', {}], + ['rinkeby', {}], ['mainnet', {}], - ['goerli', {}], + ['rinkeby', {}], ['mainnet', {}], - ['goerli', {}], + ['rinkeby', {}], ]); controller.stopAllPolling(); }); @@ -327,10 +328,10 @@ describe('PollingController', () => { }); }); - it('should set the interval length to 0', () => { + it('should set the interval length to undefined', () => { controller.setPollWithBlockTracker(getNetworkClientById); - expect(controller.getIntervalLength()).toBe(0); + expect(controller.getIntervalLength()).toBeUndefined(); }); it('should start polling for the specified networkClientId', async () => { @@ -368,7 +369,6 @@ describe('PollingController', () => { expect(controller._executePoll).toHaveBeenCalledTimes(1); expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - // Start polling for goerli, 10ms interval await advanceTime({ clock, duration: 5 }); expect(controller._executePoll.mock.calls).toMatchObject([ @@ -450,11 +450,9 @@ describe('PollingController', () => { ['mainnet', {}, 2], ['mainnet', {}, 3], ]); - - controller.stopAllPolling(); }); - it('should should stop polling when all polling tokens for a networkClientId are deleted, even if other networkClientIds are still polling', async () => { + it('should should stop polling for one networkClientId when all polling tokens for that networkClientId are deleted, without stopping polling for networkClientIds with active pollingTokens', async () => { controller.setPollWithBlockTracker(getNetworkClientById); const pollingToken1 = diff --git a/packages/polling-controller/src/PollingController.ts b/packages/polling-controller/src/PollingController.ts index 0080387f1f..4442391316 100644 --- a/packages/polling-controller/src/PollingController.ts +++ b/packages/polling-controller/src/PollingController.ts @@ -51,7 +51,7 @@ function PollingControllerMixin(Base: TBase) { Set<(networkClientId: NetworkClientId) => void> > = new Map(); - #intervalLength = 1000; + #intervalLength: number | undefined = 1000; #getNetworkClientById: | ((networkClientId: NetworkClientId) => NetworkClient) @@ -87,7 +87,7 @@ function PollingControllerMixin(Base: TBase) { this.#getNetworkClientById = getNetworkClientById; // using block times is mutually exclusive with polling on a static interval - this.#intervalLength = 0; + this.#intervalLength = undefined; } /** @@ -143,13 +143,15 @@ function PollingControllerMixin(Base: TBase) { found = true; tokenSet.delete(pollingToken); if (tokenSet.size === 0) { + // if applicable stop polling on a static interval if (this.#intervalIds[key]) { clearTimeout(this.#intervalIds[key]); delete this.#intervalIds[key]; - } - - // if applicable stop listening for new blocks - if (this.#getNetworkClientById !== undefined) { + } else if ( + // if applicable stop listening for new blocks + this.#getNetworkClientById !== undefined && + this.#activeListeners[key] + ) { const [networkClientId] = key.split(':'); const { blockTracker } = this.#getNetworkClientById(networkClientId); @@ -210,7 +212,7 @@ function PollingControllerMixin(Base: TBase) { } throw new Error(` - Unable to retreive blockTracker for networkClientId ${networkClientId} `); + Unable to retrieve blockTracker for networkClientId ${networkClientId} `); } // if we're not polling on new blocks, use setTimeout From f6884257c6bc308bd0409218ef140b3225027243 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 7 Dec 2023 09:48:10 -0600 Subject: [PATCH 08/22] move tests --- .../src/PollingController.test.ts | 375 +++++++++--------- 1 file changed, 185 insertions(+), 190 deletions(-) diff --git a/packages/polling-controller/src/PollingController.test.ts b/packages/polling-controller/src/PollingController.test.ts index 693edf028e..601b79a3c1 100644 --- a/packages/polling-controller/src/PollingController.test.ts +++ b/packages/polling-controller/src/PollingController.test.ts @@ -276,243 +276,238 @@ describe('PollingController', () => { expect(c.stopPollingByPollingToken).toBeDefined(); }); }); - describe('startPollingByNetworkClientId after setPollWithBlockTracker', () => { - class TestBlockTracker extends EventEmitter { - private latestBlockNumber: number; + }); - public interval: number; + describe('Polling on block times', () => { + class TestBlockTracker extends EventEmitter { + private latestBlockNumber: number; - constructor({ interval } = { interval: 1000 }) { - super(); - this.latestBlockNumber = 0; - this.interval = interval; - this.start(interval); - } + public interval: number; - private start(interval: number) { - setInterval(() => { - this.latestBlockNumber += 1; - this.emit('latest', this.latestBlockNumber); - }, interval); - } + constructor({ interval } = { interval: 1000 }) { + super(); + this.latestBlockNumber = 0; + this.interval = interval; + this.start(interval); } - let getNetworkClientById: jest.Mock; - let mainnetBlockTracker: TestBlockTracker; - let goerliBlockTracker: TestBlockTracker; - let sepoliaBlockTracker: TestBlockTracker; - beforeEach(() => { - mainnetBlockTracker = new TestBlockTracker({ interval: 5 }); - goerliBlockTracker = new TestBlockTracker({ interval: 10 }); - sepoliaBlockTracker = new TestBlockTracker({ interval: 15 }); - - getNetworkClientById = jest - .fn() - .mockImplementation((networkClientId) => { - switch (networkClientId) { - case 'mainnet': - return { - blockTracker: mainnetBlockTracker, - }; - case 'goerli': - return { - blockTracker: goerliBlockTracker, - }; - case 'sepolia': - return { - blockTracker: sepoliaBlockTracker, - }; - default: - throw new Error(`Unknown networkClientId: ${networkClientId}`); - } - }); + private start(interval: number) { + setInterval(() => { + this.latestBlockNumber += 1; + this.emit('latest', this.latestBlockNumber); + }, interval); + } + } + + let getNetworkClientById: jest.Mock; + let mainnetBlockTracker: TestBlockTracker; + let goerliBlockTracker: TestBlockTracker; + let sepoliaBlockTracker: TestBlockTracker; + beforeEach(() => { + mainnetBlockTracker = new TestBlockTracker({ interval: 5 }); + goerliBlockTracker = new TestBlockTracker({ interval: 10 }); + sepoliaBlockTracker = new TestBlockTracker({ interval: 15 }); + + getNetworkClientById = jest.fn().mockImplementation((networkClientId) => { + switch (networkClientId) { + case 'mainnet': + return { + blockTracker: mainnetBlockTracker, + }; + case 'goerli': + return { + blockTracker: goerliBlockTracker, + }; + case 'sepolia': + return { + blockTracker: sepoliaBlockTracker, + }; + default: + throw new Error(`Unknown networkClientId: ${networkClientId}`); + } }); + }); - it('should set the interval length to undefined', () => { - controller.setPollWithBlockTracker(getNetworkClientById); + it('should set the interval length to undefined', () => { + controller.setPollWithBlockTracker(getNetworkClientById); - expect(controller.getIntervalLength()).toBeUndefined(); - }); + expect(controller.getIntervalLength()).toBeUndefined(); + }); - it('should start polling for the specified networkClientId', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); + it('should start polling for the specified networkClientId', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); - controller.startPollingByNetworkClientId('mainnet'); + controller.startPollingByNetworkClientId('mainnet'); - expect(getNetworkClientById).toHaveBeenCalledWith('mainnet'); + expect(getNetworkClientById).toHaveBeenCalledWith('mainnet'); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: 1 }); + await advanceTime({ clock, duration: 1 }); - expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: 4 }); + await advanceTime({ clock, duration: 4 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - expect.arrayContaining(['mainnet', {}]), - expect.arrayContaining(['mainnet', {}]), - ]); + expect(controller._executePoll.mock.calls).toMatchObject([ + expect.arrayContaining(['mainnet', {}]), + expect.arrayContaining(['mainnet', {}]), + ]); - controller.stopAllPolling(); - }); + controller.stopAllPolling(); + }); - it('should poll on new block intervals for each networkClientId', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); - - controller.startPollingByNetworkClientId('mainnet'); - controller.startPollingByNetworkClientId('goerli'); - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ]); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ['mainnet', {}, 3], - ]); - - // 15ms have passed - // Start polling for sepolia, 15ms interval - controller.startPollingByNetworkClientId('sepolia'); - - await advanceTime({ clock, duration: 15 }); - - // at 30ms, 6 blocks have passed for mainnet (every 5ms), 3 for goerli (every 10ms), and 2 for sepolia (every 15ms) - // Didn't start listening to sepolia until 15ms had passed, so we only call executePoll on the 2nd block - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ['mainnet', {}, 5], - ['mainnet', {}, 6], - ['goerli', {}, 3], - ['sepolia', {}, 2], - ]); - - controller.stopAllPolling(); - }); + it('should poll on new block intervals for each networkClientId', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); - it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); + controller.startPollingByNetworkClientId('mainnet'); + controller.startPollingByNetworkClientId('goerli'); + await advanceTime({ clock, duration: 5 }); - const pollingToken1 = - controller.startPollingByNetworkClientId('mainnet'); + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ]); - const pollingToken2 = - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ]); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ['mainnet', {}, 3], + ]); - controller.stopPollingByPollingToken(pollingToken1); + // 15ms have passed + // Start polling for sepolia, 15ms interval + controller.startPollingByNetworkClientId('sepolia'); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 15 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); + // at 30ms, 6 blocks have passed for mainnet (every 5ms), 3 for goerli (every 10ms), and 2 for sepolia (every 15ms) + // Didn't start listening to sepolia until 15ms had passed, so we only call executePoll on the 2nd block + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ['mainnet', {}, 5], + ['mainnet', {}, 6], + ['goerli', {}, 3], + ['sepolia', {}, 2], + ]); - controller.stopPollingByPollingToken(pollingToken2); + controller.stopAllPolling(); + }); - await advanceTime({ clock, duration: 15 }); + it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); - // no further polling should occur - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); - }); + const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); - it('should should stop polling for one networkClientId when all polling tokens for that networkClientId are deleted, without stopping polling for networkClientIds with active pollingTokens', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); + await advanceTime({ clock, duration: 5 }); - const pollingToken1 = - controller.startPollingByNetworkClientId('mainnet'); + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - await advanceTime({ clock, duration: 5 }); + const pollingToken2 = controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); - const pollingToken2 = - controller.startPollingByNetworkClientId('mainnet'); + controller.stopPollingByPollingToken(pollingToken1); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ]); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); - controller.startPollingByNetworkClientId('goerli'); - await advanceTime({ clock, duration: 5 }); + controller.stopPollingByPollingToken(pollingToken2); - // 3 blocks have passed for mainnet, 1 for goerli but we only started listening to goerli after 5ms - // so the next block will come at 20ms and be the 2nd block for goerli - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); + await advanceTime({ clock, duration: 15 }); - controller.stopPollingByPollingToken(pollingToken1); + // no further polling should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); + }); - await advanceTime({ clock, duration: 5 }); + it('should should stop polling for one networkClientId when all polling tokens for that networkClientId are deleted, without stopping polling for networkClientIds with active pollingTokens', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); - // 20ms have passed, 4 blocks for mainnet, 2 for goerli - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ]); + const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); - controller.stopPollingByPollingToken(pollingToken2); + await advanceTime({ clock, duration: 5 }); - await advanceTime({ clock, duration: 20 }); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - // no further polling for mainnet should occur - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ['goerli', {}, 3], - ['goerli', {}, 4], - ]); + const pollingToken2 = controller.startPollingByNetworkClientId('mainnet'); - controller.stopAllPolling(); - }); + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); + + controller.startPollingByNetworkClientId('goerli'); + await advanceTime({ clock, duration: 5 }); + + // 3 blocks have passed for mainnet, 1 for goerli but we only started listening to goerli after 5ms + // so the next block will come at 20ms and be the 2nd block for goerli + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); + + controller.stopPollingByPollingToken(pollingToken1); + + await advanceTime({ clock, duration: 5 }); + + // 20ms have passed, 4 blocks for mainnet, 2 for goerli + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ]); + + controller.stopPollingByPollingToken(pollingToken2); + + await advanceTime({ clock, duration: 20 }); + + // no further polling for mainnet should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ['goerli', {}, 3], + ['goerli', {}, 4], + ]); + + controller.stopAllPolling(); }); }); }); From f72c57047523d4f8aacb8e8165c813a4b23072f4 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 7 Dec 2023 09:49:45 -0600 Subject: [PATCH 09/22] reorganize new tests --- .../src/PollingController.test.ts | 287 +++++++++--------- 1 file changed, 148 insertions(+), 139 deletions(-) diff --git a/packages/polling-controller/src/PollingController.test.ts b/packages/polling-controller/src/PollingController.test.ts index 601b79a3c1..0a095c4547 100644 --- a/packages/polling-controller/src/PollingController.test.ts +++ b/packages/polling-controller/src/PollingController.test.ts @@ -327,187 +327,196 @@ describe('PollingController', () => { } }); }); + describe('setPollWithBlockTracker', () => { + it('should set the interval length to undefined', () => { + controller.setPollWithBlockTracker(getNetworkClientById); - it('should set the interval length to undefined', () => { - controller.setPollWithBlockTracker(getNetworkClientById); - - expect(controller.getIntervalLength()).toBeUndefined(); - }); - - it('should start polling for the specified networkClientId', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); - - controller.startPollingByNetworkClientId('mainnet'); - - expect(getNetworkClientById).toHaveBeenCalledWith('mainnet'); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - - await advanceTime({ clock, duration: 1 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - - await advanceTime({ clock, duration: 4 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - expect.arrayContaining(['mainnet', {}]), - expect.arrayContaining(['mainnet', {}]), - ]); - - controller.stopAllPolling(); + expect(controller.getIntervalLength()).toBeUndefined(); + }); }); - it('should poll on new block intervals for each networkClientId', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); + describe('startPollingByNetworkClientId', () => { + it('should start polling for the specified networkClientId', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); - controller.startPollingByNetworkClientId('mainnet'); - controller.startPollingByNetworkClientId('goerli'); - await advanceTime({ clock, duration: 5 }); + controller.startPollingByNetworkClientId('mainnet'); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + expect(getNetworkClientById).toHaveBeenCalledWith('mainnet'); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ]); + expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 1 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ['mainnet', {}, 3], - ]); + expect(controller._executePoll).toHaveBeenCalledTimes(1); - // 15ms have passed - // Start polling for sepolia, 15ms interval - controller.startPollingByNetworkClientId('sepolia'); + await advanceTime({ clock, duration: 4 }); - await advanceTime({ clock, duration: 15 }); + expect(controller._executePoll.mock.calls).toMatchObject([ + expect.arrayContaining(['mainnet', {}]), + expect.arrayContaining(['mainnet', {}]), + ]); - // at 30ms, 6 blocks have passed for mainnet (every 5ms), 3 for goerli (every 10ms), and 2 for sepolia (every 15ms) - // Didn't start listening to sepolia until 15ms had passed, so we only call executePoll on the 2nd block - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ['mainnet', {}, 5], - ['mainnet', {}, 6], - ['goerli', {}, 3], - ['sepolia', {}, 2], - ]); + controller.stopAllPolling(); + }); - controller.stopAllPolling(); + it('should poll on new block intervals for each networkClientId', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); + + controller.startPollingByNetworkClientId('mainnet'); + controller.startPollingByNetworkClientId('goerli'); + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ]); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ['mainnet', {}, 3], + ]); + + // 15ms have passed + // Start polling for sepolia, 15ms interval + controller.startPollingByNetworkClientId('sepolia'); + + await advanceTime({ clock, duration: 15 }); + + // at 30ms, 6 blocks have passed for mainnet (every 5ms), 3 for goerli (every 10ms), and 2 for sepolia (every 15ms) + // Didn't start listening to sepolia until 15ms had passed, so we only call executePoll on the 2nd block + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ['mainnet', {}, 5], + ['mainnet', {}, 6], + ['goerli', {}, 3], + ['sepolia', {}, 2], + ]); + + controller.stopAllPolling(); + }); }); - it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); + describe('stopPollingByPollingToken', () => { + it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); - const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); + const pollingToken1 = + controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - const pollingToken2 = controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + const pollingToken2 = + controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ]); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); - controller.stopPollingByPollingToken(pollingToken1); + controller.stopPollingByPollingToken(pollingToken1); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); - controller.stopPollingByPollingToken(pollingToken2); + controller.stopPollingByPollingToken(pollingToken2); - await advanceTime({ clock, duration: 15 }); + await advanceTime({ clock, duration: 15 }); - // no further polling should occur - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); - }); + // no further polling should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); + }); - it('should should stop polling for one networkClientId when all polling tokens for that networkClientId are deleted, without stopping polling for networkClientIds with active pollingTokens', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); + it('should should stop polling for one networkClientId when all polling tokens for that networkClientId are deleted, without stopping polling for networkClientIds with active pollingTokens', async () => { + controller.setPollWithBlockTracker(getNetworkClientById); - const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); + const pollingToken1 = + controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - const pollingToken2 = controller.startPollingByNetworkClientId('mainnet'); + const pollingToken2 = + controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ]); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); - controller.startPollingByNetworkClientId('goerli'); - await advanceTime({ clock, duration: 5 }); + controller.startPollingByNetworkClientId('goerli'); + await advanceTime({ clock, duration: 5 }); - // 3 blocks have passed for mainnet, 1 for goerli but we only started listening to goerli after 5ms - // so the next block will come at 20ms and be the 2nd block for goerli - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); + // 3 blocks have passed for mainnet, 1 for goerli but we only started listening to goerli after 5ms + // so the next block will come at 20ms and be the 2nd block for goerli + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); - controller.stopPollingByPollingToken(pollingToken1); + controller.stopPollingByPollingToken(pollingToken1); - await advanceTime({ clock, duration: 5 }); + await advanceTime({ clock, duration: 5 }); - // 20ms have passed, 4 blocks for mainnet, 2 for goerli - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ]); + // 20ms have passed, 4 blocks for mainnet, 2 for goerli + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ]); - controller.stopPollingByPollingToken(pollingToken2); + controller.stopPollingByPollingToken(pollingToken2); - await advanceTime({ clock, duration: 20 }); + await advanceTime({ clock, duration: 20 }); - // no further polling for mainnet should occur - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ['goerli', {}, 3], - ['goerli', {}, 4], - ]); + // no further polling for mainnet should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ['goerli', {}, 3], + ['goerli', {}, 4], + ]); - controller.stopAllPolling(); + controller.stopAllPolling(); + }); }); }); }); From 28a98fee1463eab3851a58b17c4ffd53c01a0ff0 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 7 Dec 2023 15:23:20 -0600 Subject: [PATCH 10/22] WIP --- .../src/BlockTrackerPollingController.ts | 97 +++++++ .../src/PollingController-abstract.ts | 114 ++++++++ .../src/PollingController.ts | 271 ------------------ .../src/StaticIntervalPollingController.ts | 75 +++++ packages/polling-controller/src/index.ts | 14 +- 5 files changed, 296 insertions(+), 275 deletions(-) create mode 100644 packages/polling-controller/src/BlockTrackerPollingController.ts create mode 100644 packages/polling-controller/src/PollingController-abstract.ts delete mode 100644 packages/polling-controller/src/PollingController.ts create mode 100644 packages/polling-controller/src/StaticIntervalPollingController.ts diff --git a/packages/polling-controller/src/BlockTrackerPollingController.ts b/packages/polling-controller/src/BlockTrackerPollingController.ts new file mode 100644 index 0000000000..98c49f4986 --- /dev/null +++ b/packages/polling-controller/src/BlockTrackerPollingController.ts @@ -0,0 +1,97 @@ +import { BaseController, BaseControllerV1 } from '@metamask/base-controller'; +import type { + NetworkClientId, + NetworkClient, +} from '@metamask/network-controller'; +import type { Json } from '@metamask/utils'; + +import { + PollingControllerBaseMixin, + getKey, +} from './PollingController-abstract'; +import type { PollingTokenSetId } from './PollingController-abstract'; + +type Constructor = new (...args: any[]) => object; + +/** + * BlockTrackerPollingControllerMixin + * + * @param Base - The base class to mix onto. + * @returns The composed class. + */ +function BlockTrackerPollingControllerMixin( + Base: TBase, +) { + abstract class BlockTrackerPollingController extends PollingControllerBaseMixin( + Base, + ) { + #activeListeners: Record Promise> = {}; + + constructor() { + super(); + this.setPollingStrategy({ + start: (networkClientId: NetworkClientId, options: Json) => + this.startBlockTrackingPolling(networkClientId, options), + stop: (key: PollingTokenSetId) => this.stopBlockTrackingPolling(key), + }); + } + + abstract getNetworkClientById( + networkClientId: NetworkClientId, + ): NetworkClient | undefined; + + abstract _executePoll( + networkClientId: NetworkClientId, + options: Json, + ): Promise; + + startBlockTrackingPolling(networkClientId: NetworkClientId, options: Json) { + const key = getKey(networkClientId, options); + + if (this.#activeListeners[key]) { + return; + } + + const networkClient = this.getNetworkClientById(networkClientId); + if (networkClient && networkClient.blockTracker) { + const updateOnNewBlock = this._executePoll.bind( + this, + networkClientId, + options, + ); + networkClient.blockTracker.addListener('latest', updateOnNewBlock); + this.#activeListeners[key] = updateOnNewBlock; + } else { + throw new Error( + `Unable to retrieve blockTracker for networkClientId ${networkClientId}`, + ); + } + } + + stopBlockTrackingPolling(key: string) { + const [networkClientId] = key.split(':'); + const networkClient = this.getNetworkClientById( + networkClientId as NetworkClientId, + ); + + if (networkClient && this.#activeListeners[key]) { + const listener = this.#activeListeners[key]; + if (listener) { + networkClient.blockTracker.removeListener('latest', listener); + delete this.#activeListeners[key]; + } + } + } + } + + return BlockTrackerPollingController; +} + +class Empty {} + +export const BlockTrackerPollingControllerOnly = + BlockTrackerPollingControllerMixin(Empty); +export const BlockTrackerPollingController = + BlockTrackerPollingControllerMixin(BaseController); +export const BlockTrackerPollingControllerV1 = + BlockTrackerPollingControllerMixin(BaseControllerV1); diff --git a/packages/polling-controller/src/PollingController-abstract.ts b/packages/polling-controller/src/PollingController-abstract.ts new file mode 100644 index 0000000000..74e07c3fbc --- /dev/null +++ b/packages/polling-controller/src/PollingController-abstract.ts @@ -0,0 +1,114 @@ +import type { NetworkClientId } from '@metamask/network-controller'; +import type { Json } from '@metamask/utils'; +import stringify from 'fast-json-stable-stringify'; +import { v4 as random } from 'uuid'; + +export const getKey = ( + networkClientId: NetworkClientId, + options: Json, +): PollingTokenSetId => `${networkClientId}:${stringify(options)}`; + +export type PollingTokenSetId = `${NetworkClientId}:${string}`; + +type PollingStrategy = { + start: (networkClientId: NetworkClientId, options: Json) => void; + stop: (key: PollingTokenSetId) => void; +}; + +type Constructor = new (...args: any[]) => object; + +/** + * PollingControllerBaseMixin + * + * @param Base - The base class to mix onto. + * @returns The composed class. + */ +export function PollingControllerBaseMixin( + Base: TBase, +) { + class PollingControllerBase extends Base { + readonly #pollingTokenSets: Map> = new Map(); + + #strategy: PollingStrategy | undefined; + + #callbacks: Map< + PollingTokenSetId, + Set<(networkClientId: NetworkClientId) => void> + > = new Map(); + + setPollingStrategy(strategy: PollingStrategy) { + this.#strategy = strategy; + } + + startPollingByNetworkClientId( + networkClientId: NetworkClientId, + options: Json = {}, + ): string { + if (!this.#strategy) { + throw new Error('Polling strategy not set'); + } + const pollToken = random(); + const key = getKey(networkClientId, options); + const pollingTokenSet = + this.#pollingTokenSets.get(key) || new Set(); + pollingTokenSet.add(pollToken); + this.#pollingTokenSets.set(key, pollingTokenSet); + + if (pollingTokenSet.size === 1) { + // Start new polling only if it's the first token for this key + this.#strategy.start(networkClientId, options); + } + + return pollToken; + } + + stopAllPolling() { + if (!this.#strategy) { + throw new Error('Polling strategy not set'); + } + + this.#pollingTokenSets.forEach((tokenSet, _key) => { + tokenSet.forEach((token) => { + this.stopPollingByPollingToken(token); + }); + }); + } + + stopPollingByPollingToken(pollingToken: string) { + if (!pollingToken) { + throw new Error('pollingToken required'); + } + + if (!this.#strategy) { + throw new Error('Polling strategy not set'); + } + + let keyToDelete: PollingTokenSetId | null = null; + for (const [key, tokenSet] of this.#pollingTokenSets) { + if (tokenSet.delete(pollingToken)) { + if (tokenSet.size === 0) { + keyToDelete = key; + break; + } + } + } + + if (keyToDelete) { + this.#strategy.stop(keyToDelete); + this.#pollingTokenSets.delete(keyToDelete); + } + } + + onPollingCompleteByNetworkClientId( + networkClientId: NetworkClientId, + callback: (networkClientId: NetworkClientId) => void, + options: Json = {}, + ) { + const key = getKey(networkClientId, options); + const callbacks = this.#callbacks.get(key) || new Set(); + callbacks.add(callback); + this.#callbacks.set(key, callbacks); + } + } + return PollingControllerBase; +} diff --git a/packages/polling-controller/src/PollingController.ts b/packages/polling-controller/src/PollingController.ts deleted file mode 100644 index 4442391316..0000000000 --- a/packages/polling-controller/src/PollingController.ts +++ /dev/null @@ -1,271 +0,0 @@ -import { BaseController, BaseControllerV1 } from '@metamask/base-controller'; -import type { - NetworkClientId, - NetworkClient, -} from '@metamask/network-controller'; -import type { Json } from '@metamask/utils'; -import stringify from 'fast-json-stable-stringify'; -import { v4 as random } from 'uuid'; - -// Mixin classes require a constructor with an `...any[]` parameter -// See TS2545 -type Constructor = new (...args: any[]) => object; - -/** - * Returns a unique key for a networkClientId and options. This is used to group networkClientId polls with the same options - * @param networkClientId - The networkClientId to get a key for - * @param options - The options used to group the polling events - * @returns The unique key - */ -export const getKey = ( - networkClientId: NetworkClientId, - options: Json, -): PollingTokenSetId => `${networkClientId}:${stringify(options)}`; - -type PollingTokenSetId = `${NetworkClientId}:${string}`; -/** - * PollingControllerMixin - * - * @param Base - The base class to mix onto. - * @returns The composed class. - */ -function PollingControllerMixin(Base: TBase) { - /** - * PollingController is an abstract class that implements the polling - * functionality for a controller. It is meant to be extended by a controller - * that needs to poll for data by networkClientId. - * - */ - abstract class PollingControllerBase extends Base { - readonly #pollingTokenSets: Map> = new Map(); - - readonly #intervalIds: Record = {}; - - readonly #activeListeners: Record< - PollingTokenSetId, - (options: Json) => Promise - > = {}; - - #callbacks: Map< - NetworkClientId, - Set<(networkClientId: NetworkClientId) => void> - > = new Map(); - - #intervalLength: number | undefined = 1000; - - #getNetworkClientById: - | ((networkClientId: NetworkClientId) => NetworkClient) - | undefined; - - getIntervalLength() { - return this.#intervalLength; - } - - getPollingWithBlockTracker() { - return this.#getNetworkClientById !== undefined; - } - - /** - * Sets the length of the polling interval - * - * @param length - The length of the polling interval in milliseconds - */ - setIntervalLength(length: number) { - this.#intervalLength = length; - - // setting and using an interval is mutually exclusive with polling on new blocks - this.#getNetworkClientById = undefined; - } - - setPollWithBlockTracker( - getNetworkClientById: (networkClientId: NetworkClientId) => NetworkClient, - ) { - if (!getNetworkClientById) { - throw new Error('getNetworkClientById callback required'); - } - - this.#getNetworkClientById = getNetworkClientById; - - // using block times is mutually exclusive with polling on a static interval - this.#intervalLength = undefined; - } - - /** - * Starts polling for a networkClientId - * - * @param networkClientId - The networkClientId to start polling for - * @param options - The options used to group the polling events - * @returns void - */ - startPollingByNetworkClientId( - networkClientId: NetworkClientId, - options: Json = {}, - ) { - const pollToken = random(); - - const key = getKey(networkClientId, options); - - const pollingTokenSet = this.#pollingTokenSets.get(key); - if (pollingTokenSet) { - pollingTokenSet.add(pollToken); - } else { - const set = new Set(); - set.add(pollToken); - this.#pollingTokenSets.set(key, set); - } - this.#poll(networkClientId, options); - return pollToken; - } - - /** - * Stops polling for all networkClientIds - */ - stopAllPolling() { - this.#pollingTokenSets.forEach((tokenSet, _networkClientId) => { - tokenSet.forEach((token) => { - this.stopPollingByPollingToken(token); - }); - }); - } - - /** - * Stops polling for a networkClientId - * - * @param pollingToken - The polling token to stop polling for - */ - stopPollingByPollingToken(pollingToken: string) { - if (!pollingToken) { - throw new Error('pollingToken required'); - } - let found = false; - this.#pollingTokenSets.forEach((tokenSet, key) => { - if (tokenSet.has(pollingToken)) { - found = true; - tokenSet.delete(pollingToken); - if (tokenSet.size === 0) { - // if applicable stop polling on a static interval - if (this.#intervalIds[key]) { - clearTimeout(this.#intervalIds[key]); - delete this.#intervalIds[key]; - } else if ( - // if applicable stop listening for new blocks - this.#getNetworkClientById !== undefined && - this.#activeListeners[key] - ) { - const [networkClientId] = key.split(':'); - const { blockTracker } = - this.#getNetworkClientById(networkClientId); - if (blockTracker) { - blockTracker.removeListener( - 'latest', - this.#activeListeners[key], - ); - } - delete this.#activeListeners[key]; - } - - this.#pollingTokenSets.delete(key); - this.#callbacks.get(key)?.forEach((callback) => { - callback(key); - }); - this.#callbacks.get(key)?.clear(); - } - } - }); - if (!found) { - throw new Error('pollingToken not found'); - } - } - - /** - * Executes the poll for a networkClientId - * - * @param networkClientId - The networkClientId to execute the poll for - * @param options - The options passed to startPollingByNetworkClientId - */ - abstract _executePoll( - networkClientId: NetworkClientId, - options: Json, - ): Promise; - - #poll(networkClientId: NetworkClientId, options: Json) { - const key = getKey(networkClientId, options); - - // if #getNetworkClientById is defined, we want to poll on new blocks - if (this.#getNetworkClientById !== undefined) { - // if we're already listening for new blocks for this key, don't add another listener - if (this.#activeListeners[key]) { - return; - } - const blockTracker = - this.#getNetworkClientById(networkClientId)?.blockTracker; - - if (blockTracker) { - const updateOnNewBlock = this._executePoll.bind( - this, - networkClientId, - options, - ); - blockTracker.addListener('latest', updateOnNewBlock); - this.#activeListeners[key] = updateOnNewBlock; - return; - } - - throw new Error(` - Unable to retrieve blockTracker for networkClientId ${networkClientId} `); - } - - // if we're not polling on new blocks, use setTimeout - const interval = this.#intervalIds[key]; - if (interval) { - clearTimeout(interval); - delete this.#intervalIds[key]; - } - // setTimeout is not `await`ing this async function, which is expected - // We're just using async here for improved stack traces - // eslint-disable-next-line @typescript-eslint/no-misused-promises - this.#intervalIds[key] = setTimeout( - async () => { - try { - await this._executePoll(networkClientId, options); - } catch (error) { - console.error(error); - } - this.#poll(networkClientId, options); - }, - interval ? this.#intervalLength : 0, - ); - } - - /** - * Adds a callback to execute when polling is complete - * - * @param networkClientId - The networkClientId to listen for polling complete events - * @param callback - The callback to execute when polling is complete - * @param options - The options used to group the polling events - */ - onPollingCompleteByNetworkClientId( - networkClientId: NetworkClientId, - callback: (networkClientId: NetworkClientId) => void, - options: Json = {}, - ) { - const key = getKey(networkClientId, options); - const callbacks = this.#callbacks.get(key); - - if (callbacks === undefined) { - const set = new Set(); - set.add(callback); - this.#callbacks.set(key, set); - } else { - callbacks.add(callback); - } - } - } - return PollingControllerBase; -} - -class Empty {} - -export const PollingControllerOnly = PollingControllerMixin(Empty); -export const PollingController = PollingControllerMixin(BaseController); -export const PollingControllerV1 = PollingControllerMixin(BaseControllerV1); diff --git a/packages/polling-controller/src/StaticIntervalPollingController.ts b/packages/polling-controller/src/StaticIntervalPollingController.ts new file mode 100644 index 0000000000..e1fe111cef --- /dev/null +++ b/packages/polling-controller/src/StaticIntervalPollingController.ts @@ -0,0 +1,75 @@ +import { BaseController, BaseControllerV1 } from '@metamask/base-controller'; +import type { NetworkClientId } from '@metamask/network-controller'; +import type { Json } from '@metamask/utils'; + +import { + PollingControllerBaseMixin, + getKey, +} from './PollingController-abstract'; +import type { PollingTokenSetId } from './PollingController-abstract'; + +type Constructor = new (...args: any[]) => object; + +/** + * BlockTrackerPollingControllerMixin + * + * @param Base - The base class to mix onto. + * @returns The composed class. + */ +function StaticIntervalPollingControllerMixin( + Base: TBase, +) { + abstract class StaticIntervalPollingController extends PollingControllerBaseMixin( + Base, + ) { + readonly #intervalIds: Record = {}; + + constructor() { + super(); + this.setPollingStrategy({ + start: (networkClientId: NetworkClientId, options: Json) => + this.startStaticIntervalPolling(networkClientId, options), + stop: (key: PollingTokenSetId) => this.stopStaticIntervalPolling(key), + }); + } + + abstract _executePoll( + networkClientId: NetworkClientId, + options: Json, + ): Promise; + + abstract getIntervalLength(): number; + + startStaticIntervalPolling( + networkClientId: NetworkClientId, + options: Json, + ) { + const key = getKey(networkClientId, options); + + if (!this.#intervalIds[key]) { + this.#intervalIds[key] = setInterval(() => { + this._executePoll(networkClientId, options).catch(console.error); + }, this.getIntervalLength()); + } + } + + stopStaticIntervalPolling(key: PollingTokenSetId) { + const intervalId = this.#intervalIds[key]; + if (intervalId) { + clearInterval(intervalId); + delete this.#intervalIds[key]; + } + } + } + + return StaticIntervalPollingController; +} + +class Empty {} + +export const StaticIntervalPollingControllerOnly = + StaticIntervalPollingControllerMixin(Empty); +export const StaticIntervalPollingController = + StaticIntervalPollingControllerMixin(BaseController); +export const StaticIntervalPollingControllerV1 = + StaticIntervalPollingControllerMixin(BaseControllerV1); diff --git a/packages/polling-controller/src/index.ts b/packages/polling-controller/src/index.ts index e5ed2df383..38997dda91 100644 --- a/packages/polling-controller/src/index.ts +++ b/packages/polling-controller/src/index.ts @@ -1,5 +1,11 @@ export { - PollingController, - PollingControllerV1, - PollingControllerOnly, -} from './PollingController'; + BlockTrackerPollingControllerOnly, + BlockTrackerPollingController, + BlockTrackerPollingControllerV1, +} from './BlockTrackerPollingController'; + +export { + StaticIntervalPollingControllerOnly, + StaticIntervalPollingController, + StaticIntervalPollingControllerV1, +} from './StaticIntervalPollingController'; From 1c297c228cd4ba8cfb510c0ede28f39d05b96169 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 8 Dec 2023 10:29:14 -0600 Subject: [PATCH 11/22] moving toward pure template pattern --- .../src/BlockTrackerPollingController.ts | 9 ++------ .../src/PollingController-abstract.ts | 22 +++++++++---------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/packages/polling-controller/src/BlockTrackerPollingController.ts b/packages/polling-controller/src/BlockTrackerPollingController.ts index 98c49f4986..13a8e03eb5 100644 --- a/packages/polling-controller/src/BlockTrackerPollingController.ts +++ b/packages/polling-controller/src/BlockTrackerPollingController.ts @@ -27,13 +27,8 @@ function BlockTrackerPollingControllerMixin( ) { #activeListeners: Record Promise> = {}; - constructor() { - super(); - this.setPollingStrategy({ - start: (networkClientId: NetworkClientId, options: Json) => - this.startBlockTrackingPolling(networkClientId, options), - stop: (key: PollingTokenSetId) => this.stopBlockTrackingPolling(key), - }); + _start(networkClientId: NetworkClientId, options: Json) { + this.startBlockTrackingPolling(networkClientId, options); } abstract getNetworkClientById( diff --git a/packages/polling-controller/src/PollingController-abstract.ts b/packages/polling-controller/src/PollingController-abstract.ts index 74e07c3fbc..14d274813f 100644 --- a/packages/polling-controller/src/PollingController-abstract.ts +++ b/packages/polling-controller/src/PollingController-abstract.ts @@ -18,15 +18,15 @@ type PollingStrategy = { type Constructor = new (...args: any[]) => object; /** - * PollingControllerBaseMixin + * AbstractPollingControllerBaseMixin * * @param Base - The base class to mix onto. * @returns The composed class. */ -export function PollingControllerBaseMixin( +export function AbstractPollingControllerBaseMixin( Base: TBase, ) { - class PollingControllerBase extends Base { + abstract class AbstractPollingControllerBase extends Base { readonly #pollingTokenSets: Map> = new Map(); #strategy: PollingStrategy | undefined; @@ -36,9 +36,13 @@ export function PollingControllerBaseMixin( Set<(networkClientId: NetworkClientId) => void> > = new Map(); - setPollingStrategy(strategy: PollingStrategy) { - this.#strategy = strategy; - } + // setPollingStrategy(strategy: PollingStrategy) { + // this.#strategy = strategy; + // } + + abstract _start(networkClientId: NetworkClientId, options: Json): void; + + abstract _stop(key: PollingTokenSetId): void; startPollingByNetworkClientId( networkClientId: NetworkClientId, @@ -63,10 +67,6 @@ export function PollingControllerBaseMixin( } stopAllPolling() { - if (!this.#strategy) { - throw new Error('Polling strategy not set'); - } - this.#pollingTokenSets.forEach((tokenSet, _key) => { tokenSet.forEach((token) => { this.stopPollingByPollingToken(token); @@ -110,5 +110,5 @@ export function PollingControllerBaseMixin( this.#callbacks.set(key, callbacks); } } - return PollingControllerBase; + return AbstractPollingControllerBase; } From 9f0efe95f309a2f50f35c531636923c6a1424ca1 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 8 Dec 2023 12:36:22 -0600 Subject: [PATCH 12/22] wip template pattern complete for BlockTrackerPollingController, now to StaticIntervalPollingController --- ...stract.ts => AbstractPollingController.ts} | 32 +-- .../src/BlockTrackerPollingController.test.ts | 268 ++++++++++++++++++ .../src/BlockTrackerPollingController.ts | 27 +- .../src/StaticIntervalPollingController.ts | 4 +- 4 files changed, 292 insertions(+), 39 deletions(-) rename packages/polling-controller/src/{PollingController-abstract.ts => AbstractPollingController.ts} (79%) create mode 100644 packages/polling-controller/src/BlockTrackerPollingController.test.ts diff --git a/packages/polling-controller/src/PollingController-abstract.ts b/packages/polling-controller/src/AbstractPollingController.ts similarity index 79% rename from packages/polling-controller/src/PollingController-abstract.ts rename to packages/polling-controller/src/AbstractPollingController.ts index 14d274813f..8d7d725abb 100644 --- a/packages/polling-controller/src/PollingController-abstract.ts +++ b/packages/polling-controller/src/AbstractPollingController.ts @@ -10,11 +10,6 @@ export const getKey = ( export type PollingTokenSetId = `${NetworkClientId}:${string}`; -type PollingStrategy = { - start: (networkClientId: NetworkClientId, options: Json) => void; - stop: (key: PollingTokenSetId) => void; -}; - type Constructor = new (...args: any[]) => object; /** @@ -29,28 +24,27 @@ export function AbstractPollingControllerBaseMixin( abstract class AbstractPollingControllerBase extends Base { readonly #pollingTokenSets: Map> = new Map(); - #strategy: PollingStrategy | undefined; - #callbacks: Map< PollingTokenSetId, Set<(networkClientId: NetworkClientId) => void> > = new Map(); - // setPollingStrategy(strategy: PollingStrategy) { - // this.#strategy = strategy; - // } + abstract _executePoll( + networkClientId: NetworkClientId, + options: Json, + ): Promise; - abstract _start(networkClientId: NetworkClientId, options: Json): void; + abstract _startPollingByNetworkClientId( + networkClientId: NetworkClientId, + options: Json, + ): void; - abstract _stop(key: PollingTokenSetId): void; + abstract _stopPollingByPollingTokenSetId(key: PollingTokenSetId): void; startPollingByNetworkClientId( networkClientId: NetworkClientId, options: Json = {}, ): string { - if (!this.#strategy) { - throw new Error('Polling strategy not set'); - } const pollToken = random(); const key = getKey(networkClientId, options); const pollingTokenSet = @@ -60,7 +54,7 @@ export function AbstractPollingControllerBaseMixin( if (pollingTokenSet.size === 1) { // Start new polling only if it's the first token for this key - this.#strategy.start(networkClientId, options); + this._startPollingByNetworkClientId(networkClientId, options); } return pollToken; @@ -79,10 +73,6 @@ export function AbstractPollingControllerBaseMixin( throw new Error('pollingToken required'); } - if (!this.#strategy) { - throw new Error('Polling strategy not set'); - } - let keyToDelete: PollingTokenSetId | null = null; for (const [key, tokenSet] of this.#pollingTokenSets) { if (tokenSet.delete(pollingToken)) { @@ -94,7 +84,7 @@ export function AbstractPollingControllerBaseMixin( } if (keyToDelete) { - this.#strategy.stop(keyToDelete); + this._stopPollingByPollingTokenSetId(keyToDelete); this.#pollingTokenSets.delete(keyToDelete); } } diff --git a/packages/polling-controller/src/BlockTrackerPollingController.test.ts b/packages/polling-controller/src/BlockTrackerPollingController.test.ts new file mode 100644 index 0000000000..6c6323e719 --- /dev/null +++ b/packages/polling-controller/src/BlockTrackerPollingController.test.ts @@ -0,0 +1,268 @@ +import { ControllerMessenger } from '@metamask/base-controller'; +import type { NetworkClient } from '@metamask/network-controller'; +import EventEmitter from 'events'; +import { useFakeTimers } from 'sinon'; + +import { advanceTime } from '../../../tests/helpers'; +import { BlockTrackerPollingController } from './BlockTrackerPollingController'; + +const createExecutePollMock = () => { + const executePollMock = jest.fn().mockImplementation(async () => { + return true; + }); + return executePollMock; +}; + +let getNetworkClientByIdStub: jest.Mock; +class MyGasFeeController extends BlockTrackerPollingController { + _executePoll = createExecutePollMock(); + + _getNetworkClientById(networkClientId: string): NetworkClient | undefined { + return getNetworkClientByIdStub(networkClientId); + } +} + +describe('PollingController', () => { + let clock: sinon.SinonFakeTimers; + let mockMessenger: any; + let controller: any; + beforeEach(() => { + mockMessenger = new ControllerMessenger(); + controller = new MyGasFeeController({ + messenger: mockMessenger, + metadata: {}, + name: 'PollingController', + state: { foo: 'bar' }, + }); + clock = useFakeTimers(); + }); + afterEach(() => { + clock.restore(); + }); + + describe('Polling on block times', () => { + class TestBlockTracker extends EventEmitter { + private latestBlockNumber: number; + + public interval: number; + + constructor({ interval } = { interval: 1000 }) { + super(); + this.latestBlockNumber = 0; + this.interval = interval; + this.start(interval); + } + + private start(interval: number) { + setInterval(() => { + this.latestBlockNumber += 1; + this.emit('latest', this.latestBlockNumber); + }, interval); + } + } + + let mainnetBlockTracker: TestBlockTracker; + let goerliBlockTracker: TestBlockTracker; + let sepoliaBlockTracker: TestBlockTracker; + beforeEach(() => { + mainnetBlockTracker = new TestBlockTracker({ interval: 5 }); + goerliBlockTracker = new TestBlockTracker({ interval: 10 }); + sepoliaBlockTracker = new TestBlockTracker({ interval: 15 }); + + getNetworkClientByIdStub = jest + .fn() + .mockImplementation((networkClientId) => { + switch (networkClientId) { + case 'mainnet': + return { + blockTracker: mainnetBlockTracker, + }; + case 'goerli': + return { + blockTracker: goerliBlockTracker, + }; + case 'sepolia': + return { + blockTracker: sepoliaBlockTracker, + }; + default: + throw new Error(`Unknown networkClientId: ${networkClientId}`); + } + }); + }); + + describe('startPollingByNetworkClientId', () => { + it('should start polling for the specified networkClientId', async () => { + controller.startPollingByNetworkClientId('mainnet'); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + + await advanceTime({ clock, duration: 1 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + + await advanceTime({ clock, duration: 4 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + expect.arrayContaining(['mainnet', {}]), + expect.arrayContaining(['mainnet', {}]), + ]); + + controller.stopAllPolling(); + }); + + it('should poll on new block intervals for each networkClientId', async () => { + controller.startPollingByNetworkClientId('mainnet'); + controller.startPollingByNetworkClientId('goerli'); + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ]); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ['mainnet', {}, 3], + ]); + + // 15ms have passed + // Start polling for sepolia, 15ms interval + controller.startPollingByNetworkClientId('sepolia'); + + await advanceTime({ clock, duration: 15 }); + + // at 30ms, 6 blocks have passed for mainnet (every 5ms), 3 for goerli (every 10ms), and 2 for sepolia (every 15ms) + // Didn't start listening to sepolia until 15ms had passed, so we only call executePoll on the 2nd block + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['goerli', {}, 1], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ['mainnet', {}, 5], + ['mainnet', {}, 6], + ['goerli', {}, 3], + ['sepolia', {}, 2], + ]); + + controller.stopAllPolling(); + }); + }); + + describe('stopPollingByPollingToken', () => { + it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { + const pollingToken1 = + controller.startPollingByNetworkClientId('mainnet'); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + + const pollingToken2 = + controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); + + controller.stopPollingByPollingToken(pollingToken1); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); + + controller.stopPollingByPollingToken(pollingToken2); + + await advanceTime({ clock, duration: 15 }); + + // no further polling should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); + }); + + it('should should stop polling for one networkClientId when all polling tokens for that networkClientId are deleted, without stopping polling for networkClientIds with active pollingTokens', async () => { + const pollingToken1 = + controller.startPollingByNetworkClientId('mainnet'); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + + const pollingToken2 = + controller.startPollingByNetworkClientId('mainnet'); + + await advanceTime({ clock, duration: 5 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); + + controller.startPollingByNetworkClientId('goerli'); + await advanceTime({ clock, duration: 5 }); + + // 3 blocks have passed for mainnet, 1 for goerli but we only started listening to goerli after 5ms + // so the next block will come at 20ms and be the 2nd block for goerli + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); + + controller.stopPollingByPollingToken(pollingToken1); + + await advanceTime({ clock, duration: 5 }); + + // 20ms have passed, 4 blocks for mainnet, 2 for goerli + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ]); + + controller.stopPollingByPollingToken(pollingToken2); + + await advanceTime({ clock, duration: 20 }); + + // no further polling for mainnet should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 2], + ['goerli', {}, 3], + ['goerli', {}, 4], + ]); + + controller.stopAllPolling(); + }); + }); + }); +}); diff --git a/packages/polling-controller/src/BlockTrackerPollingController.ts b/packages/polling-controller/src/BlockTrackerPollingController.ts index 13a8e03eb5..9beb21ffd9 100644 --- a/packages/polling-controller/src/BlockTrackerPollingController.ts +++ b/packages/polling-controller/src/BlockTrackerPollingController.ts @@ -6,10 +6,10 @@ import type { import type { Json } from '@metamask/utils'; import { - PollingControllerBaseMixin, + AbstractPollingControllerBaseMixin, getKey, -} from './PollingController-abstract'; -import type { PollingTokenSetId } from './PollingController-abstract'; +} from './AbstractPollingController'; +import type { PollingTokenSetId } from './AbstractPollingController'; type Constructor = new (...args: any[]) => object; @@ -22,33 +22,28 @@ type Constructor = new (...args: any[]) => object; function BlockTrackerPollingControllerMixin( Base: TBase, ) { - abstract class BlockTrackerPollingController extends PollingControllerBaseMixin( + abstract class BlockTrackerPollingController extends AbstractPollingControllerBaseMixin( Base, ) { #activeListeners: Record Promise> = {}; - _start(networkClientId: NetworkClientId, options: Json) { - this.startBlockTrackingPolling(networkClientId, options); - } - - abstract getNetworkClientById( + abstract _getNetworkClientById( networkClientId: NetworkClientId, ): NetworkClient | undefined; - abstract _executePoll( + _startPollingByNetworkClientId( networkClientId: NetworkClientId, options: Json, - ): Promise; - - startBlockTrackingPolling(networkClientId: NetworkClientId, options: Json) { + ) { const key = getKey(networkClientId, options); if (this.#activeListeners[key]) { return; } - const networkClient = this.getNetworkClientById(networkClientId); + const networkClient = this._getNetworkClientById(networkClientId); if (networkClient && networkClient.blockTracker) { + console.log('networkClient', networkClient); const updateOnNewBlock = this._executePoll.bind( this, networkClientId, @@ -63,9 +58,9 @@ function BlockTrackerPollingControllerMixin( } } - stopBlockTrackingPolling(key: string) { + _stopPollingByPollingTokenSetId(key: PollingTokenSetId) { const [networkClientId] = key.split(':'); - const networkClient = this.getNetworkClientById( + const networkClient = this._getNetworkClientById( networkClientId as NetworkClientId, ); diff --git a/packages/polling-controller/src/StaticIntervalPollingController.ts b/packages/polling-controller/src/StaticIntervalPollingController.ts index e1fe111cef..82a35a8ed1 100644 --- a/packages/polling-controller/src/StaticIntervalPollingController.ts +++ b/packages/polling-controller/src/StaticIntervalPollingController.ts @@ -5,8 +5,8 @@ import type { Json } from '@metamask/utils'; import { PollingControllerBaseMixin, getKey, -} from './PollingController-abstract'; -import type { PollingTokenSetId } from './PollingController-abstract'; +} from './AbstractPollingController'; +import type { PollingTokenSetId } from './AbstractPollingController'; type Constructor = new (...args: any[]) => object; From 64043055668698e39f95accc4cfffe30c550a085 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 8 Dec 2023 13:56:35 -0600 Subject: [PATCH 13/22] StaticIntervalPollingController wip --- .../src/BlockTrackerPollingController.ts | 1 + .../StaticIntervalPollingController.test.ts | 248 ++++++++++++++++++ .../src/StaticIntervalPollingController.ts | 49 ++-- 3 files changed, 273 insertions(+), 25 deletions(-) create mode 100644 packages/polling-controller/src/StaticIntervalPollingController.test.ts diff --git a/packages/polling-controller/src/BlockTrackerPollingController.ts b/packages/polling-controller/src/BlockTrackerPollingController.ts index 9beb21ffd9..313390686e 100644 --- a/packages/polling-controller/src/BlockTrackerPollingController.ts +++ b/packages/polling-controller/src/BlockTrackerPollingController.ts @@ -15,6 +15,7 @@ type Constructor = new (...args: any[]) => object; /** * BlockTrackerPollingControllerMixin + * A polling controller that polls using a block tracker. * * @param Base - The base class to mix onto. * @returns The composed class. diff --git a/packages/polling-controller/src/StaticIntervalPollingController.test.ts b/packages/polling-controller/src/StaticIntervalPollingController.test.ts new file mode 100644 index 0000000000..624dc1ceb9 --- /dev/null +++ b/packages/polling-controller/src/StaticIntervalPollingController.test.ts @@ -0,0 +1,248 @@ +import { ControllerMessenger } from '@metamask/base-controller'; +import { useFakeTimers } from 'sinon'; + +import { advanceTime } from '../../../tests/helpers'; +import { StaticIntervalPollingController } from './StaticIntervalPollingController'; + +const TICK_TIME = 1000; + +const createExecutePollMock = () => { + const executePollMock = jest.fn().mockImplementation(async () => { + return true; + }); + return executePollMock; +}; + +class MyGasFeeController extends StaticIntervalPollingController< + any, + any, + any +> { + _executePoll = createExecutePollMock(); + + _intervalLength = TICK_TIME; +} + +describe('PollingController', () => { + let clock: sinon.SinonFakeTimers; + let mockMessenger: any; + let controller: any; + beforeEach(() => { + mockMessenger = new ControllerMessenger(); + controller = new MyGasFeeController({ + messenger: mockMessenger, + metadata: {}, + name: 'PollingController', + state: { foo: 'bar' }, + }); + clock = useFakeTimers(); + }); + afterEach(() => { + clock.restore(); + }); + + describe('startPollingByNetworkClientId', () => { + it('should start polling if not polling', async () => { + controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 0 }); + expect(controller._executePoll).toHaveBeenCalledTimes(1); + await advanceTime({ clock, duration: TICK_TIME }); + expect(controller._executePoll).toHaveBeenCalledTimes(2); + controller.stopAllPolling(); + }); + it('should call _executePoll immediately and on interval if polling', async () => { + controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 0 }); + expect(controller._executePoll).toHaveBeenCalledTimes(1); + await advanceTime({ clock, duration: TICK_TIME * 2 }); + expect(controller._executePoll).toHaveBeenCalledTimes(3); + }); + it('should call _executePoll immediately once and continue calling _executePoll on interval when start is called again with the same networkClientId', async () => { + controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 0 }); + + controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 0 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + await advanceTime({ clock, duration: TICK_TIME * 2 }); + + expect(controller._executePoll).toHaveBeenCalledTimes(3); + controller.stopAllPolling(); + }); + describe('multiple networkClientIds', () => { + it('should poll for each networkClientId', async () => { + controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 0 }); + + controller.startPollingByNetworkClientId('rinkeby'); + await advanceTime({ clock, duration: 0 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}], + ['rinkeby', {}], + ]); + await advanceTime({ clock, duration: TICK_TIME }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}], + ['rinkeby', {}], + ['mainnet', {}], + ['rinkeby', {}], + ]); + await advanceTime({ clock, duration: TICK_TIME }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}], + ['rinkeby', {}], + ['mainnet', {}], + ['rinkeby', {}], + ['mainnet', {}], + ['rinkeby', {}], + ]); + controller.stopAllPolling(); + }); + + it('should poll multiple networkClientIds when setting interval length', async () => { + controller.setIntervalLength(TICK_TIME * 2); + controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 0 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}], + ]); + await advanceTime({ clock, duration: TICK_TIME }); + + controller.startPollingByNetworkClientId('sepolia'); + await advanceTime({ clock, duration: 0 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}], + ['sepolia', {}], + ]); + await advanceTime({ clock, duration: TICK_TIME }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}], + ['sepolia', {}], + ['mainnet', {}], + ]); + await advanceTime({ clock, duration: TICK_TIME }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}], + ['sepolia', {}], + ['mainnet', {}], + ['sepolia', {}], + ]); + await advanceTime({ clock, duration: TICK_TIME }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}], + ['sepolia', {}], + ['mainnet', {}], + ['sepolia', {}], + ['mainnet', {}], + ]); + await advanceTime({ clock, duration: TICK_TIME }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}], + ['sepolia', {}], + ['mainnet', {}], + ['sepolia', {}], + ['mainnet', {}], + ['sepolia', {}], + ]); + }); + }); + }); + + describe('stopPollingByPollingToken', () => { + it('should stop polling when called with a valid polling that was the only active pollingToken for a given networkClient', async () => { + const pollingToken = controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 0 }); + expect(controller._executePoll).toHaveBeenCalledTimes(1); + await advanceTime({ clock, duration: TICK_TIME }); + controller.stopPollingByPollingToken(pollingToken); + await advanceTime({ clock, duration: TICK_TIME }); + expect(controller._executePoll).toHaveBeenCalledTimes(2); + controller.stopAllPolling(); + }); + it('should not stop polling if called with one of multiple active polling tokens for a given networkClient', async () => { + const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); + await advanceTime({ clock, duration: 0 }); + + controller.startPollingByNetworkClientId('mainnet'); + expect(controller._executePoll).toHaveBeenCalledTimes(1); + await advanceTime({ clock, duration: TICK_TIME }); + controller.stopPollingByPollingToken(pollingToken1); + await advanceTime({ clock, duration: TICK_TIME }); + expect(controller._executePoll).toHaveBeenCalledTimes(3); + controller.stopAllPolling(); + }); + it('should error if no pollingToken is passed', () => { + controller.startPollingByNetworkClientId('mainnet'); + expect(() => { + controller.stopPollingByPollingToken(undefined as unknown as any); + }).toThrow('pollingToken required'); + controller.stopAllPolling(); + }); + it('should error if no matching pollingToken is found', () => { + controller.startPollingByNetworkClientId('mainnet'); + expect(() => { + controller.stopPollingByPollingToken('potato'); + }).toThrow('pollingToken not found'); + controller.stopAllPolling(); + }); + + it('should publish "pollingComplete" when stop is called', async () => { + const pollingComplete: any = jest.fn(); + controller.onPollingCompleteByNetworkClientId('mainnet', pollingComplete); + const pollingToken = controller.startPollingByNetworkClientId('mainnet'); + controller.stopPollingByPollingToken(pollingToken); + expect(pollingComplete).toHaveBeenCalledTimes(1); + }); + + it('should start and stop polling sessions for different networkClientIds with the same options', async () => { + controller.setIntervalLength(TICK_TIME); + const pollToken1 = controller.startPollingByNetworkClientId('mainnet', { + address: '0x1', + }); + controller.startPollingByNetworkClientId('mainnet', { address: '0x2' }); + await advanceTime({ clock, duration: 0 }); + + controller.startPollingByNetworkClientId('sepolia', { address: '0x2' }); + await advanceTime({ clock, duration: 0 }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', { address: '0x1' }], + ['mainnet', { address: '0x2' }], + ['sepolia', { address: '0x2' }], + ]); + await advanceTime({ clock, duration: TICK_TIME }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', { address: '0x1' }], + ['mainnet', { address: '0x2' }], + ['sepolia', { address: '0x2' }], + ['mainnet', { address: '0x1' }], + ['mainnet', { address: '0x2' }], + ['sepolia', { address: '0x2' }], + ]); + controller.stopPollingByPollingToken(pollToken1); + await advanceTime({ clock, duration: TICK_TIME }); + + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', { address: '0x1' }], + ['mainnet', { address: '0x2' }], + ['sepolia', { address: '0x2' }], + ['mainnet', { address: '0x1' }], + ['mainnet', { address: '0x2' }], + ['sepolia', { address: '0x2' }], + ['mainnet', { address: '0x2' }], + ['sepolia', { address: '0x2' }], + ]); + }); + }); +}); diff --git a/packages/polling-controller/src/StaticIntervalPollingController.ts b/packages/polling-controller/src/StaticIntervalPollingController.ts index 82a35a8ed1..d80b7928b4 100644 --- a/packages/polling-controller/src/StaticIntervalPollingController.ts +++ b/packages/polling-controller/src/StaticIntervalPollingController.ts @@ -3,7 +3,7 @@ import type { NetworkClientId } from '@metamask/network-controller'; import type { Json } from '@metamask/utils'; import { - PollingControllerBaseMixin, + AbstractPollingControllerBaseMixin, getKey, } from './AbstractPollingController'; import type { PollingTokenSetId } from './AbstractPollingController'; @@ -11,7 +11,8 @@ import type { PollingTokenSetId } from './AbstractPollingController'; type Constructor = new (...args: any[]) => object; /** - * BlockTrackerPollingControllerMixin + * StaticIntervalPollingControllerMixin + * A polling controller that polls on a static interval. * * @param Base - The base class to mix onto. * @returns The composed class. @@ -19,44 +20,42 @@ type Constructor = new (...args: any[]) => object; function StaticIntervalPollingControllerMixin( Base: TBase, ) { - abstract class StaticIntervalPollingController extends PollingControllerBaseMixin( + abstract class StaticIntervalPollingController extends AbstractPollingControllerBaseMixin( Base, ) { readonly #intervalIds: Record = {}; - constructor() { - super(); - this.setPollingStrategy({ - start: (networkClientId: NetworkClientId, options: Json) => - this.startStaticIntervalPolling(networkClientId, options), - stop: (key: PollingTokenSetId) => this.stopStaticIntervalPolling(key), - }); - } - - abstract _executePoll( - networkClientId: NetworkClientId, - options: Json, - ): Promise; + abstract _intervalLength: number; - abstract getIntervalLength(): number; + getIntervalLength() { + return this._intervalLength; + } - startStaticIntervalPolling( + _startPollingByNetworkClientId( networkClientId: NetworkClientId, options: Json, ) { const key = getKey(networkClientId, options); + const existingInterval = this.#intervalIds[key]; + this._stopPollingByPollingTokenSetId(key); - if (!this.#intervalIds[key]) { - this.#intervalIds[key] = setInterval(() => { - this._executePoll(networkClientId, options).catch(console.error); - }, this.getIntervalLength()); - } + this.#intervalIds[key] = setTimeout( + async () => { + try { + await this._executePoll(networkClientId, options); + } catch (error) { + console.error(error); + } + this._startPollingByNetworkClientId(networkClientId, options); + }, + existingInterval ? this._intervalLength : 0, + ); } - stopStaticIntervalPolling(key: PollingTokenSetId) { + _stopPollingByPollingTokenSetId(key: PollingTokenSetId) { const intervalId = this.#intervalIds[key]; if (intervalId) { - clearInterval(intervalId); + clearTimeout(intervalId); delete this.#intervalIds[key]; } } From d4992ff53ff0b7ed606ae8cedfcc245989ee7c91 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 8 Dec 2023 14:23:09 -0600 Subject: [PATCH 14/22] trying a different PollingTokenSetId type --- .../src/AbstractPollingController.ts | 21 +++++++++++++------ .../StaticIntervalPollingController.test.ts | 8 +------ .../src/StaticIntervalPollingController.ts | 14 ++++++++++--- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/packages/polling-controller/src/AbstractPollingController.ts b/packages/polling-controller/src/AbstractPollingController.ts index 8d7d725abb..b23ae89d60 100644 --- a/packages/polling-controller/src/AbstractPollingController.ts +++ b/packages/polling-controller/src/AbstractPollingController.ts @@ -6,9 +6,12 @@ import { v4 as random } from 'uuid'; export const getKey = ( networkClientId: NetworkClientId, options: Json, -): PollingTokenSetId => `${networkClientId}:${stringify(options)}`; +): PollingTokenSetId => + `${networkClientId}${options ? `:${stringify(options)}` : ''}`; -export type PollingTokenSetId = `${NetworkClientId}:${string}`; +export type PollingTokenSetId = + | `${NetworkClientId}:${string}` + | NetworkClientId; type Constructor = new (...args: any[]) => object; @@ -26,7 +29,7 @@ export function AbstractPollingControllerBaseMixin( #callbacks: Map< PollingTokenSetId, - Set<(networkClientId: NetworkClientId) => void> + Set<(PollingTokenSetId: PollingTokenSetId) => void> > = new Map(); abstract _executePoll( @@ -43,7 +46,7 @@ export function AbstractPollingControllerBaseMixin( startPollingByNetworkClientId( networkClientId: NetworkClientId, - options: Json = {}, + options: Json, ): string { const pollToken = random(); const key = getKey(networkClientId, options); @@ -84,8 +87,14 @@ export function AbstractPollingControllerBaseMixin( } if (keyToDelete) { - this._stopPollingByPollingTokenSetId(keyToDelete); - this.#pollingTokenSets.delete(keyToDelete); + // TODO figure out why this is necessary + const nonNullKey = keyToDelete; + this._stopPollingByPollingTokenSetId(nonNullKey); + this.#pollingTokenSets.delete(nonNullKey); + this.#callbacks.get(nonNullKey)?.forEach((callback) => { + callback(nonNullKey); + }); + this.#callbacks.get(nonNullKey)?.clear(); } } diff --git a/packages/polling-controller/src/StaticIntervalPollingController.test.ts b/packages/polling-controller/src/StaticIntervalPollingController.test.ts index 624dc1ceb9..fdf03f7ccd 100644 --- a/packages/polling-controller/src/StaticIntervalPollingController.test.ts +++ b/packages/polling-controller/src/StaticIntervalPollingController.test.ts @@ -188,13 +188,6 @@ describe('PollingController', () => { }).toThrow('pollingToken required'); controller.stopAllPolling(); }); - it('should error if no matching pollingToken is found', () => { - controller.startPollingByNetworkClientId('mainnet'); - expect(() => { - controller.stopPollingByPollingToken('potato'); - }).toThrow('pollingToken not found'); - controller.stopAllPolling(); - }); it('should publish "pollingComplete" when stop is called', async () => { const pollingComplete: any = jest.fn(); @@ -202,6 +195,7 @@ describe('PollingController', () => { const pollingToken = controller.startPollingByNetworkClientId('mainnet'); controller.stopPollingByPollingToken(pollingToken); expect(pollingComplete).toHaveBeenCalledTimes(1); + expect(pollingComplete).toHaveBeenCalledWith('mainnet'); }); it('should start and stop polling sessions for different networkClientIds with the same options', async () => { diff --git a/packages/polling-controller/src/StaticIntervalPollingController.ts b/packages/polling-controller/src/StaticIntervalPollingController.ts index d80b7928b4..255da9e799 100644 --- a/packages/polling-controller/src/StaticIntervalPollingController.ts +++ b/packages/polling-controller/src/StaticIntervalPollingController.ts @@ -25,16 +25,24 @@ function StaticIntervalPollingControllerMixin( ) { readonly #intervalIds: Record = {}; - abstract _intervalLength: number; + intervalLength: number | undefined = 1000; + + setIntervalLength(intervalLength: number) { + this.intervalLength = intervalLength; + } getIntervalLength() { - return this._intervalLength; + return this.intervalLength; } _startPollingByNetworkClientId( networkClientId: NetworkClientId, options: Json, ) { + if (!this.intervalLength) { + throw new Error('intervalLength must be defined and greater than 0'); + } + const key = getKey(networkClientId, options); const existingInterval = this.#intervalIds[key]; this._stopPollingByPollingTokenSetId(key); @@ -48,7 +56,7 @@ function StaticIntervalPollingControllerMixin( } this._startPollingByNetworkClientId(networkClientId, options); }, - existingInterval ? this._intervalLength : 0, + existingInterval ? this.intervalLength : 0, ); } From 4b52a02b4c8146e0177c749499c402593bae53ea Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 8 Dec 2023 14:33:39 -0600 Subject: [PATCH 15/22] change all instances over to new class --- .../assets-controllers/src/CurrencyRateController.ts | 4 ++-- .../assets-controllers/src/NftDetectionController.ts | 4 ++-- .../assets-controllers/src/TokenDetectionController.ts | 4 ++-- packages/assets-controllers/src/TokenListController.ts | 4 ++-- packages/assets-controllers/src/TokenRatesController.ts | 4 ++-- packages/gas-fee-controller/src/GasFeeController.ts | 4 ++-- .../polling-controller/src/AbstractPollingController.ts | 9 +++------ .../src/BlockTrackerPollingController.test.ts | 2 +- .../polling-controller/src/PollingController.test.ts | 4 ++-- .../src/StaticIntervalPollingController.test.ts | 4 ++-- 10 files changed, 20 insertions(+), 23 deletions(-) diff --git a/packages/assets-controllers/src/CurrencyRateController.ts b/packages/assets-controllers/src/CurrencyRateController.ts index 41c1eee8fc..bf3509f310 100644 --- a/packages/assets-controllers/src/CurrencyRateController.ts +++ b/packages/assets-controllers/src/CurrencyRateController.ts @@ -11,7 +11,7 @@ import type { NetworkClientId, NetworkControllerGetNetworkClientByIdAction, } from '@metamask/network-controller'; -import { PollingController } from '@metamask/polling-controller'; +import { StaticIntervalPollingController } from '@metamask/polling-controller'; import { Mutex } from 'async-mutex'; import { fetchExchangeRate as defaultFetchExchangeRate } from './crypto-compare'; @@ -82,7 +82,7 @@ const defaultState = { * Controller that passively polls on a set interval for an exchange rate from the current network * asset to the user's preferred currency. */ -export class CurrencyRateController extends PollingController< +export class CurrencyRateController extends StaticIntervalPollingController< typeof name, CurrencyRateState, CurrencyRateMessenger diff --git a/packages/assets-controllers/src/NftDetectionController.ts b/packages/assets-controllers/src/NftDetectionController.ts index 5ed01bca72..9db76bb073 100644 --- a/packages/assets-controllers/src/NftDetectionController.ts +++ b/packages/assets-controllers/src/NftDetectionController.ts @@ -11,7 +11,7 @@ import type { NetworkState, NetworkClient, } from '@metamask/network-controller'; -import { PollingControllerV1 } from '@metamask/polling-controller'; +import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; @@ -147,7 +147,7 @@ export interface NftDetectionConfig extends BaseConfig { /** * Controller that passively polls on a set interval for NFT auto detection */ -export class NftDetectionController extends PollingControllerV1< +export class NftDetectionController extends StaticIntervalPollingControllerV1< NftDetectionConfig, BaseState > { diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 2b877058e1..30a3249f18 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -8,7 +8,7 @@ import type { NetworkController, NetworkState, } from '@metamask/network-controller'; -import { PollingControllerV1 } from '@metamask/polling-controller'; +import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; @@ -44,7 +44,7 @@ export interface TokenDetectionConfig extends BaseConfig { /** * Controller that passively polls on a set interval for Tokens auto detection */ -export class TokenDetectionController extends PollingControllerV1< +export class TokenDetectionController extends StaticIntervalPollingControllerV1< TokenDetectionConfig, BaseState > { diff --git a/packages/assets-controllers/src/TokenListController.ts b/packages/assets-controllers/src/TokenListController.ts index 6cf489d965..73863088ee 100644 --- a/packages/assets-controllers/src/TokenListController.ts +++ b/packages/assets-controllers/src/TokenListController.ts @@ -10,7 +10,7 @@ import type { NetworkState, NetworkControllerGetNetworkClientByIdAction, } from '@metamask/network-controller'; -import { PollingController } from '@metamask/polling-controller'; +import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; @@ -91,7 +91,7 @@ const defaultState: TokenListState = { /** * Controller that passively polls on a set interval for the list of tokens from metaswaps api */ -export class TokenListController extends PollingController< +export class TokenListController extends StaticIntervalPollingController< typeof name, TokenListState, TokenListMessenger diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index d8373998df..4cb8c3c76f 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -10,7 +10,7 @@ import type { NetworkController, NetworkState, } from '@metamask/network-controller'; -import { PollingControllerV1 } from '@metamask/polling-controller'; +import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; @@ -135,7 +135,7 @@ async function getCurrencyConversionRate({ * Controller that passively polls on a set interval for token-to-fiat exchange rates * for tokens stored in the TokensController */ -export class TokenRatesController extends PollingControllerV1< +export class TokenRatesController extends StaticIntervalPollingControllerV1< TokenRatesConfig, TokenRatesState > { diff --git a/packages/gas-fee-controller/src/GasFeeController.ts b/packages/gas-fee-controller/src/GasFeeController.ts index 94f25576c5..ecb397e607 100644 --- a/packages/gas-fee-controller/src/GasFeeController.ts +++ b/packages/gas-fee-controller/src/GasFeeController.ts @@ -18,7 +18,7 @@ import type { NetworkState, ProviderProxy, } from '@metamask/network-controller'; -import { PollingController } from '@metamask/polling-controller'; +import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { Hex } from '@metamask/utils'; import { v1 as random } from 'uuid'; @@ -253,7 +253,7 @@ const defaultState: GasFeeState = { /** * Controller that retrieves gas fee estimate data and polls for updated data on a set interval */ -export class GasFeeController extends PollingController< +export class GasFeeController extends StaticIntervalPollingController< typeof name, GasFeeState, GasFeeMessenger diff --git a/packages/polling-controller/src/AbstractPollingController.ts b/packages/polling-controller/src/AbstractPollingController.ts index b23ae89d60..155ad0afb7 100644 --- a/packages/polling-controller/src/AbstractPollingController.ts +++ b/packages/polling-controller/src/AbstractPollingController.ts @@ -6,12 +6,9 @@ import { v4 as random } from 'uuid'; export const getKey = ( networkClientId: NetworkClientId, options: Json, -): PollingTokenSetId => - `${networkClientId}${options ? `:${stringify(options)}` : ''}`; +): PollingTokenSetId => `${networkClientId}:${stringify(options)}`; -export type PollingTokenSetId = - | `${NetworkClientId}:${string}` - | NetworkClientId; +export type PollingTokenSetId = `${NetworkClientId}:${string}`; type Constructor = new (...args: any[]) => object; @@ -46,7 +43,7 @@ export function AbstractPollingControllerBaseMixin( startPollingByNetworkClientId( networkClientId: NetworkClientId, - options: Json, + options: Json = {}, ): string { const pollToken = random(); const key = getKey(networkClientId, options); diff --git a/packages/polling-controller/src/BlockTrackerPollingController.test.ts b/packages/polling-controller/src/BlockTrackerPollingController.test.ts index 6c6323e719..97cb78c472 100644 --- a/packages/polling-controller/src/BlockTrackerPollingController.test.ts +++ b/packages/polling-controller/src/BlockTrackerPollingController.test.ts @@ -22,7 +22,7 @@ class MyGasFeeController extends BlockTrackerPollingController { } } -describe('PollingController', () => { +describe('BlockTrackerPollingController', () => { let clock: sinon.SinonFakeTimers; let mockMessenger: any; let controller: any; diff --git a/packages/polling-controller/src/PollingController.test.ts b/packages/polling-controller/src/PollingController.test.ts index 0a095c4547..6995783927 100644 --- a/packages/polling-controller/src/PollingController.test.ts +++ b/packages/polling-controller/src/PollingController.test.ts @@ -3,7 +3,7 @@ import EventEmitter from 'events'; import { useFakeTimers } from 'sinon'; import { advanceTime } from '../../../tests/helpers'; -import { PollingController, PollingControllerOnly } from './PollingController'; +import { AbstractPollingController } from './AbstractPollingController'; const TICK_TIME = 1000; @@ -14,7 +14,7 @@ const createExecutePollMock = () => { return executePollMock; }; -class MyGasFeeController extends PollingController { +class MyGasFeeController extends AbstractPollingController { _executePoll = createExecutePollMock(); } diff --git a/packages/polling-controller/src/StaticIntervalPollingController.test.ts b/packages/polling-controller/src/StaticIntervalPollingController.test.ts index fdf03f7ccd..161263211a 100644 --- a/packages/polling-controller/src/StaticIntervalPollingController.test.ts +++ b/packages/polling-controller/src/StaticIntervalPollingController.test.ts @@ -23,7 +23,7 @@ class MyGasFeeController extends StaticIntervalPollingController< _intervalLength = TICK_TIME; } -describe('PollingController', () => { +describe('StaticIntervalPollingController', () => { let clock: sinon.SinonFakeTimers; let mockMessenger: any; let controller: any; @@ -195,7 +195,7 @@ describe('PollingController', () => { const pollingToken = controller.startPollingByNetworkClientId('mainnet'); controller.stopPollingByPollingToken(pollingToken); expect(pollingComplete).toHaveBeenCalledTimes(1); - expect(pollingComplete).toHaveBeenCalledWith('mainnet'); + expect(pollingComplete).toHaveBeenCalledWith('mainnet:{}'); }); it('should start and stop polling sessions for different networkClientIds with the same options', async () => { From b9323656d9cfd9ba3b47978c551b46817b40b96b Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 8 Dec 2023 14:35:41 -0600 Subject: [PATCH 16/22] cleanup --- .../src/AbstractPollingController.ts | 1 + .../src/PollingController.test.ts | 522 ------------------ 2 files changed, 1 insertion(+), 522 deletions(-) delete mode 100644 packages/polling-controller/src/PollingController.test.ts diff --git a/packages/polling-controller/src/AbstractPollingController.ts b/packages/polling-controller/src/AbstractPollingController.ts index 155ad0afb7..21ecaba72a 100644 --- a/packages/polling-controller/src/AbstractPollingController.ts +++ b/packages/polling-controller/src/AbstractPollingController.ts @@ -89,6 +89,7 @@ export function AbstractPollingControllerBaseMixin( this._stopPollingByPollingTokenSetId(nonNullKey); this.#pollingTokenSets.delete(nonNullKey); this.#callbacks.get(nonNullKey)?.forEach((callback) => { + // for some reason this typescript can't tell that keyToDelete is not null here callback(nonNullKey); }); this.#callbacks.get(nonNullKey)?.clear(); diff --git a/packages/polling-controller/src/PollingController.test.ts b/packages/polling-controller/src/PollingController.test.ts deleted file mode 100644 index 6995783927..0000000000 --- a/packages/polling-controller/src/PollingController.test.ts +++ /dev/null @@ -1,522 +0,0 @@ -import { ControllerMessenger } from '@metamask/base-controller'; -import EventEmitter from 'events'; -import { useFakeTimers } from 'sinon'; - -import { advanceTime } from '../../../tests/helpers'; -import { AbstractPollingController } from './AbstractPollingController'; - -const TICK_TIME = 1000; - -const createExecutePollMock = () => { - const executePollMock = jest.fn().mockImplementation(async () => { - return true; - }); - return executePollMock; -}; - -class MyGasFeeController extends AbstractPollingController { - _executePoll = createExecutePollMock(); -} - -describe('PollingController', () => { - let clock: sinon.SinonFakeTimers; - let mockMessenger: any; - let controller: any; - beforeEach(() => { - mockMessenger = new ControllerMessenger(); - controller = new MyGasFeeController({ - messenger: mockMessenger, - metadata: {}, - name: 'PollingController', - state: { foo: 'bar' }, - }); - clock = useFakeTimers(); - }); - afterEach(() => { - clock.restore(); - }); - describe('start', () => { - it('should start polling if not polling', async () => { - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 0 }); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: TICK_TIME }); - expect(controller._executePoll).toHaveBeenCalledTimes(2); - controller.stopAllPolling(); - }); - }); - describe('stop', () => { - it('should stop polling when called with a valid polling that was the only active pollingToken for a given networkClient', async () => { - const pollingToken = controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 0 }); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: TICK_TIME }); - controller.stopPollingByPollingToken(pollingToken); - await advanceTime({ clock, duration: TICK_TIME }); - expect(controller._executePoll).toHaveBeenCalledTimes(2); - controller.stopAllPolling(); - }); - it('should not stop polling if called with one of multiple active polling tokens for a given networkClient', async () => { - const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 0 }); - - controller.startPollingByNetworkClientId('mainnet'); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: TICK_TIME }); - controller.stopPollingByPollingToken(pollingToken1); - await advanceTime({ clock, duration: TICK_TIME }); - expect(controller._executePoll).toHaveBeenCalledTimes(3); - controller.stopAllPolling(); - }); - it('should error if no pollingToken is passed', () => { - controller.startPollingByNetworkClientId('mainnet'); - expect(() => { - controller.stopPollingByPollingToken(undefined as unknown as any); - }).toThrow('pollingToken required'); - controller.stopAllPolling(); - }); - it('should error if no matching pollingToken is found', () => { - controller.startPollingByNetworkClientId('mainnet'); - expect(() => { - controller.stopPollingByPollingToken('potato'); - }).toThrow('pollingToken not found'); - controller.stopAllPolling(); - }); - }); - - describe('setIntervalLength', () => { - it('should set getNetworkClientById (if previously set by setPollWithBlockTracker) to undefined when setting interval length', async () => { - controller.setPollWithBlockTracker(() => { - throw new Error('should not be called'); - }); - expect(controller.getPollingWithBlockTracker()).toBe(true); - controller.setIntervalLength(1000); - expect(controller.getPollingWithBlockTracker()).toBe(false); - }); - }); - - describe('startPollingByNetworkClientId', () => { - it('should call _executePoll immediately and on interval if polling', async () => { - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 0 }); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: TICK_TIME * 2 }); - expect(controller._executePoll).toHaveBeenCalledTimes(3); - }); - it('should call _executePoll immediately once and continue calling _executePoll on interval when start is called again with the same networkClientId', async () => { - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 0 }); - - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 0 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: TICK_TIME * 2 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(3); - controller.stopAllPolling(); - }); - it('should publish "pollingComplete" when stop is called', async () => { - const pollingComplete: any = jest.fn(); - controller.onPollingCompleteByNetworkClientId('mainnet', pollingComplete); - const pollingToken = controller.startPollingByNetworkClientId('mainnet'); - controller.stopPollingByPollingToken(pollingToken); - expect(pollingComplete).toHaveBeenCalledTimes(1); - }); - it('should poll at the interval length when set via setIntervalLength', async () => { - controller.setIntervalLength(TICK_TIME); - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 0 }); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: TICK_TIME / 2 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: TICK_TIME / 2 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(2); - }); - it('should start and stop polling sessions for different networkClientIds with the same options', async () => { - controller.setIntervalLength(TICK_TIME); - const pollToken1 = controller.startPollingByNetworkClientId('mainnet', { - address: '0x1', - }); - controller.startPollingByNetworkClientId('mainnet', { address: '0x2' }); - await advanceTime({ clock, duration: 0 }); - - controller.startPollingByNetworkClientId('sepolia', { address: '0x2' }); - await advanceTime({ clock, duration: 0 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', { address: '0x1' }], - ['mainnet', { address: '0x2' }], - ['sepolia', { address: '0x2' }], - ]); - await advanceTime({ clock, duration: TICK_TIME }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', { address: '0x1' }], - ['mainnet', { address: '0x2' }], - ['sepolia', { address: '0x2' }], - ['mainnet', { address: '0x1' }], - ['mainnet', { address: '0x2' }], - ['sepolia', { address: '0x2' }], - ]); - controller.stopPollingByPollingToken(pollToken1); - await advanceTime({ clock, duration: TICK_TIME }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', { address: '0x1' }], - ['mainnet', { address: '0x2' }], - ['sepolia', { address: '0x2' }], - ['mainnet', { address: '0x1' }], - ['mainnet', { address: '0x2' }], - ['sepolia', { address: '0x2' }], - ['mainnet', { address: '0x2' }], - ['sepolia', { address: '0x2' }], - ]); - }); - }); - describe('multiple networkClientIds', () => { - it('should poll for each networkClientId', async () => { - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 0 }); - - controller.startPollingByNetworkClientId('rinkeby'); - await advanceTime({ clock, duration: 0 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}], - ['rinkeby', {}], - ]); - await advanceTime({ clock, duration: TICK_TIME }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}], - ['rinkeby', {}], - ['mainnet', {}], - ['rinkeby', {}], - ]); - await advanceTime({ clock, duration: TICK_TIME }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}], - ['rinkeby', {}], - ['mainnet', {}], - ['rinkeby', {}], - ['mainnet', {}], - ['rinkeby', {}], - ]); - controller.stopAllPolling(); - }); - - it('should poll multiple networkClientIds when setting interval length', async () => { - controller.setIntervalLength(TICK_TIME * 2); - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 0 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}], - ]); - await advanceTime({ clock, duration: TICK_TIME }); - - controller.startPollingByNetworkClientId('sepolia'); - await advanceTime({ clock, duration: 0 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}], - ['sepolia', {}], - ]); - await advanceTime({ clock, duration: TICK_TIME }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}], - ['sepolia', {}], - ['mainnet', {}], - ]); - await advanceTime({ clock, duration: TICK_TIME }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}], - ['sepolia', {}], - ['mainnet', {}], - ['sepolia', {}], - ]); - await advanceTime({ clock, duration: TICK_TIME }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}], - ['sepolia', {}], - ['mainnet', {}], - ['sepolia', {}], - ['mainnet', {}], - ]); - await advanceTime({ clock, duration: TICK_TIME }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}], - ['sepolia', {}], - ['mainnet', {}], - ['sepolia', {}], - ['mainnet', {}], - ['sepolia', {}], - ]); - }); - - describe('PollingControllerOnly', () => { - it('can be extended from and constructed', async () => { - class MyClass extends PollingControllerOnly { - _executePoll = createExecutePollMock(); - } - const c = new MyClass(); - expect(c._executePoll).toBeDefined(); - expect(c.getIntervalLength).toBeDefined(); - expect(c.setIntervalLength).toBeDefined(); - expect(c.stopAllPolling).toBeDefined(); - expect(c.startPollingByNetworkClientId).toBeDefined(); - expect(c.stopPollingByPollingToken).toBeDefined(); - }); - }); - }); - - describe('Polling on block times', () => { - class TestBlockTracker extends EventEmitter { - private latestBlockNumber: number; - - public interval: number; - - constructor({ interval } = { interval: 1000 }) { - super(); - this.latestBlockNumber = 0; - this.interval = interval; - this.start(interval); - } - - private start(interval: number) { - setInterval(() => { - this.latestBlockNumber += 1; - this.emit('latest', this.latestBlockNumber); - }, interval); - } - } - - let getNetworkClientById: jest.Mock; - let mainnetBlockTracker: TestBlockTracker; - let goerliBlockTracker: TestBlockTracker; - let sepoliaBlockTracker: TestBlockTracker; - beforeEach(() => { - mainnetBlockTracker = new TestBlockTracker({ interval: 5 }); - goerliBlockTracker = new TestBlockTracker({ interval: 10 }); - sepoliaBlockTracker = new TestBlockTracker({ interval: 15 }); - - getNetworkClientById = jest.fn().mockImplementation((networkClientId) => { - switch (networkClientId) { - case 'mainnet': - return { - blockTracker: mainnetBlockTracker, - }; - case 'goerli': - return { - blockTracker: goerliBlockTracker, - }; - case 'sepolia': - return { - blockTracker: sepoliaBlockTracker, - }; - default: - throw new Error(`Unknown networkClientId: ${networkClientId}`); - } - }); - }); - describe('setPollWithBlockTracker', () => { - it('should set the interval length to undefined', () => { - controller.setPollWithBlockTracker(getNetworkClientById); - - expect(controller.getIntervalLength()).toBeUndefined(); - }); - }); - - describe('startPollingByNetworkClientId', () => { - it('should start polling for the specified networkClientId', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); - - controller.startPollingByNetworkClientId('mainnet'); - - expect(getNetworkClientById).toHaveBeenCalledWith('mainnet'); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - - await advanceTime({ clock, duration: 1 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - - await advanceTime({ clock, duration: 4 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - expect.arrayContaining(['mainnet', {}]), - expect.arrayContaining(['mainnet', {}]), - ]); - - controller.stopAllPolling(); - }); - - it('should poll on new block intervals for each networkClientId', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); - - controller.startPollingByNetworkClientId('mainnet'); - controller.startPollingByNetworkClientId('goerli'); - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ]); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ['mainnet', {}, 3], - ]); - - // 15ms have passed - // Start polling for sepolia, 15ms interval - controller.startPollingByNetworkClientId('sepolia'); - - await advanceTime({ clock, duration: 15 }); - - // at 30ms, 6 blocks have passed for mainnet (every 5ms), 3 for goerli (every 10ms), and 2 for sepolia (every 15ms) - // Didn't start listening to sepolia until 15ms had passed, so we only call executePoll on the 2nd block - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ['mainnet', {}, 5], - ['mainnet', {}, 6], - ['goerli', {}, 3], - ['sepolia', {}, 2], - ]); - - controller.stopAllPolling(); - }); - }); - - describe('stopPollingByPollingToken', () => { - it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); - - const pollingToken1 = - controller.startPollingByNetworkClientId('mainnet'); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - - const pollingToken2 = - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ]); - - controller.stopPollingByPollingToken(pollingToken1); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); - - controller.stopPollingByPollingToken(pollingToken2); - - await advanceTime({ clock, duration: 15 }); - - // no further polling should occur - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); - }); - - it('should should stop polling for one networkClientId when all polling tokens for that networkClientId are deleted, without stopping polling for networkClientIds with active pollingTokens', async () => { - controller.setPollWithBlockTracker(getNetworkClientById); - - const pollingToken1 = - controller.startPollingByNetworkClientId('mainnet'); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - - const pollingToken2 = - controller.startPollingByNetworkClientId('mainnet'); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ]); - - controller.startPollingByNetworkClientId('goerli'); - await advanceTime({ clock, duration: 5 }); - - // 3 blocks have passed for mainnet, 1 for goerli but we only started listening to goerli after 5ms - // so the next block will come at 20ms and be the 2nd block for goerli - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); - - controller.stopPollingByPollingToken(pollingToken1); - - await advanceTime({ clock, duration: 5 }); - - // 20ms have passed, 4 blocks for mainnet, 2 for goerli - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ]); - - controller.stopPollingByPollingToken(pollingToken2); - - await advanceTime({ clock, duration: 20 }); - - // no further polling for mainnet should occur - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ['goerli', {}, 3], - ['goerli', {}, 4], - ]); - - controller.stopAllPolling(); - }); - }); - }); -}); From 76449d6296564ecd5d1c33bca08eceb45960881d Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 8 Dec 2023 15:45:25 -0600 Subject: [PATCH 17/22] cleanup --- .../polling-controller/src/BlockTrackerPollingController.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/polling-controller/src/BlockTrackerPollingController.ts b/packages/polling-controller/src/BlockTrackerPollingController.ts index 313390686e..07d5395562 100644 --- a/packages/polling-controller/src/BlockTrackerPollingController.ts +++ b/packages/polling-controller/src/BlockTrackerPollingController.ts @@ -43,8 +43,7 @@ function BlockTrackerPollingControllerMixin( } const networkClient = this._getNetworkClientById(networkClientId); - if (networkClient && networkClient.blockTracker) { - console.log('networkClient', networkClient); + if (networkClient) { const updateOnNewBlock = this._executePoll.bind( this, networkClientId, From 5ccf0d31e18a54fb23868de26a4d0d12f9aba83c Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 8 Dec 2023 15:48:20 -0600 Subject: [PATCH 18/22] break when pollingToken found whether or not its the last --- packages/polling-controller/src/AbstractPollingController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/polling-controller/src/AbstractPollingController.ts b/packages/polling-controller/src/AbstractPollingController.ts index 21ecaba72a..5f95a0e60a 100644 --- a/packages/polling-controller/src/AbstractPollingController.ts +++ b/packages/polling-controller/src/AbstractPollingController.ts @@ -78,8 +78,8 @@ export function AbstractPollingControllerBaseMixin( if (tokenSet.delete(pollingToken)) { if (tokenSet.size === 0) { keyToDelete = key; - break; } + break; } } From 997c8065410d61be519f067a720e9a26e5f24889 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 11 Dec 2023 10:24:30 -0600 Subject: [PATCH 19/22] add IPollingController type --- .../src/CurrencyRateController.ts | 14 +++++---- .../src/NftDetectionController.ts | 9 +++--- .../src/TokenDetectionController.ts | 9 +++--- .../src/TokenListController.ts | 14 +++++---- .../src/TokenRatesController.ts | 9 +++--- .../src/GasFeeController.ts | 14 +++++---- .../src/AbstractPollingController.ts | 29 ++++++++++++++++++- .../src/StaticIntervalPollingController.ts | 12 +++++--- packages/polling-controller/src/index.ts | 2 ++ 9 files changed, 80 insertions(+), 32 deletions(-) diff --git a/packages/assets-controllers/src/CurrencyRateController.ts b/packages/assets-controllers/src/CurrencyRateController.ts index bf3509f310..2b95fe0481 100644 --- a/packages/assets-controllers/src/CurrencyRateController.ts +++ b/packages/assets-controllers/src/CurrencyRateController.ts @@ -11,6 +11,7 @@ import type { NetworkClientId, NetworkControllerGetNetworkClientByIdAction, } from '@metamask/network-controller'; +import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import { Mutex } from 'async-mutex'; @@ -82,11 +83,14 @@ const defaultState = { * Controller that passively polls on a set interval for an exchange rate from the current network * asset to the user's preferred currency. */ -export class CurrencyRateController extends StaticIntervalPollingController< - typeof name, - CurrencyRateState, - CurrencyRateMessenger -> { +export class CurrencyRateController + extends StaticIntervalPollingController< + typeof name, + CurrencyRateState, + CurrencyRateMessenger + > + implements IPollingController +{ private readonly mutex = new Mutex(); private readonly fetchExchangeRate; diff --git a/packages/assets-controllers/src/NftDetectionController.ts b/packages/assets-controllers/src/NftDetectionController.ts index 9db76bb073..83f7ab41bb 100644 --- a/packages/assets-controllers/src/NftDetectionController.ts +++ b/packages/assets-controllers/src/NftDetectionController.ts @@ -11,6 +11,7 @@ import type { NetworkState, NetworkClient, } from '@metamask/network-controller'; +import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; @@ -147,10 +148,10 @@ export interface NftDetectionConfig extends BaseConfig { /** * Controller that passively polls on a set interval for NFT auto detection */ -export class NftDetectionController extends StaticIntervalPollingControllerV1< - NftDetectionConfig, - BaseState -> { +export class NftDetectionController + extends StaticIntervalPollingControllerV1 + implements IPollingController +{ private intervalId?: ReturnType; private getOwnerNftApi({ diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 30a3249f18..544e5a5a58 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -8,6 +8,7 @@ import type { NetworkController, NetworkState, } from '@metamask/network-controller'; +import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; @@ -44,10 +45,10 @@ export interface TokenDetectionConfig extends BaseConfig { /** * Controller that passively polls on a set interval for Tokens auto detection */ -export class TokenDetectionController extends StaticIntervalPollingControllerV1< - TokenDetectionConfig, - BaseState -> { +export class TokenDetectionController + extends StaticIntervalPollingControllerV1 + implements IPollingController +{ private intervalId?: ReturnType; /** diff --git a/packages/assets-controllers/src/TokenListController.ts b/packages/assets-controllers/src/TokenListController.ts index 73863088ee..876a749e2d 100644 --- a/packages/assets-controllers/src/TokenListController.ts +++ b/packages/assets-controllers/src/TokenListController.ts @@ -10,6 +10,7 @@ import type { NetworkState, NetworkControllerGetNetworkClientByIdAction, } from '@metamask/network-controller'; +import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; @@ -91,11 +92,14 @@ const defaultState: TokenListState = { /** * Controller that passively polls on a set interval for the list of tokens from metaswaps api */ -export class TokenListController extends StaticIntervalPollingController< - typeof name, - TokenListState, - TokenListMessenger -> { +export class TokenListController + extends StaticIntervalPollingController< + typeof name, + TokenListState, + TokenListMessenger + > + implements IPollingController +{ private readonly mutex = new Mutex(); private intervalId?: ReturnType; diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index 4cb8c3c76f..580f0dfb32 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -10,6 +10,7 @@ import type { NetworkController, NetworkState, } from '@metamask/network-controller'; +import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; @@ -135,10 +136,10 @@ async function getCurrencyConversionRate({ * Controller that passively polls on a set interval for token-to-fiat exchange rates * for tokens stored in the TokensController */ -export class TokenRatesController extends StaticIntervalPollingControllerV1< - TokenRatesConfig, - TokenRatesState -> { +export class TokenRatesController + extends StaticIntervalPollingControllerV1 + implements IPollingController +{ private handle?: ReturnType; #pollState = PollState.Inactive; diff --git a/packages/gas-fee-controller/src/GasFeeController.ts b/packages/gas-fee-controller/src/GasFeeController.ts index ecb397e607..a9c50bbc1d 100644 --- a/packages/gas-fee-controller/src/GasFeeController.ts +++ b/packages/gas-fee-controller/src/GasFeeController.ts @@ -18,6 +18,7 @@ import type { NetworkState, ProviderProxy, } from '@metamask/network-controller'; +import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { Hex } from '@metamask/utils'; import { v1 as random } from 'uuid'; @@ -253,11 +254,14 @@ const defaultState: GasFeeState = { /** * Controller that retrieves gas fee estimate data and polls for updated data on a set interval */ -export class GasFeeController extends StaticIntervalPollingController< - typeof name, - GasFeeState, - GasFeeMessenger -> { +export class GasFeeController + extends StaticIntervalPollingController< + typeof name, + GasFeeState, + GasFeeMessenger + > + implements IPollingController +{ private intervalId?: ReturnType; private readonly intervalDelay; diff --git a/packages/polling-controller/src/AbstractPollingController.ts b/packages/polling-controller/src/AbstractPollingController.ts index 5f95a0e60a..4ac1ba4569 100644 --- a/packages/polling-controller/src/AbstractPollingController.ts +++ b/packages/polling-controller/src/AbstractPollingController.ts @@ -3,6 +3,30 @@ import type { Json } from '@metamask/utils'; import stringify from 'fast-json-stable-stringify'; import { v4 as random } from 'uuid'; +export type IPollingController = { + startPollingByNetworkClientId( + networkClientId: NetworkClientId, + options: Json, + ): string; + + stopAllPolling(): void; + + stopPollingByPollingToken(pollingToken: string): void; + + onPollingCompleteByNetworkClientId( + networkClientId: NetworkClientId, + callback: (networkClientId: NetworkClientId) => void, + options: Json, + ): void; + + _executePoll(networkClientId: NetworkClientId, options: Json): Promise; + _startPollingByNetworkClientId( + networkClientId: NetworkClientId, + options: Json, + ): void; + _stopPollingByPollingTokenSetId(key: PollingTokenSetId): void; +}; + export const getKey = ( networkClientId: NetworkClientId, options: Json, @@ -21,7 +45,10 @@ type Constructor = new (...args: any[]) => object; export function AbstractPollingControllerBaseMixin( Base: TBase, ) { - abstract class AbstractPollingControllerBase extends Base { + abstract class AbstractPollingControllerBase + extends Base + implements IPollingController + { readonly #pollingTokenSets: Map> = new Map(); #callbacks: Map< diff --git a/packages/polling-controller/src/StaticIntervalPollingController.ts b/packages/polling-controller/src/StaticIntervalPollingController.ts index 255da9e799..6042a96973 100644 --- a/packages/polling-controller/src/StaticIntervalPollingController.ts +++ b/packages/polling-controller/src/StaticIntervalPollingController.ts @@ -6,7 +6,10 @@ import { AbstractPollingControllerBaseMixin, getKey, } from './AbstractPollingController'; -import type { PollingTokenSetId } from './AbstractPollingController'; +import type { + IPollingController, + PollingTokenSetId, +} from './AbstractPollingController'; type Constructor = new (...args: any[]) => object; @@ -20,9 +23,10 @@ type Constructor = new (...args: any[]) => object; function StaticIntervalPollingControllerMixin( Base: TBase, ) { - abstract class StaticIntervalPollingController extends AbstractPollingControllerBaseMixin( - Base, - ) { + abstract class StaticIntervalPollingController + extends AbstractPollingControllerBaseMixin(Base) + implements IPollingController + { readonly #intervalIds: Record = {}; intervalLength: number | undefined = 1000; diff --git a/packages/polling-controller/src/index.ts b/packages/polling-controller/src/index.ts index 38997dda91..5bfd5e5366 100644 --- a/packages/polling-controller/src/index.ts +++ b/packages/polling-controller/src/index.ts @@ -9,3 +9,5 @@ export { StaticIntervalPollingController, StaticIntervalPollingControllerV1, } from './StaticIntervalPollingController'; + +export type { IPollingController } from './AbstractPollingController'; From 823aaa188f572a8aa76973a012ae9bfa8da518e0 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 11 Dec 2023 12:16:37 -0600 Subject: [PATCH 20/22] address feedback + cleanup --- .../src/CurrencyRateController.ts | 14 +- .../src/NftDetectionController.ts | 9 +- .../src/TokenDetectionController.ts | 9 +- .../src/TokenListController.ts | 14 +- .../src/TokenRatesController.ts | 9 +- .../src/GasFeeController.ts | 14 +- .../src/AbstractPollingController.ts | 24 +- .../src/BlockTrackerPollingController.test.ts | 392 +++++++++--------- .../StaticIntervalPollingController.test.ts | 43 +- .../src/StaticIntervalPollingController.ts | 10 +- 10 files changed, 259 insertions(+), 279 deletions(-) diff --git a/packages/assets-controllers/src/CurrencyRateController.ts b/packages/assets-controllers/src/CurrencyRateController.ts index 2b95fe0481..bf3509f310 100644 --- a/packages/assets-controllers/src/CurrencyRateController.ts +++ b/packages/assets-controllers/src/CurrencyRateController.ts @@ -11,7 +11,6 @@ import type { NetworkClientId, NetworkControllerGetNetworkClientByIdAction, } from '@metamask/network-controller'; -import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import { Mutex } from 'async-mutex'; @@ -83,14 +82,11 @@ const defaultState = { * Controller that passively polls on a set interval for an exchange rate from the current network * asset to the user's preferred currency. */ -export class CurrencyRateController - extends StaticIntervalPollingController< - typeof name, - CurrencyRateState, - CurrencyRateMessenger - > - implements IPollingController -{ +export class CurrencyRateController extends StaticIntervalPollingController< + typeof name, + CurrencyRateState, + CurrencyRateMessenger +> { private readonly mutex = new Mutex(); private readonly fetchExchangeRate; diff --git a/packages/assets-controllers/src/NftDetectionController.ts b/packages/assets-controllers/src/NftDetectionController.ts index 83f7ab41bb..9db76bb073 100644 --- a/packages/assets-controllers/src/NftDetectionController.ts +++ b/packages/assets-controllers/src/NftDetectionController.ts @@ -11,7 +11,6 @@ import type { NetworkState, NetworkClient, } from '@metamask/network-controller'; -import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; @@ -148,10 +147,10 @@ export interface NftDetectionConfig extends BaseConfig { /** * Controller that passively polls on a set interval for NFT auto detection */ -export class NftDetectionController - extends StaticIntervalPollingControllerV1 - implements IPollingController -{ +export class NftDetectionController extends StaticIntervalPollingControllerV1< + NftDetectionConfig, + BaseState +> { private intervalId?: ReturnType; private getOwnerNftApi({ diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index 544e5a5a58..30a3249f18 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -8,7 +8,6 @@ import type { NetworkController, NetworkState, } from '@metamask/network-controller'; -import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; @@ -45,10 +44,10 @@ export interface TokenDetectionConfig extends BaseConfig { /** * Controller that passively polls on a set interval for Tokens auto detection */ -export class TokenDetectionController - extends StaticIntervalPollingControllerV1 - implements IPollingController -{ +export class TokenDetectionController extends StaticIntervalPollingControllerV1< + TokenDetectionConfig, + BaseState +> { private intervalId?: ReturnType; /** diff --git a/packages/assets-controllers/src/TokenListController.ts b/packages/assets-controllers/src/TokenListController.ts index 17991a5a8c..ba89c200c9 100644 --- a/packages/assets-controllers/src/TokenListController.ts +++ b/packages/assets-controllers/src/TokenListController.ts @@ -10,7 +10,6 @@ import type { NetworkState, NetworkControllerGetNetworkClientByIdAction, } from '@metamask/network-controller'; -import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; @@ -92,14 +91,11 @@ const defaultState: TokenListState = { /** * Controller that passively polls on a set interval for the list of tokens from metaswaps api */ -export class TokenListController - extends StaticIntervalPollingController< - typeof name, - TokenListState, - TokenListMessenger - > - implements IPollingController -{ +export class TokenListController extends StaticIntervalPollingController< + typeof name, + TokenListState, + TokenListMessenger +> { private readonly mutex = new Mutex(); private intervalId?: ReturnType; diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index 12fddf24c9..c8b6e8ca31 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -10,7 +10,6 @@ import type { NetworkController, NetworkState, } from '@metamask/network-controller'; -import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; @@ -137,10 +136,10 @@ async function getCurrencyConversionRate({ * Controller that passively polls on a set interval for token-to-fiat exchange rates * for tokens stored in the TokensController */ -export class TokenRatesController - extends StaticIntervalPollingControllerV1 - implements IPollingController -{ +export class TokenRatesController extends StaticIntervalPollingControllerV1< + TokenRatesConfig, + TokenRatesState +> { private handle?: ReturnType; #pollState = PollState.Inactive; diff --git a/packages/gas-fee-controller/src/GasFeeController.ts b/packages/gas-fee-controller/src/GasFeeController.ts index a9c50bbc1d..ecb397e607 100644 --- a/packages/gas-fee-controller/src/GasFeeController.ts +++ b/packages/gas-fee-controller/src/GasFeeController.ts @@ -18,7 +18,6 @@ import type { NetworkState, ProviderProxy, } from '@metamask/network-controller'; -import type { IPollingController } from '@metamask/polling-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import type { Hex } from '@metamask/utils'; import { v1 as random } from 'uuid'; @@ -254,14 +253,11 @@ const defaultState: GasFeeState = { /** * Controller that retrieves gas fee estimate data and polls for updated data on a set interval */ -export class GasFeeController - extends StaticIntervalPollingController< - typeof name, - GasFeeState, - GasFeeMessenger - > - implements IPollingController -{ +export class GasFeeController extends StaticIntervalPollingController< + typeof name, + GasFeeState, + GasFeeMessenger +> { private intervalId?: ReturnType; private readonly intervalDelay; diff --git a/packages/polling-controller/src/AbstractPollingController.ts b/packages/polling-controller/src/AbstractPollingController.ts index 4ac1ba4569..621d481125 100644 --- a/packages/polling-controller/src/AbstractPollingController.ts +++ b/packages/polling-controller/src/AbstractPollingController.ts @@ -75,12 +75,11 @@ export function AbstractPollingControllerBaseMixin( const pollToken = random(); const key = getKey(networkClientId, options); const pollingTokenSet = - this.#pollingTokenSets.get(key) || new Set(); + this.#pollingTokenSets.get(key) ?? new Set(); pollingTokenSet.add(pollToken); this.#pollingTokenSets.set(key, pollingTokenSet); if (pollingTokenSet.size === 1) { - // Start new polling only if it's the first token for this key this._startPollingByNetworkClientId(networkClientId, options); } @@ -111,15 +110,16 @@ export function AbstractPollingControllerBaseMixin( } if (keyToDelete) { - // TODO figure out why this is necessary - const nonNullKey = keyToDelete; - this._stopPollingByPollingTokenSetId(nonNullKey); - this.#pollingTokenSets.delete(nonNullKey); - this.#callbacks.get(nonNullKey)?.forEach((callback) => { - // for some reason this typescript can't tell that keyToDelete is not null here - callback(nonNullKey); - }); - this.#callbacks.get(nonNullKey)?.clear(); + this._stopPollingByPollingTokenSetId(keyToDelete); + this.#pollingTokenSets.delete(keyToDelete); + const callbacks = this.#callbacks.get(keyToDelete); + if (callbacks) { + for (const callback of callbacks) { + // eslint-disable-next-line n/callback-return + callback(keyToDelete); + } + callbacks.clear(); + } } } @@ -129,7 +129,7 @@ export function AbstractPollingControllerBaseMixin( options: Json = {}, ) { const key = getKey(networkClientId, options); - const callbacks = this.#callbacks.get(key) || new Set(); + const callbacks = this.#callbacks.get(key) ?? new Set(); callbacks.add(callback); this.#callbacks.set(key, callbacks); } diff --git a/packages/polling-controller/src/BlockTrackerPollingController.test.ts b/packages/polling-controller/src/BlockTrackerPollingController.test.ts index 97cb78c472..6205fb8642 100644 --- a/packages/polling-controller/src/BlockTrackerPollingController.test.ts +++ b/packages/polling-controller/src/BlockTrackerPollingController.test.ts @@ -3,7 +3,6 @@ import type { NetworkClient } from '@metamask/network-controller'; import EventEmitter from 'events'; import { useFakeTimers } from 'sinon'; -import { advanceTime } from '../../../tests/helpers'; import { BlockTrackerPollingController } from './BlockTrackerPollingController'; const createExecutePollMock = () => { @@ -14,7 +13,11 @@ const createExecutePollMock = () => { }; let getNetworkClientByIdStub: jest.Mock; -class MyGasFeeController extends BlockTrackerPollingController { +class ChildBlockTrackerPollingController extends BlockTrackerPollingController< + any, + any, + any +> { _executePoll = createExecutePollMock(); _getNetworkClientById(networkClientId: string): NetworkClient | undefined { @@ -22,247 +25,244 @@ class MyGasFeeController extends BlockTrackerPollingController { } } +class TestBlockTracker extends EventEmitter { + private latestBlockNumber = 0; + + emitBlockEvent() { + this.latestBlockNumber += 1; + this.emit('latest', this.latestBlockNumber); + } +} + describe('BlockTrackerPollingController', () => { let clock: sinon.SinonFakeTimers; let mockMessenger: any; let controller: any; + let mainnetBlockTracker: TestBlockTracker; + let goerliBlockTracker: TestBlockTracker; + let sepoliaBlockTracker: TestBlockTracker; beforeEach(() => { mockMessenger = new ControllerMessenger(); - controller = new MyGasFeeController({ + controller = new ChildBlockTrackerPollingController({ messenger: mockMessenger, metadata: {}, name: 'PollingController', state: { foo: 'bar' }, }); + + mainnetBlockTracker = new TestBlockTracker(); + goerliBlockTracker = new TestBlockTracker(); + sepoliaBlockTracker = new TestBlockTracker(); + + getNetworkClientByIdStub = jest + .fn() + .mockImplementation((networkClientId: string) => { + switch (networkClientId) { + case 'mainnet': + return { + blockTracker: mainnetBlockTracker, + }; + case 'goerli': + return { + blockTracker: goerliBlockTracker, + }; + case 'sepolia': + return { + blockTracker: sepoliaBlockTracker, + }; + default: + throw new Error(`Unknown networkClientId: ${networkClientId}`); + } + }); clock = useFakeTimers(); }); afterEach(() => { clock.restore(); }); - describe('Polling on block times', () => { - class TestBlockTracker extends EventEmitter { - private latestBlockNumber: number; - - public interval: number; - - constructor({ interval } = { interval: 1000 }) { - super(); - this.latestBlockNumber = 0; - this.interval = interval; - this.start(interval); - } - - private start(interval: number) { - setInterval(() => { - this.latestBlockNumber += 1; - this.emit('latest', this.latestBlockNumber); - }, interval); - } - } - - let mainnetBlockTracker: TestBlockTracker; - let goerliBlockTracker: TestBlockTracker; - let sepoliaBlockTracker: TestBlockTracker; - beforeEach(() => { - mainnetBlockTracker = new TestBlockTracker({ interval: 5 }); - goerliBlockTracker = new TestBlockTracker({ interval: 10 }); - sepoliaBlockTracker = new TestBlockTracker({ interval: 15 }); - - getNetworkClientByIdStub = jest - .fn() - .mockImplementation((networkClientId) => { - switch (networkClientId) { - case 'mainnet': - return { - blockTracker: mainnetBlockTracker, - }; - case 'goerli': - return { - blockTracker: goerliBlockTracker, - }; - case 'sepolia': - return { - blockTracker: sepoliaBlockTracker, - }; - default: - throw new Error(`Unknown networkClientId: ${networkClientId}`); - } - }); + describe('startPollingByNetworkClientId', () => { + it('should call _executePoll on "latest" block events emitted by blockTrackers for each networkClientId passed to startPollingByNetworkClientId', async () => { + controller.startPollingByNetworkClientId('mainnet'); + controller.startPollingByNetworkClientId('goerli'); + // await advanceTime({ clock, duration: 5 }); + mainnetBlockTracker.emitBlockEvent(); + + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + + mainnetBlockTracker.emitBlockEvent(); + goerliBlockTracker.emitBlockEvent(); + + expect(controller._executePoll).toHaveBeenNthCalledWith( + 2, + 'mainnet', + {}, + 2, // 2nd block for mainnet + ); + expect(controller._executePoll).toHaveBeenNthCalledWith( + 3, + 'goerli', + {}, + 1, // 1st block for goerli + ); + + mainnetBlockTracker.emitBlockEvent(); + goerliBlockTracker.emitBlockEvent(); + + // sepolioa not being listened to yet, so first block for sepolia will not cause an executePoll + sepoliaBlockTracker.emitBlockEvent(); + + expect(controller._executePoll).toHaveBeenNthCalledWith( + 4, + 'mainnet', + {}, + 3, + ); + expect(controller._executePoll).toHaveBeenNthCalledWith( + 5, + 'goerli', + {}, + 2, + ); + + controller.startPollingByNetworkClientId('sepolia'); + + mainnetBlockTracker.emitBlockEvent(); + sepoliaBlockTracker.emitBlockEvent(); + + expect(controller._executePoll).toHaveBeenNthCalledWith( + 6, + 'mainnet', + {}, + 4, + ); + expect(controller._executePoll).toHaveBeenNthCalledWith( + 7, + 'sepolia', + {}, + 2, + ); + + controller.stopAllPolling(); }); + }); - describe('startPollingByNetworkClientId', () => { - it('should start polling for the specified networkClientId', async () => { - controller.startPollingByNetworkClientId('mainnet'); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - - await advanceTime({ clock, duration: 1 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - - await advanceTime({ clock, duration: 4 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - expect.arrayContaining(['mainnet', {}]), - expect.arrayContaining(['mainnet', {}]), - ]); + describe('stopPollingByPollingToken', () => { + it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { + const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); - controller.stopAllPolling(); - }); + // await advanceTime({ clock, duration: 5 }); + mainnetBlockTracker.emitBlockEvent(); - it('should poll on new block intervals for each networkClientId', async () => { - controller.startPollingByNetworkClientId('mainnet'); - controller.startPollingByNetworkClientId('goerli'); - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll).toHaveBeenCalledTimes(1); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ]); - - await advanceTime({ clock, duration: 5 }); - - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ['mainnet', {}, 3], - ]); - - // 15ms have passed - // Start polling for sepolia, 15ms interval - controller.startPollingByNetworkClientId('sepolia'); - - await advanceTime({ clock, duration: 15 }); - - // at 30ms, 6 blocks have passed for mainnet (every 5ms), 3 for goerli (every 10ms), and 2 for sepolia (every 15ms) - // Didn't start listening to sepolia until 15ms had passed, so we only call executePoll on the 2nd block - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['goerli', {}, 1], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ['mainnet', {}, 5], - ['mainnet', {}, 6], - ['goerli', {}, 3], - ['sepolia', {}, 2], - ]); - - controller.stopAllPolling(); - }); - }); + expect(controller._executePoll).toHaveBeenCalledTimes(1); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - describe('stopPollingByPollingToken', () => { - it('should should stop polling when all polling tokens for a networkClientId are deleted', async () => { - const pollingToken1 = - controller.startPollingByNetworkClientId('mainnet'); + const pollingToken2 = controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + mainnetBlockTracker.emitBlockEvent(); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); - const pollingToken2 = - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 5 }); + controller.stopPollingByPollingToken(pollingToken1); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ]); + mainnetBlockTracker.emitBlockEvent(); - controller.stopPollingByPollingToken(pollingToken1); + // polling is still active for mainnet because pollingToken2 is still active + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); - await advanceTime({ clock, duration: 5 }); + controller.stopPollingByPollingToken(pollingToken2); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); + mainnetBlockTracker.emitBlockEvent(); + mainnetBlockTracker.emitBlockEvent(); + mainnetBlockTracker.emitBlockEvent(); - controller.stopPollingByPollingToken(pollingToken2); + // no further polling should occur regardless of how many blocks are emitted + // because all pollingTokens for mainnet have been deleted + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); + }); - await advanceTime({ clock, duration: 15 }); + it('should should stop polling for one networkClientId when all polling tokens for that networkClientId are deleted, without stopping polling for networkClientIds with active pollingTokens', async () => { + const pollingToken1 = controller.startPollingByNetworkClientId('mainnet'); - // no further polling should occur - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); - }); + mainnetBlockTracker.emitBlockEvent(); - it('should should stop polling for one networkClientId when all polling tokens for that networkClientId are deleted, without stopping polling for networkClientIds with active pollingTokens', async () => { - const pollingToken1 = - controller.startPollingByNetworkClientId('mainnet'); + expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); - await advanceTime({ clock, duration: 5 }); + const pollingToken2 = controller.startPollingByNetworkClientId('mainnet'); - expect(controller._executePoll).toHaveBeenCalledWith('mainnet', {}, 1); + mainnetBlockTracker.emitBlockEvent(); - const pollingToken2 = - controller.startPollingByNetworkClientId('mainnet'); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ]); - await advanceTime({ clock, duration: 5 }); + controller.startPollingByNetworkClientId('goerli'); - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ]); + mainnetBlockTracker.emitBlockEvent(); - controller.startPollingByNetworkClientId('goerli'); - await advanceTime({ clock, duration: 5 }); + // we are polling for mainnet and goerli but goerli has not emitted any blocks yet + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ]); - // 3 blocks have passed for mainnet, 1 for goerli but we only started listening to goerli after 5ms - // so the next block will come at 20ms and be the 2nd block for goerli - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ]); + controller.stopPollingByPollingToken(pollingToken1); - controller.stopPollingByPollingToken(pollingToken1); + mainnetBlockTracker.emitBlockEvent(); + goerliBlockTracker.emitBlockEvent(); - await advanceTime({ clock, duration: 5 }); + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 1], + ]); - // 20ms have passed, 4 blocks for mainnet, 2 for goerli - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ]); + controller.stopPollingByPollingToken(pollingToken2); - controller.stopPollingByPollingToken(pollingToken2); + mainnetBlockTracker.emitBlockEvent(); + mainnetBlockTracker.emitBlockEvent(); + mainnetBlockTracker.emitBlockEvent(); + goerliBlockTracker.emitBlockEvent(); + goerliBlockTracker.emitBlockEvent(); - await advanceTime({ clock, duration: 20 }); + // no further polling for mainnet should occur + expect(controller._executePoll.mock.calls).toMatchObject([ + ['mainnet', {}, 1], + ['mainnet', {}, 2], + ['mainnet', {}, 3], + ['mainnet', {}, 4], + ['goerli', {}, 1], + ['goerli', {}, 2], + ['goerli', {}, 3], + ]); - // no further polling for mainnet should occur - expect(controller._executePoll.mock.calls).toMatchObject([ - ['mainnet', {}, 1], - ['mainnet', {}, 2], - ['mainnet', {}, 3], - ['mainnet', {}, 4], - ['goerli', {}, 2], - ['goerli', {}, 3], - ['goerli', {}, 4], - ]); + controller.stopAllPolling(); + }); + }); - controller.stopAllPolling(); - }); + describe('onPollingCompleteByNetworkClientId', () => { + it('should publish "pollingComplete" callback function set by "onPollingCompleteByNetworkClientId" when polling stops', async () => { + const pollingComplete: any = jest.fn(); + controller.onPollingCompleteByNetworkClientId('mainnet', pollingComplete); + const pollingToken = controller.startPollingByNetworkClientId('mainnet'); + controller.stopPollingByPollingToken(pollingToken); + expect(pollingComplete).toHaveBeenCalledTimes(1); + expect(pollingComplete).toHaveBeenCalledWith('mainnet:{}'); }); }); }); diff --git a/packages/polling-controller/src/StaticIntervalPollingController.test.ts b/packages/polling-controller/src/StaticIntervalPollingController.test.ts index 161263211a..196d886050 100644 --- a/packages/polling-controller/src/StaticIntervalPollingController.test.ts +++ b/packages/polling-controller/src/StaticIntervalPollingController.test.ts @@ -4,7 +4,7 @@ import { useFakeTimers } from 'sinon'; import { advanceTime } from '../../../tests/helpers'; import { StaticIntervalPollingController } from './StaticIntervalPollingController'; -const TICK_TIME = 1000; +const TICK_TIME = 5; const createExecutePollMock = () => { const executePollMock = jest.fn().mockImplementation(async () => { @@ -13,14 +13,12 @@ const createExecutePollMock = () => { return executePollMock; }; -class MyGasFeeController extends StaticIntervalPollingController< +class ChildBlockTrackerPollingController extends StaticIntervalPollingController< any, any, any > { _executePoll = createExecutePollMock(); - - _intervalLength = TICK_TIME; } describe('StaticIntervalPollingController', () => { @@ -29,12 +27,13 @@ describe('StaticIntervalPollingController', () => { let controller: any; beforeEach(() => { mockMessenger = new ControllerMessenger(); - controller = new MyGasFeeController({ + controller = new ChildBlockTrackerPollingController({ messenger: mockMessenger, metadata: {}, name: 'PollingController', state: { foo: 'bar' }, }); + controller.setIntervalLength(TICK_TIME); clock = useFakeTimers(); }); afterEach(() => { @@ -42,7 +41,7 @@ describe('StaticIntervalPollingController', () => { }); describe('startPollingByNetworkClientId', () => { - it('should start polling if not polling', async () => { + it('should start polling if not already polling', async () => { controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); expect(controller._executePoll).toHaveBeenCalledTimes(1); @@ -50,14 +49,8 @@ describe('StaticIntervalPollingController', () => { expect(controller._executePoll).toHaveBeenCalledTimes(2); controller.stopAllPolling(); }); - it('should call _executePoll immediately and on interval if polling', async () => { - controller.startPollingByNetworkClientId('mainnet'); - await advanceTime({ clock, duration: 0 }); - expect(controller._executePoll).toHaveBeenCalledTimes(1); - await advanceTime({ clock, duration: TICK_TIME * 2 }); - expect(controller._executePoll).toHaveBeenCalledTimes(3); - }); - it('should call _executePoll immediately once and continue calling _executePoll on interval when start is called again with the same networkClientId', async () => { + + it('should call _executePoll immediately once and continue calling _executePoll on interval when called again with the same networkClientId', async () => { controller.startPollingByNetworkClientId('mainnet'); await advanceTime({ clock, duration: 0 }); @@ -184,20 +177,11 @@ describe('StaticIntervalPollingController', () => { it('should error if no pollingToken is passed', () => { controller.startPollingByNetworkClientId('mainnet'); expect(() => { - controller.stopPollingByPollingToken(undefined as unknown as any); + controller.stopPollingByPollingToken(); }).toThrow('pollingToken required'); controller.stopAllPolling(); }); - it('should publish "pollingComplete" when stop is called', async () => { - const pollingComplete: any = jest.fn(); - controller.onPollingCompleteByNetworkClientId('mainnet', pollingComplete); - const pollingToken = controller.startPollingByNetworkClientId('mainnet'); - controller.stopPollingByPollingToken(pollingToken); - expect(pollingComplete).toHaveBeenCalledTimes(1); - expect(pollingComplete).toHaveBeenCalledWith('mainnet:{}'); - }); - it('should start and stop polling sessions for different networkClientIds with the same options', async () => { controller.setIntervalLength(TICK_TIME); const pollToken1 = controller.startPollingByNetworkClientId('mainnet', { @@ -239,4 +223,15 @@ describe('StaticIntervalPollingController', () => { ]); }); }); + + describe('onPollingCompleteByNetworkClientId', () => { + it('should publish "pollingComplete" callback function set by "onPollingCompleteByNetworkClientId" when polling stops', async () => { + const pollingComplete: any = jest.fn(); + controller.onPollingCompleteByNetworkClientId('mainnet', pollingComplete); + const pollingToken = controller.startPollingByNetworkClientId('mainnet'); + controller.stopPollingByPollingToken(pollingToken); + expect(pollingComplete).toHaveBeenCalledTimes(1); + expect(pollingComplete).toHaveBeenCalledWith('mainnet:{}'); + }); + }); }); diff --git a/packages/polling-controller/src/StaticIntervalPollingController.ts b/packages/polling-controller/src/StaticIntervalPollingController.ts index 6042a96973..fb2f5bb4c6 100644 --- a/packages/polling-controller/src/StaticIntervalPollingController.ts +++ b/packages/polling-controller/src/StaticIntervalPollingController.ts @@ -29,21 +29,21 @@ function StaticIntervalPollingControllerMixin( { readonly #intervalIds: Record = {}; - intervalLength: number | undefined = 1000; + #intervalLength: number | undefined = 1000; setIntervalLength(intervalLength: number) { - this.intervalLength = intervalLength; + this.#intervalLength = intervalLength; } getIntervalLength() { - return this.intervalLength; + return this.#intervalLength; } _startPollingByNetworkClientId( networkClientId: NetworkClientId, options: Json, ) { - if (!this.intervalLength) { + if (!this.#intervalLength) { throw new Error('intervalLength must be defined and greater than 0'); } @@ -60,7 +60,7 @@ function StaticIntervalPollingControllerMixin( } this._startPollingByNetworkClientId(networkClientId, options); }, - existingInterval ? this.intervalLength : 0, + existingInterval ? this.#intervalLength : 0, ); } From 071f73b5fbf21e95d8689d5f3a282f53d5c2b780 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 11 Dec 2023 16:29:04 -0600 Subject: [PATCH 21/22] use StaticIntervalPollingController for PendingUserOperationTracker --- .../src/helpers/PendingUserOperationTracker.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts b/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts index 328720e356..914707f21f 100644 --- a/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts +++ b/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.ts @@ -1,7 +1,7 @@ import { query } from '@metamask/controller-utils'; import EthQuery from '@metamask/eth-query'; import type { Provider } from '@metamask/network-controller'; -import { PollingControllerOnly } from '@metamask/polling-controller'; +import { StaticIntervalPollingControllerOnly } from '@metamask/polling-controller'; import type { Json } from '@metamask/utils'; import { createModuleLogger } from '@metamask/utils'; import EventEmitter from 'events'; @@ -34,7 +34,7 @@ export type PendingUserOperationTrackerEventEmitter = EventEmitter & { emit(eventName: T, ...args: Events[T]): boolean; }; -export class PendingUserOperationTracker extends PollingControllerOnly { +export class PendingUserOperationTracker extends StaticIntervalPollingControllerOnly { hub: PendingUserOperationTrackerEventEmitter; #getUserOperations: () => UserOperationMetadata[]; From 77f3d2ecdce9d7b1b50adfc838adc7f221bb586f Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 12 Dec 2023 13:21:35 -0600 Subject: [PATCH 22/22] update AccountTrackerController to use StaticIntervalPollingControllerV1 --- packages/assets-controllers/src/AccountTrackerController.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/assets-controllers/src/AccountTrackerController.ts b/packages/assets-controllers/src/AccountTrackerController.ts index e170d4adb7..5aa65496c0 100644 --- a/packages/assets-controllers/src/AccountTrackerController.ts +++ b/packages/assets-controllers/src/AccountTrackerController.ts @@ -11,7 +11,7 @@ import type { NetworkController, NetworkState, } from '@metamask/network-controller'; -import { PollingControllerV1 } from '@metamask/polling-controller'; +import { StaticIntervalPollingControllerV1 } from '@metamask/polling-controller'; import type { PreferencesState } from '@metamask/preferences-controller'; import { assert } from '@metamask/utils'; import { Mutex } from 'async-mutex'; @@ -61,7 +61,7 @@ export interface AccountTrackerState extends BaseState { /** * Controller that tracks the network balances for all user accounts. */ -export class AccountTrackerController extends PollingControllerV1< +export class AccountTrackerController extends StaticIntervalPollingControllerV1< AccountTrackerConfig, AccountTrackerState > {