From d60c8ddd5a4dab5f5d2915997413beac8eed1cc1 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Tue, 29 Sep 2020 22:25:24 -0700 Subject: [PATCH] added more unit tests --- .../src/utils/market_operation_utils/index.ts | 185 +++++++++--------- .../test/market_operation_utils_test.ts | 144 +++++++++----- 2 files changed, 190 insertions(+), 139 deletions(-) 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 ecd6fda1f1..2f7b7a3133 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -32,7 +32,6 @@ import { AggregationError, DexSample, ERC20BridgeSource, - FeeSchedule, GenerateOptimizedOrdersOpts, GetMarketOrdersOpts, MarketSideLiquidity, @@ -460,6 +459,98 @@ export class MarketOperationUtils { ); } + public async _generateOptimizedOrdersAsync( + marketSideLiquidity: MarketSideLiquidity, + opts: GenerateOptimizedOrdersOpts, + ): Promise { + const { + inputToken, + outputToken, + side, + inputAmount, + nativeOrders, + orderFillableAmounts, + rfqtIndicativeQuotes, + dexQuotes, + ethToOutputRate, + ethToInputRate, + } = marketSideLiquidity; + const maxFallbackSlippage = opts.maxFallbackSlippage || 0; + + const orderOpts = { + side, + inputToken, + outputToken, + orderDomain: this._orderDomain, + contractAddresses: this.contractAddresses, + bridgeSlippage: opts.bridgeSlippage || 0, + shouldBatchBridgeOrders: !!opts.shouldBatchBridgeOrders, + }; + + // Convert native orders and dex quotes into fill paths. + const paths = createFillPaths({ + side, + // Augment native orders with their fillable amounts. + orders: [ + ...createSignedOrdersWithFillableAmounts(side, nativeOrders, orderFillableAmounts), + ...createSignedOrdersFromRfqtIndicativeQuotes(rfqtIndicativeQuotes), + ], + dexQuotes, + targetInput: inputAmount, + ethToOutputRate, + ethToInputRate, + excludedSources: opts.excludedSources, + feeSchedule: opts.feeSchedule, + }); + + // Find the optimal path. + let optimalPath = (await findOptimalPathAsync(side, paths, inputAmount, opts.runLimit)) || []; + if (optimalPath.length === 0) { + throw new Error(AggregationError.NoOptimalPath); + } + const optimalPathRate = getPathAdjustedRate(side, optimalPath, inputAmount); + + const { adjustedRate: bestTwoHopRate, quote: bestTwoHopQuote } = getBestTwoHopQuote( + marketSideLiquidity, + opts.feeSchedule, + ); + if (bestTwoHopQuote && bestTwoHopRate.isGreaterThan(optimalPathRate)) { + const twoHopOrders = createOrdersFromTwoHopSample(bestTwoHopQuote, orderOpts); + return { optimizedOrders: twoHopOrders, liquidityDelivered: bestTwoHopQuote, isTwoHop: true }; + } + + // Generate a fallback path if native orders are in the optimal path. + const nativeSubPath = optimalPath.filter(f => f.source === ERC20BridgeSource.Native); + if (opts.allowFallback && nativeSubPath.length !== 0) { + // We create a fallback path that is exclusive of Native liquidity + // This is the optimal on-chain path for the entire input amount + const nonNativePaths = paths.filter(p => p.length > 0 && p[0].source !== ERC20BridgeSource.Native); + const nonNativeOptimalPath = + (await findOptimalPathAsync(side, nonNativePaths, inputAmount, opts.runLimit)) || []; + // Calculate the slippage of on-chain sources compared to the most optimal path + const fallbackSlippage = getPathAdjustedSlippage(side, nonNativeOptimalPath, inputAmount, optimalPathRate); + if (nativeSubPath.length === optimalPath.length || fallbackSlippage <= maxFallbackSlippage) { + // If the last fill is Native and penultimate is not, then the intention was to partial fill + // In this case we drop it entirely as we can't handle a failure at the end and we don't + // want to fully fill when it gets prepended to the front below + const [last, penultimateIfExists] = optimalPath.slice().reverse(); + const lastNativeFillIfExists = + last.source === ERC20BridgeSource.Native && + penultimateIfExists && + penultimateIfExists.source !== ERC20BridgeSource.Native + ? last + : undefined; + // By prepending native paths to the front they cannot split on-chain sources and incur + // an additional protocol fee. I.e [Uniswap,Native,Kyber] becomes [Native,Uniswap,Kyber] + // In the previous step we dropped any hanging Native partial fills, as to not fully fill + optimalPath = [...nativeSubPath.filter(f => f !== lastNativeFillIfExists), ...nonNativeOptimalPath]; + } + } + const optimizedOrders = createOrdersFromPath(optimalPath, orderOpts); + const liquidityDelivered = _.flatten(optimizedOrders.map(order => order.fills)); + return { optimizedOrders, liquidityDelivered, isTwoHop: false }; + } + private async _getMarketSideOrdersAsync( nativeOrders: SignedOrder[], amount: BigNumber, @@ -557,98 +648,6 @@ export class MarketOperationUtils { } return {...optimizerResult, quoteReport}; } - - public async _generateOptimizedOrdersAsync( - marketSideLiquidity: MarketSideLiquidity, - opts: GenerateOptimizedOrdersOpts, - ): Promise { - const { - inputToken, - outputToken, - side, - inputAmount, - nativeOrders, - orderFillableAmounts, - rfqtIndicativeQuotes, - dexQuotes, - ethToOutputRate, - ethToInputRate, - } = marketSideLiquidity; - const maxFallbackSlippage = opts.maxFallbackSlippage || 0; - - const orderOpts = { - side, - inputToken, - outputToken, - orderDomain: this._orderDomain, - contractAddresses: this.contractAddresses, - bridgeSlippage: opts.bridgeSlippage || 0, - shouldBatchBridgeOrders: !!opts.shouldBatchBridgeOrders, - }; - - // Convert native orders and dex quotes into fill paths. - const paths = createFillPaths({ - side, - // Augment native orders with their fillable amounts. - orders: [ - ...createSignedOrdersWithFillableAmounts(side, nativeOrders, orderFillableAmounts), - ...createSignedOrdersFromRfqtIndicativeQuotes(rfqtIndicativeQuotes), - ], - dexQuotes, - targetInput: inputAmount, - ethToOutputRate, - ethToInputRate, - excludedSources: opts.excludedSources, - feeSchedule: opts.feeSchedule, - }); - - // Find the optimal path. - let optimalPath = (await findOptimalPathAsync(side, paths, inputAmount, opts.runLimit)) || []; - if (optimalPath.length === 0) { - throw new Error(AggregationError.NoOptimalPath); - } - const optimalPathRate = getPathAdjustedRate(side, optimalPath, inputAmount); - - const { adjustedRate: bestTwoHopRate, quote: bestTwoHopQuote } = getBestTwoHopQuote( - marketSideLiquidity, - opts.feeSchedule, - ); - if (bestTwoHopQuote && bestTwoHopRate.isGreaterThan(optimalPathRate)) { - const twoHopOrders = createOrdersFromTwoHopSample(bestTwoHopQuote, orderOpts); - return { optimizedOrders: twoHopOrders, liquidityDelivered: bestTwoHopQuote, isTwoHop: true }; - } - - // Generate a fallback path if native orders are in the optimal path. - const nativeSubPath = optimalPath.filter(f => f.source === ERC20BridgeSource.Native); - if (opts.allowFallback && nativeSubPath.length !== 0) { - // We create a fallback path that is exclusive of Native liquidity - // This is the optimal on-chain path for the entire input amount - const nonNativePaths = paths.filter(p => p.length > 0 && p[0].source !== ERC20BridgeSource.Native); - const nonNativeOptimalPath = - (await findOptimalPathAsync(side, nonNativePaths, inputAmount, opts.runLimit)) || []; - // Calculate the slippage of on-chain sources compared to the most optimal path - const fallbackSlippage = getPathAdjustedSlippage(side, nonNativeOptimalPath, inputAmount, optimalPathRate); - if (nativeSubPath.length === optimalPath.length || fallbackSlippage <= maxFallbackSlippage) { - // If the last fill is Native and penultimate is not, then the intention was to partial fill - // In this case we drop it entirely as we can't handle a failure at the end and we don't - // want to fully fill when it gets prepended to the front below - const [last, penultimateIfExists] = optimalPath.slice().reverse(); - const lastNativeFillIfExists = - last.source === ERC20BridgeSource.Native && - penultimateIfExists && - penultimateIfExists.source !== ERC20BridgeSource.Native - ? last - : undefined; - // By prepending native paths to the front they cannot split on-chain sources and incur - // an additional protocol fee. I.e [Uniswap,Native,Kyber] becomes [Native,Uniswap,Kyber] - // In the previous step we dropped any hanging Native partial fills, as to not fully fill - optimalPath = [...nativeSubPath.filter(f => f !== lastNativeFillIfExists), ...nonNativeOptimalPath]; - } - } - const optimizedOrders = createOrdersFromPath(optimalPath, orderOpts); - const liquidityDelivered = _.flatten(optimizedOrders.map(order => order.fills)); - return { optimizedOrders, liquidityDelivered, isTwoHop: false }; - } } // tslint:disable: max-file-line-count diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index c17e4486ca..e4661fdb64 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -15,6 +15,8 @@ import { BigNumber, fromTokenUnitAmount, hexUtils, NULL_ADDRESS } from '@0x/util import * as _ from 'lodash'; import * as TypeMoq from 'typemoq'; +import { RFQMIndicativeQuote, RFQTFirmQuote, RFQTIndicativeQuote } from '@0x/quote-server'; +import { noop, random } from 'lodash'; import { MarketOperation, QuoteRequestor, RfqtRequestOpts, SignedOrderWithFillableAmounts } from '../src'; import { getRfqtIndicativeQuotesAsync, MarketOperationUtils } from '../src/utils/market_operation_utils/'; import { BalancerPoolsCache } from '../src/utils/market_operation_utils/balancer_utils'; @@ -27,18 +29,18 @@ import { import { createFillPaths } from '../src/utils/market_operation_utils/fills'; import { DexOrderSampler } from '../src/utils/market_operation_utils/sampler'; import { BATCH_SOURCE_FILTERS } from '../src/utils/market_operation_utils/sampler_operations'; +import { SourceFilters } from '../src/utils/market_operation_utils/source_filters'; import { DexSample, ERC20BridgeSource, FillData, - NativeFillData, - OptimizedMarketOrder, + GenerateOptimizedOrdersOpts, MarketSideLiquidity, - GenerateOptimizedOrdersOpts + NativeFillData, + OptimizedMarketOrder } from '../src/utils/market_operation_utils/types'; -import { SourceFilters } from '../src/utils/market_operation_utils/source_filters'; -import { noop, random } from 'lodash'; import { quoteRequestorHttpClient } from '../src/utils/quote_requestor'; +import { IReturnsResult } from 'typemoq/_all'; const MAKER_TOKEN = randomAddress(); const TAKER_TOKEN = randomAddress(); @@ -63,6 +65,27 @@ describe('MarketOperationUtils tests', () => { const CHAIN_ID = 1; const contractAddresses = { ...getContractAddressesForChainOrThrow(CHAIN_ID), multiBridge: NULL_ADDRESS }; + function getMockedQuoteRequestor(type: 'indicative' | 'firm', results: SignedOrder[], verifiable: TypeMoq.Times): TypeMoq.IMock { + const args: [any, any, any, any, any] = [ + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + ]; + const requestor = TypeMoq.Mock.ofType(QuoteRequestor, TypeMoq.MockBehavior.Loose, true); + if (type === 'firm') { + requestor.setup( + r => r.requestRfqtFirmQuotesAsync(...args) + ).returns(async () => results.map(result => ({signedOrder: result}))).verifiable(verifiable) + } else { + requestor.setup( + r => r.requestRfqtIndicativeQuotesAsync(...args) + ).returns(async () => results).verifiable(verifiable); + } + return requestor; + } + function createOrder(overrides?: Partial): SignedOrder { return { chainId: CHAIN_ID, @@ -655,9 +678,9 @@ describe('MarketOperationUtils tests', () => { // Ensure that `_generateOptimizedOrdersAsync` is only called once mockedMarketOpUtils.setup( - m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny()) + m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny()), ).returns( - async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b) + async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b), ).verifiable(TypeMoq.Times.once()); const totalAssetAmount = ORDERS.map(o => o.takerAssetAmount).reduce((a, b) => a.plus(b)); @@ -669,31 +692,20 @@ describe('MarketOperationUtils tests', () => { }); it('getMarketSellOrdersAsync() will not rerun the optimizer if no orders are returned', async () => { - const requestor = TypeMoq.Mock.ofType(QuoteRequestor, TypeMoq.MockBehavior.Loose, true); - requestor - .setup(r => - r.requestRfqtFirmQuotesAsync( - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - ), - ) - .returns(() => Promise.resolve([])) - .verifiable(TypeMoq.Times.once()); // Ensure that `_generateOptimizedOrdersAsync` is only called once const mockedMarketOpUtils = TypeMoq.Mock.ofType(MarketOperationUtils, TypeMoq.MockBehavior.Loose, false, MOCK_SAMPLER, contractAddresses, ORDER_DOMAIN); mockedMarketOpUtils.callBase = true; mockedMarketOpUtils.setup( - m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny()) + m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny()), ).returns( - async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b) + async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b), ).verifiable(TypeMoq.Times.once()); + const requestor = getMockedQuoteRequestor('firm', [], TypeMoq.Times.once()); + const totalAssetAmount = ORDERS.map(o => o.takerAssetAmount).reduce((a, b) => a.plus(b)); - const results = await mockedMarketOpUtils.object.getMarketSellOrdersAsync( + await mockedMarketOpUtils.object.getMarketSellOrdersAsync( ORDERS, totalAssetAmount, { ...DEFAULT_OPTS, @@ -704,40 +716,80 @@ describe('MarketOperationUtils tests', () => { intentOnFilling: true, quoteRequestor: { requestRfqtFirmQuotesAsync: requestor.object.requestRfqtFirmQuotesAsync, - } as any - } + } as any, + }, }, ); mockedMarketOpUtils.verifyAll(); requestor.verifyAll(); }); - it.only('getMarketSellOrdersAsync() will rerun the optimizer if one or more RFQ orders are returned', async () => { - const requestor = TypeMoq.Mock.ofType(QuoteRequestor, TypeMoq.MockBehavior.Loose, true); - requestor - .setup(r => - r.requestRfqtFirmQuotesAsync( - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - ), - ) - .returns(() => Promise.resolve([{signedOrder: ORDERS[0]}])) - .verifiable(TypeMoq.Times.once()); + it('getMarketSellOrdersAsync() will rerun the optimizer if one or more indicative are returned', async () => { + const requestor = getMockedQuoteRequestor('indicative', [ORDERS[0], ORDERS[1]], TypeMoq.Times.once()); + + const numOrdersInCall: number[] = []; + const numIndicativeQuotesInCall: number[] = []; - // Ensure that `_generateOptimizedOrdersAsync` is only called once - let numOrdersInCall: number[] = []; const mockedMarketOpUtils = TypeMoq.Mock.ofType(MarketOperationUtils, TypeMoq.MockBehavior.Loose, false, MOCK_SAMPLER, contractAddresses, ORDER_DOMAIN); mockedMarketOpUtils.callBase = true; mockedMarketOpUtils.setup( - m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny()) - ).callback(async (msl: MarketSideLiquidity, opts: GenerateOptimizedOrdersOpts) => { + m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny()), + ).callback(async (msl: MarketSideLiquidity, _opts: GenerateOptimizedOrdersOpts) => { + numOrdersInCall.push(msl.nativeOrders.length); + numIndicativeQuotesInCall.push(msl.rfqtIndicativeQuotes.length); + }) + .returns( + async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b), + ) + .verifiable(TypeMoq.Times.exactly(2)); + + const totalAssetAmount = ORDERS.map(o => o.takerAssetAmount).reduce((a, b) => a.plus(b)); + await mockedMarketOpUtils.object.getMarketSellOrdersAsync( + ORDERS.slice(2, ORDERS.length), totalAssetAmount, + { + ...DEFAULT_OPTS, + rfqt: { + isIndicative: true, + apiKey: 'foo', + takerAddress: randomAddress(), + intentOnFilling: true, + quoteRequestor: { + requestRfqtIndicativeQuotesAsync: requestor.object.requestRfqtIndicativeQuotesAsync, + } as any, + }, + }, + ); + mockedMarketOpUtils.verifyAll(); + requestor.verifyAll(); + + // The first and second optimizer call contains same number of RFQ orders. + expect(numOrdersInCall.length).to.eql(2); + expect(numOrdersInCall[0]).to.eql(1); + expect(numOrdersInCall[1]).to.eql(1); + + // The first call to optimizer will have no RFQ indicative quotes. The second call will have + // two indicative quotes. + expect(numIndicativeQuotesInCall.length).to.eql(2); + expect(numIndicativeQuotesInCall[0]).to.eql(0); + expect(numIndicativeQuotesInCall[1]).to.eql(2); + }); + + it('getMarketSellOrdersAsync() will rerun the optimizer if one or more RFQ orders are returned', async () => { + const requestor = getMockedQuoteRequestor('firm', [ORDERS[0]], TypeMoq.Times.once()); + + // Ensure that `_generateOptimizedOrdersAsync` is only called once + + // TODO: Ensure fillable amounts increase too + const numOrdersInCall: number[] = []; + const mockedMarketOpUtils = TypeMoq.Mock.ofType(MarketOperationUtils, TypeMoq.MockBehavior.Loose, false, MOCK_SAMPLER, contractAddresses, ORDER_DOMAIN); + mockedMarketOpUtils.callBase = true; + mockedMarketOpUtils.setup( + m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny()), + ).callback(async (msl: MarketSideLiquidity, _opts: GenerateOptimizedOrdersOpts) => { numOrdersInCall.push(msl.nativeOrders.length); }) .returns( - async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b) + async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b), ) .verifiable(TypeMoq.Times.exactly(2)); @@ -753,8 +805,8 @@ describe('MarketOperationUtils tests', () => { intentOnFilling: true, quoteRequestor: { requestRfqtFirmQuotesAsync: requestor.object.requestRfqtFirmQuotesAsync, - } as any - } + } as any, + }, }, ); mockedMarketOpUtils.verifyAll();