-
Notifications
You must be signed in to change notification settings - Fork 224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(hermes): create latest TWAP endpoint #2131
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
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.
Amazing!
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.
Nice! Please bump the version as well.
validate_price_ids(&state, &price_id_inputs, params.ignore_invalid_price_ids).await?; | ||
|
||
// Collect start and end bounds for the TWAP window | ||
let window_seconds = path_params.window_seconds as i64; |
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.
Maybe you want this to be an i64 in the params to avoid this conversion here?
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 made window_seconds
of type DurationInSeconds
from pyth_sdk, which is a u64 (probably to enforce a nonnegative duration.) it seemed like a good fit but i see your point as well -- any preference on i64 vs u64 here?
price: pft.twap.price, | ||
conf: pft.twap.conf, | ||
expo: pft.twap.expo, | ||
publish_time: pft.twap.publish_time, |
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.
publish_time seems redundant
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.
yeah fair. i reused RpcPrice
here to avoid creating a slightly different struct just for twap prices, since the other fields fit well (price
, conf
, expo
). thoughts?
// Perform division before casting to maintain precision | ||
// Cast slot_diff to the same type as price / conf diff before division | ||
let price = (price_diff / slot_diff as i128) as i64; | ||
let conf = (conf_diff / slot_diff as u128) as u64; |
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 better to use the checked casts u128::from
and u64::try_from
generally
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.
oo great tip ty
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.
The as
keyword for integer conversion is one the biggest mistakes in rust :D very un-rusty
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.
is there a rust equivalent to the zen of python out there? :)
it is getting bumped to new minor version 0.8.0, want me to bump major version instead? |
Nice 👍 |
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.
Very nice!
/// The % of slots where the network was down over the TWAP window. | ||
/// A value of zero indicates no slots were missed over the window, and | ||
/// a value of one indicates that every slot was missed over the window. | ||
/// This is a float value stored as a string to avoid precision loss. |
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.
update this comment?
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.
it is actually being returned as a string (utoipa's default behavior for Decimal)
Purpose
Create Hermes endpoint at
/v2/updates/twap/:window_seconds/latest/
to get the latest TWAP prices for a list of price IDs with a dynamic time window (default 300s/5m).t0
andt1
, in the target chain contract.parsed
) will match what our SDK will return from on-chain.TwapsResponse
):binary
(list ofBinaryUpdate
, where each update contains the start & endTwapMessages
for a requested feed)TwapMessages
contain everything needed to compute the TWAP on-chain (cumul price, expo, slot, num down slots, etc).parsed
(list ofParsedPriceFeedTwap
, which contains the calculated TWAP for each requested feed)See a sample response here.
Testing
Next up
publish_time
.