Feat() : fetch strk price from oracle #368
Conversation
Feat/fetch price from pragma
| pub async fn fetch_eth_strk_price(&self) -> anyhow::Result<(u128, u32)> { | ||
| let response = reqwest::Client::new() | ||
| .get(self.get_fetch_url(String::from("eth"), String::from("strk"))) | ||
| .header("x-api-key", self.api_key.clone()) | ||
| .send() | ||
| .await | ||
| .expect("failed to retrieve price from pragma oracle"); | ||
|
|
||
| let oracle_api_response = response.json::<PragmaApiResponse>().await.expect("failed to parse api response"); | ||
| let eth_strk_price = u128::from_str_radix(oracle_api_response.price.trim_start_matches("0x"), 16)?; | ||
|
|
||
| Ok((eth_strk_price, oracle_api_response.decimals)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Does the node crash in case of unavailable service?
There was a problem hiding this comment.
oh yes probably, my bad, i changed expect to context to propagate the error to crates/client/eth/src/l1_gas_price.rs:17
Yes, oracle are just here to do live conversion between ETH and L2 token so, in my opinion, the solution would be to disable the ability to pay gas with STRK/L2token. |
That would fck up all the rpc endpoints that have the strk gas price tho so not a good idea. |
we could default it anyways |
| let (eth_strk_price, decimals) = | ||
| oracle_provider.fetch_eth_strk_price().await.context("failed to retrieve ETH/STRK price")?; | ||
| let strk_gas_price = (BigDecimal::new((*eth_gas_price).into(), decimals.into()) | ||
| / BigDecimal::new(eth_strk_price.into(), decimals.into())) |
There was a problem hiding this comment.
if eth_strk_price price is 0 here there is a possible 0 division?
There was a problem hiding this comment.
yeah you are right, added a check
| .context("failed to retrieve price from pragma oracle")?; | ||
|
|
||
| let oracle_api_response = response.json::<PragmaApiResponse>().await.context("failed to parse api response")?; | ||
| let eth_strk_price = u128::from_str_radix(oracle_api_response.price.trim_start_matches("0x"), 16)?; |
There was a problem hiding this comment.
if the price is not fetched in the expected format what will happen?
There was a problem hiding this comment.
it fail to parse and propagate an error without panicking
| / BigDecimal::new(eth_strk_price.into(), decimals.into())) | ||
| .as_bigint_and_exponent(); | ||
|
|
||
| l1_gas_provider.update_strk_l1_gas_price(strk_gas_price.0.to_str_radix(10).parse::<u128>()?); |
There was a problem hiding this comment.
I would expect this not to overflow but would be great to ensure clean code
|
|
||
| if l1_gas_setter.is_oracle_needed() && l1_gas_setter.oracle_provider.is_none() { | ||
| log::error!("STRK gas is not fixed and oracle is not provided"); | ||
| panic!(); |
There was a problem hiding this comment.
I would personally handle the case where we dont want the strk price by defaulting it
Pull Request type
Please add the labels corresponding to the type of changes your PR introduces:
Feature
What is the current behavior?
Resolves: #352