Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

asset-swapper: RFQ-T firm quotes #2541

Merged
merged 36 commits into from
Apr 15, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
3cdccb7
prep asset-swapper: rename variable
feuGeneA Mar 12, 2020
403fb38
asset-swapper: support firm RFQ-T quote requests
feuGeneA Mar 12, 2020
343caa1
migrations: Deploy ERC20BridgeSampler
feuGeneA Mar 26, 2020
0e1572c
migrations: add `yarn publish:private`
feuGeneA Apr 6, 2020
4b5a02c
asset-swapper, migrations: incl. prettier in lint
feuGeneA Mar 13, 2020
5602aac
mk SwapQuoterOpt rfqtTakerApiKeyWhitelist optional
feuGeneA Apr 9, 2020
1670b21
Use NULL_ADDRESS instead of literal
feuGeneA Apr 9, 2020
98fc780
Use map idiom instead of for loop
feuGeneA Apr 9, 2020
37390e0
Use Array.push instead of Array.concat
feuGeneA Apr 9, 2020
aadcf8f
Parallelize RFQ-T with orderbook query
feuGeneA Apr 9, 2020
63bfd23
Pass QuoteRequestor via SwapQuoteOpts not ctor arg
feuGeneA Apr 9, 2020
121d51b
Use try...catch instead of Promise.catch()
feuGeneA Apr 9, 2020
5f23833
rm unused SwapQuoteRequestOpts key enableRfqt
feuGeneA Apr 8, 2020
227676c
Remove unused intentOnFilling method parameter
feuGeneA Apr 9, 2020
93872ad
Push RFQ-T opts to own SwapQuoterOpts subnamespace
feuGeneA Apr 9, 2020
3c795d3
Demote instance member to just constructor a arg
feuGeneA Apr 9, 2020
fa617d2
Demote public member to private
feuGeneA Apr 9, 2020
70add44
Push RFQ-T opts to SwapQuoterReqOpts subnamespace
feuGeneA Apr 9, 2020
5f4778c
Don't throw when RFQ-T client isn't whitelisted
feuGeneA Apr 9, 2020
8cdc05f
Promote max maker response time to a global option
feuGeneA Apr 9, 2020
39c2a75
Promote a closure to a function
feuGeneA Apr 9, 2020
eb5ec58
Add `yarn prettier` script, & call it from `lint`
feuGeneA Apr 9, 2020
264407b
In Opts types, REQUIRE rfqt SUB-options
feuGeneA Apr 9, 2020
0cb5e45
Await Axios response so we don't circumvent catch
feuGeneA Apr 10, 2020
84adbcb
asset-swapper: Mockable axios for QuoteRequestor (#2549)
Apr 11, 2020
d55108a
Eliminate unnecessary `else`
feuGeneA Apr 11, 2020
ccc9e18
Type Axios response with undefined, not void
feuGeneA Apr 11, 2020
bb15f78
Validate maker endpoint responses with JSON Schema
feuGeneA Apr 11, 2020
27ca75d
Clarify parallelization of orderbook & RFQT
feuGeneA Apr 11, 2020
b854fcd
Remove an unnecessary type annotation
feuGeneA Apr 11, 2020
58d6256
Bug fix: RFQ-T orders werent going through sorting
feuGeneA Apr 11, 2020
47ef7ff
RFQ-T: validate assetData & add more tests (#2552)
Apr 15, 2020
aee758e
Fix bug: Stop ignoring default SwapQuoteRequestOps
feuGeneA Apr 14, 2020
3bdfcb8
Update {asset-s,migrat,contract-ad}* CHANGELOGs
feuGeneA Apr 15, 2020
513ddb4
Merge branch 'development' into rfq-t
feuGeneA Apr 15, 2020
1da8f68
migrations: Add independent `yarn prettier` script
feuGeneA Apr 15, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/asset-swapper/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"@0x/orderbook": "^2.2.5",
"@0x/utils": "^5.4.1",
"@0x/web3-wrapper": "^7.0.7",
"axios": "^0.19.2",
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
"heartbeats": "^5.0.1",
"lodash": "^4.17.11"
},
Expand Down
2 changes: 2 additions & 0 deletions packages/asset-swapper/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ const DEFAULT_SWAP_QUOTER_OPTS: SwapQuoterOpts = {
},
...DEFAULT_ORDER_PRUNER_OPTS,
samplerGasLimit: 250e6,
rfqtTakerApiKeyWhitelist: [],
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
rfqtMakerEndpoints: [],
};

