fix LP and MB sources leaking into getSell/BuyOperations() when they should be disabled

This commit is contained in:
Lawrence Forman 2020-09-23 21:58:45 -04:00
parent d7de191947
commit ac6b03cd4a
5 changed files with 40 additions and 22 deletions

View File

@ -1,9 +1,9 @@
import { BigNumber } from '@0x/utils'; import { BigNumber } from '@0x/utils';
import * as _ from 'lodash'; import * as _ from 'lodash';
import { constants } from '../constants';
import { MarketOperation, SwapQuote } from '../types'; import { MarketOperation, SwapQuote } from '../types';
import { ERC20BridgeSource } from '../utils/market_operation_utils/types'; import { ERC20BridgeSource } from '../utils/market_operation_utils/types';
import { constants } from '../constants';
const { ZERO_AMOUNT } = constants; const { ZERO_AMOUNT } = constants;
@ -19,6 +19,7 @@ export function getSwapMinBuyAmount(quote: SwapQuote): BigNumber {
} }
// Infer the allowed maker asset slippage from the orders. // Infer the allowed maker asset slippage from the orders.
const totalOrderMakerAssetAmount = BigNumber.sum(...quote.orders.map(o => o.fillableMakerAssetAmount)); const totalOrderMakerAssetAmount = BigNumber.sum(...quote.orders.map(o => o.fillableMakerAssetAmount));
// tslint:disable: prefer-conditional-expression
let totalFillMakerAssetAmount = ZERO_AMOUNT; let totalFillMakerAssetAmount = ZERO_AMOUNT;
for (const o of quote.orders) { for (const o of quote.orders) {
if (o.fills.length === 0 || o.fills[0].source === ERC20BridgeSource.Native) { 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))); totalFillMakerAssetAmount = totalFillMakerAssetAmount.plus(BigNumber.sum(...o.fills.map(f => f.output)));
} }
} }
// tslint:enable: prefer-conditional-expression
if (totalOrderMakerAssetAmount.eq(totalFillMakerAssetAmount)) { if (totalOrderMakerAssetAmount.eq(totalFillMakerAssetAmount)) {
// No slippage allowed across all orders. // No slippage allowed across all orders.
return quote.bestCaseQuoteInfo.makerAssetAmount; return quote.bestCaseQuoteInfo.makerAssetAmount;
} }
const slipRatio = totalOrderMakerAssetAmount.div(totalFillMakerAssetAmount); const slipRatio = totalOrderMakerAssetAmount.div(totalFillMakerAssetAmount);
console.log(slipRatio);
return quote.bestCaseQuoteInfo.makerAssetAmount.times(slipRatio).integerValue(BigNumber.ROUND_DOWN); return quote.bestCaseQuoteInfo.makerAssetAmount.times(slipRatio).integerValue(BigNumber.ROUND_DOWN);
} }

View File

