Skip to content
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

Mempool rpc events #1807

Merged
merged 5 commits into from
Oct 4, 2024
Merged

Mempool rpc events #1807

merged 5 commits into from
Oct 4, 2024

Conversation

azarovh
Copy link
Member

@azarovh azarovh commented Sep 4, 2024

Allows to subscribe to mempool event over websocket

mempool/src/rpc.rs Outdated Show resolved Hide resolved
mempool/src/pool/mod.rs Outdated Show resolved Hide resolved
Comment on lines +75 to +79
/// Subscribe to events emitted by mempool subsystem
fn subscribe_to_subsystem_events(&mut self, handler: Arc<dyn Fn(MempoolEvent) + Send + Sync>);

/// Subscribe to broadcast mempool events
fn subscribe_to_rpc_events(&mut self) -> utils_networking::broadcaster::Receiver<MempoolEvent>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the naming is adequate here, because these 2 functions subscribe to exactly the same events, only the mechanism differ.

The names of the corresponsing methods of Mempool - subscribe_to_events and subscribe_to_event_broadcast - sound better.

(I'd still rename the latter though, e.g. to subscribe_to_events_via_broadcaster or something like that)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure. This is literally reverting changes from #1501. Though I agree that these events aren't subsystem's or rpc's. subscribe_to_events_subsystem?

@azarovh azarovh merged commit e792509 into master Oct 4, 2024
18 checks passed
@azarovh azarovh deleted the feat/mempool_rpc_events branch October 4, 2024 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants