Add trade quote utility #60

cgewecke · 2021-06-08T07:18:44Z

TODO

Improve tests
Rationalize api ... like where chainId is set / should set.js cache token list, etc
One pager on next steps to finalize api and answer open questions
Too many dependencies (e.g. services that need to succeed for a trade quote to exec)

Adds utilities for Ethereum and Polygon chains to

generate trade (swap) quotes for SetToken component pairs (from ZeroEx)
generate lists and maps of token metadata (from CoinGecko, mostly)
get gas prices for a chain by speed
get coin prices for tokens (from CoinGecko)

A tradeQuote property has been added to the Set object and the Set config now takes an (optional) zeroExApiKey property. Usage looks like:

const config = {
 ...
 zeroExApiKey: process.env.ZERO_EX_API_KEY
}

const set = new Set(config);

const quote = await set.trade.fetchTradeQuoteAsync(
  fromToken: Address
  toToken: Address
  fromTokenDecimals: number
  toTokenDecimals: number,
  rawAmount: string,
  setTokenAddress: Address,
  setTokenAPI: SetTokenAPI,
  gasPrice: number // in gwei, ex: 20

  // Optional
  slippagePercentage?: number, // default: 2
  isFirmQuote?: boolean, // default: true
  feePercentage?: number, // default: 0
  feeRecipient?: Address,  // default: '0xD3D555Bb655AcBA9452bfC6D7cEa8cC7b3628C55'
  excludedSources?:  string[], // default: ['Kyber', 'Eth2Dai', 'Uniswap', 'Mesh']
);

Example Quote

{
  from: '0x1494ca1f11d487c2bbe4543e90080aeba4ba3c2b',
  fromTokenAddress: '0x9f8f72aa9304c8b593d555f12ef6589cc3a579a2',
  toTokenAddress: '0x0bc529c00c6401aef6d220be8c6ea1667f6ad93e',
  exchangeAdapterName: 'ZeroExApiAdapterV3',
  calldata: '0x415565b00000000000000000000000009f8f72aa9304c8b593d555f12ef6589cc3a579a2',
  gas: '315000',
  gasPrice: '61',
  slippagePercentage: '2.00%',
  fromTokenAmount: '1126868991563',
  toTokenAmount: '91245821628',
  display: {
     inputAmountRaw: '.5',
     inputAmount: '500000000000000000',
     quoteAmount: '499999999999793729',
     fromTokenDisplayAmount: '0.4999999999997937',
     toTokenDisplayAmount: '0.04131269116050703',
     fromTokenPriceUsd: '$1,597.20',
     toTokenPriceUsd: '$1,614.79',
     gasCostsUsd: '$47.91',
     gasCostsChainCurrency: '0.0192150 ETH',
     feePercentage: '0.00%',
     slippage: '-1.10%',
  },
}

src/api/utils/tradeQuote/tradequote.ts

cgewecke · 2021-06-08T16:59:10Z

One idea for api is

const set = new Set({
  ...
  chainId // We could get this from the provider but it's async ...
  zeroExApiKey?
})

// Generate and cache tokenList / Map for chainId
async set.trade.fetchTradeQuote(params....)
async set.trade.fetchTokenList() 
async set.trade.fetchTokenMap()
async set.trade.fetchCoinPrices(params...)
async set.trade.fetchGasPrice(speed?: string)

src/api/utils/tradeQuote/tradequote.ts

src/Set.ts

src/api/index.ts

src/api/utils/tradeQuote/coingecko.ts

src/api/utils/tradeQuote/gasOracle.ts

src/api/utils/tradeQuote/tradequote.ts

src/api/utils/tradeQuote/zeroex.ts

asoong

This is great and thorough. One question: is the token mapping still being used? I couldn't find it as I was looking through.

Can we have the same redundancies around fetching gas price as we do for CoinGecko returning 0? Also considering redundancies for when the entire API call to CoinGecko or GasNow has status code > 400

asoong · 2021-06-15T06:04:50Z

src/Set.ts

@@ -123,7 +123,7 @@ class Set {
      assertions
    );
    this.system = new SystemAPI(ethersProvider, config.controllerAddress);
-    this.trade = new TradeAPI(ethersProvider, config.tradeModuleAddress);
+    this.trade = new TradeAPI(ethersProvider, config.tradeModuleAddress, config.zeroExApiKey);


