From 285f98e9e965b0f0259feaa7b8b551345bfc08e9 Mon Sep 17 00:00:00 2001 From: Kim Persson Date: Wed, 29 Dec 2021 12:08:24 +0100 Subject: [PATCH] feat: Udate neon-router and use router estimated output amount (#354) * feat: use Rust router estimated output amount when possible * fix: use strings for sample ids, and increase samples in the rust router * fix: remove unnecessary interpolation of out of range values * fix: don't recalculate sampled dist sum in a loop * fix: use 14 samples for rust router to work around interpolation issues * fix: unintentional logic change * fix: remove local dev plotting param from route fn call * feat: make neon-router number of samples configurable * chore: bump to newly published neon-router version * fix: handle insufficient liquidity at all requested sources * chore: update asset-swapper changelog --- packages/asset-swapper/CHANGELOG.json | 9 ++ packages/asset-swapper/package.json | 2 +- .../utils/market_operation_utils/constants.ts | 1 + .../src/utils/market_operation_utils/index.ts | 4 + .../market_operation_utils/path_optimizer.ts | 91 +++++++++---------- .../utils/market_operation_utils/sampler.ts | 3 +- .../src/utils/market_operation_utils/types.ts | 5 + yarn.lock | 8 +- 8 files changed, 69 insertions(+), 54 deletions(-) diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index 863151ee95..70be5bf138 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "16.44.0", + "changes": [ + { + "note": "Update neon-router and use router estimated output amount", + "pr": 354 + } + ] + }, { "version": "16.43.0", "changes": [ diff --git a/packages/asset-swapper/package.json b/packages/asset-swapper/package.json index 87da6f2320..1f95ac0738 100644 --- a/packages/asset-swapper/package.json +++ b/packages/asset-swapper/package.json @@ -66,7 +66,7 @@ "@0x/contracts-zero-ex": "^0.30.1", "@0x/dev-utils": "^4.2.9", "@0x/json-schemas": "^6.3.0", - "@0x/neon-router": "^0.2.1", + "@0x/neon-router": "^0.3.1", "@0x/protocol-utils": "^1.10.1", "@0x/quote-server": "^6.0.6", "@0x/types": "^3.3.4", 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 d12ef84bc8..036f24196a 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts @@ -2103,4 +2103,5 @@ export const DEFAULT_GET_MARKET_ORDERS_OPTS: Omit p.length > 0 && !fragileSources.includes(p[0].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 7b152851f6..371be7eee7 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 @@ -17,7 +17,6 @@ import { DexSample, ERC20BridgeSource, FeeSchedule, Fill, FillData } from './typ // tslint:disable: prefer-for-of custom-no-magic-numbers completed-docs no-bitwise const RUN_LIMIT_DECAY_FACTOR = 0.5; -const RUST_ROUTER_NUM_SAMPLES = 200; const FILL_QUOTE_TRANSFORMER_GAS_OVERHEAD = new BigNumber(150e3); // NOTE: The Rust router will panic with less than 3 samples const MIN_NUM_SAMPLE_INPUTS = 3; @@ -69,21 +68,6 @@ function calculateOuputFee( } } -// Use linear interpolation to approximate the output -// at a certain input somewhere between the two samples -// See https://en.wikipedia.org/wiki/Linear_interpolation -const interpolateOutputFromSamples = ( - left: { input: BigNumber; output: BigNumber }, - right: { input: BigNumber; output: BigNumber }, - targetInput: BigNumber, -): BigNumber => - left.output.plus( - right.output - .minus(left.output) - .dividedBy(right.input.minus(left.input)) - .times(targetInput.minus(left.input)), - ); - function findRoutesAndCreateOptimalPath( side: MarketOperation, samples: DexSample[][], @@ -91,21 +75,10 @@ function findRoutesAndCreateOptimalPath( input: BigNumber, opts: PathPenaltyOpts, fees: FeeSchedule, + neonRouterNumSamples: number, ): Path | undefined { const createFill = (sample: DexSample) => dexSamplesToFills(side, [sample], opts.outputAmountPerEth, opts.inputAmountPerEth, fees)[0]; - // Track sample id's to integers (required by rust router) - const sampleIdLookup: { [key: string]: number } = {}; - let sampleIdCounter = 0; - const sampleToId = (source: ERC20BridgeSource, index: number): number => { - const key = `${source}-${index}`; - if (sampleIdLookup[key]) { - return sampleIdLookup[key]; - } else { - sampleIdLookup[key] = ++sampleIdCounter; - return sampleIdLookup[key]; - } - }; const samplesAndNativeOrdersWithResults: Array = []; const serializedPaths: SerializedPath[] = []; @@ -131,7 +104,7 @@ function findRoutesAndCreateOptimalPath( // TODO(kimpers): Do we need to handle 0 entries, from eg Kyber? const serializedPath = singleSourceSamplesWithOutput.reduce( (memo, sample, sampleIdx) => { - memo.ids.push(sampleToId(sample.source, sampleIdx)); + memo.ids.push(`${sample.source}-${serializedPaths.length}-${sampleIdx}`); memo.inputs.push(sample.input.integerValue().toNumber()); memo.outputs.push(sample.output.integerValue().toNumber()); memo.outputFees.push( @@ -188,7 +161,7 @@ function findRoutesAndCreateOptimalPath( .toNumber(); const outputFees = [fee, fee, fee]; // NOTE: ids can be the same for all fake samples - const id = sampleToId(ERC20BridgeSource.Native, idx); + const id = `${ERC20BridgeSource.Native}-${serializedPaths.length}-${idx}`; const ids = [id, id, id]; const serializedPath: SerializedPath = { @@ -214,7 +187,9 @@ function findRoutesAndCreateOptimalPath( const before = performance.now(); const allSourcesRustRoute = new Float64Array(rustArgs.pathsIn.length); - route(rustArgs, allSourcesRustRoute, RUST_ROUTER_NUM_SAMPLES); + const strategySourcesOutputAmounts = new Float64Array(rustArgs.pathsIn.length); + + route(rustArgs, allSourcesRustRoute, strategySourcesOutputAmounts, neonRouterNumSamples); DEFAULT_INFO_LOGGER( { router: 'neon-router', performanceMs: performance.now() - before, type: 'real' }, 'Rust router real routing performance', @@ -224,18 +199,25 @@ function findRoutesAndCreateOptimalPath( rustArgs.pathsIn.length === allSourcesRustRoute.length, 'different number of sources in the Router output than the input', ); + assert.assert( + rustArgs.pathsIn.length === strategySourcesOutputAmounts.length, + 'different number of sources in the Router output amounts results than the input', + ); - const routesAndSamples = _.zip(allSourcesRustRoute, samplesAndNativeOrdersWithResults); - + const routesAndSamplesAndOutputs = _.zip( + allSourcesRustRoute, + samplesAndNativeOrdersWithResults, + strategySourcesOutputAmounts, + ); const adjustedFills: Fill[] = []; const totalRoutedAmount = BigNumber.sum(...allSourcesRustRoute); const scale = input.dividedBy(totalRoutedAmount); - for (const [routeInput, routeSamplesAndNativeOrders] of routesAndSamples) { - if (!routeInput || !routeSamplesAndNativeOrders) { + for (const [routeInput, routeSamplesAndNativeOrders, outputAmount] of routesAndSamplesAndOutputs) { + if (!routeInput || !routeSamplesAndNativeOrders || !outputAmount || !Number.isFinite(outputAmount)) { continue; } - // TODO(kimpers): [TKR-241] amounts are sometimes clipped in the router due to precisions loss for number/f64 + // 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), @@ -270,22 +252,13 @@ function findRoutesAndCreateOptimalPath( fill = createFill(routeSamples[0]); } if (rustInputAdjusted.isGreaterThan(routeSamples[k].input)) { - // Between here and the previous fill - // HACK: Use the midpoint between the two const left = routeSamples[k]; const right = routeSamples[k + 1]; if (left && right) { - // Approximate how much output we get for the input with the surrounding samples - const interpolatedOutput = interpolateOutputFromSamples( - left, - right, - rustInputAdjusted, - ).decimalPlaces(0, side === MarketOperation.Sell ? BigNumber.ROUND_FLOOR : BigNumber.ROUND_CEIL); - fill = createFill({ ...right, // default to the greater (for gas used) input: rustInputAdjusted, - output: interpolatedOutput, + output: new BigNumber(outputAmount), }); } else { assert.assert(Boolean(left || right), 'No valid sample to use'); @@ -295,6 +268,7 @@ function findRoutesAndCreateOptimalPath( } } + // TODO(kimpers): remove once we have solved the rounding/precision loss issues in the Rust router const scaleOutput = (output: BigNumber) => output .dividedBy(fill.input) @@ -310,6 +284,10 @@ function findRoutesAndCreateOptimalPath( }); } + if (adjustedFills.length === 0) { + return undefined; + } + const pathFromRustInputs = Path.create(side, adjustedFills, input); return pathFromRustInputs; @@ -323,6 +301,7 @@ export function findOptimalRustPathFromSamples( opts: PathPenaltyOpts, fees: FeeSchedule, chainId: ChainId, + neonRouterNumSamples: number, ): Path | undefined { const before = performance.now(); const logPerformance = () => @@ -331,7 +310,15 @@ export function findOptimalRustPathFromSamples( 'Rust router total routing performance', ); - const allSourcesPath = findRoutesAndCreateOptimalPath(side, samples, nativeOrders, input, opts, fees); + const allSourcesPath = findRoutesAndCreateOptimalPath( + side, + samples, + nativeOrders, + input, + opts, + fees, + neonRouterNumSamples, + ); if (!allSourcesPath) { return undefined; } @@ -345,7 +332,15 @@ export function findOptimalRustPathFromSamples( const vipSourcesSamples = samples.filter(s => s[0] && vipSourcesSet.has(s[0].source)); if (vipSourcesSamples.length > 0) { - const vipSourcesPath = findRoutesAndCreateOptimalPath(side, vipSourcesSamples, [], input, opts, fees); + const vipSourcesPath = findRoutesAndCreateOptimalPath( + side, + vipSourcesSamples, + [], + input, + opts, + fees, + neonRouterNumSamples, + ); const { input: allSourcesInput, output: allSourcesOutput } = allSourcesPath.adjustedSize(); // NOTE: For sell quotes input is the taker asset and for buy quotes input is the maker asset diff --git a/packages/asset-swapper/src/utils/market_operation_utils/sampler.ts b/packages/asset-swapper/src/utils/market_operation_utils/sampler.ts index dda8fe04f7..8239c21d38 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/sampler.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/sampler.ts @@ -14,7 +14,8 @@ import { BatchedOperation, ERC20BridgeSource, LiquidityProviderRegistry, TokenAd */ export function getSampleAmounts(maxFillAmount: BigNumber, numSamples: number, expBase: number = 1): BigNumber[] { const distribution = [...Array(numSamples)].map((_v, i) => new BigNumber(expBase).pow(i)); - const stepSizes = distribution.map(d => d.div(BigNumber.sum(...distribution))); + const distributionSum = BigNumber.sum(...distribution); + const stepSizes = distribution.map(d => d.div(distributionSum)); const amounts = stepSizes.map((_s, i) => { if (i === numSamples - 1) { return maxFillAmount; 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 bed7c20c41..20cde38d41 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -455,6 +455,10 @@ export interface GetMarketOrdersOpts { * Default: 1.25. */ sampleDistributionBase: number; + /** + * Number of samples to use when creating fill curves with neon-router + */ + neonRouterNumSamples: number; /** * Fees for each liquidity source, expressed in gas. */ @@ -599,6 +603,7 @@ export interface GenerateOptimizedOrdersOpts { allowFallback?: boolean; shouldBatchBridgeOrders?: boolean; gasPrice: BigNumber; + neonRouterNumSamples: number; } export interface ComparisonPrice { diff --git a/yarn.lock b/yarn.lock index ef003db6ca..cd8b95d07d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -959,10 +959,10 @@ typedoc "~0.16.11" yargs "^10.0.3" -"@0x/neon-router@^0.2.1": - version "0.2.1" - resolved "https://registry.yarnpkg.com/@0x/neon-router/-/neon-router-0.2.1.tgz#23bb3cedc0eafd55a8ba6b6ea8a59ee4c538064b" - integrity sha512-feCCKuox4staZl8lxLY4nf5U256NcDHrgvSFra5cU/TUhoblLHb8F7eWAC9ygpukZUCVFLy13mExkFQHXlEOYw== +"@0x/neon-router@^0.3.1": + version "0.3.1" + resolved "https://registry.yarnpkg.com/@0x/neon-router/-/neon-router-0.3.1.tgz#4ec13e750d1435357c4928d7f2521a2b4376f27e" + integrity sha512-M4ypTov9KyxsGJpYwobrld3Y2JOlR7U0XjR6BEQE2gQ1k3nie/1wNEI2J4ZjKw++RLDxdv/RCqhgA5VnINzjxA== dependencies: "@mapbox/node-pre-gyp" "^1.0.5"