-
Notifications
You must be signed in to change notification settings - Fork 260
fix(hermes): improve TWAP reliability - non-optional price selection and consistent time windows #2521
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
fix(hermes): improve TWAP reliability - non-optional price selection and consistent time windows #2521
Conversation
…ds and add LatestTimeEarliestSlot request time
…ps_with_update_data function
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 6 Skipped Deployments
|
@@ -285,6 +285,28 @@ async fn retrieve_message_state( | |||
Some(key_cache) => { | |||
match request_time { | |||
RequestTime::Latest => key_cache.last_key_value().map(|(_, v)| v).cloned(), | |||
RequestTime::LatestTimeEarliestSlot => { |
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.
Thought i had while working on this... should we use this for all our latest
endpoints, not just TWAP?
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 don't know. If we change it to the earliest slot then effectively we'll have 1 price per second. I think the poll users (and subscribers) would still like to get the latest price.
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 that makes sense. I guess they are on the hook for ensuring the price they use is canonical?
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!
Summary
This PR refactors how Hermes fetches the data for the TWAP calculation to improve reliability.
Rationale
window_seconds: u64
andend_time: RequestTime
, instead ofstart_time: RequestTime
andend_time: RequestTime
.start_time
based on the publish time of the end messages. Previously, the actual time difference between start and end messages could differ from the intended window size since the latest timestamp in the cache could be older than the wall time. This change guarantees that the start and end messages arewindow_seconds
apart.RequestTime::LatestTimeEarliestSlot
request mode to the cache that selects messages with the latest publish time but earliest slot number. This follows the same semantics asFirstAfter
, and helps choose non-optional (canonical) prices.prev_publish_time == publish_time
. This is done to avoid optionality of prices, and we consider only the message whereprev_publish_time < publish_time
as the canonical price for that timestamp. Previously we were using theRequestTime::Latest
mode, which just returns the latest message for a feed, NOT the latest canonical message. The new mode provides these semantics.How has this been tested?
test_get_verified_twaps_with_update_data_uses_non_optional_prices
to verify correct message selectiontest_latest_time_earliest_slot_request_works
to test theLatestTimeEarliestSlot
cache request modewindow_seconds
apart.