-
Notifications
You must be signed in to change notification settings - Fork 465
asset-swapper: RFQ-T firm quotes #2541
Changes from 21 commits
3cdccb7
403fb38
343caa1
0e1572c
4b5a02c
5602aac
1670b21
98fc780
37390e0
aadcf8f
63bfd23
121d51b
5f23833
227676c
93872ad
3c795d3
fa617d2
70add44
5f4778c
8cdc05f
39c2a75
eb5ec58
264407b
0cb5e45
84adbcb
d55108a
ccc9e18
bb15f78
27ca75d
b854fcd
58d6256
47ef7ff
aee758e
3bdfcb8
513ddb4
1da8f68
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
import { assetDataUtils, SignedOrder } from '@0x/order-utils'; | ||
import { ERC20AssetData } from '@0x/types'; | ||
import { BigNumber, logUtils } from '@0x/utils'; | ||
import Axios, { AxiosResponse } from 'axios'; | ||
import * as _ from 'lodash'; | ||
|
||
import { constants } from '../constants'; | ||
import { MarketOperation, RfqtFirmQuoteRequestOpts } from '../types'; | ||
|
||
/** | ||
* Request quotes from RFQ-T providers | ||
*/ | ||
|
||
function getTokenAddressOrThrow(assetData: string): string { | ||
const decodedAssetData = assetDataUtils.decodeAssetDataOrThrow(assetData); | ||
if (decodedAssetData.hasOwnProperty('tokenAddress')) { | ||
// 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 { | ||
throw new Error(`Decoded asset data (${JSON.stringify(decodedAssetData)}) does not contain a token address`); | ||
} | ||
} | ||
|
||
export class QuoteRequestor { | ||
private readonly _rfqtMakerEndpoints: string[]; | ||
|
||
constructor(rfqtMakerEndpoints: string[]) { | ||
this._rfqtMakerEndpoints = rfqtMakerEndpoints; | ||
} | ||
|
||
public async requestRfqtFirmQuotesAsync( | ||
makerAssetData: string, | ||
takerAssetData: string, | ||
assetFillAmount: BigNumber, | ||
marketOperation: MarketOperation, | ||
takerApiKey: string, | ||
takerAddress: string, | ||
options?: Partial<RfqtFirmQuoteRequestOpts>, | ||
): Promise<SignedOrder[]> { | ||
const { makerEndpointMaxResponseTimeMs } = _.merge({}, constants.DEFAULT_RFQT_FIRM_QUOTE_REQUEST_OPTS, options); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So you like lodash now, huh? 😆 If you're using it here, would prefer to use I'm ok leaving as it though ❤️ |
||
|
||
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 responsesIfDefined: Array<void | AxiosResponse<SignedOrder>> = await Promise.all( | ||
feuGeneA marked this conversation as resolved.
Show resolved
Hide resolved
|
||
this._rfqtMakerEndpoints.map(async rfqtMakerEndpoint => { | ||
try { | ||
return Axios.get<SignedOrder>(`${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, | ||
sellAmount: | ||
marketOperation === MarketOperation.Sell ? assetFillAmount.toString() : undefined, | ||
takerAddress, | ||
}, | ||
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 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 => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Find a helper function do this for us. @steveklebanoff ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a feeliing they exist in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dekz Are you open to exporting 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; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,7 @@ | |
"build": "tsc -b", | ||
"build:ci": "yarn build", | ||
"clean": "shx rm -rf lib ${npm_package_config_snapshot_name} ${npm_package_config_snapshot_name}-*.zip", | ||
"lint": "tslint --format stylish --project .", | ||
"lint": "tslint --format stylish --project . && prettier --check 'src/**/*.{ts,tsx,json,md}' --config ../../.prettierrc", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like you agreed to keep There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 1da8f68 |
||
"fix": "tslint --fix --format stylish --project .", | ||
"test": "yarn run_mocha", | ||
"test:circleci": "yarn test", | ||
|
@@ -28,7 +28,8 @@ | |
"build:snapshot:docker": "docker build --tag ${npm_package_config_docker_snapshot_name}:${npm_package_version} --tag ${npm_package_config_docker_snapshot_name}:latest .", | ||
"publish:snapshot": "aws s3 cp ${npm_package_config_snapshot_name}-${npm_package_version}.zip ${npm_package_config_s3_snapshot_bucket} && aws s3 cp ${npm_package_config_s3_snapshot_bucket}/${npm_package_config_snapshot_name}-${npm_package_version}.zip ${npm_package_config_s3_snapshot_bucket}/${npm_package_config_snapshot_name}-latest.zip", | ||
"publish:snapshot:docker": "docker push ${npm_package_config_docker_snapshot_name}:latest && docker push ${npm_package_config_docker_snapshot_name}:${npm_package_version}", | ||
"test_contract_configs": "node ./lib/test_contract_configs.js" | ||
"test_contract_configs": "node ./lib/test_contract_configs.js", | ||
"publish:private": "yarn build && gitpkg publish" | ||
}, | ||
"config": { | ||
"s3_snapshot_bucket": "s3://ganache-snapshots.0x.org", | ||
|
@@ -42,6 +43,9 @@ | |
"0x-migrate": "bin/0x-migrate.js" | ||
}, | ||
"license": "Apache-2.0", | ||
"gitpkg": { | ||
"registry": "[email protected]:0xProject/gitpkg-registry.git" | ||
}, | ||
"devDependencies": { | ||
"@0x/dev-utils": "^3.2.1", | ||
"@0x/ts-doc-gen": "^0.0.22", | ||
|
@@ -50,6 +54,7 @@ | |
"@types/yargs": "^11.0.0", | ||
"chai": "^4.0.1", | ||
"dirty-chai": "^2.0.1", | ||
"gitpkg": "https://github.com/0xProject/gitpkg.git", | ||
"make-promises-safe": "^1.1.0", | ||
"mocha": "^6.2.0", | ||
"npm-run-all": "^4.1.2", | ||
|
@@ -69,6 +74,7 @@ | |
"@0x/contracts-dev-utils": "^1.3.3", | ||
"@0x/contracts-erc1155": "^2.1.5", | ||
"@0x/contracts-erc20": "^3.1.5", | ||
"@0x/contracts-erc20-bridge-sampler": "^1.5.1", | ||
"@0x/contracts-erc721": "^3.1.5", | ||
"@0x/contracts-exchange": "^3.2.5", | ||
"@0x/contracts-exchange-forwarder": "^4.2.5", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have the established pattern of having
prettier
as it's own script: https://github.com/0xProject/0x-monorepo/blob/development/packages/orderbook/package.json#L25There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I introduced a
prettier
script in eb5ec58, but I also kept its invocation in thelint
script.I feel pretty strongly that
yarn lint
should run everything that CI'sstatic-tests
runs. I'm open to having my mind changed if anyone else feels strongly about it, but that's why I did it this way.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's pretty weird to have different
lint
behavior forasset-swapper
compared to all other packages. IMHO it's important to be consistent here.Unless we want to make this change wholesale across all repos (a can of worms I don't think we should open right now), I think we should make this consistent.