-
Notifications
You must be signed in to change notification settings - Fork 256
Add pyth quorom server #2759
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
base: main
Are you sure you want to change the base?
Add pyth quorom server #2759
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Pull Request Overview
This PR implements a new Pyth server (named "quorom") that includes a websocket endpoint, REST API endpoints to handle observations, and integration with Pythnet for fetching guardian set information.
- Implements a websocket actor (Subscriber) to manage long-lived connections and deliver update events.
- Adds API endpoints for health checks and handling observation submissions that result in broadcasting VAAs.
- Introduces a Pythnet client to fetch guardian set data and integrates server shutdown via a global exit flag.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
apps/quorom/src/ws.rs | Implements websocket handling and subscriber actor for update events. |
apps/quorom/src/server.rs | Sets up server state, command-line options, and graceful shutdown. |
apps/quorom/src/pythnet.rs | Implements Pythnet client functionality to fetch guardian set info. |
apps/quorom/src/main.rs | Configures tracing and starts the server. |
apps/quorom/src/api.rs | Provides API endpoints to handle observations and broadcast VAAs. |
apps/quorom/rust-toolchain | Specifies the Rust toolchain version. |
apps/quorom/Cargo.toml | Declares the project metadata and dependencies. |
apps/quorom/.gitignore | Configures ignore rules for Rust build artifacts. |
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.
LGTM. My comments are mostly minor but please address them before merging.
apps/quorom/src/api.rs
Outdated
signatures, | ||
}, params.body.clone()).into(); | ||
if let Err(e) = state.ws.broadcast_sender.send(UpdateEvent::NewVaa(vaa)) { | ||
tracing::error!(error = ?e, "Failed to broadcast new VAA"); |
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 you only get error here when there is no receiver and we should ignore it.
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.
Isnt it good to shout when there is no receiver?
apps/quorom/src/api.rs
Outdated
if let Err(e) = state.ws.broadcast_sender.send(UpdateEvent::NewVaa(vaa)) { | ||
tracing::error!(error = ?e, "Failed to broadcast new VAA"); | ||
} | ||
verification_writer.remove(¶ms.body); |
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.
something that can happen here is that we wait for all these to finish when we shut down. i am somewhat surprised that axum graceful shutdown is not doing it.
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 used task tracker to make sure that we handle all observation before shutting down. Do you think it's not necessary to handle all observations before shutting down?
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.
This means we are not going to accept new requests, but we will wait until we handle all prv requests that we received
This PR aims to add pyth quorom server