Skip to content

Commit 5f76d25

Browse files
authored
chore: refactor and unify low return warning (#29918)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Users are experiencing issues with receiving significantly lower amounts of destination tokens than expected during swaps. This is due to excessive price differentials between the estimated swap amount and the actual return. Currently, the logic for triggering a low return warning differs between swaps and bridges, leading to inconsistencies and user confusion. This PR brings swap to parity with bridge by adding a warning when a swap results in a return below 80% of the value of the source tokens. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29918?quickstart=1) ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to swaps 2. Select tokens and input amount to force warning (eg. 0.01 ETH to ENA) 3. See warning ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** ![Screenshot 2025-01-27 at 19 55 48](https://github.com/user-attachments/assets/828a13ac-10bc-4624-b538-ca1e05e36fff) <!-- [screenshots/recordings] --> ### **After** ![Screenshot 2025-01-27 at 19 55 18](https://github.com/user-attachments/assets/5109052f-25c0-4e32-bc4a-4ab5a7e9d526) <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
1 parent 0b202e3 commit 5f76d25

File tree

11 files changed

+173
-19
lines changed

11 files changed

+173
-19
lines changed

shared/constants/bridge.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export const ETH_USDT_ADDRESS = '0xdac17f958d2ee523a2206206994597c13d831ec7';
3333
export const METABRIDGE_ETHEREUM_ADDRESS =
3434
'0x0439e60F02a8900a951603950d8D4527f400C3f1';
3535
export const BRIDGE_QUOTE_MAX_ETA_SECONDS = 60 * 60; // 1 hour
36-
export const BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE = 0.5; // if a quote returns in x times less return than the best quote, ignore it
36+
export const BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE = 0.35; // if a quote returns in x times less return than the best quote, ignore it
3737

3838
export const BRIDGE_PREFERRED_GAS_ESTIMATE = 'high';
3939
export const BRIDGE_DEFAULT_SLIPPAGE = 0.5;

shared/constants/swaps.ts

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export const SLIPPAGE_LOW_ERROR = 'slippage-low';
2424
export const SLIPPAGE_NEGATIVE_ERROR = 'slippage-negative';
2525

2626
export const MAX_ALLOWED_SLIPPAGE = 15;
27+
export const SWAPS_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE = 0.35;
2728

2829
// An address that the metaswap-api recognizes as the default token for the current network,
2930
// in place of the token address that ERC-20 tokens have

ui/ducks/bridge/selectors.test.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -1183,7 +1183,7 @@ describe('Bridge selectors', () => {
11831183
).toStrictEqual(false);
11841184
});
11851185

