fix(swapper): show the proper slippage tag on Portals trades - #12597
Conversation
Portals only report a distinct minOutputAmount on validated orders. Every rate, and any quote whose validated call fails (an unapproved ERC20 sell, or a pair where the requested tolerance is too tight), reports the post-slippage minimum in both amount fields - so the buffer recovered from those amounts was always zero, and MaxSlippage read a zero tolerance as a user-set one because bnOrZero coerced an unset preference to zero. Recover the pre-slippage output from the tolerance we requested when the amounts carry no buffer, advertise the wider of the requested and applied tolerances, and require a positive buy-asset match before adding the Portals fee back to the buy amount. Portals report amounts net of their fee, and take that fee in the sell asset on ERC20 sells. Also fixes Portals under-quoting by the full tolerance (2.5% by default), since the post-slippage minimum was being displayed as the expected output. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan includes up to 2 reviews per rolling hour; 1 remains after this review. 📝 WalkthroughWalkthroughPortals trade context now calculates fee-aware slippage from the output token and tolerance. Quote and rate flows pass the new parameters. The UI avoids marking trades as custom when user slippage is undefined. Comment guidance was updated. ChangesPortals slippage handling
Documentation rules
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change corrects Portals slippage labels and quote amounts without changing transaction construction or contract interactions. It is merge-ready after normal checks, with no actionable merge-blocking risk remaining. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Description
Portals trades showed a purple Custom slippage tag reading 0.00%, and — as a second effect of the same root cause — under-quoted by the full slippage tolerance.
Two bugs compounding:
1. Portals advertised a 0% tolerance.
/v2/portalonly returns a distinctminOutputAmounton validated orders. Unvalidated ones return the post-slippage minimum in both amount fields, so the buffer recovered from the amounts was structurally always zero. That hits every rate (getPortalsTradeRateis alwaysvalidate: false) and any quote whose validated call fails — an unapproved ERC20 sell, or a pair where the requested tolerance is too tight ("Insufficient buy: slippage too high").2.
MaxSlippageread 0% as user-set.bnOrZero(userSlippagePercentage)coerced an unset preference to0, so any quote reporting 0% matched and was labelled Custom. Portals was simply the swapper that always did.Because
buyAmountAfterFeeswas that same post-slippage minimum, Portals also quoted ~2.5% low on every rate — losing quote comparisons against swappers it routes through. In a $10 ETH→USDC test, Portals routed via 0x V2 and showed 9.696949 USDC while the Zrx quote beside it showed 9.943. It now shows 9.945589 — within 0.03% of Zrx, as it should be.Changes
min / (1 - requested)) when the amounts carry no buffermax(requested, applied)so receive-at-least never promises more than Portals guaranteebuyAmountBeforeFees, but only on a positive buy-asset match — their fee comes out of the sell asset on ERC20 sells, and the amounts they report are already net of itMaxSlippagenow requires a user preference to actually exist before showing CustomThe tag fix is swapper-agnostic — any swapper reporting 0% was mislabelled.
Issue (if applicable)
closes #11341
Linear: SS-5270
Risk
Medium. No on-chain transaction is constructed or modified here —
transactionDatastill passes through from Portals verbatim, andendpoints.tsnever reads the slippage field. The change is confined to quote/rate-time display values.Portals only for the amount and tolerance changes.
MaxSlippageis shared, so the tag change touches every swapper — with no preference set it now always reads Auto, which also fixes the auto-slippage swappers (Relay/Across/Debridge) that could report 0% and get mislabelled Custom.Second-order value shifts on Portals, both in the more-correct direction: the Mixpanel event payload (
buyAmountBeforeFees) and the "extra delta" denominator on the trade success screen.Untestable right now: Portals cross-chain 500s upstream on every pair, direction and endpoint I tried — including directly against
api.portals.fi, bypassing our proxy. So the cross-chain path could not be validated end to end. The display invariant holds by construction under either possible cross-chain semantics, but it deserves a re-check when Portals restore that endpoint. Worth its own ticket: cross-chain Portals quotes currently fail outright, not just display wrong.Testing
Engineering
Validated against the live Portals API — 30+ calls across 5 chains, both
validatemodes, four slippage values:outputAmountontominOutputAmount, alwaysquoted × (1 - requested): implied quoted output stayed constant to 0.0074% across a 0.1→2.5% sweep, and the recovered value matched a validated simulation within 0.03%outputAmountis net offeeAmount: their sum was byte-identical (1911487049) across threefeePercentagevalues back to backexpected × (1 - tolerance)lands on Portals' ownminOutputAmountin both market regimes — when they apply more slippage than requested and when they apply lessDegenerate payloads (missing minimum, both amounts zero, min above output, one-base-unit buffer) all fall back to the requested tolerance with no
NaN/Infinity. A checksummedoutputTokenproduces an identical buy amount to a lower-cased one — that comparison guards a 3,000,000× amount inflation if the casing ever drifts, so it is deliberately case-insensitive.Existing suites pass: 192 swapper tests, 16 web state tests.
Operations
Not behind a flag. On any Portals-routable pair (e.g. ETH→USDC on Ethereum or Base), open the trade details expander:
developshows Custom 0.00%.developand competitive rather than systematically last. Price impact drops by about the same amount.Follow-ups (not in this PR)
DEFAULT_PORTALS_SLIPPAGE_DECIMAL_PERCENTAGEis 2.5%, which is why the honest number reads high — their estimate endpoint suggests ~0.25% is realisticisAutoSlippageSupportedBySwapperusePriceImpacttruncates the percentage to 2dp before scaling it to fiat, so the-$X.XXon a quote card can be off bysellAmount × 0.0001— $0.05 on $500, $5 on $50k🤖 Generated with Claude Code
Summary by CodeRabbit