Fix/routing glue optimization (#439)

* fix: only create fills if using js router, remove `unoptimizedPath`

* minor optimizations

* remove the cap of route output in buys

* change PR number

* fix: Update Ropsten UniswapV3 Router address (#441)

* fix: Update Ropsten UniswapV3 Router address

* Update CHANGELOG

* Updated CHANGELOGS & MD docs

* Publish

 - @0x/asset-swapper@16.50.2

* change PR number

Co-authored-by: Kim Persson <kimpersson88@gmail.com>
Co-authored-by: Jacob Evans <jacob@dekz.net>
Co-authored-by: Github Actions <github-actions@github.com>

packages/asset-swapper/CHANGELOG.json

@@ -1,4 +1,13 @@
 [
+    {
+        "version": "16.50.3",
+        "changes": [
+            {
+                "note": "Routing glue optimization",
+                "pr": 439
+            }
+        ]
+    },
     {
         "version": "16.50.2",
         "changes": [

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

@@ -175,9 +175,9 @@ export function dexSamplesToFills(
         const { source, fillData } = sample;
         const input = sample.input.minus(prevSample ? prevSample.input : 0);
         const output = sample.output.minus(prevSample ? prevSample.output : 0);
-        const fee = fees[source] === undefined ? 0 : fees[source]!(sample.fillData) || 0;
         let penalty = ZERO_AMOUNT;
         if (i === 0) {
+            const fee = fees[source] === undefined ? 0 : fees[source]!(sample.fillData) || 0;
             // Only the first fill in a DEX path incurs a penalty.
             penalty = ethToOutputAmount({
                 input,

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

@@ -42,7 +42,7 @@ import { createFills } from './fills';
 import { getBestTwoHopQuote } from './multihop_utils';
 import { createOrdersFromTwoHopSample } from './orders';
 import { Path, PathPenaltyOpts } from './path';
-import { fillsToSortedPaths, findOptimalPathJSAsync, findOptimalRustPathFromSamples } from './path_optimizer';
+import { findOptimalPathJSAsync, findOptimalRustPathFromSamples } from './path_optimizer';
 import { DexOrderSampler, getSampleAmounts } from './sampler';
 import { SourceFilters } from './source_filters';
 import {
@@ -493,18 +493,6 @@ export class MarketOperationUtils {
                 } as NativeOrderWithFillableAmounts),
         );
 
-        // Convert native orders and dex quotes into `Fill` objects.
-        const fills = createFills({
-            side,
-            orders: [...nativeOrders, ...augmentedRfqtIndicativeQuotes],
-            dexQuotes,
-            targetInput: inputAmount,
-            outputAmountPerEth,
-            inputAmountPerEth,
-            excludedSources: opts.excludedSources,
-            feeSchedule: opts.feeSchedule,
-        });
-
         // Find the optimal path.
         const penaltyOpts: PathPenaltyOpts = {
             outputAmountPerEth,
@@ -517,13 +505,11 @@ export class MarketOperationUtils {
         const takerAmountPerEth = side === MarketOperation.Sell ? inputAmountPerEth : outputAmountPerEth;
         const makerAmountPerEth = side === MarketOperation.Sell ? outputAmountPerEth : inputAmountPerEth;
 
-        // Find the unoptimized best rate to calculate savings from optimizer
+        let fills: Fill[][];
-        const _unoptimizedPath = fillsToSortedPaths(fills, side, inputAmount, penaltyOpts)[0];
-        const unoptimizedPath = _unoptimizedPath ? _unoptimizedPath.collapse(orderOpts) : undefined;
-
         // Find the optimal path using Rust router if enabled, otherwise fallback to JS Router
         let optimalPath: Path | undefined;
         if (SHOULD_USE_RUST_ROUTER) {
+            fills = [[]];
             optimalPath = findOptimalRustPathFromSamples(
                 side,
                 dexQuotes,
@@ -536,6 +522,18 @@ export class MarketOperationUtils {
                 opts.samplerMetrics,
             );
         } else {
+            // Convert native orders and dex quotes into `Fill` objects.
+            fills = createFills({
+                side,
+                orders: [...nativeOrders, ...augmentedRfqtIndicativeQuotes],
+                dexQuotes,
+                targetInput: inputAmount,
+                outputAmountPerEth,
+                inputAmountPerEth,
+                excludedSources: opts.excludedSources,
+                feeSchedule: opts.feeSchedule,
+            });
+
             optimalPath = await findOptimalPathJSAsync(
                 side,
                 fills,
@@ -561,7 +559,6 @@ export class MarketOperationUtils {
                 sourceFlags: SOURCE_FLAGS[ERC20BridgeSource.MultiHop],
                 marketSideLiquidity,
                 adjustedRate: bestTwoHopRate,
-                unoptimizedPath,
                 takerAmountPerEth,
                 makerAmountPerEth,
             };
@@ -582,7 +579,6 @@ export class MarketOperationUtils {
             sourceFlags: collapsedPath.sourceFlags,
             marketSideLiquidity,
             adjustedRate: optimalPathRate,
-            unoptimizedPath,
             takerAmountPerEth,
             makerAmountPerEth,
         };

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

@@ -122,17 +122,8 @@ export class Path {
                 ++i;
                 continue;
             }
-            // If there are contiguous bridge orders, we can batch them together.
-            // TODO jacob pretty sure this is from DFB and we can remove
-            const contiguousBridgeFills = [collapsedFills[i]];
-            for (let j = i + 1; j < collapsedFills.length; ++j) {
-                if (collapsedFills[j].source === ERC20BridgeSource.Native) {
-                    break;
-                }
-                contiguousBridgeFills.push(collapsedFills[j]);
-            }
 
-            this.orders.push(createBridgeOrder(contiguousBridgeFills[0], makerToken, takerToken, opts.side));
+            this.orders.push(createBridgeOrder(collapsedFills[i], makerToken, takerToken, opts.side));
             i += 1;
         }
         return this as CollapsedPath;

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

@@ -303,7 +303,7 @@ function findRoutesAndCreateOptimalPath(
 
         // 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 (unknown liquidity) or below 1 base unit (unfillable)
+        // 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)) {
@@ -313,8 +313,9 @@ function findRoutesAndCreateOptimalPath(
             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(BigNumber.min(scaled, maxSampledOutput), 1);
+            return BigNumber.max(capped, 1);
         };
 
         adjustedFills.push({

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

@@ -10,7 +10,6 @@ import { NativeOrderWithFillableAmounts, RfqFirmQuoteValidator, RfqRequestOpts }
 import { QuoteRequestor, V4RFQIndicativeQuoteMM } from '../../utils/quote_requestor';
 import { ExtendedQuoteReportSources, PriceComparisonsReport, QuoteReport } from '../quote_report_generator';
 
-import { CollapsedPath } from './path';
 import { SourceFilters } from './source_filters';
 
 /**
@@ -579,7 +578,6 @@ export interface OptimizerResult {
     liquidityDelivered: CollapsedFill[] | DexSample<MultiHopFillData>;
     marketSideLiquidity: MarketSideLiquidity;
     adjustedRate: BigNumber;
-    unoptimizedPath?: CollapsedPath;
     takerAmountPerEth: BigNumber;
     makerAmountPerEth: BigNumber;
 }