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

@@ -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
             }
         ]
     },

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",

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

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<ERC20BridgeSource>,
+): { 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,146 +95,23 @@ function findRoutesAndCreateOptimalPath(
         return fills[0];
     };
 
-    const samplesAndNativeOrdersWithResults: Array<DexSample[] | NativeOrderWithFillableAmounts[]> = [];
+    const createPathFromStrategy = (sourcesRustRoute: Float64Array, sourcesOutputAmounts: Float64Array) => {
-    const serializedPaths: SerializedPath[] = [];
-    const sampleSourcePathIds: string[] = [];
-    for (const singleSourceSamples of samples) {
-        if (singleSourceSamples.length === 0) {
-            continue;
-        }
-
-        const sourcePathId = hexUtils.random();
-        const singleSourceSamplesWithOutput = [...singleSourceSamples];
-        for (let i = singleSourceSamples.length - 1; i >= 0; i--) {
-            if (singleSourceSamples[i].output.isZero()) {
-                // Remove trailing 0 output samples
-                singleSourceSamplesWithOutput.pop();
-            } else {
-                break;
-            }
-        }
-
-        if (singleSourceSamplesWithOutput.length < MIN_NUM_SAMPLE_INPUTS) {
-            continue;
-        }
-
-        // TODO(kimpers): Do we need to handle 0 entries, from eg Kyber?
-        const serializedPath = singleSourceSamplesWithOutput.reduce<SerializedPath>(
-            (memo, sample, 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(
-                    calculateOuputFee(side, sample, opts.outputAmountPerEth, opts.inputAmountPerEth, fees)
-                        .integerValue()
-                        .toNumber(),
-                );
-
-                return memo;
-            },
-            {
-                ids: [],
-                inputs: [],
-                outputs: [],
-                outputFees: [],
-            },
-        );
-
-        samplesAndNativeOrdersWithResults.push(singleSourceSamplesWithOutput);
-        serializedPaths.push(serializedPath);
-        sampleSourcePathIds.push(sourcePathId);
-    }
-
-    const nativeOrdersourcePathId = hexUtils.random();
-    for (const [idx, nativeOrder] of nativeOrders.entries()) {
-        const { input: normalizedOrderInput, output: normalizedOrderOutput } = nativeOrderToNormalizedAmounts(
-            side,
-            nativeOrder,
-        );
-        // NOTE: skip dummy order created in swap_quoter
-        // TODO: remove dummy order and this logic once we don't need the JS router
-        if (normalizedOrderInput.isLessThanOrEqualTo(0) || normalizedOrderOutput.isLessThanOrEqualTo(0)) {
-            continue;
-        }
-        const fee = calculateOuputFee(side, nativeOrder, opts.outputAmountPerEth, opts.inputAmountPerEth, fees)
-            .integerValue()
-            .toNumber();
-
-        // HACK: due to an issue with the Rust router interpolation we need to create exactly 13 samples from the native order
-        const ids = [];
-        const inputs = [];
-        const outputs = [];
-        const outputFees = [];
-
-        // NOTE: Limit orders can be both larger or smaller than the input amount
-        // If the order is larger than the input we can scale the order to the size of
-        // the quote input (order pricing is constant) and then create 13 "samples" up to
-        // and including the full quote input amount.
-        // If the order is smaller we don't need to scale anything, we will just end up
-        // with trailing duplicate samples for the order input as we cannot go higher
-        const scaleToInput = BigNumber.min(input.dividedBy(normalizedOrderInput), 1);
-        for (let i = 1; i <= 13; i++) {
-            const fraction = i / 13;
-            const currentInput = BigNumber.min(
-                normalizedOrderInput.times(scaleToInput).times(fraction),
-                normalizedOrderInput,
-            );
-            const currentOutput = BigNumber.min(
-                normalizedOrderOutput.times(scaleToInput).times(fraction),
-                normalizedOrderOutput,
-            );
-            const id = `${ERC20BridgeSource.Native}-${serializedPaths.length}-${idx}-${i}`;
-            inputs.push(currentInput.integerValue().toNumber());
-            outputs.push(currentOutput.integerValue().toNumber());
-            outputFees.push(fee);
-            ids.push(id);
-        }
-
-        const serializedPath: SerializedPath = {
-            ids,
-            inputs,
-            outputs,
-            outputFees,
-        };
-
-        samplesAndNativeOrdersWithResults.push([nativeOrder]);
-        serializedPaths.push(serializedPath);
-        sampleSourcePathIds.push(nativeOrdersourcePathId);
-    }
-
-    if (serializedPaths.length === 0) {
-        return undefined;
-    }
-
-    const rustArgs: OptimizerCapture = {
-        side,
-        targetInput: input.toNumber(),
-        pathsIn: serializedPaths,
-    };
-
-    const allSourcesRustRoute = new Float64Array(rustArgs.pathsIn.length);
-    const strategySourcesOutputAmounts = new Float64Array(rustArgs.pathsIn.length);
-    route(rustArgs, allSourcesRustRoute, strategySourcesOutputAmounts, 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,
-        'different number of sources in the Router output amounts results than the input',
-    );
-
         const routesAndSamplesAndOutputs = _.zip(
-        allSourcesRustRoute,
+            sourcesRustRoute,
             samplesAndNativeOrdersWithResults,
-        strategySourcesOutputAmounts,
+            sourcesOutputAmounts,
             sampleSourcePathIds,
         );
         const adjustedFills: Fill[] = [];
-    const totalRoutedAmount = BigNumber.sum(...allSourcesRustRoute);
+        const totalRoutedAmount = BigNumber.sum(...sourcesRustRoute);
 
         const scale = input.dividedBy(totalRoutedAmount);
-    for (const [routeInput, routeSamplesAndNativeOrders, outputAmount, sourcePathId] of routesAndSamplesAndOutputs) {
+        for (const [
+            routeInput,
+            routeSamplesAndNativeOrders,
+            outputAmount,
+            sourcePathId,
+        ] of routesAndSamplesAndOutputs) {
             if (!Number.isFinite(outputAmount)) {
                 DEFAULT_WARNING_LOGGER(rustArgs, `neon-router: invalid route outputAmount ${outputAmount}`);
                 return undefined;
@@ -336,6 +214,164 @@ function findRoutesAndCreateOptimalPath(
         const pathFromRustInputs = Path.create(side, adjustedFills, input, opts);
 
         return pathFromRustInputs;
+    };
+
+    const samplesAndNativeOrdersWithResults: Array<DexSample[] | NativeOrderWithFillableAmounts[]> = [];
+    const serializedPaths: SerializedPath[] = [];
+    const sampleSourcePathIds: string[] = [];
+    for (const singleSourceSamples of samples) {
+        if (singleSourceSamples.length === 0) {
+            continue;
+        }
+
+        const sourcePathId = hexUtils.random();
+        const singleSourceSamplesWithOutput = [...singleSourceSamples];
+        for (let i = singleSourceSamples.length - 1; i >= 0; i--) {
+            if (singleSourceSamples[i].output.isZero()) {
+                // Remove trailing 0 output samples
+                singleSourceSamplesWithOutput.pop();
+            } else {
+                break;
+            }
+        }
+
+        if (singleSourceSamplesWithOutput.length < MIN_NUM_SAMPLE_INPUTS) {
+            continue;
+        }
+
+        // TODO(kimpers): Do we need to handle 0 entries, from eg Kyber?
+        const serializedPath = singleSourceSamplesWithOutput.reduce<SerializedPath>(
+            (memo, sample, 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(
+                    calculateOuputFee(side, sample, opts.outputAmountPerEth, opts.inputAmountPerEth, fees)
+                        .integerValue()
+                        .toNumber(),
+                );
+
+                return memo;
+            },
+            {
+                ids: [],
+                inputs: [],
+                outputs: [],
+                outputFees: [],
+                isVip: vipSourcesSet.has(singleSourceSamplesWithOutput[0]?.source),
+            },
+        );
+
+        samplesAndNativeOrdersWithResults.push(singleSourceSamplesWithOutput);
+        serializedPaths.push(serializedPath);
+        sampleSourcePathIds.push(sourcePathId);
+    }
+
+    const nativeOrdersourcePathId = hexUtils.random();
+    for (const [idx, nativeOrder] of nativeOrders.entries()) {
+        const { input: normalizedOrderInput, output: normalizedOrderOutput } = nativeOrderToNormalizedAmounts(
+            side,
+            nativeOrder,
+        );
+        // NOTE: skip dummy order created in swap_quoter
+        // TODO: remove dummy order and this logic once we don't need the JS router
+        if (normalizedOrderInput.isLessThanOrEqualTo(0) || normalizedOrderOutput.isLessThanOrEqualTo(0)) {
+            continue;
+        }
+        const fee = calculateOuputFee(side, nativeOrder, opts.outputAmountPerEth, opts.inputAmountPerEth, fees)
+            .integerValue()
+            .toNumber();
+
+        // HACK: due to an issue with the Rust router interpolation we need to create exactly 13 samples from the native order
+        const ids = [];
+        const inputs = [];
+        const outputs = [];
+        const outputFees = [];
+
+        // NOTE: Limit orders can be both larger or smaller than the input amount
+        // If the order is larger than the input we can scale the order to the size of
+        // the quote input (order pricing is constant) and then create 13 "samples" up to
+        // and including the full quote input amount.
+        // If the order is smaller we don't need to scale anything, we will just end up
+        // with trailing duplicate samples for the order input as we cannot go higher
+        const scaleToInput = BigNumber.min(input.dividedBy(normalizedOrderInput), 1);
+        for (let i = 1; i <= 13; i++) {
+            const fraction = i / 13;
+            const currentInput = BigNumber.min(
+                normalizedOrderInput.times(scaleToInput).times(fraction),
+                normalizedOrderInput,
+            );
+            const currentOutput = BigNumber.min(
+                normalizedOrderOutput.times(scaleToInput).times(fraction),
+                normalizedOrderOutput,
+            );
+            const id = `${ERC20BridgeSource.Native}-${serializedPaths.length}-${idx}-${i}`;
+            inputs.push(currentInput.integerValue().toNumber());
+            outputs.push(currentOutput.integerValue().toNumber());
+            outputFees.push(fee);
+            ids.push(id);
+        }
+
+        const serializedPath: SerializedPath = {
+            ids,
+            inputs,
+            outputs,
+            outputFees,
+            isVip: true,
+        };
+
+        samplesAndNativeOrdersWithResults.push([nativeOrder]);
+        serializedPaths.push(serializedPath);
+        sampleSourcePathIds.push(nativeOrdersourcePathId);
+    }
+
+    if (serializedPaths.length === 0) {
+        return undefined;
+    }
+
+    const rustArgs: OptimizerCapture = {
+        side,
+        targetInput: input.toNumber(),
+        pathsIn: serializedPaths,
+    };
+
+    const allSourcesRustRoute = new Float64Array(rustArgs.pathsIn.length);
+    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 === 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 allSourcesPath = createPathFromStrategy(allSourcesRustRoute, allSourcesOutputAmounts);
+    const vipSourcesPath = createPathFromStrategy(vipSourcesRustRoute, vipSourcesOutputAmounts);
+
+    return {
+        allSourcesPath,
+        vipSourcesPath,
+    };
 }
 
 export function findOptimalRustPathFromSamples(
@@ -349,9 +385,18 @@ export function findOptimalRustPathFromSamples(
     neonRouterNumSamples: number,
     samplerMetrics?: SamplerMetrics,
 ): Path | undefined {
-    const beforeAllTimeMs = performance.now();
+    const beforeTimeMs = performance.now();
-    let beforeTimeMs = performance.now();
+    const sendMetrics = () => {
-    const allSourcesPath = findRoutesAndCreateOptimalPath(
+        // 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 &&
+    if (!paths) {
-        samplerMetrics.logRouterDetails({
+        sendMetrics();
-            router: 'neon-router',
-            type: 'all',
-            timingMs: performance.now() - beforeTimeMs,
-        });
-    if (!allSourcesPath) {
         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
+    if (!allSourcesPath || vipSourcesPath?.isBetterThan(allSourcesPath)) {
-    // we need to try to route them in isolation and compare with the results all sources
+        sendMetrics();
-    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;
     }
-        }
-    }
-    // tslint:disable-next-line: no-unused-expression
-    samplerMetrics &&
-        samplerMetrics.logRouterDetails({
-            router: 'neon-router',
-            type: 'total',
-            timingMs: performance.now() - beforeAllTimeMs,
-        });
 
+    sendMetrics();
     return allSourcesPath;
 }
 

yarn.lock

@@ -952,9 +952,10 @@
     typedoc "~0.16.11"
     yargs "^10.0.3"
 
-"@0x/neon-router@^0.3.3":
+"@0x/neon-router@^0.3.5":
-  version "0.3.3"
+  version "0.3.5"
-  resolved "https://registry.yarnpkg.com/@0x/neon-router/-/neon-router-0.3.3.tgz#dab540f4cd2aea6441ba29cbc35c28ca3f7a2b4f"
+  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"