feat(pyth-lazer-protocol): add validations for price request types, add unknown_symbols to InvalidFeedSubscriptionDetails #3065
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
…b/lazer-protocol/add-missing-symbols-response-type
darunrs
left a comment
darunrs
left a comment
There was a problem hiding this comment.
Can you also update the JS SDK too? I'll check your Lazer PR in a sec, but we should make sure the change is backwards compatible and also that the javascript sdk client and types work with docs updated.
| impl<'de> Deserialize<'de> for LatestPriceRequest { | ||
| fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> | ||
| where | ||
| D: serde::Deserializer<'de>, | ||
| { | ||
| let value = LatestPriceRequestRepr::deserialize(deserializer)?; | ||
| Self::new(value).map_err(Error::custom) | ||
| } | ||
| } |
There was a problem hiding this comment.
Is there a reason that this was manually implemented? It looks functionally very similar to the actual trait. This follows for the other trait impls below.
There was a problem hiding this comment.
It's handling two decoupled tasks, deserializing the input representation and applying the validation rules in LatestPriceRequest::new. It gives some type safety - once you have a PriceRequest, you know it's valid.
| } | ||
|
|
||
| impl LatestPriceRequest { | ||
| pub fn new(value: LatestPriceRequestRepr) -> Result<Self, &'static str> { |
There was a problem hiding this comment.
Is there a significance in the error being a static string here, instead of using anyhow Error types?
There was a problem hiding this comment.
I don't think so, but was conforming with the existing pattern in the file :)
Updated the Currently the JS SDK is only focused on streaming, i'm making it more unified to handle History svc as well (future PR). |
…b/lazer-protocol/add-missing-symbols-response-type
tejasbadadare
requested review from
ali-behjati and
keyvankhademi
as code owners
| #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct LatestPriceRequest { | ||
| pub struct LatestPriceRequestRepr { |
There was a problem hiding this comment.
I think you should increase the protocol version here too.
tejasbadadare
dismissed stale reviews from Riateche and keyvankhademi
via
2d1c8a9
tejasbadadare
deleted the
tb/lazer-protocol/add-missing-symbols-response-type
branch
Summary
unknown_symbolstoInvalidFeedSubscriptionDetailsHow has this been tested?