ojo-network / price-feeder Goto Github PK

The order of operations when calculating prices can be optimized making it faster and easier to read.

Current order of operations:

1. Loop converting candles to USD
   a. Find pairs to convert with
   b. Filter deviations for conversion pair
   i. Calculate TVWAP per provider
   ii. Filter all prices outside standard deviation
   c. Compute TVWAP for conversion pairs and calc conversion rate
   d. Loop over all candles converting them to USD using conversion rate
2. Filter candle deviations per provider/pair
3. Compute TVWAPS per provider (only used for REST API)
4. Compute TVWAPS per pair
   (If there are zero TVWAP prices repeat the same logic for Tickers)

Suggested order of operations:

1. Compute TVWAPS per provider/pair
2. Filter provider/pairs that are outside the standard deviation (save full list for REST API)
3. Compute TVWAPS per pair
4. Convert to USD for non-USD TVWAPS

The suggested way would substantially decrease the number of times we do currency conversions and computation of TVWAPs.

When refactoring these functions we should consider adding information about both the quote AND base denoms related to the price stored in AggregatedProviderCandles and AggregatedProviderPrices. It will make writing the convert function much easier and resolve this issue: #108

Refactor Steps:

1. Improve integration test to check final price-feeder prices against coingecko
   a. Add make command for running it
2. PR to Add base and quote denom information to stored prices as described above
3. PR to update the order we compute TVWAPS/TWAPS and convert prices to USD

Summary

Problem Definition

Instead of backporting dependencies like this:
#115

The umee branch of the price feeder needs its own dependabot, especially since we're likely going to migrate ojo to SDK0.47

• Turn off automatic backport tags for dependabot on main branch
• Turn on dependabot -> umee branch

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

          I'm not really a big fan of having two files here tbh - if we have a section in the price feeder config docs on how to add a forex asset, I think that will suffice 

Also, might want to split this into another PR

Originally posted by @adamewozniak in #25 (comment)

Summary

Implement the crescent provider for the indexer we've built

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

There are issues in the price feeder CI process. In unit-test:

3:36AM WRN Coin gecko found 0 providers for USDT
    config_test.go:766: 
        	Error Trace:	/home/runner/work/price-feeder/price-feeder/config/config_test.go:766
        	Error:      	An error is expected but got nil.
        	Test:       	TestCheckProviderMins_Invalid
--- FAIL: TestCheckProviderMins_Invalid (0.31s)
=== RUN   TestProviderWithAPIKey_Valid
3:36AM ERR failed to start currency provider tracker error="json: cannot unmarshal object into Go value of type []config.coinList"

In test-integration:

3:38AM WRN failed to get candle prices for STINJINJ provider=astroport
    provider_test.go:100: 
        	Error Trace:	/home/runner/work/price-feeder/price-feeder/tests/integration/provider_test.go:100
        	            				/home/runner/work/price-feeder/price-feeder/tests/integration/provider_test.go:66
        	            				/opt/hostedtoolcache/go/1.19.13/x64/src/runtime/asm_amd64.s:1594
        	Error:      	no candle prices
        	Test:       	TestServiceTestSuite/TestWebsocketProviders
        	Messages:   	provider astroport pair STINJINJ
--- FAIL: TestServiceTestSuite (62.47s)
    --- FAIL: TestServiceTestSuite/TestWebsocketProviders (62.47s)

Summary

Problem Definition

1. The price-feeder log output is noisy and it should be easier to run it with different log levels to reduce the noise.
2. Only serious issues that need to be immediately addressed should be output as errors. Everything else should be at a different alert level.

Proposal

Comb through all the different log output messages in the price-feeder and adjust their log levels to an appropriate level based on severity.

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

Summary

Problem Definition

Each time we release a new price feeder config version, validators have to re-configure the rest of their settings (especially their keyring)

This proposes that we separate out the providers into its own file so that they can just clone that each new asset listing and not have to mess around with their regular settigns.

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

Summary

Problem Definition

Unit tests should not hit outside services. This will prevent the required unit tests check from blocking PRs when the coingecko API is not responding.

OR - we should mock the API response in the unit tests and move anything testing the API request to an integration test if we still need it.

Sample failure output:

