Nialexsan/port multiswapper fixes

cadence/contracts/connectors/SwapConnectors.cdc

@@ -146,26 +146,82 @@ access(all) contract SwapConnectors {
         access(all) view fun outType(): Type  {
             return self.outVault
         }
-        /// The estimated amount required to provide a Vault with the desired output balance
+        /// The estimated amount required to provide a Vault with the desired output balance.
+        ///
+        /// Selection policy (two-tier):
+        ///   1. Full-coverage routes (outAmount >= forDesired): prefer minimum inAmount
+        ///   2. Partial-coverage routes (outAmount < forDesired, pool capped): prefer maximum outAmount
+        /// Full-coverage always wins over partial-coverage regardless of inAmount.
         access(all) fun quoteIn(forDesired: UFix64, reverse: Bool): {DeFiActions.Quote} {
-            let estimate = self._estimate(amount: forDesired, out: false, reverse: reverse)
+            var hasFull = false
+            var bestIdx = 0
+            var bestInAmount = UFix64.max
+            var bestOutAmount = 0.0
+            var partialIdx = 0
+            var partialInAmount = 0.0
+            var partialOutAmount = 0.0
+
+            for i in InclusiveRange(0, self.swappers.length - 1) {
+                let quote = (&self.swappers[i] as &{DeFiActions.Swapper})
+                    .quoteIn(forDesired: forDesired, reverse: reverse)
+                if quote.inAmount == 0.0 || quote.outAmount == 0.0 { continue }
+
+                if quote.outAmount >= forDesired {
+                    // full coverage — prefer minimum inAmount
+                    if !hasFull || quote.inAmount < bestInAmount {
+                        hasFull = true
+                        bestIdx = i
+                        bestInAmount = quote.inAmount
+                        bestOutAmount = quote.outAmount
+                    }
+                } else if !hasFull {
+                    // partial coverage — prefer maximum outAmount (only when no full route found)
+                    if quote.outAmount > partialOutAmount {
+                        partialIdx = i
+                        partialInAmount = quote.inAmount
+                        partialOutAmount = quote.outAmount
+                    }
+                }
+            }
+
+            let idx = hasFull ? bestIdx : partialIdx
+            let inAmt = hasFull ? bestInAmount : partialInAmount
+            let outAmt = hasFull ? bestOutAmount : partialOutAmount
             return MultiSwapperQuote(
                 inType: reverse ? self.outType() : self.inType(),
                 outType: reverse ? self.inType() : self.outType(),
-                inAmount: estimate[1],
-                outAmount: forDesired,
-                swapperIndex: Int(estimate[0])
+                inAmount: inAmt,
+                outAmount: outAmt,
+                swapperIndex: idx
             )
         }
-        /// The estimated amount delivered out for a provided input balance
+        /// The estimated amount delivered out for a provided input balance.
+        ///
+        /// Selection policy: prefer maximum outAmount across all routes.
         access(all) fun quoteOut(forProvided: UFix64, reverse: Bool): {DeFiActions.Quote} {
-            let estimate = self._estimate(amount: forProvided, out: true, reverse: reverse)
+            var hasBest = false
+            var bestIdx = 0
+            var bestInAmount = forProvided
+            var bestOutAmount = 0.0
+
+            for i in InclusiveRange(0, self.swappers.length - 1) {
+                let quote = (&self.swappers[i] as &{DeFiActions.Swapper})
+                    .quoteOut(forProvided: forProvided, reverse: reverse)
+                if quote.inAmount == 0.0 || quote.outAmount == 0.0 { continue }
+                if !hasBest || quote.outAmount > bestOutAmount {
+                    hasBest = true
+                    bestIdx = i
+                    bestInAmount = quote.inAmount
+                    bestOutAmount = quote.outAmount
+                }
+            }
+
             return MultiSwapperQuote(
                 inType: reverse ? self.outType() : self.inType(),
                 outType: reverse ? self.inType() : self.outType(),
-                inAmount: forProvided,
-                outAmount: estimate[1],
-                swapperIndex: Int(estimate[0])
+                inAmount: bestInAmount,
+                outAmount: bestOutAmount,
+                swapperIndex: bestIdx
             )
         }
         /// Performs a swap taking a Vault of type inVault, outputting a resulting outVault. Implementations may choose
@@ -183,24 +239,6 @@ access(all) contract SwapConnectors {
         access(all) fun swapBack(quote: {DeFiActions.Quote}?, residual: @{FungibleToken.Vault}): @{FungibleToken.Vault} {
             return <-self._swap(quote: quote, from: <-residual, reverse: true)
         }
-        /// Returns the the index of the optimal Swapper (result[0]) and the associated amountOut or amountIn (result[0])
-        /// as a UFix64 array
-        access(self) fun _estimate(amount: UFix64, out: Bool, reverse: Bool): [UFix64; 2] {
-            var res: [UFix64; 2] = out ? [0.0, 0.0] : [0.0, UFix64.max] // maximizing for out, minimizing for in
-            for i in InclusiveRange(0, self.swappers.length - 1) {
-                let swapper = &self.swappers[i] as &{DeFiActions.Swapper}
-                // call the appropriate estimator
-                let estimate = out
-                    ? swapper.quoteOut(forProvided: amount, reverse: reverse).outAmount
-                    : swapper.quoteIn(forDesired: amount, reverse: reverse).inAmount
-                // estimates of 0.0 are presumed to be graceful failures - skip them
-                if estimate > 0.0 && (out ? res[1] < estimate : estimate < res[1]) {
-                    // take minimum for in, maximum for out
-                    res = [UFix64(i), estimate]
-                }
-            }
-            return res
-        }
         /// Swaps the provided Vault in the defined direction. If the quote is not a MultiSwapperQuote, a new quote is
         /// requested and the current optimal Swapper used to fulfill the swap.
         access(self) fun _swap(quote: {DeFiActions.Quote}?, from: @{FungibleToken.Vault}, reverse: Bool): @{FungibleToken.Vault} {
@@ -607,7 +645,8 @@ access(all) contract SwapConnectors {
             }
 
             // expect output amount as the lesser between the amount available and the maximum amount
-            var quote = minimumAvail < maxAmount
+            let usingQuoteOut = minimumAvail < maxAmount
+            var quote = usingQuoteOut
                 ? self.swapper.quoteOut(forProvided: self.source.minimumAvailable(), reverse: false)
                 : self.swapper.quoteIn(forDesired: maxAmount, reverse: false)
 

cadence/contracts/connectors/evm/UniswapV3SwapConnectors.cdc

@@ -682,7 +682,6 @@ access(all) contract UniswapV3SwapConnectors {
             // optional dev guards
             let _chkIn  = EVMAbiHelpers.abiUInt256(evmAmountIn)
             let _chkMin = EVMAbiHelpers.abiUInt256(minOutUint)
-            //panic("path: \(EVMAbiHelpers.toHex(pathBytes.value)), amountIn: \(evmAmountIn.toString()), amountOutMin: \(minOutUint.toString())")
             assert(_chkIn.length == 32,  message: "amountIn not 32 bytes")
             assert(_chkMin.length == 32, message: "amountOutMin not 32 bytes")
 

cadence/tests/SwapConnectorsMultiSwapper_test.cdc

@@ -0,0 +1,191 @@
+import Test
+import BlockchainHelpers
+
+import "TokenA"
+import "TokenB"
+
+import "DeFiActions"
+import "SwapConnectors"
+import "MockSwapper"
+
+access(all) let testTokenAccount = Test.getAccount(0x0000000000000010)
+access(all) let serviceAccount = Test.serviceAccount()
+
+access(all)
+fun transferFlow(signer: Test.TestAccount, recipient: Address, amount: UFix64) {
+    let code = Test.readFile("../transactions/flow-token/transfer_flow.cdc")
+    let txn = Test.Transaction(code: code, authorizers: [signer.address], signers: [signer], arguments: [recipient, amount])
+    Test.expect(Test.executeTransaction(txn), Test.beSucceeded())
+}
+
+// inVault: TokenA, outVault: TokenB — shared across all multi-swapper tests
+access(all) let inVaultType  = Type<@TokenA.Vault>()
+access(all) let outVaultType = Type<@TokenB.Vault>()
+
+/// Returns a CapLimitedSwapper config using TokenA → TokenB vaults.
+access(all) fun makeConfig(priceRatio: UFix64, maxOut: UFix64): {String: AnyStruct} {
+    return {
+        "inVault":     inVaultType,
+        "outVault":    outVaultType,
+        "inVaultPath": TokenA.VaultStoragePath,
+        "outVaultPath": TokenB.VaultStoragePath,
+        "priceRatio":  priceRatio,
+        "maxOut":      maxOut
+    }
+}
+
+access(all)
+fun setup() {
+    log("================== Setting up SwapConnectorsMultiSwapper test ==================")
+    var err = Test.deployContract(name: "TestTokenMinter", path: "./contracts/TestTokenMinter.cdc", arguments: [])
+    Test.expect(err, Test.beNil())
+    err = Test.deployContract(name: "TokenA", path: "./contracts/TokenA.cdc", arguments: [])
+    Test.expect(err, Test.beNil())
+    err = Test.deployContract(name: "TokenB", path: "./contracts/TokenB.cdc", arguments: [])
+    Test.expect(err, Test.beNil())
+    err = Test.deployContract(name: "DeFiActionsUtils", path: "../contracts/utils/DeFiActionsUtils.cdc", arguments: [])
+    Test.expect(err, Test.beNil())
+    err = Test.deployContract(name: "DeFiActions", path: "../contracts/interfaces/DeFiActions.cdc", arguments: [])
+    Test.expect(err, Test.beNil())
+    err = Test.deployContract(name: "FungibleTokenConnectors", path: "../contracts/connectors/FungibleTokenConnectors.cdc", arguments: [])
+    Test.expect(err, Test.beNil())
+    err = Test.deployContract(name: "SwapConnectors", path: "../contracts/connectors/SwapConnectors.cdc", arguments: [])
+    Test.expect(err, Test.beNil())
+    err = Test.deployContract(name: "MockSwapper", path: "./contracts/MockSwapper.cdc", arguments: [])
+    Test.expect(err, Test.beNil())
+
+    transferFlow(signer: serviceAccount, recipient: testTokenAccount.address, amount: 10.0)
+}
+
+/// quoteIn — among two full-coverage routes, the one with the lower inAmount wins.
+///
+/// Swapper 0: priceRatio=0.5 → inAmount = 10.0/0.5 = 20.0  (expensive, full coverage)
+/// Swapper 1: priceRatio=0.8 → inAmount = 10.0/0.8 = 12.5  (cheaper, full coverage)
+/// Expected: index 1, inAmount=12.5, outAmount=10.0
+///
+access(all)
+fun testQuoteInPreferMinInAmongFullCoverage() {
+    let forDesired = 10.0
+    let configs = [
+        makeConfig(priceRatio: 0.5, maxOut: 100.0),
+        makeConfig(priceRatio: 0.8, maxOut: 100.0)
+    ]
+
+    let result = executeScript(
+        "./scripts/multi-swapper/mock_quote_in.cdc",
+        [testTokenAccount.address, configs, inVaultType, outVaultType, forDesired, false]
+    )
+    Test.expect(result, Test.beSucceeded())
+    let quote = result.returnValue! as! SwapConnectors.MultiSwapperQuote
+
+    Test.assertEqual(1, quote.swapperIndex)
+    Test.assertEqual(10.0 / 0.8, quote.inAmount)
+    Test.assertEqual(forDesired, quote.outAmount)
+    Test.assertEqual(inVaultType,  quote.inType)
+    Test.assertEqual(outVaultType, quote.outType)
+}
+
+/// quoteIn — a full-coverage route wins over a partial-coverage route even when the partial
+/// route has a lower inAmount.
+///
+/// Swapper 0: priceRatio=1.0, maxOut=5.0  → partial (outAmount=5.0 < 10.0), inAmount=5.0
+/// Swapper 1: priceRatio=0.5, maxOut=100.0 → full   (outAmount=10.0),        inAmount=20.0
+/// Expected: index 1 (full coverage wins despite higher inAmount)
+///
+access(all)
+fun testQuoteInFullWinsOverPartial() {
+    let forDesired = 10.0
+    let configs = [
+        makeConfig(priceRatio: 1.0, maxOut: 5.0),
+        makeConfig(priceRatio: 0.5, maxOut: 100.0)
+    ]
+
+    let result = executeScript(
+        "./scripts/multi-swapper/mock_quote_in.cdc",
+        [testTokenAccount.address, configs, inVaultType, outVaultType, forDesired, false]
+    )
+    Test.expect(result, Test.beSucceeded())
+    let quote = result.returnValue! as! SwapConnectors.MultiSwapperQuote
+
+    Test.assertEqual(1, quote.swapperIndex)
+    Test.assertEqual(forDesired, quote.outAmount)
+}
+
+/// quoteIn — when no full-coverage route exists, the partial route with the highest outAmount wins.
+///
+/// Swapper 0: priceRatio=0.8, maxOut=3.0 → partial (outAmount=3.0)
+/// Swapper 1: priceRatio=1.0, maxOut=7.0 → partial (outAmount=7.0)
+/// Expected: index 1 (higher outAmount among partials)
+///
+access(all)
+fun testQuoteInPartialFallbackMaxOut() {
+    let forDesired = 10.0
+    let configs = [
+        makeConfig(priceRatio: 0.8, maxOut: 3.0),
+        makeConfig(priceRatio: 1.0, maxOut: 7.0)
+    ]
+
+    let result = executeScript(
+        "./scripts/multi-swapper/mock_quote_in.cdc",
+        [testTokenAccount.address, configs, inVaultType, outVaultType, forDesired, false]
+    )
+    Test.expect(result, Test.beSucceeded())
+    let quote = result.returnValue! as! SwapConnectors.MultiSwapperQuote
+
+    Test.assertEqual(1, quote.swapperIndex)
+    Test.assertEqual(7.0, quote.outAmount)
+    Test.assertEqual(7.0, quote.inAmount) // 7.0 / priceRatio=1.0
+}
+
+/// quoteOut — the route with the highest outAmount wins.
+///
+/// Swapper 0: priceRatio=0.5, maxOut=100.0 → outAmount=5.0
+/// Swapper 1: priceRatio=0.8, maxOut=100.0 → outAmount=8.0
+/// Expected: index 1 (higher outAmount)
+///
+access(all)
+fun testQuoteOutPreferMaxOut() {
+    let forProvided = 10.0
+    let configs = [
+        makeConfig(priceRatio: 0.5, maxOut: 100.0),
+        makeConfig(priceRatio: 0.8, maxOut: 100.0)
+    ]
+
+    let result = executeScript(
+        "./scripts/multi-swapper/mock_quote_out.cdc",
+        [testTokenAccount.address, configs, inVaultType, outVaultType, forProvided, false]
+    )
+    Test.expect(result, Test.beSucceeded())
+    let quote = result.returnValue! as! SwapConnectors.MultiSwapperQuote
+
+    Test.assertEqual(1, quote.swapperIndex)
+    Test.assertEqual(10.0 * 0.8, quote.outAmount)
+    Test.assertEqual(inVaultType,  quote.inType)
+    Test.assertEqual(outVaultType, quote.outType)
+}
+
+/// quoteOut — a cap constraint causes a higher-ratio route to deliver less output than a
+/// lower-ratio uncapped route, so the uncapped route wins.
+///
+/// Swapper 0: priceRatio=0.5, maxOut=100.0 → outAmount=5.0  (uncapped)
+/// Swapper 1: priceRatio=0.9, maxOut=4.0   → rawOut=9.0, capped to 4.0
+/// Expected: index 0 (outAmount=5.0 > 4.0 after cap)
+///
+access(all)
+fun testQuoteOutCapLimitsRoute() {
+    let forProvided = 10.0
+    let configs = [
+        makeConfig(priceRatio: 0.5, maxOut: 100.0),
+        makeConfig(priceRatio: 0.9, maxOut: 4.0)
+    ]
+
+    let result = executeScript(
+        "./scripts/multi-swapper/mock_quote_out.cdc",
+        [testTokenAccount.address, configs, inVaultType, outVaultType, forProvided, false]
+    )
+    Test.expect(result, Test.beSucceeded())
+    let quote = result.returnValue! as! SwapConnectors.MultiSwapperQuote
+
+    Test.assertEqual(0, quote.swapperIndex)
+    Test.assertEqual(5.0, quote.outAmount) // 10.0 * 0.5
+}