1186-
it('should return isEstimatedReturnLow=true return value is 50% less than sent funds', () => {
1186+
it('should return isEstimatedReturnLow=true return value is less than 65% of sent funds', () => {
11871187
const state = createBridgeMockStore({
11881188
featureFlagOverrides: {
11891189
extensionConfig: {
@@ -1239,7 +1239,7 @@ describe('Bridge selectors', () => {
12391239
expect(result.isEstimatedReturnLow).toStrictEqual(true);
12401240
});
12411241

1242-
it('should return isEstimatedReturnLow=false when return value is more than 50% of sent funds', () => {
1242+
it('should return isEstimatedReturnLow=false when return value is more than 65% of sent funds', () => {
12431243
const state = createBridgeMockStore({
12441244
featureFlagOverrides: {
12451245
extensionConfig: {
@@ -1254,7 +1254,7 @@ describe('Bridge selectors', () => {
12541254
fromToken: { address: zeroAddress(), symbol: 'ETH' },
12551255
toToken: { address: zeroAddress(), symbol: 'TEST' },
12561256
fromTokenExchangeRate: 2524.25,
1257-
toTokenExchangeRate: 0.63,
1257+
toTokenExchangeRate: 0.95,
12581258
fromTokenInputValue: 1,
12591259
},
12601260
bridgeStateOverrides: {
@@ -1292,7 +1292,7 @@ describe('Bridge selectors', () => {
12921292
expect(
12931293
getBridgeQuotes(state as never).activeQuote?.adjustedReturn
12941294
.valueInCurrency,
1295-
).toStrictEqual(new BigNumber('12.87194306627291988'));
1295+
).toStrictEqual(new BigNumber('20.69239170627291988'));
12961296
expect(result.isEstimatedReturnLow).toStrictEqual(false);
12971297
});
12981298

ui/ducks/bridge/selectors.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ export const getValidationErrors = createDeepEqualSelector(
547547
fromTokenInputValue
548548
? activeQuote.adjustedReturn.valueInCurrency.lt(
549549
new BigNumber(
550-
BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE,
550+
1 - BRIDGE_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE,
551551
).times(activeQuote.sentAmount.valueInCurrency),
552552
)
553553
: false,

ui/ducks/swaps/swaps.js

+54
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ import {
8686
SWAPS_FETCH_ORDER_CONFLICT,
8787
ALLOWED_SMART_TRANSACTIONS_CHAIN_IDS,
8888
Slippage,
89+
SWAPS_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE,
8990
} from '../../../shared/constants/swaps';
9091
import {
9192
IN_PROGRESS_TRANSACTION_STATUSES,
@@ -100,6 +101,7 @@ import {
100101
import { EtherDenomination } from '../../../shared/constants/common';
101102
import { Numeric } from '../../../shared/modules/Numeric';
102103
import { calculateMaxGasLimit } from '../../../shared/lib/swaps-utils';
104+
import { useTokenFiatAmount } from '../../hooks/useTokenFiatAmount';
103105

104106
const debugLog = createProjectLogger('swaps');
105107

@@ -1435,3 +1437,55 @@ export function cancelSwapsSmartTransaction(uuid) {
14351437
}
14361438
};
14371439
}
1440+
1441+
export const getIsEstimatedReturnLow = ({ usedQuote, rawNetworkFees }) => {
1442+
const sourceTokenAmount = calcTokenAmount(
1443+
usedQuote?.sourceAmount,
1444+
usedQuote?.sourceTokenInfo?.decimals,
1445+
);
1446+
// Disabled because it's not a hook
1447+
// eslint-disable-next-line react-hooks/rules-of-hooks
1448+
const sourceTokenFiatAmount = useTokenFiatAmount(
1449+
usedQuote?.sourceTokenInfo?.address,
1450+
sourceTokenAmount || 0,
1451+
usedQuote?.sourceTokenInfo?.symbol,
1452+
{
1453+
showFiat: true,
1454+
},
1455+
true,
1456+
null,
1457+
false,
1458+
);
1459+
const destinationTokenAmount = calcTokenAmount(
1460+
usedQuote?.destinationAmount,
1461+
usedQuote?.destinationTokenInfo?.decimals,
1462+
);
1463+
// Disabled because it's not a hook
1464+
// eslint-disable-next-line react-hooks/rules-of-hooks
1465+
const destinationTokenFiatAmount = useTokenFiatAmount(
1466+
usedQuote?.destinationTokenInfo?.address,
1467+
destinationTokenAmount || 0,
1468+
usedQuote?.destinationTokenInfo?.symbol,
1469+
{
1470+
showFiat: true,
1471+
},
1472+
true,
1473+
null,
1474+
false,
1475+
);
1476+
const adjustedReturnValue =
1477+
destinationTokenFiatAmount && rawNetworkFees
1478+
? new BigNumber(destinationTokenFiatAmount).minus(
1479+
new BigNumber(rawNetworkFees),
1480+
)
1481+
: null;
1482+
const isEstimatedReturnLow =
1483+
sourceTokenFiatAmount && adjustedReturnValue
1484+
? adjustedReturnValue.lt(
1485+
new BigNumber(sourceTokenFiatAmount).times(
1486+
1 - SWAPS_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE,
1487+
),
1488+
)
1489+
: false;
1490+
return isEstimatedReturnLow;
1491+
};

ui/helpers/utils/token-util.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ export function getTokenFiatAmount(
254254

255255
currentTokenInFiat = currentTokenInFiat.round(2).toString();
256256
let result;
257-
if (hideCurrencySymbol) {
257+
if (hideCurrencySymbol && formatted) {
258258
result = formatCurrency(currentTokenInFiat, currentCurrency);
259259
} else if (formatted) {
260260
result = `${formatCurrency(

ui/hooks/useTokenFiatAmount.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import { isEqualCaseInsensitive } from '../../shared/modules/string-utils';
2828
* @param {boolean} hideCurrencySymbol - Indicates whether the returned formatted amount should include the trailing currency symbol
2929
* @returns {string} The formatted token amount in the user's chosen fiat currency
3030
* @param {string} [chainId] - The chain id
31+
* @param {boolean} formatted - Whether the return value should be formatted or not
3132
*/
3233
export function useTokenFiatAmount(
3334
tokenAddress,
@@ -36,6 +37,7 @@ export function useTokenFiatAmount(
3637
overrides = {},
3738
hideCurrencySymbol,
3839
chainId = null,
40+
formatted = true,
3941
) {
4042
const allMarketData = useSelector(getMarketData);
4143

@@ -91,7 +93,7 @@ export function useTokenFiatAmount(
9193
currentCurrency,
9294
tokenAmount,
9395
tokenSymbol,
94-
true,
96+
formatted,
9597
hideCurrencySymbol,
9698
),
9799
[

ui/pages/swaps/prepare-swap-page/prepare-swap-page.js

+28-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
TextVariant,
3030
BLOCK_SIZES,
3131
FontWeight,
32+
TextAlign,
3233
} from '../../../helpers/constants/design-system';
3334
import {
3435
fetchQuotesAndSetQuoteState,
@@ -95,6 +96,7 @@ import {
9596
QUOTES_NOT_AVAILABLE_ERROR,
9697
QUOTES_EXPIRED_ERROR,
9798
MAX_ALLOWED_SLIPPAGE,
99+
SWAPS_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE,
98100
} from '../../../../shared/constants/swaps';
99101
import {
100102
CHAINID_DEFAULT_BLOCK_EXPLORER_URL_MAP,
@@ -132,6 +134,7 @@ import {
132134
Text,
133135
TextField,
134136
TextFieldSize,
137+
BannerAlertSeverity,
135138
} from '../../../components/component-library';
136139
import { ModalContent } from '../../../components/component-library/modal-content/deprecated';
137140
import { ModalHeader } from '../../../components/component-library/modal-header/deprecated';
@@ -178,6 +181,8 @@ export default function PrepareSwapPage({
178181
const [quoteCount, updateQuoteCount] = useState(0);
179182
const [prefetchingQuotes, setPrefetchingQuotes] = useState(false);
180183
const [rotateSwitchTokens, setRotateSwitchTokens] = useState(false);
184+
const [isLowReturnBannerOpen, setIsLowReturnBannerOpen] = useState(true);
185+
const [isEstimatedReturnLow, setIsEstimatedReturnLow] = useState(false);
181186

182187
const isBridgeSupported = useSelector(getIsBridgeEnabled);
183188
const isFeatureFlagLoaded = useSelector(getIsFeatureFlagLoaded);
@@ -358,6 +363,8 @@ export default function PrepareSwapPage({
358363
) {
359364
dispatch(setSwapsErrorKey(QUOTES_NOT_AVAILABLE_ERROR));
360365
}
366+
// Resets the banner visibility when the estimated return is low
367+
setIsLowReturnBannerOpen(true);
361368
};
362369

363370
// The below logic simulates a sequential loading of the aggregator quotes, even though we are fetching them all with a single call.
@@ -396,6 +403,11 @@ export default function PrepareSwapPage({
396403
prefetchingQuotes,
397404
]);
398405

406+
useEffect(() => {
407+
// Reopens the low return banner if a new quote is selected
408+
setIsLowReturnBannerOpen(true);
409+
}, [usedQuote]);
410+
399411
const onFromSelect = (token) => {
400412
if (
401413
token?.address &&
@@ -1165,6 +1177,18 @@ export default function PrepareSwapPage({
11651177
</BannerAlert>
11661178
</Box>
11671179
)}
1180+
{isEstimatedReturnLow && isLowReturnBannerOpen && (
1181+
<BannerAlert
1182+
marginTop={3}
1183+
title={t('lowEstimatedReturnTooltipTitle')}
1184+
severity={BannerAlertSeverity.Warning}
1185+
description={t('lowEstimatedReturnTooltipMessage', [
1186+
SWAPS_QUOTE_MAX_RETURN_DIFFERENCE_PERCENTAGE * 100,
1187+
])}
1188+
textAlign={TextAlign.Left}
1189+
onClose={() => setIsLowReturnBannerOpen(false)}
1190+
/>
1191+
)}
11681192
{swapsErrorKey && (
11691193
<Box display={DISPLAY.FLEX} marginTop={2}>
11701194
<SwapsBannerAlert
@@ -1193,7 +1217,10 @@ export default function PrepareSwapPage({
11931217
/>
11941218
)}
11951219
{showReviewQuote && (
1196-
<ReviewQuote setReceiveToAmount={setReceiveToAmount} />
1220+
<ReviewQuote
1221+
setReceiveToAmount={setReceiveToAmount}
1222+
setIsEstimatedReturnLow={setIsEstimatedReturnLow}
1223+
/>
11971224
)}
11981225
</div>
11991226
{!areQuotesPresent && (

ui/pages/swaps/prepare-swap-page/review-quote.js

+22-10
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
fetchSwapsSmartTransactionFees,
4545
getSmartTransactionFees,
4646
getCurrentSmartTransactionsEnabled,
47+
getIsEstimatedReturnLow,
4748
} from '../../../ducks/swaps/swaps';
4849
import { getCurrentChainId } from '../../../../shared/modules/selectors/networks';
4950
import {
@@ -181,7 +182,10 @@ ViewAllQuotesLink.propTypes = {
181182
t: PropTypes.func.isRequired,
182183
};
183184

184-
export default function ReviewQuote({ setReceiveToAmount }) {
185+
export default function ReviewQuote({
186+
setReceiveToAmount,
187+
setIsEstimatedReturnLow,
188+
}) {
185189
const history = useHistory();
186190
const dispatch = useDispatch();
187191
const t = useContext(I18nContext);
@@ -421,7 +425,7 @@ export default function ReviewQuote({ setReceiveToAmount }) {
421425
sourceTokenValue,
422426
} = renderableDataForUsedQuote;
423427

424-
let { feeInFiat, feeInEth, rawEthFee, feeInUsd } =
428+
let { feeInFiat, feeInEth, rawEthFee, feeInUsd, rawNetworkFees } =
425429
getRenderableNetworkFeesForQuote({
426430
tradeGas: usedGasLimit,
427431
approveGas,
@@ -472,14 +476,15 @@ export default function ReviewQuote({ setReceiveToAmount }) {
472476
smartTransactionFees?.tradeTxFees.maxFeeEstimate +
473477
(smartTransactionFees?.approvalTxFees?.maxFeeEstimate || 0);
474478

475-
({ feeInFiat, feeInEth, rawEthFee, feeInUsd } = getFeeForSmartTransaction({
476-
chainId,
477-
currentCurrency,
478-
conversionRate,
479-
USDConversionRate,
480-
nativeCurrencySymbol,
481-
feeInWeiDec: stxEstimatedFeeInWeiDec,
482-
}));
479+
({ feeInFiat, feeInEth, rawEthFee, feeInUsd, rawNetworkFees } =
480+
getFeeForSmartTransaction({
481+
chainId,
482+
currentCurrency,
483+
conversionRate,
484+
USDConversionRate,
485+
nativeCurrencySymbol,
486+
feeInWeiDec: stxEstimatedFeeInWeiDec,
487+
}));
483488
additionalTrackingParams.stx_fee_in_usd = Number(feeInUsd);
484489
additionalTrackingParams.stx_fee_in_eth = Number(rawEthFee);
485490
additionalTrackingParams.estimated_gas =
@@ -1122,6 +1127,12 @@ export default function ReviewQuote({ setReceiveToAmount }) {
11221127
currentCurrency,
11231128
]);
11241129

1130+
const isEstimatedReturnLow = getIsEstimatedReturnLow({
1131+
usedQuote,
1132+
rawNetworkFees,
1133+
});
1134+
setIsEstimatedReturnLow(isEstimatedReturnLow);
1135+
11251136
return (
11261137
<div className="review-quote">
11271138
<div className="review-quote__content">
@@ -1489,4 +1500,5 @@ export default function ReviewQuote({ setReceiveToAmount }) {
14891500

14901501
ReviewQuote.propTypes = {
14911502
setReceiveToAmount: PropTypes.func.isRequired,
1503+
setIsEstimatedReturnLow: PropTypes.func.isRequired,
14921504
};

ui/pages/swaps/prepare-swap-page/review-quote.test.js

+57
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ const middleware = [thunk];
3434
const createProps = (customProps = {}) => {
3535
return {
3636
setReceiveToAmount: jest.fn(),
37+
setIsEstimatedReturnLow: jest.fn(),
3738
...customProps,
3839
};
3940
};
@@ -143,6 +144,62 @@ describe('ReviewQuote', () => {
143144
expect(getByText('Swap')).toBeInTheDocument();
144145
});
145146

147+
it('should call setIsEstimatedReturnLow(true) when return value is less than 65% of sent funds', async () => {
148+
const setReceiveToAmountMock = jest.fn();
149+
const setIsEstimatedReturnLowMock = jest.fn();
150+
const props = {
151+
setReceiveToAmount: setReceiveToAmountMock,
152+
setIsEstimatedReturnLow: setIsEstimatedReturnLowMock,
153+
};
154+
155+
const state = createSwapsMockStore();
156+
157+
// Set up market data for price calculations
158+
state.metamask.marketData = {
159+
[CHAIN_IDS.MAINNET]: {
160+
'0x6B175474E89094C44Da98b954EedeAC495271d0F': {
161+
// DAI
162+
price: 100,
163+
decimal: 18,
164+
},
165+
'0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48': {
166+
// USDC
167+
price: 60,
168+
decimal: 6,
169+
},
170+
},
171+
};
172+
173+
// Set up the quotes with amounts that will result in less than 65% return
174+
state.metamask.swapsState.quotes = {
175+
TEST_AGG_2: {
176+
sourceAmount: '1000000000000000000', // 1 DAI (18 decimals)
177+
destinationAmount: '1000000', // 1 USDC (6 decimals)
178+
trade: {
179+
value: '0x0',
180+
},
181+
sourceTokenInfo: {
182+
symbol: 'DAI',
183+
decimals: 18,
184+
address: '0x6B175474E89094C44Da98b954EedeAC495271d0F',
185+
},
186+
destinationTokenInfo: {
187+
symbol: 'USDC',
188+
decimals: 6,
189+
address: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48',
190+
},
191+
},
192+
};
193+
194+
const store = configureMockStore(middleware)(state);
195+
196+
await act(async () => {
197+
renderWithProvider(<ReviewQuote {...props} />, store);
198+
});
199+
200+
expect(setIsEstimatedReturnLowMock).toHaveBeenCalledWith(true);
201+
});
202+
146203
describe('uses gas fee estimates from transaction controller if 1559 and smart disabled', () => {
147204
let smartDisabled1559State;
148205

0 commit comments

Comments
 (0)