Skip to content

Commit 61d1bb0

Browse files
micaelaemicaelae
authored
refactor: move Swaps QuoteReceived event publishing to bridge-controller (#7242)
## Explanation Publishes QuotesReceived event through BridgeController when quote polling stops on submit to ensure it happens before navigation resets the state <!-- 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? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## 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) - [x] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Routes QuotesReceived publishing through BridgeController when polling stops on submit; submitTx now passes context; adds TransactionSubmitted abort reason and improves logging. > > - **Bridge Controller**: > - Update `stopPollingForQuotes(reason?, context?)` to optionally publish `Unified SwapBridge Quotes Received` if quotes are still loading before abort; accept `AbortReason` and metric context. > - Add `AbortReason.TransactionSubmitted`; ignore it in fetch error early-returns; export `AbortReason`. > - Improve error log to include `eventName` in `trackUnifiedSwapBridgeEvent` catch. > - **Bridge Status Controller**: > - Change `submitTx` signature to accept `quotesReceivedContext` and call `BridgeController:stopPollingForQuotes(AbortReason.TransactionSubmitted, context)` instead of publishing `QuotesReceived` directly. > - Narrow `#trackUnifiedSwapBridgeEvent` generic to exclude `QuotesReceived`. > - **Tests/Changelogs**: > - Update snapshots and tests to reflect new stopPolling parameters and event routing. > - Document behavior changes in both packages’ CHANGELOGs. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 1506266. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent b2da989 commit 61d1bb0

File tree

9 files changed

+93
-35
lines changed

9 files changed

+93
-35
lines changed

‎packages/bridge-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+
### Changed
11+
12+
- Update `stopPollingForQuotes` to accept metrics context for the QuotesReceived event. If context is provided and quotes are still loading when the handler is called, the `Unified SwapBridge Quotes Received` is published before the poll is cancelled ([#7242](https://github.com/MetaMask/core/pull/7242))
13+
1014
## [63.1.0]
1115

1216
### Added

‎packages/bridge-controller/src/bridge-controller.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2673,7 +2673,7 @@ describe('BridgeController', function () {
26732673
expect(trackMetaMetricsFn).toHaveBeenCalledTimes(0);
26742674
expect(errorSpy).toHaveBeenCalledTimes(1);
26752675
expect(errorSpy).toHaveBeenCalledWith(
2676-
'Error tracking cross-chain swaps MetaMetrics event',
2676+
'Error tracking cross-chain swaps MetaMetrics event Unified SwapBridge Quotes Received',
26772677
new TypeError("Cannot read properties of undefined (reading 'type')"),
26782678
);
26792679
});

‎packages/bridge-controller/src/bridge-controller.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -496,8 +496,19 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
496496
);
497497
};
498498

499-
stopPollingForQuotes = (reason?: AbortReason) => {
499+
stopPollingForQuotes = (
500+
reason?: AbortReason,
501+
context?: RequiredEventContextFromClient[UnifiedSwapBridgeEventName.QuotesReceived],
502+
) => {
500503
this.stopAllPolling();
504+
// If polling is stopped before quotes finish loading, track QuotesReceived
505+
if (this.state.quotesLoadingStatus === RequestStatus.LOADING && context) {
506+
this.trackUnifiedSwapBridgeEvent(
507+
UnifiedSwapBridgeEventName.QuotesReceived,
508+
context,
509+
);
510+
}
511+
// Clears quotes list in state
501512
this.#abortController?.abort(reason);
502513
};
503514

@@ -627,6 +638,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
627638
AbortReason.ResetState,
628639
AbortReason.NewQuoteRequest,
629640
AbortReason.QuoteRequestUpdated,
641+
AbortReason.TransactionSubmitted,
630642
].includes(error as AbortReason)
631643
) {
632644
// Exit the function early to prevent other state updates
@@ -972,7 +984,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
972984
this.#trackMetaMetricsFn(eventName, combinedPropertiesForEvent);
973985
} catch (error) {
974986
console.error(
975-
'Error tracking cross-chain swaps MetaMetrics event',
987+
`Error tracking cross-chain swaps MetaMetrics event ${eventName}`,
976988
error,
977989
);
978990
}

‎packages/bridge-controller/src/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ export type {
5252
FeatureFlagsPlatformConfig,
5353
} from './types';
5454

55+
export { AbortReason } from './utils/metrics/constants';
56+
5557
export { StatusTypes } from './types';
5658