=== RUN TestCheckProviderMins_Valid
3:59PM WRN Coin gecko found 0 providers for OJO
3:59PM INF providers supporting ATOM: [BingX WhiteBIT Gate.io DigiFinex LBank Binance Phemex BitMart Bitget MEXC XT.COM CITEX Deepcoin P2B Bybit CoinTR Pro Dex-Trade OKX Helix KuCoin PointPay AscendEX (BitMax) Huobi Toobit DIFX Trubit BTCEX QMall CoinEx BtcTurk PRO LATOKEN LocalTrade Coinbase Exchange Bitazza BitStorage FameEX Kraken Poloniex Binance US Bitfinex Bitci TR]
3:59PM INF providers supporting USDT: [Bitfinex Kraken Crypto.com Exchange Coinbase Exchange WhiteBIT BTSE Coinzoom]
--- PASS: TestCheckProviderMins_Valid (1.60s)
=== RUN TestCheckProviderMins_Invalid
3:59PM WRN Coin gecko found 0 providers for OJO
--- PASS: TestCheckProviderMins_Invalid (0.54s)
=== RUN TestProviderWithAPIKey_Valid
3:59PM INF providers supporting ATOM: [BingX WhiteBIT Gate.io DigiFinex LBank Binance Phemex BitMart Bitget MEXC XT.COM CITEX Deepcoin P2B Bybit CoinTR Pro Dex-Trade OKX Helix KuCoin PointPay AscendEX (BitMax) Huobi Toobit DIFX Trubit BTCEX QMall CoinEx BtcTurk PRO LATOKEN LocalTrade Coinbase Exchange Bitazza BitStorage FameEX Kraken Poloniex Binance US Bitfinex Bitci TR]
3:59PM INF providers supporting USDT: [Bitfinex Kraken Crypto.com Exchange Coinbase Exchange WhiteBIT BTSE Coinzoom]
3:59PM ERR failed to start currency provider tracker error="json: cannot unmarshal object into Go value of type []config.coinList"

Proposal

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

• possibles fixes, swap coingecko api or increase deviation

Error output from price-feeder when starting:

7:53PM DBG failed to broadcast tx; retrying... error="rpc error: code = Unknown desc = rpc error: code = Unknown desc = must provide at least one oracle exchange rate: invalid request [cosmossdk.io/[email protected]/errors.go:149] With gas wanted: '0' and gas used: '6666' : unknown request" last_check_height=299825 max_height=299830 module=oracle_client tx_code=0 tx_hash=
Mar 06 19:53:56 devnet-n1 price-feeder[252390]: 7:53PM DBG failed to broadcast tx; retrying... error="rpc error: code = Unknown desc = rpc error: code = Unknown desc = must provide at least one oracle exchange rate: invalid request [cosmossdk.io/[email protected]/errors.go:149] With gas wanted: '0' and gas used: '6666' : unknown request" last_check_height=299826 max_height=299830 module=oracle_client tx_code=0 tx_hash=

Summary of Bug

Currently, our price feeder only drops prices which are too far in the past. In the case that an API is sending pricing info in the future, the price feeder should drop these as well

Summary of Bug

When trying to get a working price feed of WETH/USDC using the eth-uniswap provider, I noticed that if I add the currency pair information for USDC after WETH/USDC in the config, the WETH/USDC price feed does not work. When placing USDC before it in the config however, the price feed was back to working suggesting that the order of currency pairs in the config can have some effect on the price feeds working or not.

Version

Please provide the output of the following commands:

• $ price-feeder version
• $ go version
• $ uname -a

Steps to Reproduce

Steps to reproduce the behavior:

1. ...

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

Some websocket providers check the websocket msg type before processing the messages; we should do this in all of them

Summary of Bug

A clear and concise description of what the 🐞 is.

Version

Please provide the output of the following commands:

• $ price-feeder version
• $ go version
• $ uname -a

Steps to Reproduce

Steps to reproduce the behavior:

1. ...

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

Summary of Bug

Version is empty using the CLI command

$ price-feeder version
version: ""
commit: ""
sdk: ""
go: go1.19.9 linux/amd64

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

Summary of Bug

Exchange rates occasionally disappear from the ojo node in a single-node local setup.

Version

PF commit: e4c04e8
Ojo @ main

Steps to Reproduce

Steps to reproduce the behavior:

1. Checkout PF at e4c04e8
2. Checkout ojo at main
3. Run price feeder against ojo with example configuration
4. Run ojod q oracle exchange-rates to see rates go in and out

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

The prices that are passed into ConvertCandlesToUSD and ConvertTickersToUSD only have the base denom associated with them. Therefore when looping over the prices and converting them if the same provider has two different pairs with the same base denom it will only store the second conversion rate in the conversionRates array and use it for all currency conversion for that base.

Example bad config:

[[currency_pairs]]
base = "ATOM"
providers = [
"kraken",
]
quote = "USDT"

[[currency_pairs]]
base = "ATOM"
providers = [
"kraken",
]
quote = "USD"

