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

Add mempool notifier sub #2529

Conversation

ecioppettini
Copy link
Contributor

base branch is #2378 to show changes more clearly (and it would have to be merged first anyway).

Motivation

To be able to know about fragment rejection or acceptance asynchronously.

Summary

This basically sends a message each time a fragment status is modified in the logs, to anyone with an open connection on the api/v1/notifier/mempool endpoint. For now rejection reason is not added, only the id.

This also moves the previous endpoint from api/v1/notifer to api/v1/notifier/blocks.

Other

TODO: update the tests
To think about: graphql has subscriptions which can be used with websockets, so we could have a little schema instead of multiple endpoints. I'm not sure if it's worth it, especially considering that juniper doesn't have them finished.

@ecioppettini ecioppettini added enhancement New feature or request subsys-rest issues or PRs for the REST interface labels Sep 8, 2020
@ecioppettini ecioppettini self-assigned this Sep 8, 2020
the serialization can change, but use json for now
add also setting to configuration builder, in order to test it, and
improve the panic handling of the notifier client
in order to avoid platform specific dependencies in Cargo.lock
the new connection requests are handled through a new message
tip changes are handled with tokio::watch because we only care about the
latest state, but block changes use a broadcast to keep old ones too
@ecioppettini ecioppettini force-pushed the add-blockchain-event-notifier-service branch from 5c7253c to 10afb4e Compare September 10, 2020 16:55
@ecioppettini ecioppettini marked this pull request as ready for review September 10, 2020 19:40
@rinor rinor requested a review from a team September 15, 2020 08:56
Comment on lines +95 to +98
mempool_sender.send(MemPoolMessage::FragmentRejected(fragment_id))
{
()
}
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 really understand the purpose of the if let block here. Can't we use map_err or something else?

@ecioppettini ecioppettini force-pushed the add-blockchain-event-notifier-service branch from 941a3f0 to 7c6c721 Compare October 30, 2020 23:01
@ecioppettini ecioppettini force-pushed the add-blockchain-event-notifier-service branch from 7c6c721 to 5c92bf8 Compare November 25, 2020 15:33
@ecioppettini ecioppettini force-pushed the add-blockchain-event-notifier-service branch from 5c92bf8 to 1cc7bf3 Compare March 3, 2021 19:50
@ecioppettini ecioppettini deleted the add-mempool-notifier-sub branch October 20, 2021 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request subsys-rest issues or PRs for the REST interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants