From 4cd767ecb8ee3baca74861946ea5042969f03a16 Mon Sep 17 00:00:00 2001 From: Kim Persson Date: Wed, 9 Mar 2022 15:39:47 +0100 Subject: [PATCH] feat: VIP routing in the router, don't create fallback orders for native [TKR-243] (#440) * feat: calculate all routes and VIP only routes in a single router call * fix: Try to disable using fallback orders for quotes with native orders * fix: create VIP sources set once per routing call * chore: use private publish version of neon-router to test * chore(lint): comment out unused fn _addOptionalFallbackAsync * chore: update to latest private publish of neon-router * refactor: fix router metrics beforeTimeMs naming * feat: don't recompute isVip for ever sample of a source * chore: update neon-router to real published version * fix: merge conflict resolution issue * chore: add asset-swapper changelog entry --- packages/asset-swapper/CHANGELOG.json | 4 + packages/asset-swapper/package.json | 2 +- .../src/utils/market_operation_utils/index.ts | 8 +- .../market_operation_utils/path_optimizer.ts | 337 +++++++++--------- yarn.lock | 7 +- 5 files changed, 189 insertions(+), 169 deletions(-) diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index 3f3ee0a0fa..39ac3a8ba4 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -5,6 +5,10 @@ { "note": "Routing glue optimization", "pr": 439 + }, + { + "note": "Move VIP source routing into neon-router & disable fallback orders for native/plp", + "pr": 440 } ] }, diff --git a/packages/asset-swapper/package.json b/packages/asset-swapper/package.json index 86c8c1c9d2..13ff4be653 100644 --- a/packages/asset-swapper/package.json +++ b/packages/asset-swapper/package.json @@ -66,7 +66,7 @@ "@0x/contracts-zero-ex": "^0.31.1", "@0x/dev-utils": "^4.2.11", "@0x/json-schemas": "^6.4.1", - "@0x/neon-router": "^0.3.3", + "@0x/neon-router": "^0.3.5", "@0x/protocol-utils": "^1.11.1", "@0x/quote-server": "^6.0.6", "@0x/types": "^3.3.4", 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 60dbedbd03..7b219f2fc9 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -570,7 +570,10 @@ export class MarketOperationUtils { } // Generate a fallback path if required - await this._addOptionalFallbackAsync(side, inputAmount, optimalPath, dexQuotes, fills, opts, penaltyOpts); + // TODO(kimpers): Will experiment with disabling this and see how it affects revert rate + // to avoid yet another router roundtrip + // TODO: clean this up if we don't need it + // await this._addOptionalFallbackAsync(side, inputAmount, optimalPath, dexQuotes, fills, opts, penaltyOpts); const collapsedPath = optimalPath.collapse(orderOpts); return { @@ -774,6 +777,8 @@ export class MarketOperationUtils { ); } + /* + * TODO(kimpers): Remove this when we know that it's safe to drop the fallbacks on native orders // tslint:disable-next-line: prefer-function-over-method private async _addOptionalFallbackAsync( side: MarketOperation, @@ -839,6 +844,7 @@ export class MarketOperationUtils { } } } + */ } // tslint:disable: max-file-line-count 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 c405b085d2..0b3dec1608 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 @@ -76,7 +76,8 @@ function findRoutesAndCreateOptimalPath( opts: PathPenaltyOpts, fees: FeeSchedule, neonRouterNumSamples: number, -): Path | undefined { + vipSourcesSet: Set, +): { allSourcesPath: Path | undefined; vipSourcesPath: Path | undefined } | undefined { // Currently the rust router is unable to handle 1 base unit sized quotes and will error out // To avoid flooding the logs with these errors we just return an insufficient liquidity error // which is how the JS router handles these quotes today @@ -94,6 +95,127 @@ function findRoutesAndCreateOptimalPath( return fills[0]; }; + const createPathFromStrategy = (sourcesRustRoute: Float64Array, sourcesOutputAmounts: Float64Array) => { + const routesAndSamplesAndOutputs = _.zip( + sourcesRustRoute, + samplesAndNativeOrdersWithResults, + sourcesOutputAmounts, + sampleSourcePathIds, + ); + const adjustedFills: Fill[] = []; + const totalRoutedAmount = BigNumber.sum(...sourcesRustRoute); + + const scale = input.dividedBy(totalRoutedAmount); + for (const [ + routeInput, + routeSamplesAndNativeOrders, + outputAmount, + sourcePathId, + ] of routesAndSamplesAndOutputs) { + if (!Number.isFinite(outputAmount)) { + DEFAULT_WARNING_LOGGER(rustArgs, `neon-router: invalid route outputAmount ${outputAmount}`); + return undefined; + } + if (!routeInput || !routeSamplesAndNativeOrders || !outputAmount) { + continue; + } + // TODO(kimpers): [TKR-241] amounts are sometimes clipped in the router due to precision loss for number/f64 + // we can work around it by scaling it and rounding up. However now we end up with a total amount of a couple base units too much + const rustInputAdjusted = BigNumber.min( + new BigNumber(routeInput).multipliedBy(scale).integerValue(BigNumber.ROUND_CEIL), + input, + ); + + const current = routeSamplesAndNativeOrders[routeSamplesAndNativeOrders.length - 1]; + if (!isDexSample(current)) { + const nativeFill = nativeOrdersToFills( + side, + [current], + rustInputAdjusted, + opts.outputAmountPerEth, + opts.inputAmountPerEth, + fees, + false, + )[0] as Fill | undefined; + // Note: If the order has an adjusted rate of less than or equal to 0 it will be skipped + // and nativeFill will be `undefined` + if (nativeFill) { + // NOTE: For Limit/RFQ orders we are done here. No need to scale output + adjustedFills.push({ ...nativeFill, sourcePathId: sourcePathId ?? hexUtils.random() }); + } + continue; + } + + // NOTE: For DexSamples only + let fill = createFill(current); + if (!fill) { + continue; + } + const routeSamples = routeSamplesAndNativeOrders as Array>; + // Descend to approach a closer fill for fillData which may not be consistent + // throughout the path (UniswapV3) and for a closer guesstimate at + // gas used + + assert.assert(routeSamples.length >= 1, 'Found no sample to use for source'); + for (let k = routeSamples.length - 1; k >= 0; k--) { + if (k === 0) { + fill = createFill(routeSamples[0]) ?? fill; + } + if (rustInputAdjusted.isGreaterThan(routeSamples[k].input)) { + const left = routeSamples[k]; + const right = routeSamples[k + 1]; + if (left && right) { + fill = + createFill({ + ...right, // default to the greater (for gas used) + input: rustInputAdjusted, + output: new BigNumber(outputAmount), + }) ?? fill; + } else { + assert.assert(Boolean(left || right), 'No valid sample to use'); + fill = createFill(left || right) ?? fill; + } + break; + } + } + + // TODO(kimpers): remove once we have solved the rounding/precision loss issues in the Rust router + const maxSampledOutput = BigNumber.max(...routeSamples.map(s => s.output)); + // Scale output by scale factor but never go above the largest sample in sell quotes (unknown liquidity) or below 1 base unit (unfillable) + const scaleOutput = (output: BigNumber) => { + // Don't try to scale 0 output as it will be clamped to 1 + if (output.eq(ZERO_AMOUNT)) { + return output; + } + + const scaled = output + .times(scale) + .decimalPlaces(0, side === MarketOperation.Sell ? BigNumber.ROUND_FLOOR : BigNumber.ROUND_CEIL); + const capped = MarketOperation.Sell ? BigNumber.min(scaled, maxSampledOutput) : scaled; + + return BigNumber.max(capped, 1); + }; + + adjustedFills.push({ + ...fill, + input: rustInputAdjusted, + output: scaleOutput(fill.output), + adjustedOutput: scaleOutput(fill.adjustedOutput), + index: 0, + parent: undefined, + sourcePathId: sourcePathId ?? hexUtils.random(), + }); + } + + if (adjustedFills.length === 0) { + return undefined; + } + + const pathFromRustInputs = Path.create(side, adjustedFills, input, opts); + + return pathFromRustInputs; + }; + const samplesAndNativeOrdersWithResults: Array = []; const serializedPaths: SerializedPath[] = []; const sampleSourcePathIds: string[] = []; @@ -136,6 +258,7 @@ function findRoutesAndCreateOptimalPath( inputs: [], outputs: [], outputFees: [], + isVip: vipSourcesSet.has(singleSourceSamplesWithOutput[0]?.source), }, ); @@ -194,6 +317,7 @@ function findRoutesAndCreateOptimalPath( inputs, outputs, outputFees, + isVip: true, }; samplesAndNativeOrdersWithResults.push([nativeOrder]); @@ -212,130 +336,42 @@ function findRoutesAndCreateOptimalPath( }; const allSourcesRustRoute = new Float64Array(rustArgs.pathsIn.length); - const strategySourcesOutputAmounts = new Float64Array(rustArgs.pathsIn.length); - route(rustArgs, allSourcesRustRoute, strategySourcesOutputAmounts, neonRouterNumSamples); + const allSourcesOutputAmounts = new Float64Array(rustArgs.pathsIn.length); + const vipSourcesRustRoute = new Float64Array(rustArgs.pathsIn.length); + const vipSourcesOutputAmounts = new Float64Array(rustArgs.pathsIn.length); + + route( + rustArgs, + allSourcesRustRoute, + allSourcesOutputAmounts, + vipSourcesRustRoute, + vipSourcesOutputAmounts, + neonRouterNumSamples, + ); assert.assert( rustArgs.pathsIn.length === allSourcesRustRoute.length, 'different number of sources in the Router output than the input', ); assert.assert( - rustArgs.pathsIn.length === strategySourcesOutputAmounts.length, + rustArgs.pathsIn.length === allSourcesOutputAmounts.length, + 'different number of sources in the Router output amounts results than the input', + ); + assert.assert( + rustArgs.pathsIn.length === vipSourcesRustRoute.length, + 'different number of sources in the Router output than the input', + ); + assert.assert( + rustArgs.pathsIn.length === vipSourcesOutputAmounts.length, 'different number of sources in the Router output amounts results than the input', ); - const routesAndSamplesAndOutputs = _.zip( - allSourcesRustRoute, - samplesAndNativeOrdersWithResults, - strategySourcesOutputAmounts, - sampleSourcePathIds, - ); - const adjustedFills: Fill[] = []; - const totalRoutedAmount = BigNumber.sum(...allSourcesRustRoute); + const allSourcesPath = createPathFromStrategy(allSourcesRustRoute, allSourcesOutputAmounts); + const vipSourcesPath = createPathFromStrategy(vipSourcesRustRoute, vipSourcesOutputAmounts); - const scale = input.dividedBy(totalRoutedAmount); - for (const [routeInput, routeSamplesAndNativeOrders, outputAmount, sourcePathId] of routesAndSamplesAndOutputs) { - if (!Number.isFinite(outputAmount)) { - DEFAULT_WARNING_LOGGER(rustArgs, `neon-router: invalid route outputAmount ${outputAmount}`); - return undefined; - } - if (!routeInput || !routeSamplesAndNativeOrders || !outputAmount) { - continue; - } - // TODO(kimpers): [TKR-241] amounts are sometimes clipped in the router due to precision loss for number/f64 - // we can work around it by scaling it and rounding up. However now we end up with a total amount of a couple base units too much - const rustInputAdjusted = BigNumber.min( - new BigNumber(routeInput).multipliedBy(scale).integerValue(BigNumber.ROUND_CEIL), - input, - ); - - const current = routeSamplesAndNativeOrders[routeSamplesAndNativeOrders.length - 1]; - if (!isDexSample(current)) { - const nativeFill = nativeOrdersToFills( - side, - [current], - rustInputAdjusted, - opts.outputAmountPerEth, - opts.inputAmountPerEth, - fees, - false, - )[0] as Fill | undefined; - // Note: If the order has an adjusted rate of less than or equal to 0 it will be skipped - // and nativeFill will be `undefined` - if (nativeFill) { - // NOTE: For Limit/RFQ orders we are done here. No need to scale output - adjustedFills.push({ ...nativeFill, sourcePathId: sourcePathId ?? hexUtils.random() }); - } - continue; - } - - // NOTE: For DexSamples only - let fill = createFill(current); - if (!fill) { - continue; - } - const routeSamples = routeSamplesAndNativeOrders as Array>; - // Descend to approach a closer fill for fillData which may not be consistent - // throughout the path (UniswapV3) and for a closer guesstimate at - // gas used - - assert.assert(routeSamples.length >= 1, 'Found no sample to use for source'); - for (let k = routeSamples.length - 1; k >= 0; k--) { - if (k === 0) { - fill = createFill(routeSamples[0]) ?? fill; - } - if (rustInputAdjusted.isGreaterThan(routeSamples[k].input)) { - const left = routeSamples[k]; - const right = routeSamples[k + 1]; - if (left && right) { - fill = - createFill({ - ...right, // default to the greater (for gas used) - input: rustInputAdjusted, - output: new BigNumber(outputAmount), - }) ?? fill; - } else { - assert.assert(Boolean(left || right), 'No valid sample to use'); - fill = createFill(left || right) ?? fill; - } - break; - } - } - - // TODO(kimpers): remove once we have solved the rounding/precision loss issues in the Rust router - const maxSampledOutput = BigNumber.max(...routeSamples.map(s => s.output)); - // Scale output by scale factor but never go above the largest sample in sell quotes (unknown liquidity) or below 1 base unit (unfillable) - const scaleOutput = (output: BigNumber) => { - // Don't try to scale 0 output as it will be clamped to 1 - if (output.eq(ZERO_AMOUNT)) { - return output; - } - - const scaled = output - .times(scale) - .decimalPlaces(0, side === MarketOperation.Sell ? BigNumber.ROUND_FLOOR : BigNumber.ROUND_CEIL); - const capped = MarketOperation.Sell ? BigNumber.min(scaled, maxSampledOutput) : scaled; - - return BigNumber.max(capped, 1); - }; - - adjustedFills.push({ - ...fill, - input: rustInputAdjusted, - output: scaleOutput(fill.output), - adjustedOutput: scaleOutput(fill.adjustedOutput), - index: 0, - parent: undefined, - sourcePathId: sourcePathId ?? hexUtils.random(), - }); - } - - if (adjustedFills.length === 0) { - return undefined; - } - - const pathFromRustInputs = Path.create(side, adjustedFills, input, opts); - - return pathFromRustInputs; + return { + allSourcesPath, + vipSourcesPath, + }; } export function findOptimalRustPathFromSamples( @@ -349,9 +385,18 @@ export function findOptimalRustPathFromSamples( neonRouterNumSamples: number, samplerMetrics?: SamplerMetrics, ): Path | undefined { - const beforeAllTimeMs = performance.now(); - let beforeTimeMs = performance.now(); - const allSourcesPath = findRoutesAndCreateOptimalPath( + const beforeTimeMs = performance.now(); + const sendMetrics = () => { + // tslint:disable-next-line: no-unused-expression + samplerMetrics && + samplerMetrics.logRouterDetails({ + router: 'neon-router', + type: 'total', + timingMs: performance.now() - beforeTimeMs, + }); + }; + const vipSourcesSet = new Set(VIP_ERC20_BRIDGE_SOURCES_BY_CHAIN_ID[chainId]); + const paths = findRoutesAndCreateOptimalPath( side, samples, nativeOrders, @@ -359,58 +404,22 @@ export function findOptimalRustPathFromSamples( opts, fees, neonRouterNumSamples, + vipSourcesSet, ); - // tslint:disable-next-line: no-unused-expression - samplerMetrics && - samplerMetrics.logRouterDetails({ - router: 'neon-router', - type: 'all', - timingMs: performance.now() - beforeTimeMs, - }); - if (!allSourcesPath) { + + if (!paths) { + sendMetrics(); return undefined; } - const vipSources = VIP_ERC20_BRIDGE_SOURCES_BY_CHAIN_ID[chainId]; + const { allSourcesPath, vipSourcesPath } = paths; - // HACK(kimpers): The Rust router currently doesn't account for VIP sources correctly - // we need to try to route them in isolation and compare with the results all sources - if (vipSources.length > 0) { - beforeTimeMs = performance.now(); - const vipSourcesSet = new Set(vipSources); - const vipSourcesSamples = samples.filter(s => s[0] && vipSourcesSet.has(s[0].source)); - - if (vipSourcesSamples.length > 0) { - const vipSourcesPath = findRoutesAndCreateOptimalPath( - side, - vipSourcesSamples, - [], - input, - opts, - fees, - neonRouterNumSamples, - ); - // tslint:disable-next-line: no-unused-expression - samplerMetrics && - samplerMetrics.logRouterDetails({ - router: 'neon-router', - type: 'vip', - timingMs: performance.now() - beforeTimeMs, - }); - - if (vipSourcesPath?.isBetterThan(allSourcesPath)) { - return vipSourcesPath; - } - } + if (!allSourcesPath || vipSourcesPath?.isBetterThan(allSourcesPath)) { + sendMetrics(); + return vipSourcesPath; } - // tslint:disable-next-line: no-unused-expression - samplerMetrics && - samplerMetrics.logRouterDetails({ - router: 'neon-router', - type: 'total', - timingMs: performance.now() - beforeAllTimeMs, - }); + sendMetrics(); return allSourcesPath; } diff --git a/yarn.lock b/yarn.lock index 8f829b8780..563d9571e1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -952,9 +952,10 @@ typedoc "~0.16.11" yargs "^10.0.3" -"@0x/neon-router@^0.3.3": - version "0.3.3" - resolved "https://registry.yarnpkg.com/@0x/neon-router/-/neon-router-0.3.3.tgz#dab540f4cd2aea6441ba29cbc35c28ca3f7a2b4f" +"@0x/neon-router@^0.3.5": + version "0.3.5" + resolved "https://registry.yarnpkg.com/@0x/neon-router/-/neon-router-0.3.5.tgz#895e7a2dc65d492a413daaea283cbc0ca6df83fa" + integrity sha512-8wizP3smc5o4jVg1smZzCCFo4ohOrgDhO4JFjF+/oNHbFImlGHOvmH9HQ2FJXAXiLEOTxrbp3T5XxP5GNATq3w== dependencies: "@mapbox/node-pre-gyp" "^1.0.5"