Currently the price-feeder will only fallback to using ticker prices if no candle prices exist (https://github.com/ojo-network/price-feeder/blob/main/oracle/oracle.go#L310). It should fallback to using ticker prices on a per currency pair - or at least a per provider basis.

Summary of Bug

• The new price-feeder.example.toml has only binance as a provider. The umee price-feeder increments the missing count with an error log when only binance is set as the provider.

6:00PM ERR failed to get ticker prices from provider error="osmosisv2 has no ticker data for requested pairs: [UMEEATOM OSMOATOM stATOMATOM stOSMOOSMO ISTOSMO JUNOOSMO stkATOMATOM]" module=oracle

Version

$ price-feeder version
version: HEAD-e14f2798d442c4b657dabed4b8e52f85a8caa311
commit: e14f2798d442c4b657dabed4b8e52f85a8caa311
sdk: v0.46.10-umee
go: go1.19.1 linux/amd64

$ go version
go version go1.19.1 linux/amd64

$ uname -a
Linux  5.4.0-125-generic #141-Ubuntu SMP Wed Aug 10 13:42:03 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Steps to Reproduce

Steps to reproduce the behavior:

1. Start the price-feeder with price-feeder.example.toml.
2. The following error log is output

6:00PM ERR failed to get ticker prices from provider error="osmosisv2 has no ticker data for requested pairs: [UMEEATOM OSMOATOM stATOMATOM stOSMOOSMO ISTOSMO JUNOOSMO stkATOMATOM]" module=oracle

3. The missing count of price-feeder is incremented.

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

This ticket was created to focus on all the different causes for prices being outside of standard deviations, not correctly retrieved by providers, etc. We have pretty generic logs right now, such as:

Jun 30 20:54:25 agamotto-val-stage-1 price-feeder-v0.1.4-rc1[210035]: 8:54PM ERR failed to get ticker price for AXLUSD module=oracle provider=kraken
Jun 30 20:54:25 agamotto-val-stage-1 price-feeder-v0.1.4-rc1[210035]: 8:54PM ERR failed to get candle prices for AXLUSD module=oracle provider=kraken
Jun 30 20:54:25 agamotto-val-stage-1 price-feeder-v0.1.4-rc1[210035]: 8:54PM ERR failed to get ticker price for AXLUSDT module=oracle provider=bitget
Jun 30 20:54:25 agamotto-val-stage-1 price-feeder-v0.1.4-rc1[210035]: 8:54PM ERR failed to get candle prices for AXLUSDT module=oracle provider=bitget

Price feeder right now uses only 2 times of logs - info and error. Some errors might be ignores for instance lack of provider, but some errors like for instance "cant get price" should be fatals.

PF requires code review with logs + errors assessment:
info - general info
debug - info for devs
warn - minor issue - issues that should be reviewed within 1 week
error - major issue - issues that should be reviewed within 1 day
fatal - showstopper - pagerduty alert

Summary

Problem Definition

We need to add uniswap v3 as a provider for the price feeder to support certain assets.

This may require an indexer to provider OHCL data, but hopefully we can use the uniswap subgraph.

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

Summary

We need to set up a repo for the price feeder on Ojo

• ojo-network/ojo#45
• #2
• #3
• #4
• #5
• #10
• #20
• #9
• #33
• #34
• #47

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

The price-feeder has a bunch of duplicate logic in the grpc client that connects to the Ojo/Umee nodes. We should replace this with the generic client that is currently being built.

ref: umee-network/umee#1888

This is an issue to track any issues we want to bring over to Price Feeder V2

• add fees config option (ref umee-network/umee#1399)
• Kujira Fin provider
• Curve provider
• Bingex provider

          I love the idea. What do you think about making that its own PR since this one is already large and that will add a ton more changes?

Originally posted by @zarazan in #165 (comment)

Summary

We're going to start integrating SILK as a price feed for Ojo

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

Summary of Bug

We've found cases where validators report old prices for certain assets, likely due to old candles being in memory. We need to take a look at the filtering logic or implement a safeguard that resets all the candles in memory.

Version

Please provide the output of the following commands:

• $ price-feeder version
• $ go version
• $ uname -a

Steps to Reproduce

Steps to reproduce the behavior:

1. ...

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

Summary

Add price feeder support for TMKMS keys

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

Summary of Bug

We need to ensure that validators can not use the PF tool to add multiple conversion paths down to USD. We can do this in the config validation.

An example would be:

ETH -> BTC
BTC -> USDT
BTC -> MATIC

Summary

We must add binance as a provider to the OSMO token feed config. This goes for Ojo & the Umee PF versions.

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

Summary of Bug

If the price feeder fails to convert one asset to USD, all the other assets fail as well and the validator misses votes. In this case we should instead log the error and continue so that the validator can vote on other assets.

The Umee and Ojo e2e tests rely on docker images of the price-feeder and right now these docker images are only updated manually (ex: https://github.com/ojo-network/ojo/blob/main/Makefile#L113-L114). We need to auto push new images for each branch (Ojo/Umee) when changes are merged.

• Ojo version
• Umee version

Summary

Start an Umee-specific branch so we can migrate off of the umee repo

• Create umee branch
• Add branch protections
• Update to use the umee library instead of the ojo library
• Update release process on the umee branch to allow for releases like umee/vx.x.x
• Add umee branch backport label
• Submit a PR to umee which removes the price feeder code & updates the umee e2e to use this repo's dockerfile, + Add a markdown file in the umee/price-feeder folder to point to this repo
• Make first price feeder release for use by the umee chain (Let's make that v2.1.1 - once we launch Ojo we'll do releases of PF for umee & ojo at the same time, makes it less confusing)
• Announce this version to the validators

Summary

We need to add some more tests for the astroport provider. Since it doesn't currently have candles, this can wait for a bit.