diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index dd0ad365b3..8c9b3d45d7 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -18,6 +18,10 @@ "note": "Support more varied curves", "pr": 2633 }, + { + "note": "Make path optimization go faster", + "pr": 2640 + }, { "note": "Adds `getBidAskLiquidityForMakerTakerAssetPairAsync` to return more detailed sample information", "pr": 2641 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 6908eca747..eb7b0aacff 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts @@ -1,4 +1,4 @@ -import { BigNumber } from '@0x/utils'; +import { BigNumber, hexUtils } from '@0x/utils'; import { MarketOperation, SignedOrderWithFillableAmounts } from '../../types'; import { fillableAmountsUtils } from '../../utils/fillable_amounts_utils'; @@ -56,6 +56,7 @@ function nativeOrdersToPath( ethToOutputRate: BigNumber, fees: FeeSchedule, ): Fill[] { + const sourcePathId = hexUtils.random(); // Create a single path from all orders. let path: Fill[] = []; for (const order of orders) { @@ -84,6 +85,7 @@ function nativeOrdersToPath( continue; } path.push({ + sourcePathId, input: clippedInput, output: clippedOutput, rate, @@ -114,6 +116,7 @@ function dexQuotesToPaths( ): Fill[][] { const paths: Fill[][] = []; for (let quote of dexQuotes) { + const sourcePathId = hexUtils.random(); const path: Fill[] = []; // Drop any non-zero entries. This can occur if the any fills on Kyber were UniswapReserves // We need not worry about Kyber fills going to UniswapReserve as the input amount @@ -136,6 +139,7 @@ function dexQuotesToPaths( const adjustedRate = side === MarketOperation.Sell ? adjustedOutput.div(input) : input.div(adjustedOutput); path.push({ + sourcePathId, input, output, rate, @@ -261,35 +265,12 @@ export function collapsePath(path: Fill[]): CollapsedFill[] { return collapsed; } -export function getFallbackSourcePaths(optimalPath: Fill[], allPaths: Fill[][]): Fill[][] { - const optimalSources: ERC20BridgeSource[] = []; - for (const fill of optimalPath) { - if (!optimalSources.includes(fill.source)) { - optimalSources.push(fill.source); - } - } - const fallbackPaths: Fill[][] = []; - for (const path of allPaths) { - 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; -} - export function getPathAdjustedRate(side: MarketOperation, path: Fill[], targetInput: BigNumber): BigNumber { - const [input, output] = getPathAdjustedSize(path, targetInput); - if (input.eq(0) || output.eq(0)) { + const [, output] = getPathAdjustedSize(path, targetInput); + if (output.eq(0)) { return ZERO_AMOUNT; } - return side === MarketOperation.Sell ? output.div(input) : input.div(output); + return side === MarketOperation.Sell ? output.div(targetInput) : targetInput.div(output); } export function getPathAdjustedSlippage( 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 f4870e5d2f..dd5627fa10 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 @@ -3,16 +3,12 @@ import { BigNumber } from '@0x/utils'; import { MarketOperation } from '../../types'; import { ZERO_AMOUNT } from './constants'; -import { getPathAdjustedSize, getPathSize, isValidPath } from './fills'; +import { getPathAdjustedRate, getPathSize, isValidPath } from './fills'; import { Fill } from './types'; // tslint:disable: prefer-for-of custom-no-magic-numbers completed-docs -const RUN_LIMIT_DECAY_FACTOR = 0.8; -// Used to yield the event loop when performing CPU intensive tasks -// tislint:disable-next-line:no-inferred-empty-object-type -const setImmediateAsync = async (delay: number = 0) => - new Promise(resolve => setImmediate(() => resolve(), delay)); +const RUN_LIMIT_DECAY_FACTOR = 0.5; /** * Find the optimal mixture of paths that maximizes (for sells) or minimizes @@ -22,16 +18,19 @@ export async function findOptimalPathAsync( side: MarketOperation, paths: Fill[][], targetInput: BigNumber, - runLimit: number = 2 ** 15, + runLimit: number = 2 ** 8, ): Promise { - // Sort paths in descending order by adjusted output amount. + // Sort paths by descending adjusted rate. const sortedPaths = paths .slice(0) - .sort((a, b) => getPathAdjustedSize(b, targetInput)[1].comparedTo(getPathAdjustedSize(a, targetInput)[1])); + .sort((a, b) => + getPathAdjustedRate(side, b, targetInput).comparedTo(getPathAdjustedRate(side, a, targetInput)), + ); let optimalPath = sortedPaths[0] || []; for (const [i, path] of sortedPaths.slice(1).entries()) { optimalPath = mixPaths(side, optimalPath, path, targetInput, runLimit * RUN_LIMIT_DECAY_FACTOR ** i); - await setImmediateAsync(); + // Yield to event loop. + await Promise.resolve(); } return isPathComplete(optimalPath, targetInput) ? optimalPath : undefined; } @@ -43,9 +42,10 @@ function mixPaths( targetInput: BigNumber, maxSteps: number, ): Fill[] { - let bestPath: Fill[] = []; - let bestPathInput = ZERO_AMOUNT; - let bestPathRate = ZERO_AMOUNT; + const _maxSteps = Math.max(maxSteps, 16); + let bestPath: Fill[] = pathA; + let bestPathInput = getPathSize(pathA, targetInput)[0]; + let bestPathRate = getPathAdjustedRate(side, pathA, targetInput); let steps = 0; const _isBetterPath = (input: BigNumber, rate: BigNumber) => { if (bestPathInput.lt(targetInput)) { @@ -57,7 +57,7 @@ function mixPaths( }; const _walk = (path: Fill[], input: BigNumber, output: BigNumber, allFills: Fill[]) => { steps += 1; - const rate = getRate(side, input, output); + const rate = getRate(side, targetInput, output); if (_isBetterPath(input, rate)) { bestPath = path; bestPathInput = input; @@ -65,13 +65,11 @@ function mixPaths( } const remainingInput = targetInput.minus(input); if (remainingInput.gt(0)) { - for (let i = 0; i < allFills.length; ++i) { + for (let i = 0; i < allFills.length && steps < _maxSteps; ++i) { const fill = allFills[i]; - if (steps + 1 >= maxSteps) { - break; - } - const childPath = [...path, fill]; - if (!isValidPath(childPath, true)) { + const nextPath = [...path, fill]; + // Only walk valid paths. + if (!isValidPath(nextPath, true)) { continue; } // Remove this fill from the next list of candidate fills. @@ -79,7 +77,7 @@ function mixPaths( nextAllFills.splice(i, 1); // Recurse. _walk( - childPath, + nextPath, input.plus(BigNumber.min(remainingInput, fill.input)), output.plus( // Clip the output of the next fill to the remaining @@ -91,7 +89,27 @@ function mixPaths( } } }; - _walk(bestPath, ZERO_AMOUNT, ZERO_AMOUNT, [...pathA, ...pathB].sort((a, b) => b.rate.comparedTo(a.rate))); + const allPaths = [...pathA, ...pathB]; + const sources = allPaths.filter(f => f.index === 0).map(f => f.sourcePathId); + const rateBySource = Object.assign( + {}, + ...sources.map(s => ({ + [s]: getPathAdjustedRate(side, allPaths.filter(f => f.sourcePathId === s), targetInput), + })), + ); + _walk( + [], + ZERO_AMOUNT, + ZERO_AMOUNT, + // Sort subpaths by rate and keep fills contiguous to improve our + // chances of walking ideal, valid paths first. + allPaths.sort((a, b) => { + if (a.sourcePathId !== b.sourcePathId) { + return rateBySource[b.sourcePathId].comparedTo(rateBySource[a.sourcePathId]); + } + return a.index - b.index; + }), + ); return bestPath; } 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 e432f5497e..a2b27ee401 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -120,6 +120,10 @@ export enum FillFlags { * Represents a node on a fill path. */ export interface Fill { + // Unique ID of the original source path this fill belongs to. + // This is generated when the path is generated and is useful to distinguish + // paths that have the same `source` IDs but are distinct (e.g., Curves). + sourcePathId: string; // See `FillFlags`. flags: FillFlags; // Input fill amount (taker asset amount in a sell, maker asset amount in a buy). diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 4392762120..42e54e214d 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -703,15 +703,9 @@ describe('MarketOperationUtils tests', () => { ); const improvedOrders = improvedOrdersResponse.optimizedOrders; const orderSources = improvedOrders.map(o => o.fills[0].source); - const firstSources = [ - ERC20BridgeSource.Native, - ERC20BridgeSource.Native, - ERC20BridgeSource.Native, - ERC20BridgeSource.Uniswap, - ]; - const secondSources = [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Kyber]; - expect(orderSources.slice(0, firstSources.length).sort()).to.deep.eq(firstSources.sort()); - expect(orderSources.slice(firstSources.length).sort()).to.deep.eq(secondSources.sort()); + const firstSources = orderSources.slice(0, 4); + const secondSources = orderSources.slice(4); + expect(_.intersection(firstSources, secondSources)).to.be.length(0); }); it('does not create a fallback if below maxFallbackSlippage', async () => { @@ -819,9 +813,9 @@ describe('MarketOperationUtils tests', () => { expect(improvedOrders).to.be.length(3); const orderFillSources = improvedOrders.map(o => o.fills.map(f => f.source)); expect(orderFillSources).to.deep.eq([ - [ERC20BridgeSource.Uniswap], + [ERC20BridgeSource.Uniswap, ERC20BridgeSource.Curve], [ERC20BridgeSource.Native], - [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Curve], + [ERC20BridgeSource.Eth2Dai], ]); }); }); @@ -1091,15 +1085,9 @@ describe('MarketOperationUtils tests', () => { ); const improvedOrders = improvedOrdersResponse.optimizedOrders; const orderSources = improvedOrders.map(o => o.fills[0].source); - 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()); + const firstSources = orderSources.slice(0, 4); + const secondSources = orderSources.slice(4); + expect(_.intersection(firstSources, secondSources)).to.be.length(0); }); it('does not create a fallback if below maxFallbackSlippage', async () => { @@ -1145,7 +1133,7 @@ describe('MarketOperationUtils tests', () => { const orderFillSources = improvedOrders.map(o => o.fills.map(f => f.source)); expect(orderFillSources).to.deep.eq([ [ERC20BridgeSource.Native], - [ERC20BridgeSource.Eth2Dai, ERC20BridgeSource.Uniswap], + [ERC20BridgeSource.Uniswap, ERC20BridgeSource.Eth2Dai], ]); }); });