From 708e34602b0197b8cdde87cef1990128047e77eb Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Tue, 29 Sep 2020 23:00:02 -0700 Subject: [PATCH] added further unit tests --- .../src/utils/market_operation_utils/index.ts | 2 + .../utils/market_operation_utils/orders.ts | 6 ++ .../test/market_operation_utils_test.ts | 67 +++++++++++++++++++ 3 files changed, 75 insertions(+) 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 2f7b7a3133..216f9d30d8 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -575,6 +575,8 @@ export class MarketOperationUtils { optimizerResult = await this._generateOptimizedOrdersAsync(marketSideLiquidity, optimizerOpts); } catch (e) { // If no on-chain or off-chain Open Orderbook orders are present, a `NoOptimalPath` will be thrown. + // If this happens at this stage, there is still a chance that an RFQ order is fillable, therefore + // we catch the error and continue. if (e.message !== AggregationError.NoOptimalPath) { throw e; } diff --git a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts index efcb699a7a..0cbb725153 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts @@ -121,6 +121,12 @@ export function createSignedOrdersWithFillableAmounts( orders: SignedOrder[], fillableAmounts: BigNumber[], ): SignedOrderWithFillableAmounts[] { + + // Quick safety check: ensures that orders maps perfectly to fillable amounts. + if (orders.length !== fillableAmounts.length) { + throw new Error(`Number of orders was ${orders.length} but fillable amounts was ${fillableAmounts.length}. This should never happen`); + } + return orders .map((order: SignedOrder, i: number) => { const fillableAmount = fillableAmounts[i]; diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index e4661fdb64..84bf575972 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -31,6 +31,7 @@ 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 { + AggregationError, DexSample, ERC20BridgeSource, FillData, @@ -819,6 +820,72 @@ describe('MarketOperationUtils tests', () => { expect(numOrdersInCall[1]).to.eql(3); }); + it('getMarketSellOrdersAsync() will not raise a NoOptimalPath error if no initial path was found during on-chain DEX optimization, but a path was found after RFQ optimization', async () => { + let hasFirstOptimizationRun = false; + let hasSecondOptimizationRun = false; + const requestor = getMockedQuoteRequestor('firm', [ORDERS[0], ORDERS[1]], TypeMoq.Times.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()), + ).returns(async (msl: MarketSideLiquidity, _opts: GenerateOptimizedOrdersOpts) => { + if (msl.nativeOrders.length === 1) { + hasFirstOptimizationRun = true; + throw new Error(AggregationError.NoOptimalPath); + } else if (msl.nativeOrders.length === 3) { + hasSecondOptimizationRun = true; + return mockedMarketOpUtils.target._generateOptimizedOrdersAsync(msl, _opts); + } else { + throw new Error('Invalid path. this error message should never appear') + } + }).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: false, + apiKey: 'foo', + takerAddress: randomAddress(), + intentOnFilling: true, + quoteRequestor: { + requestRfqtFirmQuotesAsync: requestor.object.requestRfqtFirmQuotesAsync, + } as any, + }, + }, + ); + mockedMarketOpUtils.verifyAll(); + requestor.verifyAll(); + + expect(hasFirstOptimizationRun).to.eql(true); + expect(hasSecondOptimizationRun).to.eql(true); + }); + + it.only('getMarketSellOrdersAsync() will raise a NoOptimalPath error if no path was found during on-chain DEX optimization and RFQ optimization', async () => { + 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()), + ).returns(async (msl: MarketSideLiquidity, _opts: GenerateOptimizedOrdersOpts) => { + throw new Error(AggregationError.NoOptimalPath); + }).verifiable(TypeMoq.Times.exactly(1)); + + try { + await mockedMarketOpUtils.object.getMarketSellOrdersAsync( + ORDERS.slice(2, ORDERS.length), ORDERS[0].takerAssetAmount, DEFAULT_OPTS, + ); + expect.fail(`Call should have thrown "${AggregationError.NoOptimalPath}" but instead succeded`); + } catch (e) { + if (e.message !== AggregationError.NoOptimalPath) { + expect.fail(e); + } + } + mockedMarketOpUtils.verifyAll(); + }); + it('generates bridge orders with correct taker amount', async () => { const improvedOrdersResponse = await marketOperationUtils.getMarketSellOrdersAsync( // Pass in empty orders to prevent native orders from being used.