feat(convert): warn on high slippage tolerance (>=5%), hard-cap input at 49% - #1146
Merged
Conversation
Post GHSA-jx33-xg6c-px39 (#1144) the Ambient route encodes slippage correctly, but a 100% tolerance still encodes a zero minimum output. Close the top-of-range footgun in the Convert page UI: - warning badge (frontrun risk) when entered tolerance exceeds 5% - >49% is invalid: input flagged, error badge shown, confirm disabled; enforced in logic because the HTML max attribute alone does not block typed values (max lowered from 100 to 49 as well) - boundary tests: 5 / 5.01 / 6 / 49 / 49.01 / 50 / 100 DEFAULT_SLIPPAGE_TOLERANCE (0.5) and the unrelated price-impact gate MAXIMUM_ALLOWED_SLIPPAGE (1.5) are unchanged. UI-only; SDK untouched.
Tyrone's call: warn at exactly 5% too, to catch 5 typed instead of 0.5. Boundary tests updated: 4.99 no warning, 5 warns; visually verified on the dev server (badge shows at exactly 5%).
|
✅ Deploy Preview for sovryn-dapp ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Tightens the slippage tolerance input on the Convert page:
5typed instead of0.5is flagged): amberErrorBadge— "High slippage tolerance: your transaction may be frontrun and result in an unfavorable trade". The swap remains allowed.invalid, a criticalErrorBadgeshows "Slippage tolerance must be 49% or less", and the Confirm button is disabled. Enforced in logic (newConvertPage.slippage.tspredicates), not just via the input'smaxattribute, because HTMLmaxdoes not block typed values. Themaxattribute is also lowered from 100 to 49.0.5(DEFAULT_SLIPPAGE_TOLERANCEunchanged). The error state replaces the warning rather than stacking.Why
Follow-up to #1144 / advisory GHSA-jx33-xg6c-px39. With the Ambient bps encoding fixed, the input still accepted up to 100%, and a 100% tolerance encodes a zero minimum output — this closes that remaining top-of-range footgun. UI-only: the SDK and the unrelated price-impact gate (
MAXIMUM_ALLOWED_SLIPPAGE = 1.5) are untouched.Notes for review
ConvertPage.slippage.ts) so the boundaries are unit-testable; a comment there disambiguates it fromMAXIMUM_ALLOWED_SLIPPAGEinConvertPage.constants.ts.ConvertPage.slippage.test.ts): 0.5 / 4.99 / empty → clean · 5 / 5.01 / 6 / 49 → warn, allowed · 49.01 / 50 / 100 → blocked. Full frontend suite green, eslint clean.enonly;eshas noconvertPagekeys and falls back toen.