From a55f22ee866f0ce8a8e5164829bc511838019a47 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Thu, 7 May 2020 20:16:21 -0400 Subject: [PATCH] feat: RFQ-T follow-ups (#201) * Adapt to asset-swapper: expiry in indicative quote * Relay asset-swapper info logs to app's logger This will enable the logging of aggregate time spent waiting on RFQ-T makers in each request. * Adapt to asset-swapper: offerings, not endpoints Rather than "RFQ-T maker endpoints," asset-swapper now has the notion of "RFQ-T maker asset offerings," incorporating "supported asset pairs," so that time isn't wasted requesting quotes for unsupported asset pairs. * Make RFQT API key env var handling conventional That is, following the convention in the file. * Skip buy requests for RFQ-T * test: lengthen timeout of dependency startup This was done to prevent test timeouts, which were experienced in CI, at the recommendation of Alex Towle. * Log the basis for indicative quotes * test: lengthen timeout of dependency startup This was done to prevent test timeouts, which were experienced in CI, at the recommendation of Alex Towle. * Assert that RFQT maker asset pairs don't dupe Assert that an RFQ-T maker's asset offerings specification doesn't include an asset pair with identical assets. Addresses review comment https://github.com/0xProject/0x-api/pull/201#discussion_r417775612 * Pin asset-swapper to monorepo `development` https://github.com/0xProject/0x-monorepo/commit/ff1811ef9035fc0ec6d76e2f3fab0296454092fd * test: lengthen timeout of dependency startup This was done to prevent test timeouts, which were experienced in CI, at the recommendation of Alex Towle. --- package.json | 4 +-- src/config.ts | 54 +++++++++++++++++++++++++++++++---- src/constants.ts | 1 + src/handlers/swap_handlers.ts | 13 ++++++++- src/services/swap_service.ts | 7 +++-- src/types.ts | 1 + test/app_test.ts | 19 ++++++++++++ test/utils/deployment.ts | 2 +- test/utils/mocks.ts | 1 + yarn.lock | 4 +-- 10 files changed, 93 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index e05d2b512..230645d11 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "clean:ts": "shx rm -rf lib", "clean:docker": "shx rm -rf 0x_mesh/db", "build": "tsc -p tsconfig.json", - "test": "ETHEREUM_RPC_URL=http://localhost:8545 CHAIN_ID=1337 RFQT_API_KEY_WHITELIST='koolApiKey1,koolApikey2' RFQT_MAKER_ENDPOINTS='https://mock-rfqt1.club' META_TXN_RELAY_ADDRESS=0x9eFCa436873b55a0d6AEa260f92DE50150360dca META_TXN_RELAY_PRIVATE_KEY=82b9c3b8d45f608badd8fda250a0d95088381540e850734519b659e1e1ac3e71 mocha --require source-map-support/register --require make-promises-safe lib/test/**/*_test.js --timeout 200000 --exit", + "test": "ETHEREUM_RPC_URL=http://localhost:8545 CHAIN_ID=1337 RFQT_API_KEY_WHITELIST='koolApiKey1,koolApikey2' RFQT_MAKER_ASSET_OFFERINGS='{\"https://mock-rfqt1.club\": [[\"0x871dd7c2b4b25e1aa18728e9d5f2af4c4e431f5c\",\"0x0b1ba0af832d7c05fd64161e0db78e85978e8082\"]]}' META_TXN_RELAY_ADDRESS=0x9eFCa436873b55a0d6AEa260f92DE50150360dca META_TXN_RELAY_PRIVATE_KEY=82b9c3b8d45f608badd8fda250a0d95088381540e850734519b659e1e1ac3e71 mocha --require source-map-support/register --require make-promises-safe lib/test/**/*_test.js --timeout 200000 --exit", "dev": "nodemon -r dotenv/config src/index.ts | pino-pretty", "dev:service:http": "nodemon -r dotenv/config src/runners/http_service_runner.ts | pino-pretty", "dev:service:sra_http": "nodemon -r dotenv/config src/runners/http_sra_service_runner.ts | pino-pretty", @@ -116,7 +116,7 @@ }, "dependencies": { "@0x/assert": "^3.0.4", - "@0x/asset-swapper": "0xProject/gitpkg-registry#0x-asset-swapper-v4.4.0-fb0311e67", + "@0x/asset-swapper": "0xProject/gitpkg-registry#0x-asset-swapper-v4.4.0-ff1811ef9", "@0x/connect": "^6.0.4", "@0x/contract-addresses": "0xProject/gitpkg-registry#0x-contract-addresses-v4.9.0-fb0311e67", "@0x/contract-wrappers": "0xProject/gitpkg-registry#0x-contract-wrappers-v13.6.3-fb0311e67", diff --git a/src/config.ts b/src/config.ts index e975b4e78..64c4f8d42 100644 --- a/src/config.ts +++ b/src/config.ts @@ -1,6 +1,6 @@ // tslint:disable:custom-no-magic-numbers import { assert } from '@0x/assert'; -import { ERC20BridgeSource, SwapQuoteRequestOpts } from '@0x/asset-swapper'; +import { ERC20BridgeSource, RfqtMakerAssetOfferings, SwapQuoteRequestOpts } from '@0x/asset-swapper'; import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; import * as validateUUID from 'uuid-validate'; @@ -10,6 +10,7 @@ import { DEFAULT_LOCAL_POSTGRES_URI, DEFAULT_LOGGER_INCLUDE_TIMESTAMP, DEFAULT_QUOTE_SLIPPAGE_PERCENTAGE, + DEFAULT_RFQT_SKIP_BUY_REQUESTS, NULL_ADDRESS, NULL_BYTES, } from './constants'; @@ -30,6 +31,7 @@ enum EnvVarType { FeeAssetData, NonEmptyString, APIKeys, + RfqtMakerAssetOfferings, } // Network port to listen on @@ -149,11 +151,22 @@ export const LIQUIDITY_POOL_REGISTRY_ADDRESS: string | undefined = _.isEmpty( EnvVarType.ETHAddressHex, ); -export const RFQT_API_KEY_WHITELIST: string[] = - process.env.RFQT_API_KEY_WHITELIST === undefined ? [] : process.env.RFQT_API_KEY_WHITELIST.split(','); +export const RFQT_API_KEY_WHITELIST: string[] = _.isEmpty(process.env.RFQT_API_KEY_WHITELIST) + ? [] + : assertEnvVarType('RFQT_API_KEY_WHITELIST', process.env.RFQT_API_KEY_WHITELIST, EnvVarType.StringList); + +export const RFQT_MAKER_ASSET_OFFERINGS: RfqtMakerAssetOfferings = _.isEmpty(process.env.RFQT_MAKER_ASSET_OFFERINGS) + ? {} + : assertEnvVarType( + 'RFQT_MAKER_ASSET_OFFERINGS', + process.env.RFQT_MAKER_ASSET_OFFERINGS, + EnvVarType.RfqtMakerAssetOfferings, + ); -export const RFQT_MAKER_ENDPOINTS: string[] = - process.env.RFQT_MAKER_ENDPOINTS === undefined ? [] : process.env.RFQT_MAKER_ENDPOINTS.split(','); +// tslint:disable-next-line:boolean-naming +export const RFQT_SKIP_BUY_REQUESTS: boolean = _.isEmpty(process.env.RFQT_SKIP_BUY_REQUESTS) + ? DEFAULT_RFQT_SKIP_BUY_REQUESTS + : assertEnvVarType('RFQT_SKIP_BUY_REQUESTS', process.env.RFQT_SKIP_BUY_REQUESTS, EnvVarType.Boolean); // Whitelisted 0x API keys that can use the meta-txn /submit endpoint export const WHITELISTED_API_KEYS_META_TXN_SUBMIT: string[] = @@ -297,6 +310,37 @@ function assertEnvVarType(name: string, value: any, expectedType: EnvVarType): a } }); return apiKeys; + + case EnvVarType.RfqtMakerAssetOfferings: + const offerings: RfqtMakerAssetOfferings = JSON.parse(value); + // tslint:disable-next-line:forin + for (const makerEndpoint in offerings) { + assert.isWebUri('market maker endpoint', makerEndpoint); + + const assetOffering = offerings[makerEndpoint]; + assert.isArray(`value in maker endpoint mapping, for index ${makerEndpoint},`, assetOffering); + assetOffering.forEach((assetPair, i) => { + assert.isArray(`asset pair array ${i} for maker endpoint ${makerEndpoint}`, assetPair); + assert.assert( + assetPair.length === 2, + `asset pair array ${i} for maker endpoint ${makerEndpoint} does not consist of exactly two elements.`, + ); + assert.isETHAddressHex( + `first token address for asset pair ${i} for maker endpoint ${makerEndpoint}`, + assetPair[0], + ); + assert.isETHAddressHex( + `second token address for asset pair ${i} for maker endpoint ${makerEndpoint}`, + assetPair[1], + ); + assert.assert( + assetPair[0] !== assetPair[1], + `asset pair array ${i} for maker endpoint ${makerEndpoint} has identical assets`, + ); + }); + } + return offerings; + default: throw new Error(`Unrecognised EnvVarType: ${expectedType} encountered for variable ${name}.`); } diff --git a/src/constants.ts b/src/constants.ts index fda2059ab..4fec3302d 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -36,6 +36,7 @@ export const PROTOCOL_FEE_UTILS_POLLING_INTERVAL_IN_MS = 6000; export const UNWRAP_QUOTE_GAS = new BigNumber(60000); export const WRAP_QUOTE_GAS = new BigNumber(40000); export const ONE_GWEI = new BigNumber(1000000000); +export const DEFAULT_RFQT_SKIP_BUY_REQUESTS = true; // API namespaces export const SRA_PATH = '/sra/v3'; diff --git a/src/handlers/swap_handlers.ts b/src/handlers/swap_handlers.ts index 2d8fd7b18..1f5a53c34 100644 --- a/src/handlers/swap_handlers.ts +++ b/src/handlers/swap_handlers.ts @@ -57,7 +57,18 @@ export class SwapHandlers { const params = parseGetSwapQuoteRequestParams(req, 'price'); params.skipValidation = true; const quote = await this._calculateSwapQuoteAsync(params); - const { price, value, gasPrice, gas, protocolFee, buyAmount, sellAmount, sources } = quote; + const { price, value, gasPrice, gas, protocolFee, buyAmount, sellAmount, sources, orders } = quote; + logger.info({ + indicativeQuoteServed: { + taker: params.takerAddress, + apiKey: params.apiKey, + buyToken: params.buyToken, + sellToken: params.sellToken, + buyAmount: params.buyAmount, + sellAmount: params.sellAmount, + makers: orders.map(o => o.makerAddress), + }, + }); res.status(HttpStatus.OK).send({ price, value, gasPrice, gas, protocolFee, buyAmount, sellAmount, sources }); } // tslint:disable-next-line:prefer-function-over-method diff --git a/src/services/swap_service.ts b/src/services/swap_service.ts index b38616680..d6caa007a 100644 --- a/src/services/swap_service.ts +++ b/src/services/swap_service.ts @@ -19,7 +19,8 @@ import { CHAIN_ID, LIQUIDITY_POOL_REGISTRY_ADDRESS, RFQT_API_KEY_WHITELIST, - RFQT_MAKER_ENDPOINTS, + RFQT_MAKER_ASSET_OFFERINGS, + RFQT_SKIP_BUY_REQUESTS, } from '../config'; import { GAS_LIMIT_BUFFER_PERCENTAGE, @@ -52,8 +53,10 @@ export class SwapService { liquidityProviderRegistryAddress: LIQUIDITY_POOL_REGISTRY_ADDRESS, rfqt: { takerApiKeyWhitelist: RFQT_API_KEY_WHITELIST, - makerEndpoints: RFQT_MAKER_ENDPOINTS, + makerAssetOfferings: RFQT_MAKER_ASSET_OFFERINGS, + skipBuyRequests: RFQT_SKIP_BUY_REQUESTS, warningLogger: logger.warn.bind(logger), + infoLogger: logger.info.bind(logger), }, permittedOrderFeeTypes: new Set([OrderPrunerPermittedFeeTypes.NoFees]), }; diff --git a/src/types.ts b/src/types.ts index 37cd92fe8..af68359ff 100644 --- a/src/types.ts +++ b/src/types.ts @@ -453,6 +453,7 @@ export interface CalculateSwapQuoteParams { rfqt?: { intentOnFilling?: boolean; isIndicative?: boolean; + skipBuyRequests?: boolean; }; skipValidation: boolean; } diff --git a/test/app_test.ts b/test/app_test.ts index f91c05b86..f8270ac04 100644 --- a/test/app_test.ts +++ b/test/app_test.ts @@ -286,6 +286,25 @@ describe(SUITE_NAME, () => { expect(validationErrors.length).to.eql(1); expect(validationErrors[0].reason).to.eql('INSUFFICIENT_ASSET_LIQUIDITY'); }); + it("should fail when it's a buy order and those are disabled (which is the default)", async () => { + const buyAmount = new BigNumber(100000000000000000); + + const wethContract = new WETH9Contract(contractAddresses.etherToken, provider); + await wethContract + .approve(contractAddresses.erc20Proxy, new BigNumber(0)) + .sendTransactionAsync({ from: takerAddress }); + + const appResponse = await request(app) + .get( + `${SWAP_PATH}/quote?buyToken=ZRX&sellToken=WETH&buyAmount=${buyAmount.toString()}&takerAddress=${takerAddress}&intentOnFilling=true&excludedSources=Uniswap,Eth2Dai,Kyber,LiquidityProvider&skipValidation=true`, + ) + .set('0x-api-key', 'koolApiKey1') + .expect(HttpStatus.BAD_REQUEST) + .expect('Content-Type', /json/); + const validationErrors = appResponse.body.validationErrors; + expect(validationErrors.length).to.eql(1); + expect(validationErrors[0].reason).to.eql('INSUFFICIENT_ASSET_LIQUIDITY'); + }); it('should succeed when taker can not actually fill but we skip validation', async () => { const sellAmount = new BigNumber(100000000000000000); diff --git a/test/utils/deployment.ts b/test/utils/deployment.ts index 89774dbf1..1a7766140 100644 --- a/test/utils/deployment.ts +++ b/test/utils/deployment.ts @@ -206,6 +206,6 @@ async function waitForDependencyStartupAsync(logStream: ChildProcessWithoutNullS }); setTimeout(() => { reject(new Error('Timed out waiting for dependency logs')); - }, 180000); // tslint:disable-line:custom-no-magic-numbers + }, 300000); // tslint:disable-line:custom-no-magic-numbers }); } diff --git a/test/utils/mocks.ts b/test/utils/mocks.ts index 9f2d3ef53..1eefe3301 100644 --- a/test/utils/mocks.ts +++ b/test/utils/mocks.ts @@ -24,4 +24,5 @@ export const rfqtIndicativeQuoteResponse = { takerAssetAmount: '100000000000000000', makerAssetData: '0xf47261b0000000000000000000000000871dd7c2b4b25e1aa18728e9d5f2af4c4e431f5c', takerAssetData: '0xf47261b00000000000000000000000000b1ba0af832d7c05fd64161e0db78e85978e8082', + expirationTimeSeconds: '1903620548', // in the year 2030 }; diff --git a/yarn.lock b/yarn.lock index 314a3071e..f5ea27659 100644 --- a/yarn.lock +++ b/yarn.lock @@ -32,9 +32,9 @@ lodash "^4.17.11" valid-url "^1.0.9" -"@0x/asset-swapper@0xProject/gitpkg-registry#0x-asset-swapper-v4.4.0-fb0311e67": +"@0x/asset-swapper@0xProject/gitpkg-registry#0x-asset-swapper-v4.4.0-ff1811ef9": version "4.4.0" - resolved "https://codeload.github.com/0xProject/gitpkg-registry/tar.gz/d2a51170beec721e992ecdefcaab79a8ae4e6ab0" + resolved "https://codeload.github.com/0xProject/gitpkg-registry/tar.gz/64535d1f229d124c2d57102f6a9d28a9e94cc820" dependencies: "@0x/assert" "^3.0.7" "@0x/contract-addresses" "^4.9.0"