From a57cf68ee4703f6e41927bb9e26e98ab6679409f Mon Sep 17 00:00:00 2001
From: Michael Zhu <mchl.zhu.96@gmail.com>
Date: Wed, 23 Sep 2020 12:46:50 -0700
Subject: [PATCH 1/8] Handle max in/out ratio reverts in Balancer sampling
 functions

---
 packages/asset-swapper/CHANGELOG.json         |  4 ++
 .../contracts/src/BalancerSampler.sol         | 39 +++++++++++++++++++
 .../market_operation_utils/balancer_utils.ts  |  9 ++++-
 3 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json
index 29a521bd58..168d5f3b00 100644
--- a/packages/asset-swapper/CHANGELOG.json
+++ b/packages/asset-swapper/CHANGELOG.json
@@ -125,6 +125,10 @@
             {
                 "note": "Pass back fillData from quote reporter",
                 "pr": 2702
+            },
+            {
+                "note": "Fix Balancer sampling",
+                "pr": 2711
             }
         ]
     },
diff --git a/packages/asset-swapper/contracts/src/BalancerSampler.sol b/packages/asset-swapper/contracts/src/BalancerSampler.sol
index 0950cc8e15..8606935574 100644
--- a/packages/asset-swapper/contracts/src/BalancerSampler.sol
+++ b/packages/asset-swapper/contracts/src/BalancerSampler.sol
@@ -27,6 +27,12 @@ contract BalancerSampler {
     /// @dev Base gas limit for Balancer calls.
     uint256 constant private BALANCER_CALL_GAS = 300e3; // 300k
 
+    // Balancer math constants
+    // https://github.com/balancer-labs/balancer-core/blob/master/contracts/BConst.sol
+    uint256 constant private BONE = 10 ** 18;
+    uint256 constant private MAX_IN_RATIO = BONE / 2;
+    uint256 constant private MAX_OUT_RATIO = (BONE / 3) + 1 wei;
+
     struct BalancerState {
         uint256 takerTokenBalance;
         uint256 makerTokenBalance;
@@ -67,6 +73,11 @@ contract BalancerSampler {
         poolState.swapFee = pool.getSwapFee();
 
         for (uint256 i = 0; i < numSamples; i++) {
+            // Handles this revert scenario:
+            // https://github.com/balancer-labs/balancer-core/blob/master/contracts/BPool.sol#L443
+            if (takerTokenAmounts[i] > _bmul(poolState.takerTokenBalance, MAX_IN_RATIO)) {
+                break;
+            }
             (bool didSucceed, bytes memory resultData) =
                 poolAddress.staticcall.gas(BALANCER_CALL_GAS)(
                     abi.encodeWithSelector(
@@ -120,6 +131,11 @@ contract BalancerSampler {
         poolState.swapFee = pool.getSwapFee();
 
         for (uint256 i = 0; i < numSamples; i++) {
+            // Handles this revert scenario:
+            // https://github.com/balancer-labs/balancer-core/blob/master/contracts/BPool.sol#L505
+            if (makerTokenAmounts[i] > _bmul(poolState.makerTokenBalance, MAX_OUT_RATIO)) {
+                break;
+            }
             (bool didSucceed, bytes memory resultData) =
                 poolAddress.staticcall.gas(BALANCER_CALL_GAS)(
                     abi.encodeWithSelector(
@@ -140,4 +156,27 @@ contract BalancerSampler {
             takerTokenAmounts[i] = sellAmount;
         }
     }
+
+    /// @dev Hacked version of Balancer's `bmul` function, returning 0 instead
+    ///      of reverting.
+    ///      https://github.com/balancer-labs/balancer-core/blob/master/contracts/BNum.sol#L63-L73
+    /// @param a The first operand.
+    /// @param b The second operand.
+    /// @param c The result of the multiplication, or 0 if `bmul` would've reverted.
+    function _bmul(uint256 a, uint256 b)
+        private
+        pure
+        returns (uint256 c)
+    {
+        uint c0 = a * b;
+        if (a != 0 && c0 / a != b) {
+            return 0;
+        }
+        uint c1 = c0 + (BONE / 2);
+        if (c1 < c0) {
+            return 0;
+        }
+        uint c2 = c1 / BONE;
+        return c2;
+    }
 }
diff --git a/packages/asset-swapper/src/utils/market_operation_utils/balancer_utils.ts b/packages/asset-swapper/src/utils/market_operation_utils/balancer_utils.ts
index 936cd07e37..6a76c29f03 100644
--- a/packages/asset-swapper/src/utils/market_operation_utils/balancer_utils.ts
+++ b/packages/asset-swapper/src/utils/market_operation_utils/balancer_utils.ts
@@ -2,6 +2,8 @@ import { BigNumber } from '@0x/utils';
 import { bmath, getPoolsWithTokens, parsePoolData } from '@balancer-labs/sor';
 import { Decimal } from 'decimal.js';
 
+import { ZERO_AMOUNT } from './constants';
+
 // tslint:disable:boolean-naming
 
 export interface BalancerPool {
@@ -118,6 +120,9 @@ export class BalancerPoolsCache {
 
 // tslint:disable completed-docs
 export function computeBalancerSellQuote(pool: BalancerPool, takerFillAmount: BigNumber): BigNumber {
+    if (takerFillAmount.isGreaterThan(bmath.bmul(pool.balanceIn, bmath.MAX_IN_RATIO))) {
+        return ZERO_AMOUNT;
+    }
     const weightRatio = pool.weightIn.dividedBy(pool.weightOut);
     const adjustedIn = bmath.BONE.minus(pool.swapFee)
         .dividedBy(bmath.BONE)
@@ -130,8 +135,8 @@ export function computeBalancerSellQuote(pool: BalancerPool, takerFillAmount: Bi
 }
 
 export function computeBalancerBuyQuote(pool: BalancerPool, makerFillAmount: BigNumber): BigNumber {
-    if (makerFillAmount.isGreaterThanOrEqualTo(pool.balanceOut)) {
-        return new BigNumber(0);
+    if (makerFillAmount.isGreaterThan(bmath.bmul(pool.balanceOut, bmath.MAX_OUT_RATIO))) {
+        return ZERO_AMOUNT;
     }
     const weightRatio = pool.weightOut.dividedBy(pool.weightIn);
     const diff = pool.balanceOut.minus(makerFillAmount);

From 6138955f933855d80be8eea0f2b9b5adf5b05ff9 Mon Sep 17 00:00:00 2001
From: Lawrence Forman <me@merklejerk.com>
Date: Wed, 23 Sep 2020 15:52:44 -0400
Subject: [PATCH 2/8] `@0x/asset-swapper`: respect max slippage in EP consumer

---
 packages/asset-swapper/CHANGELOG.json         |  4 +++
 .../exchange_proxy_swap_quote_consumer.ts     |  6 ++--
 .../src/quote_consumers/utils.ts              | 28 +++++++++++++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 packages/asset-swapper/src/quote_consumers/utils.ts

diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json
index 168d5f3b00..cc4d84e97a 100644
--- a/packages/asset-swapper/CHANGELOG.json
+++ b/packages/asset-swapper/CHANGELOG.json
@@ -129,6 +129,10 @@
             {
                 "note": "Fix Balancer sampling",
                 "pr": 2711
+            },
+            {
+                "note": "Respect max slippage in EP consumer",
+                "pr": 2712
             }
         ]
     },
diff --git a/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts b/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts
index 3ff074ac04..a53a4a8be0 100644
--- a/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts
+++ b/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts
@@ -30,6 +30,8 @@ import { assert } from '../utils/assert';
 import { ERC20BridgeSource, UniswapV2FillData } from '../utils/market_operation_utils/types';
 import { getTokenFromAssetData } from '../utils/utils';
 
+import { getMinBuyAmount } from './utils';
+
 // tslint:disable-next-line:custom-no-magic-numbers
 const MAX_UINT256 = new BigNumber(2).pow(256).minus(1);
 const { NULL_ADDRESS, ZERO_AMOUNT } = constants;
@@ -93,6 +95,7 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase {
         const sellToken = getTokenFromAssetData(quote.takerAssetData);
         const buyToken = getTokenFromAssetData(quote.makerAssetData);
         const sellAmount = quote.worstCaseQuoteInfo.totalTakerAssetAmount;
+        const minBuyAmount = getMinBuyAmount(quote);
 
         // VIP routes.
         if (isDirectUniswapCompatible(quote, optsWithDefaults)) {
@@ -111,7 +114,7 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase {
                             return a;
                         }),
                         sellAmount,
-                        quote.worstCaseQuoteInfo.makerAssetAmount,
+                        minBuyAmount,
                         source === ERC20BridgeSource.SushiSwap,
                     )
                     .getABIEncodedTransactionData(),
@@ -225,7 +228,6 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase {
             }),
         });
 
-        const minBuyAmount = BigNumber.max(0, quote.worstCaseQuoteInfo.makerAssetAmount.minus(buyTokenFeeAmount));
         const calldataHexString = this._exchangeProxy
             .transformERC20(
                 isFromETH ? ETH_TOKEN_ADDRESS : sellToken,
diff --git a/packages/asset-swapper/src/quote_consumers/utils.ts b/packages/asset-swapper/src/quote_consumers/utils.ts
new file mode 100644
index 0000000000..f87f8318e1
--- /dev/null
+++ b/packages/asset-swapper/src/quote_consumers/utils.ts
@@ -0,0 +1,28 @@
+import { BigNumber } from '@0x/utils';
+import * as _ from 'lodash';
+
+import { MarketOperation, SwapQuote } from '../types';
+
+/**
+ * Compute the mminimum buy token amount for market operations by inferring
+ * the slippage from the orders in a quote. We cannot rely on
+ * `worstCaseQuoteInfo.makerAssetAmount` because that does not stop at
+ * maximum slippage.
+ */
+export function getMinBuyAmount(quote: SwapQuote): BigNumber {
+    // Infer the allowed maker asset slippage from the orders.
+    const totalOrderMakerAssetAmount = BigNumber.sum(...quote.orders.map(o => o.makerAssetAmount));
+    const totalFillMakerAssetAmount =
+        quote.type === MarketOperation.Sell
+            ? BigNumber.sum(...quote.orders.map(o => BigNumber.sum(0, ...o.fills.map(f => f.output))))
+            : BigNumber.sum(...quote.orders.map(o => BigNumber.sum(0, ...o.fills.map(f => f.input))));
+    if (totalFillMakerAssetAmount.eq(0)) {
+        return quote.worstCaseQuoteInfo.makerAssetAmount;
+    }
+    if (totalOrderMakerAssetAmount.eq(totalFillMakerAssetAmount)) {
+        // No slippage allowed on bought tokens.
+        return quote.bestCaseQuoteInfo.makerAssetAmount;
+    }
+    const slipRatio = totalOrderMakerAssetAmount.div(totalFillMakerAssetAmount);
+    return quote.bestCaseQuoteInfo.makerAssetAmount.times(slipRatio).integerValue(BigNumber.ROUND_DOWN);
+}

From 4cf566cad8965a02e4bf960a57524bf641bdeaed Mon Sep 17 00:00:00 2001
From: Lawrence Forman <me@merklejerk.com>
Date: Wed, 23 Sep 2020 16:52:09 -0400
Subject: [PATCH 3/8] `@0x/asset-swapper`: Special case two-hop quotes in
 `getMinBuyAmount()`

---
 packages/asset-swapper/src/quote_consumers/utils.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/asset-swapper/src/quote_consumers/utils.ts b/packages/asset-swapper/src/quote_consumers/utils.ts
index f87f8318e1..e5c009d6da 100644
--- a/packages/asset-swapper/src/quote_consumers/utils.ts
+++ b/packages/asset-swapper/src/quote_consumers/utils.ts
@@ -16,7 +16,7 @@ export function getMinBuyAmount(quote: SwapQuote): BigNumber {
         quote.type === MarketOperation.Sell
             ? BigNumber.sum(...quote.orders.map(o => BigNumber.sum(0, ...o.fills.map(f => f.output))))
             : BigNumber.sum(...quote.orders.map(o => BigNumber.sum(0, ...o.fills.map(f => f.input))));
-    if (totalFillMakerAssetAmount.eq(0)) {
+    if (quote.isTwoHop || totalFillMakerAssetAmount.eq(0)) {
         return quote.worstCaseQuoteInfo.makerAssetAmount;
     }
     if (totalOrderMakerAssetAmount.eq(totalFillMakerAssetAmount)) {

From 99de5a3814de33f2c6b3b5fcb6f83ac3770386ad Mon Sep 17 00:00:00 2001
From: Lawrence Forman <me@merklejerk.com>
Date: Wed, 23 Sep 2020 16:57:46 -0400
Subject: [PATCH 4/8] `@0x/asset-swapper`: factor affiliate fees into new
 `minBuyAmount` calculation in EP consumer

---
 .../src/quote_consumers/exchange_proxy_swap_quote_consumer.ts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts b/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts
index a53a4a8be0..6f4aaf1f6a 100644
--- a/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts
+++ b/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts
@@ -95,7 +95,7 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase {
         const sellToken = getTokenFromAssetData(quote.takerAssetData);
         const buyToken = getTokenFromAssetData(quote.makerAssetData);
         const sellAmount = quote.worstCaseQuoteInfo.totalTakerAssetAmount;
-        const minBuyAmount = getMinBuyAmount(quote);
+        let minBuyAmount = getMinBuyAmount(quote);
 
         // VIP routes.
         if (isDirectUniswapCompatible(quote, optsWithDefaults)) {
@@ -213,6 +213,8 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase {
                     ],
                 }),
             });
+            // Adjust the minimum buy amount by the fee.
+            minBuyAmount = BigNumber.max(0, minBuyAmount.minus(buyTokenFeeAmount));
         }
 
         if (sellTokenFeeAmount.isGreaterThan(0) && feeRecipient !== NULL_ADDRESS) {

From de7f1fc207f136b8cfebef8dc39d063b2dfcf5ab Mon Sep 17 00:00:00 2001
From: Lawrence Forman <me@merklejerk.com>
Date: Wed, 23 Sep 2020 17:12:24 -0400
Subject: [PATCH 5/8] `@0x/asset-swapper: rename `getMinBuyAmount()` to
 `getSwapMinBuyAmount()` and export for use in API.

---
 packages/asset-swapper/src/index.ts                           | 1 +
 .../src/quote_consumers/exchange_proxy_swap_quote_consumer.ts | 4 ++--
 packages/asset-swapper/src/quote_consumers/utils.ts           | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/packages/asset-swapper/src/index.ts b/packages/asset-swapper/src/index.ts
index 96bf1d4073..f9029e256b 100644
--- a/packages/asset-swapper/src/index.ts
+++ b/packages/asset-swapper/src/index.ts
@@ -84,6 +84,7 @@ export {
 export { artifacts } from './artifacts';
 export { InsufficientAssetLiquidityError } from './errors';
 export { SwapQuoteConsumer } from './quote_consumers/swap_quote_consumer';
+export { getSwapMinBuyAmount } from './quote_consumers/utils';
 export { SwapQuoter } from './swap_quoter';
 export {
     AffiliateFee,
diff --git a/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts b/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts
index 6f4aaf1f6a..14e0bc7d72 100644
--- a/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts
+++ b/packages/asset-swapper/src/quote_consumers/exchange_proxy_swap_quote_consumer.ts
@@ -30,7 +30,7 @@ import { assert } from '../utils/assert';
 import { ERC20BridgeSource, UniswapV2FillData } from '../utils/market_operation_utils/types';
 import { getTokenFromAssetData } from '../utils/utils';
 
-import { getMinBuyAmount } from './utils';
+import { getSwapMinBuyAmount } from './utils';
 
 // tslint:disable-next-line:custom-no-magic-numbers
 const MAX_UINT256 = new BigNumber(2).pow(256).minus(1);
@@ -95,7 +95,7 @@ export class ExchangeProxySwapQuoteConsumer implements SwapQuoteConsumerBase {
         const sellToken = getTokenFromAssetData(quote.takerAssetData);
         const buyToken = getTokenFromAssetData(quote.makerAssetData);
         const sellAmount = quote.worstCaseQuoteInfo.totalTakerAssetAmount;
-        let minBuyAmount = getMinBuyAmount(quote);
+        let minBuyAmount = getSwapMinBuyAmount(quote);
 
         // VIP routes.
         if (isDirectUniswapCompatible(quote, optsWithDefaults)) {
diff --git a/packages/asset-swapper/src/quote_consumers/utils.ts b/packages/asset-swapper/src/quote_consumers/utils.ts
index e5c009d6da..ba236bc877 100644
--- a/packages/asset-swapper/src/quote_consumers/utils.ts
+++ b/packages/asset-swapper/src/quote_consumers/utils.ts
@@ -9,7 +9,7 @@ import { MarketOperation, SwapQuote } from '../types';
  * `worstCaseQuoteInfo.makerAssetAmount` because that does not stop at
  * maximum slippage.
  */
-export function getMinBuyAmount(quote: SwapQuote): BigNumber {
+export function getSwapMinBuyAmount(quote: SwapQuote): BigNumber {
     // Infer the allowed maker asset slippage from the orders.
     const totalOrderMakerAssetAmount = BigNumber.sum(...quote.orders.map(o => o.makerAssetAmount));
     const totalFillMakerAssetAmount =

From d7de191947fa7d81858d5ebadfb940e0097119fd Mon Sep 17 00:00:00 2001
From: Lawrence Forman <me@merklejerk.com>
Date: Wed, 23 Sep 2020 18:37:13 -0400
Subject: [PATCH 6/8] `@0x/asset-swapper`: Handle native orders in
 `getSwapMinBuyAmount()`

---
 .../src/quote_consumers/utils.ts              | 26 +++++++++++++------
 .../utils/market_operation_utils/orders.ts    |  2 +-
 ...exchange_proxy_swap_quote_consumer_test.ts |  7 ++---
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/packages/asset-swapper/src/quote_consumers/utils.ts b/packages/asset-swapper/src/quote_consumers/utils.ts
index ba236bc877..c805e79939 100644
--- a/packages/asset-swapper/src/quote_consumers/utils.ts
+++ b/packages/asset-swapper/src/quote_consumers/utils.ts
@@ -2,6 +2,10 @@ import { BigNumber } from '@0x/utils';
 import * as _ from 'lodash';
 
 import { MarketOperation, SwapQuote } from '../types';
+import { ERC20BridgeSource } from '../utils/market_operation_utils/types';
+import { constants } from '../constants';
+
+const { ZERO_AMOUNT } = constants;
 
 /**
  * Compute the mminimum buy token amount for market operations by inferring
@@ -10,19 +14,25 @@ import { MarketOperation, SwapQuote } from '../types';
  * maximum slippage.
  */
 export function getSwapMinBuyAmount(quote: SwapQuote): BigNumber {
-    // Infer the allowed maker asset slippage from the orders.
-    const totalOrderMakerAssetAmount = BigNumber.sum(...quote.orders.map(o => o.makerAssetAmount));
-    const totalFillMakerAssetAmount =
-        quote.type === MarketOperation.Sell
-            ? BigNumber.sum(...quote.orders.map(o => BigNumber.sum(0, ...o.fills.map(f => f.output))))
-            : BigNumber.sum(...quote.orders.map(o => BigNumber.sum(0, ...o.fills.map(f => f.input))));
-    if (quote.isTwoHop || totalFillMakerAssetAmount.eq(0)) {
+    if (quote.type === MarketOperation.Buy || quote.isTwoHop) {
         return quote.worstCaseQuoteInfo.makerAssetAmount;
     }
+    // Infer the allowed maker asset slippage from the orders.
+    const totalOrderMakerAssetAmount = BigNumber.sum(...quote.orders.map(o => o.fillableMakerAssetAmount));
+    let totalFillMakerAssetAmount = ZERO_AMOUNT;
+    for (const o of quote.orders) {
+        if (o.fills.length === 0 || o.fills[0].source === ERC20BridgeSource.Native) {
+            // No slippage on natuve orders.
+            totalFillMakerAssetAmount = totalFillMakerAssetAmount.plus(o.fillableMakerAssetAmount);
+        } else {
+            totalFillMakerAssetAmount = totalFillMakerAssetAmount.plus(BigNumber.sum(...o.fills.map(f => f.output)));
+        }
+    }
     if (totalOrderMakerAssetAmount.eq(totalFillMakerAssetAmount)) {
-        // No slippage allowed on bought tokens.
+        // No slippage allowed across all orders.
         return quote.bestCaseQuoteInfo.makerAssetAmount;
     }
     const slipRatio = totalOrderMakerAssetAmount.div(totalFillMakerAssetAmount);
+    console.log(slipRatio);
     return quote.bestCaseQuoteInfo.makerAssetAmount.times(slipRatio).integerValue(BigNumber.ROUND_DOWN);
 }
diff --git a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts
index efcb699a7a..804dcfa3a6 100644
--- a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts
+++ b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts
@@ -291,7 +291,7 @@ function createBridgeOrder(
             makerAssetData = assetDataUtils.encodeERC20BridgeAssetData(
                 makerToken,
                 bridgeAddress,
-                createBalancerBridgeData(takerToken, balancerFillData.poolAddress),
+                createBalancerBridgeData(takerToken, makerToken),
             );
             break;
         case ERC20BridgeSource.Bancor:
diff --git a/packages/asset-swapper/test/exchange_proxy_swap_quote_consumer_test.ts b/packages/asset-swapper/test/exchange_proxy_swap_quote_consumer_test.ts
index 5963c712f2..924d7782ce 100644
--- a/packages/asset-swapper/test/exchange_proxy_swap_quote_consumer_test.ts
+++ b/packages/asset-swapper/test/exchange_proxy_swap_quote_consumer_test.ts
@@ -18,6 +18,7 @@ import 'mocha';
 
 import { constants } from '../src/constants';
 import { ExchangeProxySwapQuoteConsumer } from '../src/quote_consumers/exchange_proxy_swap_quote_consumer';
+import { getSwapMinBuyAmount } from '../src/quote_consumers/utils';
 import { MarketBuySwapQuote, MarketOperation, MarketSellSwapQuote } from '../src/types';
 import { OptimizedMarketOrder } from '../src/utils/market_operation_utils/types';
 
@@ -191,7 +192,7 @@ describe('ExchangeProxySwapQuoteConsumer', () => {
             expect(callArgs.inputToken).to.eq(TAKER_TOKEN);
             expect(callArgs.outputToken).to.eq(MAKER_TOKEN);
             expect(callArgs.inputTokenAmount).to.bignumber.eq(quote.worstCaseQuoteInfo.totalTakerAssetAmount);
-            expect(callArgs.minOutputTokenAmount).to.bignumber.eq(quote.worstCaseQuoteInfo.makerAssetAmount);
+            expect(callArgs.minOutputTokenAmount).to.bignumber.eq(getSwapMinBuyAmount(quote));
             expect(callArgs.transformations).to.be.length(2);
             expect(
                 callArgs.transformations[0].deploymentNonce.toNumber() ===
@@ -220,7 +221,7 @@ describe('ExchangeProxySwapQuoteConsumer', () => {
             expect(callArgs.inputToken).to.eq(TAKER_TOKEN);
             expect(callArgs.outputToken).to.eq(MAKER_TOKEN);
             expect(callArgs.inputTokenAmount).to.bignumber.eq(quote.worstCaseQuoteInfo.totalTakerAssetAmount);
-            expect(callArgs.minOutputTokenAmount).to.bignumber.eq(quote.worstCaseQuoteInfo.makerAssetAmount);
+            expect(callArgs.minOutputTokenAmount).to.bignumber.eq(getSwapMinBuyAmount(quote));
             expect(callArgs.transformations).to.be.length(2);
             expect(
                 callArgs.transformations[0].deploymentNonce.toNumber() ===
@@ -318,7 +319,7 @@ describe('ExchangeProxySwapQuoteConsumer', () => {
             expect(callArgs.inputToken).to.eq(TAKER_TOKEN);
             expect(callArgs.outputToken).to.eq(MAKER_TOKEN);
             expect(callArgs.inputTokenAmount).to.bignumber.eq(quote.worstCaseQuoteInfo.totalTakerAssetAmount);
-            expect(callArgs.minOutputTokenAmount).to.bignumber.eq(quote.worstCaseQuoteInfo.makerAssetAmount);
+            expect(callArgs.minOutputTokenAmount).to.bignumber.eq(getSwapMinBuyAmount(quote));
             expect(callArgs.transformations).to.be.length(3);
             expect(
                 callArgs.transformations[0].deploymentNonce.toNumber() ===

From ac6b03cd4a3c7e58ec20b4b2cb8fbe3dd1c53c6c Mon Sep 17 00:00:00 2001
From: Lawrence Forman <me@merklejerk.com>
Date: Wed, 23 Sep 2020 21:58:45 -0400
Subject: [PATCH 7/8] fix LP and MB sources leaking into
 `getSell/BuyOperations()` when they should be disabled

---
 .../src/quote_consumers/utils.ts              |  5 +--
 .../utils/market_operation_utils/orders.ts    |  2 +-
 .../sampler_operations.ts                     | 18 ++++++----
 .../src/utils/quote_simulation.ts             | 33 ++++++++++++-------
 .../test/quote_simulation_test.ts             |  4 +--
 5 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/packages/asset-swapper/src/quote_consumers/utils.ts b/packages/asset-swapper/src/quote_consumers/utils.ts
index c805e79939..9f5d40b871 100644
--- a/packages/asset-swapper/src/quote_consumers/utils.ts
+++ b/packages/asset-swapper/src/quote_consumers/utils.ts
@@ -1,9 +1,9 @@
 import { BigNumber } from '@0x/utils';
 import * as _ from 'lodash';
 
+import { constants } from '../constants';
 import { MarketOperation, SwapQuote } from '../types';
 import { ERC20BridgeSource } from '../utils/market_operation_utils/types';
-import { constants } from '../constants';
 
 const { ZERO_AMOUNT } = constants;
 
@@ -19,6 +19,7 @@ export function getSwapMinBuyAmount(quote: SwapQuote): BigNumber {
     }
     // Infer the allowed maker asset slippage from the orders.
     const totalOrderMakerAssetAmount = BigNumber.sum(...quote.orders.map(o => o.fillableMakerAssetAmount));
+    // tslint:disable: prefer-conditional-expression
     let totalFillMakerAssetAmount = ZERO_AMOUNT;
     for (const o of quote.orders) {
         if (o.fills.length === 0 || o.fills[0].source === ERC20BridgeSource.Native) {
@@ -28,11 +29,11 @@ export function getSwapMinBuyAmount(quote: SwapQuote): BigNumber {
             totalFillMakerAssetAmount = totalFillMakerAssetAmount.plus(BigNumber.sum(...o.fills.map(f => f.output)));
         }
     }
+    // tslint:enable: prefer-conditional-expression
     if (totalOrderMakerAssetAmount.eq(totalFillMakerAssetAmount)) {
         // No slippage allowed across all orders.
         return quote.bestCaseQuoteInfo.makerAssetAmount;
     }
     const slipRatio = totalOrderMakerAssetAmount.div(totalFillMakerAssetAmount);
-    console.log(slipRatio);
     return quote.bestCaseQuoteInfo.makerAssetAmount.times(slipRatio).integerValue(BigNumber.ROUND_DOWN);
 }
diff --git a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts
index 804dcfa3a6..efcb699a7a 100644
--- a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts
+++ b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts
@@ -291,7 +291,7 @@ function createBridgeOrder(
             makerAssetData = assetDataUtils.encodeERC20BridgeAssetData(
                 makerToken,
                 bridgeAddress,
-                createBalancerBridgeData(takerToken, makerToken),
+                createBalancerBridgeData(takerToken, balancerFillData.poolAddress),
             );
             break;
         case ERC20BridgeSource.Bancor:
diff --git a/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts b/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts
index 62bcaeba2b..9ef979049c 100644
--- a/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts
+++ b/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts
@@ -798,9 +798,8 @@ export class SamplerOperations {
         liquidityProviderRegistryAddress?: string,
         multiBridgeAddress?: string,
     ): BatchedOperation<DexSample[][]> {
-        const _sources = BATCH_SOURCE_FILTERS.getAllowed(sources);
         const subOps = this._getSellQuoteOperations(
-            _sources,
+            sources,
             makerToken,
             takerToken,
             takerFillAmounts,
@@ -839,9 +838,8 @@ export class SamplerOperations {
         wethAddress: string,
         liquidityProviderRegistryAddress?: string,
     ): BatchedOperation<DexSample[][]> {
-        const _sources = BATCH_SOURCE_FILTERS.getAllowed(sources);
         const subOps = this._getBuyQuoteOperations(
-            _sources,
+            sources,
             makerToken,
             takerToken,
             makerFillAmounts,
@@ -880,8 +878,13 @@ export class SamplerOperations {
         liquidityProviderRegistryAddress?: string,
         multiBridgeAddress?: string,
     ): SourceQuoteOperation[] {
+        const _sources = BATCH_SOURCE_FILTERS.exclude(
+            liquidityProviderRegistryAddress ? [] : [ERC20BridgeSource.LiquidityProvider],
+        )
+            .exclude(multiBridgeAddress ? [] : [ERC20BridgeSource.MultiBridge])
+            .getAllowed(sources);
         return _.flatten(
-            sources.map(
+            _sources.map(
                 (source): SourceQuoteOperation | SourceQuoteOperation[] => {
                     switch (source) {
                         case ERC20BridgeSource.Eth2Dai:
@@ -984,8 +987,11 @@ export class SamplerOperations {
         wethAddress: string,
         liquidityProviderRegistryAddress?: string,
     ): SourceQuoteOperation[] {
+        const _sources = BATCH_SOURCE_FILTERS.exclude(
+            liquidityProviderRegistryAddress ? [] : [ERC20BridgeSource.LiquidityProvider],
+        ).getAllowed(sources);
         return _.flatten(
-            sources.map(
+            _sources.map(
                 (source): SourceQuoteOperation | SourceQuoteOperation[] => {
                     switch (source) {
                         case ERC20BridgeSource.Eth2Dai:
diff --git a/packages/asset-swapper/src/utils/quote_simulation.ts b/packages/asset-swapper/src/utils/quote_simulation.ts
index 2e0b2240bb..5e46046e79 100644
--- a/packages/asset-swapper/src/utils/quote_simulation.ts
+++ b/packages/asset-swapper/src/utils/quote_simulation.ts
@@ -3,7 +3,7 @@ import { BigNumber } from '@0x/utils';
 import { constants } from '../constants';
 import { MarketOperation } from '../types';
 
-import { CollapsedFill, FeeSchedule, OptimizedMarketOrder } from './market_operation_utils/types';
+import { CollapsedFill, ERC20BridgeSource, FeeSchedule, OptimizedMarketOrder } from './market_operation_utils/types';
 import { isOrderTakerFeePayableWithMakerAsset, isOrderTakerFeePayableWithTakerAsset } from './utils';
 
 const { PROTOCOL_FEE_MULTIPLIER, ZERO_AMOUNT } = constants;
@@ -261,16 +261,27 @@ function createWorstCaseFillOrderCalls(quoteInfo: QuoteFillInfo): QuoteFillOrder
 
 // Apply order slippage to its fill paths.
 function getSlippedOrderFills(order: OptimizedMarketOrder, side: MarketOperation): CollapsedFill[] {
-    const totalInput = BigNumber.sum(...order.fills.map(f => f.input));
-    const totalOutput = BigNumber.sum(...order.fills.map(f => f.output));
-    const inputScaling =
-        side === MarketOperation.Sell
-            ? order.fillableTakerAssetAmount.div(totalInput)
-            : order.fillableMakerAssetAmount.div(totalInput);
-    const outputScaling =
-        side === MarketOperation.Sell
-            ? order.fillableMakerAssetAmount.div(totalOutput)
-            : order.fillableTakerAssetAmount.div(totalOutput);
+    // Infer the slippage from the order amounts vs fill amounts.
+    let inputScaling: BigNumber;
+    let outputScaling: BigNumber;
+    const source = order.fills[0].source;
+    if (source === ERC20BridgeSource.Native) {
+        // Native orders do not have slippage applied to them.
+        inputScaling = new BigNumber(1);
+        outputScaling = new BigNumber(1);
+    } else {
+        if (side === MarketOperation.Sell) {
+            const totalFillableTakerAssetAmount = BigNumber.sum(...order.fills.map(f => f.input));
+            const totalFillableMakerAssetAmount = BigNumber.sum(...order.fills.map(f => f.output));
+            inputScaling = order.fillableTakerAssetAmount.div(totalFillableTakerAssetAmount);
+            outputScaling = order.fillableMakerAssetAmount.div(totalFillableMakerAssetAmount);
+        } else {
+            const totalFillableTakerAssetAmount = BigNumber.sum(...order.fills.map(f => f.output));
+            const totalFillableMakerAssetAmount = BigNumber.sum(...order.fills.map(f => f.input));
+            inputScaling = order.fillableMakerAssetAmount.div(totalFillableMakerAssetAmount);
+            outputScaling = order.fillableTakerAssetAmount.div(totalFillableTakerAssetAmount);
+        }
+    }
     return order.fills.map(f => ({
         ...f,
         input: f.input.times(inputScaling),
diff --git a/packages/asset-swapper/test/quote_simulation_test.ts b/packages/asset-swapper/test/quote_simulation_test.ts
index 07493b47d6..ae106b30df 100644
--- a/packages/asset-swapper/test/quote_simulation_test.ts
+++ b/packages/asset-swapper/test/quote_simulation_test.ts
@@ -22,7 +22,7 @@ describe('quote_simulation tests', async () => {
     const TAKER_TOKEN = randomAddress();
     const DEFAULT_MAKER_ASSET_DATA = assetDataUtils.encodeERC20AssetData(MAKER_TOKEN);
     const DEFAULT_TAKER_ASSET_DATA = assetDataUtils.encodeERC20AssetData(TAKER_TOKEN);
-    const GAS_SCHEDULE = { [ERC20BridgeSource.Native]: _.constant(1) };
+    const GAS_SCHEDULE = { [ERC20BridgeSource.Uniswap]: _.constant(1) };
 
     // Check if two numbers are within `maxError` error rate within each other.
     function assertRoughlyEquals(n1: BigNumber, n2: BigNumber, maxError: BigNumber | number = 1e-10): void {
@@ -164,7 +164,7 @@ describe('quote_simulation tests', async () => {
             const subFillOutputs = subdivideAmount(outputs[i], count);
             return {
                 sourcePathId: nativeSourcePathId,
-                source: ERC20BridgeSource.Native,
+                source: ERC20BridgeSource.Uniswap,
                 input: inputs[i],
                 output: outputs[i],
                 subFills: _.times(count, j => ({

From 4672c72fefda414e294b7c384897906cfc69a38c Mon Sep 17 00:00:00 2001
From: Lawrence Forman <me@merklejerk.com>
Date: Thu, 24 Sep 2020 10:54:14 -0400
Subject: [PATCH 8/8] `@0x/asset-swapper`: compute max quote slippage from the
 first non-native order in `getSwapMinBuyAmount()`

---
 .../src/quote_consumers/utils.ts              | 22 +++++++------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/packages/asset-swapper/src/quote_consumers/utils.ts b/packages/asset-swapper/src/quote_consumers/utils.ts
index 9f5d40b871..67b2677acf 100644
--- a/packages/asset-swapper/src/quote_consumers/utils.ts
+++ b/packages/asset-swapper/src/quote_consumers/utils.ts
@@ -1,12 +1,9 @@
 import { BigNumber } from '@0x/utils';
 import * as _ from 'lodash';
 
-import { constants } from '../constants';
 import { MarketOperation, SwapQuote } from '../types';
 import { ERC20BridgeSource } from '../utils/market_operation_utils/types';
 
-const { ZERO_AMOUNT } = constants;
-
 /**
  * Compute the mminimum buy token amount for market operations by inferring
  * the slippage from the orders in a quote. We cannot rely on
@@ -17,23 +14,20 @@ export function getSwapMinBuyAmount(quote: SwapQuote): BigNumber {
     if (quote.type === MarketOperation.Buy || quote.isTwoHop) {
         return quote.worstCaseQuoteInfo.makerAssetAmount;
     }
-    // Infer the allowed maker asset slippage from the orders.
-    const totalOrderMakerAssetAmount = BigNumber.sum(...quote.orders.map(o => o.fillableMakerAssetAmount));
-    // tslint:disable: prefer-conditional-expression
-    let totalFillMakerAssetAmount = ZERO_AMOUNT;
+    let slipRatio = new BigNumber(1);
+    // Infer the allowed maker asset slippage from any non-native order.
     for (const o of quote.orders) {
         if (o.fills.length === 0 || o.fills[0].source === ERC20BridgeSource.Native) {
-            // No slippage on natuve orders.
-            totalFillMakerAssetAmount = totalFillMakerAssetAmount.plus(o.fillableMakerAssetAmount);
-        } else {
-            totalFillMakerAssetAmount = totalFillMakerAssetAmount.plus(BigNumber.sum(...o.fills.map(f => f.output)));
+            // No slippage on native orders.
+            continue;
         }
+        const totalFillMakerAssetAmount = BigNumber.sum(...o.fills.map(f => f.output));
+        slipRatio = o.fillableMakerAssetAmount.div(totalFillMakerAssetAmount);
+        break;
     }
-    // tslint:enable: prefer-conditional-expression
-    if (totalOrderMakerAssetAmount.eq(totalFillMakerAssetAmount)) {
+    if (slipRatio.gte(1)) {
         // No slippage allowed across all orders.
         return quote.bestCaseQuoteInfo.makerAssetAmount;
     }
-    const slipRatio = totalOrderMakerAssetAmount.div(totalFillMakerAssetAmount);
     return quote.bestCaseQuoteInfo.makerAssetAmount.times(slipRatio).integerValue(BigNumber.ROUND_DOWN);
 }