Skip to content

Commit 572b1f0

Browse files
imblue-dabadeeimblue-dabadee
authored
fix: transaction listener for mobile (#7139)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> Currently, the Phishing Controller is correctly listening to the Transaction Controller's state changes and scanning the received tokens within a transaction correctly. This feature does not work correctly in the Mobile due to differences in how state changes are emitted. This fix aims to correct for that by also listening for state changes that reveal the same information. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Process `transactions[*].simulationData` patches to trigger bulk token scanning; add tests and changelog entry. > > - **PhishingController**: > - Add `#isSimulationDataPatch` and update `#onTransactionControllerStateChange` to process `transactions[*].simulationData` patches and group tokens by chain for `bulkScanTokens`. > - Continue skipping `remove` ops; retain transaction-level handling. > - **Tests (`packages/phishing-controller/src/PhishingController.test.ts`)**: > - Add integration tests ensuring scans trigger on `simulationData` patches, skip on `remove`, and handle no token changes/default params and error paths. > - **Changelog**: > - Note fix for Transaction Controller listener picking up mobile state changes. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5c887b8. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent a7a17b6 commit 572b1f0

File tree

3 files changed

+194
-149
lines changed

3 files changed

+194
-149
lines changed

‎packages/phishing-controller/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Fixed the Transaction Controller listener to correctly pick up state changes for mobile ([#7139](https://github.com/MetaMask/core/pull/7139))
13+
1014
## [15.0.0]
1115

1216
### Changed

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

Lines changed: 170 additions & 149 deletions
Original file line numberDiff line numberDiff line change
@@ -3600,194 +3600,215 @@ describe('URL Scan Cache', () => {
36003600
`);
36013601
});
36023602
});
3603+
});
36033604

3604-
describe('Transaction Controller State Change Integration', () => {
3605-
let controller: PhishingController;
3606-
let globalMessenger: RootMessenger;
3607-
let bulkScanTokensSpy: jest.SpyInstance;
3605+
describe('Transaction Controller State Change Integration', () => {
3606+
let controller: PhishingController;
3607+
let globalMessenger: RootMessenger;
3608+
let bulkScanTokensSpy: jest.SpyInstance;
36083609

3609-
beforeEach(() => {
3610-
const { messenger, rootMessenger } = setupMessenger();
3610+
beforeEach(() => {
3611+
const { messenger, rootMessenger } = setupMessenger();
36113612

3612-
globalMessenger = rootMessenger;
3613+
globalMessenger = rootMessenger;
36133614

3614-
controller = new PhishingController({
3615-
messenger,
3616-
});
3617-
3618-
bulkScanTokensSpy = jest
3619-
.spyOn(controller, 'bulkScanTokens')
3620-
.mockResolvedValue({});
3615+
controller = new PhishingController({
3616+
messenger,
36213617
});
36223618

3623-
afterEach(() => {
3624-
bulkScanTokensSpy.mockRestore();
3625-
});
3619+
bulkScanTokensSpy = jest
3620+
.spyOn(controller, 'bulkScanTokens')
3621+
.mockResolvedValue({});
3622+
});
36263623

3627-
it('should trigger bulk token scanning when transaction with token balance changes is added', async () => {
3628-
const mockTransaction = createMockTransaction('test-tx-1', [
3629-
TEST_ADDRESSES.USDC,
3630-
TEST_ADDRESSES.MOCK_TOKEN_1,
3631-
]);
3632-
const stateChangePayload = createMockStateChangePayload([
3633-
mockTransaction,
3634-
]);
3624+
afterEach(() => {
3625+
bulkScanTokensSpy.mockRestore();
3626+
});
36353627

3636-
globalMessenger.publish(
3637-
'TransactionController:stateChange',
3638-
stateChangePayload,
3639-
[
3640-
{
3641-
op: 'add' as const,
3642-
path: ['transactions', 0],
3643-
value: mockTransaction,
3644-
},
3645-
],
3646-
);
3628+
it('triggers bulk token scanning when transaction with token balance changes is added', async () => {
3629+
const mockTransaction = createMockTransaction('test-tx-1', [
3630+
TEST_ADDRESSES.USDC,
3631+
TEST_ADDRESSES.MOCK_TOKEN_1,
3632+
]);
3633+
const stateChangePayload = createMockStateChangePayload([mockTransaction]);
3634+
3635+
globalMessenger.publish(
3636+
'TransactionController:stateChange',
3637+
stateChangePayload,
3638+
[
3639+
{
3640+
op: 'add' as const,
3641+
path: ['transactions', 0],
3642+
value: mockTransaction,
3643+
},
3644+
],
3645+
);
36473646

3648-
await new Promise(process.nextTick);
3647+
await new Promise(process.nextTick);
36493648

3650-
expect(bulkScanTokensSpy).toHaveBeenCalledWith({
3651-
chainId: mockTransaction.chainId.toLowerCase(),
3652-
tokens: [
3653-
TEST_ADDRESSES.USDC.toLowerCase(),
3654-
TEST_ADDRESSES.MOCK_TOKEN_1.toLowerCase(),
3655-
],
3656-
});
3649+
expect(bulkScanTokensSpy).toHaveBeenCalledWith({
3650+
chainId: mockTransaction.chainId.toLowerCase(),
3651+
tokens: [
3652+
TEST_ADDRESSES.USDC.toLowerCase(),
3653+
TEST_ADDRESSES.MOCK_TOKEN_1.toLowerCase(),
3654+
],
36573655
});
3656+
});
36583657

3659-
it('should skip processing when patch operation is remove', async () => {
3660-
const mockTransaction = createMockTransaction('test-tx-1', [
3661-
TEST_ADDRESSES.USDC,
3662-
]);
3658+
it('triggers bulk token scanning when patch path includes simulationData', async () => {
3659+
const mockTransaction = createMockTransaction('test-tx-1', [
3660+
TEST_ADDRESSES.USDC,
3661+
TEST_ADDRESSES.MOCK_TOKEN_1,
3662+
]);
3663+
const stateChangePayload = createMockStateChangePayload([mockTransaction]);
3664+
3665+
globalMessenger.publish(
3666+
'TransactionController:stateChange',
3667+
stateChangePayload,
3668+
[
3669+
{
3670+
op: 'add' as const,
3671+
path: ['transactions', 0, 'simulationData'],
3672+
value: mockTransaction.simulationData,
3673+
},
3674+
],
3675+
);
3676+
await new Promise(process.nextTick);
36633677

3664-
const stateChangePayload = createMockStateChangePayload([]);
3678+
expect(bulkScanTokensSpy).toHaveBeenCalledWith({
3679+
chainId: mockTransaction.chainId.toLowerCase(),
3680+
tokens: [
3681+
TEST_ADDRESSES.USDC.toLowerCase(),
3682+
TEST_ADDRESSES.MOCK_TOKEN_1.toLowerCase(),
3683+
],
3684+
});
3685+
});
36653686

3666-
globalMessenger.publish(
3667-
'TransactionController:stateChange',
3668-
stateChangePayload,
3669-
[
3670-
{
3671-
op: 'remove' as const,
3672-
path: ['transactions', 0],
3673-
value: mockTransaction,
3674-
},
3675-
],
3676-
);
3687+
it('skips processing when patch operation is remove', async () => {
3688+
const mockTransaction = createMockTransaction('test-tx-1', [
3689+
TEST_ADDRESSES.USDC,
3690+
]);
36773691

3678-
await new Promise(process.nextTick);
3692+
const stateChangePayload = createMockStateChangePayload([]);
36793693

3680-
expect(bulkScanTokensSpy).not.toHaveBeenCalled();
3681-
});
3694+
globalMessenger.publish(
3695+
'TransactionController:stateChange',
3696+
stateChangePayload,
3697+
[
3698+
{
3699+
op: 'remove' as const,
3700+
path: ['transactions', 0],
3701+
value: mockTransaction,
3702+
},
3703+
],
3704+
);
36823705

3683-
it('should not trigger bulk token scanning when transaction has no token balance changes', async () => {
3684-
const mockTransaction = createMockTransaction('test-tx-1', []);
3706+
await new Promise(process.nextTick);
36853707

3686-
const stateChangePayload = createMockStateChangePayload([
3687-
mockTransaction,
3688-
]);
3708+
expect(bulkScanTokensSpy).not.toHaveBeenCalled();
3709+
});
36893710

3690-
globalMessenger.publish(
3691-
'TransactionController:stateChange',
3692-
stateChangePayload,
3693-
[
3694-
{
3695-
op: 'add' as const,
3696-
path: ['transactions', 0],
3697-
value: mockTransaction,
3698-
},
3699-
],
3700-
);
3711+
it('does not trigger bulk token scanning when transaction has no token balance changes', async () => {
3712+
const mockTransaction = createMockTransaction('test-tx-1', []);
37013713

3702-
await new Promise(process.nextTick);
3714+
const stateChangePayload = createMockStateChangePayload([mockTransaction]);
37033715

3704-
expect(bulkScanTokensSpy).not.toHaveBeenCalled();
3705-
});
3716+
globalMessenger.publish(
3717+
'TransactionController:stateChange',
3718+
stateChangePayload,
3719+
[
3720+
{
3721+
op: 'add' as const,
3722+
path: ['transactions', 0],
3723+
value: mockTransaction,
3724+
},
3725+
],
3726+
);
37063727

3707-
it('should not trigger bulk token scanning when using default tokenAddresses parameter', async () => {
3708-
const mockTransaction = createMockTransaction('test-tx-2');
3728+
await new Promise(process.nextTick);
37093729

3710-
const stateChangePayload = createMockStateChangePayload([
3711-
mockTransaction,
3712-
]);
3730+
expect(bulkScanTokensSpy).not.toHaveBeenCalled();
3731+
});
37133732

3714-
globalMessenger.publish(
3715-
'TransactionController:stateChange',
3716-
stateChangePayload,
3717-
[
3718-
{
3719-
op: 'add' as const,
3720-
path: ['transactions', 0],
3721-
value: mockTransaction,
3722-
},
3723-
],
3724-
);
3733+
it('does not trigger bulk token scanning when using default tokenAddresses parameter', async () => {
3734+
const mockTransaction = createMockTransaction('test-tx-2');
37253735

3726-
await new Promise(process.nextTick);
3736+
const stateChangePayload = createMockStateChangePayload([mockTransaction]);
37273737

3728-
expect(bulkScanTokensSpy).not.toHaveBeenCalled();
3729-
});
3738+
globalMessenger.publish(
3739+
'TransactionController:stateChange',
3740+
stateChangePayload,
3741+
[
3742+
{
3743+
op: 'add' as const,
3744+
path: ['transactions', 0],
3745+
value: mockTransaction,
3746+
},
3747+
],
3748+
);
37303749

3731-
it('should handle errors in transaction state change processing', async () => {
3732-
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
3750+
await new Promise(process.nextTick);
37333751

3734-
const stateChangePayload = createMockStateChangePayload([]);
3752+
expect(bulkScanTokensSpy).not.toHaveBeenCalled();
3753+
});
37353754

3736-
globalMessenger.publish(
3737-
'TransactionController:stateChange',
3738-
stateChangePayload,
3739-
[
3740-
{
3741-
op: 'add' as const,
3742-
path: ['transactions', 0],
3743-
value: null,
3744-
},
3745-
],
3746-
);
3755+
it('handles errors in transaction state change processing', async () => {
3756+
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
37473757

3748-
await new Promise(process.nextTick);
3758+
const stateChangePayload = createMockStateChangePayload([]);
37493759

3750-
expect(consoleErrorSpy).toHaveBeenCalledWith(
3751-
'Error processing transaction state change:',
3752-
expect.any(Error),
3753-
);
3760+
globalMessenger.publish(
3761+
'TransactionController:stateChange',
3762+
stateChangePayload,
3763+
[
3764+
{
3765+
op: 'add' as const,
3766+
path: ['transactions', 0],
3767+
value: null,
3768+
},
3769+
],
3770+
);
37543771

3755-
consoleErrorSpy.mockRestore();
3756-
});
3772+
await new Promise(process.nextTick);
37573773

3758-
it('should handle errors in bulk token scanning', async () => {
3759-
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
3774+
expect(consoleErrorSpy).toHaveBeenCalledWith(
3775+
'Error processing transaction state change:',
3776+
expect.any(Error),
3777+
);
37603778

3761-
bulkScanTokensSpy.mockRejectedValue(new Error('Scanning failed'));
3779+
consoleErrorSpy.mockRestore();
3780+
});
37623781

3763-
const mockTransaction = createMockTransaction('test-tx-1', [
3764-
TEST_ADDRESSES.USDC,
3765-
]);
3782+
it('handles errors in bulk token scanning', async () => {
3783+
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation();
37663784

3767-
const stateChangePayload = createMockStateChangePayload([
3768-
mockTransaction,
3769-
]);
3785+
bulkScanTokensSpy.mockRejectedValue(new Error('Scanning failed'));
37703786

3771-
globalMessenger.publish(
3772-
'TransactionController:stateChange',
3773-
stateChangePayload,
3774-
[
3775-
{
3776-
op: 'add' as const,
3777-
path: ['transactions', 0],
3778-
value: mockTransaction,
3779-
},
3780-
],
3781-
);
3787+
const mockTransaction = createMockTransaction('test-tx-1', [
3788+
TEST_ADDRESSES.USDC,
3789+
]);
37823790

3783-
await new Promise(process.nextTick);
3791+
const stateChangePayload = createMockStateChangePayload([mockTransaction]);
37843792

3785-
expect(consoleErrorSpy).toHaveBeenCalledWith(
3786-
'Error scanning tokens for chain 0x1:',
3787-
expect.any(Error),
3788-
);
3793+
globalMessenger.publish(
3794+
'TransactionController:stateChange',
3795+
stateChangePayload,
3796+
[
3797+
{
3798+
op: 'add' as const,
3799+
path: ['transactions', 0],
3800+
value: mockTransaction,
3801+
},
3802+
],
3803+
);
37893804

3790-
consoleErrorSpy.mockRestore();
3791-
});
3805+
await new Promise(process.nextTick);
3806+
3807+
expect(consoleErrorSpy).toHaveBeenCalledWith(
3808+
'Error scanning tokens for chain 0x1:',
3809+
expect.any(Error),
3810+
);
3811+
3812+
consoleErrorSpy.mockRestore();
37923813
});
37933814
});

0 commit comments

Comments
 (0)