From f359de2cac473e08819ed8688e210074bf754fc3 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Wed, 22 Jul 2020 22:46:25 -0700 Subject: [PATCH 1/4] fixed a small bug in indicative quotes --- .../src/utils/market_operation_utils/index.ts | 5 +- .../test/market_operation_utils_test.ts | 75 +++++++++++++++++-- 2 files changed, 70 insertions(+), 10 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 4b67ccaa73..f7f1d12eff 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -26,14 +26,15 @@ import { OrderDomain, } from './types'; -async function getRfqtIndicativeQuotesAsync( +export async function getRfqtIndicativeQuotesAsync( makerAssetData: string, takerAssetData: string, marketOperation: MarketOperation, assetFillAmount: BigNumber, opts: Partial, ): Promise { - if (opts.rfqt && opts.rfqt.isIndicative === true && opts.rfqt.quoteRequestor) { + const hasExcludedNativeLiquidity = opts.excludedSources && opts.excludedSources.includes(ERC20BridgeSource.Native); + if (!hasExcludedNativeLiquidity && opts.rfqt && opts.rfqt.isIndicative === true && opts.rfqt.quoteRequestor) { return opts.rfqt.quoteRequestor.requestRfqtIndicativeQuotesAsync( makerAssetData, takerAssetData, diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 52f291fae1..317838b9e3 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -10,16 +10,22 @@ import { } from '@0x/contracts-test-utils'; import { Web3Wrapper } from '@0x/dev-utils'; import { assetDataUtils, generatePseudoRandomSalt } from '@0x/order-utils'; -import { AssetProxyId, ERC20BridgeAssetData, SignedOrder } from '@0x/types'; +import { AssetProxyId, ERC20BridgeAssetData, SignedOrder, Type } from '@0x/types'; import { BigNumber, fromTokenUnitAmount, hexUtils, NULL_ADDRESS } from '@0x/utils'; import * as _ from 'lodash'; -import { MarketOperation, SignedOrderWithFillableAmounts } from '../src'; -import { MarketOperationUtils } from '../src/utils/market_operation_utils/'; +import { MarketOperation, SignedOrderWithFillableAmounts, QuoteRequestor, RfqtRequestOpts } from '../src'; +import { MarketOperationUtils, getRfqtIndicativeQuotesAsync } from '../src/utils/market_operation_utils/'; import { BUY_SOURCES, POSITIVE_INF, SELL_SOURCES, ZERO_AMOUNT } from '../src/utils/market_operation_utils/constants'; import { createFillPaths } from '../src/utils/market_operation_utils/fills'; import { DexOrderSampler } from '../src/utils/market_operation_utils/sampler'; import { DexSample, ERC20BridgeSource, FillData, NativeFillData } from '../src/utils/market_operation_utils/types'; +import * as TypeMoq from 'typemoq'; + +const MAKER_TOKEN = randomAddress(); +const TAKER_TOKEN = randomAddress(); +const MAKER_ASSET_DATA = assetDataUtils.encodeERC20AssetData(MAKER_TOKEN); +const TAKER_ASSET_DATA = assetDataUtils.encodeERC20AssetData(TAKER_TOKEN); // tslint:disable: custom-no-magic-numbers promise-function-async describe('MarketOperationUtils tests', () => { @@ -30,11 +36,6 @@ describe('MarketOperationUtils tests', () => { const UNISWAP_BRIDGE_ADDRESS = contractAddresses.uniswapBridge; const UNISWAP_V2_BRIDGE_ADDRESS = contractAddresses.uniswapV2Bridge; const CURVE_BRIDGE_ADDRESS = contractAddresses.curveBridge; - - const MAKER_TOKEN = randomAddress(); - const TAKER_TOKEN = randomAddress(); - const MAKER_ASSET_DATA = assetDataUtils.encodeERC20AssetData(MAKER_TOKEN); - const TAKER_ASSET_DATA = assetDataUtils.encodeERC20AssetData(TAKER_TOKEN); let originalSamplerOperations: any; before(() => { @@ -334,6 +335,64 @@ describe('MarketOperationUtils tests', () => { }, } as any) as DexOrderSampler; + describe.only('getRfqtIndicativeQuotesAsync', () => { + const partialRfqt: RfqtRequestOpts = { + apiKey: 'foo', + takerAddress: NULL_ADDRESS, + isIndicative: true, + intentOnFilling: false, + }; + + it('returns an empty array if native liquidity is excluded from the salad', async () => { + const requestor = TypeMoq.Mock.ofType(QuoteRequestor, TypeMoq.MockBehavior.Strict); + const result = await getRfqtIndicativeQuotesAsync( + MAKER_ASSET_DATA, + TAKER_ASSET_DATA, + MarketOperation.Sell, + new BigNumber('100e18'), + { + rfqt: {quoteRequestor: requestor.object, ...partialRfqt}, + excludedSources: [ERC20BridgeSource.Native], + }, + ); + expect(result.length).to.eql(0); + requestor.verify( + r => r.requestRfqtIndicativeQuotesAsync( + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + ), + TypeMoq.Times.never(), + ); + }); + + it('calls RFQT if Native source is not excluded', async () => { + const requestor = TypeMoq.Mock.ofType(QuoteRequestor, TypeMoq.MockBehavior.Loose); + requestor.setup( + r => r.requestRfqtIndicativeQuotesAsync( + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + ), + ).returns(() => Promise.resolve([])).verifiable(TypeMoq.Times.once()); + await getRfqtIndicativeQuotesAsync( + MAKER_ASSET_DATA, + TAKER_ASSET_DATA, + MarketOperation.Sell, + new BigNumber('100e18'), + { + rfqt: {quoteRequestor: requestor.object, ...partialRfqt}, + excludedSources: [], + }, + ); + requestor.verifyAll(); + }); + }); + describe('MarketOperationUtils', () => { let marketOperationUtils: MarketOperationUtils; From e6dcf92ec8696229707296c2bbe72cad8125fb12 Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Wed, 22 Jul 2020 23:02:40 -0700 Subject: [PATCH 2/4] added new parameter `nativeExclusivelyRFQT` --- packages/asset-swapper/src/constants.ts | 1 - packages/asset-swapper/src/swap_quoter.ts | 28 +++++++++---------- packages/asset-swapper/src/types.ts | 7 ++++- .../test/market_operation_utils_test.ts | 2 +- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/packages/asset-swapper/src/constants.ts b/packages/asset-swapper/src/constants.ts index 8886c5bcaa..c7dc690e32 100644 --- a/packages/asset-swapper/src/constants.ts +++ b/packages/asset-swapper/src/constants.ts @@ -47,7 +47,6 @@ const DEFAULT_SWAP_QUOTER_OPTS: SwapQuoterOpts = { rfqt: { takerApiKeyWhitelist: [], makerAssetOfferings: {}, - skipBuyRequests: false, }, }; diff --git a/packages/asset-swapper/src/swap_quoter.ts b/packages/asset-swapper/src/swap_quoter.ts index fd93848447..2690942f81 100644 --- a/packages/asset-swapper/src/swap_quoter.ts +++ b/packages/asset-swapper/src/swap_quoter.ts @@ -48,7 +48,6 @@ export class SwapQuoter { private readonly _orderStateUtils: OrderStateUtils; private readonly _quoteRequestor: QuoteRequestor; private readonly _rfqtTakerApiKeyWhitelist: string[]; - private readonly _rfqtSkipBuyRequests: boolean; /** * Instantiates a new SwapQuoter instance given existing liquidity in the form of orders and feeOrders. @@ -170,10 +169,6 @@ export class SwapQuoter { this.expiryBufferMs = expiryBufferMs; this.permittedOrderFeeTypes = permittedOrderFeeTypes; this._rfqtTakerApiKeyWhitelist = rfqt ? rfqt.takerApiKeyWhitelist || [] : []; - this._rfqtSkipBuyRequests = - rfqt && rfqt.skipBuyRequests !== undefined - ? rfqt.skipBuyRequests - : (r => r !== undefined && r.skipBuyRequests === true)(constants.DEFAULT_SWAP_QUOTER_OPTS.rfqt); this._contractAddresses = options.contractAddresses || getContractAddressesForChainOrThrow(chainId); this._devUtilsContract = new DevUtilsContract(this._contractAddresses.devUtils, provider); this._protocolFeeUtils = ProtocolFeeUtils.getInstance( @@ -561,20 +556,26 @@ export class SwapQuoter { } else { gasPrice = await this.getGasPriceEstimationOrThrowAsync(); } + + // If RFQT is enabled and `nativeExclusivelyRFQT` is set, then `ERC20BridgeSource.Native` should + // never be excluded. + if ((opts.rfqt && opts.rfqt.nativeExclusivelyRFQT === true) && opts.excludedSources.includes(ERC20BridgeSource.Native)) { + throw new Error('Native liquidity cannot be excluded if "rfqt.nativeExclusivelyRFQT" is set'); + } + // get batches of orders from different sources, awaiting sources in parallel const orderBatchPromises: Array> = []; orderBatchPromises.push( - // Don't fetch from the DB if Native has been excluded - opts.excludedSources.includes(ERC20BridgeSource.Native) + // Don't fetch Open Orderbook orders from the DB if Native has been excluded, or if `nativeExclusivelyRFQT` has been set. + opts.excludedSources.includes(ERC20BridgeSource.Native) || (opts.rfqt && opts.rfqt.nativeExclusivelyRFQT === true) ? Promise.resolve([]) : this._getSignedOrdersAsync(makerAssetData, takerAssetData), ); if ( - opts.rfqt && - opts.rfqt.intentOnFilling && - opts.rfqt.apiKey && - this._rfqtTakerApiKeyWhitelist.includes(opts.rfqt.apiKey) && - !(marketOperation === MarketOperation.Buy && this._rfqtSkipBuyRequests) + opts.rfqt && // This is an RFQT-enabled API request + opts.rfqt.intentOnFilling && // The requestor is asking for a firm quote + !opts.excludedSources.includes(ERC20BridgeSource.Native) && // Native liquidity is not excluded + this._rfqtTakerApiKeyWhitelist.includes(opts.rfqt.apiKey) // A valid API key was provided ) { if (!opts.rfqt.takerAddress || opts.rfqt.takerAddress === constants.NULL_ADDRESS) { throw new Error('RFQ-T requests must specify a taker address'); @@ -636,8 +637,7 @@ export class SwapQuoter { opts !== undefined && opts.isIndicative !== undefined && opts.isIndicative && - this._rfqtTakerApiKeyWhitelist.includes(opts.apiKey) && - !(op === MarketOperation.Buy && this._rfqtSkipBuyRequests) + this._rfqtTakerApiKeyWhitelist.includes(opts.apiKey) ); } } diff --git a/packages/asset-swapper/src/types.ts b/packages/asset-swapper/src/types.ts index 798e320f8c..1bf1d1e69c 100644 --- a/packages/asset-swapper/src/types.ts +++ b/packages/asset-swapper/src/types.ts @@ -199,12 +199,18 @@ export interface SwapQuoteOrdersBreakdown { [source: string]: BigNumber; } +/** + * nativeExclusivelyRFQT: if set to `true`, Swap quote will exclude Open Orderbook liquidity. + * If set to `true` and `ERC20BridgeSource.Native` is part of the `excludedSources` + * array in `SwapQuoteRequestOpts`, an Error will be raised. + */ export interface RfqtRequestOpts { takerAddress: string; apiKey: string; intentOnFilling: boolean; isIndicative?: boolean; makerEndpointMaxResponseTimeMs?: number; + nativeExclusivelyRFQT?: boolean; } /** @@ -249,7 +255,6 @@ export interface SwapQuoterOpts extends OrderPrunerOpts { rfqt?: { takerApiKeyWhitelist: string[]; makerAssetOfferings: RfqtMakerAssetOfferings; - skipBuyRequests?: boolean; warningLogger?: LogFunction; infoLogger?: LogFunction; }; diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 317838b9e3..6b52497666 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -335,7 +335,7 @@ describe('MarketOperationUtils tests', () => { }, } as any) as DexOrderSampler; - describe.only('getRfqtIndicativeQuotesAsync', () => { + describe('getRfqtIndicativeQuotesAsync', () => { const partialRfqt: RfqtRequestOpts = { apiKey: 'foo', takerAddress: NULL_ADDRESS, From d7918ea047893ed841fa59e995b3b797e2adb35d Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Thu, 23 Jul 2020 10:16:22 -0700 Subject: [PATCH 3/4] prettified --- packages/asset-swapper/src/swap_quoter.ts | 17 +++++--- .../src/utils/market_operation_utils/index.ts | 8 ++++ .../test/market_operation_utils_test.ts | 40 ++++++++++--------- 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/packages/asset-swapper/src/swap_quoter.ts b/packages/asset-swapper/src/swap_quoter.ts index 2690942f81..5817743e40 100644 --- a/packages/asset-swapper/src/swap_quoter.ts +++ b/packages/asset-swapper/src/swap_quoter.ts @@ -559,7 +559,11 @@ export class SwapQuoter { // If RFQT is enabled and `nativeExclusivelyRFQT` is set, then `ERC20BridgeSource.Native` should // never be excluded. - if ((opts.rfqt && opts.rfqt.nativeExclusivelyRFQT === true) && opts.excludedSources.includes(ERC20BridgeSource.Native)) { + if ( + opts.rfqt && + opts.rfqt.nativeExclusivelyRFQT === true && + opts.excludedSources.includes(ERC20BridgeSource.Native) + ) { throw new Error('Native liquidity cannot be excluded if "rfqt.nativeExclusivelyRFQT" is set'); } @@ -567,15 +571,16 @@ export class SwapQuoter { const orderBatchPromises: Array> = []; orderBatchPromises.push( // Don't fetch Open Orderbook orders from the DB if Native has been excluded, or if `nativeExclusivelyRFQT` has been set. - opts.excludedSources.includes(ERC20BridgeSource.Native) || (opts.rfqt && opts.rfqt.nativeExclusivelyRFQT === true) + opts.excludedSources.includes(ERC20BridgeSource.Native) || + (opts.rfqt && opts.rfqt.nativeExclusivelyRFQT === true) ? Promise.resolve([]) : this._getSignedOrdersAsync(makerAssetData, takerAssetData), ); if ( - opts.rfqt && // This is an RFQT-enabled API request - opts.rfqt.intentOnFilling && // The requestor is asking for a firm quote - !opts.excludedSources.includes(ERC20BridgeSource.Native) && // Native liquidity is not excluded - this._rfqtTakerApiKeyWhitelist.includes(opts.rfqt.apiKey) // A valid API key was provided + opts.rfqt && // This is an RFQT-enabled API request + opts.rfqt.intentOnFilling && // The requestor is asking for a firm quote + !opts.excludedSources.includes(ERC20BridgeSource.Native) && // Native liquidity is not excluded + this._rfqtTakerApiKeyWhitelist.includes(opts.rfqt.apiKey) // A valid API key was provided ) { if (!opts.rfqt.takerAddress || opts.rfqt.takerAddress === constants.NULL_ADDRESS) { throw new Error('RFQ-T requests must specify a taker address'); 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 f7f1d12eff..597dcb0fc1 100644 --- a/packages/asset-swapper/src/utils/market_operation_utils/index.ts +++ b/packages/asset-swapper/src/utils/market_operation_utils/index.ts @@ -26,6 +26,14 @@ import { OrderDomain, } from './types'; +/** + * Returns a indicative quotes or an empty array if RFQT is not enabled or requested + * @param makerAssetData the maker asset data + * @param takerAssetData the taker asset data + * @param marketOperation Buy or Sell + * @param assetFillAmount the amount to fill, in base units + * @param opts market request options + */ export async function getRfqtIndicativeQuotesAsync( makerAssetData: string, takerAssetData: string, diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 6b52497666..7702aa7c9f 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -351,41 +351,45 @@ describe('MarketOperationUtils tests', () => { MarketOperation.Sell, new BigNumber('100e18'), { - rfqt: {quoteRequestor: requestor.object, ...partialRfqt}, + rfqt: { quoteRequestor: requestor.object, ...partialRfqt }, excludedSources: [ERC20BridgeSource.Native], }, ); expect(result.length).to.eql(0); requestor.verify( - r => r.requestRfqtIndicativeQuotesAsync( - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - ), + r => + r.requestRfqtIndicativeQuotesAsync( + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + ), TypeMoq.Times.never(), ); }); it('calls RFQT if Native source is not excluded', async () => { const requestor = TypeMoq.Mock.ofType(QuoteRequestor, TypeMoq.MockBehavior.Loose); - requestor.setup( - r => r.requestRfqtIndicativeQuotesAsync( - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - TypeMoq.It.isAny(), - ), - ).returns(() => Promise.resolve([])).verifiable(TypeMoq.Times.once()); + requestor + .setup(r => + r.requestRfqtIndicativeQuotesAsync( + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + ), + ) + .returns(() => Promise.resolve([])) + .verifiable(TypeMoq.Times.once()); await getRfqtIndicativeQuotesAsync( MAKER_ASSET_DATA, TAKER_ASSET_DATA, MarketOperation.Sell, new BigNumber('100e18'), { - rfqt: {quoteRequestor: requestor.object, ...partialRfqt}, + rfqt: { quoteRequestor: requestor.object, ...partialRfqt }, excludedSources: [], }, ); From 6553ee0130676ab0c27877524c85cd3d3b494e9e Mon Sep 17 00:00:00 2001 From: Daniel Pyrathon Date: Thu, 23 Jul 2020 11:04:51 -0700 Subject: [PATCH 4/4] reordered imports --- .../asset-swapper/test/market_operation_utils_test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/asset-swapper/test/market_operation_utils_test.ts b/packages/asset-swapper/test/market_operation_utils_test.ts index 7702aa7c9f..f3eabf604e 100644 --- a/packages/asset-swapper/test/market_operation_utils_test.ts +++ b/packages/asset-swapper/test/market_operation_utils_test.ts @@ -10,17 +10,17 @@ import { } from '@0x/contracts-test-utils'; import { Web3Wrapper } from '@0x/dev-utils'; import { assetDataUtils, generatePseudoRandomSalt } from '@0x/order-utils'; -import { AssetProxyId, ERC20BridgeAssetData, SignedOrder, Type } from '@0x/types'; +import { AssetProxyId, ERC20BridgeAssetData, SignedOrder } from '@0x/types'; import { BigNumber, fromTokenUnitAmount, hexUtils, NULL_ADDRESS } from '@0x/utils'; import * as _ from 'lodash'; +import * as TypeMoq from 'typemoq'; -import { MarketOperation, SignedOrderWithFillableAmounts, QuoteRequestor, RfqtRequestOpts } from '../src'; -import { MarketOperationUtils, getRfqtIndicativeQuotesAsync } from '../src/utils/market_operation_utils/'; +import { MarketOperation, QuoteRequestor, RfqtRequestOpts, SignedOrderWithFillableAmounts } from '../src'; +import { getRfqtIndicativeQuotesAsync, MarketOperationUtils } from '../src/utils/market_operation_utils/'; import { BUY_SOURCES, POSITIVE_INF, SELL_SOURCES, ZERO_AMOUNT } from '../src/utils/market_operation_utils/constants'; import { createFillPaths } from '../src/utils/market_operation_utils/fills'; import { DexOrderSampler } from '../src/utils/market_operation_utils/sampler'; import { DexSample, ERC20BridgeSource, FillData, NativeFillData } from '../src/utils/market_operation_utils/types'; -import * as TypeMoq from 'typemoq'; const MAKER_TOKEN = randomAddress(); const TAKER_TOKEN = randomAddress();