fix: add fallbacks for swap quote sorting#25928
Conversation
| + return (0, lodash_1.orderBy)(quotesWithMetadata, ({ cost }) => Number(cost.valueInCurrency), 'asc'); | ||
| + } | ||
| + if (quotesWithMetadata.every((quote) => quote.quote.priceData?.priceImpact)) { | ||
| + return (0, lodash_1.orderBy)(quotesWithMetadata, ({ quote }) => Number(quote.priceData?.priceImpact), 'asc'); |
There was a problem hiding this comment.
Price impact ascending sort recommends worst quote first
High Severity
The priceImpact fallback sorts ascending ('asc'), but mock data shows priceImpact values are negative (e.g., "-0.001991570073761955"), where less negative means a better quote. Ascending sort places the most negative (worst) value first, so selectRecommendedQuote (which picks quotes[0]) would recommend the quote with the worst price impact. The sort direction likely needs to be 'desc' to match the destTokenAmount fallback pattern, which correctly uses descending to put the best quote first.
Additional Locations (1)
There was a problem hiding this comment.
A negative priceImpact indicates a better quote so ascending is the correct order. For example: a quote with -.5 priceImpact is better than one with .1 priceImpact
0b16e85
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection: Performance Test Selection: |
|
Description
Uses the priceImpact or destTokenAmount to sort bridge quotes if fiat cost is not available
Changelog
CHANGELOG entry: fix: fall back to priceImpact or destTokenAmount for swap quote sorting
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Changes quote ordering logic and timing of exchange-rate fetching, which can affect which quote is recommended and potentially alter user-facing swap/bridge outcomes when data is missing.
Overview
Improves
selectSortedBridgeQuotessorting behavior in the patched@metamask/bridge-controllerso the default sort no longer treats missing fiat cost as0; it now sorts by fiat cost when available for all quotes, otherwise falls back topriceImpact, and finallydestTokenAmount.Moves
fetchAssetExchangeRatesfrom the quote-request update path to thefetchBridgeQuotespath (fire-and-forget with logging), aligning rate fetching with quote retrieval.Written by Cursor Bugbot for commit b977cf1. This will update automatically on new commits. Configure here.