Merge pull request #2711 from 0xProject/fix/balancer-sampling

Handle max in/out ratio reverts in Balancer sampling functions
This commit is contained in:
Lawrence Forman 2020-09-25 00:45:47 -04:00 committed by GitHub
commit 66e2d93e9c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 134 additions and 26 deletions

View File

@ -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
}
]
},

View File

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

View File

@ -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,

View File

@ -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,

View File

@ -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);
}

View File

@ -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);

View File

@ -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:

View File

@ -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),

View File

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

View File

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