ithacaxyz / op-rs

Apache License 2.0
58 stars 6 forks source link

feat(net): Peer Backchannel [RFC] #65

Closed refcell closed 1 month ago

refcell commented 2 months ago

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.

merklefruit commented 2 months ago

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) {
        // ...
    }
}
refcell commented 1 month ago

Closing for now