From 6ccadcb928798ee1a19380fd11c5cebc0bb57c03 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Thu, 5 Mar 2020 14:23:54 -0500 Subject: [PATCH 01/14] `@0x/asset-swapper`: Refactor market op utils. `@0x/asset-swapper`: add fallback orders. `@0x/asset-swapper`: Remove `noConflicts` and `dustThreshold` options. `@0x/asset-swapper`: Add `allowFallback` option. --- packages/asset-swapper/CHANGELOG.json | 12 + packages/asset-swapper/src/constants.ts | 48 +- packages/asset-swapper/src/swap_quoter.ts | 13 +- packages/asset-swapper/src/utils/assert.ts | 17 +- .../src/utils/calculate_liquidity.ts | 6 +- .../src/utils/fillable_amounts_utils.ts | 6 +- .../utils/market_operation_utils/constants.ts | 68 ++- .../dummy_order_utils.ts | 32 - .../market_operation_utils/fill_optimizer.ts | 181 ------ .../src/utils/market_operation_utils/fills.ts | 237 ++++++++ .../src/utils/market_operation_utils/index.ts | 570 +++++------------- .../utils/market_operation_utils/orders.ts | 252 ++++++++ .../market_operation_utils/path_optimizer.ts | 155 +++++ .../src/utils/market_operation_utils/types.ts | 47 +- .../src/utils/order_prune_utils.ts | 6 +- .../asset-swapper/src/utils/sorting_utils.ts | 6 +- .../src/utils/swap_quote_calculator.ts | 18 +- .../src/utils/swap_quote_consumer_utils.ts | 4 +- packages/asset-swapper/src/utils/utils.ts | 206 ++++--- .../test/market_operation_utils_test.ts | 252 ++++---- .../test/swap_quote_calculator_test.ts | 4 +- packages/asset-swapper/test/utils_test.ts | 34 +- 22 files changed, 1159 insertions(+), 1015 deletions(-) delete mode 100644 packages/asset-swapper/src/utils/market_operation_utils/dummy_order_utils.ts delete mode 100644 packages/asset-swapper/src/utils/market_operation_utils/fill_optimizer.ts create mode 100644 packages/asset-swapper/src/utils/market_operation_utils/fills.ts create mode 100644 packages/asset-swapper/src/utils/market_operation_utils/orders.ts create mode 100644 packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index da374599e8..b91116928f 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -5,6 +5,18 @@ { "note": "Add support for private liquidity providers", "pr": 2505 + }, + { + "note": "Big refactor of market operation utils", + "pr": 2513 + }, + { + "note": "Revamp fill optimization algorithm", + "pr": 2513 + }, + { + "note": "Add fallback orders to quotes", + "pr": 2513 } ] }, diff --git a/packages/asset-swapper/src/constants.ts b/packages/asset-swapper/src/constants.ts index cc94a15535..0e50b89bf0 100644 --- a/packages/asset-swapper/src/constants.ts +++ b/packages/asset-swapper/src/constants.ts @@ -11,8 +11,7 @@ import { SwapQuoterOpts, } from './types'; -import { constants as marketOperationUtilConstants } from './utils/market_operation_utils/constants'; -import { ERC20BridgeSource } from './utils/market_operation_utils/types'; +import { DEFAULT_GET_MARKET_ORDERS_OPTS } from './utils/market_operation_utils/constants'; const ETH_GAS_STATION_API_BASE_URL = 'https://ethgasstation.info'; const NULL_BYTES = '0x'; @@ -43,7 +42,7 @@ const DEFAULT_SWAP_QUOTER_OPTS: SwapQuoterOpts = { orderRefreshIntervalMs: 10000, // 10 seconds }, ...DEFAULT_ORDER_PRUNER_OPTS, - samplerGasLimit: 59e6, + samplerGasLimit: 250e6, }; const DEFAULT_FORWARDER_EXTENSION_CONTRACT_OPTS: ForwarderExtensionContractOpts = { @@ -60,47 +59,9 @@ const DEFAULT_FORWARDER_SWAP_QUOTE_EXECUTE_OPTS: SwapQuoteExecutionOpts = DEFAUL const DEFAULT_SWAP_QUOTE_REQUEST_OPTS: SwapQuoteRequestOpts = { ...{ - slippagePercentage: 0.2, // 20% slippage protection, - }, - ...marketOperationUtilConstants.DEFAULT_GET_MARKET_ORDERS_OPTS, -}; - -// Mainnet Curve configuration -const DEFAULT_CURVE_OPTS: { [source: string]: { version: number; curveAddress: string; tokens: string[] } } = { - [ERC20BridgeSource.CurveUsdcDai]: { - version: 1, - curveAddress: '0xa2b47e3d5c44877cca798226b7b8118f9bfb7a56', - tokens: ['0x6b175474e89094c44da98b954eedeac495271d0f', '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'], - }, - [ERC20BridgeSource.CurveUsdcDaiUsdt]: { - version: 1, - curveAddress: '0x52ea46506b9cc5ef470c5bf89f17dc28bb35d85c', - tokens: [ - '0x6b175474e89094c44da98b954eedeac495271d0f', - '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', - '0xdac17f958d2ee523a2206206994597c13d831ec7', - ], - }, - [ERC20BridgeSource.CurveUsdcDaiUsdtTusd]: { - version: 1, - curveAddress: '0x45f783cce6b7ff23b2ab2d70e416cdb7d6055f51', - tokens: [ - '0x6b175474e89094c44da98b954eedeac495271d0f', - '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', - '0xdac17f958d2ee523a2206206994597c13d831ec7', - '0x0000000000085d4780b73119b644ae5ecd22b376', - ], - }, - [ERC20BridgeSource.CurveUsdcDaiUsdtBusd]: { - version: 1, - curveAddress: '0x79a8c46dea5ada233abaffd40f3a0a2b1e5a4f27', - tokens: [ - '0x6b175474e89094c44da98b954eedeac495271d0f', - '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', - '0xdac17f958d2ee523a2206206994597c13d831ec7', - '0x4fabb145d64652a948d72533023f6e7a623c7c53', - ], + slippagePercentage: 0.05, // 5% slippage protection, }, + ...DEFAULT_GET_MARKET_ORDERS_OPTS, }; export const constants = { @@ -123,5 +84,4 @@ export const constants = { PROTOCOL_FEE_UTILS_POLLING_INTERVAL_IN_MS, MARKET_UTILS_AMOUNT_BUFFER_PERCENTAGE, BRIDGE_ASSET_DATA_PREFIX: '0xdc1600f3', - DEFAULT_CURVE_OPTS, }; diff --git a/packages/asset-swapper/src/swap_quoter.ts b/packages/asset-swapper/src/swap_quoter.ts index 6c4ad4cb88..f98818457a 100644 --- a/packages/asset-swapper/src/swap_quoter.ts +++ b/packages/asset-swapper/src/swap_quoter.ts @@ -21,9 +21,8 @@ import { } from './types'; import { assert } from './utils/assert'; import { calculateLiquidity } from './utils/calculate_liquidity'; -import { MarketOperationUtils } from './utils/market_operation_utils'; -import { dummyOrderUtils } from './utils/market_operation_utils/dummy_order_utils'; -import { DexOrderSampler } from './utils/market_operation_utils/sampler'; +import { DexOrderSampler, MarketOperationUtils } from './utils/market_operation_utils'; +import { createDummyOrderForSampler } from './utils/market_operation_utils/orders'; import { orderPrunerUtils } from './utils/order_prune_utils'; import { OrderStateUtils } from './utils/order_state_utils'; import { ProtocolFeeUtils } from './utils/protocol_fee_utils'; @@ -264,7 +263,7 @@ export class SwapQuoter { ); if (prunedOrders.length === 0) { return [ - dummyOrderUtils.createDummyOrderForSampler( + createDummyOrderForSampler( makerAssetDatas[i], takerAssetData, this._contractAddresses.uniswapBridge, @@ -537,11 +536,7 @@ export class SwapQuoter { // if no native orders, pass in a dummy order for the sampler to have required metadata for sampling if (prunedOrders.length === 0) { prunedOrders = [ - dummyOrderUtils.createDummyOrderForSampler( - makerAssetData, - takerAssetData, - this._contractAddresses.uniswapBridge, - ), + createDummyOrderForSampler(makerAssetData, takerAssetData, this._contractAddresses.uniswapBridge), ]; } diff --git a/packages/asset-swapper/src/utils/assert.ts b/packages/asset-swapper/src/utils/assert.ts index 92d62c1183..73303fad85 100644 --- a/packages/asset-swapper/src/utils/assert.ts +++ b/packages/asset-swapper/src/utils/assert.ts @@ -6,7 +6,12 @@ import * as _ from 'lodash'; import { MarketOperation, OrderProviderRequest, SwapQuote, SwapQuoteInfo } from '../types'; -import { utils } from './utils'; +import { + isAssetDataEquivalent, + isExactAssetData, + isOrderTakerFeePayableWithMakerAsset, + isOrderTakerFeePayableWithTakerAsset, +} from './utils'; export const assert = { ...sharedAssert, @@ -36,13 +41,13 @@ export const assert = { ): void { _.every(orders, (order: SignedOrder, index: number) => { assert.assert( - utils.isAssetDataEquivalent(takerAssetData, order.takerAssetData), + isAssetDataEquivalent(takerAssetData, order.takerAssetData), `Expected ${variableName}[${index}].takerAssetData to be ${takerAssetData} but found ${ order.takerAssetData }`, ); assert.assert( - utils.isAssetDataEquivalent(makerAssetData, order.makerAssetData), + isAssetDataEquivalent(makerAssetData, order.makerAssetData), `Expected ${variableName}[${index}].makerAssetData to be ${makerAssetData} but found ${ order.makerAssetData }`, @@ -53,8 +58,8 @@ export const assert = { _.every(orders, (order: T, index: number) => { assert.assert( order.takerFee.isZero() || - utils.isOrderTakerFeePayableWithTakerAsset(order) || - utils.isOrderTakerFeePayableWithMakerAsset(order), + isOrderTakerFeePayableWithTakerAsset(order) || + isOrderTakerFeePayableWithMakerAsset(order), `Expected ${variableName}[${index}].takerFeeAssetData to be ${order.makerAssetData} or ${ order.takerAssetData } but found ${order.takerFeeAssetData}`, @@ -72,7 +77,7 @@ export const assert = { }, isValidForwarderSignedOrder(variableName: string, order: SignedOrder, wethAssetData: string): void { assert.assert( - utils.isExactAssetData(order.takerAssetData, wethAssetData), + isExactAssetData(order.takerAssetData, wethAssetData), `Expected ${variableName} to have takerAssetData set as ${wethAssetData}, but is ${order.takerAssetData}`, ); }, diff --git a/packages/asset-swapper/src/utils/calculate_liquidity.ts b/packages/asset-swapper/src/utils/calculate_liquidity.ts index 331d4843e1..4a0711d55f 100644 --- a/packages/asset-swapper/src/utils/calculate_liquidity.ts +++ b/packages/asset-swapper/src/utils/calculate_liquidity.ts @@ -2,17 +2,17 @@ import { BigNumber } from '@0x/utils'; import { LiquidityForTakerMakerAssetDataPair, SignedOrderWithFillableAmounts } from '../types'; -import { utils } from './utils'; +import { isOrderTakerFeePayableWithMakerAsset, isOrderTakerFeePayableWithTakerAsset } from './utils'; export const calculateLiquidity = ( prunedOrders: SignedOrderWithFillableAmounts[], ): LiquidityForTakerMakerAssetDataPair => { const liquidityInBigNumbers = prunedOrders.reduce( (acc, order) => { - const fillableMakerAssetAmount = utils.isOrderTakerFeePayableWithMakerAsset(order) + const fillableMakerAssetAmount = isOrderTakerFeePayableWithMakerAsset(order) ? order.fillableMakerAssetAmount.minus(order.fillableTakerFeeAmount) : order.fillableMakerAssetAmount; - const fillableTakerAssetAmount = utils.isOrderTakerFeePayableWithTakerAsset(order) + const fillableTakerAssetAmount = isOrderTakerFeePayableWithTakerAsset(order) ? order.fillableTakerAssetAmount.plus(order.fillableTakerFeeAmount) : order.fillableTakerAssetAmount; return { diff --git a/packages/asset-swapper/src/utils/fillable_amounts_utils.ts b/packages/asset-swapper/src/utils/fillable_amounts_utils.ts index 0b44001aed..9f2e8308e6 100644 --- a/packages/asset-swapper/src/utils/fillable_amounts_utils.ts +++ b/packages/asset-swapper/src/utils/fillable_amounts_utils.ts @@ -3,18 +3,18 @@ import * as _ from 'lodash'; import { SignedOrderWithFillableAmounts } from '../types'; -import { utils } from './utils'; +import { isOrderTakerFeePayableWithMakerAsset, isOrderTakerFeePayableWithTakerAsset } from './utils'; export const fillableAmountsUtils = { getTakerAssetAmountSwappedAfterOrderFees(order: SignedOrderWithFillableAmounts): BigNumber { - if (utils.isOrderTakerFeePayableWithTakerAsset(order)) { + if (isOrderTakerFeePayableWithTakerAsset(order)) { return order.fillableTakerAssetAmount.plus(order.fillableTakerFeeAmount); } else { return order.fillableTakerAssetAmount; } }, getMakerAssetAmountSwappedAfterOrderFees(order: SignedOrderWithFillableAmounts): BigNumber { - if (utils.isOrderTakerFeePayableWithMakerAsset(order)) { + if (isOrderTakerFeePayableWithMakerAsset(order)) { return order.fillableMakerAssetAmount.minus(order.fillableTakerFeeAmount); } else { return order.fillableMakerAssetAmount; diff --git a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts index 8b82ccdc6f..763e34ac1a 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts @@ -2,7 +2,7 @@ import { BigNumber } from '@0x/utils'; import { ERC20BridgeSource, GetMarketOrdersOpts } from './types'; -const INFINITE_TIMESTAMP_SEC = new BigNumber(2524604400); +// tslint:disable: custom-no-magic-numbers /** * Valid sources for market sell. @@ -25,14 +25,12 @@ export const BUY_SOURCES = [ERC20BridgeSource.Uniswap, ERC20BridgeSource.Eth2Dai export const DEFAULT_GET_MARKET_ORDERS_OPTS: GetMarketOrdersOpts = { // tslint:disable-next-line: custom-no-magic-numbers - runLimit: 2 ** 15, excludedSources: [], - bridgeSlippage: 0.0005, - dustFractionThreshold: 0.0025, - numSamples: 13, - noConflicts: true, + bridgeSlippage: 0.005, + numSamples: 20, sampleDistributionBase: 1.05, fees: {}, + allowFallback: true, }; /** @@ -40,13 +38,53 @@ export const DEFAULT_GET_MARKET_ORDERS_OPTS: GetMarketOrdersOpts = { */ export const FEE_QUOTE_SOURCES = SELL_SOURCES; -export const constants = { - INFINITE_TIMESTAMP_SEC, - SELL_SOURCES, - BUY_SOURCES, - DEFAULT_GET_MARKET_ORDERS_OPTS, - ERC20_PROXY_ID: '0xf47261b0', - FEE_QUOTE_SOURCES, - WALLET_SIGNATURE: '0x04', - ONE_ETHER: new BigNumber(1e18), +/** + * Mainnet Curve configuration + */ +export const DEFAULT_CURVE_OPTS: { [source: string]: { version: number; curveAddress: string; tokens: string[] } } = { + [ERC20BridgeSource.CurveUsdcDai]: { + version: 1, + curveAddress: '0xa2b47e3d5c44877cca798226b7b8118f9bfb7a56', + tokens: ['0x6b175474e89094c44da98b954eedeac495271d0f', '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'], + }, + [ERC20BridgeSource.CurveUsdcDaiUsdt]: { + version: 1, + curveAddress: '0x52ea46506b9cc5ef470c5bf89f17dc28bb35d85c', + tokens: [ + '0x6b175474e89094c44da98b954eedeac495271d0f', + '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + '0xdac17f958d2ee523a2206206994597c13d831ec7', + ], + }, + [ERC20BridgeSource.CurveUsdcDaiUsdtTusd]: { + version: 1, + curveAddress: '0x45f783cce6b7ff23b2ab2d70e416cdb7d6055f51', + tokens: [ + '0x6b175474e89094c44da98b954eedeac495271d0f', + '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + '0xdac17f958d2ee523a2206206994597c13d831ec7', + '0x0000000000085d4780b73119b644ae5ecd22b376', + ], + }, + [ERC20BridgeSource.CurveUsdcDaiUsdtBusd]: { + version: 1, + curveAddress: '0x79a8c46dea5ada233abaffd40f3a0a2b1e5a4f27', + tokens: [ + '0x6b175474e89094c44da98b954eedeac495271d0f', + '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48', + '0xdac17f958d2ee523a2206206994597c13d831ec7', + '0x4fabb145d64652a948d72533023f6e7a623c7c53', + ], + }, }; + +export const ERC20_PROXY_ID = '0xf47261b0'; +export const WALLET_SIGNATURE = '0x04'; +export const ONE_ETHER = new BigNumber(1e18); +export const NEGATIVE_INF = new BigNumber('-Infinity'); +export const POSITIVE_INF = new BigNumber('Infinity'); +export const ZERO_AMOUNT = new BigNumber(0); +export const ONE_HOUR_IN_SECONDS = 60 * 60; +export const ONE_SECOND_MS = 1000; +export const NULL_BYTES = '0x'; +export const NULL_ADDRESS = '0x0000000000000000000000000000000000000000'; diff --git a/packages/asset-swapper/src/utils/market_operation_utils/dummy_order_utils.ts b/packages/asset-swapper/src/utils/market_operation_utils/dummy_order_utils.ts deleted file mode 100644 index 3b46def0ee..0000000000 --- a/packages/asset-swapper/src/utils/market_operation_utils/dummy_order_utils.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { SignedOrder } from '@0x/types'; - -import { constants } from '../../constants'; - -import { constants as marketOperationUtilConstants } from './constants'; - -const { NULL_ADDRESS, NULL_BYTES, ZERO_AMOUNT } = constants; -const { INFINITE_TIMESTAMP_SEC } = marketOperationUtilConstants; - -export const dummyOrderUtils = { - createDummyOrderForSampler(makerAssetData: string, takerAssetData: string, makerAddress: string): SignedOrder { - return { - makerAddress, - takerAddress: NULL_ADDRESS, - senderAddress: NULL_ADDRESS, - feeRecipientAddress: NULL_ADDRESS, - salt: ZERO_AMOUNT, - expirationTimeSeconds: INFINITE_TIMESTAMP_SEC, - makerAssetData, - takerAssetData, - makerFeeAssetData: NULL_BYTES, - takerFeeAssetData: NULL_BYTES, - makerFee: ZERO_AMOUNT, - takerFee: ZERO_AMOUNT, - makerAssetAmount: ZERO_AMOUNT, - takerAssetAmount: ZERO_AMOUNT, - signature: NULL_BYTES, - chainId: 1, - exchangeAddress: NULL_ADDRESS, - }; - }, -}; diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fill_optimizer.ts b/packages/asset-swapper/src/utils/market_operation_utils/fill_optimizer.ts deleted file mode 100644 index a191fbab88..0000000000 --- a/packages/asset-swapper/src/utils/market_operation_utils/fill_optimizer.ts +++ /dev/null @@ -1,181 +0,0 @@ -import { BigNumber } from '@0x/utils'; - -import { constants } from '../../constants'; - -import { Fill } from './types'; - -const { ZERO_AMOUNT } = constants; - -// Used internally by `FillsOptimizer`. -interface FillsOptimizerContext { - currentPath: Fill[]; - currentPathInput: BigNumber; - currentPathAdjustedOutput: BigNumber; - currentPathFlags: number; -} - -/** - * Class for finding optimized fill paths. - */ -export class FillsOptimizer { - private readonly _runLimit: number; - private readonly _shouldMinimize: boolean; - private _currentRunCount: number = 0; - private _optimalPath?: Fill[] = undefined; - private _optimalPathAdjustedOutput: BigNumber = ZERO_AMOUNT; - - constructor(runLimit: number, shouldMinimize?: boolean) { - this._runLimit = runLimit; - this._shouldMinimize = !!shouldMinimize; - } - - public optimize(fills: Fill[], input: BigNumber, upperBoundPath?: Fill[]): Fill[] | undefined { - this._currentRunCount = 0; - this._optimalPath = upperBoundPath; - this._optimalPathAdjustedOutput = upperBoundPath ? getPathAdjustedOutput(upperBoundPath, input) : ZERO_AMOUNT; - const ctx = { - currentPath: [], - currentPathInput: ZERO_AMOUNT, - currentPathAdjustedOutput: ZERO_AMOUNT, - currentPathFlags: 0, - }; - // Visit all valid combinations of fills to find the optimal path. - this._walk(fills, input, ctx); - if (this._optimalPath) { - return sortFillsByAdjustedRate(this._optimalPath, this._shouldMinimize); - } - return undefined; - } - - private _walk(fills: Fill[], input: BigNumber, ctx: FillsOptimizerContext): void { - const { currentPath, currentPathInput, currentPathAdjustedOutput, currentPathFlags } = ctx; - - // Stop if the current path is already complete. - if (currentPathInput.gte(input)) { - this._updateOptimalPath(currentPath, currentPathAdjustedOutput); - return; - } - - const lastNode = currentPath.length !== 0 ? currentPath[currentPath.length - 1] : undefined; - // Visit next fill candidates. - for (const nextFill of fills) { - // Subsequent fills must be a root node or be preceded by its parent, - // enforcing contiguous fills. - if (nextFill.parent && nextFill.parent !== lastNode) { - continue; - } - // Stop if we've hit our run limit. - if (this._currentRunCount++ >= this._runLimit) { - break; - } - const nextPath = [...currentPath, nextFill]; - const nextPathInput = BigNumber.min(input, currentPathInput.plus(nextFill.input)); - const nextPathAdjustedOutput = currentPathAdjustedOutput.plus( - getPartialFillOutput(nextFill, nextPathInput.minus(currentPathInput)).minus(nextFill.fillPenalty), - ); - // tslint:disable-next-line: no-bitwise - const nextPathFlags = currentPathFlags | nextFill.flags; - this._walk( - // Filter out incompatible fills. - // tslint:disable-next-line no-bitwise - fills.filter(f => f !== nextFill && (nextPathFlags & f.exclusionMask) === 0), - input, - { - currentPath: nextPath, - currentPathInput: nextPathInput, - currentPathAdjustedOutput: nextPathAdjustedOutput, - // tslint:disable-next-line: no-bitwise - currentPathFlags: nextPathFlags, - }, - ); - } - } - - private _updateOptimalPath(path: Fill[], adjustedOutput: BigNumber): void { - if (!this._optimalPath || this._compareOutputs(adjustedOutput, this._optimalPathAdjustedOutput) === 1) { - this._optimalPath = path; - this._optimalPathAdjustedOutput = adjustedOutput; - } - } - - private _compareOutputs(a: BigNumber, b: BigNumber): number { - return comparePathOutputs(a, b, this._shouldMinimize); - } -} - -/** - * Compute the total output minus penalty for a fill path, optionally clipping the input - * to `maxInput`. - */ -export function getPathAdjustedOutput(path: Fill[], maxInput?: BigNumber): BigNumber { - let currentInput = ZERO_AMOUNT; - let currentOutput = ZERO_AMOUNT; - let currentPenalty = ZERO_AMOUNT; - for (const fill of path) { - currentPenalty = currentPenalty.plus(fill.fillPenalty); - if (maxInput && currentInput.plus(fill.input).gte(maxInput)) { - const partialInput = maxInput.minus(currentInput); - currentOutput = currentOutput.plus(getPartialFillOutput(fill, partialInput)); - currentInput = partialInput; - break; - } else { - currentInput = currentInput.plus(fill.input); - currentOutput = currentOutput.plus(fill.output); - } - } - return currentOutput.minus(currentPenalty); -} - -/** - * Compares two rewards, returning -1, 0, or 1 - * if `a` is less than, equal to, or greater than `b`. - */ -export function comparePathOutputs(a: BigNumber, b: BigNumber, shouldMinimize: boolean): number { - return shouldMinimize ? b.comparedTo(a) : a.comparedTo(b); -} - -// Get the partial output earned by a fill at input `partialInput`. -function getPartialFillOutput(fill: Fill, partialInput: BigNumber): BigNumber { - return BigNumber.min(fill.output, fill.output.div(fill.input).times(partialInput)); -} - -/** - * Sort a path by adjusted input -> output rate while keeping sub-fills contiguous. - */ -export function sortFillsByAdjustedRate(path: Fill[], shouldMinimize: boolean = false): Fill[] { - return path.slice(0).sort((a, b) => { - const rootA = getFillRoot(a); - const rootB = getFillRoot(b); - const adjustedRateA = rootA.output.minus(rootA.fillPenalty).div(rootA.input); - const adjustedRateB = rootB.output.minus(rootB.fillPenalty).div(rootB.input); - if ((!a.parent && !b.parent) || a.fillData.source !== b.fillData.source) { - return shouldMinimize ? adjustedRateA.comparedTo(adjustedRateB) : adjustedRateB.comparedTo(adjustedRateA); - } - if (isFillAncestorOf(a, b)) { - return -1; - } - if (isFillAncestorOf(b, a)) { - return 1; - } - return 0; - }); -} - -function getFillRoot(fill: Fill): Fill { - let root = fill; - while (root.parent) { - root = root.parent; - } - return root; -} - -function isFillAncestorOf(ancestor: Fill, fill: Fill): boolean { - let currFill = fill.parent; - while (currFill) { - if (currFill === ancestor) { - return true; - } - currFill = currFill.parent; - } - return false; -} diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts new file mode 100644 index 0000000000..a3644453a3 --- /dev/null +++ b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts @@ -0,0 +1,237 @@ +import { BigNumber } from '@0x/utils'; + +import { MarketOperation, SignedOrderWithFillableAmounts } from '../../types'; +import { fillableAmountsUtils } from '../../utils/fillable_amounts_utils'; + +import { POSITIVE_INF, ZERO_AMOUNT } from './constants'; +import { + CollapsedFill, + DexSample, + ERC20BridgeSource, + Fill, + FillFlags, + NativeCollapsedFill, + NativeFillData, +} from './types'; + +// tslint:disable: prefer-for-of no-bitwise completed-docs + +/** + * Create fill paths from orders and dex quotes. + */ +export function createFillPaths(opts: { + side: MarketOperation; + orders?: SignedOrderWithFillableAmounts[]; + dexQuotes?: DexSample[][]; + ethToOutputRate?: BigNumber; + excludedSources?: ERC20BridgeSource[]; + fees?: { [source: string]: BigNumber }; +}): Fill[][] { + const { side } = opts; + const excludedSources = opts.excludedSources || []; + const fees = opts.fees || {}; + const orders = opts.orders || []; + const dexQuotes = opts.dexQuotes || []; + const ethToOutputRate = opts.ethToOutputRate || ZERO_AMOUNT; + // Create native fill paths. + const nativeFills = orders.map(o => nativeOrderToPath(side, o, ethToOutputRate, fees)); + // Create DEX fill paths. + const dexPaths = dexQuotesToPaths(side, dexQuotes, ethToOutputRate, fees); + return filterPaths([...dexPaths, ...nativeFills], excludedSources); +} + +function filterPaths(paths: Fill[][], excludedSources: ERC20BridgeSource[]): Fill[][] { + return paths.filter(path => { + if (path.length === 0) { + return false; + } + const [input, output] = getPathSize(path); + if (input.eq(0) || output.eq(0)) { + return false; + } + if (excludedSources.includes(path[0].source)) { + return false; + } + return true; + }); +} + +function nativeOrderToPath( + side: MarketOperation, + order: SignedOrderWithFillableAmounts, + ethToOutputRate: BigNumber, + fees: { [source: string]: BigNumber }, +): Fill[] { + const makerAmount = fillableAmountsUtils.getMakerAssetAmountSwappedAfterOrderFees(order); + const takerAmount = fillableAmountsUtils.getTakerAssetAmountSwappedAfterOrderFees(order); + const input = side === MarketOperation.Sell ? takerAmount : makerAmount; + const output = side === MarketOperation.Sell ? makerAmount : takerAmount; + const penalty = ethToOutputRate.times(fees[ERC20BridgeSource.Native] || 0); + const adjustedOutput = side === MarketOperation.Sell ? output.minus(penalty) : output.plus(penalty); + const rate = makerAmount.div(takerAmount); + const adjustedRate = + side === MarketOperation.Sell + ? makerAmount.minus(penalty).div(takerAmount) + : makerAmount.div(takerAmount.plus(penalty)); + // Native orders can be filled in any order, so they're all root nodes. + return [ + { + input, + output, + rate, + adjustedRate, + adjustedOutput, + index: 0, + flags: 0, + source: ERC20BridgeSource.Native, + fillData: { + order, + }, + }, + ]; +} + +function dexQuotesToPaths( + side: MarketOperation, + dexQuotes: DexSample[][], + ethToOutputRate: BigNumber, + fees: { [source: string]: BigNumber }, +): Fill[][] { + const paths: Fill[][] = []; + for (const quote of dexQuotes) { + const path: Fill[] = []; + for (let i = 0; i < quote.length; i++) { + const sample = quote[i]; + const prevSample = i === 0 ? undefined : quote[i - 1]; + const source = sample.source; + // Stop of the sample has zero output, which can occur if the source + // cannot fill the full amount. + // TODO(dorothy-zbornak): Sometimes Kyber will dip to zero then pick back up. + if (sample.output.eq(0)) { + break; + } + const input = sample.input.minus(prevSample ? prevSample.input : 0); + const output = sample.output.minus(prevSample ? prevSample.output : 0); + const penalty = + i === 0 // Only the first fill in a DEX path incurs a penalty. + ? ethToOutputRate.times(fees[source] || 0) + : ZERO_AMOUNT; + const adjustedOutput = side === MarketOperation.Sell ? output.minus(penalty) : output.plus(penalty); + const rate = side === MarketOperation.Sell ? output.div(input) : input.div(output); + const adjustedRate = + side === MarketOperation.Sell ? output.minus(penalty).div(input) : input.div(output.plus(penalty)); + + path.push({ + input, + output, + rate, + adjustedRate, + adjustedOutput, + source, + index: i, + parent: i !== 0 ? path[path.length - 1] : undefined, + flags: sourceToFillFlags(source), + }); + } + paths.push(path); + } + return paths; +} + +function sourceToFillFlags(source: ERC20BridgeSource): number { + if (source === ERC20BridgeSource.Kyber) { + return FillFlags.Kyber; + } + if (source === ERC20BridgeSource.Eth2Dai) { + return FillFlags.ConflictsWithKyber; + } + if (source === ERC20BridgeSource.Uniswap) { + return FillFlags.ConflictsWithKyber; + } + return 0; +} + +export function getPathSize(path: Fill[], maxInput: BigNumber = POSITIVE_INF): [BigNumber, BigNumber] { + let input = ZERO_AMOUNT; + let output = ZERO_AMOUNT; + for (const fill of path) { + if (input.plus(fill.input).gte(maxInput)) { + const di = maxInput.minus(input).div(fill.input); + input = input.plus(di); + output = output.plus(fill.output.times(di.div(fill.input))); + break; + } else { + input = input.plus(fill.input); + output = output.plus(fill.output); + } + } + return [input.integerValue(), output.integerValue()]; +} + +export function getPathAdjustedSize(path: Fill[], maxInput: BigNumber = POSITIVE_INF): [BigNumber, BigNumber] { + let input = ZERO_AMOUNT; + let output = ZERO_AMOUNT; + for (const fill of path) { + if (input.plus(fill.input).gte(maxInput)) { + const di = maxInput.minus(input).div(fill.input); + input = input.plus(di); + output = output.plus(fill.adjustedOutput.times(di.div(fill.input))); + break; + } else { + input = input.plus(fill.input); + output = output.plus(fill.adjustedOutput); + } + } + return [input.integerValue(), output.integerValue()]; +} + +export function isValidPath(path: Fill[]): boolean { + let flags = 0; + for (let i = 0; i < path.length; ++i) { + if (path[i].parent) { + if (i === 0 || path[i - 1] !== path[i].parent) { + return false; + } + } + for (let j = 0; j < i; ++j) { + if (path[i] === path[j]) { + return false; + } + } + flags |= path[i].flags; + } + const conflictFlags = FillFlags.Kyber | FillFlags.ConflictsWithKyber; + return (flags & conflictFlags) !== conflictFlags; +} + +export function collapsePath(side: MarketOperation, path: Fill[]): CollapsedFill[] { + const collapsed: Array = []; + for (const fill of path) { + const makerAssetAmount = side === MarketOperation.Sell ? fill.output : fill.input; + const takerAssetAmount = side === MarketOperation.Sell ? fill.input : fill.output; + const source = fill.source; + if (collapsed.length !== 0 && source !== ERC20BridgeSource.Native) { + const prevFill = collapsed[collapsed.length - 1]; + // If the last fill is from the same source, merge them. + if (prevFill.source === source) { + prevFill.totalMakerAssetAmount = prevFill.totalMakerAssetAmount.plus(makerAssetAmount); + prevFill.totalTakerAssetAmount = prevFill.totalTakerAssetAmount.plus(takerAssetAmount); + prevFill.subFills.push({ makerAssetAmount, takerAssetAmount }); + continue; + } + } + collapsed.push({ + source: fill.source, + totalMakerAssetAmount: makerAssetAmount, + totalTakerAssetAmount: takerAssetAmount, + subFills: [{ makerAssetAmount, takerAssetAmount }], + nativeOrder: fill.source === ERC20BridgeSource.Native ? (fill.fillData as NativeFillData).order : undefined, + }); + } + return collapsed; +} + +export function getUnusedSourcePaths(usedPath: Fill[], allPaths: Fill[][]): Fill[][] { + const usedSources = usedPath.map(f => f.source); + return allPaths.filter(p => !usedSources.includes(p[0].source)); +} diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index 93d0ad68bf..67ea911937 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -1,55 +1,37 @@ import { ContractAddresses } from '@0x/contract-addresses'; -import { assetDataUtils, ERC20AssetData, orderCalculationUtils } from '@0x/order-utils'; import { SignedOrder } from '@0x/types'; import { BigNumber, NULL_ADDRESS } from '@0x/utils'; -import { constants } from '../../constants'; -import { MarketOperation, SignedOrderWithFillableAmounts } from '../../types'; -import { fillableAmountsUtils } from '../fillable_amounts_utils'; +import { MarketOperation } from '../../types'; +import { difference } from '../utils'; -import { constants as marketOperationUtilConstants } from './constants'; -import { CreateOrderUtils } from './create_order'; -import { comparePathOutputs, FillsOptimizer, getPathAdjustedOutput, sortFillsByAdjustedRate } from './fill_optimizer'; +import { BUY_SOURCES, DEFAULT_GET_MARKET_ORDERS_OPTS, FEE_QUOTE_SOURCES, ONE_ETHER, SELL_SOURCES } from './constants'; +import { createFillPaths, getUnusedSourcePaths } from './fills'; +import { createOrdersFromPath, createSignedOrdersWithFillableAmounts, getNativeOrderTokens } from './orders'; +import { findOptimalPath } from './path_optimizer'; import { DexOrderSampler, getSampleAmounts } from './sampler'; import { AggregationError, - CollapsedFill, DexSample, ERC20BridgeSource, Fill, - FillData, - FillFlags, GetMarketOrdersOpts, - NativeCollapsedFill, - NativeFillData, OptimizedMarketOrder, OrderDomain, } from './types'; export { DexOrderSampler } from './sampler'; -const { ZERO_AMOUNT } = constants; -const { - BUY_SOURCES, - DEFAULT_GET_MARKET_ORDERS_OPTS, - ERC20_PROXY_ID, - FEE_QUOTE_SOURCES, - ONE_ETHER, - SELL_SOURCES, -} = marketOperationUtilConstants; - export class MarketOperationUtils { - private readonly _createOrderUtils: CreateOrderUtils; private readonly _wethAddress: string; constructor( private readonly _sampler: DexOrderSampler, - contractAddresses: ContractAddresses, + private readonly contractAddresses: ContractAddresses, private readonly _orderDomain: OrderDomain, private readonly _liquidityProviderRegistry: string = NULL_ADDRESS, ) { - this._createOrderUtils = new CreateOrderUtils(contractAddresses); - this._wethAddress = contractAddresses.etherToken; + this._wethAddress = contractAddresses.etherToken.toLowerCase(); } /** @@ -68,24 +50,25 @@ export class MarketOperationUtils { if (nativeOrders.length === 0) { throw new Error(AggregationError.EmptyOrders); } - const _opts = { - ...DEFAULT_GET_MARKET_ORDERS_OPTS, - ...opts, - }; - const [makerToken, takerToken] = getOrderTokens(nativeOrders[0]); + const _opts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, ...opts }; + const [makerToken, takerToken] = getNativeOrderTokens(nativeOrders[0]); + // Call the sampler contract. const [ - fillableAmounts, + orderFillableAmounts, liquidityProviderAddress, ethToMakerAssetRate, dexQuotes, ] = await this._sampler.executeAsync( + // Get native order fillable amounts. DexOrderSampler.ops.getOrderFillableTakerAmounts(nativeOrders), + // Get the custom liquidity provider from registry. DexOrderSampler.ops.getLiquidityProviderFromRegistry( this._liquidityProviderRegistry, - takerToken, makerToken, + takerToken, ), - makerToken.toLowerCase() === this._wethAddress.toLowerCase() + // Get ETH -> maker token price. + makerToken === this._wethAddress ? DexOrderSampler.ops.constant(new BigNumber(1)) : DexOrderSampler.ops.getMedianSellRate( difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( @@ -96,6 +79,7 @@ export class MarketOperationUtils { ONE_ETHER, this._liquidityProviderRegistry, ), + // Get sell quotes for taker -> maker. DexOrderSampler.ops.getSellQuotes( difference(SELL_SOURCES, _opts.excludedSources).concat( this._liquidityProviderSourceIfAvailable(_opts.excludedSources), @@ -106,50 +90,21 @@ export class MarketOperationUtils { this._liquidityProviderRegistry, ), ); - - const nativeOrdersWithFillableAmounts = createSignedOrdersWithFillableAmounts( + return this._generateOptimizedOrders({ + orderFillableAmounts, nativeOrders, - fillableAmounts, - MarketOperation.Sell, - ); - - const nativeFills = pruneNativeFills( - sortFillsByAdjustedRate( - createSellPathFromNativeOrders(nativeOrdersWithFillableAmounts, ethToMakerAssetRate, _opts), - ), - takerAmount, - _opts.dustFractionThreshold, - ); - const dexPaths = createSellPathsFromDexQuotes(dexQuotes, ethToMakerAssetRate, _opts); - const allPaths = [...dexPaths]; - const allFills = flattenDexPaths(dexPaths); - // If native orders are allowed, splice them in. - if (!_opts.excludedSources.includes(ERC20BridgeSource.Native)) { - allPaths.splice(0, 0, nativeFills); - allFills.splice(0, 0, ...nativeFills); - } - - const optimizer = new FillsOptimizer(_opts.runLimit); - const upperBoundPath = pickBestUpperBoundPath(allPaths, takerAmount); - const optimalPath = optimizer.optimize( - // Sorting the orders by price effectively causes the optimizer to walk - // the greediest solution first, which is the optimal solution in most - // cases. - sortFillsByAdjustedRate(allFills), - takerAmount, - upperBoundPath, - ); - if (!optimalPath) { - throw new Error(AggregationError.NoOptimalPath); - } - return this._createOrderUtils.createSellOrdersFromPath( - this._orderDomain, - takerToken, - makerToken, - collapsePath(optimalPath, false), - _opts.bridgeSlippage, + dexQuotes, liquidityProviderAddress, - ); + inputToken: takerToken, + outputToken: makerToken, + side: MarketOperation.Sell, + inputAmount: takerAmount, + ethToOutputRate: ethToMakerAssetRate, + bridgeSlippage: _opts.bridgeSlippage, + excludedSources: _opts.excludedSources, + fees: _opts.fees, + allowFallback: _opts.allowFallback, + }); } /** @@ -168,24 +123,25 @@ export class MarketOperationUtils { if (nativeOrders.length === 0) { throw new Error(AggregationError.EmptyOrders); } - const _opts = { - ...DEFAULT_GET_MARKET_ORDERS_OPTS, - ...opts, - }; - const [makerToken, takerToken] = getOrderTokens(nativeOrders[0]); + const _opts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, ...opts }; + const [makerToken, takerToken] = getNativeOrderTokens(nativeOrders[0]); + // Call the sampler contract. const [ - fillableAmounts, + orderFillableAmounts, liquidityProviderAddress, ethToTakerAssetRate, dexQuotes, ] = await this._sampler.executeAsync( + // Get native order fillable amounts. DexOrderSampler.ops.getOrderFillableMakerAmounts(nativeOrders), + // Get the custom liquidity provider from registry. DexOrderSampler.ops.getLiquidityProviderFromRegistry( this._liquidityProviderRegistry, - takerToken, makerToken, + takerToken, ), - takerToken.toLowerCase() === this._wethAddress.toLowerCase() + // Get ETH -> taker token price. + takerToken === this._wethAddress ? DexOrderSampler.ops.constant(new BigNumber(1)) : DexOrderSampler.ops.getMedianSellRate( difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( @@ -196,6 +152,7 @@ export class MarketOperationUtils { ONE_ETHER, this._liquidityProviderRegistry, ), + // Get buy quotes for taker -> maker. DexOrderSampler.ops.getBuyQuotes( difference(BUY_SOURCES, _opts.excludedSources).concat( this._liquidityProviderSourceIfAvailable(_opts.excludedSources), @@ -206,19 +163,22 @@ export class MarketOperationUtils { this._liquidityProviderRegistry, ), ); - const signedOrderWithFillableAmounts = this._createBuyOrdersPathFromSamplerResultIfExists( + + return this._generateOptimizedOrders({ + orderFillableAmounts, nativeOrders, - makerAmount, - fillableAmounts, dexQuotes, - ethToTakerAssetRate, - _opts, liquidityProviderAddress, - ); - if (!signedOrderWithFillableAmounts) { - throw new Error(AggregationError.NoOptimalPath); - } - return signedOrderWithFillableAmounts; + inputToken: makerToken, + outputToken: takerToken, + side: MarketOperation.Buy, + inputAmount: makerAmount, + ethToOutputRate: ethToTakerAssetRate, + bridgeSlippage: _opts.bridgeSlippage, + excludedSources: _opts.excludedSources, + fees: _opts.fees, + allowFallback: _opts.allowFallback, + }); } /** @@ -240,10 +200,7 @@ export class MarketOperationUtils { if (batchNativeOrders.length === 0) { throw new Error(AggregationError.EmptyOrders); } - const _opts = { - ...DEFAULT_GET_MARKET_ORDERS_OPTS, - ...opts, - }; + const _opts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, ...opts }; const sources = difference(BUY_SOURCES, _opts.excludedSources); const ops = [ @@ -251,32 +208,97 @@ export class MarketOperationUtils { ...batchNativeOrders.map(orders => DexOrderSampler.ops.getMedianSellRate( difference(FEE_QUOTE_SOURCES, _opts.excludedSources), + getNativeOrderTokens(orders[0])[1], this._wethAddress, - getOrderTokens(orders[0])[1], ONE_ETHER, ), ), ...batchNativeOrders.map((orders, i) => - DexOrderSampler.ops.getBuyQuotes(sources, getOrderTokens(orders[0])[0], getOrderTokens(orders[0])[1], [ - makerAmounts[i], - ]), + DexOrderSampler.ops.getBuyQuotes( + sources, + getNativeOrderTokens(orders[0])[0], + getNativeOrderTokens(orders[0])[1], + [makerAmounts[i]], + ), ), ]; + const executeResults = await this._sampler.executeBatchAsync(ops); - const batchFillableAmounts = executeResults.splice(0, batchNativeOrders.length) as BigNumber[][]; + const batchOrderFillableAmounts = executeResults.splice(0, batchNativeOrders.length) as BigNumber[][]; const batchEthToTakerAssetRate = executeResults.splice(0, batchNativeOrders.length) as BigNumber[]; const batchDexQuotes = executeResults.splice(0, batchNativeOrders.length) as DexSample[][][]; - return batchFillableAmounts.map((fillableAmounts, i) => - this._createBuyOrdersPathFromSamplerResultIfExists( - batchNativeOrders[i], - makerAmounts[i], - fillableAmounts, - batchDexQuotes[i], - batchEthToTakerAssetRate[i], - _opts, - ), - ); + return batchNativeOrders.map((nativeOrders, i) => { + if (nativeOrders.length === 0) { + throw new Error(AggregationError.EmptyOrders); + } + const [makerToken, takerToken] = getNativeOrderTokens(nativeOrders[0]); + const orderFillableAmounts = batchOrderFillableAmounts[i]; + const ethToTakerAssetRate = batchEthToTakerAssetRate[i]; + const dexQuotes = batchDexQuotes[i]; + const makerAmount = makerAmounts[i]; + return this._generateOptimizedOrders({ + orderFillableAmounts, + nativeOrders, + dexQuotes, + inputToken: makerToken, + outputToken: takerToken, + side: MarketOperation.Buy, + inputAmount: makerAmount, + ethToOutputRate: ethToTakerAssetRate, + bridgeSlippage: _opts.bridgeSlippage, + excludedSources: _opts.excludedSources, + fees: _opts.fees, + allowFallback: _opts.allowFallback, + }); + }); + } + + private _generateOptimizedOrders(opts: { + side: MarketOperation; + inputToken: string; + outputToken: string; + inputAmount: BigNumber; + nativeOrders: SignedOrder[]; + orderFillableAmounts: BigNumber[]; + dexQuotes: DexSample[][]; + ethToOutputRate?: BigNumber; + bridgeSlippage?: number; + excludedSources?: ERC20BridgeSource[]; + fees?: { [source: string]: BigNumber }; + allowFallback?: boolean; + liquidityProviderAddress?: string; + }): OptimizedMarketOrder[] { + const { inputToken, outputToken, side, inputAmount } = opts; + // Convert native orders and dex quotes into fill paths. + const paths = createFillPaths({ + side, + // Augment native orders with their fillable amounts. + orders: createSignedOrdersWithFillableAmounts(side, opts.nativeOrders, opts.orderFillableAmounts), + dexQuotes: opts.dexQuotes, + ethToOutputRate: opts.ethToOutputRate, + excludedSources: opts.excludedSources, + fees: opts.fees, + }); + // Find the optimal path. + const optimalPath = findOptimalPath(side, paths, inputAmount); + // console.log(JSON.stringify(paths, null, ' ')); + if (!optimalPath) { + throw new Error(AggregationError.NoOptimalPath); + } + // Find a fallback path from sources not used in the first path. + let fallbackPath: Fill[] = []; + if (opts.allowFallback) { + fallbackPath = findOptimalPath(side, getUnusedSourcePaths(optimalPath, paths), inputAmount) || []; + } + return createOrdersFromPath([...optimalPath, ...fallbackPath], { + side, + inputToken, + outputToken, + orderDomain: this._orderDomain, + contractAddresses: this.contractAddresses, + bridgeSlippage: opts.bridgeSlippage || 0, + }); } private _liquidityProviderSourceIfAvailable(excludedSources: ERC20BridgeSource[]): ERC20BridgeSource[] { @@ -285,328 +307,6 @@ export class MarketOperationUtils { ? [ERC20BridgeSource.LiquidityProvider] : []; } - - private _createBuyOrdersPathFromSamplerResultIfExists( - nativeOrders: SignedOrder[], - makerAmount: BigNumber, - nativeOrderFillableAmounts: BigNumber[], - dexQuotes: DexSample[][], - ethToTakerAssetRate: BigNumber, - opts: GetMarketOrdersOpts, - liquidityProviderAddress?: string, - ): OptimizedMarketOrder[] | undefined { - const nativeOrdersWithFillableAmounts = createSignedOrdersWithFillableAmounts( - nativeOrders, - nativeOrderFillableAmounts, - MarketOperation.Buy, - ); - const nativeFills = pruneNativeFills( - sortFillsByAdjustedRate( - createBuyPathFromNativeOrders(nativeOrdersWithFillableAmounts, ethToTakerAssetRate, opts), - true, - ), - makerAmount, - opts.dustFractionThreshold, - ); - const dexPaths = createBuyPathsFromDexQuotes(dexQuotes, ethToTakerAssetRate, opts); - const allPaths = [...dexPaths]; - const allFills = flattenDexPaths(dexPaths); - // If native orders are allowed, splice them in. - if (!opts.excludedSources.includes(ERC20BridgeSource.Native)) { - allPaths.splice(0, 0, nativeFills); - allFills.splice(0, 0, ...nativeFills); - } - const optimizer = new FillsOptimizer(opts.runLimit, true); - const upperBoundPath = pickBestUpperBoundPath(allPaths, makerAmount, true); - const optimalPath = optimizer.optimize( - // Sorting the orders by price effectively causes the optimizer to walk - // the greediest solution first, which is the optimal solution in most - // cases. - sortFillsByAdjustedRate(allFills, true), - makerAmount, - upperBoundPath, - ); - if (!optimalPath) { - return undefined; - } - const [inputToken, outputToken] = getOrderTokens(nativeOrders[0]); - return this._createOrderUtils.createBuyOrdersFromPath( - this._orderDomain, - inputToken, - outputToken, - collapsePath(optimalPath, true), - opts.bridgeSlippage, - liquidityProviderAddress, - ); - } -} - -function createSignedOrdersWithFillableAmounts( - signedOrders: SignedOrder[], - fillableAmounts: BigNumber[], - operation: MarketOperation, -): SignedOrderWithFillableAmounts[] { - return signedOrders - .map((order: SignedOrder, i: number) => { - const fillableAmount = fillableAmounts[i]; - const fillableMakerAssetAmount = - operation === MarketOperation.Buy - ? fillableAmount - : orderCalculationUtils.getMakerFillAmount(order, fillableAmount); - const fillableTakerAssetAmount = - operation === MarketOperation.Sell - ? fillableAmount - : orderCalculationUtils.getTakerFillAmount(order, fillableAmount); - const fillableTakerFeeAmount = orderCalculationUtils.getTakerFeeAmount(order, fillableTakerAssetAmount); - return { - fillableMakerAssetAmount, - fillableTakerAssetAmount, - fillableTakerFeeAmount, - ...order, - }; - }) - .filter(order => { - return !order.fillableMakerAssetAmount.isZero() && !order.fillableTakerAssetAmount.isZero(); - }); -} - -// Gets the difference between two sets. -function difference(a: T[], b: T[]): T[] { - return a.filter(x => b.indexOf(x) === -1); -} - -function createSellPathFromNativeOrders( - orders: SignedOrderWithFillableAmounts[], - ethToOutputAssetRate: BigNumber, - opts: GetMarketOrdersOpts, -): Fill[] { - const path: Fill[] = []; - // tslint:disable-next-line: prefer-for-of - for (let i = 0; i < orders.length; i++) { - const order = orders[i]; - const makerAmount = fillableAmountsUtils.getMakerAssetAmountSwappedAfterOrderFees(order); - const takerAmount = fillableAmountsUtils.getTakerAssetAmountSwappedAfterOrderFees(order); - // Native orders can be filled in any order, so they're all root nodes. - path.push({ - flags: FillFlags.SourceNative, - exclusionMask: 0, - input: takerAmount, - output: makerAmount, - // Every fill from native orders incurs a penalty. - fillPenalty: ethToOutputAssetRate.times(opts.fees[ERC20BridgeSource.Native] || 0), - fillData: { - source: ERC20BridgeSource.Native, - order, - }, - }); - } - return path; -} - -function createBuyPathFromNativeOrders( - orders: SignedOrderWithFillableAmounts[], - ethToOutputAssetRate: BigNumber, - opts: GetMarketOrdersOpts, -): Fill[] { - const path: Fill[] = []; - // tslint:disable-next-line: prefer-for-of - for (let i = 0; i < orders.length; i++) { - const order = orders[i]; - const makerAmount = fillableAmountsUtils.getMakerAssetAmountSwappedAfterOrderFees(order); - const takerAmount = fillableAmountsUtils.getTakerAssetAmountSwappedAfterOrderFees(order); - // Native orders can be filled in any order, so they're all root nodes. - path.push({ - flags: FillFlags.SourceNative, - exclusionMask: 0, - input: makerAmount, - output: takerAmount, - // Every fill from native orders incurs a penalty. - // Negated because we try to minimize the output in buys. - fillPenalty: ethToOutputAssetRate.times(opts.fees[ERC20BridgeSource.Native] || 0).negated(), - fillData: { - source: ERC20BridgeSource.Native, - order, - }, - }); - } - return path; -} - -function pruneNativeFills(fills: Fill[], fillAmount: BigNumber, dustFractionThreshold: number): Fill[] { - const minInput = fillAmount.times(dustFractionThreshold); - const pruned = []; - let totalInput = ZERO_AMOUNT; - for (const fill of fills) { - if (totalInput.gte(fillAmount)) { - break; - } - if (fill.input.lt(minInput)) { - continue; - } - totalInput = totalInput.plus(fill.input); - pruned.push(fill); - } - return pruned; -} - -function createSellPathsFromDexQuotes( - dexQuotes: DexSample[][], - ethToOutputAssetRate: BigNumber, - opts: GetMarketOrdersOpts, -): Fill[][] { - return createPathsFromDexQuotes(dexQuotes, ethToOutputAssetRate, opts); -} - -function createBuyPathsFromDexQuotes( - dexQuotes: DexSample[][], - ethToOutputAssetRate: BigNumber, - opts: GetMarketOrdersOpts, -): Fill[][] { - return createPathsFromDexQuotes( - dexQuotes, - // Negated because we try to minimize the output in buys. - ethToOutputAssetRate.negated(), - opts, - ); -} - -function createPathsFromDexQuotes( - dexQuotes: DexSample[][], - ethToOutputAssetRate: BigNumber, - opts: GetMarketOrdersOpts, -): Fill[][] { - const paths: Fill[][] = []; - for (const quote of dexQuotes) { - const path: Fill[] = []; - let prevSample: DexSample | undefined; - // tslint:disable-next-line: prefer-for-of - for (let i = 0; i < quote.length; i++) { - const sample = quote[i]; - // Stop of the sample has zero output, which can occur if the source - // cannot fill the full amount. - if (sample.output.isZero()) { - break; - } - path.push({ - input: sample.input.minus(prevSample ? prevSample.input : 0), - output: sample.output.minus(prevSample ? prevSample.output : 0), - fillPenalty: ZERO_AMOUNT, - parent: path.length !== 0 ? path[path.length - 1] : undefined, - flags: sourceToFillFlags(sample.source), - exclusionMask: opts.noConflicts ? sourceToExclusionMask(sample.source) : 0, - fillData: { source: sample.source }, - }); - prevSample = quote[i]; - } - // Don't push empty paths. - if (path.length > 0) { - // Only the first fill in a DEX path incurs a penalty. - path[0].fillPenalty = ethToOutputAssetRate.times(opts.fees[path[0].fillData.source] || 0); - paths.push(path); - } - } - return paths; -} - -function sourceToFillFlags(source: ERC20BridgeSource): number { - if (source === ERC20BridgeSource.Kyber) { - return FillFlags.SourceKyber; - } - if (source === ERC20BridgeSource.Eth2Dai) { - return FillFlags.SourceEth2Dai; - } - if (source === ERC20BridgeSource.Uniswap) { - return FillFlags.SourceUniswap; - } - if (source === ERC20BridgeSource.LiquidityProvider) { - return FillFlags.SourceLiquidityPool; - } - return FillFlags.SourceNative; -} - -function sourceToExclusionMask(source: ERC20BridgeSource): number { - if (source === ERC20BridgeSource.Kyber) { - // tslint:disable-next-line: no-bitwise - return FillFlags.SourceEth2Dai | FillFlags.SourceUniswap; - } - if (source === ERC20BridgeSource.Eth2Dai) { - return FillFlags.SourceKyber; - } - if (source === ERC20BridgeSource.Uniswap) { - return FillFlags.SourceKyber; - } - return 0; -} - -// Convert a list of DEX paths to a flattened list of `Fills`. -function flattenDexPaths(dexFills: Fill[][]): Fill[] { - const fills: Fill[] = []; - for (const quote of dexFills) { - for (const fill of quote) { - fills.push(fill); - } - } - return fills; -} - -// Picks the path with the highest (or lowest if `shouldMinimize` is true) output. -function pickBestUpperBoundPath(paths: Fill[][], maxInput: BigNumber, shouldMinimize?: boolean): Fill[] | undefined { - let optimalPath: Fill[] | undefined; - let optimalPathOutput: BigNumber = ZERO_AMOUNT; - for (const path of paths) { - if (getPathInput(path).gte(maxInput)) { - const output = getPathAdjustedOutput(path, maxInput); - if (!optimalPath || comparePathOutputs(output, optimalPathOutput, !!shouldMinimize) === 1) { - optimalPath = path; - optimalPathOutput = output; - } - } - } - return optimalPath; -} - -// Gets the total input of a path. -function getPathInput(path: Fill[]): BigNumber { - return BigNumber.sum(...path.map(p => p.input)); -} - -// Merges contiguous fills from the same DEX. -function collapsePath(path: Fill[], isBuy: boolean): CollapsedFill[] { - const collapsed: Array = []; - for (const fill of path) { - const makerAssetAmount = isBuy ? fill.input : fill.output; - const takerAssetAmount = isBuy ? fill.output : fill.input; - const source = (fill.fillData as FillData).source; - if (collapsed.length !== 0 && source !== ERC20BridgeSource.Native) { - const prevFill = collapsed[collapsed.length - 1]; - // If the last fill is from the same source, merge them. - if (prevFill.source === source) { - prevFill.totalMakerAssetAmount = prevFill.totalMakerAssetAmount.plus(makerAssetAmount); - prevFill.totalTakerAssetAmount = prevFill.totalTakerAssetAmount.plus(takerAssetAmount); - prevFill.subFills.push({ makerAssetAmount, takerAssetAmount }); - continue; - } - } - collapsed.push({ - source: fill.fillData.source, - totalMakerAssetAmount: makerAssetAmount, - totalTakerAssetAmount: takerAssetAmount, - subFills: [{ makerAssetAmount, takerAssetAmount }], - nativeOrder: (fill.fillData as NativeFillData).order, - }); - } - return collapsed; -} - -function getOrderTokens(order: SignedOrder): [string, string] { - const assets = [order.makerAssetData, order.takerAssetData].map(a => assetDataUtils.decodeAssetDataOrThrow(a)) as [ - ERC20AssetData, - ERC20AssetData - ]; - if (assets.some(a => a.assetProxyId !== ERC20_PROXY_ID)) { - throw new Error(AggregationError.NotERC20AssetData); - } - return assets.map(a => a.tokenAddress) as [string, string]; } // tslint:disable: max-file-line-count diff --git a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts new file mode 100644 index 0000000000..35f41f94fb --- /dev/null +++ b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts @@ -0,0 +1,252 @@ +import { ContractAddresses } from '@0x/contract-addresses'; +import { assetDataUtils, ERC20AssetData, generatePseudoRandomSalt, orderCalculationUtils } from '@0x/order-utils'; +import { SignedOrder } from '@0x/types'; +import { AbiEncoder, BigNumber } from '@0x/utils'; + +import { MarketOperation, SignedOrderWithFillableAmounts } from '../../types'; + +import { + DEFAULT_CURVE_OPTS, + ERC20_PROXY_ID, + NULL_ADDRESS, + NULL_BYTES, + ONE_HOUR_IN_SECONDS, + ONE_SECOND_MS, + WALLET_SIGNATURE, + ZERO_AMOUNT, +} from './constants'; +import { collapsePath } from './fills'; +import { + AggregationError, + CollapsedFill, + ERC20BridgeSource, + Fill, + NativeCollapsedFill, + OptimizedMarketOrder, + OrderDomain, +} from './types'; + +// tslint:disable completed-docs + +export function createDummyOrderForSampler( + makerAssetData: string, + takerAssetData: string, + makerAddress: string, +): SignedOrder { + return { + makerAddress, + takerAddress: NULL_ADDRESS, + senderAddress: NULL_ADDRESS, + feeRecipientAddress: NULL_ADDRESS, + salt: ZERO_AMOUNT, + expirationTimeSeconds: ZERO_AMOUNT, + makerAssetData, + takerAssetData, + makerFeeAssetData: NULL_BYTES, + takerFeeAssetData: NULL_BYTES, + makerFee: ZERO_AMOUNT, + takerFee: ZERO_AMOUNT, + makerAssetAmount: ZERO_AMOUNT, + takerAssetAmount: ZERO_AMOUNT, + signature: NULL_BYTES, + chainId: 1, + exchangeAddress: NULL_ADDRESS, + }; +} + +export function getNativeOrderTokens(order: SignedOrder): [string, string] { + const assets = [order.makerAssetData, order.takerAssetData].map(a => assetDataUtils.decodeAssetDataOrThrow(a)) as [ + ERC20AssetData, + ERC20AssetData + ]; + if (assets.some(a => a.assetProxyId !== ERC20_PROXY_ID)) { + throw new Error(AggregationError.NotERC20AssetData); + } + return assets.map(a => a.tokenAddress.toLowerCase()) as [string, string]; +} + +export function convertNativeOrderToFullyFillableOptimizedOrders(order: SignedOrder): OptimizedMarketOrder { + return { + ...order, + fillableMakerAssetAmount: order.makerAssetAmount, + fillableTakerAssetAmount: order.takerAssetAmount, + fillableTakerFeeAmount: order.takerFee, + fill: { + source: ERC20BridgeSource.Native, + totalMakerAssetAmount: order.makerAssetAmount, + totalTakerAssetAmount: order.takerAssetAmount, + subFills: [], + }, + }; +} + +/** + * Augments native orders with fillable amounts and filters out unfillable orders. + */ +export function createSignedOrdersWithFillableAmounts( + side: MarketOperation, + orders: SignedOrder[], + fillableAmounts: BigNumber[], +): SignedOrderWithFillableAmounts[] { + return orders + .map((order: SignedOrder, i: number) => { + const fillableAmount = fillableAmounts[i]; + const fillableMakerAssetAmount = + side === MarketOperation.Buy + ? fillableAmount + : orderCalculationUtils.getMakerFillAmount(order, fillableAmount); + const fillableTakerAssetAmount = + side === MarketOperation.Sell + ? fillableAmount + : orderCalculationUtils.getTakerFillAmount(order, fillableAmount); + const fillableTakerFeeAmount = orderCalculationUtils.getTakerFeeAmount(order, fillableTakerAssetAmount); + return { + ...order, + fillableMakerAssetAmount, + fillableTakerAssetAmount, + fillableTakerFeeAmount, + }; + }) + .filter(order => { + return !order.fillableMakerAssetAmount.isZero() && !order.fillableTakerAssetAmount.isZero(); + }); +} + +export interface CreateOrderFromPathOpts { + side: MarketOperation; + inputToken: string; + outputToken: string; + orderDomain: OrderDomain; + contractAddresses: ContractAddresses; + bridgeSlippage: number; +} + +// Convert sell fills into orders. +export function createOrdersFromPath(path: Fill[], opts: CreateOrderFromPathOpts): OptimizedMarketOrder[] { + const collapsedPath = collapsePath(opts.side, path); + const orders: OptimizedMarketOrder[] = []; + for (const fill of collapsedPath) { + if (fill.source === ERC20BridgeSource.Native) { + orders.push(createNativeOrder(fill)); + } else { + orders.push(createBridgeOrder(fill, opts)); + } + } + return orders; +} + +function getBridgeAddressFromSource(source: ERC20BridgeSource, contractAddresses: ContractAddresses): string { + switch (source) { + case ERC20BridgeSource.Eth2Dai: + return contractAddresses.eth2DaiBridge; + case ERC20BridgeSource.Kyber: + return contractAddresses.kyberBridge; + case ERC20BridgeSource.Uniswap: + return contractAddresses.uniswapBridge; + case ERC20BridgeSource.CurveUsdcDai: + case ERC20BridgeSource.CurveUsdcDaiUsdt: + case ERC20BridgeSource.CurveUsdcDaiUsdtTusd: + case ERC20BridgeSource.CurveUsdcDaiUsdtBusd: + return contractAddresses.curveBridge; + default: + break; + } + throw new Error(AggregationError.NoBridgeForSource); +} + +function createBridgeOrder(fill: CollapsedFill, opts: CreateOrderFromPathOpts): OptimizedMarketOrder { + const takerToken = opts.side === MarketOperation.Sell ? opts.inputToken : opts.outputToken; + const makerToken = opts.side === MarketOperation.Sell ? opts.outputToken : opts.inputToken; + const bridgeAddress = getBridgeAddressFromSource(fill.source, opts.contractAddresses); + + let makerAssetData; + if (Object.keys(DEFAULT_CURVE_OPTS).includes(fill.source)) { + const { curveAddress, tokens, version } = DEFAULT_CURVE_OPTS[fill.source]; + const fromTokenIdx = tokens.indexOf(takerToken); + const toTokenIdx = tokens.indexOf(makerToken); + makerAssetData = assetDataUtils.encodeERC20BridgeAssetData( + makerToken, + bridgeAddress, + createCurveBridgeData(curveAddress, fromTokenIdx, toTokenIdx, version), + ); + } else { + makerAssetData = assetDataUtils.encodeERC20BridgeAssetData( + makerToken, + bridgeAddress, + createBridgeData(takerToken), + ); + } + return { + makerAddress: bridgeAddress, + makerAssetData, + takerAssetData: assetDataUtils.encodeERC20AssetData(takerToken), + ...createCommonBridgeOrderFields(fill, opts), + }; +} + +function createBridgeData(tokenAddress: string): string { + const encoder = AbiEncoder.create([{ name: 'tokenAddress', type: 'address' }]); + return encoder.encode({ tokenAddress }); +} + +function createCurveBridgeData( + curveAddress: string, + fromTokenIdx: number, + toTokenIdx: number, + version: number, +): string { + const curveBridgeDataEncoder = AbiEncoder.create([ + { name: 'curveAddress', type: 'address' }, + { name: 'fromTokenIdx', type: 'int128' }, + { name: 'toTokenIdx', type: 'int128' }, + { name: 'version', type: 'int128' }, + ]); + return curveBridgeDataEncoder.encode([curveAddress, fromTokenIdx, toTokenIdx, version]); +} + +type CommonBridgeOrderFields = Pick< + OptimizedMarketOrder, + Exclude +>; + +function createCommonBridgeOrderFields(fill: CollapsedFill, opts: CreateOrderFromPathOpts): CommonBridgeOrderFields { + const makerAssetAmountAdjustedWithSlippage = + opts.side === MarketOperation.Sell + ? fill.totalMakerAssetAmount.times(1 - opts.bridgeSlippage).integerValue(BigNumber.ROUND_DOWN) + : fill.totalMakerAssetAmount; + const takerAssetAmountAdjustedWithSlippage = + opts.side === MarketOperation.Buy + ? fill.totalTakerAssetAmount + : fill.totalTakerAssetAmount.times(opts.bridgeSlippage + 1).integerValue(BigNumber.ROUND_UP); + return { + fill, + takerAddress: NULL_ADDRESS, + senderAddress: NULL_ADDRESS, + feeRecipientAddress: NULL_ADDRESS, + salt: generatePseudoRandomSalt(), + expirationTimeSeconds: new BigNumber(Math.floor(Date.now() / ONE_SECOND_MS) + ONE_HOUR_IN_SECONDS), + makerFeeAssetData: NULL_BYTES, + takerFeeAssetData: NULL_BYTES, + makerFee: ZERO_AMOUNT, + takerFee: ZERO_AMOUNT, + makerAssetAmount: makerAssetAmountAdjustedWithSlippage, + fillableMakerAssetAmount: makerAssetAmountAdjustedWithSlippage, + takerAssetAmount: takerAssetAmountAdjustedWithSlippage, + fillableTakerAssetAmount: takerAssetAmountAdjustedWithSlippage, + fillableTakerFeeAmount: ZERO_AMOUNT, + signature: WALLET_SIGNATURE, + ...opts.orderDomain, + }; +} + +function createNativeOrder(fill: CollapsedFill): OptimizedMarketOrder { + return { + fill: { + source: fill.source, + totalMakerAssetAmount: fill.totalMakerAssetAmount, + totalTakerAssetAmount: fill.totalTakerAssetAmount, + subFills: fill.subFills, + }, + ...(fill as NativeCollapsedFill).nativeOrder, + }; +} diff --git a/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts b/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts new file mode 100644 index 0000000000..61893ca26f --- /dev/null +++ b/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts @@ -0,0 +1,155 @@ +import { BigNumber } from '@0x/utils'; + +import { MarketOperation } from '../../types'; + +import { getPathAdjustedSize, getPathSize, isValidPath } from './fills'; +import { Fill } from './types'; + +// tslint:disable: prefer-for-of custom-no-magic-numbers completed-docs + +/** + * Find the optimal mixture of paths that maximizes (for sells) or minimizes + * (for buys) output, while meeting the input requirement. + */ +export function findOptimalPath(side: MarketOperation, paths: Fill[][], targetInput: BigNumber): Fill[] | undefined { + // Get all convex and concave rate paths. + const [convexPaths, concavePaths] = splitPathsByRateConvexity(paths); + // Hill climb convex paths. + let optimalPath = hillClimbToOptimalPath(convexPaths, targetInput); + // Attempt to splice in concave paths. + for (const path of concavePaths) { + // TODO(dorothy-zbornak): This will probably not be optimal if we are dealing + // more than one concave path. Right now there's just Kyber. + optimalPath = prependConcavePath(side, optimalPath, path, targetInput); + } + return isPathComplete(optimalPath, targetInput) ? optimalPath : undefined; +} + +const RATE_DECIMALS = 8; + +function hillClimbToOptimalPath(paths: Fill[][], targetInput: BigNumber): Fill[] { + // Flatten and sort path fills by descending ADJUSTED rate. + const fills = paths + .reduce((acc, p) => acc.concat(p)) + .sort((a, b) => b.adjustedRate.dp(RATE_DECIMALS).comparedTo(a.adjustedRate.dp(RATE_DECIMALS))); + // Build up a path by picking the next best, valid fill until we meet our input target. + const path: Fill[] = []; + while (!isPathComplete(path, targetInput)) { + let wasAdded = false; + for (const fill of fills) { + // If we can just legally append this fill to the path, do that. + if (isValidPath([...path, fill])) { + path.push(fill); + wasAdded = true; + break; + } else if (fill.parent && !path.includes(fill)) { + // If the fill's parent is in the path, we can insert it right + // after it. + for (let i = 0; i < path.length; ++i) { + if (path[i] === fill.parent) { + path.splice(i + 1, 0, fill); + wasAdded = true; + break; + } + } + if (wasAdded) { + break; + } + } + } + if (!wasAdded) { + break; + } + } + return path; +} + +function prependConcavePath( + side: MarketOperation, + convexPath: Fill[], + concavePath: Fill[], + targetInput: BigNumber, +): Fill[] { + // Try to prepend increasing lenths of the the concave path, keeping track + // of the best path. + // HACK(dorothy-zbornak): We prepend because placing it at the end has the + // possibility of turning it into a partial fill, which can invalidate the + // quote for Kyber. + let bestPath = convexPath; + for (let i = 0; i < convexPath.length; ++i) { + const path = concavePath.slice(0, i); + const [concaveInput] = getPathSize(path); + if (concaveInput.lt(targetInput)) { + const remainingInput = targetInput.minus(concaveInput); + // We sub-optimize the fills from the original path. + const tailPath = hillClimbToOptimalPath(getSubPaths(convexPath), remainingInput); + path.push(...tailPath); + } + bestPath = findBestCompletePath(side, [bestPath, path], targetInput) || bestPath; + } + return bestPath; +} + +function getSubPaths(path: Fill[]): Fill[][] { + const fillsBySource: { [source: string]: Fill[] } = {}; + for (const fill of path) { + fillsBySource[fill.source] = fillsBySource[fill.source] || []; + fillsBySource[fill.source].push(fill); + } + return Object.values(fillsBySource); +} + +function isPathComplete(path: Fill[], targetInput: BigNumber): boolean { + const [input] = getPathSize(path); + return input.gte(targetInput); +} + +function findBestCompletePath(side: MarketOperation, paths: Fill[][], targetInput: BigNumber): Fill[] | undefined { + let bestPath: Fill[] | undefined; + for (const path of paths) { + const [input, output] = getPathAdjustedSize(path, targetInput); + if (input.gte(targetInput)) { + continue; + } + if (bestPath) { + const [, bestPathOutput] = getPathAdjustedSize(bestPath, targetInput); + if (side === MarketOperation.Sell) { + if (output.lt(bestPathOutput)) { + continue; + } + } else { + if (output.gt(bestPathOutput)) { + continue; + } + } + } + bestPath = path; + } + return bestPath; +} + +function splitPathsByRateConvexity(paths: Fill[][]): [Fill[][], Fill[][]] { + const convexPaths: Fill[][] = []; + const concavePaths: Fill[][] = []; + for (const path of paths) { + if (isPathConvex(path)) { + convexPaths.push(path); + } else { + concavePaths.push(path); + } + } + return [convexPaths, concavePaths]; +} + +function isPathConvex(path: Fill[]): boolean { + // Convex paths have descending prices. + // HACK(dorothy-zbornak): We use the the `rate` instead of the `adjustedRate` + // because the `adjustedRate` can make paths appear artificially concave + // due to the fee incurred on the first fill. + for (let i = 1; i < path.length; ++i) { + if (path[i - 1].rate.dp(RATE_DECIMALS).lt(path[i].rate.dp(RATE_DECIMALS))) { + return false; + } + } + return true; +} diff --git a/packages/asset-swapper/src/utils/market_operation_utils/types.ts b/packages/asset-swapper/src/utils/market_operation_utils/types.ts index ae8c55593e..512e3aed93 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -37,9 +37,7 @@ export enum ERC20BridgeSource { } // Internal `fillData` field for `Fill` objects. -export interface FillData { - source: ERC20BridgeSource; -} +export interface FillData {} // `FillData` for native fills. export interface NativeFillData extends FillData { @@ -59,11 +57,8 @@ export interface DexSample { * Flags for `Fill` objects. */ export enum FillFlags { - SourceNative = 0x1, - SourceUniswap = 0x2, - SourceEth2Dai = 0x4, - SourceKyber = 0x8, - SourceLiquidityPool = 0x10, + ConflictsWithKyber = 0x1, + Kyber = 0x2, } /** @@ -72,20 +67,25 @@ export enum FillFlags { export interface Fill { // See `FillFlags`. flags: FillFlags; - // `FillFlags` that are incompatible with this fill, e.g., to prevent - // Kyber from mixing with Uniswap and Eth2Dai and vice versa. - exclusionMask: number; // Input fill amount (taker asset amount in a sell, maker asset amount in a buy). input: BigNumber; // Output fill amount (maker asset amount in a sell, taker asset amount in a buy). output: BigNumber; - // Output penalty for this fill. - fillPenalty: BigNumber; + // The maker/taker rate. + rate: BigNumber; + // The maker/taker rate, adjusted by fees. + adjustedRate: BigNumber; + // The output fill amount, ajdusted by fees. + adjustedOutput: BigNumber; // Fill that must precede this one. This enforces certain fills to be contiguous. parent?: Fill; + // The index of the fill in the original path. + index: number; + // The source of the fill. See `ERC20BridgeSource`. + source: ERC20BridgeSource; // Data associated with this this Fill object. Used to reconstruct orders // from paths. - fillData: FillData | NativeFillData; + fillData?: FillData | NativeFillData; } /** @@ -138,15 +138,6 @@ export interface GetMarketOrdersOpts { * Liquidity sources to exclude. Default is none. */ excludedSources: ERC20BridgeSource[]; - /** - * Whether to prevent mixing Kyber orders with Uniswap and Eth2Dai orders. - */ - noConflicts: boolean; - /** - * Complexity limit on the search algorithm, i.e., maximum number of - * nodes to visit. Default is 1024. - */ - runLimit: number; /** * When generating bridge orders, we use * sampled rate * (1 - bridgeSlippage) @@ -160,11 +151,6 @@ export interface GetMarketOrdersOpts { * Number of samples to take for each DEX quote. */ numSamples: number; - /** - * Dust amount, as a fraction of the fill amount. - * Default is 0.01 (100 basis points). - */ - dustFractionThreshold: number; /** * The exponential sampling distribution base. * A value of 1 will result in evenly spaced samples. @@ -177,6 +163,11 @@ export interface GetMarketOrdersOpts { * Fees for each liquidity source, expressed in gas. */ fees: { [source: string]: BigNumber }; + /** + * Whether to pad the quote with a redundant fallback quote using different + * sources. + */ + allowFallback: boolean; } /** diff --git a/packages/asset-swapper/src/utils/order_prune_utils.ts b/packages/asset-swapper/src/utils/order_prune_utils.ts index 6e6e039af1..35b7a72928 100644 --- a/packages/asset-swapper/src/utils/order_prune_utils.ts +++ b/packages/asset-swapper/src/utils/order_prune_utils.ts @@ -4,7 +4,7 @@ import * as _ from 'lodash'; import { constants } from '../constants'; import { OrderPrunerPermittedFeeTypes } from '../types'; -import { utils } from '../utils/utils'; +import { isOrderTakerFeePayableWithMakerAsset, isOrderTakerFeePayableWithTakerAsset } from '../utils/utils'; export const orderPrunerUtils = { pruneForUsableSignedOrders( @@ -19,9 +19,9 @@ export const orderPrunerUtils = { ((permittedOrderFeeTypes.has(OrderPrunerPermittedFeeTypes.NoFees) && order.takerFee.eq(constants.ZERO_AMOUNT)) || (permittedOrderFeeTypes.has(OrderPrunerPermittedFeeTypes.TakerDenominatedTakerFee) && - utils.isOrderTakerFeePayableWithTakerAsset(order)) || + isOrderTakerFeePayableWithTakerAsset(order)) || (permittedOrderFeeTypes.has(OrderPrunerPermittedFeeTypes.MakerDenominatedTakerFee) && - utils.isOrderTakerFeePayableWithMakerAsset(order))) + isOrderTakerFeePayableWithMakerAsset(order))) ); }); return result; diff --git a/packages/asset-swapper/src/utils/sorting_utils.ts b/packages/asset-swapper/src/utils/sorting_utils.ts index 4f41944379..84554392e8 100644 --- a/packages/asset-swapper/src/utils/sorting_utils.ts +++ b/packages/asset-swapper/src/utils/sorting_utils.ts @@ -4,7 +4,7 @@ import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; import { assert } from './assert'; -import { utils } from './utils'; +import { getAdjustedMakerAndTakerAmountsFromTakerFees } from './utils'; export const sortingUtils = { sortOrders(orders: T[]): T[] { @@ -21,9 +21,7 @@ export const sortingUtils = { }; function getTakerFeeAdjustedRateOfOrder(order: Order): BigNumber { - const [adjustedMakerAssetAmount, adjustedTakerAssetAmount] = utils.getAdjustedMakerAndTakerAmountsFromTakerFees( - order, - ); + const [adjustedMakerAssetAmount, adjustedTakerAssetAmount] = getAdjustedMakerAndTakerAmountsFromTakerFees(order); const rate = adjustedTakerAssetAmount.div(adjustedMakerAssetAmount); return rate; } diff --git a/packages/asset-swapper/src/utils/swap_quote_calculator.ts b/packages/asset-swapper/src/utils/swap_quote_calculator.ts index eb3c59396f..f4231e79a2 100644 --- a/packages/asset-swapper/src/utils/swap_quote_calculator.ts +++ b/packages/asset-swapper/src/utils/swap_quote_calculator.ts @@ -19,10 +19,14 @@ import { import { fillableAmountsUtils } from './fillable_amounts_utils'; import { MarketOperationUtils } from './market_operation_utils'; -import { CreateOrderUtils } from './market_operation_utils/create_order'; +import { convertNativeOrderToFullyFillableOptimizedOrders } from './market_operation_utils/orders'; import { ERC20BridgeSource, OptimizedMarketOrder } from './market_operation_utils/types'; import { ProtocolFeeUtils } from './protocol_fee_utils'; -import { utils } from './utils'; +import { + isOrderTakerFeePayableWithMakerAsset, + isOrderTakerFeePayableWithTakerAsset, + isSupportedAssetDataInOrders, +} from './utils'; // TODO(dave4506) How do we want to reintroduce InsufficientAssetLiquidityError? export class SwapQuoteCalculator { @@ -129,7 +133,7 @@ export class SwapQuoteCalculator { opts: CalculateSwapQuoteOpts, ): Promise { // checks if maker asset is ERC721 or ERC20 and taker asset is ERC20 - if (!utils.isSupportedAssetDataInOrders(prunedOrders)) { + if (!isSupportedAssetDataInOrders(prunedOrders)) { throw Error(SwapQuoterError.AssetDataUnsupported); } // since prunedOrders do not have fillState, we will add a buffer of fillable orders to consider that some native are orders are partially filled @@ -150,9 +154,7 @@ export class SwapQuoteCalculator { if (firstOrderMakerAssetData.assetProxyId === AssetProxyId.ERC721) { // HACK: to conform ERC721 orders to the output of market operation utils, assumes complete fillable - resultOrders = prunedOrders.map(o => - CreateOrderUtils.convertNativeOrderToFullyFillableOptimizedOrders(o), - ); + resultOrders = prunedOrders.map(o => convertNativeOrderToFullyFillableOptimizedOrders(o)); } else { if (operation === MarketOperation.Buy) { resultOrders = await this._marketOperationUtils.getMarketBuyOrdersAsync( @@ -464,7 +466,7 @@ function getTakerAssetAmountBreakDown( order: SignedOrderWithFillableAmounts, takerAssetAmountWithFees: BigNumber, ): { feeTakerAssetAmount: BigNumber; takerAssetAmount: BigNumber } { - if (utils.isOrderTakerFeePayableWithTakerAsset(order)) { + if (isOrderTakerFeePayableWithTakerAsset(order)) { const adjustedTakerAssetAmount = order.takerAssetAmount.plus(order.takerFee); const filledRatio = takerAssetAmountWithFees.div(adjustedTakerAssetAmount); const takerAssetAmount = filledRatio.multipliedBy(order.takerAssetAmount).integerValue(BigNumber.ROUND_CEIL); @@ -472,7 +474,7 @@ function getTakerAssetAmountBreakDown( takerAssetAmount, feeTakerAssetAmount: takerAssetAmountWithFees.minus(takerAssetAmount), }; - } else if (utils.isOrderTakerFeePayableWithMakerAsset(order)) { + } else if (isOrderTakerFeePayableWithMakerAsset(order)) { if (takerAssetAmountWithFees.isZero()) { return { takerAssetAmount: constants.ZERO_AMOUNT, diff --git a/packages/asset-swapper/src/utils/swap_quote_consumer_utils.ts b/packages/asset-swapper/src/utils/swap_quote_consumer_utils.ts index 8cd5815960..075c770750 100644 --- a/packages/asset-swapper/src/utils/swap_quote_consumer_utils.ts +++ b/packages/asset-swapper/src/utils/swap_quote_consumer_utils.ts @@ -15,9 +15,9 @@ import { SwapQuoteConsumerError, SwapQuoteExecutionOpts, } from '../types'; -import { utils } from '../utils/utils'; import { assert } from './assert'; +import { isExactAssetData } from './utils'; export const swapQuoteConsumerUtils = { async getTakerAddressOrThrowAsync( @@ -66,7 +66,7 @@ export const swapQuoteConsumerUtils = { return _.every(orders, order => swapQuoteConsumerUtils.isValidForwarderSignedOrder(order, wethAssetData)); }, isValidForwarderSignedOrder(order: SignedOrder, wethAssetData: string): boolean { - return utils.isExactAssetData(order.takerAssetData, wethAssetData); + return isExactAssetData(order.takerAssetData, wethAssetData); }, async getExtensionContractTypeForSwapQuoteAsync( quote: SwapQuote, diff --git a/packages/asset-swapper/src/utils/utils.ts b/packages/asset-swapper/src/utils/utils.ts index 9bb80bb803..4be46b773c 100644 --- a/packages/asset-swapper/src/utils/utils.ts +++ b/packages/asset-swapper/src/utils/utils.ts @@ -5,103 +5,111 @@ import { Web3Wrapper } from '@0x/web3-wrapper'; import { constants } from '../constants'; -// tslint:disable:no-unnecessary-type-assertion -export const utils = { - isSupportedAssetDataInOrders(orders: SignedOrder[]): boolean { - const firstOrderMakerAssetData = !!orders[0] - ? assetDataUtils.decodeAssetDataOrThrow(orders[0].makerAssetData) - : { assetProxyId: '' }; - return orders.every(o => { - const takerAssetData = assetDataUtils.decodeAssetDataOrThrow(o.takerAssetData); - const makerAssetData = assetDataUtils.decodeAssetDataOrThrow(o.makerAssetData); - return ( - (makerAssetData.assetProxyId === AssetProxyId.ERC20 || - makerAssetData.assetProxyId === AssetProxyId.ERC721) && - takerAssetData.assetProxyId === AssetProxyId.ERC20 && - firstOrderMakerAssetData.assetProxyId === makerAssetData.assetProxyId - ); // checks that all native order maker assets are of the same type - }); - }, - numberPercentageToEtherTokenAmountPercentage(percentage: number): BigNumber { - return Web3Wrapper.toBaseUnitAmount(constants.ONE_AMOUNT, constants.ETHER_TOKEN_DECIMALS).multipliedBy( - percentage, - ); - }, - isOrderTakerFeePayableWithMakerAsset(order: T): boolean { - return !order.takerFee.isZero() && utils.isAssetDataEquivalent(order.takerFeeAssetData, order.makerAssetData); - }, - isOrderTakerFeePayableWithTakerAsset(order: T): boolean { - return !order.takerFee.isZero() && utils.isAssetDataEquivalent(order.takerFeeAssetData, order.takerAssetData); - }, - getAdjustedMakerAndTakerAmountsFromTakerFees(order: T): [BigNumber, BigNumber] { - const adjustedMakerAssetAmount = utils.isOrderTakerFeePayableWithMakerAsset(order) - ? order.makerAssetAmount.minus(order.takerFee) - : order.makerAssetAmount; - const adjustedTakerAssetAmount = utils.isOrderTakerFeePayableWithTakerAsset(order) - ? order.takerAssetAmount.plus(order.takerFee) - : order.takerAssetAmount; - return [adjustedMakerAssetAmount, adjustedTakerAssetAmount]; - }, - isExactAssetData(expectedAssetData: string, actualAssetData: string): boolean { - return expectedAssetData === actualAssetData; - }, - /** - * Compare the Asset Data for equivalency. Expected is the asset data the user provided (wanted), - * actual is the asset data found or created. - */ - isAssetDataEquivalent(expectedAssetData: string, actualAssetData: string): boolean { - if (utils.isExactAssetData(expectedAssetData, actualAssetData)) { - return true; - } - const decodedExpectedAssetData = assetDataUtils.decodeAssetDataOrThrow(expectedAssetData); - const decodedActualAssetData = assetDataUtils.decodeAssetDataOrThrow(actualAssetData); - // ERC20 === ERC20, ERC20 === ERC20Bridge - if ( - utils.isERC20EquivalentAssetData(decodedExpectedAssetData) && - utils.isERC20EquivalentAssetData(decodedActualAssetData) - ) { - const doesTokenAddressMatch = decodedExpectedAssetData.tokenAddress === decodedActualAssetData.tokenAddress; - return doesTokenAddressMatch; - } - // ERC1155 === ERC1155 - if ( - assetDataUtils.isERC1155TokenAssetData(decodedExpectedAssetData) && - assetDataUtils.isERC1155TokenAssetData(decodedActualAssetData) - ) { - const doesTokenAddressMatch = decodedExpectedAssetData.tokenAddress === decodedActualAssetData.tokenAddress; - // IDs may be out of order yet still equivalent - // i.e (["a", "b"], [1,2]) === (["b", "a"], [2, 1]) - // (["a", "b"], [2,1]) !== (["b", "a"], [2, 1]) - const hasAllIds = decodedExpectedAssetData.tokenIds.every( - id => decodedActualAssetData.tokenIds.findIndex(v => id.eq(v)) !== -1, - ); - const hasAllValues = decodedExpectedAssetData.tokenIds.every((id, i) => - decodedExpectedAssetData.tokenValues[i].eq( - decodedActualAssetData.tokenValues[decodedActualAssetData.tokenIds.findIndex(v => id.eq(v))], - ), - ); - // If expected contains callback data, ensure it is present - // if actual has callbackdata and expected provided none then ignore it - const hasEquivalentCallback = - decodedExpectedAssetData.callbackData === NULL_BYTES || - decodedExpectedAssetData.callbackData === decodedActualAssetData.callbackData; - return doesTokenAddressMatch && hasAllIds && hasAllValues && hasEquivalentCallback; - } - // ERC721 === ERC721 - if ( - assetDataUtils.isERC721TokenAssetData(decodedExpectedAssetData) || - assetDataUtils.isERC721TokenAssetData(decodedActualAssetData) - ) { - // Asset Data should exactly match for ERC721 - return utils.isExactAssetData(expectedAssetData, actualAssetData); - } +// tslint:disable: no-unnecessary-type-assertion completed-docs - // TODO(dekz): Unsupported cases - // ERCXX(token) === MAP(token, staticCall) - // MAP(a, b) === MAP(b, a) === MAP(b, a, staticCall) - return false; - }, - isERC20EquivalentAssetData(assetData: AssetData): assetData is ERC20AssetData | ERC20BridgeAssetData { - return assetDataUtils.isERC20TokenAssetData(assetData) || assetDataUtils.isERC20BridgeAssetData(assetData); - }, -}; +export function isSupportedAssetDataInOrders(orders: SignedOrder[]): boolean { + const firstOrderMakerAssetData = !!orders[0] + ? assetDataUtils.decodeAssetDataOrThrow(orders[0].makerAssetData) + : { assetProxyId: '' }; + return orders.every(o => { + const takerAssetData = assetDataUtils.decodeAssetDataOrThrow(o.takerAssetData); + const makerAssetData = assetDataUtils.decodeAssetDataOrThrow(o.makerAssetData); + return ( + (makerAssetData.assetProxyId === AssetProxyId.ERC20 || + makerAssetData.assetProxyId === AssetProxyId.ERC721) && + takerAssetData.assetProxyId === AssetProxyId.ERC20 && + firstOrderMakerAssetData.assetProxyId === makerAssetData.assetProxyId + ); // checks that all native order maker assets are of the same type + }); +} + +export function numberPercentageToEtherTokenAmountPercentage(percentage: number): BigNumber { + return Web3Wrapper.toBaseUnitAmount(constants.ONE_AMOUNT, constants.ETHER_TOKEN_DECIMALS).multipliedBy(percentage); +} + +export function isOrderTakerFeePayableWithMakerAsset(order: T): boolean { + return !order.takerFee.isZero() && isAssetDataEquivalent(order.takerFeeAssetData, order.makerAssetData); +} + +export function isOrderTakerFeePayableWithTakerAsset(order: T): boolean { + return !order.takerFee.isZero() && isAssetDataEquivalent(order.takerFeeAssetData, order.takerAssetData); +} + +export function getAdjustedMakerAndTakerAmountsFromTakerFees(order: T): [BigNumber, BigNumber] { + const adjustedMakerAssetAmount = isOrderTakerFeePayableWithMakerAsset(order) + ? order.makerAssetAmount.minus(order.takerFee) + : order.makerAssetAmount; + const adjustedTakerAssetAmount = isOrderTakerFeePayableWithTakerAsset(order) + ? order.takerAssetAmount.plus(order.takerFee) + : order.takerAssetAmount; + return [adjustedMakerAssetAmount, adjustedTakerAssetAmount]; +} + +export function isExactAssetData(expectedAssetData: string, actualAssetData: string): boolean { + return expectedAssetData === actualAssetData; +} + +/** + * Compare the Asset Data for equivalency. Expected is the asset data the user provided (wanted), + * actual is the asset data found or created. + */ +export function isAssetDataEquivalent(expectedAssetData: string, actualAssetData: string): boolean { + if (isExactAssetData(expectedAssetData, actualAssetData)) { + return true; + } + const decodedExpectedAssetData = assetDataUtils.decodeAssetDataOrThrow(expectedAssetData); + const decodedActualAssetData = assetDataUtils.decodeAssetDataOrThrow(actualAssetData); + // ERC20 === ERC20, ERC20 === ERC20Bridge + if (isERC20EquivalentAssetData(decodedExpectedAssetData) && isERC20EquivalentAssetData(decodedActualAssetData)) { + const doesTokenAddressMatch = decodedExpectedAssetData.tokenAddress === decodedActualAssetData.tokenAddress; + return doesTokenAddressMatch; + } + // ERC1155 === ERC1155 + if ( + assetDataUtils.isERC1155TokenAssetData(decodedExpectedAssetData) && + assetDataUtils.isERC1155TokenAssetData(decodedActualAssetData) + ) { + const doesTokenAddressMatch = decodedExpectedAssetData.tokenAddress === decodedActualAssetData.tokenAddress; + // IDs may be out of order yet still equivalent + // i.e (["a", "b"], [1,2]) === (["b", "a"], [2, 1]) + // (["a", "b"], [2,1]) !== (["b", "a"], [2, 1]) + const hasAllIds = decodedExpectedAssetData.tokenIds.every( + id => decodedActualAssetData.tokenIds.findIndex(v => id.eq(v)) !== -1, + ); + const hasAllValues = decodedExpectedAssetData.tokenIds.every((id, i) => + decodedExpectedAssetData.tokenValues[i].eq( + decodedActualAssetData.tokenValues[decodedActualAssetData.tokenIds.findIndex(v => id.eq(v))], + ), + ); + // If expected contains callback data, ensure it is present + // if actual has callbackdata and expected provided none then ignore it + const hasEquivalentCallback = + decodedExpectedAssetData.callbackData === NULL_BYTES || + decodedExpectedAssetData.callbackData === decodedActualAssetData.callbackData; + return doesTokenAddressMatch && hasAllIds && hasAllValues && hasEquivalentCallback; + } + // ERC721 === ERC721 + if ( + assetDataUtils.isERC721TokenAssetData(decodedExpectedAssetData) || + assetDataUtils.isERC721TokenAssetData(decodedActualAssetData) + ) { + // Asset Data should exactly match for ERC721 + return isExactAssetData(expectedAssetData, actualAssetData); + } + + // TODO(dekz): Unsupported cases + // ERCXX(token) === MAP(token, staticCall) + // MAP(a, b) === MAP(b, a) === MAP(b, a, staticCall) + return false; +} + +export function isERC20EquivalentAssetData(assetData: AssetData): assetData is ERC20AssetData | ERC20BridgeAssetData { + return assetDataUtils.isERC20TokenAssetData(assetData) || assetDataUtils.isERC20BridgeAssetData(assetData); +} + +/** + * Gets the difference between two sets. + */ +export function difference(a: T[], b: T[]): T[] { + return a.filter(x => b.indexOf(x) === -1); +} diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 5375327269..16871bd873 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -14,14 +14,11 @@ import { AssetProxyId, ERC20BridgeAssetData, SignedOrder } from '@0x/types'; import { BigNumber, fromTokenUnitAmount, hexUtils, NULL_ADDRESS } from '@0x/utils'; import * as _ from 'lodash'; -import { constants as assetSwapperConstants } from '../src/constants'; import { MarketOperationUtils } from '../src/utils/market_operation_utils/'; -import { constants as marketOperationUtilConstants } from '../src/utils/market_operation_utils/constants'; +import { BUY_SOURCES, DEFAULT_CURVE_OPTS, SELL_SOURCES } from '../src/utils/market_operation_utils/constants'; import { DexOrderSampler } from '../src/utils/market_operation_utils/sampler'; import { DexSample, ERC20BridgeSource } from '../src/utils/market_operation_utils/types'; -const { BUY_SOURCES, SELL_SOURCES } = marketOperationUtilConstants; - // tslint:disable: custom-no-magic-numbers describe('MarketOperationUtils tests', () => { const CHAIN_ID = 1; @@ -81,8 +78,8 @@ describe('MarketOperationUtils tests', () => { case UNISWAP_BRIDGE_ADDRESS.toLowerCase(): return ERC20BridgeSource.Uniswap; case CURVE_BRIDGE_ADDRESS.toLowerCase(): - const curveSource = Object.keys(assetSwapperConstants.DEFAULT_CURVE_OPTS).filter( - k => assetData.indexOf(assetSwapperConstants.DEFAULT_CURVE_OPTS[k].curveAddress.slice(2)) !== -1, + const curveSource = Object.keys(DEFAULT_CURVE_OPTS).filter( + k => assetData.indexOf(DEFAULT_CURVE_OPTS[k].curveAddress.slice(2)) !== -1, ); return curveSource[0] as ERC20BridgeSource; default: @@ -120,20 +117,21 @@ describe('MarketOperationUtils tests', () => { chainId: CHAIN_ID, }; - type GetQuotesOperation = (makerToken: string, takerToken: string, fillAmounts: BigNumber[]) => BigNumber[]; - - function createGetSellQuotesOperationFromRates(rates: Numberish[]): GetQuotesOperation { - return (...args) => { - const fillAmounts = args.pop() as BigNumber[]; - return fillAmounts.map((a, i) => a.times(rates[i]).integerValue()); - }; - } - - function createGetBuyQuotesOperationFromRates(rates: Numberish[]): GetQuotesOperation { - return (...args) => { - const fillAmounts = args.pop() as BigNumber[]; - return fillAmounts.map((a, i) => a.div(rates[i]).integerValue()); - }; + function createSamplesFromRates(source: ERC20BridgeSource, inputs: Numberish[], rates: Numberish[]): DexSample[] { + const samples: DexSample[] = []; + inputs.forEach((input, i) => { + const rate = rates[i]; + samples.push({ + source, + input: new BigNumber(input), + output: new BigNumber(input) + .minus(i === 0 ? 0 : samples[i - 1].input) + .times(rate) + .plus(i === 0 ? 0 : samples[i - 1].output) + .integerValue(), + }); + }); + return samples; } type GetMultipleQuotesOperation = ( @@ -146,13 +144,7 @@ describe('MarketOperationUtils tests', () => { function createGetMultipleSellQuotesOperationFromRates(rates: RatesBySource): GetMultipleQuotesOperation { return (sources: ERC20BridgeSource[], makerToken: string, takerToken: string, fillAmounts: BigNumber[]) => { - return sources.map(s => - fillAmounts.map((a, i) => ({ - source: s, - input: a, - output: a.times(rates[s][i]).integerValue(), - })), - ); + return sources.map(s => createSamplesFromRates(s, fillAmounts, rates[s])); }; } @@ -180,13 +172,7 @@ describe('MarketOperationUtils tests', () => { function createGetMultipleBuyQuotesOperationFromRates(rates: RatesBySource): GetMultipleQuotesOperation { return (sources: ERC20BridgeSource[], makerToken: string, takerToken: string, fillAmounts: BigNumber[]) => { - return sources.map(s => - fillAmounts.map((a, i) => ({ - source: s, - input: a, - output: a.div(rates[s][i]).integerValue(), - })), - ); + return sources.map(s => createSamplesFromRates(s, fillAmounts, rates[s].map(r => new BigNumber(1).div(r)))); }; } @@ -264,22 +250,6 @@ describe('MarketOperationUtils tests', () => { [ERC20BridgeSource.LiquidityProvider]: _.times(NUM_SAMPLES, () => 0), }; - function findSourceWithMaxOutput(rates: RatesBySource): ERC20BridgeSource { - const minSourceRates = Object.keys(rates).map(s => _.last(rates[s]) as BigNumber); - const bestSourceRate = BigNumber.max(...minSourceRates); - let source = Object.keys(rates)[_.findIndex(minSourceRates, t => bestSourceRate.eq(t))] as ERC20BridgeSource; - // Native order rates play by different rules. - if (source !== ERC20BridgeSource.Native) { - const nativeTotalRate = BigNumber.sum(...rates[ERC20BridgeSource.Native]).div( - rates[ERC20BridgeSource.Native].length, - ); - if (nativeTotalRate.gt(bestSourceRate)) { - source = ERC20BridgeSource.Native; - } - } - return source; - } - const DEFAULT_OPS = { getOrderFillableTakerAmounts(orders: SignedOrder[]): BigNumber[] { return orders.map(o => o.takerAssetAmount); @@ -287,12 +257,6 @@ describe('MarketOperationUtils tests', () => { getOrderFillableMakerAmounts(orders: SignedOrder[]): BigNumber[] { return orders.map(o => o.makerAssetAmount); }, - getKyberSellQuotes: createGetSellQuotesOperationFromRates(DEFAULT_RATES[ERC20BridgeSource.Kyber]), - getUniswapSellQuotes: createGetSellQuotesOperationFromRates(DEFAULT_RATES[ERC20BridgeSource.Uniswap]), - getEth2DaiSellQuotes: createGetSellQuotesOperationFromRates(DEFAULT_RATES[ERC20BridgeSource.Eth2Dai]), - getUniswapBuyQuotes: createGetBuyQuotesOperationFromRates(DEFAULT_RATES[ERC20BridgeSource.Uniswap]), - getEth2DaiBuyQuotes: createGetBuyQuotesOperationFromRates(DEFAULT_RATES[ERC20BridgeSource.Eth2Dai]), - getCurveSellQuotes: createGetSellQuotesOperationFromRates(DEFAULT_RATES[ERC20BridgeSource.CurveUsdcDai]), getSellQuotes: createGetMultipleSellQuotesOperationFromRates(DEFAULT_RATES), getBuyQuotes: createGetMultipleBuyQuotesOperationFromRates(DEFAULT_RATES), getMedianSellRate: createGetMedianSellRate(1), @@ -330,10 +294,10 @@ describe('MarketOperationUtils tests', () => { ); const DEFAULT_OPTS = { numSamples: NUM_SAMPLES, - runLimit: 0, sampleDistributionBase: 1, bridgeSlippage: 0, - excludedSources: Object.keys(assetSwapperConstants.DEFAULT_CURVE_OPTS) as ERC20BridgeSource[], + excludedSources: Object.keys(DEFAULT_CURVE_OPTS) as ERC20BridgeSource[], + allowFallback: false, }; beforeEach(() => { @@ -341,7 +305,7 @@ describe('MarketOperationUtils tests', () => { }); it('queries `numSamples` samples', async () => { - const numSamples = _.random(1, 16); + const numSamples = _.random(1, NUM_SAMPLES); let actualNumSamples = 0; replaceSamplerOps({ getSellQuotes: (sources, makerToken, takerToken, amounts) => { @@ -412,18 +376,6 @@ describe('MarketOperationUtils tests', () => { expect(sourcesPolled.sort()).to.deep.eq(_.without(SELL_SOURCES, ...excludedSources).sort()); }); - it('returns the most cost-effective single source if `runLimit == 0`', async () => { - const bestSource = findSourceWithMaxOutput(DEFAULT_RATES); - expect(bestSource).to.exist(''); - const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync(ORDERS, FILL_AMOUNT, { - ...DEFAULT_OPTS, - runLimit: 0, - }); - const uniqueAssetDatas = _.uniq(improvedOrders.map(o => o.makerAssetData)); - expect(uniqueAssetDatas).to.be.length(1); - expect(getSourceFromAssetData(uniqueAssetDatas[0])).to.be.eq(bestSource); - }); - it('generates bridge orders with correct asset data', async () => { const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync( // Pass in empty orders to prevent native orders from being used. @@ -469,10 +421,9 @@ describe('MarketOperationUtils tests', () => { ); expect(improvedOrders).to.not.be.length(0); for (const order of improvedOrders) { - const source = getSourceFromAssetData(order.makerAssetData); - const expectedMakerAmount = FILL_AMOUNT.times(_.last(DEFAULT_RATES[source]) as BigNumber); + const expectedMakerAmount = order.fill.totalMakerAssetAmount; const slippage = 1 - order.makerAssetAmount.div(expectedMakerAmount.plus(1)).toNumber(); - assertRoughlyEquals(slippage, bridgeSlippage, 8); + assertRoughlyEquals(slippage, bridgeSlippage, 1); } }); @@ -488,19 +439,19 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync( createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512, noConflicts: false }, + { ...DEFAULT_OPTS, numSamples: 4 }, ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ ERC20BridgeSource.Kyber, - ERC20BridgeSource.Eth2Dai, - ERC20BridgeSource.Uniswap, + ERC20BridgeSource.Native, + ERC20BridgeSource.Native, ERC20BridgeSource.Native, ]; expect(orderSources).to.deep.eq(expectedSources); }); - it('excludes Kyber when `noConflicts` enabled and Uniswap or Eth2Dai are used first', async () => { + it('excludes Kyber when Uniswap or Eth2Dai are used first', async () => { const rates: RatesBySource = {}; rates[ERC20BridgeSource.Native] = [0.3, 0.2, 0.1, 0.05]; rates[ERC20BridgeSource.Uniswap] = [0.5, 0.05, 0.05, 0.05]; @@ -512,7 +463,7 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync( createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512, noConflicts: true }, + { ...DEFAULT_OPTS, numSamples: 4 }, ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ @@ -524,7 +475,7 @@ describe('MarketOperationUtils tests', () => { expect(orderSources).to.deep.eq(expectedSources); }); - it('excludes Uniswap and Eth2Dai when `noConflicts` enabled and Kyber is used first', async () => { + it('excludes Uniswap and Eth2Dai when Kyber is used first', async () => { const rates: RatesBySource = {}; rates[ERC20BridgeSource.Native] = [0.1, 0.05, 0.05, 0.05]; rates[ERC20BridgeSource.Uniswap] = [0.15, 0.05, 0.05, 0.05]; @@ -536,15 +487,10 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync( createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512, noConflicts: true }, + { ...DEFAULT_OPTS, numSamples: 4 }, ); const orderSources = improvedOrders.map(o => o.fill.source); - const expectedSources = [ - ERC20BridgeSource.Kyber, - ERC20BridgeSource.Native, - ERC20BridgeSource.Native, - ERC20BridgeSource.Native, - ]; + const expectedSources = [ERC20BridgeSource.Kyber, ERC20BridgeSource.Native]; expect(orderSources).to.deep.eq(expectedSources); }); @@ -572,7 +518,7 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync( createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512, noConflicts: false, fees }, + { ...DEFAULT_OPTS, numSamples: 4, fees }, ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ @@ -587,17 +533,17 @@ describe('MarketOperationUtils tests', () => { it('factors in fees for dexes', async () => { // Kyber will have the best rates but will have fees, // dropping its effective rates. - const kyberFeeRate = 0.2; + const uniswapFeeRate = 0.2; const rates: RatesBySource = { [ERC20BridgeSource.Native]: [0.95, 0.1, 0.1, 0.1], - [ERC20BridgeSource.Uniswap]: [0.1, 0.1, 0.1, 0.1], + [ERC20BridgeSource.Kyber]: [0.1, 0.1, 0.1, 0.1], [ERC20BridgeSource.Eth2Dai]: [0.92, 0.1, 0.1, 0.1], // Effectively [0.8, ~0.5, ~0, ~0] - [ERC20BridgeSource.Kyber]: [1, 0.7, 0.2, 0.2], + [ERC20BridgeSource.Uniswap]: [1, 0.7, 0.2, 0.2], }; const fees = { - [ERC20BridgeSource.Kyber]: FILL_AMOUNT.div(4) - .times(kyberFeeRate) + [ERC20BridgeSource.Uniswap]: FILL_AMOUNT.div(4) + .times(uniswapFeeRate) .dividedToIntegerBy(ETH_TO_MAKER_RATE), }; replaceSamplerOps({ @@ -607,10 +553,65 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync( createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512, noConflicts: false, fees }, + { ...DEFAULT_OPTS, numSamples: 4, fees }, ); const orderSources = improvedOrders.map(o => o.fill.source); - const expectedSources = [ERC20BridgeSource.Native, ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Kyber]; + const expectedSources = [ + ERC20BridgeSource.Native, + ERC20BridgeSource.Eth2Dai, + ERC20BridgeSource.Uniswap, + ]; + expect(orderSources).to.deep.eq(expectedSources); + }); + + it('can mix one concave source', async () => { + const rates: RatesBySource = { + [ERC20BridgeSource.Kyber]: [0, 0, 0, 0], // Won't use + [ERC20BridgeSource.Eth2Dai]: [0.5, 0.85, 0.75, 0.75], // Concave + [ERC20BridgeSource.Uniswap]: [0.96, 0.2, 0.1, 0.1], + [ERC20BridgeSource.Native]: [0.95, 0.2, 0.2, 0.1], + }; + replaceSamplerOps({ + getSellQuotes: createGetMultipleSellQuotesOperationFromRates(rates), + getMedianSellRate: createGetMedianSellRate(ETH_TO_MAKER_RATE), + }); + const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync( + createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), + FILL_AMOUNT, + { ...DEFAULT_OPTS, numSamples: 4 }, + ); + const orderSources = improvedOrders.map(o => o.fill.source); + const expectedSources = [ + ERC20BridgeSource.Eth2Dai, + ERC20BridgeSource.Uniswap, + ERC20BridgeSource.Native, + ]; + expect(orderSources).to.deep.eq(expectedSources); + }); + + it('fallback orders use different sources', async () => { + const rates: RatesBySource = {}; + rates[ERC20BridgeSource.Eth2Dai] = [0.9, 0.8, 0.5, 0.5]; + rates[ERC20BridgeSource.Uniswap] = [0.6, 0.05, 0.01, 0.01]; + rates[ERC20BridgeSource.Native] = [0.4, 0.3, 0.01, 0.01]; + 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 }, + ); + const orderSources = improvedOrders.map(o => o.fill.source); + const expectedSources = [ + ERC20BridgeSource.Eth2Dai, + ERC20BridgeSource.Uniswap, + // Fallback + ERC20BridgeSource.Native, + ERC20BridgeSource.Kyber, + ERC20BridgeSource.Native, + ]; expect(orderSources).to.deep.eq(expectedSources); }); @@ -679,9 +680,9 @@ describe('MarketOperationUtils tests', () => { ); const DEFAULT_OPTS = { numSamples: NUM_SAMPLES, - runLimit: 0, sampleDistributionBase: 1, - excludedSources: Object.keys(assetSwapperConstants.DEFAULT_CURVE_OPTS) as ERC20BridgeSource[], + excludedSources: Object.keys(DEFAULT_CURVE_OPTS) as ERC20BridgeSource[], + allowFallback: false, }; beforeEach(() => { @@ -760,26 +761,6 @@ describe('MarketOperationUtils tests', () => { expect(sourcesPolled).to.deep.eq(_.without(BUY_SOURCES, ...excludedSources)); }); - it('returns the most cost-effective single source if `runLimit == 0`', async () => { - const bestSource = findSourceWithMaxOutput( - _.omit( - DEFAULT_RATES, - ERC20BridgeSource.Kyber, - ERC20BridgeSource.CurveUsdcDai, - ERC20BridgeSource.CurveUsdcDaiUsdt, - ERC20BridgeSource.CurveUsdcDaiUsdtTusd, - ), - ); - expect(bestSource).to.exist(''); - const improvedOrders = await marketOperationUtils.getMarketBuyOrdersAsync(ORDERS, FILL_AMOUNT, { - ...DEFAULT_OPTS, - runLimit: 0, - }); - const uniqueAssetDatas = _.uniq(improvedOrders.map(o => o.makerAssetData)); - expect(uniqueAssetDatas).to.be.length(1); - expect(getSourceFromAssetData(uniqueAssetDatas[0])).to.be.eq(bestSource); - }); - it('generates bridge orders with correct asset data', async () => { const improvedOrders = await marketOperationUtils.getMarketBuyOrdersAsync( // Pass in empty orders to prevent native orders from being used. @@ -825,10 +806,9 @@ describe('MarketOperationUtils tests', () => { ); expect(improvedOrders).to.not.be.length(0); for (const order of improvedOrders) { - const source = getSourceFromAssetData(order.makerAssetData); - const expectedTakerAmount = FILL_AMOUNT.div(_.last(DEFAULT_RATES[source]) as BigNumber); + const expectedTakerAmount = order.fill.totalTakerAssetAmount; const slippage = order.takerAssetAmount.div(expectedTakerAmount.plus(1)).toNumber() - 1; - assertRoughlyEquals(slippage, bridgeSlippage, 8); + assertRoughlyEquals(slippage, bridgeSlippage, 1); } }); @@ -843,7 +823,7 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketBuyOrdersAsync( createOrdersFromBuyRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512 }, + { ...DEFAULT_OPTS, numSamples: 4 }, ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ @@ -879,7 +859,7 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketBuyOrdersAsync( createOrdersFromBuyRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512, fees }, + { ...DEFAULT_OPTS, numSamples: 4, fees }, ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ @@ -913,7 +893,7 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketBuyOrdersAsync( createOrdersFromBuyRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, runLimit: 512, fees }, + { ...DEFAULT_OPTS, numSamples: 4, fees }, ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ @@ -923,6 +903,32 @@ describe('MarketOperationUtils tests', () => { ]; expect(orderSources).to.deep.eq(expectedSources); }); + + it('fallback orders use different sources', async () => { + const rates: RatesBySource = {}; + rates[ERC20BridgeSource.Eth2Dai] = [0.9, 0.8, 0.5, 0.5]; + rates[ERC20BridgeSource.Uniswap] = [0.6, 0.05, 0.01, 0.01]; + rates[ERC20BridgeSource.Native] = [0.4, 0.3, 0.01, 0.01]; + replaceSamplerOps({ + getBuyQuotes: createGetMultipleBuyQuotesOperationFromRates(rates), + }); + const improvedOrders = await marketOperationUtils.getMarketBuyOrdersAsync( + createOrdersFromBuyRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), + FILL_AMOUNT, + { ...DEFAULT_OPTS, numSamples: 4, allowFallback: true }, + ); + const orderSources = improvedOrders.map(o => o.fill.source); + const expectedSources = [ + ERC20BridgeSource.Eth2Dai, + ERC20BridgeSource.Uniswap, + // Fallback + ERC20BridgeSource.Native, + ERC20BridgeSource.Native, + ERC20BridgeSource.Native, + ERC20BridgeSource.Native, + ]; + expect(orderSources).to.deep.eq(expectedSources); + }); }); }); }); diff --git a/packages/asset-swapper/test/swap_quote_calculator_test.ts b/packages/asset-swapper/test/swap_quote_calculator_test.ts index d740c42e6f..d63d1f8fba 100644 --- a/packages/asset-swapper/test/swap_quote_calculator_test.ts +++ b/packages/asset-swapper/test/swap_quote_calculator_test.ts @@ -9,7 +9,7 @@ import 'mocha'; import { constants } from '../src/constants'; import { CalculateSwapQuoteOpts, SignedOrderWithFillableAmounts } from '../src/types'; import { DexOrderSampler, MarketOperationUtils } from '../src/utils/market_operation_utils/'; -import { constants as marketOperationUtilConstants } from '../src/utils/market_operation_utils/constants'; +import { DEFAULT_GET_MARKET_ORDERS_OPTS, SELL_SOURCES } from '../src/utils/market_operation_utils/constants'; import { ProtocolFeeUtils } from '../src/utils/protocol_fee_utils'; import { SwapQuoteCalculator } from '../src/utils/swap_quote_calculator'; @@ -33,8 +33,6 @@ const ONE_ETH_IN_WEI = new BigNumber(1000000000000000000); // ); const TESTRPC_CHAIN_ID = devConstants.TESTRPC_CHAIN_ID; -const { DEFAULT_GET_MARKET_ORDERS_OPTS, SELL_SOURCES } = marketOperationUtilConstants; - // Excludes all non native sources const CALCULATE_SWAP_QUOTE_OPTS: CalculateSwapQuoteOpts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, diff --git a/packages/asset-swapper/test/utils_test.ts b/packages/asset-swapper/test/utils_test.ts index 8236c1b46a..2ee2cb13bc 100644 --- a/packages/asset-swapper/test/utils_test.ts +++ b/packages/asset-swapper/test/utils_test.ts @@ -4,7 +4,7 @@ import { BigNumber, NULL_ADDRESS, NULL_BYTES } from '@0x/utils'; import * as chai from 'chai'; import 'mocha'; -import { utils } from '../src/utils/utils'; +import { isAssetDataEquivalent } from '../src/utils/utils'; import { chaiSetup } from './utils/chai_setup'; @@ -16,35 +16,35 @@ describe('utils', () => { describe('ERC20', () => { const [tokenA, tokenB] = tokenUtils.getDummyERC20TokenAddresses(); it('should succeed ERC20 to be ERC20Bridge', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC20AssetData(tokenA), assetDataUtils.encodeERC20BridgeAssetData(tokenA, NULL_ADDRESS, NULL_BYTES), ); expect(isEquivalent).to.be.true(); }); it('should succeed ERC20Bridge to be ERC20', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC20BridgeAssetData(tokenA, NULL_ADDRESS, NULL_BYTES), assetDataUtils.encodeERC20AssetData(tokenA), ); expect(isEquivalent).to.be.true(); }); it('should succeed ERC20 to be ERC20', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC20AssetData(tokenA), assetDataUtils.encodeERC20AssetData(tokenA), ); expect(isEquivalent).to.be.true(); }); it('should fail if ERC20Bridge is not the same ERC20 token', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC20AssetData(tokenA), assetDataUtils.encodeERC20BridgeAssetData(tokenB, NULL_ADDRESS, NULL_BYTES), ); expect(isEquivalent).to.be.false(); }); it('should fail if ERC20 is not the same ERC20 token', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC20AssetData(tokenA), assetDataUtils.encodeERC20AssetData(tokenB), ); @@ -56,28 +56,28 @@ describe('utils', () => { const tokenIdA = new BigNumber(1); const tokenIdB = new BigNumber(2); it('should succeed if ERC721 the same ERC721 token and id', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC721AssetData(tokenA, tokenIdA), assetDataUtils.encodeERC721AssetData(tokenA, tokenIdA), ); expect(isEquivalent).to.be.true(); }); it('should fail if ERC721 is not the same ERC721 token', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC721AssetData(tokenA, tokenIdA), assetDataUtils.encodeERC721AssetData(tokenB, tokenIdA), ); expect(isEquivalent).to.be.false(); }); it('should fail if ERC721 is not the same ERC721 id', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC721AssetData(tokenA, tokenIdA), assetDataUtils.encodeERC721AssetData(tokenA, tokenIdB), ); expect(isEquivalent).to.be.false(); }); it('should fail if ERC721 is compared with ERC20', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC721AssetData(tokenA, tokenIdA), assetDataUtils.encodeERC20AssetData(tokenA), ); @@ -91,49 +91,49 @@ describe('utils', () => { const valueA = new BigNumber(1); const valueB = new BigNumber(2); it('should succeed if ERC1155 is the same ERC1155 token and id', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA], [valueA], NULL_BYTES), assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA], [valueA], NULL_BYTES), ); expect(isEquivalent).to.be.true(); }); it('should succeed if ERC1155 is the same ERC1155 token and ids', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA, tokenIdB], [valueA, valueB], NULL_BYTES), assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA, tokenIdB], [valueA, valueB], NULL_BYTES), ); expect(isEquivalent).to.be.true(); }); it('should succeed if ERC1155 is the same ERC1155 token and ids in different orders', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdB, tokenIdA], [valueB, valueA], NULL_BYTES), assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA, tokenIdB], [valueA, valueB], NULL_BYTES), ); expect(isEquivalent).to.be.true(); }); it('should succeed if ERC1155 is the same ERC1155 token and ids with different callback data', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdB, tokenIdA], [valueB, valueA], NULL_BYTES), assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA, tokenIdB], [valueA, valueB], tokenA), ); expect(isEquivalent).to.be.true(); }); it('should fail if ERC1155 contains different ids', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdB, tokenIdA], [valueB, valueA], NULL_BYTES), assetDataUtils.encodeERC1155AssetData(tokenB, [tokenIdA], [valueB], NULL_BYTES), ); expect(isEquivalent).to.be.false(); }); it('should fail if ERC1155 is a different ERC1155 token', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdB, tokenIdA], [valueB, valueA], NULL_BYTES), assetDataUtils.encodeERC1155AssetData(tokenB, [tokenIdA, tokenIdB], [valueA, valueB], NULL_BYTES), ); expect(isEquivalent).to.be.false(); }); it('should fail if expected ERC1155 has different callback data', () => { - const isEquivalent = utils.isAssetDataEquivalent( + const isEquivalent = isAssetDataEquivalent( assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdB, tokenIdA], [valueB, valueA], tokenA), assetDataUtils.encodeERC1155AssetData(tokenA, [tokenIdA, tokenIdB], [valueA, valueB], NULL_BYTES), ); From f901c401b73d0d5ed4e15b28503aa7632a637154 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Fri, 6 Mar 2020 16:55:23 -0500 Subject: [PATCH 02/14] rebase --- .../src/utils/market_operation_utils/sampler_operations.ts | 1 + 1 file changed, 1 insertion(+) 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 f5fbf71482..ebf444d42f 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 @@ -211,6 +211,7 @@ export const samplerOperations = { } const flatSortedSamples = samples .reduce((acc, v) => acc.concat(...v)) + .filter(v => !v.output.isZero()) .sort((a, b) => a.output.comparedTo(b.output)); if (flatSortedSamples.length === 0) { return new BigNumber(0); From d0805d4bbba51a6e4b0113d3fd08192afec883bb Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Fri, 6 Mar 2020 18:40:38 -0500 Subject: [PATCH 03/14] `@0x/asset-swapper`: Rebase and fix some minor bugs. --- packages/asset-swapper/src/swap_quoter.ts | 3 +- .../market_operation_utils/create_order.ts | 234 ------------------ .../src/utils/market_operation_utils/index.ts | 3 +- .../utils/market_operation_utils/orders.ts | 18 +- .../market_operation_utils/path_optimizer.ts | 2 +- .../sampler_operations.ts | 8 +- .../test/market_operation_utils_test.ts | 6 +- .../test/swap_quote_calculator_test.ts | 3 +- 8 files changed, 25 insertions(+), 252 deletions(-) delete mode 100644 packages/asset-swapper/src/utils/market_operation_utils/create_order.ts diff --git a/packages/asset-swapper/src/swap_quoter.ts b/packages/asset-swapper/src/swap_quoter.ts index f98818457a..67d5361ffe 100644 --- a/packages/asset-swapper/src/swap_quoter.ts +++ b/packages/asset-swapper/src/swap_quoter.ts @@ -21,8 +21,9 @@ import { } from './types'; import { assert } from './utils/assert'; import { calculateLiquidity } from './utils/calculate_liquidity'; -import { DexOrderSampler, MarketOperationUtils } from './utils/market_operation_utils'; +import { MarketOperationUtils } from './utils/market_operation_utils'; import { createDummyOrderForSampler } from './utils/market_operation_utils/orders'; +import { DexOrderSampler } from './utils/market_operation_utils/sampler'; import { orderPrunerUtils } from './utils/order_prune_utils'; import { OrderStateUtils } from './utils/order_state_utils'; import { ProtocolFeeUtils } from './utils/protocol_fee_utils'; diff --git a/packages/asset-swapper/src/utils/market_operation_utils/create_order.ts b/packages/asset-swapper/src/utils/market_operation_utils/create_order.ts deleted file mode 100644 index 82a5f9b054..0000000000 --- a/packages/asset-swapper/src/utils/market_operation_utils/create_order.ts +++ /dev/null @@ -1,234 +0,0 @@ -import { assert } from '@0x/assert'; -import { ContractAddresses } from '@0x/contract-addresses'; -import { assetDataUtils, generatePseudoRandomSalt } from '@0x/order-utils'; -import { SignedOrder } from '@0x/types'; -import { AbiEncoder, BigNumber } from '@0x/utils'; - -import { constants } from '../../constants'; - -import { constants as marketOperationUtilConstants } from './constants'; -import { - AggregationError, - CollapsedFill, - ERC20BridgeSource, - NativeCollapsedFill, - OptimizedMarketOrder, - OrderDomain, -} from './types'; - -const { NULL_BYTES, NULL_ADDRESS, ZERO_AMOUNT } = constants; -const { INFINITE_TIMESTAMP_SEC, WALLET_SIGNATURE } = marketOperationUtilConstants; - -export class CreateOrderUtils { - private readonly _contractAddress: ContractAddresses; - - // utility function for asset-swapper to ignore market operation utils for specific asset types - public static convertNativeOrderToFullyFillableOptimizedOrders(order: SignedOrder): OptimizedMarketOrder { - return { - ...order, - fillableMakerAssetAmount: order.makerAssetAmount, - fillableTakerAssetAmount: order.takerAssetAmount, - fillableTakerFeeAmount: order.takerFee, - fill: { - source: ERC20BridgeSource.Native, - totalMakerAssetAmount: order.makerAssetAmount, - totalTakerAssetAmount: order.takerAssetAmount, - subFills: [], - }, - }; - } - - constructor(contractAddress: ContractAddresses) { - this._contractAddress = contractAddress; - } - - // Convert sell fills into orders. - public createSellOrdersFromPath( - orderDomain: OrderDomain, - inputToken: string, - outputToken: string, - path: CollapsedFill[], - bridgeSlippage: number, - liquidityProviderAddress?: string, - ): OptimizedMarketOrder[] { - const orders: OptimizedMarketOrder[] = []; - for (const fill of path) { - if (fill.source === ERC20BridgeSource.Native) { - orders.push(createNativeOrder(fill)); - } else { - orders.push( - createBridgeOrder( - orderDomain, - fill, - this._getBridgeAddressFromSource(fill.source, liquidityProviderAddress), - outputToken, - inputToken, - bridgeSlippage, - ), - ); - } - } - return orders; - } - - // Convert buy fills into orders. - public createBuyOrdersFromPath( - orderDomain: OrderDomain, - inputToken: string, - outputToken: string, - path: CollapsedFill[], - bridgeSlippage: number, - liquidityProviderAddress?: string, - ): OptimizedMarketOrder[] { - const orders: OptimizedMarketOrder[] = []; - for (const fill of path) { - if (fill.source === ERC20BridgeSource.Native) { - orders.push(createNativeOrder(fill)); - } else { - orders.push( - createBridgeOrder( - orderDomain, - fill, - this._getBridgeAddressFromSource(fill.source, liquidityProviderAddress), - inputToken, - outputToken, - bridgeSlippage, - true, - ), - ); - } - } - return orders; - } - - private _getBridgeAddressFromSource(source: ERC20BridgeSource, liquidityProviderAddress?: string): string { - switch (source) { - case ERC20BridgeSource.Eth2Dai: - return this._contractAddress.eth2DaiBridge; - case ERC20BridgeSource.Kyber: - return this._contractAddress.kyberBridge; - case ERC20BridgeSource.Uniswap: - return this._contractAddress.uniswapBridge; - case ERC20BridgeSource.CurveUsdcDai: - case ERC20BridgeSource.CurveUsdcDaiUsdt: - case ERC20BridgeSource.CurveUsdcDaiUsdtTusd: - case ERC20BridgeSource.CurveUsdcDaiUsdtBusd: - return this._contractAddress.curveBridge; - case ERC20BridgeSource.LiquidityProvider: - if (liquidityProviderAddress === undefined) { - throw new Error( - 'Cannot create a LiquidityProvider order without a LiquidityProvider pool address.', - ); - } - assert.isETHAddressHex('liquidityProviderAddress', liquidityProviderAddress); - return liquidityProviderAddress; - default: - break; - } - throw new Error(AggregationError.NoBridgeForSource); - } -} - -function createBridgeOrder( - orderDomain: OrderDomain, - fill: CollapsedFill, - bridgeAddress: string, - makerToken: string, - takerToken: string, - slippage: number, - isBuy: boolean = false, -): OptimizedMarketOrder { - let makerAssetData; - if (Object.keys(constants.DEFAULT_CURVE_OPTS).includes(fill.source)) { - const { curveAddress, tokens, version } = constants.DEFAULT_CURVE_OPTS[fill.source]; - const fromTokenIdx = tokens.indexOf(takerToken); - const toTokenIdx = tokens.indexOf(makerToken); - makerAssetData = assetDataUtils.encodeERC20BridgeAssetData( - makerToken, - bridgeAddress, - createCurveBridgeData(curveAddress, fromTokenIdx, toTokenIdx, version), - ); - } else { - makerAssetData = assetDataUtils.encodeERC20BridgeAssetData( - makerToken, - bridgeAddress, - createBridgeData(takerToken), - ); - } - return { - makerAddress: bridgeAddress, - makerAssetData, - takerAssetData: assetDataUtils.encodeERC20AssetData(takerToken), - ...createCommonOrderFields(orderDomain, fill, slippage, isBuy), - }; -} - -function createBridgeData(tokenAddress: string): string { - const encoder = AbiEncoder.create([{ name: 'tokenAddress', type: 'address' }]); - return encoder.encode({ tokenAddress }); -} - -function createCurveBridgeData( - curveAddress: string, - fromTokenIdx: number, - toTokenIdx: number, - version: number, -): string { - const curveBridgeDataEncoder = AbiEncoder.create([ - { name: 'curveAddress', type: 'address' }, - { name: 'fromTokenIdx', type: 'int128' }, - { name: 'toTokenIdx', type: 'int128' }, - { name: 'version', type: 'int128' }, - ]); - return curveBridgeDataEncoder.encode([curveAddress, fromTokenIdx, toTokenIdx, version]); -} - -type CommonOrderFields = Pick< - OptimizedMarketOrder, - Exclude ->; - -function createCommonOrderFields( - orderDomain: OrderDomain, - fill: CollapsedFill, - slippage: number, - isBuy: boolean = false, -): CommonOrderFields { - const makerAssetAmountAdjustedWithSlippage = isBuy - ? fill.totalMakerAssetAmount - : fill.totalMakerAssetAmount.times(1 - slippage).integerValue(BigNumber.ROUND_DOWN); - const takerAssetAmountAdjustedWithSlippage = isBuy - ? fill.totalTakerAssetAmount.times(slippage + 1).integerValue(BigNumber.ROUND_UP) - : fill.totalTakerAssetAmount; - return { - fill, - takerAddress: NULL_ADDRESS, - senderAddress: NULL_ADDRESS, - feeRecipientAddress: NULL_ADDRESS, - salt: generatePseudoRandomSalt(), - expirationTimeSeconds: INFINITE_TIMESTAMP_SEC, - makerFeeAssetData: NULL_BYTES, - takerFeeAssetData: NULL_BYTES, - makerFee: ZERO_AMOUNT, - takerFee: ZERO_AMOUNT, - makerAssetAmount: makerAssetAmountAdjustedWithSlippage, - fillableMakerAssetAmount: makerAssetAmountAdjustedWithSlippage, - takerAssetAmount: takerAssetAmountAdjustedWithSlippage, - fillableTakerAssetAmount: takerAssetAmountAdjustedWithSlippage, - fillableTakerFeeAmount: ZERO_AMOUNT, - signature: WALLET_SIGNATURE, - ...orderDomain, - }; -} - -function createNativeOrder(fill: CollapsedFill): OptimizedMarketOrder { - return { - fill: { - source: fill.source, - totalMakerAssetAmount: fill.totalMakerAssetAmount, - totalTakerAssetAmount: fill.totalTakerAssetAmount, - subFills: fill.subFills, - }, - ...(fill as NativeCollapsedFill).nativeOrder, - }; -} diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index 67ea911937..2d8ac2e747 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -20,8 +20,6 @@ import { OrderDomain, } from './types'; -export { DexOrderSampler } from './sampler'; - export class MarketOperationUtils { private readonly _wethAddress: string; @@ -298,6 +296,7 @@ export class MarketOperationUtils { orderDomain: this._orderDomain, contractAddresses: this.contractAddresses, bridgeSlippage: opts.bridgeSlippage || 0, + liquidityProviderAddress: opts.liquidityProviderAddress, }); } 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 35f41f94fb..e4e082b2b9 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts @@ -119,6 +119,7 @@ export interface CreateOrderFromPathOpts { orderDomain: OrderDomain; contractAddresses: ContractAddresses; bridgeSlippage: number; + liquidityProviderAddress?: string; } // Convert sell fills into orders. @@ -135,19 +136,24 @@ export function createOrdersFromPath(path: Fill[], opts: CreateOrderFromPathOpts return orders; } -function getBridgeAddressFromSource(source: ERC20BridgeSource, contractAddresses: ContractAddresses): string { +function getBridgeAddressFromSource(source: ERC20BridgeSource, opts: CreateOrderFromPathOpts): string { switch (source) { case ERC20BridgeSource.Eth2Dai: - return contractAddresses.eth2DaiBridge; + return opts.contractAddresses.eth2DaiBridge; case ERC20BridgeSource.Kyber: - return contractAddresses.kyberBridge; + return opts.contractAddresses.kyberBridge; case ERC20BridgeSource.Uniswap: - return contractAddresses.uniswapBridge; + return opts.contractAddresses.uniswapBridge; case ERC20BridgeSource.CurveUsdcDai: case ERC20BridgeSource.CurveUsdcDaiUsdt: case ERC20BridgeSource.CurveUsdcDaiUsdtTusd: case ERC20BridgeSource.CurveUsdcDaiUsdtBusd: - return contractAddresses.curveBridge; + return opts.contractAddresses.curveBridge; + case ERC20BridgeSource.LiquidityProvider: + if (opts.liquidityProviderAddress === undefined) { + throw new Error('Cannot create a LiquidityProvider order without a LiquidityProvider pool address.'); + } + return opts.liquidityProviderAddress; default: break; } @@ -157,7 +163,7 @@ function getBridgeAddressFromSource(source: ERC20BridgeSource, contractAddresses function createBridgeOrder(fill: CollapsedFill, opts: CreateOrderFromPathOpts): OptimizedMarketOrder { const takerToken = opts.side === MarketOperation.Sell ? opts.inputToken : opts.outputToken; const makerToken = opts.side === MarketOperation.Sell ? opts.outputToken : opts.inputToken; - const bridgeAddress = getBridgeAddressFromSource(fill.source, opts.contractAddresses); + const bridgeAddress = getBridgeAddressFromSource(fill.source, opts); let makerAssetData; if (Object.keys(DEFAULT_CURVE_OPTS).includes(fill.source)) { diff --git a/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts b/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts index 61893ca26f..ef2feb51fe 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts @@ -30,7 +30,7 @@ const RATE_DECIMALS = 8; function hillClimbToOptimalPath(paths: Fill[][], targetInput: BigNumber): Fill[] { // Flatten and sort path fills by descending ADJUSTED rate. const fills = paths - .reduce((acc, p) => acc.concat(p)) + .reduce((acc, p) => acc.concat(p), []) .sort((a, b) => b.adjustedRate.dp(RATE_DECIMALS).comparedTo(a.adjustedRate.dp(RATE_DECIMALS))); // Build up a path by picking the next best, valid fill until we meet our input target. const path: Fill[] = []; 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 ebf444d42f..602c3f16ca 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 @@ -1,6 +1,6 @@ import { BigNumber, ERC20BridgeSource, SignedOrder } from '../..'; -import { constants } from '../../constants'; +import { DEFAULT_CURVE_OPTS } from './constants'; import { BatchedOperation, DexSample } from './types'; /** @@ -233,8 +233,8 @@ export const samplerOperations = { }, getLiquidityProviderFromRegistry( registryAddress: string, - takerToken: string, makerToken: string, + takerToken: string, ): BatchedOperation { return { encodeCall: contract => { @@ -263,8 +263,8 @@ export const samplerOperations = { batchedOperation = samplerOperations.getUniswapSellQuotes(makerToken, takerToken, takerFillAmounts); } else if (source === ERC20BridgeSource.Kyber) { batchedOperation = samplerOperations.getKyberSellQuotes(makerToken, takerToken, takerFillAmounts); - } else if (Object.keys(constants.DEFAULT_CURVE_OPTS).includes(source)) { - const { curveAddress, tokens } = constants.DEFAULT_CURVE_OPTS[source]; + } else if (Object.keys(DEFAULT_CURVE_OPTS).includes(source)) { + const { curveAddress, tokens } = DEFAULT_CURVE_OPTS[source]; const fromTokenIdx = tokens.indexOf(takerToken); const toTokenIdx = tokens.indexOf(makerToken); if (fromTokenIdx !== -1 && toTokenIdx !== -1) { diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 16871bd873..93ea1ef819 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -652,7 +652,7 @@ describe('MarketOperationUtils tests', () => { }), ], Web3Wrapper.toBaseUnitAmount(10, 18), - { excludedSources: SELL_SOURCES, numSamples: 4 }, + { excludedSources: SELL_SOURCES, numSamples: 4, bridgeSlippage: 0 }, ); expect(result.length).to.eql(1); expect(result[0].makerAddress).to.eql(liquidityProviderAddress); @@ -667,8 +667,8 @@ describe('MarketOperationUtils tests', () => { expect(getSellQuotesParams.sources).contains(ERC20BridgeSource.LiquidityProvider); expect(getSellQuotesParams.liquidityProviderAddress).is.eql(registryAddress); expect(getLiquidityProviderParams.registryAddress).is.eql(registryAddress); - expect(getLiquidityProviderParams.makerToken).is.eql(xAsset); - expect(getLiquidityProviderParams.takerToken).is.eql(yAsset); + expect(getLiquidityProviderParams.makerToken).is.eql(yAsset); + expect(getLiquidityProviderParams.takerToken).is.eql(xAsset); }); }); diff --git a/packages/asset-swapper/test/swap_quote_calculator_test.ts b/packages/asset-swapper/test/swap_quote_calculator_test.ts index d63d1f8fba..6e5175625b 100644 --- a/packages/asset-swapper/test/swap_quote_calculator_test.ts +++ b/packages/asset-swapper/test/swap_quote_calculator_test.ts @@ -8,8 +8,9 @@ import 'mocha'; import { constants } from '../src/constants'; import { CalculateSwapQuoteOpts, SignedOrderWithFillableAmounts } from '../src/types'; -import { DexOrderSampler, MarketOperationUtils } from '../src/utils/market_operation_utils/'; +import { MarketOperationUtils } from '../src/utils/market_operation_utils/'; import { DEFAULT_GET_MARKET_ORDERS_OPTS, SELL_SOURCES } from '../src/utils/market_operation_utils/constants'; +import { DexOrderSampler } from '../src/utils/market_operation_utils/sampler'; import { ProtocolFeeUtils } from '../src/utils/protocol_fee_utils'; import { SwapQuoteCalculator } from '../src/utils/swap_quote_calculator'; From 8179a964eaa42cf099619c23a921da79f31ad866 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Mon, 9 Mar 2020 01:37:15 -0400 Subject: [PATCH 04/14] `@0x/asset-swapper`: Reintroduce `runLimit` option. `@0x/asset-swapper`: Make native fills one single path. `@0x/asset-swapper`: Redo the optimizer algo again to be more thorough. `@0x/asset-swapper`: Make `getMedianSellRate()` return `1` if maker token == taker token. --- packages/asset-swapper/CHANGELOG.json | 6 +- packages/asset-swapper/src/constants.ts | 3 - packages/asset-swapper/src/swap_quoter.ts | 16 +- packages/asset-swapper/src/types.ts | 1 - .../utils/market_operation_utils/constants.ts | 1 + .../src/utils/market_operation_utils/fills.ts | 88 ++++++--- .../src/utils/market_operation_utils/index.ts | 8 +- .../market_operation_utils/path_optimizer.ts | 181 ++++++------------ .../sampler_operations.ts | 3 + .../src/utils/market_operation_utils/types.ts | 5 + .../src/utils/swap_quote_calculator.ts | 18 +- .../test/market_operation_utils_test.ts | 88 +++------ .../test/swap_quote_calculator_test.ts | 8 +- 13 files changed, 175 insertions(+), 251 deletions(-) diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index b91116928f..74956f8b9d 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -10,12 +10,16 @@ "note": "Big refactor of market operation utils", "pr": 2513 }, + { + "note": "Remove `dustFractionThreshold`, `noConflicts` options.", + "pr": 2513 + }, { "note": "Revamp fill optimization algorithm", "pr": 2513 }, { - "note": "Add fallback orders to quotes", + "note": "Add fallback orders to quotes via `allowFallback` option.", "pr": 2513 } ] diff --git a/packages/asset-swapper/src/constants.ts b/packages/asset-swapper/src/constants.ts index 0e50b89bf0..7a4c65f9e5 100644 --- a/packages/asset-swapper/src/constants.ts +++ b/packages/asset-swapper/src/constants.ts @@ -58,9 +58,6 @@ const DEFAULT_FORWARDER_SWAP_QUOTE_GET_OPTS: SwapQuoteGetOutputOpts = { const DEFAULT_FORWARDER_SWAP_QUOTE_EXECUTE_OPTS: SwapQuoteExecutionOpts = DEFAULT_FORWARDER_SWAP_QUOTE_GET_OPTS; const DEFAULT_SWAP_QUOTE_REQUEST_OPTS: SwapQuoteRequestOpts = { - ...{ - slippagePercentage: 0.05, // 5% slippage protection, - }, ...DEFAULT_GET_MARKET_ORDERS_OPTS, }; diff --git a/packages/asset-swapper/src/swap_quoter.ts b/packages/asset-swapper/src/swap_quoter.ts index 67d5361ffe..1ddbe95ad2 100644 --- a/packages/asset-swapper/src/swap_quoter.ts +++ b/packages/asset-swapper/src/swap_quoter.ts @@ -242,11 +242,7 @@ export class SwapQuoter { ): Promise> { makerAssetBuyAmount.map((a, i) => assert.isBigNumber(`makerAssetBuyAmount[${i}]`, a)); let gasPrice: BigNumber; - const { slippagePercentage, ...calculateSwapQuoteOpts } = _.merge( - {}, - constants.DEFAULT_SWAP_QUOTE_REQUEST_OPTS, - options, - ); + const calculateSwapQuoteOpts = _.merge({}, constants.DEFAULT_SWAP_QUOTE_REQUEST_OPTS, options); if (!!options.gasPrice) { gasPrice = options.gasPrice; assert.isBigNumber('gasPrice', gasPrice); @@ -278,7 +274,6 @@ export class SwapQuoter { const swapQuotes = await this._swapQuoteCalculator.calculateBatchMarketBuySwapQuoteAsync( allPrunedOrders, makerAssetBuyAmount, - slippagePercentage, gasPrice, calculateSwapQuoteOpts, ); @@ -517,14 +512,9 @@ export class SwapQuoter { marketOperation: MarketOperation, options: Partial, ): Promise { - const { slippagePercentage, ...calculateSwapQuoteOpts } = _.merge( - {}, - constants.DEFAULT_SWAP_QUOTE_REQUEST_OPTS, - options, - ); + const calculateSwapQuoteOpts = _.merge({}, constants.DEFAULT_SWAP_QUOTE_REQUEST_OPTS, options); assert.isString('makerAssetData', makerAssetData); assert.isString('takerAssetData', takerAssetData); - assert.isNumber('slippagePercentage', slippagePercentage); let gasPrice: BigNumber; if (!!options.gasPrice) { gasPrice = options.gasPrice; @@ -547,7 +537,6 @@ export class SwapQuoter { swapQuote = await this._swapQuoteCalculator.calculateMarketBuySwapQuoteAsync( prunedOrders, assetFillAmount, - slippagePercentage, gasPrice, calculateSwapQuoteOpts, ); @@ -555,7 +544,6 @@ export class SwapQuoter { swapQuote = await this._swapQuoteCalculator.calculateMarketSellSwapQuoteAsync( prunedOrders, assetFillAmount, - slippagePercentage, gasPrice, calculateSwapQuoteOpts, ); diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index ab466914ab..fc88260319 100644 --- a/packages/asset-swapper/src/types.ts +++ b/packages/asset-swapper/src/types.ts @@ -190,7 +190,6 @@ export interface SwapQuoteOrdersBreakdown { * gasPrice: gas price to determine protocolFee amount, default to ethGasStation fast amount */ export interface SwapQuoteRequestOpts extends CalculateSwapQuoteOpts { - slippagePercentage: number; gasPrice?: BigNumber; } diff --git a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts index 763e34ac1a..1612cb9de4 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts @@ -25,6 +25,7 @@ export const BUY_SOURCES = [ERC20BridgeSource.Uniswap, ERC20BridgeSource.Eth2Dai export const DEFAULT_GET_MARKET_ORDERS_OPTS: GetMarketOrdersOpts = { // tslint:disable-next-line: custom-no-magic-numbers + runLimit: 2 ** 15, excludedSources: [], bridgeSlippage: 0.005, numSamples: 20, diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts index a3644453a3..f1afd91b5e 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts @@ -23,6 +23,7 @@ export function createFillPaths(opts: { side: MarketOperation; orders?: SignedOrderWithFillableAmounts[]; dexQuotes?: DexSample[][]; + targetInput?: BigNumber; ethToOutputRate?: BigNumber; excludedSources?: ERC20BridgeSource[]; fees?: { [source: string]: BigNumber }; @@ -34,10 +35,10 @@ export function createFillPaths(opts: { const dexQuotes = opts.dexQuotes || []; const ethToOutputRate = opts.ethToOutputRate || ZERO_AMOUNT; // Create native fill paths. - const nativeFills = orders.map(o => nativeOrderToPath(side, o, ethToOutputRate, fees)); + const nativePath = nativeOrdersToPath(side, orders, ethToOutputRate, fees); // Create DEX fill paths. const dexPaths = dexQuotesToPaths(side, dexQuotes, ethToOutputRate, fees); - return filterPaths([...dexPaths, ...nativeFills], excludedSources); + return filterPaths([...dexPaths, nativePath].map(p => clipPathToInput(p, opts.targetInput)), excludedSources); } function filterPaths(paths: Fill[][], excludedSources: ERC20BridgeSource[]): Fill[][] { @@ -56,39 +57,51 @@ function filterPaths(paths: Fill[][], excludedSources: ERC20BridgeSource[]): Fil }); } -function nativeOrderToPath( +function nativeOrdersToPath( side: MarketOperation, - order: SignedOrderWithFillableAmounts, + orders: SignedOrderWithFillableAmounts[], ethToOutputRate: BigNumber, fees: { [source: string]: BigNumber }, ): Fill[] { - const makerAmount = fillableAmountsUtils.getMakerAssetAmountSwappedAfterOrderFees(order); - const takerAmount = fillableAmountsUtils.getTakerAssetAmountSwappedAfterOrderFees(order); - const input = side === MarketOperation.Sell ? takerAmount : makerAmount; - const output = side === MarketOperation.Sell ? makerAmount : takerAmount; - const penalty = ethToOutputRate.times(fees[ERC20BridgeSource.Native] || 0); - const adjustedOutput = side === MarketOperation.Sell ? output.minus(penalty) : output.plus(penalty); - const rate = makerAmount.div(takerAmount); - const adjustedRate = - side === MarketOperation.Sell - ? makerAmount.minus(penalty).div(takerAmount) - : makerAmount.div(takerAmount.plus(penalty)); - // Native orders can be filled in any order, so they're all root nodes. - return [ - { + // Create a single path from all orders. + let path: Fill[] = []; + for (const order of orders) { + const makerAmount = fillableAmountsUtils.getMakerAssetAmountSwappedAfterOrderFees(order); + const takerAmount = fillableAmountsUtils.getTakerAssetAmountSwappedAfterOrderFees(order); + const input = side === MarketOperation.Sell ? takerAmount : makerAmount; + const output = side === MarketOperation.Sell ? makerAmount : takerAmount; + const penalty = ethToOutputRate.times(fees[ERC20BridgeSource.Native] || 0); + const adjustedOutput = side === MarketOperation.Sell ? output.minus(penalty) : output.plus(penalty); + const rate = makerAmount.div(takerAmount); + const adjustedRate = + side === MarketOperation.Sell + ? makerAmount.minus(penalty).div(takerAmount) + : makerAmount.div(takerAmount.plus(penalty)); + // Skip orders with rates that are <= 0. + if (adjustedRate.lte(0)) { + continue; + } + path.push({ input, output, rate, adjustedRate, adjustedOutput, - index: 0, + index: path.length, flags: 0, + parent: path.length !== 0 ? path[path.length - 1] : undefined, source: ERC20BridgeSource.Native, - fillData: { - order, - }, - }, - ]; + fillData: { order }, + }); + } + // Sort by descending rate. + path = path.sort((a, b) => b.rate.comparedTo(a.rate)); + // Re-index fills. + for (let i = 0; i < path.length; ++i) { + path[i].parent = i === 0 ? undefined : path[i - 1]; + path[i].index = i; + } + return path; } function dexQuotesToPaths( @@ -151,12 +164,12 @@ function sourceToFillFlags(source: ERC20BridgeSource): number { return 0; } -export function getPathSize(path: Fill[], maxInput: BigNumber = POSITIVE_INF): [BigNumber, BigNumber] { +export function getPathSize(path: Fill[], targetInput: BigNumber = POSITIVE_INF): [BigNumber, BigNumber] { let input = ZERO_AMOUNT; let output = ZERO_AMOUNT; for (const fill of path) { - if (input.plus(fill.input).gte(maxInput)) { - const di = maxInput.minus(input).div(fill.input); + if (input.plus(fill.input).gte(targetInput)) { + const di = targetInput.minus(input).div(fill.input); input = input.plus(di); output = output.plus(fill.output.times(di.div(fill.input))); break; @@ -168,12 +181,12 @@ export function getPathSize(path: Fill[], maxInput: BigNumber = POSITIVE_INF): [ return [input.integerValue(), output.integerValue()]; } -export function getPathAdjustedSize(path: Fill[], maxInput: BigNumber = POSITIVE_INF): [BigNumber, BigNumber] { +export function getPathAdjustedSize(path: Fill[], targetInput: BigNumber = POSITIVE_INF): [BigNumber, BigNumber] { let input = ZERO_AMOUNT; let output = ZERO_AMOUNT; for (const fill of path) { - if (input.plus(fill.input).gte(maxInput)) { - const di = maxInput.minus(input).div(fill.input); + if (input.plus(fill.input).gte(targetInput)) { + const di = targetInput.minus(input).div(fill.input); input = input.plus(di); output = output.plus(fill.adjustedOutput.times(di.div(fill.input))); break; @@ -188,11 +201,13 @@ export function getPathAdjustedSize(path: Fill[], maxInput: BigNumber = POSITIVE export function isValidPath(path: Fill[]): boolean { let flags = 0; for (let i = 0; i < path.length; ++i) { + // Fill must immediately follow its parent. if (path[i].parent) { if (i === 0 || path[i - 1] !== path[i].parent) { return false; } } + // Fill must not be duplicated. for (let j = 0; j < i; ++j) { if (path[i] === path[j]) { return false; @@ -204,6 +219,19 @@ export function isValidPath(path: Fill[]): boolean { return (flags & conflictFlags) !== conflictFlags; } +export function clipPathToInput(path: Fill[], targetInput: BigNumber = POSITIVE_INF): Fill[] { + const clipped: Fill[] = []; + let input = ZERO_AMOUNT; + for (const fill of path) { + if (input.gte(targetInput)) { + break; + } + input = input.plus(fill.input); + clipped.push(fill); + } + return clipped; +} + export function collapsePath(side: MarketOperation, path: Fill[]): CollapsedFill[] { const collapsed: Array = []; for (const fill of path) { diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index 2d8ac2e747..015dac045a 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -260,6 +260,7 @@ export class MarketOperationUtils { nativeOrders: SignedOrder[]; orderFillableAmounts: BigNumber[]; dexQuotes: DexSample[][]; + runLimit?: number; ethToOutputRate?: BigNumber; bridgeSlippage?: number; excludedSources?: ERC20BridgeSource[]; @@ -274,20 +275,21 @@ export class MarketOperationUtils { // Augment native orders with their fillable amounts. orders: createSignedOrdersWithFillableAmounts(side, opts.nativeOrders, opts.orderFillableAmounts), dexQuotes: opts.dexQuotes, + targetInput: inputAmount, ethToOutputRate: opts.ethToOutputRate, excludedSources: opts.excludedSources, fees: opts.fees, }); // Find the optimal path. - const optimalPath = findOptimalPath(side, paths, inputAmount); - // console.log(JSON.stringify(paths, null, ' ')); + const optimalPath = findOptimalPath(side, paths, inputAmount, opts.runLimit); if (!optimalPath) { throw new Error(AggregationError.NoOptimalPath); } // Find a fallback path from sources not used in the first path. let fallbackPath: Fill[] = []; if (opts.allowFallback) { - fallbackPath = findOptimalPath(side, getUnusedSourcePaths(optimalPath, paths), inputAmount) || []; + fallbackPath = + findOptimalPath(side, getUnusedSourcePaths(optimalPath, paths), inputAmount, opts.runLimit) || []; } return createOrdersFromPath([...optimalPath, ...fallbackPath], { side, diff --git a/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts b/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts index ef2feb51fe..61b0d46215 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts @@ -2,7 +2,8 @@ import { BigNumber } from '@0x/utils'; import { MarketOperation } from '../../types'; -import { getPathAdjustedSize, getPathSize, isValidPath } from './fills'; +import { ZERO_AMOUNT } from './constants'; +import { getPathSize, isValidPath } from './fills'; import { Fill } from './types'; // tslint:disable: prefer-for-of custom-no-magic-numbers completed-docs @@ -11,145 +12,77 @@ import { Fill } from './types'; * Find the optimal mixture of paths that maximizes (for sells) or minimizes * (for buys) output, while meeting the input requirement. */ -export function findOptimalPath(side: MarketOperation, paths: Fill[][], targetInput: BigNumber): Fill[] | undefined { - // Get all convex and concave rate paths. - const [convexPaths, concavePaths] = splitPathsByRateConvexity(paths); - // Hill climb convex paths. - let optimalPath = hillClimbToOptimalPath(convexPaths, targetInput); - // Attempt to splice in concave paths. - for (const path of concavePaths) { - // TODO(dorothy-zbornak): This will probably not be optimal if we are dealing - // more than one concave path. Right now there's just Kyber. - optimalPath = prependConcavePath(side, optimalPath, path, targetInput); +export function findOptimalPath( + side: MarketOperation, + paths: Fill[][], + targetInput: BigNumber, + runLimit?: number, +): Fill[] | undefined { + let optimalPath = paths[0] || []; + // TODO(dorothy-zbornak): Convex paths (like kyber) should technically always be + // inserted at the front of the path because a partial fill can invalidate them. + for (const path of paths.slice(1)) { + optimalPath = mixPaths(side, optimalPath, path, targetInput, runLimit); } return isPathComplete(optimalPath, targetInput) ? optimalPath : undefined; } -const RATE_DECIMALS = 8; - -function hillClimbToOptimalPath(paths: Fill[][], targetInput: BigNumber): Fill[] { - // Flatten and sort path fills by descending ADJUSTED rate. - const fills = paths - .reduce((acc, p) => acc.concat(p), []) - .sort((a, b) => b.adjustedRate.dp(RATE_DECIMALS).comparedTo(a.adjustedRate.dp(RATE_DECIMALS))); - // Build up a path by picking the next best, valid fill until we meet our input target. - const path: Fill[] = []; - while (!isPathComplete(path, targetInput)) { - let wasAdded = false; - for (const fill of fills) { - // If we can just legally append this fill to the path, do that. - if (isValidPath([...path, fill])) { - path.push(fill); - wasAdded = true; - break; - } else if (fill.parent && !path.includes(fill)) { - // If the fill's parent is in the path, we can insert it right - // after it. - for (let i = 0; i < path.length; ++i) { - if (path[i] === fill.parent) { - path.splice(i + 1, 0, fill); - wasAdded = true; - break; - } - } - if (wasAdded) { +function mixPaths( + side: MarketOperation, + pathA: Fill[], + pathB: Fill[], + targetInput: BigNumber, + maxSteps: number = 2 ** 15, +): Fill[] { + const allFills = [...pathA, ...pathB].sort((a, b) => b.rate.comparedTo(a.rate)); + let bestPath: Fill[] = []; + let bestPathInput = ZERO_AMOUNT; + let bestPathRate = ZERO_AMOUNT; + let steps = 0; + const _isBetterPath = (input: BigNumber, rate: BigNumber) => { + if (bestPathInput.lt(targetInput)) { + return input.gt(bestPathInput); + } else if (input.gte(bestPathInput)) { + return rate.gt(bestPathRate); + } + return false; + }; + const _walk = (path: Fill[], input: BigNumber, output: BigNumber) => { + steps += 1; + const rate = getRate(side, input, output); + if (_isBetterPath(input, rate)) { + bestPath = path; + bestPathInput = input; + bestPathRate = rate; + } + if (input.lt(targetInput)) { + for (const fill of allFills) { + if (steps >= maxSteps) { break; } + const childPath = [...path, fill]; + if (!isValidPath(childPath)) { + continue; + } + _walk(childPath, input.plus(fill.input), output.plus(fill.adjustedOutput)); } } - if (!wasAdded) { - break; - } - } - return path; -} - -function prependConcavePath( - side: MarketOperation, - convexPath: Fill[], - concavePath: Fill[], - targetInput: BigNumber, -): Fill[] { - // Try to prepend increasing lenths of the the concave path, keeping track - // of the best path. - // HACK(dorothy-zbornak): We prepend because placing it at the end has the - // possibility of turning it into a partial fill, which can invalidate the - // quote for Kyber. - let bestPath = convexPath; - for (let i = 0; i < convexPath.length; ++i) { - const path = concavePath.slice(0, i); - const [concaveInput] = getPathSize(path); - if (concaveInput.lt(targetInput)) { - const remainingInput = targetInput.minus(concaveInput); - // We sub-optimize the fills from the original path. - const tailPath = hillClimbToOptimalPath(getSubPaths(convexPath), remainingInput); - path.push(...tailPath); - } - bestPath = findBestCompletePath(side, [bestPath, path], targetInput) || bestPath; - } + }; + _walk(bestPath, ZERO_AMOUNT, ZERO_AMOUNT); return bestPath; } -function getSubPaths(path: Fill[]): Fill[][] { - const fillsBySource: { [source: string]: Fill[] } = {}; - for (const fill of path) { - fillsBySource[fill.source] = fillsBySource[fill.source] || []; - fillsBySource[fill.source].push(fill); - } - return Object.values(fillsBySource); -} - function isPathComplete(path: Fill[], targetInput: BigNumber): boolean { const [input] = getPathSize(path); return input.gte(targetInput); } -function findBestCompletePath(side: MarketOperation, paths: Fill[][], targetInput: BigNumber): Fill[] | undefined { - let bestPath: Fill[] | undefined; - for (const path of paths) { - const [input, output] = getPathAdjustedSize(path, targetInput); - if (input.gte(targetInput)) { - continue; - } - if (bestPath) { - const [, bestPathOutput] = getPathAdjustedSize(bestPath, targetInput); - if (side === MarketOperation.Sell) { - if (output.lt(bestPathOutput)) { - continue; - } - } else { - if (output.gt(bestPathOutput)) { - continue; - } - } - } - bestPath = path; +function getRate(side: MarketOperation, input: BigNumber, output: BigNumber): BigNumber { + if (input.eq(0) || output.eq(0)) { + return ZERO_AMOUNT; } - return bestPath; -} - -function splitPathsByRateConvexity(paths: Fill[][]): [Fill[][], Fill[][]] { - const convexPaths: Fill[][] = []; - const concavePaths: Fill[][] = []; - for (const path of paths) { - if (isPathConvex(path)) { - convexPaths.push(path); - } else { - concavePaths.push(path); - } + if (side === MarketOperation.Sell) { + return output.div(input); } - return [convexPaths, concavePaths]; -} - -function isPathConvex(path: Fill[]): boolean { - // Convex paths have descending prices. - // HACK(dorothy-zbornak): We use the the `rate` instead of the `adjustedRate` - // because the `adjustedRate` can make paths appear artificially concave - // due to the fee incurred on the first fill. - for (let i = 1; i < path.length; ++i) { - if (path[i - 1].rate.dp(RATE_DECIMALS).lt(path[i].rate.dp(RATE_DECIMALS))) { - return false; - } - } - return true; + return input.div(output); } 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 602c3f16ca..c0b5550b08 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 @@ -191,6 +191,9 @@ export const samplerOperations = { takerFillAmount: BigNumber, liquidityProviderRegistryAddress?: string | undefined, ): BatchedOperation { + if (makerToken.toLowerCase() === takerToken.toLowerCase()) { + return samplerOperations.constant(new BigNumber(1)); + } const getSellQuotes = samplerOperations.getSellQuotes( sources, makerToken, diff --git a/packages/asset-swapper/src/utils/market_operation_utils/types.ts b/packages/asset-swapper/src/utils/market_operation_utils/types.ts index 512e3aed93..b4e86e6d22 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -138,6 +138,11 @@ export interface GetMarketOrdersOpts { * Liquidity sources to exclude. Default is none. */ excludedSources: ERC20BridgeSource[]; + /** + * Complexity limit on the search algorithm, i.e., maximum number of + * nodes to visit. Default is 1024. + */ + runLimit: number; /** * When generating bridge orders, we use * sampled rate * (1 - bridgeSlippage) diff --git a/packages/asset-swapper/src/utils/swap_quote_calculator.ts b/packages/asset-swapper/src/utils/swap_quote_calculator.ts index f4231e79a2..574ebfe014 100644 --- a/packages/asset-swapper/src/utils/swap_quote_calculator.ts +++ b/packages/asset-swapper/src/utils/swap_quote_calculator.ts @@ -41,14 +41,12 @@ export class SwapQuoteCalculator { public async calculateMarketSellSwapQuoteAsync( prunedOrders: SignedOrder[], takerAssetFillAmount: BigNumber, - slippagePercentage: number, gasPrice: BigNumber, opts: CalculateSwapQuoteOpts, ): Promise { return (await this._calculateSwapQuoteAsync( prunedOrders, takerAssetFillAmount, - slippagePercentage, gasPrice, MarketOperation.Sell, opts, @@ -58,14 +56,12 @@ export class SwapQuoteCalculator { public async calculateMarketBuySwapQuoteAsync( prunedOrders: SignedOrder[], takerAssetFillAmount: BigNumber, - slippagePercentage: number, gasPrice: BigNumber, opts: CalculateSwapQuoteOpts, ): Promise { return (await this._calculateSwapQuoteAsync( prunedOrders, takerAssetFillAmount, - slippagePercentage, gasPrice, MarketOperation.Buy, opts, @@ -75,14 +71,12 @@ export class SwapQuoteCalculator { public async calculateBatchMarketBuySwapQuoteAsync( batchPrunedOrders: SignedOrder[][], takerAssetFillAmounts: BigNumber[], - slippagePercentage: number, gasPrice: BigNumber, opts: CalculateSwapQuoteOpts, ): Promise> { return (await this._calculateBatchBuySwapQuoteAsync( batchPrunedOrders, takerAssetFillAmounts, - slippagePercentage, gasPrice, MarketOperation.Buy, opts, @@ -92,17 +86,13 @@ export class SwapQuoteCalculator { private async _calculateBatchBuySwapQuoteAsync( batchPrunedOrders: SignedOrder[][], assetFillAmounts: BigNumber[], - slippagePercentage: number, gasPrice: BigNumber, operation: MarketOperation, opts: CalculateSwapQuoteOpts, ): Promise> { - const assetFillAmountsWithSlippage = assetFillAmounts.map(a => - a.plus(a.multipliedBy(slippagePercentage).integerValue()), - ); const batchSignedOrders = await this._marketOperationUtils.getBatchMarketBuyOrdersAsync( batchPrunedOrders, - assetFillAmountsWithSlippage, + assetFillAmounts, opts, ); const batchSwapQuotes = await Promise.all( @@ -127,7 +117,6 @@ export class SwapQuoteCalculator { private async _calculateSwapQuoteAsync( prunedOrders: SignedOrder[], assetFillAmount: BigNumber, - slippagePercentage: number, gasPrice: BigNumber, operation: MarketOperation, opts: CalculateSwapQuoteOpts, @@ -138,7 +127,6 @@ export class SwapQuoteCalculator { } // since prunedOrders do not have fillState, we will add a buffer of fillable orders to consider that some native are orders are partially filled - const slippageBufferAmount = assetFillAmount.multipliedBy(slippagePercentage).integerValue(); let resultOrders: OptimizedMarketOrder[] = []; { @@ -159,13 +147,13 @@ export class SwapQuoteCalculator { if (operation === MarketOperation.Buy) { resultOrders = await this._marketOperationUtils.getMarketBuyOrdersAsync( prunedOrders, - assetFillAmount.plus(slippageBufferAmount), + assetFillAmount, _opts, ); } else { resultOrders = await this._marketOperationUtils.getMarketSellOrdersAsync( prunedOrders, - assetFillAmount.plus(slippageBufferAmount), + assetFillAmount, _opts, ); } diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 93ea1ef819..71b990f445 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -287,7 +287,7 @@ describe('MarketOperationUtils tests', () => { }); describe('getMarketSellOrdersAsync()', () => { - const FILL_AMOUNT = getRandomInteger(1, 1e18); + const FILL_AMOUNT = new BigNumber('100e18'); const ORDERS = createOrdersFromSellRates( FILL_AMOUNT, _.times(NUM_SAMPLES, i => DEFAULT_RATES[ERC20BridgeSource.Native][i]), @@ -432,31 +432,7 @@ describe('MarketOperationUtils tests', () => { rates[ERC20BridgeSource.Native] = [0.4, 0.3, 0.2, 0.1]; rates[ERC20BridgeSource.Uniswap] = [0.5, 0.05, 0.05, 0.05]; rates[ERC20BridgeSource.Eth2Dai] = [0.6, 0.05, 0.05, 0.05]; - rates[ERC20BridgeSource.Kyber] = [0.7, 0.05, 0.05, 0.05]; - replaceSamplerOps({ - getSellQuotes: createGetMultipleSellQuotesOperationFromRates(rates), - }); - const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync( - createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), - FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4 }, - ); - const orderSources = improvedOrders.map(o => o.fill.source); - const expectedSources = [ - ERC20BridgeSource.Kyber, - ERC20BridgeSource.Native, - ERC20BridgeSource.Native, - ERC20BridgeSource.Native, - ]; - expect(orderSources).to.deep.eq(expectedSources); - }); - - it('excludes Kyber when Uniswap or Eth2Dai are used first', async () => { - const rates: RatesBySource = {}; - rates[ERC20BridgeSource.Native] = [0.3, 0.2, 0.1, 0.05]; - rates[ERC20BridgeSource.Uniswap] = [0.5, 0.05, 0.05, 0.05]; - rates[ERC20BridgeSource.Eth2Dai] = [0.6, 0.05, 0.05, 0.05]; - rates[ERC20BridgeSource.Kyber] = [0.4, 0.05, 0.05, 0.05]; + rates[ERC20BridgeSource.Kyber] = [0, 0, 0, 0]; // unused replaceSamplerOps({ getSellQuotes: createGetMultipleSellQuotesOperationFromRates(rates), }); @@ -472,15 +448,15 @@ describe('MarketOperationUtils tests', () => { ERC20BridgeSource.Native, ERC20BridgeSource.Native, ]; - expect(orderSources).to.deep.eq(expectedSources); + expect(orderSources.sort()).to.deep.eq(expectedSources.sort()); }); - it('excludes Uniswap and Eth2Dai when Kyber is used first', async () => { + it('Kyber is exclusive against Uniswap and Eth2Dai', async () => { const rates: RatesBySource = {}; - rates[ERC20BridgeSource.Native] = [0.1, 0.05, 0.05, 0.05]; - rates[ERC20BridgeSource.Uniswap] = [0.15, 0.05, 0.05, 0.05]; - rates[ERC20BridgeSource.Eth2Dai] = [0.15, 0.05, 0.05, 0.05]; - rates[ERC20BridgeSource.Kyber] = [0.7, 0.05, 0.05, 0.05]; + rates[ERC20BridgeSource.Native] = [0.3, 0.2, 0.1, 0.05]; + rates[ERC20BridgeSource.Uniswap] = [0.5, 0.05, 0.05, 0.05]; + rates[ERC20BridgeSource.Eth2Dai] = [0.6, 0.05, 0.05, 0.05]; + rates[ERC20BridgeSource.Kyber] = [0.4, 0.05, 0.05, 0.05]; replaceSamplerOps({ getSellQuotes: createGetMultipleSellQuotesOperationFromRates(rates), }); @@ -490,8 +466,12 @@ describe('MarketOperationUtils tests', () => { { ...DEFAULT_OPTS, numSamples: 4 }, ); const orderSources = improvedOrders.map(o => o.fill.source); - const expectedSources = [ERC20BridgeSource.Kyber, ERC20BridgeSource.Native]; - expect(orderSources).to.deep.eq(expectedSources); + if (orderSources.includes(ERC20BridgeSource.Kyber)) { + expect(orderSources).to.not.include(ERC20BridgeSource.Uniswap); + expect(orderSources).to.not.include(ERC20BridgeSource.Eth2Dai); + } else { + expect(orderSources).to.not.include(ERC20BridgeSource.Kyber); + } }); const ETH_TO_MAKER_RATE = 1.5; @@ -501,7 +481,7 @@ describe('MarketOperationUtils tests', () => { // dropping their effective rates. const nativeFeeRate = 0.06; const rates: RatesBySource = { - [ERC20BridgeSource.Native]: [1, 0.99, 0.98, 0.97], // Effectively [0.94, ~0.93, ~0.92, ~0.91] + [ERC20BridgeSource.Native]: [1, 0.99, 0.98, 0.97], // Effectively [0.94, 0.93, 0.92, 0.91] [ERC20BridgeSource.Uniswap]: [0.96, 0.1, 0.1, 0.1], [ERC20BridgeSource.Eth2Dai]: [0.95, 0.1, 0.1, 0.1], [ERC20BridgeSource.Kyber]: [0.1, 0.1, 0.1, 0.1], @@ -522,12 +502,12 @@ describe('MarketOperationUtils tests', () => { ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ + ERC20BridgeSource.Native, ERC20BridgeSource.Uniswap, ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Native, - ERC20BridgeSource.Native, ]; - expect(orderSources).to.deep.eq(expectedSources); + expect(orderSources.sort()).to.deep.eq(expectedSources.sort()); }); it('factors in fees for dexes', async () => { @@ -561,7 +541,7 @@ describe('MarketOperationUtils tests', () => { ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Uniswap, ]; - expect(orderSources).to.deep.eq(expectedSources); + expect(orderSources.sort()).to.deep.eq(expectedSources.sort()); }); it('can mix one concave source', async () => { @@ -586,7 +566,7 @@ describe('MarketOperationUtils tests', () => { ERC20BridgeSource.Uniswap, ERC20BridgeSource.Native, ]; - expect(orderSources).to.deep.eq(expectedSources); + expect(orderSources.sort()).to.deep.eq(expectedSources.sort()); }); it('fallback orders use different sources', async () => { @@ -604,15 +584,10 @@ describe('MarketOperationUtils tests', () => { { ...DEFAULT_OPTS, numSamples: 4, allowFallback: true }, ); const orderSources = improvedOrders.map(o => o.fill.source); - const expectedSources = [ - ERC20BridgeSource.Eth2Dai, - ERC20BridgeSource.Uniswap, - // Fallback - ERC20BridgeSource.Native, - ERC20BridgeSource.Kyber, - ERC20BridgeSource.Native, - ]; - expect(orderSources).to.deep.eq(expectedSources); + const firstSources = [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Uniswap]; + const secondSources = [ERC20BridgeSource.Native, ERC20BridgeSource.Kyber, ERC20BridgeSource.Native]; + 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 () => { @@ -673,7 +648,7 @@ describe('MarketOperationUtils tests', () => { }); describe('getMarketBuyOrdersAsync()', () => { - const FILL_AMOUNT = getRandomInteger(1, 1e18); + const FILL_AMOUNT = new BigNumber('100e18'); const ORDERS = createOrdersFromBuyRates( FILL_AMOUNT, _.times(NUM_SAMPLES, () => DEFAULT_RATES[ERC20BridgeSource.Native][0]), @@ -832,7 +807,7 @@ describe('MarketOperationUtils tests', () => { ERC20BridgeSource.Native, ERC20BridgeSource.Native, ]; - expect(orderSources).to.deep.eq(expectedSources); + expect(orderSources.sort()).to.deep.eq(expectedSources.sort()); }); const ETH_TO_TAKER_RATE = 1.5; @@ -868,7 +843,7 @@ describe('MarketOperationUtils tests', () => { ERC20BridgeSource.Native, ERC20BridgeSource.Native, ]; - expect(orderSources).to.deep.eq(expectedSources); + expect(orderSources.sort()).to.deep.eq(expectedSources.sort()); }); it('factors in fees for dexes', async () => { @@ -901,7 +876,7 @@ describe('MarketOperationUtils tests', () => { ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Uniswap, ]; - expect(orderSources).to.deep.eq(expectedSources); + expect(orderSources.sort()).to.deep.eq(expectedSources.sort()); }); it('fallback orders use different sources', async () => { @@ -918,16 +893,15 @@ describe('MarketOperationUtils tests', () => { { ...DEFAULT_OPTS, numSamples: 4, allowFallback: true }, ); const orderSources = improvedOrders.map(o => o.fill.source); - const expectedSources = [ - ERC20BridgeSource.Eth2Dai, - ERC20BridgeSource.Uniswap, - // Fallback + const firstSources = [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Uniswap]; + const secondSources = [ ERC20BridgeSource.Native, ERC20BridgeSource.Native, ERC20BridgeSource.Native, ERC20BridgeSource.Native, ]; - expect(orderSources).to.deep.eq(expectedSources); + expect(orderSources.slice(0, firstSources.length).sort()).to.deep.eq(firstSources.sort()); + expect(orderSources.slice(firstSources.length).sort()).to.deep.eq(secondSources.sort()); }); }); }); diff --git a/packages/asset-swapper/test/swap_quote_calculator_test.ts b/packages/asset-swapper/test/swap_quote_calculator_test.ts index 6e5175625b..4c77a8b38b 100644 --- a/packages/asset-swapper/test/swap_quote_calculator_test.ts +++ b/packages/asset-swapper/test/swap_quote_calculator_test.ts @@ -1,3 +1,7 @@ +// tslint:disable:max-file-line-count +// TODO(dorothy-zbornak): Skipping these tests for now because they're a +// nightmare to maintain. We should replace them with simpler unit tests. +/* import { constants as devConstants } from '@0x/contracts-test-utils'; import { BlockchainLifecycle } from '@0x/dev-utils'; import { ContractAddresses, migrateOnceAsync } from '@0x/migrations'; @@ -63,10 +67,7 @@ function createSamplerFromSignedOrdersWithFillableAmounts( ); } -// tslint:disable:max-file-line-count // tslint:disable:custom-no-magic-numbers -// TODO(dorothy-zbornak): Skipping these tests for now because they're a -// nightmare to maintain. We should replace them with simpler unit tests. describe.skip('swapQuoteCalculator', () => { let protocolFeeUtils: ProtocolFeeUtils; let contractAddresses: ContractAddresses; @@ -904,3 +905,4 @@ describe.skip('swapQuoteCalculator', () => { }); }); }); +*/ From 95bebd6d1d3d29152da464085e6d6f3313f69b39 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Mon, 9 Mar 2020 12:17:02 -0400 Subject: [PATCH 05/14] `@0x/asset-swapper`: Clean up median price calls. --- .../src/utils/market_operation_utils/index.ts | 36 +++++++++---------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index 015dac045a..7fa8626403 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -66,17 +66,15 @@ export class MarketOperationUtils { takerToken, ), // Get ETH -> maker token price. - makerToken === this._wethAddress - ? DexOrderSampler.ops.constant(new BigNumber(1)) - : DexOrderSampler.ops.getMedianSellRate( - difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( - this._liquidityProviderSourceIfAvailable(_opts.excludedSources), - ), - makerToken, - this._wethAddress, - ONE_ETHER, - this._liquidityProviderRegistry, + DexOrderSampler.ops.getMedianSellRate( + difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( + this._liquidityProviderSourceIfAvailable(_opts.excludedSources), ), + makerToken, + this._wethAddress, + ONE_ETHER, + this._liquidityProviderRegistry, + ), // Get sell quotes for taker -> maker. DexOrderSampler.ops.getSellQuotes( difference(SELL_SOURCES, _opts.excludedSources).concat( @@ -139,17 +137,15 @@ export class MarketOperationUtils { takerToken, ), // Get ETH -> taker token price. - takerToken === this._wethAddress - ? DexOrderSampler.ops.constant(new BigNumber(1)) - : DexOrderSampler.ops.getMedianSellRate( - difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( - this._liquidityProviderSourceIfAvailable(_opts.excludedSources), - ), - takerToken, - this._wethAddress, - ONE_ETHER, - this._liquidityProviderRegistry, + DexOrderSampler.ops.getMedianSellRate( + difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( + this._liquidityProviderSourceIfAvailable(_opts.excludedSources), ), + takerToken, + this._wethAddress, + ONE_ETHER, + this._liquidityProviderRegistry, + ), // Get buy quotes for taker -> maker. DexOrderSampler.ops.getBuyQuotes( difference(BUY_SOURCES, _opts.excludedSources).concat( From 847a7f470c4a89acec5368479fea7aed0c19b99f Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Mon, 9 Mar 2020 14:31:33 -0400 Subject: [PATCH 06/14] `@0x/instant`: Fix for changed asset-swapper configs. --- packages/instant/CHANGELOG.json | 4 ++++ packages/instant/src/constants.ts | 2 -- packages/instant/src/util/swap_quote_updater.ts | 6 ------ 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/instant/CHANGELOG.json b/packages/instant/CHANGELOG.json index bd78ca6931..1ca3feeadd 100644 --- a/packages/instant/CHANGELOG.json +++ b/packages/instant/CHANGELOG.json @@ -9,6 +9,10 @@ { "note": "Fix ERC721 asset support", "pr": 2491 + }, + { + "note": "Remove `bridgeSlippage` SwapQuoter config.", + "pr": 2513 } ] }, diff --git a/packages/instant/src/constants.ts b/packages/instant/src/constants.ts index 69045b8bc7..1e6ae7b8ba 100644 --- a/packages/instant/src/constants.ts +++ b/packages/instant/src/constants.ts @@ -30,8 +30,6 @@ export const ONE_SECOND_MS = 1000; export const ONE_MINUTE_MS = ONE_SECOND_MS * 60; export const GIT_SHA = process.env.GIT_SHA; export const NODE_ENV = process.env.NODE_ENV; -export const ERC20_SWAP_QUOTE_SLIPPAGE_PERCENTAGE = 0.2; -export const ERC721_SWAP_QUOTE_SLIPPAGE_PERCENTAGE = 0; export const NPM_PACKAGE_VERSION = process.env.NPM_PACKAGE_VERSION; export const DEFAULT_UNKOWN_ASSET_NAME = '???'; export const ACCOUNT_UPDATE_INTERVAL_TIME_MS = ONE_SECOND_MS * 5; diff --git a/packages/instant/src/util/swap_quote_updater.ts b/packages/instant/src/util/swap_quote_updater.ts index 88f9b04188..97bfcad0ba 100644 --- a/packages/instant/src/util/swap_quote_updater.ts +++ b/packages/instant/src/util/swap_quote_updater.ts @@ -4,7 +4,6 @@ import { BigNumber } from '@0x/utils'; import { Web3Wrapper } from '@0x/web3-wrapper'; import { Dispatch } from 'redux'; -import { ERC20_SWAP_QUOTE_SLIPPAGE_PERCENTAGE, ERC721_SWAP_QUOTE_SLIPPAGE_PERCENTAGE } from '../constants'; import { Action, actions } from '../redux/actions'; import { Asset, QuoteFetchOrigin } from '../types'; @@ -37,10 +36,6 @@ export const swapQuoteUpdater = { } const wethAssetData = await swapQuoter.getEtherTokenAssetDataOrThrowAsync(); let newSwapQuote: MarketBuySwapQuote | undefined; - const slippagePercentage = - asset.metaData.assetProxyId === AssetProxyId.ERC20 - ? ERC20_SWAP_QUOTE_SLIPPAGE_PERCENTAGE - : ERC721_SWAP_QUOTE_SLIPPAGE_PERCENTAGE; try { const gasInfo = await gasPriceEstimator.getGasInfoAsync(); newSwapQuote = await swapQuoter.getMarketBuySwapQuoteForAssetDataAsync( @@ -48,7 +43,6 @@ export const swapQuoteUpdater = { wethAssetData, baseUnitValue, { - slippagePercentage, gasPrice: gasInfo.gasPriceInWei, // Only use native orders // excludedSources: [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Kyber, ERC20BridgeSource.Uniswap], From cc12ad8d8604dd4d2d2ba0b5204e9f32b5025dcf Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Mon, 9 Mar 2020 21:43:19 -0400 Subject: [PATCH 07/14] `@0x/asset-swapper`: Only generate fallbacks for native orders in optimal path. `@0x/asset-swapper`: Exclude conflicting sources across both optimal and fallback paths. --- .../src/utils/market_operation_utils/fills.ts | 24 ++++++++-- .../src/utils/market_operation_utils/index.ts | 44 ++++++++++--------- .../test/market_operation_utils_test.ts | 24 ++++++---- 3 files changed, 60 insertions(+), 32 deletions(-) diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts index f1afd91b5e..694d12aa92 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts @@ -259,7 +259,25 @@ export function collapsePath(side: MarketOperation, path: Fill[]): CollapsedFill return collapsed; } -export function getUnusedSourcePaths(usedPath: Fill[], allPaths: Fill[][]): Fill[][] { - const usedSources = usedPath.map(f => f.source); - return allPaths.filter(p => !usedSources.includes(p[0].source)); +export function getFallbackSourcePaths(optimalPath: Fill[], allPaths: Fill[][]): Fill[][] { + let optimalPathFlags = 0; + const optimalSources: ERC20BridgeSource[] = []; + for (const fill of optimalPath) { + optimalPathFlags |= fill.flags; + if (!optimalSources.includes(fill.source)) { + optimalSources.push(fill.source); + } + } + const conflictFlags = FillFlags.Kyber | FillFlags.ConflictsWithKyber; + const fallbackPaths: Fill[][] = []; + for (const path of allPaths) { + if (((optimalPathFlags | path[0].flags) & conflictFlags) === conflictFlags) { + continue; + } + if (optimalSources.includes(path[0].source)) { + continue; + } + fallbackPaths.push(path); + } + return fallbackPaths; } diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index 7fa8626403..dd3b7d46f2 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -6,7 +6,7 @@ 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, getUnusedSourcePaths } from './fills'; +import { createFillPaths, getFallbackSourcePaths, getPathSize } from './fills'; import { createOrdersFromPath, createSignedOrdersWithFillableAmounts, getNativeOrderTokens } from './orders'; import { findOptimalPath } from './path_optimizer'; import { DexOrderSampler, getSampleAmounts } from './sampler'; @@ -67,14 +67,14 @@ export class MarketOperationUtils { ), // Get ETH -> maker token price. DexOrderSampler.ops.getMedianSellRate( - difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( - this._liquidityProviderSourceIfAvailable(_opts.excludedSources), - ), - makerToken, - this._wethAddress, - ONE_ETHER, - this._liquidityProviderRegistry, - ), + difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( + this._liquidityProviderSourceIfAvailable(_opts.excludedSources), + ), + makerToken, + this._wethAddress, + ONE_ETHER, + this._liquidityProviderRegistry, + ), // Get sell quotes for taker -> maker. DexOrderSampler.ops.getSellQuotes( difference(SELL_SOURCES, _opts.excludedSources).concat( @@ -138,14 +138,14 @@ export class MarketOperationUtils { ), // Get ETH -> taker token price. DexOrderSampler.ops.getMedianSellRate( - difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( - this._liquidityProviderSourceIfAvailable(_opts.excludedSources), - ), - takerToken, - this._wethAddress, - ONE_ETHER, - this._liquidityProviderRegistry, - ), + difference(FEE_QUOTE_SOURCES, _opts.excludedSources).concat( + this._liquidityProviderSourceIfAvailable(_opts.excludedSources), + ), + takerToken, + this._wethAddress, + ONE_ETHER, + this._liquidityProviderRegistry, + ), // Get buy quotes for taker -> maker. DexOrderSampler.ops.getBuyQuotes( difference(BUY_SOURCES, _opts.excludedSources).concat( @@ -281,11 +281,15 @@ export class MarketOperationUtils { if (!optimalPath) { throw new Error(AggregationError.NoOptimalPath); } - // Find a fallback path from sources not used in the first path. + // Generate a fallback path if native orders are in the optimal paath. let fallbackPath: Fill[] = []; - if (opts.allowFallback) { + const nativeSubPath = optimalPath.filter(f => f.source === ERC20BridgeSource.Native); + if (opts.allowFallback && nativeSubPath.length !== 0) { + // The fallback path is only as large as the native path. + const [nativeInputAmount] = getPathSize(nativeSubPath, inputAmount); fallbackPath = - findOptimalPath(side, getUnusedSourcePaths(optimalPath, paths), inputAmount, opts.runLimit) || []; + findOptimalPath(side, getFallbackSourcePaths(optimalPath, paths), nativeInputAmount, opts.runLimit) || + []; } return createOrdersFromPath([...optimalPath, ...fallbackPath], { side, diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 71b990f445..9ddae1a08d 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -571,9 +571,10 @@ describe('MarketOperationUtils tests', () => { it('fallback orders use different sources', async () => { const rates: RatesBySource = {}; - rates[ERC20BridgeSource.Eth2Dai] = [0.9, 0.8, 0.5, 0.5]; + rates[ERC20BridgeSource.Native] = [0.9, 0.8, 0.5, 0.5]; rates[ERC20BridgeSource.Uniswap] = [0.6, 0.05, 0.01, 0.01]; - rates[ERC20BridgeSource.Native] = [0.4, 0.3, 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), @@ -584,8 +585,13 @@ describe('MarketOperationUtils tests', () => { { ...DEFAULT_OPTS, numSamples: 4, allowFallback: true }, ); const orderSources = improvedOrders.map(o => o.fill.source); - const firstSources = [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Uniswap]; - const secondSources = [ERC20BridgeSource.Native, ERC20BridgeSource.Kyber, ERC20BridgeSource.Native]; + const firstSources = [ + ERC20BridgeSource.Native, + ERC20BridgeSource.Native, + ERC20BridgeSource.Native, + ERC20BridgeSource.Uniswap, + ]; + const secondSources = [ERC20BridgeSource.Eth2Dai]; expect(orderSources.slice(0, firstSources.length).sort()).to.deep.eq(firstSources.sort()); expect(orderSources.slice(firstSources.length).sort()).to.deep.eq(secondSources.sort()); }); @@ -881,9 +887,9 @@ describe('MarketOperationUtils tests', () => { it('fallback orders use different sources', async () => { const rates: RatesBySource = {}; - rates[ERC20BridgeSource.Eth2Dai] = [0.9, 0.8, 0.5, 0.5]; + rates[ERC20BridgeSource.Native] = [0.9, 0.8, 0.5, 0.5]; rates[ERC20BridgeSource.Uniswap] = [0.6, 0.05, 0.01, 0.01]; - rates[ERC20BridgeSource.Native] = [0.4, 0.3, 0.01, 0.01]; + rates[ERC20BridgeSource.Eth2Dai] = [0.4, 0.3, 0.01, 0.01]; replaceSamplerOps({ getBuyQuotes: createGetMultipleBuyQuotesOperationFromRates(rates), }); @@ -893,13 +899,13 @@ describe('MarketOperationUtils tests', () => { { ...DEFAULT_OPTS, numSamples: 4, allowFallback: true }, ); const orderSources = improvedOrders.map(o => o.fill.source); - const firstSources = [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Uniswap]; - const secondSources = [ - ERC20BridgeSource.Native, + const firstSources = [ ERC20BridgeSource.Native, ERC20BridgeSource.Native, ERC20BridgeSource.Native, + ERC20BridgeSource.Uniswap, ]; + const secondSources = [ERC20BridgeSource.Eth2Dai]; expect(orderSources.slice(0, firstSources.length).sort()).to.deep.eq(firstSources.sort()); expect(orderSources.slice(firstSources.length).sort()).to.deep.eq(secondSources.sort()); }); From 05bf55dca8979ccf4d41b62c4714464d8426ee05 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Mon, 9 Mar 2020 22:22:57 -0400 Subject: [PATCH 08/14] `@0x/asset-swapper`: Add `gasSchedule` option to `SwapQuoter`. `@0x/asset-swapper`: Rename `fees` `SwapQuoter` option to `feeSchedule`. --- packages/asset-swapper/src/types.ts | 2 ++ packages/asset-swapper/src/utils/assert.ts | 1 + .../utils/market_operation_utils/constants.ts | 3 +- .../src/utils/market_operation_utils/fills.ts | 8 ++--- .../src/utils/market_operation_utils/index.ts | 10 +++---- .../src/utils/market_operation_utils/types.ts | 6 +++- .../src/utils/swap_quote_calculator.ts | 30 +++++++++++++++---- .../test/market_operation_utils_test.ts | 16 +++++----- .../asset-swapper/test/utils/swap_quote.ts | 1 + 9 files changed, 52 insertions(+), 25 deletions(-) diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index fc88260319..97b7f5cdb2 100644 --- a/packages/asset-swapper/src/types.ts +++ b/packages/asset-swapper/src/types.ts @@ -169,6 +169,7 @@ export interface MarketBuySwapQuote extends SwapQuoteBase { * totalTakerAssetAmount: The total amount of takerAsset required to complete the swap (filling orders, and paying takerFees). * makerAssetAmount: The amount of makerAsset that will be acquired through the swap. * protocolFeeInWeiAmount: The amount of ETH to pay (in WEI) as protocol fee to perform the swap for desired asset. + * gas: Amount of estimated gas needed to fill the quote. */ export interface SwapQuoteInfo { feeTakerAssetAmount: BigNumber; @@ -176,6 +177,7 @@ export interface SwapQuoteInfo { totalTakerAssetAmount: BigNumber; makerAssetAmount: BigNumber; protocolFeeInWeiAmount: BigNumber; + gas: number; } /** diff --git a/packages/asset-swapper/src/utils/assert.ts b/packages/asset-swapper/src/utils/assert.ts index 73303fad85..377bdc6e88 100644 --- a/packages/asset-swapper/src/utils/assert.ts +++ b/packages/asset-swapper/src/utils/assert.ts @@ -82,6 +82,7 @@ export const assert = { ); }, isValidSwapQuoteInfo(variableName: string, swapQuoteInfo: SwapQuoteInfo): void { + sharedAssert.isNumber(`${variableName}.gas`, swapQuoteInfo.gas); sharedAssert.isBigNumber(`${variableName}.feeTakerAssetAmount`, swapQuoteInfo.feeTakerAssetAmount); sharedAssert.isBigNumber(`${variableName}.totalTakerAssetAmount`, swapQuoteInfo.totalTakerAssetAmount); sharedAssert.isBigNumber(`${variableName}.takerAssetAmount`, swapQuoteInfo.takerAssetAmount); diff --git a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts index 1612cb9de4..02b1042004 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts @@ -30,7 +30,8 @@ export const DEFAULT_GET_MARKET_ORDERS_OPTS: GetMarketOrdersOpts = { bridgeSlippage: 0.005, numSamples: 20, sampleDistributionBase: 1.05, - fees: {}, + feeSchedule: {}, + gasSchedule: {}, allowFallback: true, }; diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts index 694d12aa92..41b7493eda 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts @@ -26,18 +26,18 @@ export function createFillPaths(opts: { targetInput?: BigNumber; ethToOutputRate?: BigNumber; excludedSources?: ERC20BridgeSource[]; - fees?: { [source: string]: BigNumber }; + feeSchedule?: { [source: string]: BigNumber }; }): Fill[][] { const { side } = opts; const excludedSources = opts.excludedSources || []; - const fees = opts.fees || {}; + const feeSchedule = opts.feeSchedule || {}; const orders = opts.orders || []; const dexQuotes = opts.dexQuotes || []; const ethToOutputRate = opts.ethToOutputRate || ZERO_AMOUNT; // Create native fill paths. - const nativePath = nativeOrdersToPath(side, orders, ethToOutputRate, fees); + const nativePath = nativeOrdersToPath(side, orders, ethToOutputRate, feeSchedule); // Create DEX fill paths. - const dexPaths = dexQuotesToPaths(side, dexQuotes, ethToOutputRate, fees); + const dexPaths = dexQuotesToPaths(side, dexQuotes, ethToOutputRate, feeSchedule); return filterPaths([...dexPaths, nativePath].map(p => clipPathToInput(p, opts.targetInput)), excludedSources); } diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index dd3b7d46f2..055a27f01f 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -98,7 +98,7 @@ export class MarketOperationUtils { ethToOutputRate: ethToMakerAssetRate, bridgeSlippage: _opts.bridgeSlippage, excludedSources: _opts.excludedSources, - fees: _opts.fees, + feeSchedule: _opts.feeSchedule, allowFallback: _opts.allowFallback, }); } @@ -170,7 +170,7 @@ export class MarketOperationUtils { ethToOutputRate: ethToTakerAssetRate, bridgeSlippage: _opts.bridgeSlippage, excludedSources: _opts.excludedSources, - fees: _opts.fees, + feeSchedule: _opts.feeSchedule, allowFallback: _opts.allowFallback, }); } @@ -242,7 +242,7 @@ export class MarketOperationUtils { ethToOutputRate: ethToTakerAssetRate, bridgeSlippage: _opts.bridgeSlippage, excludedSources: _opts.excludedSources, - fees: _opts.fees, + feeSchedule: _opts.feeSchedule, allowFallback: _opts.allowFallback, }); }); @@ -260,7 +260,7 @@ export class MarketOperationUtils { ethToOutputRate?: BigNumber; bridgeSlippage?: number; excludedSources?: ERC20BridgeSource[]; - fees?: { [source: string]: BigNumber }; + feeSchedule?: { [source: string]: BigNumber }; allowFallback?: boolean; liquidityProviderAddress?: string; }): OptimizedMarketOrder[] { @@ -274,7 +274,7 @@ export class MarketOperationUtils { targetInput: inputAmount, ethToOutputRate: opts.ethToOutputRate, excludedSources: opts.excludedSources, - fees: opts.fees, + feeSchedule: opts.feeSchedule, }); // Find the optimal path. const optimalPath = findOptimalPath(side, paths, inputAmount, opts.runLimit); diff --git a/packages/asset-swapper/src/utils/market_operation_utils/types.ts b/packages/asset-swapper/src/utils/market_operation_utils/types.ts index b4e86e6d22..e6f33dd196 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -167,7 +167,11 @@ export interface GetMarketOrdersOpts { /** * Fees for each liquidity source, expressed in gas. */ - fees: { [source: string]: BigNumber }; + feeSchedule: { [source: string]: BigNumber }; + /** + * Estimated gas consumed by each liquidity source. + */ + gasSchedule: { [source: string]: number }; /** * Whether to pad the quote with a redundant fallback quote using different * sources. diff --git a/packages/asset-swapper/src/utils/swap_quote_calculator.ts b/packages/asset-swapper/src/utils/swap_quote_calculator.ts index 574ebfe014..5c57b4a488 100644 --- a/packages/asset-swapper/src/utils/swap_quote_calculator.ts +++ b/packages/asset-swapper/src/utils/swap_quote_calculator.ts @@ -106,6 +106,7 @@ export class SwapQuoteCalculator { operation, assetFillAmounts[i], gasPrice, + opts.gasSchedule, ); } else { return undefined; @@ -133,7 +134,7 @@ export class SwapQuoteCalculator { // Scale fees by gas price. const _opts = { ...opts, - fees: _.mapValues(opts.fees, (v, k) => v.times(gasPrice)), + fees: _.mapValues(opts.feeSchedule, v => v.times(gasPrice)), }; const firstOrderMakerAssetData = !!prunedOrders[0] @@ -169,6 +170,7 @@ export class SwapQuoteCalculator { operation, assetFillAmount, gasPrice, + opts.gasSchedule, ); } private async _createSwapQuoteAsync( @@ -178,17 +180,20 @@ export class SwapQuoteCalculator { operation: MarketOperation, assetFillAmount: BigNumber, gasPrice: BigNumber, + gasSchedule: { [source: string]: number }, ): Promise { const bestCaseQuoteInfo = await this._calculateQuoteInfoAsync( resultOrders, assetFillAmount, gasPrice, + gasSchedule, operation, ); const worstCaseQuoteInfo = await this._calculateQuoteInfoAsync( resultOrders, assetFillAmount, gasPrice, + gasSchedule, operation, true, ); @@ -226,14 +231,16 @@ export class SwapQuoteCalculator { orders: OptimizedMarketOrder[], assetFillAmount: BigNumber, gasPrice: BigNumber, + gasSchedule: { [source: string]: number }, operation: MarketOperation, worstCase: boolean = false, ): Promise { - if (operation === MarketOperation.Buy) { - return this._calculateMarketBuyQuoteInfoAsync(orders, assetFillAmount, gasPrice, worstCase); - } else { - return this._calculateMarketSellQuoteInfoAsync(orders, assetFillAmount, gasPrice, worstCase); - } + return { + ...(operation === MarketOperation.Buy + ? await this._calculateMarketBuyQuoteInfoAsync(orders, assetFillAmount, gasPrice, worstCase) + : await this._calculateMarketSellQuoteInfoAsync(orders, assetFillAmount, gasPrice, worstCase)), + gas: getGasUsedByOrders(orders, gasSchedule), + }; } private async _calculateMarketSellQuoteInfoAsync( @@ -327,6 +334,7 @@ export class SwapQuoteCalculator { totalTakerAssetAmount: totalFeeTakerAssetAmount.plus(totalTakerAssetAmount), makerAssetAmount: totalMakerAssetAmount, protocolFeeInWeiAmount, + gas: 0, }; } @@ -416,6 +424,7 @@ export class SwapQuoteCalculator { totalTakerAssetAmount: totalFeeTakerAssetAmount.plus(totalTakerAssetAmount), makerAssetAmount: totalMakerAssetAmount, protocolFeeInWeiAmount, + gas: 0, }; } @@ -485,3 +494,12 @@ function getTakerAssetAmountBreakDown( takerAssetAmount: takerAssetAmountWithFees, }; } + +function getGasUsedByOrders(orders: OptimizedMarketOrder[], gasSchedule: { [source: string]: number }): number { + let totalUsage = 0; + for (const order of orders) { + totalUsage += gasSchedule[order.fill.source] || 0; + } + return totalUsage; +} +// tslint:disable: max-file-line-count diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 9ddae1a08d..b7f917556d 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -486,7 +486,7 @@ describe('MarketOperationUtils tests', () => { [ERC20BridgeSource.Eth2Dai]: [0.95, 0.1, 0.1, 0.1], [ERC20BridgeSource.Kyber]: [0.1, 0.1, 0.1, 0.1], }; - const fees = { + const feeSchedule = { [ERC20BridgeSource.Native]: FILL_AMOUNT.div(4) .times(nativeFeeRate) .dividedToIntegerBy(ETH_TO_MAKER_RATE), @@ -498,7 +498,7 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync( createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, fees }, + { ...DEFAULT_OPTS, numSamples: 4, feeSchedule }, ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ @@ -521,7 +521,7 @@ describe('MarketOperationUtils tests', () => { // Effectively [0.8, ~0.5, ~0, ~0] [ERC20BridgeSource.Uniswap]: [1, 0.7, 0.2, 0.2], }; - const fees = { + const feeSchedule = { [ERC20BridgeSource.Uniswap]: FILL_AMOUNT.div(4) .times(uniswapFeeRate) .dividedToIntegerBy(ETH_TO_MAKER_RATE), @@ -533,7 +533,7 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketSellOrdersAsync( createOrdersFromSellRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, fees }, + { ...DEFAULT_OPTS, numSamples: 4, feeSchedule }, ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ @@ -828,7 +828,7 @@ describe('MarketOperationUtils tests', () => { [ERC20BridgeSource.Eth2Dai]: [0.95, 0.1, 0.1, 0.1], [ERC20BridgeSource.Kyber]: [0.1, 0.1, 0.1, 0.1], }; - const fees = { + const feeSchedule = { [ERC20BridgeSource.Native]: FILL_AMOUNT.div(4) .times(nativeFeeRate) .dividedToIntegerBy(ETH_TO_TAKER_RATE), @@ -840,7 +840,7 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketBuyOrdersAsync( createOrdersFromBuyRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, fees }, + { ...DEFAULT_OPTS, numSamples: 4, feeSchedule }, ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ @@ -862,7 +862,7 @@ describe('MarketOperationUtils tests', () => { [ERC20BridgeSource.Uniswap]: [1, 0.7, 0.2, 0.2], [ERC20BridgeSource.Eth2Dai]: [0.92, 0.1, 0.1, 0.1], }; - const fees = { + const feeSchedule = { [ERC20BridgeSource.Uniswap]: FILL_AMOUNT.div(4) .times(uniswapFeeRate) .dividedToIntegerBy(ETH_TO_TAKER_RATE), @@ -874,7 +874,7 @@ describe('MarketOperationUtils tests', () => { const improvedOrders = await marketOperationUtils.getMarketBuyOrdersAsync( createOrdersFromBuyRates(FILL_AMOUNT, rates[ERC20BridgeSource.Native]), FILL_AMOUNT, - { ...DEFAULT_OPTS, numSamples: 4, fees }, + { ...DEFAULT_OPTS, numSamples: 4, feeSchedule }, ); const orderSources = improvedOrders.map(o => o.fill.source); const expectedSources = [ diff --git a/packages/asset-swapper/test/utils/swap_quote.ts b/packages/asset-swapper/test/utils/swap_quote.ts index 7321745f9d..c347602961 100644 --- a/packages/asset-swapper/test/utils/swap_quote.ts +++ b/packages/asset-swapper/test/utils/swap_quote.ts @@ -25,6 +25,7 @@ export async function getFullyFillableSwapQuoteWithNoFeesAsync( takerAssetAmount: totalTakerAssetAmount, totalTakerAssetAmount, protocolFeeInWeiAmount: await protocolFeeUtils.calculateWorstCaseProtocolFeeAsync(orders, gasPrice), + gas: 200e3, }; const breakdown = { From 0e7a47311675e9465d526a7dc2fa33acc937c595 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Tue, 10 Mar 2020 00:05:14 -0400 Subject: [PATCH 09/14] `@0x/asset-swapper`: Clean up source breakdown code a bit. `@0x/asset-swapper`: Allow Kyber conflicts in fallback path. --- .../src/utils/market_operation_utils/fills.ts | 12 ++--- .../src/utils/market_operation_utils/index.ts | 6 +-- .../src/utils/swap_quote_calculator.ts | 49 ++++++++----------- 3 files changed, 29 insertions(+), 38 deletions(-) diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts index 41b7493eda..69d5e0116e 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts @@ -260,23 +260,23 @@ export function collapsePath(side: MarketOperation, path: Fill[]): CollapsedFill } export function getFallbackSourcePaths(optimalPath: Fill[], allPaths: Fill[][]): Fill[][] { - let optimalPathFlags = 0; const optimalSources: ERC20BridgeSource[] = []; for (const fill of optimalPath) { - optimalPathFlags |= fill.flags; if (!optimalSources.includes(fill.source)) { optimalSources.push(fill.source); } } - const conflictFlags = FillFlags.Kyber | FillFlags.ConflictsWithKyber; const fallbackPaths: Fill[][] = []; for (const path of allPaths) { - if (((optimalPathFlags | path[0].flags) & conflictFlags) === conflictFlags) { - continue; - } if (optimalSources.includes(path[0].source)) { continue; } + // HACK(dorothy-zbornak): We *should* be filtering out paths that + // conflict with the optimal path (i.e., Kyber conflicts), but in + // practice we often end up not being able to find a fallback path + // because we've lost 2 major liquiduty sources. The end result is + // we end up with many more reverts than what would be actually caused + // by conflicts. fallbackPaths.push(path); } return fallbackPaths; diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index 055a27f01f..71e14a21be 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -285,10 +285,10 @@ export class MarketOperationUtils { let fallbackPath: Fill[] = []; const nativeSubPath = optimalPath.filter(f => f.source === ERC20BridgeSource.Native); if (opts.allowFallback && nativeSubPath.length !== 0) { - // The fallback path is only as large as the native path. - const [nativeInputAmount] = getPathSize(nativeSubPath, inputAmount); + // The fallback path is, at most, as large as the native path. + const fallbackInputAmount = BigNumber.min(inputAmount, getPathSize(nativeSubPath, inputAmount)[0]); fallbackPath = - findOptimalPath(side, getFallbackSourcePaths(optimalPath, paths), nativeInputAmount, opts.runLimit) || + findOptimalPath(side, getFallbackSourcePaths(optimalPath, paths), fallbackInputAmount, opts.runLimit) || []; } return createOrdersFromPath([...optimalPath, ...fallbackPath], { diff --git a/packages/asset-swapper/src/utils/swap_quote_calculator.ts b/packages/asset-swapper/src/utils/swap_quote_calculator.ts index 5c57b4a488..149066ab4c 100644 --- a/packages/asset-swapper/src/utils/swap_quote_calculator.ts +++ b/packages/asset-swapper/src/utils/swap_quote_calculator.ts @@ -198,7 +198,7 @@ export class SwapQuoteCalculator { true, ); - const breakdown = this._getSwapQuoteOrdersBreakdown(resultOrders, operation); + const breakdown = getSwapQuoteOrdersBreakdown(resultOrders, operation); const quoteBase: SwapQuoteBase = { takerAssetData, @@ -427,36 +427,27 @@ export class SwapQuoteCalculator { gas: 0, }; } +} - // tslint:disable-next-line: prefer-function-over-method - private _getSwapQuoteOrdersBreakdown( - orders: OptimizedMarketOrder[], - operation: MarketOperation, - ): SwapQuoteOrdersBreakdown { - // HACK: to shut up linter - const breakdown: SwapQuoteOrdersBreakdown = {}; - - // total asset amount (accounting for slippage protection) - const totalAssetAmount = BigNumber.sum( - ...[ - constants.ZERO_AMOUNT, - ...orders.map(o => (operation === MarketOperation.Buy ? o.makerAssetAmount : o.takerAssetAmount)), - ], - ); - - return orders.reduce((acc: SwapQuoteOrdersBreakdown, order: OptimizedMarketOrder): SwapQuoteOrdersBreakdown => { - const assetAmount = operation === MarketOperation.Buy ? order.makerAssetAmount : order.takerAssetAmount; - const { source } = order.fill; - return { - ...acc, - ...{ - [source]: !!acc[source] - ? acc[source].plus(assetAmount.dividedBy(totalAssetAmount)) - : assetAmount.dividedBy(totalAssetAmount), - }, - }; - }, breakdown); +function getSwapQuoteOrdersBreakdown( + orders: OptimizedMarketOrder[], + operation: MarketOperation, +): SwapQuoteOrdersBreakdown { + const orderAmounts = + operation === MarketOperation.Buy + ? orders.map(o => o.fill.totalMakerAssetAmount) + : orders.map(o => o.fill.totalTakerAssetAmount); + const amountsBySource: SwapQuoteOrdersBreakdown = {}; + orders.forEach((o, i) => { + const source = o.fill.source; + amountsBySource[source] = orderAmounts[i].plus(amountsBySource[source] || 0); + }); + const totalAmount = BigNumber.sum(0, ...orderAmounts); + const breakdown: SwapQuoteOrdersBreakdown = {}; + for (const [source, amount] of Object.entries(amountsBySource)) { + breakdown[source] = amount.div(totalAmount); } + return breakdown; } function getTakerAssetAmountBreakDown( From 5fd767b739c7e2aaf3d6979a8f3c86717f2fdafb Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Tue, 10 Mar 2020 13:25:39 -0400 Subject: [PATCH 10/14] `@0x/instant`: Fix changelog. --- packages/instant/CHANGELOG.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/instant/CHANGELOG.json b/packages/instant/CHANGELOG.json index 1ca3feeadd..150d379a9f 100644 --- a/packages/instant/CHANGELOG.json +++ b/packages/instant/CHANGELOG.json @@ -11,7 +11,7 @@ "pr": 2491 }, { - "note": "Remove `bridgeSlippage` SwapQuoter config.", + "note": "Remove `slippagePercentage` SwapQuoter config.", "pr": 2513 } ] From 37597eca753dfcb051e57b730debde6876f57e51 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Tue, 10 Mar 2020 13:26:29 -0400 Subject: [PATCH 11/14] `@0x/asset-swapper`: Fix `getPathSize()` and `getAdjustedPathSize()` bug. `@0x/asset-swapper`: Add `maxFallbackSlippage` option. --- packages/asset-swapper/CHANGELOG.json | 4 ++ packages/asset-swapper/src/types.ts | 1 - .../utils/market_operation_utils/constants.ts | 1 + .../src/utils/market_operation_utils/fills.ts | 26 ++++++++++- .../src/utils/market_operation_utils/index.ts | 28 ++++++++++-- .../src/utils/market_operation_utils/types.ts | 6 +++ .../test/market_operation_utils_test.ts | 45 ++++++++++++++++++- 7 files changed, 104 insertions(+), 7 deletions(-) diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index 74956f8b9d..6ab6da141e 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/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 } ] }, diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index 97b7f5cdb2..d809c1b061 100644 --- a/packages/asset-swapper/src/types.ts +++ b/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 { diff --git a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts index 02b1042004..c3a94add09 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/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: {}, diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts index 69d5e0116e..1d82cac0de 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts +++ b/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(); +} diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index 71e14a21be..bc40973176 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/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); - if (!optimalPath) { + const optimalPath = findOptimalPath(side, paths, inputAmount, opts.runLimit) || []; + // 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, diff --git a/packages/asset-swapper/src/utils/market_operation_utils/types.ts b/packages/asset-swapper/src/utils/market_operation_utils/types.ts index e6f33dd196..323297c7ca 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/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. */ diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index b7f917556d..87a809204c 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/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()); + }); }); }); }); From 109d466260d9b7dfb480b1b29a61015ca4d9977c Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Tue, 10 Mar 2020 23:02:41 -0400 Subject: [PATCH 12/14] `@0x/asset-swapper`: Fix double `bridgeSlippage` on generated orders. --- .../asset-swapper/src/utils/market_operation_utils/orders.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e4e082b2b9..6bef4ba01b 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts @@ -221,7 +221,7 @@ function createCommonBridgeOrderFields(fill: CollapsedFill, opts: CreateOrderFro ? fill.totalMakerAssetAmount.times(1 - opts.bridgeSlippage).integerValue(BigNumber.ROUND_DOWN) : fill.totalMakerAssetAmount; const takerAssetAmountAdjustedWithSlippage = - opts.side === MarketOperation.Buy + opts.side === MarketOperation.Sell ? fill.totalTakerAssetAmount : fill.totalTakerAssetAmount.times(opts.bridgeSlippage + 1).integerValue(BigNumber.ROUND_UP); return { From 87999f402f1ae8e4f0e37399b316e42e37040658 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Wed, 11 Mar 2020 01:17:23 -0400 Subject: [PATCH 13/14] `@0x/asset-swapper`: Sort native path fills by ADJUSTED rate. --- .../asset-swapper/src/utils/market_operation_utils/fills.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts index 1d82cac0de..74124da85a 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts @@ -94,8 +94,8 @@ function nativeOrdersToPath( fillData: { order }, }); } - // Sort by descending rate. - path = path.sort((a, b) => b.rate.comparedTo(a.rate)); + // Sort by descending adjusted rate. + path = path.sort((a, b) => b.adjustedRate.comparedTo(a.adjustedRate)); // Re-index fills. for (let i = 0; i < path.length; ++i) { path[i].parent = i === 0 ? undefined : path[i - 1]; From 1a9063a55b1ee9c345e8d330c4752ce137311fa9 Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Wed, 11 Mar 2020 12:19:00 -0400 Subject: [PATCH 14/14] `@0x/asset-swapper`: Address review comments. --- .../src/utils/market_operation_utils/constants.ts | 4 ++-- .../src/utils/market_operation_utils/fills.ts | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts index c3a94add09..6a37a77850 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts @@ -28,8 +28,8 @@ export const DEFAULT_GET_MARKET_ORDERS_OPTS: GetMarketOrdersOpts = { runLimit: 2 ** 15, excludedSources: [], bridgeSlippage: 0.005, - maxFallbackSlippage: 0.1, - numSamples: 20, + maxFallbackSlippage: 0.05, + numSamples: 13, sampleDistributionBase: 1.05, feeSchedule: {}, gasSchedule: {}, diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts index 74124da85a..0f7564330e 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts @@ -87,9 +87,9 @@ function nativeOrdersToPath( rate, adjustedRate, adjustedOutput, - index: path.length, flags: 0, - parent: path.length !== 0 ? path[path.length - 1] : undefined, + index: 0, // TBD + parent: undefined, // TBD source: ERC20BridgeSource.Native, fillData: { order }, }); @@ -131,8 +131,7 @@ function dexQuotesToPaths( : ZERO_AMOUNT; const adjustedOutput = side === MarketOperation.Sell ? output.minus(penalty) : output.plus(penalty); const rate = side === MarketOperation.Sell ? output.div(input) : input.div(output); - const adjustedRate = - side === MarketOperation.Sell ? output.minus(penalty).div(input) : input.div(output.plus(penalty)); + const adjustedRate = side === MarketOperation.Sell ? adjustedOutput.div(input) : input.div(adjustedOutput); path.push({ input,