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

feat: channels for dlc messages #251

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bennyhodl
Copy link
Contributor

Instead of polling for messages for MessageHandler consumers. You can subscribe to receive the messages over the channel.

This PR is to start a conversation of what we would like to implement.

  • Tokio broadcast channel allows multiple consumers and works similar to how rust nostr handles relay notifications.
  • Standard lib mpscchannel. Only allows one consumer, we might want to opt for a channel crate that is synchronous and allows multiple consumers

@bennyhodl bennyhodl force-pushed the dlc-message-channel branch from 4d3da8e to 5c8b089 Compare January 15, 2025 17:50
@bennyhodl bennyhodl force-pushed the dlc-message-channel branch from 5c8b089 to 1d8442b Compare January 15, 2025 20:10
@bennyhodl bennyhodl force-pushed the dlc-message-channel branch from 1d8442b to aa9361e Compare January 15, 2025 20:39
@bennyhodl
Copy link
Contributor Author

Added a crossbeam channel for better performance and thread safety. Sync channel has to be wrapped in Arc<Mutex<Receiver>> and does not allow multiple consumers.

I think we can have an async-channel and sync-channel feature for each.

@Tibo-lg
Copy link
Contributor

Tibo-lg commented Jan 16, 2025

I think there are many ways to implement a MessageHandler. For some polling will be better, for some event based will be better, and it is possible to convert one to the other as well. The implementation in the crate is there mainly to be used by those who don't have specific needs, and I don't think there is a strong need to change it from polling to a pub/sub system, especially if it means pulling in more dependencies which I try to avoid as much as possible for the core crates. If someone has that need, they can go ahead and create their own MessageHandler. If you think that some things could be improved to make it easier to implement custom handlers, I think that would be valuable.

@bennyhodl
Copy link
Contributor Author

I am not proposing to get rid of the polling. Default feature is the polling and a feature for channels (which the dependency would only be used if feature is on).

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.

2 participants