Skip to content

Commit 1578bd3

Browse files
authored
fix: fallback to recommendedQuote when selectedQuote is stale (#8154)
## Explanation Fixing a bug which causes the UI to get a nonexistent `activeQuote` on refresh. This updates the selector so it checks whether the `selectedQuote` exists in the sorted quotes list first, and falls back on the `recommendedQuote` otherwise <!-- 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 - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small, well-tested change to quote selection logic; primary risk is minor behavior change in how `activeQuote` is chosen when quotes refresh. > > **Overview** > Fixes stale quote selection by updating `selectActiveQuote` (used by `selectBridgeQuotes`) to only return `selectedQuote` if its `requestId` still exists in `sortedQuotes`, otherwise falling back to the `recommendedQuote`. > > Updates unit tests to cover both the valid-selected-quote case and the stale-selected-quote fallback, and adds a changelog entry plus minor JSDoc punctuation/clarifications in `QuoteMetadata`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 517219a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 12f91e4 commit 1578bd3

File tree

4 files changed

+61
-13
lines changed

4 files changed

+61
-13
lines changed

packages/bridge-controller/CHANGELOG.md

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

1212
- Add optional `active_ab_tests` property in Unified SwapBridge metrics event context and payload types, alongside existing `ab_tests`. ([#8152](https://github.com/MetaMask/core/pull/8152))
1313

14+
### Fixed
15+
16+
- Check whether `selectedQuote` exists in `selectBridgeQuotes.sortedQuotes` before returning it as the `activeQuote`. Fall back on the `recommendedQuote` if selectedQuote is stale ([#8154](https://github.com/MetaMask/core/pull/8154))
17+
1418
## [69.0.1]
1519

1620
### Changed

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

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,12 +1256,37 @@ describe('Bridge Selectors', () => {
12561256
});
12571257

12581258
it('should handle selected quote', () => {
1259+
const selectedQuote = {
1260+
...mockState.quotes[0],
1261+
quote: { ...mockState.quotes[0].quote, requestId: '123' },
1262+
} as never;
12591263
const result = selectBridgeQuotes(mockState, {
12601264
...mockClientParams,
1261-
selectedQuote: mockQuote as never,
1265+
selectedQuote,
12621266
});
12631267

1264-
expect(result.activeQuote).toStrictEqual(mockQuote);
1268+
expect(result.recommendedQuote).toStrictEqual(
1269+
expect.objectContaining(mockState.quotes[1]),
1270+
);
1271+
expect(result.activeQuote).toStrictEqual(
1272+
expect.objectContaining(selectedQuote),
1273+
);
1274+
});
1275+
1276+
it('should set recommendedQuote as activeQuote when selected quote is not found', () => {
1277+
const selectedQuote = {
1278+
...mockState.quotes[0],
1279+
quote: { ...mockState.quotes[0].quote, requestId: 'abc' },
1280+
} as never;
1281+
const result = selectBridgeQuotes(mockState, {
1282+
...mockClientParams,
1283+
selectedQuote,
1284+
});
1285+
1286+
expect(result.recommendedQuote).toStrictEqual(
1287+
expect.objectContaining(mockState.quotes[1]),
1288+
);
1289+
expect(result.activeQuote).toStrictEqual(result.recommendedQuote);
12651290
});
12661291

12671292
it('should handle quote refresh state', () => {

packages/bridge-controller/src/selectors.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -414,15 +414,15 @@ const selectBridgeQuotesWithMetadata = createBridgeSelector(
414414
minToTokenAmount,
415415
swapRate: calcSwapRate(sentAmount.amount, toTokenAmount.amount),
416416
/**
417-
This is the amount required to submit the transactions
418-
Includes the relayer fee or other native fees
417+
This is the amount required to submit all the transactions.
418+
Includes the relayer fee or other native fees.
419419
Should be used for balance checks and tx submission.
420420
*/
421421
totalNetworkFee: totalEstimatedNetworkFee,
422422
totalMaxNetworkFee,
423423
/**
424424
This contains gas fee estimates for the bridge transaction
425-
Does not include the relayer fee (if needed), just the gasLimit and effectiveGas returned by the bridge API
425+
Does not include the relayer fee (if needed), just the gasLimit and effectiveGas returned by the bridge API.
426426
Should only be used for display purposes.
427427
*/
428428
gasFee,
@@ -485,9 +485,13 @@ const selectRecommendedQuote = createBridgeSelector(
485485
const selectActiveQuote = createBridgeSelector(
486486
[
487487
selectRecommendedQuote,
488-
(_, { selectedQuote }: BridgeQuotesClientParams) => selectedQuote,
488+
selectSortedBridgeQuotes,
489+
(_, { selectedQuote }) => selectedQuote,
489490
],
490-
(recommendedQuote, selectedQuote) => selectedQuote ?? recommendedQuote,
491+
(recommendedQuote, sortedQuotes, selectedQuote) =>
492+
sortedQuotes.find(
493+
(quote) => quote.quote.requestId === selectedQuote?.quote.requestId,
494+
) ?? recommendedQuote,
491495
);
492496

493497
const selectIsQuoteGoingToRefresh = createBridgeSelector(

packages/bridge-controller/src/types.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,8 @@ export type ExchangeRate = { exchangeRate?: string; usdExchangeRate?: string };
114114
*/
115115
export type QuoteMetadata = {
116116
/**
117-
* If gas is included, this is the value of the src or dest token that was used to pay for the gas
117+
* If gas is included, this is the value of the src or dest token that was used to pay for the gas.
118+
* Show this value to indicate transaction fees for gasless quotes.
118119
*/
119120
includedTxFees?: TokenAmountValues | null;
120121
/**
@@ -125,6 +126,12 @@ export type QuoteMetadata = {
125126
* max is the max gas fee that will be used by the transaction.
126127
*/
127128
gasFee: Record<'effective' | 'total' | 'max', TokenAmountValues>;
129+
/**
130+
* The total network fee required to submit the trade and any approvals. This includes
131+
* the relayer fee or other native fees. Should be used for balance checks and tx submission.
132+
* Note: This is only accurate for non-gasless transactions. Use {@link QuoteMetadata.includedTxFees} to
133+
* get the total network fee for gasless transactions.
134+
*/
128135
totalNetworkFee: TokenAmountValues; // estimatedGasFees + relayerFees
129136
totalMaxNetworkFee: TokenAmountValues; // maxGasFees + relayerFees
130137
/**
@@ -136,16 +143,24 @@ export type QuoteMetadata = {
136143
*/
137144
minToTokenAmount: TokenAmountValues;
138145
/**
139-
* If gas is included: toTokenAmount
140-
* Otherwise: toTokenAmount - totalNetworkFee
146+
* If gas is included: {@link QuoteMetadata.toTokenAmount} - {@link QuoteMetadata.includedTxFees}.
147+
* Otherwise: {@link QuoteMetadata.toTokenAmount} - {@link QuoteMetadata.totalNetworkFee}.
141148
*/
142149
adjustedReturn: Omit<TokenAmountValues, 'amount'>;
143150
/**
144-
* The amount that the user will send, including fees
145-
* srcTokenAmount + metabridgeFee + txFee
151+
* The amount that the user will send, including fees that are paid in the src token
152+
* {@link Quote.srcTokenAmount} + {@link Quote.feeData[FeeType.METABRIDGE].amount} + {@link Quote.feeData[FeeType.TX_FEE].amount}
146153
*/
147154
sentAmount: TokenAmountValues;
148-
swapRate: string; // destTokenAmount / sentAmount
155+
/**
156+
* The swap rate is the amount that the user will receive per amount sent. Accounts for fees paid in the src or dest token.
157+
* This is calculated as {@link QuoteMetadata.toTokenAmount} / {@link QuoteMetadata.sentAmount}.
158+
*/
159+
swapRate: string;
160+
/**
161+
* The cost of the trade, which is the difference between the amount sent and the adjusted return.
162+
* This is calculated as {@link QuoteMetadata.sentAmount} - {@link QuoteMetadata.adjustedReturn}.
163+
*/
149164
cost: Omit<TokenAmountValues, 'amount'>; // sentAmount - adjustedReturn
150165
};
151166

0 commit comments

Comments
 (0)