diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index 8c9b3d45d7..10fc717e7a 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -25,6 +25,10 @@ { "note": "Adds `getBidAskLiquidityForMakerTakerAssetPairAsync` to return more detailed sample information", "pr": 2641 + }, + { + "note": "Fix regression where a split on the same source was collapsed into a single fill", + "pr": 2654 } ] }, diff --git a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts index eb7b0aacff..05e9da0d72 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/fills.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/fills.ts @@ -247,7 +247,7 @@ export function collapsePath(path: Fill[]): CollapsedFill[] { if (collapsed.length !== 0 && source !== ERC20BridgeSource.Native) { const prevFill = collapsed[collapsed.length - 1]; // If the last fill is from the same source, merge them. - if (prevFill.source === source) { + if (prevFill.sourcePathId === fill.sourcePathId) { prevFill.input = prevFill.input.plus(fill.input); prevFill.output = prevFill.output.plus(fill.output); prevFill.subFills.push(fill); @@ -255,6 +255,7 @@ export function collapsePath(path: Fill[]): CollapsedFill[] { } } collapsed.push({ + sourcePathId: fill.sourcePathId, source: fill.source, fillData: fill.fillData, input: fill.input, 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 a2b27ee401..f36f27f0ec 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -151,6 +151,10 @@ export interface Fill { * Represents continguous fills on a path that have been merged together. */ export interface CollapsedFill { + // Unique ID of the original source path this fill belongs to. + // This is generated when the path is generated and is useful to distinguish + // paths that have the same `source` IDs but are distinct (e.g., Curves). + sourcePathId: string; /** * The source DEX. */ diff --git a/packages/asset-swapper/test/quote_report_generator_test.ts b/packages/asset-swapper/test/quote_report_generator_test.ts index a4a1a6783f..e90b19eaf5 100644 --- a/packages/asset-swapper/test/quote_report_generator_test.ts +++ b/packages/asset-swapper/test/quote_report_generator_test.ts @@ -1,7 +1,7 @@ // tslint:disable:custom-no-magic-numbers import { orderHashUtils } from '@0x/order-utils'; import { SignedOrder } from '@0x/types'; -import { BigNumber } from '@0x/utils'; +import { BigNumber, hexUtils } from '@0x/utils'; import * as chai from 'chai'; import * as _ from 'lodash'; import 'mocha'; @@ -31,6 +31,7 @@ const expect = chai.expect; const collapsedFillFromNativeOrder = (order: SignedOrder): NativeCollapsedFill => { return { + sourcePathId: hexUtils.random(), source: ERC20BridgeSource.Native, input: order.takerAssetAmount, output: order.makerAssetAmount, @@ -102,8 +103,8 @@ describe('QuoteReportGenerator', async () => { ]; // generate path - const uniswap2Fill: CollapsedFill = { ...uniswapSample2, subFills: [] }; - const kyber2Fill: CollapsedFill = { ...kyberSample2, subFills: [] }; + const uniswap2Fill: CollapsedFill = { ...uniswapSample2, subFills: [], sourcePathId: hexUtils.random() }; + const kyber2Fill: CollapsedFill = { ...kyberSample2, subFills: [], sourcePathId: hexUtils.random() }; const orderbookOrder2Fill: CollapsedFill = collapsedFillFromNativeOrder(orderbookOrder2); const rfqtOrder2Fill: CollapsedFill = collapsedFillFromNativeOrder(rfqtOrder2); const pathGenerated: CollapsedFill[] = [rfqtOrder2Fill, orderbookOrder2Fill, uniswap2Fill, kyber2Fill]; @@ -274,8 +275,8 @@ describe('QuoteReportGenerator', async () => { // generate path const orderbookOrder1Fill: CollapsedFill = collapsedFillFromNativeOrder(orderbookOrder1); - const uniswap1Fill: CollapsedFill = { ...uniswapSample1, subFills: [] }; - const kyber1Fill: CollapsedFill = { ...kyberSample1, subFills: [] }; + const uniswap1Fill: CollapsedFill = { ...uniswapSample1, subFills: [], sourcePathId: hexUtils.random() }; + const kyber1Fill: CollapsedFill = { ...kyberSample1, subFills: [], sourcePathId: hexUtils.random() }; const pathGenerated: CollapsedFill[] = [orderbookOrder1Fill, uniswap1Fill, kyber1Fill]; const orderReport = new QuoteReportGenerator( diff --git a/packages/asset-swapper/test/quote_simulation_test.ts b/packages/asset-swapper/test/quote_simulation_test.ts index 8ec591e687..c934f23d79 100644 --- a/packages/asset-swapper/test/quote_simulation_test.ts +++ b/packages/asset-swapper/test/quote_simulation_test.ts @@ -1,6 +1,6 @@ import { constants, expect, getRandomInteger, randomAddress } from '@0x/contracts-test-utils'; import { assetDataUtils } from '@0x/order-utils'; -import { BigNumber } from '@0x/utils'; +import { BigNumber, hexUtils } from '@0x/utils'; import * as _ from 'lodash'; import { MarketOperation } from '../src/types'; @@ -155,7 +155,7 @@ describe('quote_simulation tests', async () => { signature: '0x', }; } - + const nativeSourcePathId = hexUtils.random(); function createOrderCollapsedFills(input: BigNumber, output: BigNumber, count: number): CollapsedFill[] { const inputs = subdivideAmount(input, count); const outputs = subdivideAmount(output, count); @@ -163,6 +163,7 @@ describe('quote_simulation tests', async () => { const subFillInputs = subdivideAmount(inputs[i], count); const subFillOutputs = subdivideAmount(outputs[i], count); return { + sourcePathId: nativeSourcePathId, source: ERC20BridgeSource.Native, input: inputs[i], output: outputs[i],