Skip to content

Fix MultiSwapper quote backcompat#161

Closed
liobrasil wants to merge 8 commits into
v0from
lionel/fix-multiswapper-alex-backcompat
Closed

Fix MultiSwapper quote backcompat#161
liobrasil wants to merge 8 commits into
v0from
lionel/fix-multiswapper-alex-backcompat

Conversation

@liobrasil
Copy link
Copy Markdown
Contributor

@liobrasil liobrasil commented Mar 28, 2026

Summary

This finishes the remaining MultiSwapper follow-up work after #158.

It keeps the improved route-selection behavior from #158, restores the outward quote semantics expected by existing callers, fixes the remaining partial-route tie case, and aligns the public Swapper documentation with the actual quote model used by the implementations.

Remaining Issues From #158

  1. swap(nil, ...) and swapBack(nil, ...) could become inconsistent with quoteOut(...).
    In #158, quoteOut(forProvided) could return an inner route's smaller executable inAmount, while MultiSwapper._swap(...) still forwarded the full vault balance when it fell back to quoteOut(from.balance).
    That broke strict or capacity-limited inner swappers that enforce input <= quote.inAmount.

    Example before this PR:

    • input vault balance = 10 TokenA
    • inner route is capped at 4 TokenB
    • raw inner quoteOut(10) = { inAmount: 4, outAmount: 4 }
    • _swap(...) still forwards the full 10 TokenA vault
    • strict inner swapper rejects the call because 10 > quote.inAmount (4)

    Example after this PR:

    • outward MultiSwapper.quoteOut(10) = { inAmount: 10, outAmount: 4 }
    • route selection still chooses the capped route
    • swap(nil, ...) and swapBack(nil, ...) remain consistent with the full vault balance being swapped
    • strict inner swappers no longer revert on the fallback path
  2. SwapSource.withdrawAvailable(maxAmount) could return more than maxAmount.
    quoteIn(forDesired) started exposing the chosen route's raw outAmount, which can overshoot the requested amount due to rounding or quote refinement. SwapSource then forwarded that quote directly into swap(...).

    Example before this PR:

    • caller requests withdrawAvailable(maxAmount: 10)
    • selected route returns quoteIn(10) = { inAmount: 10, outAmount: 11 }
    • SwapSource forwards that quote directly into swap(...)
    • caller receives 11, even though maxAmount was 10

    Example after this PR:

    • outward MultiSwapper.quoteIn(10) is clamped to { inAmount: 10, outAmount: 10 }
    • SwapSource.withdrawAvailable(maxAmount: 10) cannot return more than 10
  3. quoteIn(...) still had a partial-route tie regression.
    When no route could fully satisfy forDesired, #158 preferred the partial route with the highest outAmount, but if two partial routes had the same outAmount it kept the first one seen even if another route required less input.

    Example before this PR:

    • Route A: { inAmount: 10, outAmount: 5 }
    • Route B: { inAmount: 5, outAmount: 5 }
    • request: quoteIn(forDesired: 10)
    • both routes are partial and tie on outAmount = 5
    • #158 keeps Route A if it appears first, even though Route B is strictly better

    Example after this PR:

    • same two routes
    • quoteIn(forDesired: 10) still falls back to the best partial route
    • tie on outAmount is broken by lower inAmount
    • Route B is chosen
  4. The public Swapper interface comments were still not aligned with implementation behavior.
    The reverse flag flips the quoted token types, but the quote amounts still describe quote.inType -> quote.outType in the chosen direction.

    Example before this PR:

    • docs implied reverse=true should be interpreted as “provide forDesired output tokens and receive quote.inAmount input tokens”
    • implementations instead return quotes where quote.inAmount and quote.outAmount still describe the swap in the quoted direction, with only the token types flipped

    Example after this PR:

    • docs now state that quote.inAmount always refers to quote.inType
    • docs now state that quote.outAmount always refers to quote.outType
    • reverse is documented as changing the quoted token direction, not redefining what the quote fields mean

Fix

  • Reintroduce a shared MultiSwapper._estimate(...) helper that selects the winning route while preserving the improved #158 route-selection rules.
  • For quoteIn(...), keep the selected route's inAmount, but clamp the outward outAmount to forDesired.
  • For quoteOut(...), keep the selected route's outAmount, but preserve inAmount = forProvided in the outward quote.
  • Keep quoteOut(...) preferring the highest outAmount, with lower inAmount as a tiebreak when routes tie on output.
  • Keep quoteIn(...) preferring full coverage over partial coverage, and prefer lower inAmount when partial routes tie on outAmount.
  • Update the Swapper interface documentation so the quote semantics and reverse behavior match the current implementations.

Tests

  • flow test ./cadence/tests/SwapConnectorsMultiSwapper_test.cdc

Added or expanded regression coverage for:

  • reverse-direction quoteIn(...) and quoteOut(...)
  • quoteIn(...) partial-route tie breaking on lower inAmount
  • quoteOut(...) preserving the caller-provided input amount on capped routes
  • swap(nil, ...) succeeding against a strict capped inner swapper
  • SwapSource.withdrawAvailable(maxAmount) never returning more than maxAmount
  • existing quoteOut(...) route-selection behavior, including capped and partial-consumption routes

@liobrasil liobrasil marked this pull request as ready for review March 28, 2026 01:26
@liobrasil liobrasil force-pushed the lionel/fix-multiswapper-alex-backcompat branch from 9237a3e to 9a266df Compare March 30, 2026 17:08
@liobrasil liobrasil force-pushed the lionel/fix-multiswapper-alex-backcompat branch from 618eefc to f824ade Compare March 30, 2026 18:56
Base automatically changed from nialexsan/fix-multi-swapper to v0 March 31, 2026 02:14
@liobrasil liobrasil closed this Apr 1, 2026
@liobrasil liobrasil deleted the lionel/fix-multiswapper-alex-backcompat branch April 1, 2026 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants