@0x/asset-swapper: Fix getPathSize() and getAdjustedPathSize() bug.

`@0x/asset-swapper`: Add `maxFallbackSlippage` option.

packages/asset-swapper/CHANGELOG.json

@@ -21,6 +21,10 @@
             {
                 "note": "Add fallback orders to quotes via `allowFallback` option.",
                 "pr": 2513
+            },
+            {
+                "note": "Add `maxFallbackSlippage` option.",
+                "pr": 2513
             }
         ]
     },

packages/asset-swapper/src/types.ts

@@ -188,7 +188,6 @@ export interface SwapQuoteOrdersBreakdown {
 }
 
 /**
- * slippagePercentage: The percentage buffer to add to account for slippage. Affects max ETH price estimates. Defaults to 0.2 (20%).
  * gasPrice: gas price to determine protocolFee amount, default to ethGasStation fast amount
  */
 export interface SwapQuoteRequestOpts extends CalculateSwapQuoteOpts {

packages/asset-swapper/src/utils/market_operation_utils/constants.ts

@@ -28,6 +28,7 @@ export const DEFAULT_GET_MARKET_ORDERS_OPTS: GetMarketOrdersOpts = {
     runLimit: 2 ** 15,
     excludedSources: [],
     bridgeSlippage: 0.005,
+    maxFallbackSlippage: 0.1,
     numSamples: 20,
     sampleDistributionBase: 1.05,
     feeSchedule: {},

packages/asset-swapper/src/utils/market_operation_utils/fills.ts

@@ -169,7 +169,7 @@ export function getPathSize(path: Fill[], targetInput: BigNumber = POSITIVE_INF)
     let output = ZERO_AMOUNT;
     for (const fill of path) {
         if (input.plus(fill.input).gte(targetInput)) {
-            const di = targetInput.minus(input).div(fill.input);
+            const di = targetInput.minus(input);
             input = input.plus(di);
             output = output.plus(fill.output.times(di.div(fill.input)));
             break;
@@ -186,7 +186,7 @@ export function getPathAdjustedSize(path: Fill[], targetInput: BigNumber = POSIT
     let output = ZERO_AMOUNT;
     for (const fill of path) {
         if (input.plus(fill.input).gte(targetInput)) {
-            const di = targetInput.minus(input).div(fill.input);
+            const di = targetInput.minus(input);
             input = input.plus(di);
             output = output.plus(fill.adjustedOutput.times(di.div(fill.input)));
             break;
@@ -281,3 +281,25 @@ export function getFallbackSourcePaths(optimalPath: Fill[], allPaths: Fill[][]):
     }
     return fallbackPaths;
 }
+
+export function getPathAdjustedRate(side: MarketOperation, path: Fill[], targetInput: BigNumber): BigNumber {
+    const [input, output] = getPathAdjustedSize(path, targetInput);
+    if (input.eq(0) || output.eq(0)) {
+        return ZERO_AMOUNT;
+    }
+    return side === MarketOperation.Sell ? output.div(input) : input.div(output);
+}
+
+export function getPathAdjustedSlippage(
+    side: MarketOperation,
+    path: Fill[],
+    inputAmount: BigNumber,
+    maxRate: BigNumber,
+): number {
+    if (maxRate.eq(0)) {
+        return 0;
+    }
+    const totalRate = getPathAdjustedRate(side, path, inputAmount);
+    const rateChange = maxRate.minus(totalRate);
+    return rateChange.div(maxRate).toNumber();
+}

packages/asset-swapper/src/utils/market_operation_utils/index.ts

@@ -6,7 +6,13 @@ import { MarketOperation } from '../../types';
 import { difference } from '../utils';
 
 import { BUY_SOURCES, DEFAULT_GET_MARKET_ORDERS_OPTS, FEE_QUOTE_SOURCES, ONE_ETHER, SELL_SOURCES } from './constants';
-import { createFillPaths, getFallbackSourcePaths, getPathSize } from './fills';
+import {
+    createFillPaths,
+    getFallbackSourcePaths,
+    getPathAdjustedRate,
+    getPathAdjustedSlippage,
+    getPathSize,
+} from './fills';
 import { createOrdersFromPath, createSignedOrdersWithFillableAmounts, getNativeOrderTokens } from './orders';
 import { findOptimalPath } from './path_optimizer';
 import { DexOrderSampler, getSampleAmounts } from './sampler';
@@ -97,6 +103,7 @@ export class MarketOperationUtils {
             inputAmount: takerAmount,
             ethToOutputRate: ethToMakerAssetRate,
             bridgeSlippage: _opts.bridgeSlippage,
+            maxFallbackSlippage: _opts.maxFallbackSlippage,
             excludedSources: _opts.excludedSources,
             feeSchedule: _opts.feeSchedule,
             allowFallback: _opts.allowFallback,
@@ -169,6 +176,7 @@ export class MarketOperationUtils {
             inputAmount: makerAmount,
             ethToOutputRate: ethToTakerAssetRate,
             bridgeSlippage: _opts.bridgeSlippage,
+            maxFallbackSlippage: _opts.maxFallbackSlippage,
             excludedSources: _opts.excludedSources,
             feeSchedule: _opts.feeSchedule,
             allowFallback: _opts.allowFallback,
@@ -241,6 +249,7 @@ export class MarketOperationUtils {
                 inputAmount: makerAmount,
                 ethToOutputRate: ethToTakerAssetRate,
                 bridgeSlippage: _opts.bridgeSlippage,
+                maxFallbackSlippage: _opts.maxFallbackSlippage,
                 excludedSources: _opts.excludedSources,
                 feeSchedule: _opts.feeSchedule,
                 allowFallback: _opts.allowFallback,
@@ -259,12 +268,14 @@ export class MarketOperationUtils {
         runLimit?: number;
         ethToOutputRate?: BigNumber;
         bridgeSlippage?: number;
+        maxFallbackSlippage?: number;
         excludedSources?: ERC20BridgeSource[];
         feeSchedule?: { [source: string]: BigNumber };
         allowFallback?: boolean;
         liquidityProviderAddress?: string;
     }): OptimizedMarketOrder[] {
         const { inputToken, outputToken, side, inputAmount } = opts;
+        const maxFallbackSlippage = opts.maxFallbackSlippage || 0;
         // Convert native orders and dex quotes into fill paths.
         const paths = createFillPaths({
             side,
@@ -277,8 +288,10 @@ export class MarketOperationUtils {
             feeSchedule: opts.feeSchedule,
         });
         // Find the optimal path.
-        const optimalPath = findOptimalPath(side, paths, inputAmount, opts.runLimit);
+        const optimalPath = findOptimalPath(side, paths, inputAmount, opts.runLimit) || [];
-        if (!optimalPath) {
+        // TODO(dorothy-zbornak): Ensure the slippage on the optimal path is <= maxFallbackSlippage
+        // once we decide on a good baseline.
+        if (optimalPath.length === 0) {
             throw new Error(AggregationError.NoOptimalPath);
         }
         // Generate a fallback path if native orders are in the optimal paath.
@@ -290,6 +303,15 @@ export class MarketOperationUtils {
             fallbackPath =
                 findOptimalPath(side, getFallbackSourcePaths(optimalPath, paths), fallbackInputAmount, opts.runLimit) ||
                 [];
+            const fallbackSlippage = getPathAdjustedSlippage(
+                side,
+                fallbackPath,
+                fallbackInputAmount,
+                getPathAdjustedRate(side, optimalPath, inputAmount),
+            );
+            if (fallbackSlippage > maxFallbackSlippage) {
+                fallbackPath = [];
+            }
         }
         return createOrdersFromPath([...optimalPath, ...fallbackPath], {
             side,

packages/asset-swapper/src/utils/market_operation_utils/types.ts

@@ -152,6 +152,12 @@ export interface GetMarketOrdersOpts {
      * Default is 0.0005 (5 basis points).
      */
     bridgeSlippage: number;
+    /**
+     * The maximum price slippage allowed in the fallback quote. If the slippage
+     * between the optimal quote and the fallback quote is greater than this
+     * percentage, no fallback quote will be provided.
+     */
+    maxFallbackSlippage: number;
     /**
      * Number of samples to take for each DEX quote.
      */

packages/asset-swapper/test/market_operation_utils_test.ts

@@ -296,6 +296,7 @@ describe('MarketOperationUtils tests', () => {
                 numSamples: NUM_SAMPLES,
                 sampleDistributionBase: 1,
                 bridgeSlippage: 0,
+                maxFallbackSlippage: 100,
                 excludedSources: Object.keys(DEFAULT_CURVE_OPTS) as ERC20BridgeSource[],
                 allowFallback: false,
             };
@@ -574,7 +575,6 @@ describe('MarketOperationUtils tests', () => {
                 rates[ERC20BridgeSource.Native] = [0.9, 0.8, 0.5, 0.5];
                 rates[ERC20BridgeSource.Uniswap] = [0.6, 0.05, 0.01, 0.01];
                 rates[ERC20BridgeSource.Eth2Dai] = [0.4, 0.3, 0.01, 0.01];
-                // Won't be included because of conflicts.
                 rates[ERC20BridgeSource.Kyber] = [0.35, 0.2, 0.01, 0.01];
                 replaceSamplerOps({
                     getSellQuotes: createGetMultipleSellQuotesOperationFromRates(rates),
@@ -596,6 +596,27 @@ describe('MarketOperationUtils tests', () => {
                 expect(orderSources.slice(firstSources.length).sort()).to.deep.eq(secondSources.sort());
             });
 
+            it('does not create a fallback if below maxFallbackSlippage', async () => {
+                const rates: RatesBySource = {};
+                rates[ERC20BridgeSource.Native] = [1, 1, 0.01, 0.01];
+                rates[ERC20BridgeSource.Uniswap] = [1, 1, 0.01, 0.01];
+                rates[ERC20BridgeSource.Eth2Dai] = [0.49, 0.49, 0.49, 0.49];
+                rates[ERC20BridgeSource.Kyber] = [0.35, 0.2, 0.01, 0.01];
+                replaceSamplerOps({
+                    getSellQuotes: createGetMultipleSellQuotesOperationFromRates(rates),
+                });
+                const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync(
+                    createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]),
+                    FILL_AMOUNT,
+                    { ...DEFAULT_OPTS, numSamples: 4, allowFallback: true, maxFallbackSlippage: 0.5 },
+                );
+                const orderSources = improvedOrders.map(o => o.fill.source);
+                const firstSources = [ERC20BridgeSource.Native, ERC20BridgeSource.Native, ERC20BridgeSource.Uniswap];
+                const secondSources: ERC20BridgeSource[] = [];
+                expect(orderSources.slice(0, firstSources.length).sort()).to.deep.eq(firstSources.sort());
+                expect(orderSources.slice(firstSources.length).sort()).to.deep.eq(secondSources.sort());
+            });
+
             it('is able to create a order from LiquidityProvider', async () => {
                 const registryAddress = randomAddress();
                 const liquidityProviderAddress = randomAddress();
@@ -662,6 +683,8 @@ describe('MarketOperationUtils tests', () => {
             const DEFAULT_OPTS = {
                 numSamples: NUM_SAMPLES,
                 sampleDistributionBase: 1,
+                bridgeSlippage: 0,
+                maxFallbackSlippage: 100,
                 excludedSources: Object.keys(DEFAULT_CURVE_OPTS) as ERC20BridgeSource[],
                 allowFallback: false,
             };
@@ -909,6 +932,26 @@ describe('MarketOperationUtils tests', () => {
                 expect(orderSources.slice(0, firstSources.length).sort()).to.deep.eq(firstSources.sort());
                 expect(orderSources.slice(firstSources.length).sort()).to.deep.eq(secondSources.sort());
             });
+
+            it('does not create a fallback if below maxFallbackSlippage', async () => {
+                const rates: RatesBySource = {};
+                rates[ERC20BridgeSource.Native] = [1, 1, 0.01, 0.01];
+                rates[ERC20BridgeSource.Uniswap] = [1, 1, 0.01, 0.01];
+                rates[ERC20BridgeSource.Eth2Dai] = [0.49, 0.49, 0.49, 0.49];
+                replaceSamplerOps({
+                    getBuyQuotes: createGetMultipleBuyQuotesOperationFromRates(rates),
+                });
+                const improvedOrders = await marketOperationUtils.getMarketBuyOrdersAsync(
+                    createOrdersFromBuyRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]),
+                    FILL_AMOUNT,
+                    { ...DEFAULT_OPTS, numSamples: 4, allowFallback: true, maxFallbackSlippage: 0.5 },
+                );
+                const orderSources = improvedOrders.map(o => o.fill.source);
+                const firstSources = [ERC20BridgeSource.Native, ERC20BridgeSource.Native, ERC20BridgeSource.Uniswap];
+                const secondSources: ERC20BridgeSource[] = [];
+                expect(orderSources.slice(0, firstSources.length).sort()).to.deep.eq(firstSources.sort());
+                expect(orderSources.slice(firstSources.length).sort()).to.deep.eq(secondSources.sort());
+            });
         });
     });
 });