Add support for token price oracle #400
Conversation
vihu
commented
Mar 2, 2023
vihu
commented
- Add price oracle skeleton
- Update file_info
- Add settings
- Update Cargo.toml and Cargo.lock
vihu
force-pushed
the
rg/price
branch
3 times, most recently
from
1801f86 to
eabd6ed
Compare
vihu
changed the title
| triggered = {workspace = true} | ||
| pyth-sdk-solana = "0.7.1" | ||
| solana-client = ">= 1.9, < 1.15" | ||
| solana-program = ">= 1.9, < 1.15" |
There was a problem hiding this comment.
Yeah, client for the RpcClient and program for the pubkey. pyth-sdk doesn't seem to automatically fetch these.
There was a problem hiding this comment.
looks like you should be able to omit the solana-program dependency since pyth-sdk-solana depends on that generally, but it only lists solana-client as a dev dependency
There was a problem hiding this comment.
Removing just the solana-program doesn't seem to work either.
| #[error("unsupported token type: {0}")] | ||
| UnsupportedTokenType(i32), | ||
| #[error("unable to fetch price")] | ||
| UnableToFetch, |
There was a problem hiding this comment.
Is there an underlying error when UnableToFetch hits?
There was a problem hiding this comment.
Doesn't look like it, get_price_no_older_than just returns None afaict.
| triggered = {workspace = true} | ||
| pyth-sdk-solana = "0.7.1" | ||
| solana-client = ">= 1.9, < 1.15" | ||
| solana-program = ">= 1.9, < 1.15" |
There was a problem hiding this comment.
looks like you should be able to omit the solana-program dependency since pyth-sdk-solana depends on that generally, but it only lists solana-client as a dev dependency
price/src/settings.rs
Outdated
| .map(|key| SolPubkey::from_str(key).expect("unable to parse")), | ||
| BlockchainTokenTypeV1::Hst => self | ||
| .cluster | ||
| .hst_price_key | ||
| .as_ref() | ||
| .map(|key| SolPubkey::from_str(key).expect("unable to parse")), | ||
| BlockchainTokenTypeV1::Mobile => self | ||
| .cluster | ||
| .mobile_price_key | ||
| .as_ref() | ||
| .map(|key| SolPubkey::from_str(key).expect("unable to parse")), | ||
| BlockchainTokenTypeV1::Iot => self | ||
| .cluster | ||
| .iot_price_key | ||
| .as_ref() | ||
| .map(|key| SolPubkey::from_str(key).expect("unable to parse")), |
There was a problem hiding this comment.
this makes sense to me as a "halt and catch fire scenario" and theoretically quick enough to fix if encountered but can we have the panic error explain which key we failed to fetch for when we have more than just the HNT price key and we aren't necessarily sure which one we failed to parse?
There was a problem hiding this comment.
Updated with panic!, cc: @madninja cuz panic! Open to other suggestions instead of panicking but as @jeffgrunewald said we want to catch this ASAP and fix it when tokens come alive. f8ab0b3
| mobile_price_generator | ||
| .run(price_sink.clone(), &shutdown, BlockchainTokenTypeV1::Mobile) | ||
| .map_err(Error::from), | ||
| iot_price_generator | ||
| .run(price_sink.clone(), &shutdown, BlockchainTokenTypeV1::Iot) | ||
| .map_err(Error::from), | ||
| hst_price_generator | ||
| .run(price_sink, &shutdown, BlockchainTokenTypeV1::Hst) | ||
| .map_err(Error::from), |
There was a problem hiding this comment.
since these generators are just going to be reaching a None else case when they check for a pricing account key do we even want to run them before those tokens have prices to be fetched? I would see running them if we were going to default to writing out a $0 pricing report but since we're not it seems like we could skip them.
for that matter, and since we are only accessing these prices via the written reports would it be easier to have a single PriceGenerator struct that loops over all "active" tokens based on configuration, stores only the last price in a map in its state and spawns a thread with a join handle for each active token, passing the previous price to the thread and A) checking for a newer price, B) writing out the report to the file sink with the current timestamp and C) returning either the previous or newer price via the join handle to the PriceGenerator?
then you only need to grab current time once and it doesn't need to be mutable, you simplify the state of the PriceGenerator process, there's only one continuous thread you need to worry about shutting down and it's just a matter of adding a token string to a config list in Settings to start checking for additional prices
There was a problem hiding this comment.
Yeah we thought about having a single PriceGenerator, but it doesn't really matter if the timestamps for different token types (as they become available) within the reports are the same as long as they are there. I believe it's already pretty simple as is imo and the state is minimal.
There was a problem hiding this comment.
Does it matter if one price gets successfully updated and others do not? ie is there any intra dependencies on the prices being current on the consumer side ? If iot gets updated but for whatever reason hnt fails to update and continues failing for a while and whilst there may be steep changes in the hnt price
There was a problem hiding this comment.
That's a good question but I'd say not something we have to solve for first pass at least. So in the scenario where HNT updated and IOT failed, we'll presumably continue to serve the same data for IOT till we:
a) either have some redundancy with our RPC endpoints (assuming it was an RPC failure to fetch IOT in the above example)
b) or we kind of "derive" IOT price from HNT in some way
@madninja What are your thoughts on this? I could look into adding support for multiple RPC endpoints I suppose. And maybe do a mean of the prices fetched from multiple RPC endpoints?
- Temp update Cargo.toml and Cargo.lock - Update file info and dump cli - Add docker-compose.yml for local testing
