fix: asset-swapper source collapse regression (#2654)

fixes source collapse regression where a split on the same source was collapsed into a single fill.
This should be kept distinct as separate fills.
This commit is contained in:
Jacob Evans 2020-08-04 15:09:17 +10:00 committed by GitHub
parent 222fd5d822
commit 788bdba8cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 19 additions and 8 deletions

View File

@ -25,6 +25,10 @@
{ {
"note": "Adds `getBidAskLiquidityForMakerTakerAssetPairAsync` to return more detailed sample information", "note": "Adds `getBidAskLiquidityForMakerTakerAssetPairAsync` to return more detailed sample information",
"pr": 2641 "pr": 2641
},
{
"note": "Fix regression where a split on the same source was collapsed into a single fill",
"pr": 2654
} }
] ]
}, },

View File

@ -247,7 +247,7 @@ export function collapsePath(path: Fill[]): CollapsedFill[] {
if (collapsed.length !== 0 && source !== ERC20BridgeSource.Native) { if (collapsed.length !== 0 && source !== ERC20BridgeSource.Native) {
const prevFill = collapsed[collapsed.length - 1]; const prevFill = collapsed[collapsed.length - 1];
// If the last fill is from the same source, merge them. // 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.input = prevFill.input.plus(fill.input);
prevFill.output = prevFill.output.plus(fill.output); prevFill.output = prevFill.output.plus(fill.output);
prevFill.subFills.push(fill); prevFill.subFills.push(fill);
@ -255,6 +255,7 @@ export function collapsePath(path: Fill[]): CollapsedFill[] {
} }
} }
collapsed.push({ collapsed.push({
sourcePathId: fill.sourcePathId,
source: fill.source, source: fill.source,
fillData: fill.fillData, fillData: fill.fillData,
input: fill.input, input: fill.input,

View File

@ -151,6 +151,10 @@ export interface Fill<TFillData extends FillData = FillData> {
* Represents continguous fills on a path that have been merged together. * Represents continguous fills on a path that have been merged together.
*/ */
export interface CollapsedFill<TFillData extends FillData = FillData> { export interface CollapsedFill<TFillData extends FillData = FillData> {
// 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. * The source DEX.
*/ */

View File

@ -1,7 +1,7 @@
// tslint:disable:custom-no-magic-numbers // tslint:disable:custom-no-magic-numbers
import { orderHashUtils } from '@0x/order-utils'; import { orderHashUtils } from '@0x/order-utils';
import { SignedOrder } from '@0x/types'; import { SignedOrder } from '@0x/types';
import { BigNumber } from '@0x/utils'; import { BigNumber, hexUtils } from '@0x/utils';
import * as chai from 'chai'; import * as chai from 'chai';
import * as _ from 'lodash'; import * as _ from 'lodash';
import 'mocha'; import 'mocha';
@ -31,6 +31,7 @@ const expect = chai.expect;
const collapsedFillFromNativeOrder = (order: SignedOrder): NativeCollapsedFill => { const collapsedFillFromNativeOrder = (order: SignedOrder): NativeCollapsedFill => {
return { return {
sourcePathId: hexUtils.random(),
source: ERC20BridgeSource.Native, source: ERC20BridgeSource.Native,
input: order.takerAssetAmount, input: order.takerAssetAmount,
output: order.makerAssetAmount, output: order.makerAssetAmount,
@ -102,8 +103,8 @@ describe('QuoteReportGenerator', async () => {
]; ];
// generate path // generate path
const uniswap2Fill: CollapsedFill = { ...uniswapSample2, subFills: [] }; const uniswap2Fill: CollapsedFill = { ...uniswapSample2, subFills: [], sourcePathId: hexUtils.random() };
const kyber2Fill: CollapsedFill = { ...kyberSample2, subFills: [] }; const kyber2Fill: CollapsedFill = { ...kyberSample2, subFills: [], sourcePathId: hexUtils.random() };
const orderbookOrder2Fill: CollapsedFill = collapsedFillFromNativeOrder(orderbookOrder2); const orderbookOrder2Fill: CollapsedFill = collapsedFillFromNativeOrder(orderbookOrder2);
const rfqtOrder2Fill: CollapsedFill = collapsedFillFromNativeOrder(rfqtOrder2); const rfqtOrder2Fill: CollapsedFill = collapsedFillFromNativeOrder(rfqtOrder2);
const pathGenerated: CollapsedFill[] = [rfqtOrder2Fill, orderbookOrder2Fill, uniswap2Fill, kyber2Fill]; const pathGenerated: CollapsedFill[] = [rfqtOrder2Fill, orderbookOrder2Fill, uniswap2Fill, kyber2Fill];
@ -274,8 +275,8 @@ describe('QuoteReportGenerator', async () => {
// generate path // generate path
const orderbookOrder1Fill: CollapsedFill = collapsedFillFromNativeOrder(orderbookOrder1); const orderbookOrder1Fill: CollapsedFill = collapsedFillFromNativeOrder(orderbookOrder1);
const uniswap1Fill: CollapsedFill = { ...uniswapSample1, subFills: [] }; const uniswap1Fill: CollapsedFill = { ...uniswapSample1, subFills: [], sourcePathId: hexUtils.random() };
const kyber1Fill: CollapsedFill = { ...kyberSample1, subFills: [] }; const kyber1Fill: CollapsedFill = { ...kyberSample1, subFills: [], sourcePathId: hexUtils.random() };
const pathGenerated: CollapsedFill[] = [orderbookOrder1Fill, uniswap1Fill, kyber1Fill]; const pathGenerated: CollapsedFill[] = [orderbookOrder1Fill, uniswap1Fill, kyber1Fill];
const orderReport = new QuoteReportGenerator( const orderReport = new QuoteReportGenerator(

View File

@ -1,6 +1,6 @@
import { constants, expect, getRandomInteger, randomAddress } from '@0x/contracts-test-utils'; import { constants, expect, getRandomInteger, randomAddress } from '@0x/contracts-test-utils';
import { assetDataUtils } from '@0x/order-utils'; import { assetDataUtils } from '@0x/order-utils';
import { BigNumber } from '@0x/utils'; import { BigNumber, hexUtils } from '@0x/utils';
import * as _ from 'lodash'; import * as _ from 'lodash';
import { MarketOperation } from '../src/types'; import { MarketOperation } from '../src/types';
@ -155,7 +155,7 @@ describe('quote_simulation tests', async () => {
signature: '0x', signature: '0x',
}; };
} }
const nativeSourcePathId = hexUtils.random();
function createOrderCollapsedFills(input: BigNumber, output: BigNumber, count: number): CollapsedFill[] { function createOrderCollapsedFills(input: BigNumber, output: BigNumber, count: number): CollapsedFill[] {
const inputs = subdivideAmount(input, count); const inputs = subdivideAmount(input, count);
const outputs = subdivideAmount(output, count); const outputs = subdivideAmount(output, count);
@ -163,6 +163,7 @@ describe('quote_simulation tests', async () => {
const subFillInputs = subdivideAmount(inputs[i], count); const subFillInputs = subdivideAmount(inputs[i], count);
const subFillOutputs = subdivideAmount(outputs[i], count); const subFillOutputs = subdivideAmount(outputs[i], count);
return { return {
sourcePathId: nativeSourcePathId,
source: ERC20BridgeSource.Native, source: ERC20BridgeSource.Native,
input: inputs[i], input: inputs[i],
output: outputs[i], output: outputs[i],