@ -291,7 +291,7 @@ function createBridgeOrder(
makerAssetData = assetDataUtils.encodeERC20BridgeAssetData( makerAssetData = assetDataUtils.encodeERC20BridgeAssetData(
makerToken, makerToken,
bridgeAddress, bridgeAddress,
createBalancerBridgeData(takerToken, makerToken), createBalancerBridgeData(takerToken, balancerFillData.poolAddress),
); );
break; break;
case ERC20BridgeSource.Bancor: case ERC20BridgeSource.Bancor:

View File

@ -798,9 +798,8 @@ export class SamplerOperations {
liquidityProviderRegistryAddress?: string, liquidityProviderRegistryAddress?: string,
multiBridgeAddress?: string, multiBridgeAddress?: string,
): BatchedOperation<DexSample[][]> { ): BatchedOperation<DexSample[][]> {
const _sources = BATCH_SOURCE_FILTERS.getAllowed(sources);
const subOps = this._getSellQuoteOperations( const subOps = this._getSellQuoteOperations(
_sources, sources,
makerToken, makerToken,
takerToken, takerToken,
takerFillAmounts, takerFillAmounts,
@ -839,9 +838,8 @@ export class SamplerOperations {
wethAddress: string, wethAddress: string,
liquidityProviderRegistryAddress?: string, liquidityProviderRegistryAddress?: string,
): BatchedOperation<DexSample[][]> { ): BatchedOperation<DexSample[][]> {
const _sources = BATCH_SOURCE_FILTERS.getAllowed(sources);
const subOps = this._getBuyQuoteOperations( const subOps = this._getBuyQuoteOperations(
_sources, sources,
makerToken, makerToken,
takerToken, takerToken,
makerFillAmounts, makerFillAmounts,
@ -880,8 +878,13 @@ export class SamplerOperations {
liquidityProviderRegistryAddress?: string, liquidityProviderRegistryAddress?: string,
multiBridgeAddress?: string, multiBridgeAddress?: string,
): SourceQuoteOperation[] { ): SourceQuoteOperation[] {
const _sources = BATCH_SOURCE_FILTERS.exclude(
liquidityProviderRegistryAddress ? [] : [ERC20BridgeSource.LiquidityProvider],
)
.exclude(multiBridgeAddress ? [] : [ERC20BridgeSource.MultiBridge])
.getAllowed(sources);
return _.flatten( return _.flatten(
sources.map( _sources.map(
(source): SourceQuoteOperation | SourceQuoteOperation[] => { (source): SourceQuoteOperation | SourceQuoteOperation[] => {
switch (source) { switch (source) {
case ERC20BridgeSource.Eth2Dai: case ERC20BridgeSource.Eth2Dai:
@ -984,8 +987,11 @@ export class SamplerOperations {
wethAddress: string, wethAddress: string,
liquidityProviderRegistryAddress?: string, liquidityProviderRegistryAddress?: string,
): SourceQuoteOperation[] { ): SourceQuoteOperation[] {
const _sources = BATCH_SOURCE_FILTERS.exclude(
liquidityProviderRegistryAddress ? [] : [ERC20BridgeSource.LiquidityProvider],
).getAllowed(sources);
return _.flatten( return _.flatten(
sources.map( _sources.map(
(source): SourceQuoteOperation | SourceQuoteOperation[] => { (source): SourceQuoteOperation | SourceQuoteOperation[] => {
switch (source) { switch (source) {
case ERC20BridgeSource.Eth2Dai: case ERC20BridgeSource.Eth2Dai:

View File

@ -3,7 +3,7 @@ import { BigNumber } from '@0x/utils';
import { constants } from '../constants'; import { constants } from '../constants';
import { MarketOperation } from '../types'; 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'; import { isOrderTakerFeePayableWithMakerAsset, isOrderTakerFeePayableWithTakerAsset } from './utils';
const { PROTOCOL_FEE_MULTIPLIER, ZERO_AMOUNT } = constants; const { PROTOCOL_FEE_MULTIPLIER, ZERO_AMOUNT } = constants;
@ -261,16 +261,27 @@ function createWorstCaseFillOrderCalls(quoteInfo: QuoteFillInfo): QuoteFillOrder
// Apply order slippage to its fill paths. // Apply order slippage to its fill paths.
function getSlippedOrderFills(order: OptimizedMarketOrder, side: MarketOperation): CollapsedFill[] { 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 => ({ return order.fills.map(f => ({
...f, ...f,
input: f.input.times(inputScaling), input: f.input.times(inputScaling),

View File

@ -22,7 +22,7 @@ describe('quote_simulation tests', async () => {
const TAKER_TOKEN = randomAddress(); const TAKER_TOKEN = randomAddress();
const DEFAULT_MAKER_ASSET_DATA = assetDataUtils.encodeERC20AssetData(MAKER_TOKEN); const DEFAULT_MAKER_ASSET_DATA = assetDataUtils.encodeERC20AssetData(MAKER_TOKEN);
const DEFAULT_TAKER_ASSET_DATA = assetDataUtils.encodeERC20AssetData(TAKER_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. // Check if two numbers are within `maxError` error rate within each other.
function assertRoughlyEquals(n1: BigNumber, n2: BigNumber, maxError: BigNumber | number = 1e-10): void { 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); const subFillOutputs = subdivideAmount(outputs[i], count);
return { return {
sourcePathId: nativeSourcePathId, sourcePathId: nativeSourcePathId,
source: ERC20BridgeSource.Native, source: ERC20BridgeSource.Uniswap,
input: inputs[i], input: inputs[i],
output: outputs[i], output: outputs[i],
subFills: _.times(count, j => ({ subFills: _.times(count, j => ({