From fabb97d6bfafca6acd94ee9475484420f249b234 Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Wed, 12 Nov 2025 09:35:23 -0600 Subject: [PATCH 1/5] fix: checks if a patch is a simulationData change --- .../src/PhishingController.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index e11b622aaf9..b2faeae77e5 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -12,6 +12,7 @@ import { type Messenger } from '@metamask/messenger'; import type { TransactionControllerStateChangeEvent, TransactionMeta, + SimulationData, } from '@metamask/transaction-controller'; import type { Patch } from 'immer'; import { toASCII } from 'punycode/punycode.js'; @@ -571,6 +572,22 @@ export class PhishingController extends BaseController< ); } + /** + * Checks if a patch represents a simulation data change + * + * @param patch - Immer patch to check + * @returns True if patch represents a simulation data change + */ + #isSimulationDataPatch(patch: Patch): boolean { + const { path } = patch; + return ( + path.length === 3 && + path[0] === 'transactions' && + typeof path[1] === 'number' && + path[2] === 'simulationData' + ); + } + /** * Handle transaction controller state changes using Immer patches * Extracts token addresses from simulation data and groups them by chain for bulk scanning @@ -595,6 +612,10 @@ export class PhishingController extends BaseController< if (this.#isTransactionPatch(patch)) { const transaction = patch.value as TransactionMeta; this.#getTokensFromTransaction(transaction, tokensByChain); + } else if (this.#isSimulationDataPatch(patch)) { + const transactionIndex = patch.path[1] as number; + const transaction = _state.transactions?.[transactionIndex]; + this.#getTokensFromTransaction(transaction, tokensByChain); } } From c1415c165f1fd0f2f43e69eb05730b349fd9497b Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Wed, 12 Nov 2025 09:35:58 -0600 Subject: [PATCH 2/5] test(refactor): move tx controller describe group out --- .../src/PhishingController.test.ts | 290 +++++++++--------- 1 file changed, 141 insertions(+), 149 deletions(-) diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index 5bc20d972df..a053891a20a 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -3600,194 +3600,186 @@ describe('URL Scan Cache', () => { `); }); }); +}); - describe('Transaction Controller State Change Integration', () => { - let controller: PhishingController; - let globalMessenger: RootMessenger; - let bulkScanTokensSpy: jest.SpyInstance; +describe('Transaction Controller State Change Integration', () => { + let controller: PhishingController; + let globalMessenger: RootMessenger; + let bulkScanTokensSpy: jest.SpyInstance; - beforeEach(() => { - const { messenger, rootMessenger } = setupMessenger(); - - globalMessenger = rootMessenger; + beforeEach(() => { + const { messenger, rootMessenger } = setupMessenger(); - controller = new PhishingController({ - messenger, - }); + globalMessenger = rootMessenger; - bulkScanTokensSpy = jest - .spyOn(controller, 'bulkScanTokens') - .mockResolvedValue({}); + controller = new PhishingController({ + messenger, }); - afterEach(() => { - bulkScanTokensSpy.mockRestore(); - }); + bulkScanTokensSpy = jest + .spyOn(controller, 'bulkScanTokens') + .mockResolvedValue({}); + }); - it('should trigger bulk token scanning when transaction with token balance changes is added', async () => { - const mockTransaction = createMockTransaction('test-tx-1', [ - TEST_ADDRESSES.USDC, - TEST_ADDRESSES.MOCK_TOKEN_1, - ]); - const stateChangePayload = createMockStateChangePayload([ - mockTransaction, - ]); + afterEach(() => { + bulkScanTokensSpy.mockRestore(); + }); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'add' as const, - path: ['transactions', 0], - value: mockTransaction, - }, - ], - ); + it('triggers bulk token scanning when transaction with token balance changes is added', async () => { + const mockTransaction = createMockTransaction('test-tx-1', [ + TEST_ADDRESSES.USDC, + TEST_ADDRESSES.MOCK_TOKEN_1, + ]); + const stateChangePayload = createMockStateChangePayload([mockTransaction]); + + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0], + value: mockTransaction, + }, + ], + ); - await new Promise(process.nextTick); + await new Promise(process.nextTick); - expect(bulkScanTokensSpy).toHaveBeenCalledWith({ - chainId: mockTransaction.chainId.toLowerCase(), - tokens: [ - TEST_ADDRESSES.USDC.toLowerCase(), - TEST_ADDRESSES.MOCK_TOKEN_1.toLowerCase(), - ], - }); + expect(bulkScanTokensSpy).toHaveBeenCalledWith({ + chainId: mockTransaction.chainId.toLowerCase(), + tokens: [ + TEST_ADDRESSES.USDC.toLowerCase(), + TEST_ADDRESSES.MOCK_TOKEN_1.toLowerCase(), + ], }); + }); - it('should skip processing when patch operation is remove', async () => { - const mockTransaction = createMockTransaction('test-tx-1', [ - TEST_ADDRESSES.USDC, - ]); + it('skips processing when patch operation is remove', async () => { + const mockTransaction = createMockTransaction('test-tx-1', [ + TEST_ADDRESSES.USDC, + ]); - const stateChangePayload = createMockStateChangePayload([]); + const stateChangePayload = createMockStateChangePayload([]); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'remove' as const, - path: ['transactions', 0], - value: mockTransaction, - }, - ], - ); + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'remove' as const, + path: ['transactions', 0], + value: mockTransaction, + }, + ], + ); - await new Promise(process.nextTick); + await new Promise(process.nextTick); - expect(bulkScanTokensSpy).not.toHaveBeenCalled(); - }); + expect(bulkScanTokensSpy).not.toHaveBeenCalled(); + }); - it('should not trigger bulk token scanning when transaction has no token balance changes', async () => { - const mockTransaction = createMockTransaction('test-tx-1', []); + it('does not trigger bulk token scanning when transaction has no token balance changes', async () => { + const mockTransaction = createMockTransaction('test-tx-1', []); - const stateChangePayload = createMockStateChangePayload([ - mockTransaction, - ]); + const stateChangePayload = createMockStateChangePayload([mockTransaction]); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'add' as const, - path: ['transactions', 0], - value: mockTransaction, - }, - ], - ); + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0], + value: mockTransaction, + }, + ], + ); - await new Promise(process.nextTick); + await new Promise(process.nextTick); - expect(bulkScanTokensSpy).not.toHaveBeenCalled(); - }); + expect(bulkScanTokensSpy).not.toHaveBeenCalled(); + }); - it('should not trigger bulk token scanning when using default tokenAddresses parameter', async () => { - const mockTransaction = createMockTransaction('test-tx-2'); + it('does not trigger bulk token scanning when using default tokenAddresses parameter', async () => { + const mockTransaction = createMockTransaction('test-tx-2'); - const stateChangePayload = createMockStateChangePayload([ - mockTransaction, - ]); + const stateChangePayload = createMockStateChangePayload([mockTransaction]); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'add' as const, - path: ['transactions', 0], - value: mockTransaction, - }, - ], - ); + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0], + value: mockTransaction, + }, + ], + ); - await new Promise(process.nextTick); + await new Promise(process.nextTick); - expect(bulkScanTokensSpy).not.toHaveBeenCalled(); - }); + expect(bulkScanTokensSpy).not.toHaveBeenCalled(); + }); - it('should handle errors in transaction state change processing', async () => { - const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + it('handles errors in transaction state change processing', async () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); - const stateChangePayload = createMockStateChangePayload([]); + const stateChangePayload = createMockStateChangePayload([]); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'add' as const, - path: ['transactions', 0], - value: null, - }, - ], - ); + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0], + value: null, + }, + ], + ); - await new Promise(process.nextTick); + await new Promise(process.nextTick); - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Error processing transaction state change:', - expect.any(Error), - ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Error processing transaction state change:', + expect.any(Error), + ); - consoleErrorSpy.mockRestore(); - }); + consoleErrorSpy.mockRestore(); + }); - it('should handle errors in bulk token scanning', async () => { - const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + it('handles errors in bulk token scanning', async () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); - bulkScanTokensSpy.mockRejectedValue(new Error('Scanning failed')); + bulkScanTokensSpy.mockRejectedValue(new Error('Scanning failed')); - const mockTransaction = createMockTransaction('test-tx-1', [ - TEST_ADDRESSES.USDC, - ]); + const mockTransaction = createMockTransaction('test-tx-1', [ + TEST_ADDRESSES.USDC, + ]); - const stateChangePayload = createMockStateChangePayload([ - mockTransaction, - ]); + const stateChangePayload = createMockStateChangePayload([mockTransaction]); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'add' as const, - path: ['transactions', 0], - value: mockTransaction, - }, - ], - ); + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0], + value: mockTransaction, + }, + ], + ); - await new Promise(process.nextTick); + await new Promise(process.nextTick); - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Error scanning tokens for chain 0x1:', - expect.any(Error), - ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Error scanning tokens for chain 0x1:', + expect.any(Error), + ); - consoleErrorSpy.mockRestore(); - }); + consoleErrorSpy.mockRestore(); }); }); From fb2a0e50faeec8e9947f823d34d121a3c6e31b9c Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Wed, 12 Nov 2025 09:36:18 -0600 Subject: [PATCH 3/5] test: simulationData patch scanning --- .../src/PhishingController.test.ts | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index a053891a20a..f4a329e7fbf 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -3655,6 +3655,35 @@ describe('Transaction Controller State Change Integration', () => { }); }); + it('triggers bulk token scanning when patch path includes simulationData', async () => { + const mockTransaction = createMockTransaction('test-tx-1', [ + TEST_ADDRESSES.USDC, + TEST_ADDRESSES.MOCK_TOKEN_1, + ]); + const stateChangePayload = createMockStateChangePayload([mockTransaction]); + + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0, 'simulationData'], + value: mockTransaction.simulationData, + }, + ], + ); + await new Promise(process.nextTick); + + expect(bulkScanTokensSpy).toHaveBeenCalledWith({ + chainId: mockTransaction.chainId.toLowerCase(), + tokens: [ + TEST_ADDRESSES.USDC.toLowerCase(), + TEST_ADDRESSES.MOCK_TOKEN_1.toLowerCase(), + ], + }); + }); + it('skips processing when patch operation is remove', async () => { const mockTransaction = createMockTransaction('test-tx-1', [ TEST_ADDRESSES.USDC, From 4847305775cffa9e00601230aad739478dabccfb Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Wed, 12 Nov 2025 09:51:57 -0600 Subject: [PATCH 4/5] chore: remove unused var --- packages/phishing-controller/src/PhishingController.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index b2faeae77e5..ffd287a7f41 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -12,7 +12,6 @@ import { type Messenger } from '@metamask/messenger'; import type { TransactionControllerStateChangeEvent, TransactionMeta, - SimulationData, } from '@metamask/transaction-controller'; import type { Patch } from 'immer'; import { toASCII } from 'punycode/punycode.js'; From 5c887b81e0a22efde7677e3a4f210fd8deb5d1d5 Mon Sep 17 00:00:00 2001 From: imblue-dabadee Date: Thu, 13 Nov 2025 09:07:31 -0600 Subject: [PATCH 5/5] chore: update changelog --- packages/phishing-controller/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/phishing-controller/CHANGELOG.md b/packages/phishing-controller/CHANGELOG.md index 2941f4a5c30..e6c04c045cc 100644 --- a/packages/phishing-controller/CHANGELOG.md +++ b/packages/phishing-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fixed the Transaction Controller listener to correctly pick up state changes for mobile ([#7139](https://github.com/MetaMask/core/pull/7139)) + ## [15.0.0] ### Changed