-
Notifications
You must be signed in to change notification settings - Fork 278
[hermes] add /v2/updates/price/<timestamp> endpoint #1269
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
@@ -27,7 +27,7 @@ use { | |||
mod doc_examples; | |||
mod metrics_middleware; | |||
mod rest; | |||
mod types; | |||
pub mod types; |
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.
reusing PriceUpdate
in types for state/benchmarks.rs
serde_qs::axum::QsQuery, | ||
utoipa::IntoParams, | ||
}; | ||
|
||
|
||
#[derive(Debug, serde::Deserialize, IntoParams)] | ||
#[derive(Debug, Deserialize, IntoParams)] |
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.
drive-by: refactor
hermes/src/state/benchmarks.rs
Outdated
@@ -56,22 +55,50 @@ impl TryFrom<BinaryBlob> for Vec<Vec<u8>> { | |||
} | |||
} | |||
|
|||
impl TryFrom<BenchmarkUpdates> for PriceFeedsWithUpdateData { | |||
|
|||
impl TryFrom<PriceUpdate> for PriceFeedsWithUpdateData { |
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 this definition should be close to PriceUpdate.
nit: I do have a mixed feeling of having them all in the api module. we could entirely remove PriceFeedsWithUpdateData it it was not necessary for v1 api. Maybe let's move it after we deprecate v1 api.
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.
hmm should we move it to types.rs
instead then?
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 address my comments before merging.
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 address my comments before merging.
No description provided.