const DEFAULT_FORWARDER_EXTENSION_CONTRACT_OPTS: ForwarderExtensionContractOpts = {
Expand Down
2 changes: 2 additions & 0 deletions packages/asset-swapper/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export {
GetExtensionContractTypeOpts,
LiquidityForTakerMakerAssetDataPair,
MarketBuySwapQuote,
MarketOperation,
MarketSellSwapQuote,
SwapQuote,
SwapQuoteConsumerBase,
Expand All @@ -64,3 +65,4 @@ export {
} from './utils/market_operation_utils/types';
export { affiliateFeeUtils } from './utils/affiliate_fee_utils';
export { ProtocolFeeUtils } from './utils/protocol_fee_utils';
export { QuoteRequestor } from './utils/quote_requestor';
43 changes: 37 additions & 6 deletions packages/asset-swapper/src/swap_quoter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { DexOrderSampler } from './utils/market_operation_utils/sampler';
import { orderPrunerUtils } from './utils/order_prune_utils';
import { OrderStateUtils } from './utils/order_state_utils';
import { ProtocolFeeUtils } from './utils/protocol_fee_utils';
import { QuoteRequestor } from './utils/quote_requestor';
import { sortingUtils } from './utils/sorting_utils';
import { SwapQuoteCalculator } from './utils/swap_quote_calculator';

Expand All @@ -36,12 +37,15 @@ export class SwapQuoter {
public readonly expiryBufferMs: number;
public readonly chainId: number;
public readonly permittedOrderFeeTypes: Set<OrderPrunerPermittedFeeTypes>;
public readonly rfqtTakerApiKeyWhitelist: string[];
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
public readonly rfqtMakerEndpoints: string[];
private readonly _contractAddresses: ContractAddresses;
private readonly _protocolFeeUtils: ProtocolFeeUtils;
private readonly _swapQuoteCalculator: SwapQuoteCalculator;
private readonly _devUtilsContract: DevUtilsContract;
private readonly _marketOperationUtils: MarketOperationUtils;
private readonly _orderStateUtils: OrderStateUtils;
private readonly _quoteRequestor: QuoteRequestor;

/**
* Instantiates a new SwapQuoter instance given existing liquidity in the form of orders and feeOrders.
Expand Down Expand Up @@ -144,7 +148,12 @@ export class SwapQuoter {
*
* @return An instance of SwapQuoter
*/
constructor(supportedProvider: SupportedProvider, orderbook: Orderbook, options: Partial<SwapQuoterOpts> = {}) {
constructor(
supportedProvider: SupportedProvider,
orderbook: Orderbook,
options: Partial<SwapQuoterOpts> = {},
quoteRequestor?: QuoteRequestor,
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
) {
const {
chainId,
expiryBufferMs,
Expand All @@ -161,10 +170,13 @@ export class SwapQuoter {
this.orderbook = orderbook;
this.expiryBufferMs = expiryBufferMs;
this.permittedOrderFeeTypes = permittedOrderFeeTypes;
this.rfqtTakerApiKeyWhitelist = options.rfqtTakerApiKeyWhitelist || [];
this.rfqtMakerEndpoints = options.rfqtMakerEndpoints || [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rfqtMakerEndpoints seems to only be used to construct a QuoteRequestor, does it need to be set as a public accessor?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we asserting they are uris?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed the rfqtMakerEndpoints bit in 3c795d3.

We are not directly asserting that they are URI's. We're just passing them through to the quote requestor, and letting it fail silently if they're not valid.

this._contractAddresses = options.contractAddresses || getContractAddressesForChainOrThrow(chainId);
this._devUtilsContract = new DevUtilsContract(this._contractAddresses.devUtils, provider);
this._protocolFeeUtils = new ProtocolFeeUtils(constants.PROTOCOL_FEE_UTILS_POLLING_INTERVAL_IN_MS);
this._orderStateUtils = new OrderStateUtils(this._devUtilsContract);
this._quoteRequestor = quoteRequestor || new QuoteRequestor(this.rfqtMakerEndpoints);
const sampler = new DexOrderSampler(
new IERC20BridgeSamplerContract(this._contractAddresses.erc20BridgeSampler, this.provider, {
gas: samplerGasLimit,
Expand Down Expand Up @@ -523,10 +535,29 @@ export class SwapQuoter {
gasPrice = await this._protocolFeeUtils.getGasPriceEstimationOrThrowAsync();
}
// get the relevant orders for the makerAsset
let prunedOrders = await this._getSignedOrdersAsync(makerAssetData, takerAssetData);
let orders = await this._getSignedOrdersAsync(makerAssetData, takerAssetData);
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
if (options.intentOnFilling && options.apiKey) {
if (!this.rfqtTakerApiKeyWhitelist.includes(options.apiKey)) {
throw new Error('API key not permissioned for RFQ-T');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a throwable validation at this layer? Or should it fail silently and give a normal quote?

If someone gets delisted they're going to start getting Errors rather than quotes, seems quite disruptive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agree that we should fallback to providing a regular quote in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 5f4778c

}
if (!options.takerAddress || options.takerAddress === '0x0000000000000000000000000000000000000000') {
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
throw new Error('RFQ-T requests must specify a taker address');
}
orders = orders.concat(
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
await this._quoteRequestor.requestRfqtFirmQuotesAsync(
makerAssetData,
takerAssetData,
assetFillAmount,
marketOperation,
options.intentOnFilling,
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
options.apiKey,
options.takerAddress,
),
);
}
// if no native orders, pass in a dummy order for the sampler to have required metadata for sampling
if (prunedOrders.length === 0) {
prunedOrders = [
if (orders.length === 0) {
orders = [
createDummyOrderForSampler(makerAssetData, takerAssetData, this._contractAddresses.uniswapBridge),
];
}
Expand All @@ -535,14 +566,14 @@ export class SwapQuoter {

if (marketOperation === MarketOperation.Buy) {
swapQuote = await this._swapQuoteCalculator.calculateMarketBuySwapQuoteAsync(
prunedOrders,
orders,
assetFillAmount,
gasPrice,
calculateSwapQuoteOpts,
);
} else {
swapQuote = await this._swapQuoteCalculator.calculateMarketSellSwapQuoteAsync(
prunedOrders,
orders,
assetFillAmount,
gasPrice,
calculateSwapQuoteOpts,
Expand Down
6 changes: 6 additions & 0 deletions packages/asset-swapper/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ export interface SwapQuoteOrdersBreakdown {
*/
export interface SwapQuoteRequestOpts extends CalculateSwapQuoteOpts {
gasPrice?: BigNumber;
takerAddress?: string;
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
apiKey?: string;
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
enableRfqt?: boolean;
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
intentOnFilling?: boolean;
}

/**
Expand All @@ -213,6 +217,8 @@ export interface SwapQuoterOpts extends OrderPrunerOpts {
contractAddresses?: ContractAddresses;
samplerGasLimit?: number;
liquidityProviderRegistryAddress?: string;
rfqtTakerApiKeyWhitelist: string[];
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
rfqtMakerEndpoints?: string[];
}

/**
Expand Down
99 changes: 99 additions & 0 deletions packages/asset-swapper/src/utils/quote_requestor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { assetDataUtils, SignedOrder } from '@0x/order-utils';
import { ERC20AssetData } from '@0x/types';
import { BigNumber, logUtils } from '@0x/utils';
import Axios, { AxiosResponse } from 'axios';

import { MarketOperation } from '../types';

/**
* Request quotes from RFQ-T providers
*/
export class QuoteRequestor {
private readonly _rfqtMakerEndpoints: string[];

constructor(rfqtMakerEndpoints: string[]) {
this._rfqtMakerEndpoints = rfqtMakerEndpoints;
}

public async requestRfqtFirmQuotesAsync(
makerAssetData: string,
takerAssetData: string,
assetFillAmount: BigNumber,
marketOperation: MarketOperation,
intentOnFilling: boolean,
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
takerApiKey: string,
takerAddress: string,
): Promise<SignedOrder[]> {
const makerEndpointMaxResponseTimeMs = 1000;
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved

const getTokenAddressOrThrow = (assetData: string): string => {
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
const decodedAssetData = assetDataUtils.decodeAssetDataOrThrow(assetData);
if (decodedAssetData.hasOwnProperty('tokenAddress')) {
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
// type cast necessary here as decodeAssetDataOrThrow returns
// an AssetData object, which doesn't necessarily contain a
// token address. (it could possibly be a StaticCallAssetData,
// which lacks an address.) so we'll just assume it's a token
// here. should be safe, with the enclosing guard condition
// and subsequent error.
// tslint:disable-next-line:no-unnecessary-type-assertion
return (decodedAssetData as ERC20AssetData).tokenAddress;
} else {
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
throw new Error(
`Decoded asset data (${JSON.stringify(decodedAssetData)}) does not contain a token address`,
);
}
};

const buyToken = getTokenAddressOrThrow(makerAssetData);
const sellToken = getTokenAddressOrThrow(takerAssetData);

// create an array of promises for quote responses, using "undefined"
// as a placeholder for failed requests.

const responsePromises: Array<Promise<AxiosResponse<SignedOrder> | undefined>> = [];
for (const rfqtMakerEndpoint of this._rfqtMakerEndpoints) {
responsePromises.push(
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
Axios.get(`${rfqtMakerEndpoint}/quote`, {
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
headers: { '0x-api-key': takerApiKey },
params: {
sellToken,
buyToken,
buyAmount: marketOperation === MarketOperation.Buy ? assetFillAmount.toString() : undefined,
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
sellAmount: marketOperation === MarketOperation.Sell ? assetFillAmount.toString() : undefined,
takerAddress,
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
},
timeout: makerEndpointMaxResponseTimeMs,
}).catch(err => {
logUtils.warn(
`Failed to get RFQ-T quote from market maker endpoint ${rfqtMakerEndpoint} for API key ${takerApiKey} for taker address ${takerAddress}: ${JSON.stringify(
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
err,
)}`,
);
return undefined;
}),
);
}

const responsesIfDefined: Array<AxiosResponse<SignedOrder> | undefined> = await Promise.all(responsePromises);

const responses: Array<AxiosResponse<SignedOrder>> = responsesIfDefined.filter(
feuGeneA marked this conversation as resolved.
Show resolved Hide resolved
(respIfDefd): respIfDefd is AxiosResponse<SignedOrder> => respIfDefd !== undefined,
);

const ordersWithStringInts = responses.map(response => response.data); // not yet BigNumber
steveklebanoff marked this conversation as resolved.
Show resolved Hide resolved

const orders: SignedOrder[] = ordersWithStringInts.map(orderWithStringInts => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Find a helper function do this for us. @steveklebanoff ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeliing they exist in @0x/connect

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dekz Are you open to exporting typeConverters or orderParsingUtils from the @0x/orderbook package?

https://github.com/0xProject/0x-monorepo/blob/master/packages/connect/src/utils/type_converters.ts#L24

Or is there somewhere else where this is already defined & exported?

return {
...orderWithStringInts,
makerAssetAmount: new BigNumber(orderWithStringInts.makerAssetAmount),
takerAssetAmount: new BigNumber(orderWithStringInts.takerAssetAmount),
makerFee: new BigNumber(orderWithStringInts.makerFee),
takerFee: new BigNumber(orderWithStringInts.takerFee),
expirationTimeSeconds: new BigNumber(orderWithStringInts.expirationTimeSeconds),
salt: new BigNumber(orderWithStringInts.salt),
};
});

return orders;
}
}