Skip to content

Commit 595f36d

Browse files
authored
Merge branch 'main' into chore/revokation-rpc-request
2 parents bf3f8c7 + 9b71a82 commit 595f36d

File tree

9 files changed

+488
-29
lines changed

9 files changed

+488
-29
lines changed

packages/phishing-controller/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515
- Add `tokenScanCache` to `PhishingControllerState`
1616
- Add proper action registration for `bulkScanTokens` method as `PhishingControllerBulkScanTokensAction`
1717
- Support for multiple chains including Ethereum, Polygon, BSC, Arbitrum, Avalanche, Base, Optimism, ect...
18+
- Add token screening from transaction simulation data ([#6617](https://github.com/MetaMask/core/pull/6617))
19+
- Add `#onTransactionControllerStateChange` method to handle transaction state changes
20+
- Add `#scanTokensFromSimulation` method to extract and scan tokens from transaction simulation data
21+
- Add `start` and `stop` methods to manage Transaction Controller state change subscription
1822
- Add two new controller state metadata properties: `includeInStateLogs` and `usedInUi` ([#6587](https://github.com/MetaMask/core/pull/6587))
1923

2024
### Changed

packages/phishing-controller/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
},
5858
"devDependencies": {
5959
"@metamask/auto-changelog": "^3.4.4",
60+
"@metamask/transaction-controller": "^60.4.0",
6061
"@types/jest": "^27.4.1",
6162
"deepmerge": "^4.2.2",
6263
"jest": "^27.5.1",
@@ -67,6 +68,9 @@
6768
"typedoc-plugin-missing-exports": "^2.0.0",
6869
"typescript": "~5.2.2"
6970
},
71+
"peerDependencies": {
72+
"@metamask/transaction-controller": "^60.4.0"
73+
},
7074
"engines": {
7175
"node": "^18.18 || >=20"
7276
},

packages/phishing-controller/src/BulkTokenScan.test.ts

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { Messenger } from '@metamask/base-controller';
22
import { safelyExecuteWithTimeout } from '@metamask/controller-utils';
3+
import type { TransactionControllerStateChangeEvent } from '@metamask/transaction-controller';
34
import nock, { cleanAll } from 'nock';
45
import sinon from 'sinon';
56

7+
import type { PhishingControllerEvents } from './PhishingController';
68
import {
79
PhishingController,
810
type PhishingControllerActions,
@@ -29,18 +31,24 @@ const mockSafelyExecuteWithTimeout =
2931
const controllerName = 'PhishingController';
3032

3133
/**
32-
* Constructs a restricted messenger.
34+
* Constructs a restricted messenger with transaction events enabled.
3335
*
34-
* @returns A restricted messenger.
36+
* @returns A restricted messenger that can listen to TransactionController events.
3537
*/
36-
function getRestrictedMessenger() {
37-
const messenger = new Messenger<PhishingControllerActions, never>();
38-
39-
return messenger.getRestricted({
40-
name: controllerName,
41-
allowedActions: [],
42-
allowedEvents: [],
43-
});
38+
function getRestrictedMessengerWithTransactionEvents() {
39+
const messenger = new Messenger<
40+
PhishingControllerActions,
41+
PhishingControllerEvents | TransactionControllerStateChangeEvent
42+
>();
43+
44+
return {
45+
messenger: messenger.getRestricted({
46+
name: controllerName,
47+
allowedActions: [],
48+
allowedEvents: ['TransactionController:stateChange'],
49+
}),
50+
globalMessenger: messenger,
51+
};
4452
}
4553

4654
/**
@@ -51,7 +59,7 @@ function getRestrictedMessenger() {
5159
*/
5260
function getPhishingController(options?: Partial<PhishingControllerOptions>) {
5361
return new PhishingController({
54-
messenger: getRestrictedMessenger(),
62+
messenger: getRestrictedMessengerWithTransactionEvents().messenger,
5563
...options,
5664
});
5765
}

packages/phishing-controller/src/PhishingController.test.ts

Lines changed: 217 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { deriveStateFromMetadata, Messenger } from '@metamask/base-controller';
2+
import type { TransactionControllerStateChangeEvent } from '@metamask/transaction-controller';
23
import { strict as assert } from 'assert';
34
import nock, { cleanAll, isDone, pendingMocks } from 'nock';
45
import sinon from 'sinon';
@@ -10,6 +11,7 @@ import {
1011
PhishingController,
1112
PHISHING_CONFIG_BASE_URL,
1213
type PhishingControllerActions,
14+
type PhishingControllerEvents,
1315
type PhishingControllerOptions,
1416
CLIENT_SIDE_DETECION_BASE_URL,
1517
C2_DOMAIN_BLOCKLIST_ENDPOINT,
@@ -18,26 +20,37 @@ import {
1820
PHISHING_DETECTION_BULK_SCAN_ENDPOINT,
1921
type BulkPhishingDetectionScanResponse,
2022
} from './PhishingController';
21-
import { formatHostnameToUrl } from './tests/utils';
23+
import {
24+
createMockStateChangePayload,
25+
createMockTransaction,
26+
formatHostnameToUrl,
27+
TEST_ADDRESSES,
28+
} from './tests/utils';
2229
import type { PhishingDetectionScanResult } from './types';
2330
import { PhishingDetectorResultType, RecommendedAction } from './types';
2431
import { getHostnameFromUrl } from './utils';
2532

2633
const controllerName = 'PhishingController';
2734

2835
/**
29-
* Constructs a restricted messenger.
36+
* Constructs a restricted messenger with transaction events enabled.
3037
*
31-
* @returns A restricted messenger.
38+
* @returns A restricted messenger that can listen to TransactionController events.
3239
*/
33-
function getRestrictedMessenger() {
34-
const messenger = new Messenger<PhishingControllerActions, never>();
35-
36-
return messenger.getRestricted({
37-
name: controllerName,
38-
allowedActions: [],
39-
allowedEvents: [],
40-
});
40+
function getRestrictedMessengerWithTransactionEvents() {
41+
const messenger = new Messenger<
42+
PhishingControllerActions,
43+
PhishingControllerEvents | TransactionControllerStateChangeEvent
44+
>();
45+
46+
return {
47+
messenger: messenger.getRestricted({
48+
name: controllerName,
49+
allowedActions: [],
50+
allowedEvents: ['TransactionController:stateChange'],
51+
}),
52+
globalMessenger: messenger,
53+
};
4154
}
4255

4356
/**
@@ -48,7 +61,7 @@ function getRestrictedMessenger() {
4861
*/
4962
function getPhishingController(options?: Partial<PhishingControllerOptions>) {
5063
return new PhishingController({
51-
messenger: getRestrictedMessenger(),
64+
messenger: getRestrictedMessengerWithTransactionEvents().messenger,
5265
...options,
5366
});
5467
}
@@ -3416,4 +3429,196 @@ describe('URL Scan Cache', () => {
34163429
`);
34173430
});
34183431
});
3432+
3433+
describe('Transaction Controller State Change Integration', () => {
3434+
let controller: PhishingController;
3435+
let globalMessenger: Messenger<
3436+
PhishingControllerActions,
3437+
PhishingControllerEvents | TransactionControllerStateChangeEvent
3438+
>;
3439+
let bulkScanTokensSpy: jest.SpyInstance;
3440+
3441+
beforeEach(() => {
3442+
const messengerSetup = getRestrictedMessengerWithTransactionEvents();
3443+
globalMessenger = messengerSetup.globalMessenger;
3444+
3445+
controller = new PhishingController({
3446+
messenger: messengerSetup.messenger,
3447+
});
3448+
3449+
bulkScanTokensSpy = jest
3450+
.spyOn(controller, 'bulkScanTokens')
3451+
.mockResolvedValue({});
3452+
});
3453+
3454+
afterEach(() => {
3455+
bulkScanTokensSpy.mockRestore();
3456+
});
3457+
3458+
it('should trigger bulk token scanning when transaction with token balance changes is added', async () => {
3459+
const mockTransaction = createMockTransaction('test-tx-1', [
3460+
TEST_ADDRESSES.USDC,
3461+
TEST_ADDRESSES.MOCK_TOKEN_1,
3462+
]);
3463+
const stateChangePayload = createMockStateChangePayload([
3464+
mockTransaction,
3465+
]);
3466+
3467+
globalMessenger.publish(
3468+
'TransactionController:stateChange',
3469+
stateChangePayload,
3470+
[
3471+
{
3472+
op: 'add' as const,
3473+
path: ['transactions', 0],
3474+
value: mockTransaction,
3475+
},
3476+
],
3477+
);
3478+
3479+
await new Promise(process.nextTick);
3480+
3481+
expect(bulkScanTokensSpy).toHaveBeenCalledWith({
3482+
chainId: mockTransaction.chainId.toLowerCase(),
3483+
tokens: [
3484+
TEST_ADDRESSES.USDC.toLowerCase(),
3485+
TEST_ADDRESSES.MOCK_TOKEN_1.toLowerCase(),
3486+
],
3487+
});
3488+
});
3489+
3490+
it('should skip processing when patch operation is remove', async () => {
3491+
const mockTransaction = createMockTransaction('test-tx-1', [
3492+
TEST_ADDRESSES.USDC,
3493+
]);
3494+
3495+
const stateChangePayload = createMockStateChangePayload([]);
3496+
3497+
globalMessenger.publish(
3498+
'TransactionController:stateChange',
3499+
stateChangePayload,
3500+
[
3501+
{
3502+
op: 'remove' as const,
3503+
path: ['transactions', 0],
3504+
value: mockTransaction,
3505+
},
3506+
],
3507+
);
3508+
3509+
await new Promise(process.nextTick);
3510+
3511+
expect(bulkScanTokensSpy).not.toHaveBeenCalled();
3512+
});
3513+
3514+
it('should not trigger bulk token scanning when transaction has no token balance changes', async () => {
3515+
const mockTransaction = createMockTransaction('test-tx-1', []);
3516+
3517+
const stateChangePayload = createMockStateChangePayload([
3518+
mockTransaction,
3519+
]);
3520+
3521+
globalMessenger.publish(
3522+
'TransactionController:stateChange',
3523+
stateChangePayload,
3524+
[
3525+
{
3526+
op: 'add' as const,
3527+
path: ['transactions', 0],
3528+
value: mockTransaction,
3529+
},
3530+
],
3531+
);
3532+
3533+
await new Promise(process.nextTick);
3534+
3535+
expect(bulkScanTokensSpy).not.toHaveBeenCalled();
3536+
});
3537+
3538+
it('should not trigger bulk token scanning when using default tokenAddresses parameter', async () => {
3539+
const mockTransaction = createMockTransaction('test-tx-2');
3540+
3541+
const stateChangePayload = createMockStateChangePayload([
3542+
mockTransaction,
3543+
]);
3544+
3545+
globalMessenger.publish(
3546+
'TransactionController:stateChange',
3547+
stateChangePayload,
3548+
[
3549+
{
3550+
op: 'add' as const,
3551+
path: ['transactions', 0],
3552+
value: mockTransaction,
3553+
},
3554+
],
3555+
);
3556+
3557+
await new Promise(process.nextTick);
3558+
3559+
expect(bulkScanTokensSpy).not.toHaveBeenCalled();
3560+
});
3561+
3562+
it('should handle errors in transaction state change processing', async () => {
3563+
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
3564+
3565+
const stateChangePayload = createMockStateChangePayload([]);
3566+
3567+
globalMessenger.publish(
3568+
'TransactionController:stateChange',
3569+
stateChangePayload,
3570+
[
3571+
{
3572+
op: 'add' as const,
3573+
path: ['transactions', 0],
3574+
value: null,
3575+
},
3576+
],
3577+
);
3578+
3579+
await new Promise(process.nextTick);
3580+
3581+
expect(consoleErrorSpy).toHaveBeenCalledWith(
3582+
'Error processing transaction state change:',
3583+
expect.any(Error),
3584+
);
3585+
3586+
consoleErrorSpy.mockRestore();
3587+
});
3588+
3589+
it('should handle errors in bulk token scanning', async () => {
3590+
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
3591+
3592+
bulkScanTokensSpy.mockRejectedValue(new Error('Scanning failed'));
3593+
3594+
const mockTransaction = createMockTransaction('test-tx-1', [
3595+
TEST_ADDRESSES.USDC,
3596+
]);
3597+
3598+
const stateChangePayload = createMockStateChangePayload([
3599+
mockTransaction,
3600+
]);
3601+
3602+
globalMessenger.publish(
3603+
'TransactionController:stateChange',
3604+
stateChangePayload,
3605+
[
3606+
{
3607+
op: 'add' as const,
3608+
path: ['transactions', 0],
3609+
value: mockTransaction,
3610+
},
3611+
],
3612+
);
3613+
3614+
await new Promise(process.nextTick);
3615+
3616+
expect(consoleErrorSpy).toHaveBeenCalledWith(
3617+
'Error scanning tokens for chain 0x1:',
3618+
expect.any(Error),
3619+
);
3620+
3621+
consoleErrorSpy.mockRestore();
3622+
});
3623+
});
34193624
});

0 commit comments

Comments
 (0)