fix: don't create fills for 0 output samples and negative adjusted rate orders (#387)

* fix: don't try to create fills for 0 output samples

* fix: negative adjusted output native orders causing undefined fills

* fix: make sure to use the same sourcePathId for fills from same source

* fix: should be same sourcePathId within the same DexSample[]

* fix: split native orders into 13 samples to align with interpolation

* chore: add changelog entry for asset-swapper

packages/asset-swapper/CHANGELOG.json

@@ -1,4 +1,13 @@
 [
+    {
+        "version": "16.45.2",
+        "changes": [
+            {
+                "note": "Handle 0 output samples and negative adjusted rate native orders in routing",
+                "pr": 387
+            }
+        ]
+    },
     {
         "version": "16.45.1",
         "changes": [

packages/asset-swapper/src/utils/market_operation_utils/path_optimizer.ts

@@ -1,7 +1,7 @@
 import { assert } from '@0x/assert';
 import { ChainId } from '@0x/contract-addresses';
 import { OptimizerCapture, route, SerializedPath } from '@0x/neon-router';
-import { BigNumber } from '@0x/utils';
+import { BigNumber, hexUtils } from '@0x/utils';
 import * as _ from 'lodash';
 import { performance } from 'perf_hooks';
 
@@ -76,16 +76,25 @@ function findRoutesAndCreateOptimalPath(
     fees: FeeSchedule,
     neonRouterNumSamples: number,
 ): Path | undefined {
-    const createFill = (sample: DexSample) =>
+    const createFill = (sample: DexSample): Fill | undefined => {
-        dexSamplesToFills(side, [sample], opts.outputAmountPerEth, opts.inputAmountPerEth, fees)[0];
+        const fills = dexSamplesToFills(side, [sample], opts.outputAmountPerEth, opts.inputAmountPerEth, fees);
+        // NOTE: If the sample has 0 output dexSamplesToFills will return [] because no fill can be created
+        if (fills.length === 0) {
+            return undefined;
+        }
+
+        return fills[0];
+    };
 
     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()) {
@@ -124,8 +133,10 @@ function findRoutesAndCreateOptimalPath(
 
         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,
@@ -136,32 +147,25 @@ function findRoutesAndCreateOptimalPath(
         if (normalizedOrderInput.isLessThanOrEqualTo(0) || normalizedOrderOutput.isLessThanOrEqualTo(0)) {
             continue;
         }
-
-        // HACK: the router requires at minimum 3 samples as a basis for interpolation
-        const inputs = [
-            0,
-            normalizedOrderInput
-                .dividedBy(2)
-                .integerValue()
-                .toNumber(),
-            normalizedOrderInput.integerValue().toNumber(),
-        ];
-        const outputs = [
-            0,
-            normalizedOrderOutput
-                .dividedBy(2)
-                .integerValue()
-                .toNumber(),
-            normalizedOrderOutput.integerValue().toNumber(),
-        ];
-        // NOTE: same fee no matter if full or partial fill
         const fee = calculateOuputFee(side, nativeOrder, opts.outputAmountPerEth, opts.inputAmountPerEth, fees)
             .integerValue()
             .toNumber();
-        const outputFees = [fee, fee, fee];
+
-        // NOTE: ids can be the same for all fake samples
+        // HACK: due to an issue with the Rust router interpolation we need to create exactly 13 samples from the native order
-        const id = `${ERC20BridgeSource.Native}-${serializedPaths.length}-${idx}`;
+        const ids = [];
-        const ids = [id, id, id];
+        const inputs = [];
+        const outputs = [];
+        const outputFees = [];
+        for (let i = 1; i <= 13; i++) {
+            const fraction = i / 13;
+            const currentInput = BigNumber.min(normalizedOrderInput.times(fraction), normalizedOrderInput);
+            const currentOutput = BigNumber.min(normalizedOrderOutput.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,
@@ -172,6 +176,7 @@ function findRoutesAndCreateOptimalPath(
 
         samplesAndNativeOrdersWithResults.push([nativeOrder]);
         serializedPaths.push(serializedPath);
+        sampleSourcePathIds.push(nativeOrdersourcePathId);
     }
 
     if (serializedPaths.length === 0) {
@@ -200,12 +205,13 @@ function findRoutesAndCreateOptimalPath(
         allSourcesRustRoute,
         samplesAndNativeOrdersWithResults,
         strategySourcesOutputAmounts,
+        sampleSourcePathIds,
     );
     const adjustedFills: Fill[] = [];
     const totalRoutedAmount = BigNumber.sum(...allSourcesRustRoute);
 
     const scale = input.dividedBy(totalRoutedAmount);
-    for (const [routeInput, routeSamplesAndNativeOrders, outputAmount] of routesAndSamplesAndOutputs) {
+    for (const [routeInput, routeSamplesAndNativeOrders, outputAmount, sourcePathId] of routesAndSamplesAndOutputs) {
         if (!routeInput || !routeSamplesAndNativeOrders || !outputAmount || !Number.isFinite(outputAmount)) {
             continue;
         }
@@ -225,14 +231,21 @@ function findRoutesAndCreateOptimalPath(
                 opts.outputAmountPerEth,
                 opts.inputAmountPerEth,
                 fees,
-            )[0];
+            )[0] as Fill | undefined;
-            // NOTE: For Limit/RFQ orders we are done here. No need to scale output
+            // Note: If the order has an adjusted rate of less than or equal to 0 it will be skipped
-            adjustedFills.push(nativeFill);
+            // 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<DexSample<FillData>>;
         // Descend to approach a closer fill for fillData which may not be consistent
         // throughout the path (UniswapV3) and for a closer guesstimate at
@@ -241,38 +254,40 @@ function findRoutesAndCreateOptimalPath(
         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 = createFill(routeSamples[0]) ?? fill;
             }
             if (rustInputAdjusted.isGreaterThan(routeSamples[k].input)) {
                 const left = routeSamples[k];
                 const right = routeSamples[k + 1];
                 if (left && right) {
-                    fill = createFill({
+                    fill =
-                        ...right, // default to the greater (for gas used)
+                        createFill({
-                        input: rustInputAdjusted,
+                            ...right, // default to the greater (for gas used)
-                        output: new BigNumber(outputAmount),
+                            input: rustInputAdjusted,
-                    });
+                            output: new BigNumber(outputAmount),
+                        }) ?? fill;
                 } else {
                     assert.assert(Boolean(left || right), 'No valid sample to use');
-                    fill = createFill(left || right);
+                    fill = createFill(left || right) ?? fill;
                 }
                 break;
             }
         }
 
         // TODO(kimpers): remove once we have solved the rounding/precision loss issues in the Rust router
-        const scaleOutput = (output: BigNumber) =>
+        const scaleOutput = (fillInput: BigNumber, output: BigNumber) =>
             output
-                .dividedBy(fill.input)
+                .dividedBy(fillInput)
                 .times(rustInputAdjusted)
                 .decimalPlaces(0, side === MarketOperation.Sell ? BigNumber.ROUND_FLOOR : BigNumber.ROUND_CEIL);
         adjustedFills.push({
             ...fill,
             input: rustInputAdjusted,
-            output: scaleOutput(fill.output),
+            output: scaleOutput(fill.input, fill.output),
-            adjustedOutput: scaleOutput(fill.adjustedOutput),
+            adjustedOutput: scaleOutput(fill.input, fill.adjustedOutput),
             index: 0,
             parent: undefined,
+            sourcePathId: sourcePathId ?? hexUtils.random(),
         });
     }