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 diff --git a/packages/phishing-controller/src/PhishingController.test.ts b/packages/phishing-controller/src/PhishingController.test.ts index 5bc20d972df..f4a329e7fbf 100644 --- a/packages/phishing-controller/src/PhishingController.test.ts +++ b/packages/phishing-controller/src/PhishingController.test.ts @@ -3600,194 +3600,215 @@ 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(); + beforeEach(() => { + const { messenger, rootMessenger } = setupMessenger(); - globalMessenger = rootMessenger; + globalMessenger = rootMessenger; - controller = new PhishingController({ - messenger, - }); - - 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('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); - const stateChangePayload = createMockStateChangePayload([]); + expect(bulkScanTokensSpy).toHaveBeenCalledWith({ + chainId: mockTransaction.chainId.toLowerCase(), + tokens: [ + TEST_ADDRESSES.USDC.toLowerCase(), + TEST_ADDRESSES.MOCK_TOKEN_1.toLowerCase(), + ], + }); + }); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'remove' as const, - path: ['transactions', 0], - value: mockTransaction, - }, - ], - ); + it('skips processing when patch operation is remove', async () => { + const mockTransaction = createMockTransaction('test-tx-1', [ + TEST_ADDRESSES.USDC, + ]); - await new Promise(process.nextTick); + const stateChangePayload = createMockStateChangePayload([]); - expect(bulkScanTokensSpy).not.toHaveBeenCalled(); - }); + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'remove' as const, + path: ['transactions', 0], + value: mockTransaction, + }, + ], + ); - it('should not trigger bulk token scanning when transaction has no token balance changes', async () => { - const mockTransaction = createMockTransaction('test-tx-1', []); + await new Promise(process.nextTick); - const stateChangePayload = createMockStateChangePayload([ - mockTransaction, - ]); + expect(bulkScanTokensSpy).not.toHaveBeenCalled(); + }); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'add' as const, - path: ['transactions', 0], - value: mockTransaction, - }, - ], - ); + it('does not trigger bulk token scanning when transaction has no token balance changes', async () => { + const mockTransaction = createMockTransaction('test-tx-1', []); - await new Promise(process.nextTick); + const stateChangePayload = createMockStateChangePayload([mockTransaction]); - expect(bulkScanTokensSpy).not.toHaveBeenCalled(); - }); + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0], + value: mockTransaction, + }, + ], + ); - it('should not trigger bulk token scanning when using default tokenAddresses parameter', async () => { - const mockTransaction = createMockTransaction('test-tx-2'); + await new Promise(process.nextTick); - const stateChangePayload = createMockStateChangePayload([ - mockTransaction, - ]); + expect(bulkScanTokensSpy).not.toHaveBeenCalled(); + }); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'add' as const, - path: ['transactions', 0], - value: mockTransaction, - }, - ], - ); + it('does not trigger bulk token scanning when using default tokenAddresses parameter', async () => { + const mockTransaction = createMockTransaction('test-tx-2'); - await new Promise(process.nextTick); + const stateChangePayload = createMockStateChangePayload([mockTransaction]); - expect(bulkScanTokensSpy).not.toHaveBeenCalled(); - }); + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0], + value: mockTransaction, + }, + ], + ); - it('should handle errors in transaction state change processing', async () => { - const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + await new Promise(process.nextTick); - const stateChangePayload = createMockStateChangePayload([]); + expect(bulkScanTokensSpy).not.toHaveBeenCalled(); + }); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'add' as const, - path: ['transactions', 0], - value: null, - }, - ], - ); + it('handles errors in transaction state change processing', async () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); - await new Promise(process.nextTick); + const stateChangePayload = createMockStateChangePayload([]); - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Error processing transaction state change:', - expect.any(Error), - ); + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0], + value: null, + }, + ], + ); - consoleErrorSpy.mockRestore(); - }); + await new Promise(process.nextTick); - it('should handle errors in bulk token scanning', async () => { - const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Error processing transaction state change:', + expect.any(Error), + ); - bulkScanTokensSpy.mockRejectedValue(new Error('Scanning failed')); + consoleErrorSpy.mockRestore(); + }); - const mockTransaction = createMockTransaction('test-tx-1', [ - TEST_ADDRESSES.USDC, - ]); + it('handles errors in bulk token scanning', async () => { + const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(); - const stateChangePayload = createMockStateChangePayload([ - mockTransaction, - ]); + bulkScanTokensSpy.mockRejectedValue(new Error('Scanning failed')); - globalMessenger.publish( - 'TransactionController:stateChange', - stateChangePayload, - [ - { - op: 'add' as const, - path: ['transactions', 0], - value: mockTransaction, - }, - ], - ); + const mockTransaction = createMockTransaction('test-tx-1', [ + TEST_ADDRESSES.USDC, + ]); - await new Promise(process.nextTick); + const stateChangePayload = createMockStateChangePayload([mockTransaction]); - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Error scanning tokens for chain 0x1:', - expect.any(Error), - ); + globalMessenger.publish( + 'TransactionController:stateChange', + stateChangePayload, + [ + { + op: 'add' as const, + path: ['transactions', 0], + value: mockTransaction, + }, + ], + ); - consoleErrorSpy.mockRestore(); - }); + await new Promise(process.nextTick); + + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Error scanning tokens for chain 0x1:', + expect.any(Error), + ); + + consoleErrorSpy.mockRestore(); }); }); diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index e11b622aaf9..ffd287a7f41 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -571,6 +571,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 +611,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); } }