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(net): Peer Backchannel [RFC] #65

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

refcell
Copy link
Collaborator

@refcell refcell commented Aug 30, 2024

Description

Not particularly keen with this. It effectively allows the consumer of the NetworkDriver to have a handle into peers dialed for discovery. This is done this way since the driver is consumed when starting the network service. Ultimately, this leads me to believe the net crate needs better trait abstractions.

@refcell refcell self-assigned this Aug 30, 2024
@merklefruit
Copy link
Collaborator

merklefruit commented Sep 2, 2024

One alternative would be to have the NetworkDriver also hold a peer_tx: broadcast::Sender<Peer> field, and a function subscribe_new_peers() that returns a new receiver via resubscribe(), which can be used at will by the consumer of the driver.

One one hand, using broadcast channels means having to clone all elements that enter it, but we get the added benefit of being able to subscribe as many times as we want. Otherwise, it would work with a single subscriber using an mpsc channel if we also keep a peer_rx: Option<mpsc::Receiver<Peer>> field in the driver, and have a method to take() it from there to the consumer. In particular this makes it also possible to skip sending anything through the channel if the receiver hasn't been taken, like this:

// if it's None, it means the caller took it and is listening on the receiving end
if self.peer_rx.is_none() {
    if let Err(e) = self.peer_tx.try_send(peer) {
        // ...
    }
}

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