From 09d05d09c9fc7da945d528e8ff4666b5da5c599c Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Thu, 16 Jan 2020 14:37:15 -0500 Subject: [PATCH] `@0x/asset-swapper`: Provide more accurate best quote price. --- packages/asset-swapper/CHANGELOG.json | 4 + packages/asset-swapper/src/index.ts | 7 +- .../utils/market_operation_utils/constants.ts | 4 +- .../market_operation_utils/create_order.ts | 77 ++--- .../src/utils/market_operation_utils/index.ts | 86 +++--- .../src/utils/market_operation_utils/types.ts | 42 +++ .../src/utils/swap_quote_calculator.ts | 269 ++++++++++-------- .../test/calculate_liquidity_test.ts | 6 +- .../test/swap_quote_calculator_test.ts | 124 ++++---- .../asset-swapper/test/utils/test_orders.ts | 4 +- 10 files changed, 376 insertions(+), 247 deletions(-) diff --git a/packages/asset-swapper/CHANGELOG.json b/packages/asset-swapper/CHANGELOG.json index 01adea1a91..e376a42ba7 100644 --- a/packages/asset-swapper/CHANGELOG.json +++ b/packages/asset-swapper/CHANGELOG.json @@ -17,6 +17,10 @@ { "note": "Add exponential sampling distribution and `sampleDistributionBase` option to `SwapQuoter`", "pr": 2427 + }, + { + "note": "Compute more accurate best quote price", + "pr": 2427 } ] }, diff --git a/packages/asset-swapper/src/index.ts b/packages/asset-swapper/src/index.ts index 5327764bbc..cc0746c41a 100644 --- a/packages/asset-swapper/src/index.ts +++ b/packages/asset-swapper/src/index.ts @@ -54,6 +54,11 @@ export { SwapQuoteConsumerError, SignedOrderWithFillableAmounts, } from './types'; -export { ERC20BridgeSource } from './utils/market_operation_utils/types'; +export { + ERC20BridgeSource, + CollapsedFill, + NativeCollapsedFill, + OptimizedMarketOrder, +} from './utils/market_operation_utils/types'; export { affiliateFeeUtils } from './utils/affiliate_fee_utils'; export { ProtocolFeeUtils } from './utils/protocol_fee_utils'; diff --git a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts index 03b36c73c8..965079f144 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts @@ -29,9 +29,9 @@ export const DEFAULT_GET_MARKET_ORDERS_OPTS: GetMarketOrdersOpts = { excludedSources: [], bridgeSlippage: 0.0005, dustFractionThreshold: 0.0025, - numSamples: 12, + numSamples: 13, noConflicts: true, - sampleDistributionBase: 1.25, + sampleDistributionBase: 1.05, }; export const constants = { diff --git a/packages/asset-swapper/src/utils/market_operation_utils/create_order.ts b/packages/asset-swapper/src/utils/market_operation_utils/create_order.ts index d150da6376..bf64552106 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/create_order.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/create_order.ts @@ -3,11 +3,17 @@ import { assetDataUtils, generatePseudoRandomSalt } from '@0x/order-utils'; import { AbiEncoder, BigNumber } from '@0x/utils'; import { constants } from '../../constants'; -import { SignedOrderWithFillableAmounts } from '../../types'; import { sortingUtils } from '../../utils/sorting_utils'; import { constants as marketOperationUtilConstants } from './constants'; -import { AggregationError, ERC20BridgeSource, Fill, FillData, NativeFillData, OrderDomain } from './types'; +import { + AggregationError, + CollapsedFill, + ERC20BridgeSource, + NativeCollapsedFill, + OptimizedMarketOrder, + OrderDomain, +} from './types'; const { NULL_BYTES, NULL_ADDRESS, ZERO_AMOUNT } = constants; const { INFINITE_TIMESTAMP_SEC, WALLET_SIGNATURE } = marketOperationUtilConstants; @@ -24,23 +30,21 @@ export class CreateOrderUtils { orderDomain: OrderDomain, inputToken: string, outputToken: string, - path: Fill[], + path: CollapsedFill[], bridgeSlippage: number, - ): SignedOrderWithFillableAmounts[] { - const orders: SignedOrderWithFillableAmounts[] = []; + ): OptimizedMarketOrder[] { + const orders: OptimizedMarketOrder[] = []; for (const fill of path) { - const source = (fill.fillData as FillData).source; - if (source === ERC20BridgeSource.Native) { - orders.push((fill.fillData as NativeFillData).order); + if (fill.source === ERC20BridgeSource.Native) { + orders.push(createNativeOrder(fill)); } else { orders.push( createBridgeOrder( orderDomain, - this._getBridgeAddressFromSource(source), + fill, + this._getBridgeAddressFromSource(fill.source), outputToken, inputToken, - fill.output, - fill.input, bridgeSlippage, ), ); @@ -54,23 +58,21 @@ export class CreateOrderUtils { orderDomain: OrderDomain, inputToken: string, outputToken: string, - path: Fill[], + path: CollapsedFill[], bridgeSlippage: number, - ): SignedOrderWithFillableAmounts[] { - const orders: SignedOrderWithFillableAmounts[] = []; + ): OptimizedMarketOrder[] { + const orders: OptimizedMarketOrder[] = []; for (const fill of path) { - const source = (fill.fillData as FillData).source; - if (source === ERC20BridgeSource.Native) { - orders.push((fill.fillData as NativeFillData).order); + if (fill.source === ERC20BridgeSource.Native) { + orders.push(createNativeOrder(fill)); } else { orders.push( createBridgeOrder( orderDomain, - this._getBridgeAddressFromSource(source), + fill, + this._getBridgeAddressFromSource(fill.source), inputToken, outputToken, - fill.input, - fill.output, bridgeSlippage, true, ), @@ -97,14 +99,13 @@ export class CreateOrderUtils { function createBridgeOrder( orderDomain: OrderDomain, + fill: CollapsedFill, bridgeAddress: string, makerToken: string, takerToken: string, - makerAssetAmount: BigNumber, - takerAssetAmount: BigNumber, slippage: number, isBuy: boolean = false, -): SignedOrderWithFillableAmounts { +): OptimizedMarketOrder { return { makerAddress: bridgeAddress, makerAssetData: assetDataUtils.encodeERC20BridgeAssetData( @@ -113,7 +114,7 @@ function createBridgeOrder( createBridgeData(takerToken), ), takerAssetData: assetDataUtils.encodeERC20AssetData(takerToken), - ...createCommonOrderFields(orderDomain, makerAssetAmount, takerAssetAmount, slippage, isBuy), + ...createCommonOrderFields(orderDomain, fill, slippage, isBuy), }; } @@ -123,24 +124,24 @@ function createBridgeData(tokenAddress: string): string { } type CommonOrderFields = Pick< - SignedOrderWithFillableAmounts, - Exclude + OptimizedMarketOrder, + Exclude >; function createCommonOrderFields( orderDomain: OrderDomain, - makerAssetAmount: BigNumber, - takerAssetAmount: BigNumber, + fill: CollapsedFill, slippage: number, isBuy: boolean = false, ): CommonOrderFields { const makerAssetAmountAdjustedWithSlippage = isBuy - ? makerAssetAmount - : makerAssetAmount.times(1 - slippage).integerValue(BigNumber.ROUND_DOWN); + ? fill.totalMakerAssetAmount + : fill.totalMakerAssetAmount.times(1 - slippage).integerValue(BigNumber.ROUND_DOWN); const takerAssetAmountAdjustedWithSlippage = isBuy - ? takerAssetAmount.times(slippage + 1).integerValue(BigNumber.ROUND_UP) - : takerAssetAmount; + ? fill.totalTakerAssetAmount.times(slippage + 1).integerValue(BigNumber.ROUND_UP) + : fill.totalTakerAssetAmount; return { + fill, takerAddress: NULL_ADDRESS, senderAddress: NULL_ADDRESS, feeRecipientAddress: NULL_ADDRESS, @@ -159,3 +160,15 @@ function createCommonOrderFields( ...orderDomain, }; } + +function createNativeOrder(fill: CollapsedFill): OptimizedMarketOrder { + return { + fill: { + source: fill.source, + totalMakerAssetAmount: fill.totalMakerAssetAmount, + totalTakerAssetAmount: fill.totalTakerAssetAmount, + subFills: fill.subFills, + }, + ...(fill as NativeCollapsedFill).nativeOrder, + }; +} diff --git a/packages/asset-swapper/src/utils/market_operation_utils/index.ts b/packages/asset-swapper/src/utils/market_operation_utils/index.ts index a5bbf087e5..aed438f35e 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -14,12 +14,16 @@ import { comparePathOutputs, FillsOptimizer, getPathOutput } from './fill_optimi import { DexOrderSampler } from './sampler'; import { AggregationError, + CollapsedFill, DexSample, ERC20BridgeSource, Fill, FillData, FillFlags, GetMarketOrdersOpts, + NativeCollapsedFill, + NativeFillData, + OptimizedMarketOrder, OrderDomain, } from './types'; @@ -53,7 +57,7 @@ export class MarketOperationUtils { nativeOrders: SignedOrder[], takerAmount: BigNumber, opts?: Partial, - ): Promise { + ): Promise { if (nativeOrders.length === 0) { throw new Error(AggregationError.EmptyOrders); } @@ -77,7 +81,7 @@ export class MarketOperationUtils { takerAmount, _opts.dustFractionThreshold, ); - const clippedNativePath = clipPathToInput(prunedNativePath, takerAmount); + const clippedNativePath = clipPathToInput(sortFillsByPrice(prunedNativePath), takerAmount); const dexPaths = createPathsFromDexQuotes(dexQuotes, _opts.noConflicts); const allPaths = [...dexPaths]; const allFills = flattenDexPaths(dexPaths); @@ -105,7 +109,7 @@ export class MarketOperationUtils { this._orderDomain, inputToken, outputToken, - simplifyPath(optimalPath), + collapsePath(optimalPath, false), _opts.bridgeSlippage, ); } @@ -122,7 +126,7 @@ export class MarketOperationUtils { nativeOrders: SignedOrder[], makerAmount: BigNumber, opts?: Partial, - ): Promise { + ): Promise { if (nativeOrders.length === 0) { throw new Error(AggregationError.EmptyOrders); } @@ -161,7 +165,7 @@ export class MarketOperationUtils { batchNativeOrders: SignedOrder[][], makerAmounts: BigNumber[], opts?: Partial, - ): Promise> { + ): Promise> { if (batchNativeOrders.length === 0) { throw new Error(AggregationError.EmptyOrders); } @@ -192,7 +196,7 @@ export class MarketOperationUtils { nativeOrderFillableAmounts: BigNumber[], dexQuotes: DexSample[][], opts: GetMarketOrdersOpts, - ): SignedOrderWithFillableAmounts[] | undefined { + ): OptimizedMarketOrder[] | undefined { const nativeOrdersWithFillableAmounts = createSignedOrdersWithFillableAmounts( nativeOrders, nativeOrderFillableAmounts, @@ -203,7 +207,7 @@ export class MarketOperationUtils { makerAmount, opts.dustFractionThreshold, ); - const clippedNativePath = clipPathToInput(prunedNativePath, makerAmount); + const clippedNativePath = clipPathToInput(sortFillsByPrice(prunedNativePath).reverse(), makerAmount); const dexPaths = createPathsFromDexQuotes(dexQuotes, opts.noConflicts); const allPaths = [...dexPaths]; const allFills = flattenDexPaths(dexPaths); @@ -230,7 +234,7 @@ export class MarketOperationUtils { this._orderDomain, inputToken, outputToken, - simplifyPath(optimalPath), + collapsePath(optimalPath, true), opts.bridgeSlippage, ); } @@ -242,26 +246,24 @@ function createSignedOrdersWithFillableAmounts( operation: MarketOperation, ): SignedOrderWithFillableAmounts[] { return signedOrders - .map( - (order: SignedOrder, i: number): SignedOrderWithFillableAmounts => { - const fillableAmount = fillableAmounts[i]; - const fillableMakerAssetAmount = - operation === MarketOperation.Buy - ? fillableAmount - : orderCalculationUtils.getMakerFillAmount(order, fillableAmount); - const fillableTakerAssetAmount = - operation === MarketOperation.Sell - ? fillableAmount - : orderCalculationUtils.getTakerFillAmount(order, fillableAmount); - const fillableTakerFeeAmount = orderCalculationUtils.getTakerFeeAmount(order, fillableTakerAssetAmount); - return { - fillableMakerAssetAmount, - fillableTakerAssetAmount, - fillableTakerFeeAmount, - ...order, - }; - }, - ) + .map((order: SignedOrder, i: number) => { + const fillableAmount = fillableAmounts[i]; + const fillableMakerAssetAmount = + operation === MarketOperation.Buy + ? fillableAmount + : orderCalculationUtils.getMakerFillAmount(order, fillableAmount); + const fillableTakerAssetAmount = + operation === MarketOperation.Sell + ? fillableAmount + : orderCalculationUtils.getTakerFillAmount(order, fillableAmount); + const fillableTakerFeeAmount = orderCalculationUtils.getTakerFeeAmount(order, fillableTakerAssetAmount); + return { + fillableMakerAssetAmount, + fillableTakerAssetAmount, + fillableTakerFeeAmount, + ...order, + }; + }) .filter(order => { return !order.fillableMakerAssetAmount.isZero() && !order.fillableTakerAssetAmount.isZero(); }); @@ -412,23 +414,31 @@ function getPathInput(path: Fill[]): BigNumber { } // Merges contiguous fills from the same DEX. -function simplifyPath(path: Fill[]): Fill[] { - const simplified: Fill[] = []; +function collapsePath(path: Fill[], isBuy: boolean): CollapsedFill[] { + const collapsed: Array = []; for (const fill of path) { + const makerAssetAmount = isBuy ? fill.input : fill.output; + const takerAssetAmount = isBuy ? fill.output : fill.input; const source = (fill.fillData as FillData).source; - if (simplified.length !== 0 && source !== ERC20BridgeSource.Native) { - const prevFill = simplified[simplified.length - 1]; - const prevSource = (prevFill.fillData as FillData).source; + 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 (prevSource === source) { - prevFill.input = prevFill.input.plus(fill.input); - prevFill.output = prevFill.output.plus(fill.output); + if (prevFill.source === source) { + prevFill.totalMakerAssetAmount = prevFill.totalMakerAssetAmount.plus(makerAssetAmount); + prevFill.totalTakerAssetAmount = prevFill.totalTakerAssetAmount.plus(takerAssetAmount); + prevFill.subFills.push({ makerAssetAmount, takerAssetAmount }); continue; } } - simplified.push(fill); + collapsed.push({ + source: fill.fillData.source, + totalMakerAssetAmount: makerAssetAmount, + totalTakerAssetAmount: takerAssetAmount, + subFills: [{ makerAssetAmount, takerAssetAmount }], + nativeOrder: (fill.fillData as NativeFillData).order, + }); } - return simplified; + return collapsed; } // Sort fills by descending price. 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 4ffc55f496..0629579c74 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -79,6 +79,48 @@ export interface Fill { fillData: FillData | NativeFillData; } +/** + * Represents continguous fills on a path that have been merged together. + */ +export interface CollapsedFill { + /** + * The source DEX. + */ + source: ERC20BridgeSource; + /** + * Total maker asset amount. + */ + totalMakerAssetAmount: BigNumber; + /** + * Total taker asset amount. + */ + totalTakerAssetAmount: BigNumber; + /** + * All the fill asset amounts that were collapsed into this node. + */ + subFills: Array<{ + makerAssetAmount: BigNumber; + takerAssetAmount: BigNumber; + }>; +} + +/** + * A `CollapsedFill` wrapping a native order. + */ +export interface NativeCollapsedFill extends CollapsedFill { + nativeOrder: SignedOrderWithFillableAmounts; +} + +/** + * Optimized orders to fill. + */ +export interface OptimizedMarketOrder extends SignedOrderWithFillableAmounts { + /** + * The optimized fills that generated this order. + */ + fill: CollapsedFill; +} + /** * Options for `getMarketSellOrdersAsync()` and `getMarketBuyOrdersAsync()`. */ diff --git a/packages/asset-swapper/src/utils/swap_quote_calculator.ts b/packages/asset-swapper/src/utils/swap_quote_calculator.ts index f9c4a1743e..58010bc68d 100644 --- a/packages/asset-swapper/src/utils/swap_quote_calculator.ts +++ b/packages/asset-swapper/src/utils/swap_quote_calculator.ts @@ -16,6 +16,7 @@ import { import { fillableAmountsUtils } from './fillable_amounts_utils'; import { MarketOperationUtils } from './market_operation_utils'; +import { ERC20BridgeSource, OptimizedMarketOrder } from './market_operation_utils/types'; import { ProtocolFeeUtils } from './protocol_fee_utils'; import { utils } from './utils'; @@ -79,6 +80,7 @@ export class SwapQuoteCalculator { opts, )) as Array; } + private async _calculateBatchBuySwapQuoteAsync( batchPrunedOrders: SignedOrder[][], assetFillAmounts: BigNumber[], @@ -106,7 +108,6 @@ export class SwapQuoteCalculator { operation, assetFillAmounts[i], gasPrice, - opts, ); } else { return undefined; @@ -126,7 +127,7 @@ export class SwapQuoteCalculator { // since prunedOrders do not have fillState, we will add a buffer of fillable orders to consider that some native are orders are partially filled const slippageBufferAmount = assetFillAmount.multipliedBy(slippagePercentage).integerValue(); - let resultOrders: SignedOrderWithFillableAmounts[] = []; + let resultOrders: OptimizedMarketOrder[] = []; if (operation === MarketOperation.Buy) { resultOrders = await this._marketOperationUtils.getMarketBuyOrdersAsync( @@ -151,37 +152,35 @@ export class SwapQuoteCalculator { operation, assetFillAmount, gasPrice, - opts, ); } private async _createSwapQuoteAsync( makerAssetData: string, takerAssetData: string, - resultOrders: SignedOrderWithFillableAmounts[], + resultOrders: OptimizedMarketOrder[], operation: MarketOperation, assetFillAmount: BigNumber, gasPrice: BigNumber, - opts: CalculateSwapQuoteOpts, ): Promise { const bestCaseQuoteInfo = await this._calculateQuoteInfoAsync( - createBestCaseOrders(resultOrders, operation, opts.bridgeSlippage), + resultOrders, assetFillAmount, gasPrice, operation, ); - // in order to calculate the maxRate, reverse the ordersAndFillableAmounts - // such that they are sorted from worst rate to best rate const worstCaseQuoteInfo = await this._calculateQuoteInfoAsync( - _.reverse(resultOrders.slice()), + resultOrders, assetFillAmount, gasPrice, operation, + true, ); const quoteBase = { takerAssetData, makerAssetData, - orders: resultOrders, + // Remove fill metadata. + orders: resultOrders.map(o => _.omit(o, 'fill')) as SignedOrderWithFillableAmounts[], bestCaseQuoteInfo, worstCaseQuoteInfo, gasPrice, @@ -204,31 +203,39 @@ export class SwapQuoteCalculator { // tslint:disable-next-line: prefer-function-over-method private async _calculateQuoteInfoAsync( - prunedOrders: SignedOrderWithFillableAmounts[], + orders: OptimizedMarketOrder[], assetFillAmount: BigNumber, gasPrice: BigNumber, operation: MarketOperation, + worstCase: boolean = false, ): Promise { if (operation === MarketOperation.Buy) { - return this._calculateMarketBuyQuoteInfoAsync(prunedOrders, assetFillAmount, gasPrice); + return this._calculateMarketBuyQuoteInfoAsync(orders, assetFillAmount, gasPrice, worstCase); } else { - return this._calculateMarketSellQuoteInfoAsync(prunedOrders, assetFillAmount, gasPrice); + return this._calculateMarketSellQuoteInfoAsync(orders, assetFillAmount, gasPrice, worstCase); } } private async _calculateMarketSellQuoteInfoAsync( - prunedOrders: SignedOrderWithFillableAmounts[], + orders: OptimizedMarketOrder[], takerAssetSellAmount: BigNumber, gasPrice: BigNumber, + worstCase: boolean = false, ): Promise { - const result = prunedOrders.reduce( - (acc, order) => { - const { - totalMakerAssetAmount, - totalTakerAssetAmount, - totalFeeTakerAssetAmount, - remainingTakerAssetFillAmount, - } = acc; + let totalMakerAssetAmount = constants.ZERO_AMOUNT; + let totalTakerAssetAmount = constants.ZERO_AMOUNT; + let totalFeeTakerAssetAmount = constants.ZERO_AMOUNT; + let remainingTakerAssetFillAmount = takerAssetSellAmount; + const filledOrders = [] as OptimizedMarketOrder[]; + const _orders = !worstCase ? orders : orders.slice().reverse(); + for (const order of _orders) { + let makerAssetAmount = constants.ZERO_AMOUNT; + let takerAssetAmount = constants.ZERO_AMOUNT; + let feeTakerAssetAmount = constants.ZERO_AMOUNT; + if (remainingTakerAssetFillAmount.lte(0)) { + break; + } + if (order.fill.source === ERC20BridgeSource.Native) { const adjustedFillableMakerAssetAmount = fillableAmountsUtils.getMakerAssetAmountSwappedAfterFees( order, ); @@ -239,99 +246,155 @@ export class SwapQuoteCalculator { remainingTakerAssetFillAmount, adjustedFillableTakerAssetAmount, ); - const { takerAssetAmount, feeTakerAssetAmount } = getTakerAssetAmountBreakDown( - order, - takerAssetAmountWithFees, - ); - const makerAssetAmount = takerAssetAmountWithFees + const takerAssetAmountBreakDown = getTakerAssetAmountBreakDown(order, takerAssetAmountWithFees); + takerAssetAmount = takerAssetAmountBreakDown.takerAssetAmount; + feeTakerAssetAmount = takerAssetAmountBreakDown.feeTakerAssetAmount; + makerAssetAmount = takerAssetAmountWithFees .div(adjustedFillableTakerAssetAmount) - .multipliedBy(adjustedFillableMakerAssetAmount) + .times(adjustedFillableMakerAssetAmount) .integerValue(BigNumber.ROUND_DOWN); - return { - totalMakerAssetAmount: totalMakerAssetAmount.plus(makerAssetAmount), - totalTakerAssetAmount: totalTakerAssetAmount.plus(takerAssetAmount), - totalFeeTakerAssetAmount: totalFeeTakerAssetAmount.plus(feeTakerAssetAmount), - remainingTakerAssetFillAmount: BigNumber.max( - constants.ZERO_AMOUNT, - remainingTakerAssetFillAmount.minus(takerAssetAmountWithFees), - ), - }; - }, - { - totalMakerAssetAmount: constants.ZERO_AMOUNT, - totalTakerAssetAmount: constants.ZERO_AMOUNT, - totalFeeTakerAssetAmount: constants.ZERO_AMOUNT, - remainingTakerAssetFillAmount: takerAssetSellAmount, - }, - ); + } else { + // This is a collapsed bridge order. + // Because collapsed bridge orders actually fill at different rates, + // we can iterate over the uncollapsed fills to get the actual + // asset amounts transfered. + // We can also assume there are no fees and the order is not + // partially filled. + + // Infer the bridge slippage from the difference between the fill + // size and the atual order asset amounts. + const makerAssetBridgeSlippage = !worstCase + ? constants.ONE_AMOUNT + : order.makerAssetAmount.div(order.fill.totalMakerAssetAmount); + const takerAssetBridgeSlippage = !worstCase + ? constants.ONE_AMOUNT + : order.takerAssetAmount.div(order.fill.totalTakerAssetAmount); + // Consecutively fill the subfills in this order. + const subFills = !worstCase ? order.fill.subFills : order.fill.subFills.slice().reverse(); + for (const subFill of subFills) { + if (remainingTakerAssetFillAmount.minus(takerAssetAmount).lte(0)) { + break; + } + const partialTakerAssetAmount = subFill.takerAssetAmount.times(takerAssetBridgeSlippage); + const partialMakerAssetAmount = subFill.makerAssetAmount.times(makerAssetBridgeSlippage); + const partialTakerAssetFillAmount = BigNumber.min( + partialTakerAssetAmount, + remainingTakerAssetFillAmount.minus(takerAssetAmount), + ); + const partialMakerAssetFillAmount = partialTakerAssetFillAmount + .div(partialTakerAssetAmount) + .times(partialMakerAssetAmount) + .integerValue(BigNumber.ROUND_DOWN); + takerAssetAmount = takerAssetAmount.plus(partialTakerAssetFillAmount); + makerAssetAmount = makerAssetAmount.plus(partialMakerAssetFillAmount); + } + } + totalMakerAssetAmount = totalMakerAssetAmount.plus(makerAssetAmount); + totalTakerAssetAmount = totalTakerAssetAmount.plus(takerAssetAmount); + totalFeeTakerAssetAmount = totalFeeTakerAssetAmount.plus(feeTakerAssetAmount); + remainingTakerAssetFillAmount = remainingTakerAssetFillAmount + .minus(takerAssetAmount) + .minus(feeTakerAssetAmount); + filledOrders.push(order); + } const protocolFeeInWeiAmount = await this._protocolFeeUtils.calculateWorstCaseProtocolFeeAsync( - prunedOrders, + filledOrders, gasPrice, ); return { - feeTakerAssetAmount: result.totalFeeTakerAssetAmount, - takerAssetAmount: result.totalTakerAssetAmount, - totalTakerAssetAmount: result.totalFeeTakerAssetAmount.plus(result.totalTakerAssetAmount), - makerAssetAmount: result.totalMakerAssetAmount, + feeTakerAssetAmount: totalFeeTakerAssetAmount, + takerAssetAmount: totalTakerAssetAmount, + totalTakerAssetAmount: totalFeeTakerAssetAmount.plus(totalTakerAssetAmount), + makerAssetAmount: totalMakerAssetAmount, protocolFeeInWeiAmount, }; } private async _calculateMarketBuyQuoteInfoAsync( - prunedOrders: SignedOrderWithFillableAmounts[], + orders: OptimizedMarketOrder[], makerAssetBuyAmount: BigNumber, gasPrice: BigNumber, + worstCase: boolean = false, ): Promise { - const result = prunedOrders.reduce( - (acc, order) => { - const { - totalMakerAssetAmount, - totalTakerAssetAmount, - totalFeeTakerAssetAmount, - remainingMakerAssetFillAmount, - } = acc; + let totalMakerAssetAmount = constants.ZERO_AMOUNT; + let totalTakerAssetAmount = constants.ZERO_AMOUNT; + let totalFeeTakerAssetAmount = constants.ZERO_AMOUNT; + let remainingMakerAssetFillAmount = makerAssetBuyAmount; + const filledOrders = [] as OptimizedMarketOrder[]; + const _orders = !worstCase ? orders : orders.slice().reverse(); + for (const order of _orders) { + let makerAssetAmount = constants.ZERO_AMOUNT; + let takerAssetAmount = constants.ZERO_AMOUNT; + let feeTakerAssetAmount = constants.ZERO_AMOUNT; + if (remainingMakerAssetFillAmount.lte(0)) { + break; + } + if (order.fill.source === ERC20BridgeSource.Native) { const adjustedFillableMakerAssetAmount = fillableAmountsUtils.getMakerAssetAmountSwappedAfterFees( order, ); const adjustedFillableTakerAssetAmount = fillableAmountsUtils.getTakerAssetAmountSwappedAfterFees( order, ); - const makerFillAmount = BigNumber.min(remainingMakerAssetFillAmount, adjustedFillableMakerAssetAmount); - const takerAssetAmountWithFees = makerFillAmount + makerAssetAmount = BigNumber.min(remainingMakerAssetFillAmount, adjustedFillableMakerAssetAmount); + const takerAssetAmountWithFees = makerAssetAmount .div(adjustedFillableMakerAssetAmount) .multipliedBy(adjustedFillableTakerAssetAmount) .integerValue(BigNumber.ROUND_UP); + const takerAssetAmountBreakDown = getTakerAssetAmountBreakDown(order, takerAssetAmountWithFees); + takerAssetAmount = takerAssetAmountBreakDown.takerAssetAmount; + feeTakerAssetAmount = takerAssetAmountBreakDown.feeTakerAssetAmount; + } else { + // This is a collapsed bridge order. + // Because collapsed bridge orders actually fill at different rates, + // we can iterate over the uncollapsed fills to get the actual + // asset amounts transfered. + // We can also assume there are no fees and the order is not + // partially filled. - const { takerAssetAmount, feeTakerAssetAmount } = getTakerAssetAmountBreakDown( - order, - takerAssetAmountWithFees, - ); - return { - totalMakerAssetAmount: totalMakerAssetAmount.plus(makerFillAmount), - totalTakerAssetAmount: totalTakerAssetAmount.plus(takerAssetAmount), - totalFeeTakerAssetAmount: totalFeeTakerAssetAmount.plus(feeTakerAssetAmount), - remainingMakerAssetFillAmount: BigNumber.max( - constants.ZERO_AMOUNT, - remainingMakerAssetFillAmount.minus(makerFillAmount), - ), - }; - }, - { - totalMakerAssetAmount: constants.ZERO_AMOUNT, - totalTakerAssetAmount: constants.ZERO_AMOUNT, - totalFeeTakerAssetAmount: constants.ZERO_AMOUNT, - remainingMakerAssetFillAmount: makerAssetBuyAmount, - }, - ); + // Infer the bridge slippage from the difference between the fill + // size and the atual order asset amounts. + const makerAssetBridgeSlippage = !worstCase + ? constants.ONE_AMOUNT + : order.makerAssetAmount.div(order.fill.totalMakerAssetAmount); + const takerAssetBridgeSlippage = !worstCase + ? constants.ONE_AMOUNT + : order.takerAssetAmount.div(order.fill.totalTakerAssetAmount); + // Consecutively fill the subfills in this order. + const subFills = !worstCase ? order.fill.subFills : order.fill.subFills.slice().reverse(); + for (const subFill of subFills) { + if (remainingMakerAssetFillAmount.minus(makerAssetAmount).lte(0)) { + break; + } + const partialTakerAssetAmount = subFill.takerAssetAmount.times(takerAssetBridgeSlippage); + const partialMakerAssetAmount = subFill.makerAssetAmount.times(makerAssetBridgeSlippage); + const partialMakerAssetFillAmount = BigNumber.min( + partialMakerAssetAmount, + remainingMakerAssetFillAmount.minus(makerAssetAmount), + ); + const partialTakerAssetFillAmount = partialMakerAssetFillAmount + .div(partialMakerAssetAmount) + .times(partialTakerAssetAmount) + .integerValue(BigNumber.ROUND_UP); + takerAssetAmount = takerAssetAmount.plus(partialTakerAssetFillAmount); + makerAssetAmount = makerAssetAmount.plus(partialMakerAssetFillAmount); + } + } + totalMakerAssetAmount = totalMakerAssetAmount.plus(makerAssetAmount); + totalTakerAssetAmount = totalTakerAssetAmount.plus(takerAssetAmount); + totalFeeTakerAssetAmount = totalFeeTakerAssetAmount.plus(feeTakerAssetAmount); + remainingMakerAssetFillAmount = remainingMakerAssetFillAmount.minus(makerAssetAmount); + filledOrders.push(order); + } const protocolFeeInWeiAmount = await this._protocolFeeUtils.calculateWorstCaseProtocolFeeAsync( - prunedOrders, + filledOrders, gasPrice, ); return { - feeTakerAssetAmount: result.totalFeeTakerAssetAmount, - takerAssetAmount: result.totalTakerAssetAmount, - totalTakerAssetAmount: result.totalFeeTakerAssetAmount.plus(result.totalTakerAssetAmount), - makerAssetAmount: result.totalMakerAssetAmount, + feeTakerAssetAmount: totalFeeTakerAssetAmount, + takerAssetAmount: totalTakerAssetAmount, + totalTakerAssetAmount: totalFeeTakerAssetAmount.plus(totalTakerAssetAmount), + makerAssetAmount: totalMakerAssetAmount, protocolFeeInWeiAmount, }; } @@ -372,35 +435,3 @@ function getTakerAssetAmountBreakDown( takerAssetAmount: takerAssetAmountWithFees, }; } - -function createBestCaseOrders( - orders: SignedOrderWithFillableAmounts[], - operation: MarketOperation, - bridgeSlippage: number, -): SignedOrderWithFillableAmounts[] { - // Scale the asset amounts to undo the bridge slippage applied to - // bridge orders. - const bestCaseOrders: SignedOrderWithFillableAmounts[] = []; - for (const order of orders) { - if (!order.makerAssetData.startsWith(constants.BRIDGE_ASSET_DATA_PREFIX)) { - bestCaseOrders.push(order); - continue; - } - bestCaseOrders.push({ - ...order, - ...(operation === MarketOperation.Sell - ? { - makerAssetAmount: order.makerAssetAmount - .dividedBy(1 - bridgeSlippage) - .integerValue(BigNumber.ROUND_DOWN), - } - : // Buy operation - { - takerAssetAmount: order.takerAssetAmount - .dividedBy(bridgeSlippage + 1) - .integerValue(BigNumber.ROUND_UP), - }), - }); - } - return bestCaseOrders; -} diff --git a/packages/asset-swapper/test/calculate_liquidity_test.ts b/packages/asset-swapper/test/calculate_liquidity_test.ts index 308e98813f..3f20737c48 100644 --- a/packages/asset-swapper/test/calculate_liquidity_test.ts +++ b/packages/asset-swapper/test/calculate_liquidity_test.ts @@ -23,7 +23,7 @@ describe('#calculateLiquidity', () => { const { makerAssetAvailableInBaseUnits, takerAssetAvailableInBaseUnits } = calculateLiquidity( prunedSignedOrders, ); - expect(makerAssetAvailableInBaseUnits).to.bignumber.eq(baseUnitAmount(10)); + expect(makerAssetAvailableInBaseUnits).to.bignumber.eq(baseUnitAmount(11)); expect(takerAssetAvailableInBaseUnits).to.bignumber.eq(baseUnitAmount(9)); }); it('should provide correct liquidity result with orders with takerFees in takerAsset', () => { @@ -31,7 +31,7 @@ describe('#calculateLiquidity', () => { const { makerAssetAvailableInBaseUnits, takerAssetAvailableInBaseUnits } = calculateLiquidity( prunedSignedOrders, ); - expect(makerAssetAvailableInBaseUnits).to.bignumber.eq(baseUnitAmount(10)); + expect(makerAssetAvailableInBaseUnits).to.bignumber.eq(baseUnitAmount(11)); expect(takerAssetAvailableInBaseUnits).to.bignumber.eq(baseUnitAmount(15)); }); it('should provide correct liquidity result with orders with takerFees in makerAsset', () => { @@ -51,7 +51,7 @@ describe('#calculateLiquidity', () => { const { makerAssetAvailableInBaseUnits, takerAssetAvailableInBaseUnits } = calculateLiquidity( prunedSignedOrders, ); - expect(makerAssetAvailableInBaseUnits).to.bignumber.eq(baseUnitAmount(25)); + expect(makerAssetAvailableInBaseUnits).to.bignumber.eq(baseUnitAmount(27)); expect(takerAssetAvailableInBaseUnits).to.bignumber.eq(baseUnitAmount(33)); }); }); diff --git a/packages/asset-swapper/test/swap_quote_calculator_test.ts b/packages/asset-swapper/test/swap_quote_calculator_test.ts index a8c760d9a0..f595683065 100644 --- a/packages/asset-swapper/test/swap_quote_calculator_test.ts +++ b/packages/asset-swapper/test/swap_quote_calculator_test.ts @@ -65,6 +65,9 @@ const createSamplerFromSignedOrdersWithFillableAmounts = ( return sampler; }; +// TODO(dorothy-zbornak): Replace these tests entirely with unit tests because +// omg they're a nightmare to maintain. + // tslint:disable:max-file-line-count // tslint:disable:custom-no-magic-numbers describe('swapQuoteCalculator', () => { @@ -272,7 +275,7 @@ describe('swapQuoteCalculator', () => { }); }); it('calculates a correct swapQuote with slippage (feeless orders)', async () => { - const assetSellAmount = baseUnitAmount(1); + const assetSellAmount = baseUnitAmount(4); const slippagePercentage = 0.2; const sampler = createSamplerFromSignedOrdersWithFillableAmounts( testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEELESS, @@ -289,10 +292,10 @@ describe('swapQuoteCalculator', () => { GAS_PRICE, CALCULATE_SWAP_QUOTE_OPTS, ); - // test if orders are correct expect(swapQuote.orders).to.deep.equal([ testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEELESS[0], + testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEELESS[2], testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEELESS[1], ]); expect(swapQuote.takerAssetFillAmount).to.bignumber.equal(assetSellAmount); @@ -301,15 +304,15 @@ describe('swapQuoteCalculator', () => { feeTakerAssetAmount: baseUnitAmount(0), takerAssetAmount: assetSellAmount, totalTakerAssetAmount: assetSellAmount, - makerAssetAmount: baseUnitAmount(6), + makerAssetAmount: baseUnitAmount(9), protocolFeeInWeiAmount: baseUnitAmount(30, 4), }); expect(swapQuote.worstCaseQuoteInfo).to.deep.equal({ feeTakerAssetAmount: baseUnitAmount(0), takerAssetAmount: assetSellAmount, totalTakerAssetAmount: assetSellAmount, - makerAssetAmount: baseUnitAmount(0.4), - protocolFeeInWeiAmount: baseUnitAmount(30, 4), + makerAssetAmount: baseUnitAmount(1.6), + protocolFeeInWeiAmount: baseUnitAmount(15, 4), }); }); it('calculates a correct swapQuote with no slippage (takerAsset denominated fee orders)', async () => { @@ -372,7 +375,7 @@ describe('swapQuoteCalculator', () => { // test if orders are correct expect(swapQuote.orders).to.deep.equal([ testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_TAKER_ASSET[0], - testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_TAKER_ASSET[1], + testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_TAKER_ASSET[2], ]); expect(swapQuote.takerAssetFillAmount).to.bignumber.equal(assetSellAmount); // test if rates are correct @@ -381,14 +384,14 @@ describe('swapQuoteCalculator', () => { takerAssetAmount: assetSellAmount.minus(baseUnitAmount(2.25)), totalTakerAssetAmount: assetSellAmount, makerAssetAmount: baseUnitAmount(4.5), - protocolFeeInWeiAmount: baseUnitAmount(30, 4), + protocolFeeInWeiAmount: baseUnitAmount(15, 4), }); expect(swapQuote.worstCaseQuoteInfo).to.deep.equal({ - feeTakerAssetAmount: baseUnitAmount(0.5), - takerAssetAmount: assetSellAmount.minus(baseUnitAmount(0.5)), + feeTakerAssetAmount: baseUnitAmount(1.2), + takerAssetAmount: assetSellAmount.minus(baseUnitAmount(1.2)), totalTakerAssetAmount: assetSellAmount, - makerAssetAmount: baseUnitAmount(1), - protocolFeeInWeiAmount: baseUnitAmount(30, 4), + makerAssetAmount: baseUnitAmount(1.8), + protocolFeeInWeiAmount: baseUnitAmount(15, 4), }); }); it('calculates a correct swapQuote with no slippage (makerAsset denominated fee orders)', async () => { @@ -411,23 +414,24 @@ describe('swapQuoteCalculator', () => { ); // test if orders are correct expect(swapQuote.orders).to.deep.equal([ - testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[0], + testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[1], + testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[2], ]); expect(swapQuote.takerAssetFillAmount).to.bignumber.equal(assetSellAmount); // test if rates are correct expect(swapQuote.bestCaseQuoteInfo).to.deep.equal({ - feeTakerAssetAmount: baseUnitAmount(2), - takerAssetAmount: assetSellAmount.minus(baseUnitAmount(2)), + feeTakerAssetAmount: baseUnitAmount(1.5).minus(1), + takerAssetAmount: assetSellAmount.minus(baseUnitAmount(1.5)).plus(1), totalTakerAssetAmount: assetSellAmount, - makerAssetAmount: baseUnitAmount(0.8), - protocolFeeInWeiAmount: baseUnitAmount(15, 4), + makerAssetAmount: baseUnitAmount(4), + protocolFeeInWeiAmount: baseUnitAmount(30, 4), }); expect(swapQuote.worstCaseQuoteInfo).to.deep.equal({ - feeTakerAssetAmount: baseUnitAmount(2), - takerAssetAmount: assetSellAmount.minus(baseUnitAmount(2)), + feeTakerAssetAmount: baseUnitAmount(1.5).minus(1), + takerAssetAmount: assetSellAmount.minus(baseUnitAmount(1.5)).plus(1), totalTakerAssetAmount: assetSellAmount, - makerAssetAmount: baseUnitAmount(0.8), - protocolFeeInWeiAmount: baseUnitAmount(15, 4), + makerAssetAmount: baseUnitAmount(4), + protocolFeeInWeiAmount: baseUnitAmount(30, 4), }); }); it('calculates a correct swapQuote with slippage (makerAsset denominated fee orders)', async () => { @@ -451,24 +455,24 @@ describe('swapQuoteCalculator', () => { // test if orders are correct expect(swapQuote.orders).to.deep.equal([ testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[1], + testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[2], testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[0], ]); expect(swapQuote.takerAssetFillAmount).to.bignumber.equal(assetSellAmount); // test if rates are correct - // 50 takerAsset units to fill the first order + 100 takerAsset units for fees + expect(swapQuote.bestCaseQuoteInfo).to.deep.equal({ + feeTakerAssetAmount: baseUnitAmount(1.5).minus(1), + takerAssetAmount: assetSellAmount.minus(baseUnitAmount(1.5)).plus(1), + totalTakerAssetAmount: assetSellAmount, + makerAssetAmount: baseUnitAmount(4), + protocolFeeInWeiAmount: baseUnitAmount(30, 4), + }); expect(swapQuote.worstCaseQuoteInfo).to.deep.equal({ feeTakerAssetAmount: baseUnitAmount(2), takerAssetAmount: assetSellAmount.minus(baseUnitAmount(2)), totalTakerAssetAmount: assetSellAmount, makerAssetAmount: baseUnitAmount(0.8), - protocolFeeInWeiAmount: baseUnitAmount(30, 4), - }); - expect(swapQuote.bestCaseQuoteInfo).to.deep.equal({ - feeTakerAssetAmount: baseUnitAmount(2), - takerAssetAmount: assetSellAmount.minus(baseUnitAmount(2)), - totalTakerAssetAmount: assetSellAmount, - makerAssetAmount: baseUnitAmount(3.6), - protocolFeeInWeiAmount: baseUnitAmount(30, 4), + protocolFeeInWeiAmount: baseUnitAmount(15, 4), }); }); }); @@ -678,7 +682,7 @@ describe('swapQuoteCalculator', () => { // test if orders are correct expect(swapQuote.orders).to.deep.equal([ testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEELESS[0], - testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEELESS[1], + testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEELESS[2], ]); expect(swapQuote.makerAssetFillAmount).to.bignumber.equal(assetBuyAmount); @@ -692,12 +696,16 @@ describe('swapQuoteCalculator', () => { takerAssetAmount, totalTakerAssetAmount: takerAssetAmount, makerAssetAmount: assetBuyAmount, - protocolFeeInWeiAmount: baseUnitAmount(30, 4), + protocolFeeInWeiAmount: baseUnitAmount(15, 4), }); expect(swapQuote.worstCaseQuoteInfo).to.deep.equal({ feeTakerAssetAmount: baseUnitAmount(0), - takerAssetAmount: baseUnitAmount(5.5), - totalTakerAssetAmount: baseUnitAmount(5.5), + takerAssetAmount: baseUnitAmount(20) + .div(6) + .integerValue(BigNumber.ROUND_UP), + totalTakerAssetAmount: baseUnitAmount(20) + .div(6) + .integerValue(BigNumber.ROUND_UP), makerAssetAmount: assetBuyAmount, protocolFeeInWeiAmount: baseUnitAmount(30, 4), }); @@ -766,7 +774,7 @@ describe('swapQuoteCalculator', () => { // test if orders are correct expect(swapQuote.orders).to.deep.equal([ testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_TAKER_ASSET[0], - testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_TAKER_ASSET[1], + testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_TAKER_ASSET[2], ]); expect(swapQuote.makerAssetFillAmount).to.bignumber.equal(assetBuyAmount); // test if rates are correct @@ -775,12 +783,16 @@ describe('swapQuoteCalculator', () => { takerAssetAmount: fiveSixthEthInWei, totalTakerAssetAmount: baseUnitAmount(2.5).plus(fiveSixthEthInWei), makerAssetAmount: assetBuyAmount, - protocolFeeInWeiAmount: baseUnitAmount(30, 4), + protocolFeeInWeiAmount: baseUnitAmount(15, 4), }); expect(swapQuote.worstCaseQuoteInfo).to.deep.equal({ - feeTakerAssetAmount: baseUnitAmount(2.5), - takerAssetAmount: baseUnitAmount(5.5), - totalTakerAssetAmount: baseUnitAmount(8), + feeTakerAssetAmount: baseUnitAmount(3), + takerAssetAmount: baseUnitAmount(10) + .div(3) + .integerValue(BigNumber.ROUND_UP), // 3.3333... + totalTakerAssetAmount: baseUnitAmount(19) + .div(3) + .integerValue(BigNumber.ROUND_UP), // 6.3333... makerAssetAmount: assetBuyAmount, protocolFeeInWeiAmount: baseUnitAmount(30, 4), }); @@ -805,21 +817,33 @@ describe('swapQuoteCalculator', () => { ); // test if orders are correct expect(swapQuote.orders).to.deep.equal([ - testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[0], + testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[1], ]); expect(swapQuote.makerAssetFillAmount).to.bignumber.equal(assetBuyAmount); // test if rates are correct expect(swapQuote.bestCaseQuoteInfo).to.deep.equal({ - feeTakerAssetAmount: baseUnitAmount(2.5), - takerAssetAmount: baseUnitAmount(2.5), - totalTakerAssetAmount: baseUnitAmount(5), + feeTakerAssetAmount: baseUnitAmount(0.5) + .div(3) + .integerValue(BigNumber.ROUND_UP), // 0.16666... + takerAssetAmount: baseUnitAmount(0.5) + .div(3) + .integerValue(BigNumber.ROUND_UP), // 0.1666... + totalTakerAssetAmount: baseUnitAmount(1) + .div(3) + .integerValue(BigNumber.ROUND_UP), // 0.3333... makerAssetAmount: assetBuyAmount, protocolFeeInWeiAmount: baseUnitAmount(15, 4), }); expect(swapQuote.worstCaseQuoteInfo).to.deep.equal({ - feeTakerAssetAmount: baseUnitAmount(2.5), - takerAssetAmount: baseUnitAmount(2.5), - totalTakerAssetAmount: baseUnitAmount(5), + feeTakerAssetAmount: baseUnitAmount(0.5) + .div(3) + .integerValue(BigNumber.ROUND_UP), // 0.16666... + takerAssetAmount: baseUnitAmount(0.5) + .div(3) + .integerValue(BigNumber.ROUND_UP), // 0.1666... + totalTakerAssetAmount: baseUnitAmount(1) + .div(3) + .integerValue(BigNumber.ROUND_UP), // 0.3333... makerAssetAmount: assetBuyAmount, protocolFeeInWeiAmount: baseUnitAmount(15, 4), }); @@ -845,14 +869,14 @@ describe('swapQuoteCalculator', () => { // test if orders are correct expect(swapQuote.orders).to.deep.equal([ testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[1], - testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[0], + testOrders.SIGNED_ORDERS_WITH_FILLABLE_AMOUNTS_FEE_IN_MAKER_ASSET[2], ]); expect(swapQuote.makerAssetFillAmount).to.bignumber.equal(assetBuyAmount); // test if rates are correct expect(swapQuote.worstCaseQuoteInfo).to.deep.equal({ - feeTakerAssetAmount: baseUnitAmount(2.75), - takerAssetAmount: baseUnitAmount(2.75), - totalTakerAssetAmount: baseUnitAmount(5.5), + feeTakerAssetAmount: baseUnitAmount(1.25).minus(1), + takerAssetAmount: baseUnitAmount(2.25).plus(1), + totalTakerAssetAmount: baseUnitAmount(3.5), makerAssetAmount: assetBuyAmount, protocolFeeInWeiAmount: baseUnitAmount(30, 4), }); @@ -879,7 +903,7 @@ describe('swapQuoteCalculator', () => { .multipliedBy(0.1) .integerValue(BigNumber.ROUND_CEIL), makerAssetAmount: assetBuyAmount, - protocolFeeInWeiAmount: baseUnitAmount(30, 4), + protocolFeeInWeiAmount: baseUnitAmount(15, 4), }); }); }); diff --git a/packages/asset-swapper/test/utils/test_orders.ts b/packages/asset-swapper/test/utils/test_orders.ts index 3817b57783..5a2581c231 100644 --- a/packages/asset-swapper/test/utils/test_orders.ts +++ b/packages/asset-swapper/test/utils/test_orders.ts @@ -51,7 +51,7 @@ const PARTIAL_ORDERS_WITH_FILLABLE_AMOUNTS_FEELESS: Array