Merge branch 'development' of github.com:0xProject/0x-monorepo into refactor_rfq_request_positioning

packages/asset-swapper/CHANGELOG.json

@@ -125,6 +125,14 @@
             {
                 "note": "Pass back fillData from quote reporter",
                 "pr": 2702
+            },
+            {
+                "note": "Fix Balancer sampling",
+                "pr": 2711
+            },
+            {
+                "note": "Respect max slippage in EP consumer",
+                "pr": 2712
             }
         ]
     },

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;
+    }
 }

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,

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 { getSwapMinBuyAmount } 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;
+        let minBuyAmount = getSwapMinBuyAmount(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(),
@@ -210,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) {
@@ -225,7 +230,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,

packages/asset-swapper/src/quote_consumers/utils.ts

@@ -0,0 +1,33 @@
+import { BigNumber } from '@0x/utils';
+import * as _ from 'lodash';
+
+import { MarketOperation, SwapQuote } from '../types';
+import { ERC20BridgeSource } from '../utils/market_operation_utils/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 getSwapMinBuyAmount(quote: SwapQuote): BigNumber {
+    if (quote.type === MarketOperation.Buy || quote.isTwoHop) {
+        return quote.worstCaseQuoteInfo.makerAssetAmount;
+    }
+    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 native orders.
+            continue;
+        }
+        const totalFillMakerAssetAmount = BigNumber.sum(...o.fills.map(f => f.output));
+        slipRatio = o.fillableMakerAssetAmount.div(totalFillMakerAssetAmount);
+        break;
+    }
+    if (slipRatio.gte(1)) {
+        // No slippage allowed across all orders.
+        return quote.bestCaseQuoteInfo.makerAssetAmount;
+    }
+    return quote.bestCaseQuoteInfo.makerAssetAmount.times(slipRatio).integerValue(BigNumber.ROUND_DOWN);
+}

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)) {
+    if (makerFillAmount.isGreaterThan(bmath.bmul(pool.balanceOut, bmath.MAX_OUT_RATIO))) {
-        return new BigNumber(0);
+        return ZERO_AMOUNT;
     }
     const weightRatio = pool.weightOut.dividedBy(pool.weightIn);
     const diff = pool.balanceOut.minus(makerFillAmount);

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:

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));
+    // Infer the slippage from the order amounts vs fill amounts.
-    const totalOutput = BigNumber.sum(...order.fills.map(f => f.output));
+    let inputScaling: BigNumber;
-    const inputScaling =
+    let outputScaling: BigNumber;
-        side === MarketOperation.Sell
+    const source = order.fills[0].source;
-            ? order.fillableTakerAssetAmount.div(totalInput)
+    if (source === ERC20BridgeSource.Native) {
-            : order.fillableMakerAssetAmount.div(totalInput);
+        // Native orders do not have slippage applied to them.
-    const outputScaling =
+        inputScaling = new BigNumber(1);
-        side === MarketOperation.Sell
+        outputScaling = new BigNumber(1);
-            ? order.fillableMakerAssetAmount.div(totalOutput)
+    } else {
-            : order.fillableTakerAssetAmount.div(totalOutput);
+        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),

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() ===

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 => ({