I don't know if people can generate their own API keys for 0x. We may have to supply it for it to be used by anyone else

The 0x API is configured with a list of API keys which are permitted to access RFQ-T liquidity. For the instance at api.0x.org, the 0x team is maintaining a list of trusted integrators

@asoong based on the statement from their docs it's not possible to request one. If I understand correctly it will still works but I just may not get the best deal available, right?

asoong · 2021-06-15T06:05:46Z

src/api/TradeAPI.ts

+   * @param  fromAddress          SetToken address which holds the buy / sell components
+   * @param  setToken             SetTokenAPI instance
+   * @param  gasPrice             (Optional) gasPrice to calculate gas costs with (Default: fetched from GasNow)
+   * @param  slippagePercentage   (Optional) maximum slippage, determines min receive quantity. (Default: 2%)


Did you grab these defaults from the service?

Yes... from heroku, here: https://dashboard.heroku.com/apps/set-core-production/settings

The whole number percentage is then divided by 100 before being passed to Zero Ex here:

set.js/src/api/utils/tradeQuoter.ts

Line 231 in 2c360f0

(slippagePercentage / 100),

asoong · 2021-06-15T06:08:05Z

src/api/TradeAPI.ts

+   *
+   * @return List of tradeable tokens for chain platform
+   */
+  public async fetchTokenListAsync(): Promise<CoinGeckoTokenData[]> {


Just want to confirm this is not currently being consumed? It was part of the old flow for grabbing the decimals? This might belong in a different utils type of API

We're using this stuff in the admin UI. This API is helpful I think.

asoong · 2021-06-15T06:08:31Z

src/api/TradeAPI.ts

+   *
+   * @return Map of token addresses to token metadata
+   */
+  public async fetchTokenMapAsync(): Promise<CoinGeckoTokenMap> {


asoong · 2021-06-15T06:09:33Z

src/api/TradeAPI.ts

+   *
+   * @return                           List of prices vs currencies
+   */
+  public async fetchCoinPricesAsync(


I think most of these below belong in a different API, perhaps ERC20 API or something of the sort

asoong · 2021-06-15T06:20:21Z

src/api/utils/tradeQuoter.ts

+
+    const amount = this.sanitizeAmount(options.rawAmount, options.fromTokenDecimals);
+
+    const setOnChainDetails = await options.setToken.fetchSetDetailsAsync(


I forgot we also make this call. Feels redundant since the client will have all of this information as well as the manager address.. The data only serves a check that the component is a component in the Set.

The manager address is going to be required to fetch a gas estimate since only the manager can submit the trade.

asoong · 2021-06-15T06:21:34Z

src/api/utils/tradeQuoter.ts

+    const coinGecko = new CoinGeckoDataService(chainId);
+    const coinPrices = await coinGecko.fetchCoinPrices({
+      contractAddresses: [this.chainCurrencyAddress(chainId), fromTokenAddress, toTokenAddress],
+      vsCurrencies: [ USD_CURRENCY_CODE, USD_CURRENCY_CODE, USD_CURRENCY_CODE ],


I think you only need USD currency code once, not contractAddresses.length() times

asoong · 2021-06-15T06:23:06Z

src/api/utils/tradeQuoter.ts

+    };
+  }
+
+  private sanitizeAddress(fromToken: Address, toToken: Address, fromAddress: Address) {


This sanitization was only useful so that we could map to the token list for the decimals in a standardized way. if the decimals are already being passed in then this is not necessary

asoong · 2021-06-15T06:28:30Z

test/api/TradeAPI.spec.ts

+    }
+
+    it('should call the TradeQuoter with correct params', async () => {
+      const expectedQuoteOptions = {


nit: I usually like the expected values to be defined right before the expectation (line 272)

asoong · 2021-06-15T06:29:10Z

test/api/TradeAPI.spec.ts

+
+    describe('when the rawAmount quantity is invalid', () => {
+      beforeEach(async () => {
+        subjectRawAmount = <unknown>5 as string;


What is <unknowng> 👀

TS complains if you make an incoherent cast (like number to string) ... you have to cast to unknown so it forgets 5 is a number

cgewecke · 2021-07-12T22:02:04Z

CoinGecko returning 0

Maybe this should be part of an opt in "tolerant" setting. I don't think we'd want this to quietly return 0 when fetching prices for a rebalance for example.

… sources

cgewecke · 2021-07-28T18:35:00Z

Admin merging per suggestion in Telegram. The only remainder from the review is to create an ERC20 api that houses the price fetching, token data collection stuff.

Add trade quote utility

05f9027

cgewecke force-pushed the chris/trade_quote_util branch from d2acb92 to 05f9027 Compare June 8, 2021 07:28

0.2.0

d85ae3d

cgewecke commented Jun 8, 2021

View reviewed changes

src/api/utils/tradeQuote/tradequote.ts Outdated Show resolved Hide resolved

cgewecke commented Jun 8, 2021

View reviewed changes

src/api/utils/tradeQuote/tradequote.ts Outdated Show resolved Hide resolved

asoong reviewed Jun 8, 2021

View reviewed changes

cgewecke added 2 commits June 8, 2021 16:47

Add polygon quote test

eb873b7

Use GasNow instead of EthGasStation

3548d1c

cgewecke mentioned this pull request Jun 9, 2021

Trade API: next steps #61

Open

3 tasks

cgewecke added 6 commits June 9, 2021 10:26

Make ZeroEx quoter constants TradeQuote params

eb01164

Re-order files and integrate utils into TradeAPI

4e8a0f1

Remove tokenMap dependency / add fetchTradeQuote arg validation

d7c7de7

Add gasPrice option to fetchTradeAsync / tradeQuote.generate

d317dc3

Estimate gas via tradeModuleWrapper

9ccac88

Add TradeAPI#fetchTradeQuote tests

45b0b6b

cgewecke force-pushed the chris/trade_quote_util branch from cea3ce6 to 45b0b6b Compare June 10, 2021 00:45

cgewecke added 9 commits June 9, 2021 18:01

0.2.1

9231a41

Set coin prices to zero if CoinGecko fails

42a7b20

0.2.2

d8a7256

Rename tradequote file to tradeQuoter

c7c5ac6

Make sure dist is cleared before build

b5b52f0

0.2.3

3940d82

Fix bug reading from options.slippagePercentage

8d76606

0.2.4

e4a87aa

Additional tests for TradeAPI utils and public method comments

2c360f0

This was referenced Jun 14, 2021

Make gas price oracle fault tolerant #64

Open

Internationalize trade quote currency values #65

Open

cgewecke marked this pull request as ready for review June 14, 2021 17:46

asoong reviewed Jun 15, 2021

View reviewed changes

cgewecke added 5 commits July 12, 2021 15:58

Pass fee percentage to ZeroEx api call / remove Uniswap from excluded…

140edb3

… sources

0.2.5

c0b8941

Include fee in toTokenAmount adjustment

184406f

0.2.6

324f6a4

Merge branch 'master' into chris/trade_quote_util

4c81300

cgewecke merged commit 7aeac75 into master Jul 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add trade quote utility #60

Add trade quote utility #60

cgewecke commented Jun 8, 2021 •

edited

Loading

cgewecke commented Jun 8, 2021 •

edited

Loading

asoong left a comment

asoong Jun 15, 2021

NelsonGaldeman Jul 21, 2021

asoong Jun 15, 2021

cgewecke Jul 12, 2021

asoong Jun 15, 2021

cgewecke Jul 12, 2021

asoong Jun 15, 2021

asoong Jun 15, 2021

asoong Jun 15, 2021

asoong Jun 15, 2021

asoong Jun 15, 2021

asoong Jun 15, 2021

asoong Jun 15, 2021

cgewecke Jul 12, 2021

cgewecke commented Jul 12, 2021

cgewecke commented Jul 28, 2021


		const amount = this.sanitizeAmount(options.rawAmount, options.fromTokenDecimals);

		const setOnChainDetails = await options.setToken.fetchSetDetailsAsync(

Add trade quote utility #60

Add trade quote utility #60

Conversation

cgewecke commented Jun 8, 2021 • edited Loading

cgewecke commented Jun 8, 2021 • edited Loading

asoong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgewecke commented Jul 12, 2021

cgewecke commented Jul 28, 2021

cgewecke commented Jun 8, 2021 •

edited

Loading

cgewecke commented Jun 8, 2021 •

edited

Loading