5759
export {

‎packages/bridge-controller/src/utils/metrics/constants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export enum AbortReason {
2626
NewQuoteRequest = 'New Quote Request',
2727
QuoteRequestUpdated = 'Quote Request Updated',
2828
ResetState = 'Reset controller state',
29+
TransactionSubmitted = 'Transaction submitted',
2930
}
3031

3132
/**

‎packages/bridge-status-controller/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2222
- Note, however, that the versions specified in the client's `package.json` always "win", and you are expected to keep them up to date so as not to break controller and service intercommunication.
2323
- Bump `@metamask/bridge-controller` from `^63.0.0` to `^63.1.0` ([#7238](https://github.com/MetaMask/core/pull/7238))
2424

25+
### Removed
26+
27+
- Remove direct QuotesReceived event publishing to avoid race conditions that can happen when clients navigate and reset state. Update `submitTx` to accept quotesReceivedContext (replace isLoading/warnings) and propagate context to the BridgeController through the `stopPollingForQuotes call ([#7242](https://github.com/MetaMask/core/pull/7242))
28+
2529
## [63.0.0]
2630

2731
### Changed

‎packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap‎

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,8 @@ exports[`BridgeStatusController submitTx: EVM bridge should call handleMobileHar
531531
Array [
532532
Array [
533533
"BridgeController:stopPollingForQuotes",
534+
"Transaction submitted",
535+
undefined,
534536
],
535537
Array [
536538
"AccountsController:getAccountByAddress",
@@ -764,6 +766,8 @@ exports[`BridgeStatusController submitTx: EVM bridge should delay after submitti
764766
Array [
765767
Array [
766768
"BridgeController:stopPollingForQuotes",
769+
"Transaction submitted",
770+
undefined,
767771
],
768772
Array [
769773
"AccountsController:getAccountByAddress",
@@ -997,6 +1001,8 @@ exports[`BridgeStatusController submitTx: EVM bridge should delay after submitti
9971001
Array [
9981002
Array [
9991003
"BridgeController:stopPollingForQuotes",
1004+
"Transaction submitted",
1005+
undefined,
10001006
],
10011007
Array [
10021008
"AccountsController:getAccountByAddress",
@@ -1081,7 +1087,7 @@ Array [
10811087
]
10821088
`;
10831089

1084-
exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and publish QuotesReceived event if quotes are still loading 1`] = `
1090+
exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and include quotesReceivedContext 1`] = `
10851091
Object {
10861092
"batchId": "batchId1",
10871093
"chainId": "0xa4b1",
@@ -1105,7 +1111,7 @@ Object {
11051111
}
11061112
`;
11071113

1108-
exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and publish QuotesReceived event if quotes are still loading 2`] = `
1114+
exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and include quotesReceivedContext 2`] = `
11091115
Object {
11101116
"account": "0xaccount1",
11111117
"approvalTxId": undefined,
@@ -1227,7 +1233,7 @@ Object {
12271233
}
12281234
`;
12291235

1230-
exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and publish QuotesReceived event if quotes are still loading 3`] = `
1236+
exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and include quotesReceivedContext 3`] = `
12311237
Array [
12321238
Array [
12331239
Object {
@@ -1245,7 +1251,7 @@ Array [
12451251
]
12461252
`;
12471253

1248-
exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and publish QuotesReceived event if quotes are still loading 4`] = `
1254+
exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and include quotesReceivedContext 4`] = `
12491255
Array [
12501256
Array [
12511257
Object {
@@ -1279,13 +1285,12 @@ Array [
12791285
]
12801286
`;
12811287

1282-
exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and publish QuotesReceived event if quotes are still loading 5`] = `
1288+
exports[`BridgeStatusController submitTx: EVM bridge should handle smart transactions and include quotesReceivedContext 5`] = `
12831289
Array [
12841290
Array [
1285-
"BridgeController:trackUnifiedSwapBridgeEvent",
1286-
"Unified SwapBridge Quotes Received",
1291+
"BridgeController:stopPollingForQuotes",
1292+
"Transaction submitted",
12871293
Object {
1288-
"action_type": "swapbridge-v1",
12891294
"best_quote_provider": "lifi_across",
12901295
"can_submit": true,
12911296
"gas_included": false,
@@ -1300,9 +1305,6 @@ Array [
13001305
],
13011306
},
13021307
],
1303-
Array [
1304-
"BridgeController:stopPollingForQuotes",
1305-
],
13061308
Array [
13071309
"AccountsController:getAccountByAddress",
13081310
"0xaccount1",
@@ -1496,6 +1498,8 @@ exports[`BridgeStatusController submitTx: EVM bridge should not call handleMobil
14961498
Array [
14971499
Array [
14981500
"BridgeController:stopPollingForQuotes",
1501+
"Transaction submitted",
1502+
undefined,
14991503
],
15001504
Array [
15011505
"AccountsController:getAccountByAddress",
@@ -1729,6 +1733,8 @@ exports[`BridgeStatusController submitTx: EVM bridge should not call handleMobil
17291733
Array [
17301734
Array [
17311735
"BridgeController:stopPollingForQuotes",
1736+
"Transaction submitted",
1737+
undefined,
17321738
],
17331739
Array [
17341740
"AccountsController:getAccountByAddress",
@@ -2077,6 +2083,8 @@ exports[`BridgeStatusController submitTx: EVM bridge should reset USDT allowance
20772083
Array [
20782084
Array [
20792085
"BridgeController:stopPollingForQuotes",
2086+
"Transaction submitted",
2087+
undefined,
20802088
],
20812089
Array [
20822090
"AccountsController:getAccountByAddress",
@@ -2349,6 +2357,8 @@ exports[`BridgeStatusController submitTx: EVM bridge should successfully submit
23492357
Array [
23502358
Array [
23512359
"BridgeController:stopPollingForQuotes",
2360+
"Transaction submitted",
2361+
undefined,
23522362
],
23532363
Array [
23542364
"AccountsController:getAccountByAddress",
@@ -2602,6 +2612,8 @@ exports[`BridgeStatusController submitTx: EVM bridge should successfully submit
26022612
Array [
26032613
Array [
26042614
"BridgeController:stopPollingForQuotes",
2615+
"Transaction submitted",
2616+
undefined,
26052617
],
26062618
Array [
26072619
"AccountsController:getAccountByAddress",
@@ -2676,6 +2688,8 @@ exports[`BridgeStatusController submitTx: EVM bridge should throw an error if ap
26762688
Array [
26772689
Array [
26782690
"BridgeController:stopPollingForQuotes",
2691+
"Transaction submitted",
2692+
undefined,
26792693
],
26802694
Array [
26812695
"AccountsController:getAccountByAddress",
@@ -2747,6 +2761,8 @@ exports[`BridgeStatusController submitTx: EVM bridge should throw an error if ap
27472761
Array [
27482762
Array [
27492763
"BridgeController:stopPollingForQuotes",
2764+
"Transaction submitted",
2765+
undefined,
27502766
],
27512767
Array [
27522768
"AccountsController:getAccountByAddress",
@@ -3015,6 +3031,8 @@ exports[`BridgeStatusController submitTx: EVM swap should handle smart transacti
30153031
Array [
30163032
Array [
30173033
"BridgeController:stopPollingForQuotes",
3034+
"Transaction submitted",
3035+
undefined,
30183036
],
30193037
Array [
30203038
"AccountsController:getAccountByAddress",
@@ -3105,6 +3123,8 @@ exports[`BridgeStatusController submitTx: EVM swap should successfully submit an
31053123
Array [
31063124
Array [
31073125
"BridgeController:stopPollingForQuotes",
3126+
"Transaction submitted",
3127+
undefined,
31083128
],
31093129
Array [
31103130
"AccountsController:getAccountByAddress",
@@ -3286,6 +3306,8 @@ exports[`BridgeStatusController submitTx: EVM swap should successfully submit an
32863306
Array [
32873307
Array [
32883308
"BridgeController:stopPollingForQuotes",
3309+
"Transaction submitted",
3310+
undefined,
32893311
],
32903312
Array [
32913313
"AccountsController:getAccountByAddress",
@@ -3335,6 +3357,8 @@ exports[`BridgeStatusController submitTx: Solana bridge should handle snap contr
33353357
Array [
33363358
Array [
33373359
"BridgeController:stopPollingForQuotes",
3360+
"Transaction submitted",
3361+
undefined,
33383362
],
33393363
Array [
33403364
"AccountsController:getAccountByAddress",
@@ -3412,6 +3436,8 @@ exports[`BridgeStatusController submitTx: Solana bridge should successfully subm
34123436
Array [
34133437
Array [
34143438
"BridgeController:stopPollingForQuotes",
3439+
"Transaction submitted",
3440+
undefined,
34153441
],
34163442
Array [
34173443
"AccountsController:getAccountByAddress",
@@ -3603,6 +3629,8 @@ exports[`BridgeStatusController submitTx: Solana bridge should throw error when
36033629
Array [
36043630
Array [
36053631
"BridgeController:stopPollingForQuotes",
3632+
"Transaction submitted",
3633+
undefined,
36063634
],
36073635
Array [
36083636
"AccountsController:getAccountByAddress",
@@ -3662,6 +3690,8 @@ exports[`BridgeStatusController submitTx: Solana swap should handle snap control
36623690
Array [
36633691
Array [
36643692
"BridgeController:stopPollingForQuotes",
3693+
"Transaction submitted",
3694+
undefined,
36653695
],
36663696
Array [
36673697
"AccountsController:getAccountByAddress",
@@ -3739,6 +3769,8 @@ exports[`BridgeStatusController submitTx: Solana swap should successfully submit
37393769
Array [
37403770
Array [
37413771
"BridgeController:stopPollingForQuotes",
3772+
"Transaction submitted",
3773+
undefined,
37423774
],
37433775
Array [
37443776
"AccountsController:getAccountByAddress",
@@ -3929,6 +3961,8 @@ exports[`BridgeStatusController submitTx: Solana swap should throw error when ac
39293961
Array [
39303962
Array [
39313963
"BridgeController:stopPollingForQuotes",
3964+
"Transaction submitted",
3965+
undefined,
39323966
],
39333967
Array [
39343968
"AccountsController:getAccountByAddress",
@@ -3941,6 +3975,8 @@ exports[`BridgeStatusController submitTx: Solana swap should throw error when sn
39413975
Array [
39423976
Array [
39433977
"BridgeController:stopPollingForQuotes",
3978+
"Transaction submitted",
3979+
undefined,
39443980
],
39453981
Array [
39463982
"AccountsController:getAccountByAddress",
@@ -4000,6 +4036,8 @@ exports[`BridgeStatusController submitTx: Tron swap with approval should handle
40004036
Array [
40014037
Array [
40024038
"BridgeController:stopPollingForQuotes",
4039+
"Transaction submitted",
4040+
undefined,
40034041
],
40044042
Array [
40054043
"AccountsController:getAccountByAddress",
@@ -4081,6 +4119,8 @@ exports[`BridgeStatusController submitTx: Tron swap with approval should success
40814119
Array [
40824120
Array [
40834121
"BridgeController:stopPollingForQuotes",
4122+
"Transaction submitted",
4123+
undefined,
40844124
],
40854125
Array [
40864126
"AccountsController:getAccountByAddress",
@@ -4296,6 +4336,8 @@ exports[`BridgeStatusController submitTx: Tron swap with approval should success
42964336
Array [
42974337
Array [
42984338
"BridgeController:stopPollingForQuotes",
4339+
"Transaction submitted",
4340+
undefined,
42994341
],
43004342
Array [
43014343
"AccountsController:getAccountByAddress",

‎packages/bridge-status-controller/src/bridge-status-controller.test.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
BridgeController,
1313
getNativeAssetForChainId,
1414
FeatureId,
15+
getQuotesReceivedProperties,
1516
} from '@metamask/bridge-controller';
1617
import { ChainId } from '@metamask/bridge-controller';
1718
import { ActionTypes, FeeType } from '@metamask/bridge-controller';
@@ -2559,8 +2560,7 @@ describe('BridgeStatusController', () => {
25592560
expect(mockMessengerCall.mock.calls).toMatchSnapshot();
25602561
});
25612562

2562-
it('should handle smart transactions and publish QuotesReceived event if quotes are still loading', async () => {
2563-
mockMessengerCall.mockImplementationOnce(jest.fn()); // track QuotesReceived event
2563+
it('should handle smart transactions and include quotesReceivedContext', async () => {
25642564
setupEventTrackingMocks(mockMessengerCall);
25652565
setupBridgeStxMocks(mockMessengerCall);
25662566
addTransactionBatchFn.mockResolvedValueOnce({
@@ -2574,8 +2574,7 @@ describe('BridgeStatusController', () => {
25742574
(quoteWithoutApproval.trade as TxData).from,
25752575
quoteWithoutApproval,
25762576
true,
2577-
true,
2578-
['low_return'],
2577+
getQuotesReceivedProperties(quoteWithoutApproval, ['low_return'], true),
25792578
);
25802579
controller.stopAllPolling();
25812580

0 commit comments

Comments
 (0)