From 7bc9701c813308bdf8f12a59b08e7b5848a2176e Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Mon, 28 Sep 2020 13:15:44 -0700 Subject: [PATCH 01/32] initial implementation --- packages/asset-swapper/src/swap_quoter.ts | 29 ++------ .../src/utils/market_operation_utils/index.ts | 67 +++++++++++++++---- .../src/utils/market_operation_utils/types.ts | 12 ++++ 3 files changed, 74 insertions(+), 34 deletions(-) diff --git a/packages/asset-swapper/src/swap_quoter.ts b/packages/asset-swapper/src/swap_quoter.ts index 7b99bc55b4..43b9a59894 100644 --- a/packages/asset-swapper/src/swap_quoter.ts +++ b/packages/asset-swapper/src/swap_quoter.ts @@ -683,33 +683,18 @@ export class SwapQuoter { this.expiryBufferMs, ); - if ( - opts.rfqt && // This is an RFQT-enabled API request - opts.rfqt.intentOnFilling && // The requestor is asking for a firm quote - opts.rfqt.apiKey && - this._isApiKeyWhitelisted(opts.rfqt.apiKey) && // A valid API key was provided - sourceFilters.isAllowed(ERC20BridgeSource.Native) // Native liquidity is not excluded - ) { - if (!opts.rfqt.takerAddress || opts.rfqt.takerAddress === constants.NULL_ADDRESS) { - throw new Error('RFQ-T requests must specify a taker address'); + // If an API key was provided, but the key is not whitelisted, raise a warning and disable RFQ + if (opts.rfqt && opts.rfqt.apiKey && !this._isApiKeyWhitelisted(opts.rfqt.apiKey)) { + if (rfqtOptions && rfqtOptions.warningLogger) { + rfqtOptions.warningLogger({ + apiKey: opts.rfqt.apiKey, + }, 'Attempt at using an RFQ API key that is not whitelisted. Disabling RFQ for the request lifetime.'); } - orderBatchPromises.push( - quoteRequestor - .requestRfqtFirmQuotesAsync( - makerAssetData, - takerAssetData, - assetFillAmount, - marketOperation, - opts.rfqt, - ) - .then(firmQuotes => firmQuotes.map(quote => quote.signedOrder)), - ); + opts.rfqt = undefined; } const orderBatches: SignedOrder[][] = await Promise.all(orderBatchPromises); - const unsortedOrders: SignedOrder[] = orderBatches.reduce((_orders, batch) => _orders.concat(...batch), []); - const orders = sortingUtils.sortOrders(unsortedOrders); // if no native orders, pass in a dummy order for the sampler to have required metadata for sampling 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 c922496400..830f1603ed 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -232,6 +232,7 @@ export class MarketOperationUtils { ethToInputRate: ethToTakerAssetRate, rfqtIndicativeQuotes, twoHopQuotes, + quoteSourceFilters, }; } @@ -345,6 +346,7 @@ export class MarketOperationUtils { ethToInputRate: ethToMakerAssetRate, rfqtIndicativeQuotes, twoHopQuotes, + quoteSourceFilters, }; } @@ -362,15 +364,63 @@ export class MarketOperationUtils { opts?: Partial, ): Promise { const defaultOpts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, ...opts }; - const marketSideLiquidity = await this.getMarketSellLiquidityAsync(nativeOrders, takerAmount, defaultOpts); - const optimizerResult = await this._generateOptimizedOrdersAsync(marketSideLiquidity, { + const optimizerOpts: GenerateOptimizedOrdersOpts = { bridgeSlippage: defaultOpts.bridgeSlippage, maxFallbackSlippage: defaultOpts.maxFallbackSlippage, excludedSources: defaultOpts.excludedSources, feeSchedule: defaultOpts.feeSchedule, allowFallback: defaultOpts.allowFallback, shouldBatchBridgeOrders: defaultOpts.shouldBatchBridgeOrders, - }); + }; + + // Compute an optimized path for on-chain DEX and open-orderbook. This should not include RFQ liquidity. + const marketSideLiquidity = await this.getMarketSellLiquidityAsync(nativeOrders, takerAmount, defaultOpts); + let optimizerResult = await this._generateOptimizedOrdersAsync(marketSideLiquidity, optimizerOpts); + + // If RFQ liquidity is enabled, make a request to check RFQ liquidity + const { rfqt } = defaultOpts; + if (rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { + + // If we are making an indicative quote, make the RFQT request and then re-run the sampler if new orders come back. + if (rfqt.isIndicative) { + const indicativeQuotes = await getRfqtIndicativeQuotesAsync( + nativeOrders[0].makerAssetData, + nativeOrders[0].takerAssetData, + MarketOperation.Sell, + takerAmount, + defaultOpts, + ); + if (indicativeQuotes.length > 0) { + optimizerResult = await this._generateOptimizedOrdersAsync({ + ...marketSideLiquidity, + rfqtIndicativeQuotes: indicativeQuotes, + }, optimizerOpts); + } + } else { + // A firm quote is being requested. Ensure that `intentOnFilling` is enabled. + if (rfqt.intentOnFilling) { + + // Extra validation happens when requesting a firm quote, such as ensuring that the takerAddress + // is indeed valid. + if (!rfqt.takerAddress || rfqt.takerAddress === NULL_ADDRESS) { + throw new Error('RFQ-T requests must specify a taker address'); + } + const firmQuotes = await rfqt.quoteRequestor.requestRfqtFirmQuotesAsync( + nativeOrders[0].makerAssetData, + nativeOrders[0].takerAssetData, + takerAmount, + MarketOperation.Sell, + rfqt, + ); + if (firmQuotes.length > 0) { + optimizerResult = await this._generateOptimizedOrdersAsync({ + ...marketSideLiquidity, + nativeOrders: marketSideLiquidity.nativeOrders.concat(firmQuotes.map(quote => quote.signedOrder)), + }, optimizerOpts); + } + } + } + } // Compute Quote Report and return the results. let quoteReport: QuoteReport | undefined; @@ -494,6 +544,7 @@ export class MarketOperationUtils { inputToken: makerToken, outputToken: takerToken, twoHopQuotes: [], + quoteSourceFilters, }, { bridgeSlippage: _opts.bridgeSlippage, @@ -516,15 +567,7 @@ export class MarketOperationUtils { private async _generateOptimizedOrdersAsync( marketSideLiquidity: MarketSideLiquidity, - opts: { - runLimit?: number; - bridgeSlippage?: number; - maxFallbackSlippage?: number; - excludedSources?: ERC20BridgeSource[]; - feeSchedule?: FeeSchedule; - allowFallback?: boolean; - shouldBatchBridgeOrders?: boolean; - }, + opts: GenerateOptimizedOrdersOpts, ): Promise { const { inputToken, 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 5e7ea158fd..c80a722342 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -5,6 +5,7 @@ import { BigNumber } from '@0x/utils'; import { RfqtRequestOpts, SignedOrderWithFillableAmounts } from '../../types'; import { QuoteRequestor } from '../../utils/quote_requestor'; import { QuoteReport } from '../quote_report_generator'; +import { SourceFilters } from './source_filters'; /** * Order domain keys: chainId and exchange @@ -348,8 +349,19 @@ export interface MarketSideLiquidity { ethToInputRate: BigNumber; rfqtIndicativeQuotes: RFQTIndicativeQuote[]; twoHopQuotes: Array>; + quoteSourceFilters: SourceFilters; } export interface TokenAdjacencyGraph { [token: string]: string[]; } + +export interface GenerateOptimizedOrdersOpts { + runLimit?: number; + bridgeSlippage?: number; + maxFallbackSlippage?: number; + excludedSources?: ERC20BridgeSource[]; + feeSchedule?: FeeSchedule; + allowFallback?: boolean; + shouldBatchBridgeOrders?: boolean; +} From 90ad5eb6c4a3f4d9aa1c8fe2e98ba3f876396120 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Mon, 28 Sep 2020 13:57:30 -0700 Subject: [PATCH 02/32] refactor market side ops --- .../src/utils/market_operation_utils/index.ts | 168 +++++++++--------- 1 file changed, 82 insertions(+), 86 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 830f1603ed..2f7d1753c9 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -33,6 +33,7 @@ import { DexSample, ERC20BridgeSource, FeeSchedule, + GenerateOptimizedOrdersOpts, GetMarketOrdersOpts, MarketSideLiquidity, OptimizedMarketOrder, @@ -40,7 +41,6 @@ import { OptimizerResultWithReport, OrderDomain, TokenAdjacencyGraph, - GenerateOptimizedOrdersOpts, } from './types'; // tslint:disable:boolean-naming @@ -363,76 +363,7 @@ export class MarketOperationUtils { takerAmount: BigNumber, opts?: Partial, ): Promise { - const defaultOpts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, ...opts }; - const optimizerOpts: GenerateOptimizedOrdersOpts = { - bridgeSlippage: defaultOpts.bridgeSlippage, - maxFallbackSlippage: defaultOpts.maxFallbackSlippage, - excludedSources: defaultOpts.excludedSources, - feeSchedule: defaultOpts.feeSchedule, - allowFallback: defaultOpts.allowFallback, - shouldBatchBridgeOrders: defaultOpts.shouldBatchBridgeOrders, - }; - - // Compute an optimized path for on-chain DEX and open-orderbook. This should not include RFQ liquidity. - const marketSideLiquidity = await this.getMarketSellLiquidityAsync(nativeOrders, takerAmount, defaultOpts); - let optimizerResult = await this._generateOptimizedOrdersAsync(marketSideLiquidity, optimizerOpts); - - // If RFQ liquidity is enabled, make a request to check RFQ liquidity - const { rfqt } = defaultOpts; - if (rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { - - // If we are making an indicative quote, make the RFQT request and then re-run the sampler if new orders come back. - if (rfqt.isIndicative) { - const indicativeQuotes = await getRfqtIndicativeQuotesAsync( - nativeOrders[0].makerAssetData, - nativeOrders[0].takerAssetData, - MarketOperation.Sell, - takerAmount, - defaultOpts, - ); - if (indicativeQuotes.length > 0) { - optimizerResult = await this._generateOptimizedOrdersAsync({ - ...marketSideLiquidity, - rfqtIndicativeQuotes: indicativeQuotes, - }, optimizerOpts); - } - } else { - // A firm quote is being requested. Ensure that `intentOnFilling` is enabled. - if (rfqt.intentOnFilling) { - - // Extra validation happens when requesting a firm quote, such as ensuring that the takerAddress - // is indeed valid. - if (!rfqt.takerAddress || rfqt.takerAddress === NULL_ADDRESS) { - throw new Error('RFQ-T requests must specify a taker address'); - } - const firmQuotes = await rfqt.quoteRequestor.requestRfqtFirmQuotesAsync( - nativeOrders[0].makerAssetData, - nativeOrders[0].takerAssetData, - takerAmount, - MarketOperation.Sell, - rfqt, - ); - if (firmQuotes.length > 0) { - optimizerResult = await this._generateOptimizedOrdersAsync({ - ...marketSideLiquidity, - nativeOrders: marketSideLiquidity.nativeOrders.concat(firmQuotes.map(quote => quote.signedOrder)), - }, optimizerOpts); - } - } - } - } - - // Compute Quote Report and return the results. - let quoteReport: QuoteReport | undefined; - if (defaultOpts.shouldGenerateQuoteReport) { - quoteReport = MarketOperationUtils._computeQuoteReport( - nativeOrders, - defaultOpts.rfqt ? defaultOpts.rfqt.quoteRequestor : undefined, - marketSideLiquidity, - optimizerResult, - ); - } - return {...optimizerResult, quoteReport}; + return this._getMarketSideOrdersAsync(nativeOrders, takerAmount, MarketOperation.Sell, opts); } /** @@ -448,21 +379,7 @@ export class MarketOperationUtils { makerAmount: BigNumber, opts?: Partial, ): Promise { - const defaultOpts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, ...opts }; - const marketSideLiquidity = await this.getMarketBuyLiquidityAsync(nativeOrders, makerAmount, defaultOpts); - const optimizerResult = await this._generateOptimizedOrdersAsync(marketSideLiquidity, { - bridgeSlippage: defaultOpts.bridgeSlippage, - maxFallbackSlippage: defaultOpts.maxFallbackSlippage, - excludedSources: defaultOpts.excludedSources, - feeSchedule: defaultOpts.feeSchedule, - allowFallback: defaultOpts.allowFallback, - shouldBatchBridgeOrders: defaultOpts.shouldBatchBridgeOrders, - }); - let quoteReport: QuoteReport | undefined; - if (defaultOpts.shouldGenerateQuoteReport && defaultOpts.rfqt && defaultOpts.rfqt.quoteRequestor) { - quoteReport = MarketOperationUtils._computeQuoteReport(nativeOrders, defaultOpts.rfqt.quoteRequestor, marketSideLiquidity, optimizerResult); - } - return {...optimizerResult, quoteReport}; + return this._getMarketSideOrdersAsync(nativeOrders, makerAmount, MarketOperation.Buy, opts); } /** @@ -565,6 +482,85 @@ export class MarketOperationUtils { ); } + private async _getMarketSideOrdersAsync( + nativeOrders: SignedOrder[], + amount: BigNumber, + side: MarketOperation, + opts?: Partial, + ): Promise { + const defaultOpts = { ...DEFAULT_GET_MARKET_ORDERS_OPTS, ...opts }; + const optimizerOpts: GenerateOptimizedOrdersOpts = { + bridgeSlippage: defaultOpts.bridgeSlippage, + maxFallbackSlippage: defaultOpts.maxFallbackSlippage, + excludedSources: defaultOpts.excludedSources, + feeSchedule: defaultOpts.feeSchedule, + allowFallback: defaultOpts.allowFallback, + shouldBatchBridgeOrders: defaultOpts.shouldBatchBridgeOrders, + }; + + // Compute an optimized path for on-chain DEX and open-orderbook. This should not include RFQ liquidity. + const marketLiquidityFnAsync = side === MarketOperation.Sell ? this.getMarketSellLiquidityAsync.bind(this) : this.getMarketBuyLiquidityAsync.bind(this); + const marketSideLiquidity = await marketLiquidityFnAsync(nativeOrders, amount, defaultOpts); + let optimizerResult = await this._generateOptimizedOrdersAsync(marketSideLiquidity, optimizerOpts); + + // If RFQ liquidity is enabled, make a request to check RFQ liquidity + const { rfqt } = defaultOpts; + if (rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { + + // If we are making an indicative quote, make the RFQT request and then re-run the sampler if new orders come back. + if (rfqt.isIndicative) { + const indicativeQuotes = await getRfqtIndicativeQuotesAsync( + nativeOrders[0].makerAssetData, + nativeOrders[0].takerAssetData, + side, + amount, + defaultOpts, + ); + if (indicativeQuotes.length > 0) { + optimizerResult = await this._generateOptimizedOrdersAsync({ + ...marketSideLiquidity, + rfqtIndicativeQuotes: indicativeQuotes, + }, optimizerOpts); + } + } else { + // A firm quote is being requested. Ensure that `intentOnFilling` is enabled. + if (rfqt.intentOnFilling) { + + // Extra validation happens when requesting a firm quote, such as ensuring that the takerAddress + // is indeed valid. + if (!rfqt.takerAddress || rfqt.takerAddress === NULL_ADDRESS) { + throw new Error('RFQ-T requests must specify a taker address'); + } + const firmQuotes = await rfqt.quoteRequestor.requestRfqtFirmQuotesAsync( + nativeOrders[0].makerAssetData, + nativeOrders[0].takerAssetData, + amount, + side, + rfqt, + ); + if (firmQuotes.length > 0) { + optimizerResult = await this._generateOptimizedOrdersAsync({ + ...marketSideLiquidity, + nativeOrders: marketSideLiquidity.nativeOrders.concat(firmQuotes.map(quote => quote.signedOrder)), + }, optimizerOpts); + } + } + } + } + + // Compute Quote Report and return the results. + let quoteReport: QuoteReport | undefined; + if (defaultOpts.shouldGenerateQuoteReport) { + quoteReport = MarketOperationUtils._computeQuoteReport( + nativeOrders, + defaultOpts.rfqt ? defaultOpts.rfqt.quoteRequestor : undefined, + marketSideLiquidity, + optimizerResult, + ); + } + return {...optimizerResult, quoteReport}; + } + private async _generateOptimizedOrdersAsync( marketSideLiquidity: MarketSideLiquidity, opts: GenerateOptimizedOrdersOpts, From 507690f9db7080e2f5549719fa45a4eceb76c011 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Tue, 29 Sep 2020 00:59:14 -0700 Subject: [PATCH 03/32] refactor coding and avoid running indicative quotes when sampling market liquidity --- .../src/utils/market_operation_utils/index.ts | 51 +++++++++---------- 1 file changed, 24 insertions(+), 27 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 2f7d1753c9..f2d4ed8598 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -195,16 +195,6 @@ export class MarketOperationUtils { ), ); - const rfqtPromise = quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) - ? getRfqtIndicativeQuotesAsync( - nativeOrders[0].makerAssetData, - nativeOrders[0].takerAssetData, - MarketOperation.Sell, - takerAmount, - _opts, - ) - : Promise.resolve([]); - const offChainBalancerPromise = sampleBalancerOffChain ? this._sampler.getBalancerSellQuotesOffChainAsync(makerToken, takerToken, sampleAmounts) : Promise.resolve([]); @@ -215,10 +205,9 @@ export class MarketOperationUtils { const [ [orderFillableAmounts, ethToMakerAssetRate, ethToTakerAssetRate, dexQuotes, twoHopQuotes], - rfqtIndicativeQuotes, offChainBalancerQuotes, offChainBancorQuotes, - ] = await Promise.all([samplerPromise, rfqtPromise, offChainBalancerPromise, offChainBancorPromise]); + ] = await Promise.all([samplerPromise, offChainBalancerPromise, offChainBancorPromise]); return { side: MarketOperation.Sell, @@ -230,7 +219,7 @@ export class MarketOperationUtils { orderFillableAmounts, ethToOutputRate: ethToMakerAssetRate, ethToInputRate: ethToTakerAssetRate, - rfqtIndicativeQuotes, + rfqtIndicativeQuotes: [], twoHopQuotes, quoteSourceFilters, }; @@ -311,25 +300,14 @@ export class MarketOperationUtils { ), ); - const rfqtPromise = quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) - ? getRfqtIndicativeQuotesAsync( - nativeOrders[0].makerAssetData, - nativeOrders[0].takerAssetData, - MarketOperation.Buy, - makerAmount, - _opts, - ) - : Promise.resolve([]); - const offChainBalancerPromise = sampleBalancerOffChain ? this._sampler.getBalancerBuyQuotesOffChainAsync(makerToken, takerToken, sampleAmounts) : Promise.resolve([]); const [ [orderFillableAmounts, ethToMakerAssetRate, ethToTakerAssetRate, dexQuotes, twoHopQuotes], - rfqtIndicativeQuotes, offChainBalancerQuotes, - ] = await Promise.all([samplerPromise, rfqtPromise, offChainBalancerPromise]); + ] = await Promise.all([samplerPromise, offChainBalancerPromise]); // Attach the MultiBridge address to the sample fillData (dexQuotes.find(quotes => quotes[0] && quotes[0].source === ERC20BridgeSource.MultiBridge) || []).forEach( q => (q.fillData = { poolAddress: this._multiBridge }), @@ -344,7 +322,7 @@ export class MarketOperationUtils { orderFillableAmounts, ethToOutputRate: ethToTakerAssetRate, ethToInputRate: ethToMakerAssetRate, - rfqtIndicativeQuotes, + rfqtIndicativeQuotes: [], twoHopQuotes, quoteSourceFilters, }; @@ -501,7 +479,15 @@ export class MarketOperationUtils { // Compute an optimized path for on-chain DEX and open-orderbook. This should not include RFQ liquidity. const marketLiquidityFnAsync = side === MarketOperation.Sell ? this.getMarketSellLiquidityAsync.bind(this) : this.getMarketBuyLiquidityAsync.bind(this); const marketSideLiquidity = await marketLiquidityFnAsync(nativeOrders, amount, defaultOpts); - let optimizerResult = await this._generateOptimizedOrdersAsync(marketSideLiquidity, optimizerOpts); + let optimizerResult: OptimizerResult | undefined; + try { + 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 (e.message !== AggregationError.NoOptimalPath) { + throw e; + } + } // If RFQ liquidity is enabled, make a request to check RFQ liquidity const { rfqt } = defaultOpts; @@ -516,6 +502,7 @@ export class MarketOperationUtils { amount, defaultOpts, ); + // Re-run optimizer with the new indicative quote if (indicativeQuotes.length > 0) { optimizerResult = await this._generateOptimizedOrdersAsync({ ...marketSideLiquidity, @@ -539,15 +526,25 @@ export class MarketOperationUtils { rfqt, ); if (firmQuotes.length > 0) { + // Re-run optimizer with the new firm quote. This is the second and last time + // we run the optimized in a block of code. In this case, we don't catch a potential `NoOptimalPath` exception + // and we let it bubble up if it happens. optimizerResult = await this._generateOptimizedOrdersAsync({ ...marketSideLiquidity, nativeOrders: marketSideLiquidity.nativeOrders.concat(firmQuotes.map(quote => quote.signedOrder)), + orderFillableAmounts: marketSideLiquidity.orderFillableAmounts.concat(firmQuotes.map(quote => quote.signedOrder.takerAssetAmount)), }, optimizerOpts); } } } } + // At this point we should have at least one valid optimizer result, therefore we manually raise + // `NoOptimalPath` if no optimizer result was ever set. + if (optimizerResult === undefined) { + throw new Error(AggregationError.NoOptimalPath); + } + // Compute Quote Report and return the results. let quoteReport: QuoteReport | undefined; if (defaultOpts.shouldGenerateQuoteReport) { From eea63292f0fef9d339c5dafa218b77e5c773bb8f Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Tue, 29 Sep 2020 17:02:12 -0700 Subject: [PATCH 04/32] added initial RFQ tests --- .../src/utils/market_operation_utils/index.ts | 2 +- .../test/market_operation_utils_test.ts | 123 ++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) 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 f2d4ed8598..ecd6fda1f1 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -558,7 +558,7 @@ export class MarketOperationUtils { return {...optimizerResult, quoteReport}; } - private async _generateOptimizedOrdersAsync( + public async _generateOptimizedOrdersAsync( marketSideLiquidity: MarketSideLiquidity, opts: GenerateOptimizedOrdersOpts, ): Promise { diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 77261f3b6b..c17e4486ca 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -33,7 +33,12 @@ import { FillData, NativeFillData, OptimizedMarketOrder, + MarketSideLiquidity, + GenerateOptimizedOrdersOpts } 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'; const MAKER_TOKEN = randomAddress(); const TAKER_TOKEN = randomAddress(); @@ -644,6 +649,124 @@ describe('MarketOperationUtils tests', () => { } }); + it('getMarketSellOrdersAsync() optimizer will be called once only if RFQ if not defined', async () => { + const mockedMarketOpUtils = TypeMoq.Mock.ofType(MarketOperationUtils, TypeMoq.MockBehavior.Loose, false, MOCK_SAMPLER, contractAddresses, ORDER_DOMAIN); + mockedMarketOpUtils.callBase = true; + + // Ensure that `_generateOptimizedOrdersAsync` is only called once + mockedMarketOpUtils.setup( + m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny()) + ).returns( + 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)); + await mockedMarketOpUtils.object.getMarketSellOrdersAsync( + ORDERS, totalAssetAmount, + DEFAULT_OPTS, + ); + mockedMarketOpUtils.verifyAll(); + }); + + 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()) + ).returns( + 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)); + const results = await mockedMarketOpUtils.object.getMarketSellOrdersAsync( + ORDERS, totalAssetAmount, + { + ...DEFAULT_OPTS, + rfqt: { + isIndicative: false, + apiKey: 'foo', + takerAddress: randomAddress(), + intentOnFilling: true, + quoteRequestor: { + requestRfqtFirmQuotesAsync: requestor.object.requestRfqtFirmQuotesAsync, + } 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()); + + // 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) => { + numOrdersInCall.push(msl.nativeOrders.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(1, 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(numOrdersInCall.length).to.eql(2); + + // The first call to optimizer was without an RFQ order. + // The first call to optimizer was with an extra RFQ order. + expect(numOrdersInCall[0]).to.eql(2); + expect(numOrdersInCall[1]).to.eql(3); + }); + 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. From d60c8ddd5a4dab5f5d2915997413beac8eed1cc1 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Tue, 29 Sep 2020 22:25:24 -0700 Subject: [PATCH 05/32] 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(); From 708e34602b0197b8cdde87cef1990128047e77eb Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Tue, 29 Sep 2020 23:00:02 -0700 Subject: [PATCH 06/32] 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. From a24f01c90f748337887e69af51cdb5041a390165 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Tue, 29 Sep 2020 23:07:14 -0700 Subject: [PATCH 07/32] added a few fixed and added some comments: --- .../asset-swapper/src/utils/market_operation_utils/index.ts | 3 +++ packages/asset-swapper/test/market_operation_utils_test.ts | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) 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 216f9d30d8..5b58721660 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -622,6 +622,9 @@ export class MarketOperationUtils { // Re-run optimizer with the new firm quote. This is the second and last time // we run the optimized in a block of code. In this case, we don't catch a potential `NoOptimalPath` exception // and we let it bubble up if it happens. + // + // NOTE: as of now, we assume that RFQ orders are 100% fillable because these are trusted market makers, therefore + // we do not perform an extra check to get fillable taker amounts. optimizerResult = await this._generateOptimizedOrdersAsync({ ...marketSideLiquidity, nativeOrders: marketSideLiquidity.nativeOrders.concat(firmQuotes.map(quote => quote.signedOrder)), diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 84bf575972..228219f986 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -864,7 +864,7 @@ describe('MarketOperationUtils tests', () => { 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 () => { + it('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( From c13ffb2072f099bbe77beb9824dcbf2f2a64e559 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Wed, 30 Sep 2020 13:54:48 -0700 Subject: [PATCH 08/32] added linting and prettifying --- packages/asset-swapper/package.json | 2 +- packages/asset-swapper/src/swap_quoter.ts | 9 +- .../src/utils/market_operation_utils/index.ts | 49 ++-- .../utils/market_operation_utils/orders.ts | 7 +- .../src/utils/market_operation_utils/types.ts | 1 + .../test/market_operation_utils_test.ts | 216 ++++++++++-------- 6 files changed, 170 insertions(+), 114 deletions(-) diff --git a/packages/asset-swapper/package.json b/packages/asset-swapper/package.json index 293c1a5ee0..f4d5d2ad19 100644 --- a/packages/asset-swapper/package.json +++ b/packages/asset-swapper/package.json @@ -17,7 +17,7 @@ "compile": "sol-compiler", "lint": "tslint --format stylish --project . --exclude ./generated-wrappers/**/* --exclude ./test/generated-wrappers/**/* --exclude ./generated-artifacts/**/* --exclude ./test/generated-artifacts/**/* --exclude **/lib/**/* && yarn lint-contracts", "lint-contracts": "#solhint -c .solhint.json contracts/**/**/**/**/*.sol", - "prettier": "prettier '**/*.{ts,tsx,json,md}' --config ../../.prettierrc --ignore-path ../../.prettierignore", + "prettier": "prettier --write '**/*.{ts,tsx,json,md}' --config ../../.prettierrc --ignore-path ../../.prettierignore", "fix": "tslint --fix --format stylish --project . --exclude ./generated-wrappers/**/* --exclude ./generated-artifacts/**/* --exclude ./test/generated-wrappers/**/* --exclude ./test/generated-artifacts/**/* --exclude **/lib/**/* && yarn lint-contracts", "test": "yarn run_mocha", "rebuild_and_test": "run-s clean build test", diff --git a/packages/asset-swapper/src/swap_quoter.ts b/packages/asset-swapper/src/swap_quoter.ts index 43b9a59894..3de0e452bd 100644 --- a/packages/asset-swapper/src/swap_quoter.ts +++ b/packages/asset-swapper/src/swap_quoter.ts @@ -686,9 +686,12 @@ export class SwapQuoter { // If an API key was provided, but the key is not whitelisted, raise a warning and disable RFQ if (opts.rfqt && opts.rfqt.apiKey && !this._isApiKeyWhitelisted(opts.rfqt.apiKey)) { if (rfqtOptions && rfqtOptions.warningLogger) { - rfqtOptions.warningLogger({ - apiKey: opts.rfqt.apiKey, - }, 'Attempt at using an RFQ API key that is not whitelisted. Disabling RFQ for the request lifetime.'); + rfqtOptions.warningLogger( + { + apiKey: opts.rfqt.apiKey, + }, + 'Attempt at using an RFQ API key that is not whitelisted. Disabling RFQ for the request lifetime.', + ); } opts.rfqt = undefined; } 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 5b58721660..9f3bc91d57 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -85,7 +85,7 @@ export class MarketOperationUtils { marketSideLiquidity: MarketSideLiquidity, optimizerResult: OptimizerResult, ): QuoteReport { - const {side, dexQuotes, twoHopQuotes, orderFillableAmounts } = marketSideLiquidity; + const { side, dexQuotes, twoHopQuotes, orderFillableAmounts } = marketSideLiquidity; const { liquidityDelivered } = optimizerResult; return generateQuoteReport( side, @@ -568,7 +568,10 @@ export class MarketOperationUtils { }; // Compute an optimized path for on-chain DEX and open-orderbook. This should not include RFQ liquidity. - const marketLiquidityFnAsync = side === MarketOperation.Sell ? this.getMarketSellLiquidityAsync.bind(this) : this.getMarketBuyLiquidityAsync.bind(this); + const marketLiquidityFnAsync = + side === MarketOperation.Sell + ? this.getMarketSellLiquidityAsync.bind(this) + : this.getMarketBuyLiquidityAsync.bind(this); const marketSideLiquidity = await marketLiquidityFnAsync(nativeOrders, amount, defaultOpts); let optimizerResult: OptimizerResult | undefined; try { @@ -585,27 +588,28 @@ export class MarketOperationUtils { // If RFQ liquidity is enabled, make a request to check RFQ liquidity const { rfqt } = defaultOpts; if (rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { - // If we are making an indicative quote, make the RFQT request and then re-run the sampler if new orders come back. if (rfqt.isIndicative) { const indicativeQuotes = await getRfqtIndicativeQuotesAsync( - nativeOrders[0].makerAssetData, - nativeOrders[0].takerAssetData, - side, - amount, - defaultOpts, + nativeOrders[0].makerAssetData, + nativeOrders[0].takerAssetData, + side, + amount, + defaultOpts, ); // Re-run optimizer with the new indicative quote if (indicativeQuotes.length > 0) { - optimizerResult = await this._generateOptimizedOrdersAsync({ - ...marketSideLiquidity, - rfqtIndicativeQuotes: indicativeQuotes, - }, optimizerOpts); + optimizerResult = await this._generateOptimizedOrdersAsync( + { + ...marketSideLiquidity, + rfqtIndicativeQuotes: indicativeQuotes, + }, + optimizerOpts, + ); } } else { // A firm quote is being requested. Ensure that `intentOnFilling` is enabled. if (rfqt.intentOnFilling) { - // Extra validation happens when requesting a firm quote, such as ensuring that the takerAddress // is indeed valid. if (!rfqt.takerAddress || rfqt.takerAddress === NULL_ADDRESS) { @@ -625,11 +629,18 @@ export class MarketOperationUtils { // // NOTE: as of now, we assume that RFQ orders are 100% fillable because these are trusted market makers, therefore // we do not perform an extra check to get fillable taker amounts. - optimizerResult = await this._generateOptimizedOrdersAsync({ - ...marketSideLiquidity, - nativeOrders: marketSideLiquidity.nativeOrders.concat(firmQuotes.map(quote => quote.signedOrder)), - orderFillableAmounts: marketSideLiquidity.orderFillableAmounts.concat(firmQuotes.map(quote => quote.signedOrder.takerAssetAmount)), - }, optimizerOpts); + optimizerResult = await this._generateOptimizedOrdersAsync( + { + ...marketSideLiquidity, + nativeOrders: marketSideLiquidity.nativeOrders.concat( + firmQuotes.map(quote => quote.signedOrder), + ), + orderFillableAmounts: marketSideLiquidity.orderFillableAmounts.concat( + firmQuotes.map(quote => quote.signedOrder.takerAssetAmount), + ), + }, + optimizerOpts, + ); } } } @@ -651,7 +662,7 @@ export class MarketOperationUtils { optimizerResult, ); } - return {...optimizerResult, quoteReport}; + return { ...optimizerResult, quoteReport }; } } 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 0cbb725153..ee062b06cf 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/orders.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/orders.ts @@ -121,10 +121,13 @@ 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`); + throw new Error( + `Number of orders was ${orders.length} but fillable amounts was ${ + fillableAmounts.length + }. This should never happen`, + ); } return orders 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 c80a722342..9a7bfc3dfc 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -5,6 +5,7 @@ import { BigNumber } from '@0x/utils'; import { RfqtRequestOpts, SignedOrderWithFillableAmounts } from '../../types'; import { QuoteRequestor } from '../../utils/quote_requestor'; import { QuoteReport } from '../quote_report_generator'; + import { SourceFilters } from './source_filters'; /** diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 228219f986..d82d5fc93f 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -1,3 +1,4 @@ +// tslint:disable: no-unbound-method import { getContractAddressesForChainOrThrow } from '@0x/contract-addresses'; import { assertRoughlyEquals, @@ -15,8 +16,6 @@ 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'; @@ -29,7 +28,6 @@ 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 { AggregationError, DexSample, @@ -38,10 +36,8 @@ import { GenerateOptimizedOrdersOpts, MarketSideLiquidity, NativeFillData, - OptimizedMarketOrder + OptimizedMarketOrder, } from '../src/utils/market_operation_utils/types'; -import { quoteRequestorHttpClient } from '../src/utils/quote_requestor'; -import { IReturnsResult } from 'typemoq/_all'; const MAKER_TOKEN = randomAddress(); const TAKER_TOKEN = randomAddress(); @@ -66,7 +62,11 @@ 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 { + 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(), @@ -76,13 +76,15 @@ describe('MarketOperationUtils tests', () => { ]; 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) + 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); + requestor + .setup(r => r.requestRfqtIndicativeQuotesAsync(...args)) + .returns(async () => results) + .verifiable(verifiable); } return requestor; } @@ -674,53 +676,58 @@ describe('MarketOperationUtils tests', () => { }); it('getMarketSellOrdersAsync() optimizer will be called once only if RFQ if not defined', async () => { - const mockedMarketOpUtils = TypeMoq.Mock.ofType(MarketOperationUtils, TypeMoq.MockBehavior.Loose, false, MOCK_SAMPLER, contractAddresses, ORDER_DOMAIN); + const mockedMarketOpUtils = TypeMoq.Mock.ofType( + MarketOperationUtils, + TypeMoq.MockBehavior.Loose, + false, + MOCK_SAMPLER, + contractAddresses, + ORDER_DOMAIN, + ); mockedMarketOpUtils.callBase = true; // Ensure that `_generateOptimizedOrdersAsync` is only called once - mockedMarketOpUtils.setup( - m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny()), - ).returns( - async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b), - ).verifiable(TypeMoq.Times.once()); + mockedMarketOpUtils + .setup(m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(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)); - await mockedMarketOpUtils.object.getMarketSellOrdersAsync( - ORDERS, totalAssetAmount, - DEFAULT_OPTS, - ); + await mockedMarketOpUtils.object.getMarketSellOrdersAsync(ORDERS, totalAssetAmount, DEFAULT_OPTS); mockedMarketOpUtils.verifyAll(); }); it('getMarketSellOrdersAsync() will not rerun the optimizer if no orders are returned', async () => { - // Ensure that `_generateOptimizedOrdersAsync` is only called once - const mockedMarketOpUtils = TypeMoq.Mock.ofType(MarketOperationUtils, TypeMoq.MockBehavior.Loose, false, MOCK_SAMPLER, contractAddresses, ORDER_DOMAIN); + 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 (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b), - ).verifiable(TypeMoq.Times.once()); + mockedMarketOpUtils + .setup(m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(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)); - await mockedMarketOpUtils.object.getMarketSellOrdersAsync( - ORDERS, totalAssetAmount, - { - ...DEFAULT_OPTS, - rfqt: { - isIndicative: false, - apiKey: 'foo', - takerAddress: randomAddress(), - intentOnFilling: true, - quoteRequestor: { - requestRfqtFirmQuotesAsync: requestor.object.requestRfqtFirmQuotesAsync, - } as any, - }, + await mockedMarketOpUtils.object.getMarketSellOrdersAsync(ORDERS, totalAssetAmount, { + ...DEFAULT_OPTS, + rfqt: { + isIndicative: false, + apiKey: 'foo', + takerAddress: randomAddress(), + intentOnFilling: true, + quoteRequestor: { + requestRfqtFirmQuotesAsync: requestor.object.requestRfqtFirmQuotesAsync, + } as any, }, - ); + }); mockedMarketOpUtils.verifyAll(); requestor.verifyAll(); }); @@ -731,22 +738,28 @@ describe('MarketOperationUtils tests', () => { const numOrdersInCall: number[] = []; const numIndicativeQuotesInCall: number[] = []; - const mockedMarketOpUtils = TypeMoq.Mock.ofType(MarketOperationUtils, TypeMoq.MockBehavior.Loose, false, MOCK_SAMPLER, contractAddresses, ORDER_DOMAIN); + 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); - numIndicativeQuotesInCall.push(msl.rfqtIndicativeQuotes.length); - }) - .returns( - async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b), - ) - .verifiable(TypeMoq.Times.exactly(2)); + mockedMarketOpUtils + .setup(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, + ORDERS.slice(2, ORDERS.length), + totalAssetAmount, { ...DEFAULT_OPTS, rfqt: { @@ -782,21 +795,27 @@ describe('MarketOperationUtils tests', () => { // TODO: Ensure fillable amounts increase too const numOrdersInCall: number[] = []; - const mockedMarketOpUtils = TypeMoq.Mock.ofType(MarketOperationUtils, TypeMoq.MockBehavior.Loose, false, MOCK_SAMPLER, contractAddresses, ORDER_DOMAIN); + 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), - ) - .verifiable(TypeMoq.Times.exactly(2)); + 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)) + .verifiable(TypeMoq.Times.exactly(2)); const totalAssetAmount = ORDERS.map(o => o.takerAssetAmount).reduce((a, b) => a.plus(b)); await mockedMarketOpUtils.object.getMarketSellOrdersAsync( - ORDERS.slice(1, ORDERS.length), totalAssetAmount, + ORDERS.slice(1, ORDERS.length), + totalAssetAmount, { ...DEFAULT_OPTS, rfqt: { @@ -825,25 +844,34 @@ describe('MarketOperationUtils tests', () => { 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); + 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)); + 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, + ORDERS.slice(2, ORDERS.length), + totalAssetAmount, { ...DEFAULT_OPTS, rfqt: { @@ -865,17 +893,27 @@ describe('MarketOperationUtils tests', () => { }); it('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); + 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)); + 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, + ORDERS.slice(2, ORDERS.length), + ORDERS[0].takerAssetAmount, + DEFAULT_OPTS, ); expect.fail(`Call should have thrown "${AggregationError.NoOptimalPath}" but instead succeded`); } catch (e) { From 9e42dce5c4d6246a7da1c0c2763137d26e0894c4 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Thu, 1 Oct 2020 14:28:36 -0700 Subject: [PATCH 09/32] added initial implementation --- .../src/utils/market_operation_utils/index.ts | 2 + .../src/utils/quote_requestor.ts | 64 ++++++++++++++----- .../test/market_operation_utils_test.ts | 4 +- .../test/quote_requestor_test.ts | 16 +++++ 4 files changed, 68 insertions(+), 18 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 0ae164ca06..bcd82dcc55 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -65,6 +65,7 @@ export async function getRfqtIndicativeQuotesAsync( takerAssetData, assetFillAmount, marketOperation, + undefined, opts.rfqt, ); } else { @@ -620,6 +621,7 @@ export class MarketOperationUtils { nativeOrders[0].takerAssetData, amount, side, + undefined, rfqt, ); if (firmQuotes.length > 0) { diff --git a/packages/asset-swapper/src/utils/quote_requestor.ts b/packages/asset-swapper/src/utils/quote_requestor.ts index cd34d0d07a..ae92690838 100644 --- a/packages/asset-swapper/src/utils/quote_requestor.ts +++ b/packages/asset-swapper/src/utils/quote_requestor.ts @@ -1,6 +1,7 @@ import { schemas, SchemaValidator } from '@0x/json-schemas'; import { assetDataUtils, orderCalculationUtils, SignedOrder } from '@0x/order-utils'; import { RFQTFirmQuote, RFQTIndicativeQuote, TakerRequest } from '@0x/quote-server'; +import { TakerRequestQueryParams } from '@0x/quote-server/lib/src/types'; import { ERC20AssetData } from '@0x/types'; import { BigNumber, logUtils } from '@0x/utils'; import Axios, { AxiosInstance } from 'axios'; @@ -113,6 +114,39 @@ export class QuoteRequestor { private readonly _schemaValidator: SchemaValidator = new SchemaValidator(); private readonly _orderSignatureToMakerUri: { [orderSignature: string]: string } = {}; + public static makeQueryParameters( + takerAddress: string, + marketOperation: MarketOperation, + makerAssetData: string, + takerAssetData: string, + assetFillAmount: BigNumber, + comparisonPrice?: BigNumber | undefined, + ): TakerRequestQueryParams { + + const {buyAmountBaseUnits, sellAmountBaseUnits, ...rest } = inferQueryParams(marketOperation, makerAssetData, takerAssetData, assetFillAmount); + const requestParamsWithBigNumbers: Pick = { + takerAddress, + comparisonPrice: comparisonPrice === undefined ? undefined : comparisonPrice.toString(), + ...rest, + }; + + // convert BigNumbers to strings + // so they are digestible by axios + if (sellAmountBaseUnits) { + return { + ...requestParamsWithBigNumbers, + sellAmountBaseUnits: sellAmountBaseUnits.toString(), + }; + } else if (buyAmountBaseUnits) { + return { + ...requestParamsWithBigNumbers, + buyAmountBaseUnits: buyAmountBaseUnits.toString(), + }; + } else { + throw new Error('Neither "buyAmountBaseUnits" or "sellAmountBaseUnits" were defined'); + } + } + constructor( private readonly _rfqtAssetOfferings: RfqtMakerAssetOfferings, private readonly _warningLogger: LogFunction = (obj, msg) => @@ -127,6 +161,7 @@ export class QuoteRequestor { takerAssetData: string, assetFillAmount: BigNumber, marketOperation: MarketOperation, + comparisonPrice: BigNumber | undefined, options: RfqtRequestOpts, ): Promise { const _opts: RfqtRequestOpts = { ...constants.DEFAULT_RFQT_REQUEST_OPTS, ...options }; @@ -145,6 +180,7 @@ export class QuoteRequestor { takerAssetData, assetFillAmount, marketOperation, + comparisonPrice, _opts, 'firm', ); @@ -215,6 +251,7 @@ export class QuoteRequestor { takerAssetData: string, assetFillAmount: BigNumber, marketOperation: MarketOperation, + comparisonPrice: BigNumber | undefined, options: RfqtRequestOpts, ): Promise { const _opts: RfqtRequestOpts = { ...constants.DEFAULT_RFQT_REQUEST_OPTS, ...options }; @@ -233,6 +270,7 @@ export class QuoteRequestor { takerAssetData, assetFillAmount, marketOperation, + comparisonPrice, _opts, 'indicative', ); @@ -333,6 +371,7 @@ export class QuoteRequestor { takerAssetData: string, assetFillAmount: BigNumber, marketOperation: MarketOperation, + comparisonPrice: BigNumber | undefined, options: RfqtRequestOpts, quoteType: 'firm' | 'indicative', ): Promise> { @@ -343,23 +382,14 @@ export class QuoteRequestor { this._makerSupportsPair(url, makerAssetData, takerAssetData) && !rfqMakerBlacklist.isMakerBlacklisted(url) ) { - const requestParamsWithBigNumbers = { - takerAddress: options.takerAddress, - ...inferQueryParams(marketOperation, makerAssetData, takerAssetData, assetFillAmount), - }; - - // convert BigNumbers to strings - // so they are digestible by axios - const requestParams = { - ...requestParamsWithBigNumbers, - sellAmountBaseUnits: requestParamsWithBigNumbers.sellAmountBaseUnits - ? requestParamsWithBigNumbers.sellAmountBaseUnits.toString() - : undefined, - buyAmountBaseUnits: requestParamsWithBigNumbers.buyAmountBaseUnits - ? requestParamsWithBigNumbers.buyAmountBaseUnits.toString() - : undefined, - }; - + const requestParams = QuoteRequestor.makeQueryParameters( + options.takerAddress, + marketOperation, + makerAssetData, + takerAssetData, + assetFillAmount, + comparisonPrice, + ); const partialLogEntry = { url, quoteType, requestParams }; const timeBeforeAwait = Date.now(); const maxResponseTimeMs = diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index d82d5fc93f..1d45ffe5d4 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -67,7 +67,8 @@ describe('MarketOperationUtils tests', () => { results: SignedOrder[], verifiable: TypeMoq.Times, ): TypeMoq.IMock { - const args: [any, any, any, any, any] = [ + const args: [any, any, any, any, any, any] = [ + TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), @@ -460,6 +461,7 @@ describe('MarketOperationUtils tests', () => { TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), + TypeMoq.It.isAny(), ), ) .returns(() => Promise.resolve([])) diff --git a/packages/asset-swapper/test/quote_requestor_test.ts b/packages/asset-swapper/test/quote_requestor_test.ts index afdd3e0fc6..38460fd8ac 100644 --- a/packages/asset-swapper/test/quote_requestor_test.ts +++ b/packages/asset-swapper/test/quote_requestor_test.ts @@ -1,3 +1,4 @@ +import { randomAddress } from '@0x/contracts-test-utils'; import { tokenUtils } from '@0x/dev-utils'; import { assetDataUtils } from '@0x/order-utils'; import { StatusCodes } from '@0x/types'; @@ -174,6 +175,7 @@ describe('QuoteRequestor', async () => { takerAssetData, new BigNumber(10000), MarketOperation.Sell, + undefined, { apiKey, takerAddress, @@ -189,6 +191,18 @@ describe('QuoteRequestor', async () => { }); }); describe('requestRfqtIndicativeQuotesAsync for Indicative quotes', async () => { + + it('should optionally accept a "comparisonPrice" parameter', async () => { + const response = QuoteRequestor.makeQueryParameters( + otherToken1, + MarketOperation.Sell, + makerAssetData, + takerAssetData, + new BigNumber(1000), + new BigNumber(300.2), + ); + expect(response.comparisonPrice).to.eql('300.2'); + }); it('should return successful RFQT requests', async () => { const takerAddress = '0xd209925defc99488e3afff1174e48b4fa628302a'; const apiKey = 'my-ko0l-api-key'; @@ -276,6 +290,7 @@ describe('QuoteRequestor', async () => { takerAssetData, new BigNumber(10000), MarketOperation.Sell, + undefined, { apiKey, takerAddress, @@ -326,6 +341,7 @@ describe('QuoteRequestor', async () => { takerAssetData, new BigNumber(10000), MarketOperation.Buy, + undefined, { apiKey, takerAddress, From 1588c1f362f32591b96fa2b9d434482ef2812b1e Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Fri, 2 Oct 2020 15:04:50 -0700 Subject: [PATCH 10/32] Added initial unit tests and implementation --- .../src/utils/market_operation_utils/index.ts | 17 ++- .../test/market_operation_utils_test.ts | 102 ++++++++++++++++++ 2 files changed, 117 insertions(+), 2 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 bcd82dcc55..e1be29f8d2 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -57,6 +57,7 @@ export async function getRfqtIndicativeQuotesAsync( takerAssetData: string, marketOperation: MarketOperation, assetFillAmount: BigNumber, + comparisonPrice: BigNumber | undefined, opts: Partial, ): Promise { if (opts.rfqt && opts.rfqt.isIndicative === true && opts.rfqt.quoteRequestor) { @@ -65,7 +66,7 @@ export async function getRfqtIndicativeQuotesAsync( takerAssetData, assetFillAmount, marketOperation, - undefined, + comparisonPrice, opts.rfqt, ); } else { @@ -589,6 +590,17 @@ export class MarketOperationUtils { // If RFQ liquidity is enabled, make a request to check RFQ liquidity const { rfqt } = _opts; if (rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { + + // Calculate a suggested price. For now, this is simply the overall price of the aggregation. + let comparisonPrice: BigNumber | undefined; + if (optimizerResult) { + const totalMakerAmount = BigNumber.sum(...optimizerResult.optimizedOrders.map(order => order.makerAssetAmount)); + const totalTakerAmount = BigNumber.sum(...optimizerResult.optimizedOrders.map(order => order.takerAssetAmount)); + if (totalTakerAmount.gt(0)) { + comparisonPrice = totalMakerAmount.div(totalTakerAmount); + } + } + // If we are making an indicative quote, make the RFQT request and then re-run the sampler if new orders come back. if (rfqt.isIndicative) { const indicativeQuotes = await getRfqtIndicativeQuotesAsync( @@ -596,6 +608,7 @@ export class MarketOperationUtils { nativeOrders[0].takerAssetData, side, amount, + comparisonPrice, _opts, ); // Re-run optimizer with the new indicative quote @@ -621,7 +634,7 @@ export class MarketOperationUtils { nativeOrders[0].takerAssetData, amount, side, - undefined, + comparisonPrice, rfqt, ); if (firmQuotes.length > 0) { diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 1d45ffe5d4..cb6c815021 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -28,6 +28,7 @@ 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 { AggregationError, DexSample, @@ -471,6 +472,7 @@ describe('MarketOperationUtils tests', () => { TAKER_ASSET_DATA, MarketOperation.Sell, new BigNumber('100e18'), + undefined, { rfqt: { quoteRequestor: requestor.object, ...partialRfqt }, }, @@ -699,6 +701,106 @@ describe('MarketOperationUtils tests', () => { mockedMarketOpUtils.verifyAll(); }); + it('optimizer will send in a comparison price to RFQ providers', async () => { + + // Set up mocked quote requestor, will return an order that is better + // than the best of the orders. + const mockedQuoteRequestor = TypeMoq.Mock.ofType( + QuoteRequestor, + TypeMoq.MockBehavior.Loose, + false, + {}, + ); + + let requestedComparisonPrice: BigNumber | undefined; + mockedQuoteRequestor.setup( + mqr => mqr.requestRfqtFirmQuotesAsync( + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + ) + ).callback(( + _makerAssetData: string, + _takerAssetData: string, + _assetFillAmount: BigNumber, + _marketOperation: MarketOperation, + comparisonPrice: BigNumber | undefined, + _options: RfqtRequestOpts, + ) => { + requestedComparisonPrice = comparisonPrice; + }).returns(async () => { + return [ + { + signedOrder: createOrder({ + makerAssetData: MAKER_ASSET_DATA, + takerAssetData: TAKER_ASSET_DATA, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(321, 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), + }), + }, + ]; + }); + + // Set up sampler, will only return 1 on-chain order + const mockedMarketOpUtils = TypeMoq.Mock.ofType( + MarketOperationUtils, + TypeMoq.MockBehavior.Loose, + false, + MOCK_SAMPLER, + contractAddresses, + ORDER_DOMAIN, + ); + mockedMarketOpUtils.callBase = true; + mockedMarketOpUtils.setup( + mou => mou.getMarketSellLiquidityAsync( + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + ) + ).returns(async () => { + return { + dexQuotes: [], + ethToInputRate: Web3Wrapper.toBaseUnitAmount(1, 18), + ethToOutputRate: Web3Wrapper.toBaseUnitAmount(1, 18), + inputAmount: Web3Wrapper.toBaseUnitAmount(1, 18), + inputToken: MAKER_TOKEN, + outputToken: TAKER_TOKEN, + nativeOrders: [ + createOrder({ + makerAssetData: MAKER_ASSET_DATA, + takerAssetData: TAKER_ASSET_DATA, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(320, 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), + }), + ], + orderFillableAmounts: [Web3Wrapper.toBaseUnitAmount(1, 18)], + rfqtIndicativeQuotes: [], + side: MarketOperation.Sell, + twoHopQuotes: [], + quoteSourceFilters: new SourceFilters(), + }; + }) + const result = await mockedMarketOpUtils.object.getMarketSellOrdersAsync(ORDERS, Web3Wrapper.toBaseUnitAmount(1, 18), { + ...DEFAULT_OPTS, + rfqt: { + isIndicative: false, + apiKey: 'foo', + takerAddress: randomAddress(), + intentOnFilling: true, + quoteRequestor: { + requestRfqtFirmQuotesAsync: mockedQuoteRequestor.object.requestRfqtFirmQuotesAsync, + } as any, + } + }); + expect(result.optimizedOrders.length).to.eql(1); + expect(requestedComparisonPrice!.toString()).to.eql("320"); + expect(result.optimizedOrders[0].makerAssetAmount.toString()).to.eql('321000000000000000000'); + expect(result.optimizedOrders[0].takerAssetAmount.toString()).to.eql('1000000000000000000'); + }); + it('getMarketSellOrdersAsync() will not rerun the optimizer if no orders are returned', async () => { // Ensure that `_generateOptimizedOrdersAsync` is only called once const mockedMarketOpUtils = TypeMoq.Mock.ofType( From 83c942ad8c1d643258bede47887ae9f5df5979d5 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Mon, 5 Oct 2020 17:20:43 -0700 Subject: [PATCH 11/32] Added fix for decimals --- .../contracts/src/NativeOrderSampler.sol | 14 +++++++++ packages/asset-swapper/src/types.ts | 2 ++ .../src/utils/market_operation_utils/index.ts | 22 ++++++++++--- .../market_operation_utils/multihop_utils.ts | 3 +- .../sampler_operations.ts | 9 ++++++ .../src/utils/market_operation_utils/types.ts | 2 ++ .../contracts/native_order_sampler_test.ts | 31 +++++++++++++++++++ .../test/market_operation_utils_test.ts | 16 +++++++--- 8 files changed, 88 insertions(+), 11 deletions(-) diff --git a/packages/asset-swapper/contracts/src/NativeOrderSampler.sol b/packages/asset-swapper/contracts/src/NativeOrderSampler.sol index 3ba797906b..c1a36cc0b4 100644 --- a/packages/asset-swapper/contracts/src/NativeOrderSampler.sol +++ b/packages/asset-swapper/contracts/src/NativeOrderSampler.sol @@ -20,6 +20,7 @@ pragma solidity ^0.5.9; pragma experimental ABIEncoderV2; import "@0x/contracts-erc20/contracts/src/interfaces/IERC20Token.sol"; +import "@0x/contracts-erc20/contracts/src/LibERC20Token.sol"; import "@0x/contracts-exchange/contracts/src/interfaces/IExchange.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; @@ -36,6 +37,19 @@ contract NativeOrderSampler { /// @dev Gas limit for calls to `getOrderFillableTakerAmount()`. uint256 constant internal DEFAULT_CALL_GAS = 200e3; // 200k + function getTokenDecimals( + address makerTokenAddress, + address takerTokenAddress + ) + public + view + returns (uint256, uint256) + { + uint256 fromTokenDecimals = LibERC20Token.decimals(makerTokenAddress); + uint256 toTokenDecimals = LibERC20Token.decimals(takerTokenAddress); + return (fromTokenDecimals, toTokenDecimals); + } + /// @dev Queries the fillable taker asset amounts of native orders. /// Effectively ignores orders that have empty signatures or /// maker/taker asset amounts (returning 0). diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index 6ec4357215..5b49152627 100644 --- a/packages/asset-swapper/src/types.ts +++ b/packages/asset-swapper/src/types.ts @@ -382,3 +382,5 @@ export interface SamplerOverrides { overrides: GethCallOverrides; block: BlockParam; } + +export type Omit = Pick>; \ No newline at end of file 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 e1be29f8d2..307b7614c0 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -28,6 +28,7 @@ import { import { findOptimalPathAsync } from './path_optimizer'; import { DexOrderSampler, getSampleAmounts } from './sampler'; import { SourceFilters } from './source_filters'; +import { Omit } from '../../types'; import { AggregationError, DexSample, @@ -41,6 +42,7 @@ import { OrderDomain, TokenAdjacencyGraph, } from './types'; +import { Web3Wrapper } from '@0x/dev-utils'; // tslint:disable:boolean-naming @@ -153,6 +155,7 @@ export class MarketOperationUtils { // Call the sampler contract. const samplerPromise = this._sampler.executeAsync( + this._sampler.getTokenDecimals(makerToken, takerToken), // Get native order fillable amounts. this._sampler.getOrderFillableTakerAmounts(nativeOrders, this.contractAddresses.exchange), // Get ETH -> maker token price. @@ -205,11 +208,12 @@ export class MarketOperationUtils { : Promise.resolve([]); const [ - [orderFillableAmounts, ethToMakerAssetRate, ethToTakerAssetRate, dexQuotes, twoHopQuotes], + [tokenDecimals, orderFillableAmounts, ethToMakerAssetRate, ethToTakerAssetRate, dexQuotes, twoHopQuotes], offChainBalancerQuotes, offChainBancorQuotes, ] = await Promise.all([samplerPromise, offChainBalancerPromise, offChainBancorPromise]); + const [makerTokenDecimals, takerTokenDecimals] = tokenDecimals; return { side: MarketOperation.Sell, inputAmount: takerAmount, @@ -223,6 +227,8 @@ export class MarketOperationUtils { rfqtIndicativeQuotes: [], twoHopQuotes, quoteSourceFilters, + makerTokenDecimals: makerTokenDecimals.toNumber(), + takerTokenDecimals: takerTokenDecimals.toNumber(), }; } @@ -259,6 +265,7 @@ export class MarketOperationUtils { // Call the sampler contract. const samplerPromise = this._sampler.executeAsync( + this._sampler.getTokenDecimals(makerToken, takerToken), // Get native order fillable amounts. this._sampler.getOrderFillableMakerAmounts(nativeOrders, this.contractAddresses.exchange), // Get ETH -> makerToken token price. @@ -306,13 +313,14 @@ export class MarketOperationUtils { : Promise.resolve([]); const [ - [orderFillableAmounts, ethToMakerAssetRate, ethToTakerAssetRate, dexQuotes, twoHopQuotes], + [tokenDecimals, orderFillableAmounts, ethToMakerAssetRate, ethToTakerAssetRate, dexQuotes, twoHopQuotes], offChainBalancerQuotes, ] = await Promise.all([samplerPromise, offChainBalancerPromise]); // Attach the MultiBridge address to the sample fillData (dexQuotes.find(quotes => quotes[0] && quotes[0].source === ERC20BridgeSource.MultiBridge) || []).forEach( q => (q.fillData = { poolAddress: this._multiBridge }), ); + const [makerTokenDecimals, takerTokenDecimals] = tokenDecimals; return { side: MarketOperation.Buy, inputAmount: makerAmount, @@ -326,6 +334,8 @@ export class MarketOperationUtils { rfqtIndicativeQuotes: [], twoHopQuotes, quoteSourceFilters, + makerTokenDecimals: makerTokenDecimals.toNumber(), + takerTokenDecimals: takerTokenDecimals.toNumber(), }; } @@ -462,7 +472,7 @@ export class MarketOperationUtils { } public async _generateOptimizedOrdersAsync( - marketSideLiquidity: MarketSideLiquidity, + marketSideLiquidity: Omit, opts: GenerateOptimizedOrdersOpts, ): Promise { const { @@ -596,8 +606,10 @@ export class MarketOperationUtils { if (optimizerResult) { const totalMakerAmount = BigNumber.sum(...optimizerResult.optimizedOrders.map(order => order.makerAssetAmount)); const totalTakerAmount = BigNumber.sum(...optimizerResult.optimizedOrders.map(order => order.takerAssetAmount)); - if (totalTakerAmount.gt(0)) { - comparisonPrice = totalMakerAmount.div(totalTakerAmount); + if (totalMakerAmount.gt(0)) { + const totalMakerAmountUnitAmount = Web3Wrapper.toUnitAmount(totalMakerAmount, marketSideLiquidity.makerTokenDecimals); + const totalTakerAmountUnitAmount = Web3Wrapper.toUnitAmount(totalTakerAmount, marketSideLiquidity.takerTokenDecimals); + comparisonPrice = totalTakerAmountUnitAmount.div(totalMakerAmountUnitAmount); } } diff --git a/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts b/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts index 28c5043d62..717d30f6f5 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts @@ -4,6 +4,7 @@ import * as _ from 'lodash'; import { ZERO_AMOUNT } from './constants'; import { getTwoHopAdjustedRate } from './fills'; import { DexSample, FeeSchedule, MarketSideLiquidity, MultiHopFillData, TokenAdjacencyGraph } from './types'; +import { Omit } from '../../types'; /** * Given a token pair, returns the intermediate tokens to consider for two-hop routes. @@ -34,7 +35,7 @@ export function getIntermediateTokens( * Returns the best two-hop quote and the fee-adjusted rate of that quote. */ export function getBestTwoHopQuote( - marketSideLiquidity: MarketSideLiquidity, + marketSideLiquidity: Omit, feeSchedule?: FeeSchedule, ): { quote: DexSample | undefined; adjustedRate: BigNumber } { const { side, inputAmount, ethToOutputRate, twoHopQuotes } = marketSideLiquidity; diff --git a/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts b/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts index 9ef979049c..668f4f1782 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/sampler_operations.ts @@ -88,6 +88,15 @@ export class SamplerOperations { return this._bancorService; } + public getTokenDecimals(makerTokenAddress: string, takerTokenAddress: string): BatchedOperation { + return new SamplerContractOperation({ + source: ERC20BridgeSource.Native, + contract: this._samplerContract, + function: this._samplerContract.getTokenDecimals, + params: [makerTokenAddress, takerTokenAddress], + }); + } + public getOrderFillableTakerAmounts(orders: SignedOrder[], exchangeAddress: string): BatchedOperation { return new SamplerContractOperation({ source: ERC20BridgeSource.Native, 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 9a7bfc3dfc..c59426864a 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/types.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/types.ts @@ -351,6 +351,8 @@ export interface MarketSideLiquidity { rfqtIndicativeQuotes: RFQTIndicativeQuote[]; twoHopQuotes: Array>; quoteSourceFilters: SourceFilters; + makerTokenDecimals: number; + takerTokenDecimals: number; } export interface TokenAdjacencyGraph { diff --git a/packages/asset-swapper/test/contracts/native_order_sampler_test.ts b/packages/asset-swapper/test/contracts/native_order_sampler_test.ts index 021ce4e32e..93afd052a3 100644 --- a/packages/asset-swapper/test/contracts/native_order_sampler_test.ts +++ b/packages/asset-swapper/test/contracts/native_order_sampler_test.ts @@ -1,3 +1,4 @@ +import { artifacts as erc20Artifacts, DummyERC20TokenContract } from '@0x/contracts-erc20'; import { assertIntegerRoughlyEquals, blockchainTests, @@ -130,6 +131,36 @@ blockchainTests.resets('NativeOrderSampler contract', env => { .awaitTransactionSuccessAsync(); } + describe('getTokenDecimals()', () => { + it('correctly returns the token balances', async () => { + const newMakerToken = await DummyERC20TokenContract.deployFrom0xArtifactAsync( + erc20Artifacts.DummyERC20Token, + env.provider, + env.txDefaults, + artifacts, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + new BigNumber(18), + constants.DUMMY_TOKEN_TOTAL_SUPPLY, + ); + const newTakerToken = await DummyERC20TokenContract.deployFrom0xArtifactAsync( + erc20Artifacts.DummyERC20Token, + env.provider, + env.txDefaults, + artifacts, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + new BigNumber(6), + constants.DUMMY_TOKEN_TOTAL_SUPPLY, + ); + const [makerDecimals, takerDecimals] = await testContract + .getTokenDecimals(newMakerToken.address, newTakerToken.address) + .callAsync(); + expect(makerDecimals.toString()).to.eql('18'); + expect(takerDecimals.toString()).to.eql('6'); + }); + }); + describe('getOrderFillableTakerAmount()', () => { it('returns the full amount for a fully funded order', async () => { const order = createOrder(); diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index cb6c815021..4fd4814acf 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -385,6 +385,10 @@ describe('MarketOperationUtils tests', () => { }; const DEFAULT_OPS = { + getTokenDecimals(_makerAddress: string, _takerAddress: string): BigNumber[] { + const result = new BigNumber(18); + return [result, result]; + }, getOrderFillableTakerAmounts(orders: SignedOrder[]): BigNumber[] { return orders.map(o => o.takerAssetAmount); }, @@ -737,7 +741,7 @@ describe('MarketOperationUtils tests', () => { signedOrder: createOrder({ makerAssetData: MAKER_ASSET_DATA, takerAssetData: TAKER_ASSET_DATA, - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(321, 18), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(321, 6), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), }), }, @@ -764,7 +768,7 @@ describe('MarketOperationUtils tests', () => { return { dexQuotes: [], ethToInputRate: Web3Wrapper.toBaseUnitAmount(1, 18), - ethToOutputRate: Web3Wrapper.toBaseUnitAmount(1, 18), + ethToOutputRate: Web3Wrapper.toBaseUnitAmount(1, 6), inputAmount: Web3Wrapper.toBaseUnitAmount(1, 18), inputToken: MAKER_TOKEN, outputToken: TAKER_TOKEN, @@ -772,7 +776,7 @@ describe('MarketOperationUtils tests', () => { createOrder({ makerAssetData: MAKER_ASSET_DATA, takerAssetData: TAKER_ASSET_DATA, - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(320, 18), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(320, 6), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), }), ], @@ -781,6 +785,8 @@ describe('MarketOperationUtils tests', () => { side: MarketOperation.Sell, twoHopQuotes: [], quoteSourceFilters: new SourceFilters(), + makerTokenDecimals: 6, + takerTokenDecimals: 18, }; }) const result = await mockedMarketOpUtils.object.getMarketSellOrdersAsync(ORDERS, Web3Wrapper.toBaseUnitAmount(1, 18), { @@ -796,8 +802,8 @@ describe('MarketOperationUtils tests', () => { } }); expect(result.optimizedOrders.length).to.eql(1); - expect(requestedComparisonPrice!.toString()).to.eql("320"); - expect(result.optimizedOrders[0].makerAssetAmount.toString()).to.eql('321000000000000000000'); + expect(requestedComparisonPrice!.toString()).to.eql("0.003125"); + expect(result.optimizedOrders[0].makerAssetAmount.toString()).to.eql('321000000'); expect(result.optimizedOrders[0].takerAssetAmount.toString()).to.eql('1000000000000000000'); }); From efef5f122fdcd759c211196dd1bc6301815ecd54 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Mon, 5 Oct 2020 17:24:23 -0700 Subject: [PATCH 12/32] added fix --- packages/asset-swapper/src/types.ts | 2 +- .../src/utils/market_operation_utils/index.ts | 5 ++--- .../src/utils/market_operation_utils/multihop_utils.ts | 3 ++- .../asset-swapper/test/market_operation_utils_test.ts | 10 +++++----- packages/asset-swapper/test/quote_requestor_test.ts | 1 - 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index 5b49152627..a62457c3a1 100644 --- a/packages/asset-swapper/src/types.ts +++ b/packages/asset-swapper/src/types.ts @@ -383,4 +383,4 @@ export interface SamplerOverrides { block: BlockParam; } -export type Omit = Pick>; \ No newline at end of file +export type Omit = Pick>; 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 307b7614c0..87d973a4aa 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -1,10 +1,11 @@ import { ContractAddresses } from '@0x/contract-addresses'; +import { Web3Wrapper } from '@0x/dev-utils'; import { RFQTIndicativeQuote } from '@0x/quote-server'; import { SignedOrder } from '@0x/types'; import { BigNumber, NULL_ADDRESS } from '@0x/utils'; import * as _ from 'lodash'; -import { MarketOperation } from '../../types'; +import { MarketOperation, Omit } from '../../types'; import { QuoteRequestor } from '../quote_requestor'; import { generateQuoteReport, QuoteReport } from './../quote_report_generator'; @@ -28,7 +29,6 @@ import { import { findOptimalPathAsync } from './path_optimizer'; import { DexOrderSampler, getSampleAmounts } from './sampler'; import { SourceFilters } from './source_filters'; -import { Omit } from '../../types'; import { AggregationError, DexSample, @@ -42,7 +42,6 @@ import { OrderDomain, TokenAdjacencyGraph, } from './types'; -import { Web3Wrapper } from '@0x/dev-utils'; // tslint:disable:boolean-naming diff --git a/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts b/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts index 717d30f6f5..fc94a1626a 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/multihop_utils.ts @@ -1,10 +1,11 @@ import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; +import { Omit } from '../../types'; + import { ZERO_AMOUNT } from './constants'; import { getTwoHopAdjustedRate } from './fills'; import { DexSample, FeeSchedule, MarketSideLiquidity, MultiHopFillData, TokenAdjacencyGraph } from './types'; -import { Omit } from '../../types'; /** * Given a token pair, returns the intermediate tokens to consider for two-hop routes. diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 4fd4814acf..6db172e8e5 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -725,7 +725,7 @@ describe('MarketOperationUtils tests', () => { TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), - ) + ), ).callback(( _makerAssetData: string, _takerAssetData: string, @@ -763,7 +763,7 @@ describe('MarketOperationUtils tests', () => { TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny(), - ) + ), ).returns(async () => { return { dexQuotes: [], @@ -788,7 +788,7 @@ describe('MarketOperationUtils tests', () => { makerTokenDecimals: 6, takerTokenDecimals: 18, }; - }) + }); const result = await mockedMarketOpUtils.object.getMarketSellOrdersAsync(ORDERS, Web3Wrapper.toBaseUnitAmount(1, 18), { ...DEFAULT_OPTS, rfqt: { @@ -799,10 +799,10 @@ describe('MarketOperationUtils tests', () => { quoteRequestor: { requestRfqtFirmQuotesAsync: mockedQuoteRequestor.object.requestRfqtFirmQuotesAsync, } as any, - } + }, }); expect(result.optimizedOrders.length).to.eql(1); - expect(requestedComparisonPrice!.toString()).to.eql("0.003125"); + expect(requestedComparisonPrice!.toString()).to.eql('0.003125'); expect(result.optimizedOrders[0].makerAssetAmount.toString()).to.eql('321000000'); expect(result.optimizedOrders[0].takerAssetAmount.toString()).to.eql('1000000000000000000'); }); diff --git a/packages/asset-swapper/test/quote_requestor_test.ts b/packages/asset-swapper/test/quote_requestor_test.ts index 38460fd8ac..f9fbf02fdb 100644 --- a/packages/asset-swapper/test/quote_requestor_test.ts +++ b/packages/asset-swapper/test/quote_requestor_test.ts @@ -1,4 +1,3 @@ -import { randomAddress } from '@0x/contracts-test-utils'; import { tokenUtils } from '@0x/dev-utils'; import { assetDataUtils } from '@0x/order-utils'; import { StatusCodes } from '@0x/types'; From dde76a4dbb782463d1f58801a108c40afa481fed Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Mon, 5 Oct 2020 17:36:52 -0700 Subject: [PATCH 13/32] Mark decimal places --- .../src/utils/market_operation_utils/constants.ts | 1 + .../asset-swapper/src/utils/market_operation_utils/index.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) 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 1b65b5538a..4ee43377c8 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/constants.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/constants.ts @@ -221,3 +221,4 @@ export const ONE_HOUR_IN_SECONDS = 60 * 60; export const ONE_SECOND_MS = 1000; export const NULL_BYTES = '0x'; export const NULL_ADDRESS = '0x0000000000000000000000000000000000000000'; +export const COMPARISON_PRICE_DECIMALS = 5; 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 87d973a4aa..e1af8102b0 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -11,6 +11,7 @@ import { QuoteRequestor } from '../quote_requestor'; import { generateQuoteReport, QuoteReport } from './../quote_report_generator'; import { BUY_SOURCE_FILTER, + COMPARISON_PRICE_DECIMALS, DEFAULT_GET_MARKET_ORDERS_OPTS, FEE_QUOTE_SOURCES, ONE_ETHER, @@ -608,7 +609,7 @@ export class MarketOperationUtils { if (totalMakerAmount.gt(0)) { const totalMakerAmountUnitAmount = Web3Wrapper.toUnitAmount(totalMakerAmount, marketSideLiquidity.makerTokenDecimals); const totalTakerAmountUnitAmount = Web3Wrapper.toUnitAmount(totalTakerAmount, marketSideLiquidity.takerTokenDecimals); - comparisonPrice = totalTakerAmountUnitAmount.div(totalMakerAmountUnitAmount); + comparisonPrice = totalMakerAmountUnitAmount.div(totalTakerAmountUnitAmount).decimalPlaces(COMPARISON_PRICE_DECIMALS); } } From 221de054f41804d0cf916a032f2edeff7ce4979f Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Wed, 7 Oct 2020 19:34:12 -0700 Subject: [PATCH 14/32] fixes a few issues with the typing --- packages/asset-swapper/src/types.ts | 9 +++------ packages/asset-swapper/test/quote_requestor_test.ts | 13 +++++++------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index 18ad80bd17..bcdfd9c3e2 100644 --- a/packages/asset-swapper/src/types.ts +++ b/packages/asset-swapper/src/types.ts @@ -1,4 +1,5 @@ import { BlockParam, ContractAddresses, GethCallOverrides } from '@0x/contract-wrappers'; +import { TakerRequestQueryParams } from '@0x/quote-server'; import { SignedOrder } from '@0x/types'; import { BigNumber } from '@0x/utils'; @@ -357,9 +358,7 @@ export enum OrderPrunerPermittedFeeTypes { export interface MockedRfqtFirmQuoteResponse { endpoint: string; requestApiKey: string; - requestParams: { - [key: string]: string | undefined; - }; + requestParams: TakerRequestQueryParams, responseData: any; responseCode: number; } @@ -370,9 +369,7 @@ export interface MockedRfqtFirmQuoteResponse { export interface MockedRfqtIndicativeQuoteResponse { endpoint: string; requestApiKey: string; - requestParams: { - [key: string]: string | undefined; - }; + requestParams: TakerRequestQueryParams, responseData: any; responseCode: number; } diff --git a/packages/asset-swapper/test/quote_requestor_test.ts b/packages/asset-swapper/test/quote_requestor_test.ts index f9fbf02fdb..ad1062799a 100644 --- a/packages/asset-swapper/test/quote_requestor_test.ts +++ b/packages/asset-swapper/test/quote_requestor_test.ts @@ -1,5 +1,6 @@ import { tokenUtils } from '@0x/dev-utils'; import { assetDataUtils } from '@0x/order-utils'; +import { TakerRequestQueryParams } from '@0x/quote-server'; import { StatusCodes } from '@0x/types'; import { BigNumber } from '@0x/utils'; import * as chai from 'chai'; @@ -35,11 +36,11 @@ describe('QuoteRequestor', async () => { // Set up RFQT responses // tslint:disable-next-line:array-type const mockedRequests: MockedRfqtFirmQuoteResponse[] = []; - const expectedParams = { + const expectedParams: TakerRequestQueryParams = { sellTokenAddress: takerToken, buyTokenAddress: makerToken, sellAmountBaseUnits: '10000', - buyAmountBaseUnits: undefined, + comparisonPrice: undefined, takerAddress, }; // Successful response @@ -209,11 +210,11 @@ describe('QuoteRequestor', async () => { // Set up RFQT responses // tslint:disable-next-line:array-type const mockedRequests: MockedRfqtIndicativeQuoteResponse[] = []; - const expectedParams = { + const expectedParams: TakerRequestQueryParams = { sellTokenAddress: takerToken, buyTokenAddress: makerToken, sellAmountBaseUnits: '10000', - buyAmountBaseUnits: undefined, + comparisonPrice: undefined, takerAddress, }; // Successful response @@ -308,11 +309,11 @@ describe('QuoteRequestor', async () => { // Set up RFQT responses // tslint:disable-next-line:array-type const mockedRequests: MockedRfqtIndicativeQuoteResponse[] = []; - const expectedParams = { + const expectedParams: TakerRequestQueryParams = { sellTokenAddress: takerToken, buyTokenAddress: makerToken, buyAmountBaseUnits: '10000', - sellAmountBaseUnits: undefined, + comparisonPrice: undefined, takerAddress, }; // Successful response From a3ff406461a7bc48fa9b3a31044d87a05becd7d3 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Wed, 7 Oct 2020 19:37:38 -0700 Subject: [PATCH 15/32] update quote server --- packages/asset-swapper/package.json | 2 +- yarn.lock | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/asset-swapper/package.json b/packages/asset-swapper/package.json index 8673709871..e39444e0d5 100644 --- a/packages/asset-swapper/package.json +++ b/packages/asset-swapper/package.json @@ -64,7 +64,7 @@ "@0x/json-schemas": "^5.1.0", "@0x/order-utils": "^10.3.0", "@0x/orderbook": "^2.2.7", - "@0x/quote-server": "^2.0.2", + "@0x/quote-server": "^2.1.0", "@0x/types": "^3.2.0", "@0x/typescript-typings": "^5.1.1", "@0x/utils": "^5.5.1", diff --git a/yarn.lock b/yarn.lock index f5a89efc2b..5bcd9071c7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -748,9 +748,10 @@ uuid "^3.3.2" websocket "^1.0.29" -"@0x/quote-server@^2.0.2": - version "2.0.2" - resolved "https://registry.yarnpkg.com/@0x/quote-server/-/quote-server-2.0.2.tgz#60d0665c1cad378c9abb89b5491bdc55b4c8412c" +"@0x/quote-server@^2.1.0": + version "2.1.0" + resolved "https://registry.yarnpkg.com/@0x/quote-server/-/quote-server-2.1.0.tgz#48b4da703116bf373728789ae67dd647616e8332" + integrity sha512-b6zDzpF3KygXyRGOREpLklSObJXvhYVSuVVOXQToR94unkzJtY5wsNSC6aRGiKsgDevLA0kDuiz55Ym3UOSxhg== dependencies: "@0x/json-schemas" "^5.0.7" "@0x/order-utils" "^10.2.4" From f8df89b506c17ad96a7bc04870c78d719ae87248 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Wed, 7 Oct 2020 23:49:04 -0700 Subject: [PATCH 16/32] fixed broken package --- packages/asset-swapper/package.json | 2 +- packages/asset-swapper/src/utils/quote_requestor.ts | 2 +- yarn.lock | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/asset-swapper/package.json b/packages/asset-swapper/package.json index e39444e0d5..13a9f771fd 100644 --- a/packages/asset-swapper/package.json +++ b/packages/asset-swapper/package.json @@ -64,7 +64,7 @@ "@0x/json-schemas": "^5.1.0", "@0x/order-utils": "^10.3.0", "@0x/orderbook": "^2.2.7", - "@0x/quote-server": "^2.1.0", + "@0x/quote-server": "^3.1.0", "@0x/types": "^3.2.0", "@0x/typescript-typings": "^5.1.1", "@0x/utils": "^5.5.1", diff --git a/packages/asset-swapper/src/utils/quote_requestor.ts b/packages/asset-swapper/src/utils/quote_requestor.ts index c20880e401..ab10cf5cb0 100644 --- a/packages/asset-swapper/src/utils/quote_requestor.ts +++ b/packages/asset-swapper/src/utils/quote_requestor.ts @@ -1,7 +1,7 @@ import { schemas, SchemaValidator } from '@0x/json-schemas'; import { assetDataUtils, orderCalculationUtils, SignedOrder } from '@0x/order-utils'; import { RFQTFirmQuote, RFQTIndicativeQuote, TakerRequest } from '@0x/quote-server'; -import { TakerRequestQueryParams } from '@0x/quote-server/lib/src/types'; +import { TakerRequestQueryParams } from '@0x/quote-server'; import { ERC20AssetData } from '@0x/types'; import { BigNumber } from '@0x/utils'; import Axios, { AxiosInstance } from 'axios'; diff --git a/yarn.lock b/yarn.lock index 5bcd9071c7..2636e2d491 100644 --- a/yarn.lock +++ b/yarn.lock @@ -748,10 +748,10 @@ uuid "^3.3.2" websocket "^1.0.29" -"@0x/quote-server@^2.1.0": - version "2.1.0" - resolved "https://registry.yarnpkg.com/@0x/quote-server/-/quote-server-2.1.0.tgz#48b4da703116bf373728789ae67dd647616e8332" - integrity sha512-b6zDzpF3KygXyRGOREpLklSObJXvhYVSuVVOXQToR94unkzJtY5wsNSC6aRGiKsgDevLA0kDuiz55Ym3UOSxhg== +"@0x/quote-server@^3.1.0": + version "3.1.0" + resolved "https://registry.yarnpkg.com/@0x/quote-server/-/quote-server-3.1.0.tgz#ba5c0de9f88fedfd522ec1ef608dd8eebb868509" + integrity sha512-o9n7wE9XmV/YMjAcIt3EJMnc0xony8VhqNtO7dGAREi/WQxJBlNAHNZxu4wQ0wV03wroH58eJTOpn4fk+kuXqQ== dependencies: "@0x/json-schemas" "^5.0.7" "@0x/order-utils" "^10.2.4" From e3b92d2c8b9aa6b3ec59de2418bb3f00680dd745 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Mon, 12 Oct 2020 18:01:22 -0400 Subject: [PATCH 17/32] Fix linter errors --- packages/asset-swapper/src/types.ts | 4 ++-- packages/asset-swapper/src/utils/quote_requestor.ts | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index bcdfd9c3e2..4eaf8adde2 100644 --- a/packages/asset-swapper/src/types.ts +++ b/packages/asset-swapper/src/types.ts @@ -358,7 +358,7 @@ export enum OrderPrunerPermittedFeeTypes { export interface MockedRfqtFirmQuoteResponse { endpoint: string; requestApiKey: string; - requestParams: TakerRequestQueryParams, + requestParams: TakerRequestQueryParams; responseData: any; responseCode: number; } @@ -369,7 +369,7 @@ export interface MockedRfqtFirmQuoteResponse { export interface MockedRfqtIndicativeQuoteResponse { endpoint: string; requestApiKey: string; - requestParams: TakerRequestQueryParams, + requestParams: TakerRequestQueryParams; responseData: any; responseCode: number; } diff --git a/packages/asset-swapper/src/utils/quote_requestor.ts b/packages/asset-swapper/src/utils/quote_requestor.ts index 0107985f27..6f1931bb55 100644 --- a/packages/asset-swapper/src/utils/quote_requestor.ts +++ b/packages/asset-swapper/src/utils/quote_requestor.ts @@ -1,7 +1,6 @@ import { schemas, SchemaValidator } from '@0x/json-schemas'; import { assetDataUtils, orderCalculationUtils, SignedOrder } from '@0x/order-utils'; -import { RFQTFirmQuote, RFQTIndicativeQuote, TakerRequest } from '@0x/quote-server'; -import { TakerRequestQueryParams } from '@0x/quote-server'; +import { RFQTFirmQuote, RFQTIndicativeQuote, TakerRequest, TakerRequestQueryParams } from '@0x/quote-server'; import { ERC20AssetData } from '@0x/types'; import { BigNumber } from '@0x/utils'; import Axios, { AxiosInstance } from 'axios'; From 7030572f1df1fbeb3c58419ec46ebfb118032552 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Wed, 14 Oct 2020 11:58:18 -0400 Subject: [PATCH 18/32] Stop referring to GetMarketOrdersOpts.shouldBat... Because it doesn't exist on that type. --- packages/asset-swapper/src/utils/market_operation_utils/index.ts | 1 - 1 file changed, 1 deletion(-) 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 4dd1e5f0eb..5d9dfd4b45 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -578,7 +578,6 @@ export class MarketOperationUtils { feeSchedule: _opts.feeSchedule, allowFallback: _opts.allowFallback, exchangeProxyOverhead: _opts.exchangeProxyOverhead, - shouldBatchBridgeOrders: _opts.shouldBatchBridgeOrders, }; // Compute an optimized path for on-chain DEX and open-orderbook. This should not include RFQ liquidity. From 2db52c6983bb4b7be7ba6214730d6f9278bd95b1 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Wed, 14 Oct 2020 12:00:47 -0400 Subject: [PATCH 19/32] Fixed linter error ERROR: 793:24 no-unnecessary-type-assertion This assertion is unnecessary since it does not change the type of the expression. --- packages/asset-swapper/test/market_operation_utils_test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 867e4e253f..5f8269c303 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -790,6 +790,7 @@ describe('MarketOperationUtils tests', () => { }, }); expect(result.optimizedOrders.length).to.eql(1); + // tslint:disable-next-line:no-unnecessary-type-assertion expect(requestedComparisonPrice!.toString()).to.eql('320'); expect(result.optimizedOrders[0].makerAssetAmount.toString()).to.eql('321000000'); expect(result.optimizedOrders[0].takerAssetAmount.toString()).to.eql('1000000000000000000'); From 05b25c6229dba83a246cf8f3b89de55decce4c68 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Wed, 14 Oct 2020 12:01:10 -0400 Subject: [PATCH 20/32] Ran prettier --- .../src/utils/market_operation_utils/index.ts | 23 ++- .../src/utils/quote_requestor.ts | 13 +- .../test/market_operation_utils_test.ts | 157 +++++++++--------- .../test/quote_requestor_test.ts | 1 - 4 files changed, 106 insertions(+), 88 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 5d9dfd4b45..962ea180c6 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -601,16 +601,27 @@ export class MarketOperationUtils { // If RFQ liquidity is enabled, make a request to check RFQ liquidity const { rfqt } = _opts; if (rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { - // Calculate a suggested price. For now, this is simply the overall price of the aggregation. let comparisonPrice: BigNumber | undefined; if (optimizerResult) { - const totalMakerAmount = BigNumber.sum(...optimizerResult.optimizedOrders.map(order => order.makerAssetAmount)); - const totalTakerAmount = BigNumber.sum(...optimizerResult.optimizedOrders.map(order => order.takerAssetAmount)); + const totalMakerAmount = BigNumber.sum( + ...optimizerResult.optimizedOrders.map(order => order.makerAssetAmount), + ); + const totalTakerAmount = BigNumber.sum( + ...optimizerResult.optimizedOrders.map(order => order.takerAssetAmount), + ); if (totalMakerAmount.gt(0)) { - const totalMakerAmountUnitAmount = Web3Wrapper.toUnitAmount(totalMakerAmount, marketSideLiquidity.makerTokenDecimals); - const totalTakerAmountUnitAmount = Web3Wrapper.toUnitAmount(totalTakerAmount, marketSideLiquidity.takerTokenDecimals); - comparisonPrice = totalMakerAmountUnitAmount.div(totalTakerAmountUnitAmount).decimalPlaces(COMPARISON_PRICE_DECIMALS); + const totalMakerAmountUnitAmount = Web3Wrapper.toUnitAmount( + totalMakerAmount, + marketSideLiquidity.makerTokenDecimals, + ); + const totalTakerAmountUnitAmount = Web3Wrapper.toUnitAmount( + totalTakerAmount, + marketSideLiquidity.takerTokenDecimals, + ); + comparisonPrice = totalMakerAmountUnitAmount + .div(totalTakerAmountUnitAmount) + .decimalPlaces(COMPARISON_PRICE_DECIMALS); } } diff --git a/packages/asset-swapper/src/utils/quote_requestor.ts b/packages/asset-swapper/src/utils/quote_requestor.ts index 6f1931bb55..8dd01a51fb 100644 --- a/packages/asset-swapper/src/utils/quote_requestor.ts +++ b/packages/asset-swapper/src/utils/quote_requestor.ts @@ -119,9 +119,16 @@ export class QuoteRequestor { assetFillAmount: BigNumber, comparisonPrice?: BigNumber | undefined, ): TakerRequestQueryParams { - - const {buyAmountBaseUnits, sellAmountBaseUnits, ...rest } = inferQueryParams(marketOperation, makerAssetData, takerAssetData, assetFillAmount); - const requestParamsWithBigNumbers: Pick = { + const { buyAmountBaseUnits, sellAmountBaseUnits, ...rest } = inferQueryParams( + marketOperation, + makerAssetData, + takerAssetData, + assetFillAmount, + ); + const requestParamsWithBigNumbers: Pick< + TakerRequestQueryParams, + 'buyTokenAddress' | 'sellTokenAddress' | 'takerAddress' | 'comparisonPrice' + > = { takerAddress, comparisonPrice: comparisonPrice === undefined ? undefined : comparisonPrice.toString(), ...rest, diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 5f8269c303..07a9c96217 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -694,47 +694,46 @@ describe('MarketOperationUtils tests', () => { }); it('optimizer will send in a comparison price to RFQ providers', async () => { - // Set up mocked quote requestor, will return an order that is better // than the best of the orders. - const mockedQuoteRequestor = TypeMoq.Mock.ofType( - QuoteRequestor, - TypeMoq.MockBehavior.Loose, - false, - {}, - ); + const mockedQuoteRequestor = TypeMoq.Mock.ofType(QuoteRequestor, TypeMoq.MockBehavior.Loose, false, {}); let requestedComparisonPrice: BigNumber | undefined; - mockedQuoteRequestor.setup( - mqr => mqr.requestRfqtFirmQuotesAsync( - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - ), - ).callback(( - _makerAssetData: string, - _takerAssetData: string, - _assetFillAmount: BigNumber, - _marketOperation: MarketOperation, - comparisonPrice: BigNumber | undefined, - _options: RfqtRequestOpts, - ) => { - requestedComparisonPrice = comparisonPrice; - }).returns(async () => { - return [ - { - signedOrder: createOrder({ - makerAssetData: MAKER_ASSET_DATA, - takerAssetData: TAKER_ASSET_DATA, - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(321, 6), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), - }), + mockedQuoteRequestor + .setup(mqr => + mqr.requestRfqtFirmQuotesAsync( + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + ), + ) + .callback( + ( + _makerAssetData: string, + _takerAssetData: string, + _assetFillAmount: BigNumber, + _marketOperation: MarketOperation, + comparisonPrice: BigNumber | undefined, + _options: RfqtRequestOpts, + ) => { + requestedComparisonPrice = comparisonPrice; }, - ]; - }); + ) + .returns(async () => { + return [ + { + signedOrder: createOrder({ + makerAssetData: MAKER_ASSET_DATA, + takerAssetData: TAKER_ASSET_DATA, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(321, 6), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), + }), + }, + ]; + }); // Set up sampler, will only return 1 on-chain order const mockedMarketOpUtils = TypeMoq.Mock.ofType( @@ -746,49 +745,51 @@ describe('MarketOperationUtils tests', () => { ORDER_DOMAIN, ); mockedMarketOpUtils.callBase = true; - mockedMarketOpUtils.setup( - mou => mou.getMarketSellLiquidityAsync( - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - ), - ).returns(async () => { - return { - dexQuotes: [], - ethToInputRate: Web3Wrapper.toBaseUnitAmount(1, 18), - ethToOutputRate: Web3Wrapper.toBaseUnitAmount(1, 6), - inputAmount: Web3Wrapper.toBaseUnitAmount(1, 18), - inputToken: MAKER_TOKEN, - outputToken: TAKER_TOKEN, - nativeOrders: [ - createOrder({ - makerAssetData: MAKER_ASSET_DATA, - takerAssetData: TAKER_ASSET_DATA, - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(320, 6), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), - }), - ], - orderFillableAmounts: [Web3Wrapper.toBaseUnitAmount(1, 18)], - rfqtIndicativeQuotes: [], - side: MarketOperation.Sell, - twoHopQuotes: [], - quoteSourceFilters: new SourceFilters(), - makerTokenDecimals: 6, - takerTokenDecimals: 18, - }; - }); - const result = await mockedMarketOpUtils.object.getMarketSellOrdersAsync(ORDERS, Web3Wrapper.toBaseUnitAmount(1, 18), { - ...DEFAULT_OPTS, - rfqt: { - isIndicative: false, - apiKey: 'foo', - takerAddress: randomAddress(), - intentOnFilling: true, - quoteRequestor: { - requestRfqtFirmQuotesAsync: mockedQuoteRequestor.object.requestRfqtFirmQuotesAsync, - } as any, + mockedMarketOpUtils + .setup(mou => + mou.getMarketSellLiquidityAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()), + ) + .returns(async () => { + return { + dexQuotes: [], + ethToInputRate: Web3Wrapper.toBaseUnitAmount(1, 18), + ethToOutputRate: Web3Wrapper.toBaseUnitAmount(1, 6), + inputAmount: Web3Wrapper.toBaseUnitAmount(1, 18), + inputToken: MAKER_TOKEN, + outputToken: TAKER_TOKEN, + nativeOrders: [ + createOrder({ + makerAssetData: MAKER_ASSET_DATA, + takerAssetData: TAKER_ASSET_DATA, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(320, 6), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), + }), + ], + orderFillableAmounts: [Web3Wrapper.toBaseUnitAmount(1, 18)], + rfqtIndicativeQuotes: [], + side: MarketOperation.Sell, + twoHopQuotes: [], + quoteSourceFilters: new SourceFilters(), + makerTokenDecimals: 6, + takerTokenDecimals: 18, + }; + }); + const result = await mockedMarketOpUtils.object.getMarketSellOrdersAsync( + ORDERS, + Web3Wrapper.toBaseUnitAmount(1, 18), + { + ...DEFAULT_OPTS, + rfqt: { + isIndicative: false, + apiKey: 'foo', + takerAddress: randomAddress(), + intentOnFilling: true, + quoteRequestor: { + requestRfqtFirmQuotesAsync: mockedQuoteRequestor.object.requestRfqtFirmQuotesAsync, + } as any, + }, }, - }); + ); expect(result.optimizedOrders.length).to.eql(1); // tslint:disable-next-line:no-unnecessary-type-assertion expect(requestedComparisonPrice!.toString()).to.eql('320'); diff --git a/packages/asset-swapper/test/quote_requestor_test.ts b/packages/asset-swapper/test/quote_requestor_test.ts index ad1062799a..a30e2bd658 100644 --- a/packages/asset-swapper/test/quote_requestor_test.ts +++ b/packages/asset-swapper/test/quote_requestor_test.ts @@ -191,7 +191,6 @@ describe('QuoteRequestor', async () => { }); }); describe('requestRfqtIndicativeQuotesAsync for Indicative quotes', async () => { - it('should optionally accept a "comparisonPrice" parameter', async () => { const response = QuoteRequestor.makeQueryParameters( otherToken1, From 2b1a1ba0db1a750749623fd891fc34af274873cb Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Wed, 14 Oct 2020 13:21:14 -0400 Subject: [PATCH 21/32] Fix failing test-doc-generation --- packages/asset-swapper/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/asset-swapper/src/index.ts b/packages/asset-swapper/src/index.ts index 0c87279cc8..4f3e1b5b9d 100644 --- a/packages/asset-swapper/src/index.ts +++ b/packages/asset-swapper/src/index.ts @@ -18,7 +18,7 @@ export { SRAPollingOrderProviderOpts, SRAWebsocketOrderProviderOpts, } from '@0x/orderbook'; -export { RFQTFirmQuote, RFQTIndicativeQuote } from '@0x/quote-server'; +export { RFQTFirmQuote, RFQTIndicativeQuote, TakerRequestQueryParams } from '@0x/quote-server'; export { APIOrder, Asset, From 2cf31f05a1b402f5e8f7853c60dc760baa0c6b3b Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Wed, 14 Oct 2020 13:23:02 -0400 Subject: [PATCH 22/32] Drop `|undefined` from type of optional param Addresses review comment https://github.com/0xProject/0x-monorepo/pull/2720#discussion_r500453526 --- packages/asset-swapper/src/utils/quote_requestor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/asset-swapper/src/utils/quote_requestor.ts b/packages/asset-swapper/src/utils/quote_requestor.ts index 8dd01a51fb..1c87ca426c 100644 --- a/packages/asset-swapper/src/utils/quote_requestor.ts +++ b/packages/asset-swapper/src/utils/quote_requestor.ts @@ -117,7 +117,7 @@ export class QuoteRequestor { makerAssetData: string, takerAssetData: string, assetFillAmount: BigNumber, - comparisonPrice?: BigNumber | undefined, + comparisonPrice?: BigNumber, ): TakerRequestQueryParams { const { buyAmountBaseUnits, sellAmountBaseUnits, ...rest } = inferQueryParams( marketOperation, From c969a8652aa7f1b6f237d70f5f433cdcc4f7dfc3 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Thu, 15 Oct 2020 13:18:31 -0400 Subject: [PATCH 23/32] Reduce solidity compiler optimizer runs... ...in order to accomodate now-too-big contract TestERC20BridgeSampler, which imports the recently-added-to NativeOrderSampler.sol. Fixes test failing like: 1) erc20-bridge-sampler "before all" hook in "erc20-bridge-sampler": RuntimeError: VM Exception while processing transaction: out of gas --- packages/asset-swapper/compiler.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/asset-swapper/compiler.json b/packages/asset-swapper/compiler.json index 20f8637f21..c15d3cd1c6 100644 --- a/packages/asset-swapper/compiler.json +++ b/packages/asset-swapper/compiler.json @@ -8,7 +8,7 @@ "evmVersion": "istanbul", "optimizer": { "enabled": true, - "runs": 1000000, + "runs": 62500, "details": { "yul": true, "deduplicate": true, "cse": true, "constantOptimizer": true } }, "outputSelection": { From a8cbd1a16c5391ddaaa910e94fc122ff91b8d184 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Fri, 16 Oct 2020 14:47:37 -0700 Subject: [PATCH 24/32] apply PR feedback --- .../src/utils/quote_requestor.ts | 43 ++++++------------- 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/packages/asset-swapper/src/utils/quote_requestor.ts b/packages/asset-swapper/src/utils/quote_requestor.ts index 1c87ca426c..b88c20c2a5 100644 --- a/packages/asset-swapper/src/utils/quote_requestor.ts +++ b/packages/asset-swapper/src/utils/quote_requestor.ts @@ -44,29 +44,6 @@ function getTokenAddressOrThrow(assetData: string): string { throw new Error(`Decoded asset data (${JSON.stringify(decodedAssetData)}) does not contain a token address`); } -function inferQueryParams( - marketOperation: MarketOperation, - makerAssetData: string, - takerAssetData: string, - assetFillAmount: BigNumber, -): Pick { - if (marketOperation === MarketOperation.Buy) { - return { - buyTokenAddress: getTokenAddressOrThrow(makerAssetData), - sellTokenAddress: getTokenAddressOrThrow(takerAssetData), - buyAmountBaseUnits: assetFillAmount, - sellAmountBaseUnits: undefined, - }; - } else { - return { - buyTokenAddress: getTokenAddressOrThrow(makerAssetData), - sellTokenAddress: getTokenAddressOrThrow(takerAssetData), - sellAmountBaseUnits: assetFillAmount, - buyAmountBaseUnits: undefined, - }; - } -} - function hasExpectedAssetData( expectedMakerAssetData: string, expectedTakerAssetData: string, @@ -119,19 +96,25 @@ export class QuoteRequestor { assetFillAmount: BigNumber, comparisonPrice?: BigNumber, ): TakerRequestQueryParams { - const { buyAmountBaseUnits, sellAmountBaseUnits, ...rest } = inferQueryParams( - marketOperation, - makerAssetData, - takerAssetData, - assetFillAmount, - ); + const buyTokenAddress = getTokenAddressOrThrow(makerAssetData); + const sellTokenAddress = getTokenAddressOrThrow(takerAssetData); + const { buyAmountBaseUnits, sellAmountBaseUnits } = marketOperation === MarketOperation.Buy + ? { + buyAmountBaseUnits: assetFillAmount, + sellAmountBaseUnits: undefined, + } : { + sellAmountBaseUnits: assetFillAmount, + buyAmountBaseUnits: undefined, + }; + const requestParamsWithBigNumbers: Pick< TakerRequestQueryParams, 'buyTokenAddress' | 'sellTokenAddress' | 'takerAddress' | 'comparisonPrice' > = { takerAddress, comparisonPrice: comparisonPrice === undefined ? undefined : comparisonPrice.toString(), - ...rest, + buyTokenAddress, + sellTokenAddress, }; // convert BigNumbers to strings From 1fcab58e2f671dff41daa36841450718ca19146c Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Fri, 16 Oct 2020 14:49:25 -0700 Subject: [PATCH 25/32] cheap lint fix --- packages/asset-swapper/src/utils/quote_requestor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/asset-swapper/src/utils/quote_requestor.ts b/packages/asset-swapper/src/utils/quote_requestor.ts index b88c20c2a5..8916e7f5f3 100644 --- a/packages/asset-swapper/src/utils/quote_requestor.ts +++ b/packages/asset-swapper/src/utils/quote_requestor.ts @@ -1,6 +1,6 @@ import { schemas, SchemaValidator } from '@0x/json-schemas'; import { assetDataUtils, orderCalculationUtils, SignedOrder } from '@0x/order-utils'; -import { RFQTFirmQuote, RFQTIndicativeQuote, TakerRequest, TakerRequestQueryParams } from '@0x/quote-server'; +import { RFQTFirmQuote, RFQTIndicativeQuote, TakerRequestQueryParams } from '@0x/quote-server'; import { ERC20AssetData } from '@0x/types'; import { BigNumber } from '@0x/utils'; import Axios, { AxiosInstance } from 'axios'; From a03d0800b014410f2bb61f5f90f45d548fad725c Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Mon, 19 Oct 2020 16:58:45 -0700 Subject: [PATCH 26/32] Add changes for the feature flag --- packages/asset-swapper/src/constants.ts | 2 ++ packages/asset-swapper/src/swap_quoter.ts | 27 ++++++++++++++- .../src/utils/market_operation_utils/index.ts | 34 ++++++++++++++++--- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/packages/asset-swapper/src/constants.ts b/packages/asset-swapper/src/constants.ts index e68c605930..6b5bd1a792 100644 --- a/packages/asset-swapper/src/constants.ts +++ b/packages/asset-swapper/src/constants.ts @@ -122,3 +122,5 @@ export const constants = { DEFAULT_INFO_LOGGER, DEFAULT_WARNING_LOGGER, }; + +export const ENABLE_PRICE_AWARE_RFQ: boolean = false; diff --git a/packages/asset-swapper/src/swap_quoter.ts b/packages/asset-swapper/src/swap_quoter.ts index 3de0e452bd..0f55949545 100644 --- a/packages/asset-swapper/src/swap_quoter.ts +++ b/packages/asset-swapper/src/swap_quoter.ts @@ -8,7 +8,7 @@ import { BlockParamLiteral, SupportedProvider, ZeroExProvider } from 'ethereum-t import * as _ from 'lodash'; import { artifacts } from './artifacts'; -import { constants } from './constants'; +import { constants, ENABLE_PRICE_AWARE_RFQ } from './constants'; import { CalculateSwapQuoteOpts, LiquidityForTakerMakerAssetDataPair, @@ -696,6 +696,31 @@ export class SwapQuoter { opts.rfqt = undefined; } + if ( + !ENABLE_PRICE_AWARE_RFQ && // Price-aware RFQ is disabled. + opts.rfqt && // This is an RFQT-enabled API request + opts.rfqt.intentOnFilling && // The requestor is asking for a firm quote + opts.rfqt.apiKey && + this._isApiKeyWhitelisted(opts.rfqt.apiKey) && // A valid API key was provided + sourceFilters.isAllowed(ERC20BridgeSource.Native) // Native liquidity is not excluded + ) { + if (!opts.rfqt.takerAddress || opts.rfqt.takerAddress === constants.NULL_ADDRESS) { + throw new Error('RFQ-T requests must specify a taker address'); + } + orderBatchPromises.push( + quoteRequestor + .requestRfqtFirmQuotesAsync( + makerAssetData, + takerAssetData, + assetFillAmount, + marketOperation, + undefined, + opts.rfqt, + ) + .then(firmQuotes => firmQuotes.map(quote => quote.signedOrder)), + ); + } + const orderBatches: SignedOrder[][] = await Promise.all(orderBatchPromises); const unsortedOrders: SignedOrder[] = orderBatches.reduce((_orders, batch) => _orders.concat(...batch), []); const orders = sortingUtils.sortOrders(unsortedOrders); 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 29576fc678..c2a2af7c4b 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -4,6 +4,7 @@ import { RFQTIndicativeQuote } from '@0x/quote-server'; import { SignedOrder } from '@0x/types'; import { BigNumber, NULL_ADDRESS } from '@0x/utils'; import * as _ from 'lodash'; +import { ENABLE_PRICE_AWARE_RFQ } from '../../constants'; import { MarketOperation, Omit } from '../../types'; import { QuoteRequestor } from '../quote_requestor'; @@ -216,6 +217,17 @@ export class MarketOperationUtils { ), ); + const rfqtPromise = !ENABLE_PRICE_AWARE_RFQ && quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) + ? getRfqtIndicativeQuotesAsync( + nativeOrders[0].makerAssetData, + nativeOrders[0].takerAssetData, + MarketOperation.Sell, + takerAmount, + undefined, + _opts, + ) + : Promise.resolve([]); + const offChainBalancerPromise = sampleBalancerOffChain ? this._sampler.getBalancerSellQuotesOffChainAsync(makerToken, takerToken, sampleAmounts) : Promise.resolve([]); @@ -230,11 +242,13 @@ export class MarketOperationUtils { const [ [tokenDecimals, orderFillableAmounts, ethToMakerAssetRate, ethToTakerAssetRate, dexQuotes, twoHopQuotes], + rfqtIndicativeQuotes, offChainBalancerQuotes, offChainCreamQuotes, offChainBancorQuotes, ] = await Promise.all([ samplerPromise, + rfqtPromise, offChainBalancerPromise, offChainCreamPromise, offChainBancorPromise, @@ -251,7 +265,7 @@ export class MarketOperationUtils { orderFillableAmounts, ethToOutputRate: ethToMakerAssetRate, ethToInputRate: ethToTakerAssetRate, - rfqtIndicativeQuotes: [], + rfqtIndicativeQuotes, twoHopQuotes, quoteSourceFilters, makerTokenDecimals: makerTokenDecimals.toNumber(), @@ -350,7 +364,16 @@ export class MarketOperationUtils { this._liquidityProviderRegistry, ), ); - + const rfqtPromise = !ENABLE_PRICE_AWARE_RFQ && quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) + ? getRfqtIndicativeQuotesAsync( + nativeOrders[0].makerAssetData, + nativeOrders[0].takerAssetData, + MarketOperation.Buy, + makerAmount, + undefined, + _opts, + ) + : Promise.resolve([]); const offChainBalancerPromise = sampleBalancerOffChain ? this._sampler.getBalancerBuyQuotesOffChainAsync(makerToken, takerToken, sampleAmounts) : Promise.resolve([]); @@ -361,9 +384,10 @@ export class MarketOperationUtils { const [ [tokenDecimals, orderFillableAmounts, ethToMakerAssetRate, ethToTakerAssetRate, dexQuotes, twoHopQuotes], + rfqtIndicativeQuotes, offChainBalancerQuotes, offChainCreamQuotes, - ] = await Promise.all([samplerPromise, offChainBalancerPromise, offChainCreamPromise]); + ] = await Promise.all([samplerPromise, rfqtPromise, offChainBalancerPromise, offChainCreamPromise]); // Attach the MultiBridge address to the sample fillData (dexQuotes.find(quotes => quotes[0] && quotes[0].source === ERC20BridgeSource.MultiBridge) || []).forEach( q => (q.fillData = { poolAddress: this._multiBridge }), @@ -379,7 +403,7 @@ export class MarketOperationUtils { orderFillableAmounts, ethToOutputRate: ethToTakerAssetRate, ethToInputRate: ethToMakerAssetRate, - rfqtIndicativeQuotes: [], + rfqtIndicativeQuotes, twoHopQuotes, quoteSourceFilters, makerTokenDecimals: makerTokenDecimals.toNumber(), @@ -651,7 +675,7 @@ export class MarketOperationUtils { // If RFQ liquidity is enabled, make a request to check RFQ liquidity const { rfqt } = _opts; - if (rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { + if (ENABLE_PRICE_AWARE_RFQ && rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { // Calculate a suggested price. For now, this is simply the overall price of the aggregation. let comparisonPrice: BigNumber | undefined; if (optimizerResult) { From 85fde8f9a525da4e397c940309d20257ff65bd45 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Mon, 19 Oct 2020 17:03:26 -0700 Subject: [PATCH 27/32] comments and renaming --- packages/asset-swapper/src/constants.ts | 8 +++++++- packages/asset-swapper/src/swap_quoter.ts | 4 ++-- .../src/utils/market_operation_utils/index.ts | 8 ++++---- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/asset-swapper/src/constants.ts b/packages/asset-swapper/src/constants.ts index 6b5bd1a792..745e816bbf 100644 --- a/packages/asset-swapper/src/constants.ts +++ b/packages/asset-swapper/src/constants.ts @@ -123,4 +123,10 @@ export const constants = { DEFAULT_WARNING_LOGGER, }; -export const ENABLE_PRICE_AWARE_RFQ: boolean = false; +// This feature flag allows us to merge the price-aware RFQ pricing +// project while still controlling when to activate the feature. We plan to do some +// data analysis work and address some of the issues with maker fillable amounts +// in later milestones. Once the feature is fully rolled out and is providing value +// and we have assessed that there is no user impact, we will proceed in cleaning up +// the feature flag. +export const IS_PRICE_AWARE_RFQ_ENABLED: boolean = false; diff --git a/packages/asset-swapper/src/swap_quoter.ts b/packages/asset-swapper/src/swap_quoter.ts index 0f55949545..e6c5513100 100644 --- a/packages/asset-swapper/src/swap_quoter.ts +++ b/packages/asset-swapper/src/swap_quoter.ts @@ -8,7 +8,7 @@ import { BlockParamLiteral, SupportedProvider, ZeroExProvider } from 'ethereum-t import * as _ from 'lodash'; import { artifacts } from './artifacts'; -import { constants, ENABLE_PRICE_AWARE_RFQ } from './constants'; +import { constants, IS_PRICE_AWARE_RFQ_ENABLED } from './constants'; import { CalculateSwapQuoteOpts, LiquidityForTakerMakerAssetDataPair, @@ -697,7 +697,7 @@ export class SwapQuoter { } if ( - !ENABLE_PRICE_AWARE_RFQ && // Price-aware RFQ is disabled. + !IS_PRICE_AWARE_RFQ_ENABLED && // Price-aware RFQ is disabled. opts.rfqt && // This is an RFQT-enabled API request opts.rfqt.intentOnFilling && // The requestor is asking for a firm quote opts.rfqt.apiKey && 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 c2a2af7c4b..6daf6d2b66 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -4,7 +4,7 @@ import { RFQTIndicativeQuote } from '@0x/quote-server'; import { SignedOrder } from '@0x/types'; import { BigNumber, NULL_ADDRESS } from '@0x/utils'; import * as _ from 'lodash'; -import { ENABLE_PRICE_AWARE_RFQ } from '../../constants'; +import { IS_PRICE_AWARE_RFQ_ENABLED } from '../../constants'; import { MarketOperation, Omit } from '../../types'; import { QuoteRequestor } from '../quote_requestor'; @@ -217,7 +217,7 @@ export class MarketOperationUtils { ), ); - const rfqtPromise = !ENABLE_PRICE_AWARE_RFQ && quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) + const rfqtPromise = !IS_PRICE_AWARE_RFQ_ENABLED && quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) ? getRfqtIndicativeQuotesAsync( nativeOrders[0].makerAssetData, nativeOrders[0].takerAssetData, @@ -364,7 +364,7 @@ export class MarketOperationUtils { this._liquidityProviderRegistry, ), ); - const rfqtPromise = !ENABLE_PRICE_AWARE_RFQ && quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) + const rfqtPromise = !IS_PRICE_AWARE_RFQ_ENABLED && quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) ? getRfqtIndicativeQuotesAsync( nativeOrders[0].makerAssetData, nativeOrders[0].takerAssetData, @@ -675,7 +675,7 @@ export class MarketOperationUtils { // If RFQ liquidity is enabled, make a request to check RFQ liquidity const { rfqt } = _opts; - if (ENABLE_PRICE_AWARE_RFQ && rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { + if (IS_PRICE_AWARE_RFQ_ENABLED && rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { // Calculate a suggested price. For now, this is simply the overall price of the aggregation. let comparisonPrice: BigNumber | undefined; if (optimizerResult) { From 6366163006393260fbbeb79c60abe55da3fd9662 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Mon, 19 Oct 2020 17:09:37 -0700 Subject: [PATCH 28/32] small lint fix --- .../asset-swapper/src/utils/market_operation_utils/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6daf6d2b66..4b7b3a5d41 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -4,8 +4,8 @@ import { RFQTIndicativeQuote } from '@0x/quote-server'; import { SignedOrder } from '@0x/types'; import { BigNumber, NULL_ADDRESS } from '@0x/utils'; import * as _ from 'lodash'; -import { IS_PRICE_AWARE_RFQ_ENABLED } from '../../constants'; +import { IS_PRICE_AWARE_RFQ_ENABLED } from '../../constants'; import { MarketOperation, Omit } from '../../types'; import { QuoteRequestor } from '../quote_requestor'; From 1cfdc490218f597215290d2f6e8fbf96b4a530b8 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Tue, 20 Oct 2020 12:15:44 -0400 Subject: [PATCH 29/32] Ran prettier --- .../src/utils/market_operation_utils/index.ts | 7 +------ .../asset-swapper/src/utils/quote_requestor.ts | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 14 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 29576fc678..fb2987335f 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -233,12 +233,7 @@ export class MarketOperationUtils { offChainBalancerQuotes, offChainCreamQuotes, offChainBancorQuotes, - ] = await Promise.all([ - samplerPromise, - offChainBalancerPromise, - offChainCreamPromise, - offChainBancorPromise, - ]); + ] = await Promise.all([samplerPromise, offChainBalancerPromise, offChainCreamPromise, offChainBancorPromise]); const [makerTokenDecimals, takerTokenDecimals] = tokenDecimals; return { diff --git a/packages/asset-swapper/src/utils/quote_requestor.ts b/packages/asset-swapper/src/utils/quote_requestor.ts index 8916e7f5f3..c2e5ea99a8 100644 --- a/packages/asset-swapper/src/utils/quote_requestor.ts +++ b/packages/asset-swapper/src/utils/quote_requestor.ts @@ -98,14 +98,16 @@ export class QuoteRequestor { ): TakerRequestQueryParams { const buyTokenAddress = getTokenAddressOrThrow(makerAssetData); const sellTokenAddress = getTokenAddressOrThrow(takerAssetData); - const { buyAmountBaseUnits, sellAmountBaseUnits } = marketOperation === MarketOperation.Buy - ? { - buyAmountBaseUnits: assetFillAmount, - sellAmountBaseUnits: undefined, - } : { - sellAmountBaseUnits: assetFillAmount, - buyAmountBaseUnits: undefined, - }; + const { buyAmountBaseUnits, sellAmountBaseUnits } = + marketOperation === MarketOperation.Buy + ? { + buyAmountBaseUnits: assetFillAmount, + sellAmountBaseUnits: undefined, + } + : { + sellAmountBaseUnits: assetFillAmount, + buyAmountBaseUnits: undefined, + }; const requestParamsWithBigNumbers: Pick< TakerRequestQueryParams, From b5e3f0b90c4ec474732a508f2869564b164952b4 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Tue, 20 Oct 2020 13:48:46 -0400 Subject: [PATCH 30/32] Disable tests per IS_PRICE_AWARE_RFQ_ENABLED flag --- .../test/market_operation_utils_test.ts | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index b51ce5d7fb..98851fc438 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -17,6 +17,7 @@ import * as _ from 'lodash'; import * as TypeMoq from 'typemoq'; import { MarketOperation, QuoteRequestor, RfqtRequestOpts, SignedOrderWithFillableAmounts } from '../src'; +import { IS_PRICE_AWARE_RFQ_ENABLED } from '../src/constants'; import { getRfqtIndicativeQuotesAsync, MarketOperationUtils } from '../src/utils/market_operation_utils/'; import { BalancerPoolsCache } from '../src/utils/market_operation_utils/balancer_utils'; import { @@ -724,7 +725,7 @@ describe('MarketOperationUtils tests', () => { } }); - it('getMarketSellOrdersAsync() optimizer will be called once only if RFQ if not defined', async () => { + it('getMarketSellOrdersAsync() optimizer will be called once only if RFQ if not defined', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { const mockedMarketOpUtils = TypeMoq.Mock.ofType( MarketOperationUtils, TypeMoq.MockBehavior.Loose, @@ -744,9 +745,9 @@ describe('MarketOperationUtils tests', () => { const totalAssetAmount = ORDERS.map(o => o.takerAssetAmount).reduce((a, b) => a.plus(b)); await mockedMarketOpUtils.object.getMarketSellOrdersAsync(ORDERS, totalAssetAmount, DEFAULT_OPTS); mockedMarketOpUtils.verifyAll(); - }); + } : undefined); - it('optimizer will send in a comparison price to RFQ providers', async () => { + it('optimizer will send in a comparison price to RFQ providers', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { // Set up mocked quote requestor, will return an order that is better // than the best of the orders. const mockedQuoteRequestor = TypeMoq.Mock.ofType(QuoteRequestor, TypeMoq.MockBehavior.Loose, false, {}); @@ -848,9 +849,9 @@ describe('MarketOperationUtils tests', () => { expect(requestedComparisonPrice!.toString()).to.eql('320'); expect(result.optimizedOrders[0].makerAssetAmount.toString()).to.eql('321000000'); expect(result.optimizedOrders[0].takerAssetAmount.toString()).to.eql('1000000000000000000'); - }); + } : undefined); - it('getMarketSellOrdersAsync() will not rerun the optimizer if no orders are returned', async () => { + it('getMarketSellOrdersAsync() will not rerun the optimizer if no orders are returned', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { // Ensure that `_generateOptimizedOrdersAsync` is only called once const mockedMarketOpUtils = TypeMoq.Mock.ofType( MarketOperationUtils, @@ -883,9 +884,9 @@ describe('MarketOperationUtils tests', () => { }); mockedMarketOpUtils.verifyAll(); requestor.verifyAll(); - }); + } : undefined); - it('getMarketSellOrdersAsync() will rerun the optimizer if one or more indicative are returned', async () => { + it('getMarketSellOrdersAsync() will rerun the optimizer if one or more indicative are returned', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { const requestor = getMockedQuoteRequestor('indicative', [ORDERS[0], ORDERS[1]], TypeMoq.Times.once()); const numOrdersInCall: number[] = []; @@ -939,9 +940,9 @@ describe('MarketOperationUtils tests', () => { expect(numIndicativeQuotesInCall.length).to.eql(2); expect(numIndicativeQuotesInCall[0]).to.eql(0); expect(numIndicativeQuotesInCall[1]).to.eql(2); - }); + } : undefined); - it('getMarketSellOrdersAsync() will rerun the optimizer if one or more RFQ orders are returned', async () => { + it('getMarketSellOrdersAsync() will rerun the optimizer if one or more RFQ orders are returned', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { const requestor = getMockedQuoteRequestor('firm', [ORDERS[0]], TypeMoq.Times.once()); // Ensure that `_generateOptimizedOrdersAsync` is only called once @@ -990,9 +991,9 @@ describe('MarketOperationUtils tests', () => { // The first call to optimizer was with an extra RFQ order. expect(numOrdersInCall[0]).to.eql(2); expect(numOrdersInCall[1]).to.eql(3); - }); + } : undefined); - 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 () => { + 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', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { let hasFirstOptimizationRun = false; let hasSecondOptimizationRun = false; const requestor = getMockedQuoteRequestor('firm', [ORDERS[0], ORDERS[1]], TypeMoq.Times.once()); @@ -1043,7 +1044,7 @@ describe('MarketOperationUtils tests', () => { expect(hasFirstOptimizationRun).to.eql(true); expect(hasSecondOptimizationRun).to.eql(true); - }); + } : undefined); it('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( From a9d0cec6d1b7c8ddbc495e6208f321c974a09ede Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Tue, 20 Oct 2020 14:17:48 -0400 Subject: [PATCH 31/32] Ran prettier --- .../src/utils/market_operation_utils/index.ts | 49 +- .../test/market_operation_utils_test.ts | 647 ++++++++++-------- 2 files changed, 378 insertions(+), 318 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 4b7b3a5d41..ff176d09dc 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -217,16 +217,17 @@ export class MarketOperationUtils { ), ); - const rfqtPromise = !IS_PRICE_AWARE_RFQ_ENABLED && quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) - ? getRfqtIndicativeQuotesAsync( - nativeOrders[0].makerAssetData, - nativeOrders[0].takerAssetData, - MarketOperation.Sell, - takerAmount, - undefined, - _opts, - ) - : Promise.resolve([]); + const rfqtPromise = + !IS_PRICE_AWARE_RFQ_ENABLED && quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) + ? getRfqtIndicativeQuotesAsync( + nativeOrders[0].makerAssetData, + nativeOrders[0].takerAssetData, + MarketOperation.Sell, + takerAmount, + undefined, + _opts, + ) + : Promise.resolve([]); const offChainBalancerPromise = sampleBalancerOffChain ? this._sampler.getBalancerSellQuotesOffChainAsync(makerToken, takerToken, sampleAmounts) @@ -364,16 +365,17 @@ export class MarketOperationUtils { this._liquidityProviderRegistry, ), ); - const rfqtPromise = !IS_PRICE_AWARE_RFQ_ENABLED && quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) - ? getRfqtIndicativeQuotesAsync( - nativeOrders[0].makerAssetData, - nativeOrders[0].takerAssetData, - MarketOperation.Buy, - makerAmount, - undefined, - _opts, - ) - : Promise.resolve([]); + const rfqtPromise = + !IS_PRICE_AWARE_RFQ_ENABLED && quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) + ? getRfqtIndicativeQuotesAsync( + nativeOrders[0].makerAssetData, + nativeOrders[0].takerAssetData, + MarketOperation.Buy, + makerAmount, + undefined, + _opts, + ) + : Promise.resolve([]); const offChainBalancerPromise = sampleBalancerOffChain ? this._sampler.getBalancerBuyQuotesOffChainAsync(makerToken, takerToken, sampleAmounts) : Promise.resolve([]); @@ -675,7 +677,12 @@ export class MarketOperationUtils { // If RFQ liquidity is enabled, make a request to check RFQ liquidity const { rfqt } = _opts; - if (IS_PRICE_AWARE_RFQ_ENABLED && rfqt && rfqt.quoteRequestor && marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native)) { + if ( + IS_PRICE_AWARE_RFQ_ENABLED && + rfqt && + rfqt.quoteRequestor && + marketSideLiquidity.quoteSourceFilters.isAllowed(ERC20BridgeSource.Native) + ) { // Calculate a suggested price. For now, this is simply the overall price of the aggregation. let comparisonPrice: BigNumber | undefined; if (optimizerResult) { diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 98851fc438..ac952760f0 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -725,326 +725,379 @@ describe('MarketOperationUtils tests', () => { } }); - it('getMarketSellOrdersAsync() optimizer will be called once only if RFQ if not defined', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { - const mockedMarketOpUtils = TypeMoq.Mock.ofType( - MarketOperationUtils, - TypeMoq.MockBehavior.Loose, - false, - MOCK_SAMPLER, - contractAddresses, - ORDER_DOMAIN, - ); - mockedMarketOpUtils.callBase = true; + it( + 'getMarketSellOrdersAsync() optimizer will be called once only if RFQ if not defined', + IS_PRICE_AWARE_RFQ_ENABLED + ? async () => { + const mockedMarketOpUtils = TypeMoq.Mock.ofType( + MarketOperationUtils, + TypeMoq.MockBehavior.Loose, + false, + MOCK_SAMPLER, + contractAddresses, + ORDER_DOMAIN, + ); + mockedMarketOpUtils.callBase = true; - // Ensure that `_generateOptimizedOrdersAsync` is only called once - mockedMarketOpUtils - .setup(m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny())) - .returns(async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b)) - .verifiable(TypeMoq.Times.once()); + // Ensure that `_generateOptimizedOrdersAsync` is only called once + mockedMarketOpUtils + .setup(m => m._generateOptimizedOrdersAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny())) + .returns(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)); - await mockedMarketOpUtils.object.getMarketSellOrdersAsync(ORDERS, totalAssetAmount, DEFAULT_OPTS); - mockedMarketOpUtils.verifyAll(); - } : undefined); + const totalAssetAmount = ORDERS.map(o => o.takerAssetAmount).reduce((a, b) => a.plus(b)); + await mockedMarketOpUtils.object.getMarketSellOrdersAsync( + ORDERS, + totalAssetAmount, + DEFAULT_OPTS, + ); + mockedMarketOpUtils.verifyAll(); + } + : undefined, + ); - it('optimizer will send in a comparison price to RFQ providers', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { - // Set up mocked quote requestor, will return an order that is better - // than the best of the orders. - const mockedQuoteRequestor = TypeMoq.Mock.ofType(QuoteRequestor, TypeMoq.MockBehavior.Loose, false, {}); + it( + 'optimizer will send in a comparison price to RFQ providers', + IS_PRICE_AWARE_RFQ_ENABLED + ? async () => { + // Set up mocked quote requestor, will return an order that is better + // than the best of the orders. + const mockedQuoteRequestor = TypeMoq.Mock.ofType( + QuoteRequestor, + TypeMoq.MockBehavior.Loose, + false, + {}, + ); - let requestedComparisonPrice: BigNumber | undefined; - mockedQuoteRequestor - .setup(mqr => - mqr.requestRfqtFirmQuotesAsync( - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - ), - ) - .callback( - ( - _makerAssetData: string, - _takerAssetData: string, - _assetFillAmount: BigNumber, - _marketOperation: MarketOperation, - comparisonPrice: BigNumber | undefined, - _options: RfqtRequestOpts, - ) => { - requestedComparisonPrice = comparisonPrice; - }, - ) - .returns(async () => { - return [ - { - signedOrder: createOrder({ - makerAssetData: MAKER_ASSET_DATA, - takerAssetData: TAKER_ASSET_DATA, - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(321, 6), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), - }), - }, - ]; - }); + let requestedComparisonPrice: BigNumber | undefined; + mockedQuoteRequestor + .setup(mqr => + mqr.requestRfqtFirmQuotesAsync( + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + ), + ) + .callback( + ( + _makerAssetData: string, + _takerAssetData: string, + _assetFillAmount: BigNumber, + _marketOperation: MarketOperation, + comparisonPrice: BigNumber | undefined, + _options: RfqtRequestOpts, + ) => { + requestedComparisonPrice = comparisonPrice; + }, + ) + .returns(async () => { + return [ + { + signedOrder: createOrder({ + makerAssetData: MAKER_ASSET_DATA, + takerAssetData: TAKER_ASSET_DATA, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(321, 6), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), + }), + }, + ]; + }); - // Set up sampler, will only return 1 on-chain order - const mockedMarketOpUtils = TypeMoq.Mock.ofType( - MarketOperationUtils, - TypeMoq.MockBehavior.Loose, - false, - MOCK_SAMPLER, - contractAddresses, - ORDER_DOMAIN, - ); - mockedMarketOpUtils.callBase = true; - mockedMarketOpUtils - .setup(mou => - mou.getMarketSellLiquidityAsync(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()), - ) - .returns(async () => { - return { - dexQuotes: [], - ethToInputRate: Web3Wrapper.toBaseUnitAmount(1, 18), - ethToOutputRate: Web3Wrapper.toBaseUnitAmount(1, 6), - inputAmount: Web3Wrapper.toBaseUnitAmount(1, 18), - inputToken: MAKER_TOKEN, - outputToken: TAKER_TOKEN, - nativeOrders: [ - createOrder({ - makerAssetData: MAKER_ASSET_DATA, - takerAssetData: TAKER_ASSET_DATA, - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(320, 6), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), - }), - ], - orderFillableAmounts: [Web3Wrapper.toBaseUnitAmount(1, 18)], - rfqtIndicativeQuotes: [], - side: MarketOperation.Sell, - twoHopQuotes: [], - quoteSourceFilters: new SourceFilters(), - makerTokenDecimals: 6, - takerTokenDecimals: 18, - }; - }); - const result = await mockedMarketOpUtils.object.getMarketSellOrdersAsync( - ORDERS, - Web3Wrapper.toBaseUnitAmount(1, 18), - { - ...DEFAULT_OPTS, - rfqt: { - isIndicative: false, - apiKey: 'foo', - takerAddress: randomAddress(), - intentOnFilling: true, - quoteRequestor: { - requestRfqtFirmQuotesAsync: mockedQuoteRequestor.object.requestRfqtFirmQuotesAsync, - } as any, - }, - }, - ); - expect(result.optimizedOrders.length).to.eql(1); - // tslint:disable-next-line:no-unnecessary-type-assertion - expect(requestedComparisonPrice!.toString()).to.eql('320'); - expect(result.optimizedOrders[0].makerAssetAmount.toString()).to.eql('321000000'); - expect(result.optimizedOrders[0].takerAssetAmount.toString()).to.eql('1000000000000000000'); - } : undefined); + // Set up sampler, will only return 1 on-chain order + const mockedMarketOpUtils = TypeMoq.Mock.ofType( + MarketOperationUtils, + TypeMoq.MockBehavior.Loose, + false, + MOCK_SAMPLER, + contractAddresses, + ORDER_DOMAIN, + ); + mockedMarketOpUtils.callBase = true; + mockedMarketOpUtils + .setup(mou => + mou.getMarketSellLiquidityAsync( + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + ), + ) + .returns(async () => { + return { + dexQuotes: [], + ethToInputRate: Web3Wrapper.toBaseUnitAmount(1, 18), + ethToOutputRate: Web3Wrapper.toBaseUnitAmount(1, 6), + inputAmount: Web3Wrapper.toBaseUnitAmount(1, 18), + inputToken: MAKER_TOKEN, + outputToken: TAKER_TOKEN, + nativeOrders: [ + createOrder({ + makerAssetData: MAKER_ASSET_DATA, + takerAssetData: TAKER_ASSET_DATA, + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(320, 6), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(1, 18), + }), + ], + orderFillableAmounts: [Web3Wrapper.toBaseUnitAmount(1, 18)], + rfqtIndicativeQuotes: [], + side: MarketOperation.Sell, + twoHopQuotes: [], + quoteSourceFilters: new SourceFilters(), + makerTokenDecimals: 6, + takerTokenDecimals: 18, + }; + }); + const result = await mockedMarketOpUtils.object.getMarketSellOrdersAsync( + ORDERS, + Web3Wrapper.toBaseUnitAmount(1, 18), + { + ...DEFAULT_OPTS, + rfqt: { + isIndicative: false, + apiKey: 'foo', + takerAddress: randomAddress(), + intentOnFilling: true, + quoteRequestor: { + requestRfqtFirmQuotesAsync: + mockedQuoteRequestor.object.requestRfqtFirmQuotesAsync, + } as any, + }, + }, + ); + expect(result.optimizedOrders.length).to.eql(1); + // tslint:disable-next-line:no-unnecessary-type-assertion + expect(requestedComparisonPrice!.toString()).to.eql('320'); + expect(result.optimizedOrders[0].makerAssetAmount.toString()).to.eql('321000000'); + expect(result.optimizedOrders[0].takerAssetAmount.toString()).to.eql('1000000000000000000'); + } + : undefined, + ); - it('getMarketSellOrdersAsync() will not rerun the optimizer if no orders are returned', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { - // 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())) - .returns(async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b)) - .verifiable(TypeMoq.Times.once()); + it( + 'getMarketSellOrdersAsync() will not rerun the optimizer if no orders are returned', + IS_PRICE_AWARE_RFQ_ENABLED + ? async () => { + // 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())) + .returns(async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b)) + .verifiable(TypeMoq.Times.once()); - const requestor = getMockedQuoteRequestor('firm', [], TypeMoq.Times.once()); + const requestor = getMockedQuoteRequestor('firm', [], TypeMoq.Times.once()); - const totalAssetAmount = ORDERS.map(o => o.takerAssetAmount).reduce((a, b) => a.plus(b)); - await mockedMarketOpUtils.object.getMarketSellOrdersAsync(ORDERS, totalAssetAmount, { - ...DEFAULT_OPTS, - rfqt: { - isIndicative: false, - apiKey: 'foo', - takerAddress: randomAddress(), - intentOnFilling: true, - quoteRequestor: { - requestRfqtFirmQuotesAsync: requestor.object.requestRfqtFirmQuotesAsync, - } as any, - }, - }); - mockedMarketOpUtils.verifyAll(); - requestor.verifyAll(); - } : undefined); + const totalAssetAmount = ORDERS.map(o => o.takerAssetAmount).reduce((a, b) => a.plus(b)); + await mockedMarketOpUtils.object.getMarketSellOrdersAsync(ORDERS, totalAssetAmount, { + ...DEFAULT_OPTS, + rfqt: { + isIndicative: false, + apiKey: 'foo', + takerAddress: randomAddress(), + intentOnFilling: true, + quoteRequestor: { + requestRfqtFirmQuotesAsync: requestor.object.requestRfqtFirmQuotesAsync, + } as any, + }, + }); + mockedMarketOpUtils.verifyAll(); + requestor.verifyAll(); + } + : undefined, + ); - it('getMarketSellOrdersAsync() will rerun the optimizer if one or more indicative are returned', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { - const requestor = getMockedQuoteRequestor('indicative', [ORDERS[0], ORDERS[1]], TypeMoq.Times.once()); + it( + 'getMarketSellOrdersAsync() will rerun the optimizer if one or more indicative are returned', + IS_PRICE_AWARE_RFQ_ENABLED + ? async () => { + const requestor = getMockedQuoteRequestor( + 'indicative', + [ORDERS[0], ORDERS[1]], + TypeMoq.Times.once(), + ); - const numOrdersInCall: number[] = []; - const numIndicativeQuotesInCall: number[] = []; + const numOrdersInCall: number[] = []; + const numIndicativeQuotesInCall: 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); - numIndicativeQuotesInCall.push(msl.rfqtIndicativeQuotes.length); - }) - .returns(async (a, b) => mockedMarketOpUtils.target._generateOptimizedOrdersAsync(a, b)) - .verifiable(TypeMoq.Times.exactly(2)); + 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); + 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(); + 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 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); - } : undefined); + // 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); + } + : undefined, + ); - it('getMarketSellOrdersAsync() will rerun the optimizer if one or more RFQ orders are returned', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { - const requestor = getMockedQuoteRequestor('firm', [ORDERS[0]], TypeMoq.Times.once()); + it( + 'getMarketSellOrdersAsync() will rerun the optimizer if one or more RFQ orders are returned', + IS_PRICE_AWARE_RFQ_ENABLED + ? async () => { + const requestor = getMockedQuoteRequestor('firm', [ORDERS[0]], TypeMoq.Times.once()); - // Ensure that `_generateOptimizedOrdersAsync` is only called 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)) - .verifiable(TypeMoq.Times.exactly(2)); + // 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)) + .verifiable(TypeMoq.Times.exactly(2)); - const totalAssetAmount = ORDERS.map(o => o.takerAssetAmount).reduce((a, b) => a.plus(b)); - await mockedMarketOpUtils.object.getMarketSellOrdersAsync( - ORDERS.slice(1, 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(numOrdersInCall.length).to.eql(2); + const totalAssetAmount = ORDERS.map(o => o.takerAssetAmount).reduce((a, b) => a.plus(b)); + await mockedMarketOpUtils.object.getMarketSellOrdersAsync( + ORDERS.slice(1, 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(numOrdersInCall.length).to.eql(2); - // The first call to optimizer was without an RFQ order. - // The first call to optimizer was with an extra RFQ order. - expect(numOrdersInCall[0]).to.eql(2); - expect(numOrdersInCall[1]).to.eql(3); - } : undefined); + // The first call to optimizer was without an RFQ order. + // The first call to optimizer was with an extra RFQ order. + expect(numOrdersInCall[0]).to.eql(2); + expect(numOrdersInCall[1]).to.eql(3); + } + : undefined, + ); - 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', IS_PRICE_AWARE_RFQ_ENABLED ? async () => { - let hasFirstOptimizationRun = false; - let hasSecondOptimizationRun = false; - const requestor = getMockedQuoteRequestor('firm', [ORDERS[0], ORDERS[1]], TypeMoq.Times.once()); + 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', + IS_PRICE_AWARE_RFQ_ENABLED + ? 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 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(); + 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); - } : undefined); + expect(hasFirstOptimizationRun).to.eql(true); + expect(hasSecondOptimizationRun).to.eql(true); + } + : undefined, + ); it('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( From 61bf93aac2fe8edbe45bc6a8a31941db21df88e9 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Tue, 20 Oct 2020 14:30:04 -0400 Subject: [PATCH 32/32] Leave link to PR in comments --- packages/asset-swapper/src/constants.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/asset-swapper/src/constants.ts b/packages/asset-swapper/src/constants.ts index 745e816bbf..d4686e9923 100644 --- a/packages/asset-swapper/src/constants.ts +++ b/packages/asset-swapper/src/constants.ts @@ -128,5 +128,6 @@ export const constants = { // data analysis work and address some of the issues with maker fillable amounts // in later milestones. Once the feature is fully rolled out and is providing value // and we have assessed that there is no user impact, we will proceed in cleaning up -// the feature flag. +// the feature flag. When that time comes, follow this PR to "undo" the feature flag: +// https://github.com/0xProject/0x-monorepo/pull/2735 export const IS_PRICE_AWARE_RFQ_ENABLED: boolean = false;