Skip to content

[FRE-1641] Improve Bad Price Error Handling #1070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 15, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/eleven-ants-knock.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@skip-go/widget": minor
---

fixed bug in displaying bad price warnings
62 changes: 50 additions & 12 deletions packages/widget/src/pages/ErrorPage/ErrorPageBadPriceWarning.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,22 @@ export const ErrorPageBadPriceWarning = ({
destAssetChainID,
} = route;

const swapDifferencePercentage = `${calculatePercentageChange(
usdAmountIn ?? 0,
usdAmountOut ?? 0,
true,
)}%`;
const hasUsdValues =
usdAmountIn &&
usdAmountOut &&
parseFloat(usdAmountIn) > 0 &&
parseFloat(usdAmountOut) > 0;

const swapDifferencePercentage = hasUsdValues
? `${calculatePercentageChange(usdAmountIn, usdAmountOut, true)}%`
: null;


const priceImpactPercentage = (() => {
const impactString = route.swapPriceImpactPercent;
if (!impactString) return null;
return `${parseFloat(impactString).toFixed(2)}%`;
})();

const sourceDetails = useGetAssetDetails({
assetDenom: sourceAssetDenom,
Expand All @@ -49,6 +60,38 @@ export const ErrorPageBadPriceWarning = ({
tokenAmount: amountOut,
});

let title = "Warning: Bad Trade";
let descriptionContent: React.ReactNode = null;

if (hasUsdValues && swapDifferencePercentage) {
title = `Warning: Bad trade (-${swapDifferencePercentage})`;
descriptionContent = (
<>
You will lose {swapDifferencePercentage} of your input value with this trade
<br />
Input: {sourceDetails?.amount} {sourceDetails?.symbol} ({usdAmountIn})
<br />
Estimated output: {destinationDetails?.amount} {destinationDetails?.symbol} ({usdAmountOut})
</>
);
} else if (priceImpactPercentage) {
title = `Warning: High Price Impact (${priceImpactPercentage})`;
descriptionContent = (
<>
Executing this trade is expected to impact the price by {priceImpactPercentage}. Please verify the amounts.
<br />
</>
);
} else {
descriptionContent = (
<>
This trade may result in a poor execution price. Please verify the amounts carefully.
<br />

</>
);
}

return (
<>
<SwapPageHeader
Expand All @@ -62,16 +105,11 @@ export const ErrorPageBadPriceWarning = ({
}}
/>
<ErrorPageContent
title={`Warning: Bad trade (-${swapDifferencePercentage})`}
title={title}
description={
<>
<SmallText color={theme.error.text} textAlign="center" textWrap="balance">
You will lose {swapDifferencePercentage} of your input value with this trade
<br />
Input: {sourceDetails?.amount} {sourceDetails?.symbol} ({usdAmountIn})
<br />
Estimated output: {destinationDetails?.amount} {destinationDetails?.symbol} (
{usdAmountOut})
{descriptionContent}
</SmallText>
<SmallTextButton
onClick={() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/widget/src/pages/SwapPage/SwapPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@
});
return;
}
if (route?.warning?.type === "BAD_PRICE_WARNING" && Number(priceChangePercentage ?? 0) < 0) {
if (route?.warning?.type === "BAD_PRICE_WARNING") {
track("error page: bad price warning", { route });
setError({
errorType: ErrorType.BadPriceWarning,
Expand Down Expand Up @@ -306,7 +306,7 @@
onClick={onClick}
/>
);
}, [

Check warning on line 309 in packages/widget/src/pages/SwapPage/SwapPage.tsx

View workflow job for this annotation

GitHub Actions / eslint

React Hook useMemo has an unnecessary dependency: 'priceChangePercentage'. Either exclude it or remove the dependency array
sourceAsset?.chainID,
sourceAsset?.amount,
sourceAccount?.address,
Expand Down