Skip to content

Commit fdc83e1

Browse files
FrederikBoldingFrederikBolding
authored
fix: Stop refreshing phishing list when updating interfaces (#3272)
Before this PR we would sometimes (every 5 minutes or so) update the phishing list before validating interfaces. When the phishing list is out of date it takes multiple seconds for this call to resolve causing an unacceptable experience. From now on, we will assume that the phishing list is kept up to date by other controllers (or the user visiting dapps for instance).
1 parent eb408a3 commit fdc83e1

File tree

13 files changed

+21
-138
lines changed

13 files changed

+21
-138
lines changed

‎packages/snaps-controllers/coverage.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
"branches": 93.34,
33
"functions": 97.37,
44
"lines": 98.33,
5-
"statements": 98.07
5+
"statements": 98.06
66
}

‎packages/snaps-controllers/src/interface/SnapInterfaceController.test.tsx‎

Lines changed: 5 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,6 @@ describe('SnapInterfaceController', () => {
147147

148148
expect(rootMessenger.call).toHaveBeenNthCalledWith(
149149
2,
150-
'PhishingController:maybeUpdateState',
151-
);
152-
153-
expect(rootMessenger.call).toHaveBeenNthCalledWith(
154-
3,
155150
'PhishingController:testOrigin',
156151
'https://foo.bar/',
157152
);
@@ -187,7 +182,7 @@ describe('SnapInterfaceController', () => {
187182
);
188183

189184
expect(rootMessenger.call).toHaveBeenNthCalledWith(
190-
4,
185+
3,
191186
'MultichainAssetsController:getState',
192187
);
193188

@@ -243,11 +238,6 @@ describe('SnapInterfaceController', () => {
243238

244239
expect(rootMessenger.call).toHaveBeenNthCalledWith(
245240
2,
246-
'PhishingController:maybeUpdateState',
247-
);
248-
249-
expect(rootMessenger.call).toHaveBeenNthCalledWith(
250-
3,
251241
'PhishingController:testOrigin',
252242
'https://foo.bar/',
253243
);
@@ -366,11 +356,6 @@ describe('SnapInterfaceController', () => {
366356
false,
367357
);
368358

369-
rootMessenger.registerActionHandler(
370-
'PhishingController:maybeUpdateState',
371-
jest.fn(),
372-
);
373-
374359
rootMessenger.registerActionHandler(
375360
'PhishingController:testOrigin',
376361
() => ({ result: true, type: 'all' }),
@@ -399,11 +384,6 @@ describe('SnapInterfaceController', () => {
399384

400385
expect(rootMessenger.call).toHaveBeenNthCalledWith(
401386
2,
402-
'PhishingController:maybeUpdateState',
403-
);
404-
405-
expect(rootMessenger.call).toHaveBeenNthCalledWith(
406-
3,
407387
'PhishingController:testOrigin',
408388
'https://foo.bar/',
409389
);
@@ -416,11 +396,6 @@ describe('SnapInterfaceController', () => {
416396
false,
417397
);
418398

419-
rootMessenger.registerActionHandler(
420-
'PhishingController:maybeUpdateState',
421-
jest.fn(),
422-
);
423-
424399
rootMessenger.registerActionHandler(
425400
'PhishingController:testOrigin',
426401
() => ({ result: true, type: 'all' }),
@@ -449,11 +424,6 @@ describe('SnapInterfaceController', () => {
449424

450425
expect(rootMessenger.call).toHaveBeenNthCalledWith(
451426
2,
452-
'PhishingController:maybeUpdateState',
453-
);
454-
455-
expect(rootMessenger.call).toHaveBeenNthCalledWith(
456-
3,
457427
'PhishingController:testOrigin',
458428
'https://foo.bar/',
459429
);
@@ -466,11 +436,6 @@ describe('SnapInterfaceController', () => {
466436
false,
467437
);
468438

469-
rootMessenger.registerActionHandler(
470-
'PhishingController:maybeUpdateState',
471-
jest.fn(),
472-
);
473-
474439
rootMessenger.registerActionHandler(
475440
'SnapController:get',
476441
() => undefined,
@@ -500,7 +465,7 @@ describe('SnapInterfaceController', () => {
500465
);
501466

502467
expect(rootMessenger.call).toHaveBeenNthCalledWith(
503-
3,
468+
2,
504469
'SnapController:get',
505470
MOCK_SNAP_ID,
506471
);
@@ -513,11 +478,6 @@ describe('SnapInterfaceController', () => {
513478
false,
514479
);
515480

516-
rootMessenger.registerActionHandler(
517-
'PhishingController:maybeUpdateState',
518-
jest.fn(),
519-
);
520-
521481
rootMessenger.registerActionHandler(
522482
'PhishingController:testOrigin',
523483
() => ({ result: true, type: 'all' }),
@@ -563,7 +523,7 @@ describe('SnapInterfaceController', () => {
563523
);
564524

565525
expect(rootMessenger.call).toHaveBeenNthCalledWith(
566-
3,
526+
2,
567527
'AccountsController:getAccountByAddress',
568528
'7S3P4HxJpyyigGzodYwHtCxZyUQe9JiBMHyRWXArAaKv',
569529
);
@@ -933,11 +893,6 @@ describe('SnapInterfaceController', () => {
933893
false,
934894
);
935895

936-
rootMessenger.registerActionHandler(
937-
'PhishingController:maybeUpdateState',
938-
jest.fn(),
939-
);
940-
941896
rootMessenger.registerActionHandler(
942897
'PhishingController:testOrigin',
943898
() => ({ result: true, type: 'all' }),
@@ -977,12 +932,7 @@ describe('SnapInterfaceController', () => {
977932
).rejects.toThrow('Invalid URL: The specified URL is not allowed.');
978933

979934
expect(rootMessenger.call).toHaveBeenNthCalledWith(
980-
4,
981-
'PhishingController:maybeUpdateState',
982-
);
983-
984-
expect(rootMessenger.call).toHaveBeenNthCalledWith(
985-
5,
935+
3,
986936
'PhishingController:testOrigin',
987937
'https://foo.bar/',
988938
);
@@ -995,11 +945,6 @@ describe('SnapInterfaceController', () => {
995945
false,
996946
);
997947

998-
rootMessenger.registerActionHandler(
999-
'PhishingController:maybeUpdateState',
1000-
jest.fn(),
1001-
);
1002-
1003948
rootMessenger.registerActionHandler(
1004949
'PhishingController:testOrigin',
1005950
() => ({ result: true, type: 'all' }),
@@ -1043,12 +988,7 @@ describe('SnapInterfaceController', () => {
1043988
).rejects.toThrow('Invalid URL: The specified URL is not allowed.');
1044989

1045990
expect(rootMessenger.call).toHaveBeenNthCalledWith(
1046-
4,
1047-
'PhishingController:maybeUpdateState',
1048-
);
1049-
1050-
expect(rootMessenger.call).toHaveBeenNthCalledWith(
1051-
5,
991+
3,
1052992
'PhishingController:testOrigin',
1053993
'https://foo.bar/',
1054994
);

‎packages/snaps-controllers/src/interface/SnapInterfaceController.ts‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,7 @@ import type {
88
ControllerStateChangeEvent,
99
} from '@metamask/base-controller';
1010
import { BaseController } from '@metamask/base-controller';
11-
import type {
12-
MaybeUpdateState,
13-
TestOrigin,
14-
} from '@metamask/phishing-controller';
11+
import type { TestOrigin } from '@metamask/phishing-controller';
1512
import type {
1613
InterfaceState,
1714
SnapId,
@@ -91,7 +88,6 @@ type MultichainAssetsControllerGetStateAction = ControllerGetStateAction<
9188

9289
export type SnapInterfaceControllerAllowedActions =
9390
| TestOrigin
94-
| MaybeUpdateState
9591
| HasApprovalRequest
9692
| AcceptRequest
9793
| GetSnap
@@ -406,13 +402,6 @@ export class SnapInterfaceController extends BaseController<
406402
);
407403
}
408404

409-
/**
410-
* Trigger a Phishing list update if needed.
411-
*/
412-
async #triggerPhishingListUpdate() {
413-
await this.messagingSystem.call('PhishingController:maybeUpdateState');
414-
}
415-
416405
/**
417406
* Check an origin against the phishing list.
418407
*
@@ -500,8 +489,6 @@ export class SnapInterfaceController extends BaseController<
500489
`A Snap UI may not be larger than ${MAX_UI_CONTENT_SIZE / 1000000} MB.`,
501490
);
502491

503-
await this.#triggerPhishingListUpdate();
504-
505492
validateJsxElements(element, {
506493
isOnPhishingList: this.#checkPhishingList.bind(this),
507494
getSnap: this.#getSnap.bind(this),

‎packages/snaps-controllers/src/test-utils/controller.ts‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,6 @@ export const getSnapControllerMessenger = (
491491
'PermissionController:revokePermissionForAllSubjects',
492492
'PermissionController:updateCaveat',
493493
'PermissionController:getSubjectNames',
494-
'PhishingController:maybeUpdateState',
495494
'PhishingController:testOrigin',
496495
'SnapController:get',
497496
'SnapController:handleRequest',
@@ -781,7 +780,6 @@ export const getRestrictedSnapInterfaceControllerMessenger = (
781780
name: 'SnapInterfaceController',
782781
allowedActions: [
783782
'PhishingController:testOrigin',
784-
'PhishingController:maybeUpdateState',
785783
'ApprovalController:hasRequest',
786784
'ApprovalController:acceptRequest',
787785
'MultichainAssetsController:getState',
@@ -795,11 +793,6 @@ export const getRestrictedSnapInterfaceControllerMessenger = (
795793
});
796794

797795
if (mocked) {
798-
messenger.registerActionHandler(
799-
'PhishingController:maybeUpdateState',
800-
async () => Promise.resolve(),
801-
);
802-
803796
messenger.registerActionHandler('PhishingController:testOrigin', () => ({
804797
result: false,
805798
type: 'all',

‎packages/snaps-jest/src/test-utils/controller.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,6 @@ export const getRootControllerMessenger = (mocked = true) => {
1010
>();
1111

1212
if (mocked) {
13-
messenger.registerActionHandler(
14-
'PhishingController:maybeUpdateState',
15-
async () => Promise.resolve(),
16-
);
17-
1813
messenger.registerActionHandler('PhishingController:testOrigin', () => ({
1914
result: false,
2015
type: 'all',
@@ -51,7 +46,6 @@ export const getRestrictedSnapInterfaceControllerMessenger = (
5146
name: 'SnapInterfaceController',
5247
allowedActions: [
5348
'PhishingController:testOrigin',
54-
'PhishingController:maybeUpdateState',
5549
'ApprovalController:hasRequest',
5650
'ApprovalController:acceptRequest',
5751
],

‎packages/snaps-simulation/src/controllers.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ export function getControllers(options: GetControllersOptions): Controllers {
8484
messenger: controllerMessenger.getRestricted({
8585
name: 'SnapInterfaceController',
8686
allowedActions: [
87-
'PhishingController:maybeUpdateState',
8887
'PhishingController:testOrigin',
8988
'ApprovalController:hasRequest',
9089
'ApprovalController:acceptRequest',

‎packages/snaps-simulation/src/request.test.tsx‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ describe('getInterfaceApi', () => {
419419
await snapInterface.clickElement('foo');
420420

421421
expect(controllerMessenger.call).toHaveBeenNthCalledWith(
422-
5,
422+
4,
423423
'ExecutionService:handleRpcRequest',
424424
MOCK_SNAP_ID,
425425
{
@@ -466,7 +466,7 @@ describe('getInterfaceApi', () => {
466466
await snapInterface.typeInField('foo', 'bar');
467467

468468
expect(controllerMessenger.call).toHaveBeenNthCalledWith(
469-
6,
469+
5,
470470
'ExecutionService:handleRpcRequest',
471471
MOCK_SNAP_ID,
472472
{
@@ -519,7 +519,7 @@ describe('getInterfaceApi', () => {
519519
await snapInterface.selectInDropdown('foo', 'option2');
520520

521521
expect(controllerMessenger.call).toHaveBeenNthCalledWith(
522-
6,
522+
5,
523523
'ExecutionService:handleRpcRequest',
524524
MOCK_SNAP_ID,
525525
{
@@ -572,7 +572,7 @@ describe('getInterfaceApi', () => {
572572
await snapInterface.selectFromRadioGroup('foo', 'option2');
573573

574574
expect(controllerMessenger.call).toHaveBeenNthCalledWith(
575-
6,
575+
5,
576576
'ExecutionService:handleRpcRequest',
577577
MOCK_SNAP_ID,
578578
{
@@ -629,7 +629,7 @@ describe('getInterfaceApi', () => {
629629
await snapInterface.selectFromSelector('foo', 'option2');
630630

631631
expect(controllerMessenger.call).toHaveBeenNthCalledWith(
632-
6,
632+
5,
633633
'ExecutionService:handleRpcRequest',
634634
MOCK_SNAP_ID,
635635
{

‎packages/snaps-simulation/src/simulation.test.ts‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,7 @@ describe('getPermittedHooks', () => {
491491
await updateInterface(id, content);
492492

493493
expect(controllerMessenger.call).toHaveBeenNthCalledWith(
494-
3,
494+
2,
495495
'SnapInterfaceController:updateInterface',
496496
snapId,
497497
id,
@@ -532,7 +532,7 @@ describe('getPermittedHooks', () => {
532532
const result = getInterfaceState(id);
533533

534534
expect(controllerMessenger.call).toHaveBeenNthCalledWith(
535-
3,
535+
2,
536536
'SnapInterfaceController:getInterface',
537537
snapId,
538538
id,
@@ -573,7 +573,7 @@ describe('getPermittedHooks', () => {
573573
const result = getInterfaceContext(id);
574574

575575
expect(controllerMessenger.call).toHaveBeenNthCalledWith(
576-
3,
576+
2,
577577
'SnapInterfaceController:getInterface',
578578
snapId,
579579
id,
@@ -616,7 +616,7 @@ describe('getPermittedHooks', () => {
616616
await resolveInterface(id, 'foobar');
617617

618618
expect(controllerMessenger.call).toHaveBeenNthCalledWith(
619-
2,
619+
1,
620620
'SnapInterfaceController:resolveInterface',
621621
snapId,
622622
id,
@@ -631,14 +631,6 @@ describe('registerActions', () => {
631631
const { runSaga, store } = createStore(getMockOptions());
632632
const controllerMessenger = getRootControllerMessenger(false);
633633

634-
it('registers `PhishingController:maybeUpdateState`', async () => {
635-
registerActions(controllerMessenger, runSaga);
636-
637-
expect(
638-
await controllerMessenger.call('PhishingController:maybeUpdateState'),
639-
).toBeUndefined();
640-
});
641-
642634
it('registers `PhishingController:testOrigin`', async () => {
643635
registerActions(controllerMessenger, runSaga);
644636

‎packages/snaps-simulation/src/simulation.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -470,11 +470,6 @@ export function registerActions(
470470
controllerMessenger: RootControllerMessenger,
471471
runSaga: RunSagaFunction,
472472
) {
473-
controllerMessenger.registerActionHandler(
474-
'PhishingController:maybeUpdateState',
475-
async () => Promise.resolve(),
476-
);
477-
478473
controllerMessenger.registerActionHandler(
479474
'PhishingController:testOrigin',
480475
() => ({ result: false, type: PhishingDetectorResultType.All }),

0 commit comments

Comments
 (0)