diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index 348137d2e88..2e053301a59 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -69,6 +69,100 @@ describe('PhishingController', () => { expect(nockScope.isDone()).toBe(false); }); + describe('maybeUpdatePhishingLists', () => { + let nockScope: nock.Scope; + + beforeEach(() => { + nockScope = nock(PHISHING_CONFIG_BASE_URL) + .get(METAMASK_CONFIG_FILE) + .reply(200, { + blacklist: ['this-should-not-be-in-default-blacklist.com'], + fuzzylist: [], + tolerance: 0, + whitelist: ['this-should-not-be-in-default-whitelist.com'], + version: 0, + }) + .get(PHISHFORT_HOTLIST_FILE) + .reply(200, []); + }); + + it('should not be out of date immediately after maybeUpdatePhishingLists is called', async () => { + const clock = sinon.useFakeTimers(); + const controller = new PhishingController({ refreshInterval: 10 }); + clock.tick(10); + expect(controller.isOutOfDate()).toBe(true); + await controller.maybeUpdatePhishingLists(); + expect(controller.isOutOfDate()).toBe(false); + + expect(nockScope.isDone()).toBe(true); + }); + + it('should not be out of date after maybeUpdatePhishingLists is called but before refresh interval has passed', async () => { + const clock = sinon.useFakeTimers(); + const controller = new PhishingController({ refreshInterval: 10 }); + clock.tick(10); + expect(controller.isOutOfDate()).toBe(true); + await controller.maybeUpdatePhishingLists(); + clock.tick(5); + expect(controller.isOutOfDate()).toBe(false); + expect(nockScope.isDone()).toBe(true); + }); + + it('should still be out of date while update is in progress', async () => { + const clock = sinon.useFakeTimers(); + const controller = new PhishingController({ refreshInterval: 10 }); + clock.tick(10); + // do not wait + const maybeUpdatePhisingListPromise = + controller.maybeUpdatePhishingLists(); + expect(controller.isOutOfDate()).toBe(true); + await maybeUpdatePhisingListPromise; + expect(controller.isOutOfDate()).toBe(false); + clock.tick(10); + expect(controller.isOutOfDate()).toBe(true); + expect(nockScope.isDone()).toBe(true); + }); + + it('should call update only if it is out of date, otherwise it should not call update', async () => { + const clock = sinon.useFakeTimers(); + const controller = new PhishingController({ refreshInterval: 10 }); + expect(controller.isOutOfDate()).toBe(false); + await controller.maybeUpdatePhishingLists(); + expect( + controller.test('this-should-not-be-in-default-blacklist.com'), + ).toMatchObject({ + result: false, + type: 'all', + }); + + expect( + controller.test('this-should-not-be-in-default-whitelist.com'), + ).toMatchObject({ + result: false, + type: 'all', + }); + + clock.tick(10); + await controller.maybeUpdatePhishingLists(); + + expect( + controller.test('this-should-not-be-in-default-blacklist.com'), + ).toMatchObject({ + result: true, + type: 'blocklist', + }); + + expect( + controller.test('this-should-not-be-in-default-whitelist.com'), + ).toMatchObject({ + result: false, + type: 'allowlist', + }); + + expect(nockScope.isDone()).toBe(true); + }); + }); + describe('isOutOfDate', () => { it('should not be out of date upon construction', () => { sinon.useFakeTimers(); diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index e78686ddd45..e6542fa14e2 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -83,6 +83,7 @@ export interface PhishingConfig extends BaseConfig { export interface PhishingState extends BaseState { phishing: EthPhishingDetectConfig[]; whitelist: string[]; + lastFetched: number; } export const PHISHING_CONFIG_BASE_URL = @@ -104,8 +105,6 @@ export class PhishingController extends BaseController< > { private detector: any; - private lastFetched = 0; - #inProgressUpdate: Promise | undefined; /** @@ -140,9 +139,11 @@ export class PhishingController extends BaseController< }, ], whitelist: [], + lastFetched: 0, }; - this.detector = new PhishingDetector(this.defaultState.phishing); + this.initialize(); + this.detector = new PhishingDetector(this.state.phishing); } /** @@ -160,7 +161,7 @@ export class PhishingController extends BaseController< * @returns Whether an update is needed */ isOutOfDate() { - return Date.now() - this.lastFetched >= this.config.refreshInterval; + return Date.now() - this.state.lastFetched >= this.config.refreshInterval; } /** @@ -215,6 +216,19 @@ export class PhishingController extends BaseController< } } + /** + * Conditionally update the phishing configuration. + * + * If the phishing configuration is out of date, this function will call `updatePhishingLists` + * to update the configuration. + */ + async maybeUpdatePhishingLists() { + const phishingListsAreOutOfDate = this.isOutOfDate(); + if (phishingListsAreOutOfDate) { + await this.updatePhishingLists(); + } + } + /** * Update the phishing configuration. * @@ -238,7 +252,9 @@ export class PhishingController extends BaseController< } finally { // Set `lastFetched` even for failed requests to prevent server from being overwhelmed with // traffic after a network disruption. - this.lastFetched = Date.now(); + this.update({ + lastFetched: Date.now(), + }); } // Correctly shaping MetaMask config.