-
Notifications
You must be signed in to change notification settings - Fork 15
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
frank/market & oracle maps #17
Conversation
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.
generally lgtm.
should be doing concurrent awaits where we can since network is the slowest part
self.unsubscriber = Some(unsub_tx); | ||
|
||
tokio::spawn({ | ||
let event_emitter = self.event_emitter.clone(); |
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 shape of the Subscribers is very similar, wonder if we can generalize that
pubkey: Pubkey, | ||
pub(crate) commitment: CommitmentConfig, | ||
pub subscribed: bool, | ||
pub event_emitter: EventEmitter, |
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.
💭 can try something like EventEmitter<T>
to get rid of the any/boxing or a drift-rs wide (concrete) enum
enum Event {
AccountUpdate(...),
AuctionUpdate(...),
// etc.
}
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.
changes lgtm.
since we're building more infra on top of the websocket_account_subscriber it will need some reconnection logic or surface failed/disconnected status (the other subscribers may have similar issue)
No description provided.