Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Give control over the peers of custom notification protocols #7072

Closed
Tracked by #989
tomaka opened this issue Sep 10, 2020 · 3 comments
Closed
Tracked by #989

Give control over the peers of custom notification protocols #7072

tomaka opened this issue Sep 10, 2020 · 3 comments
Labels
J0-enhancement An additional feature request.

Comments

@tomaka
Copy link
Contributor

tomaka commented Sep 10, 2020

The Polkadot collation and validator-to-validator communication protocols require the node to be connected to specific sets of peers. As an example, we want validators to reserve incoming slots for collators, and not have all its slots occupied by relay chain full nodes.

Rather than changing the peerset and try to fit all use-cases in it (#6087), an alternative idea is to:

  • Let the current peerset handle the default protocols (block announces, transactions, syncing)
  • Let the code that registers a notification protocol control, for this individual notification protocol, which nodes to establish outgoing substreams to, and whether to accept incoming substreams.

The exact API details are still blurry, but I prefer this approach over trying to make the peerset suit our use-cases.

@tomaka
Copy link
Contributor Author

tomaka commented Oct 14, 2020

What I have in my head with this issue is adding a new method similar to register_notifications_protocol that would additionally accept a second parameter of type mpsc::Sender<NotificationEvent> (and similarly, a new field in NetworkConfiguration that does the same).

pub enum NotificationEvent {
    IncomingRequest {
        peer_id: PeerId,
        handshake: Vec<u8>,
        // In order to accept the peer, send back a `Ok` containing the handshake response.
        accept: oneshot::Sender<Result<Vec<u8>, ()>>,
    },
    Open(PeerId),
    Notification(PeerId, Vec<u8>),
    Closed(PeerId),
}

The sc-network crate would send on this channel all events concerning the given notifications protocol. The "main" events channel, gathered through NetworkService::event_stream wouldn't receive them.

This would also solve paritytech/polkadot-sdk#556 and paritytech/polkadot-sdk#554.

@tomaka
Copy link
Contributor Author

tomaka commented Oct 14, 2020

That being said, I've been leaning more towards #6087 after thinking about it.

@tomaka
Copy link
Contributor Author

tomaka commented Nov 16, 2020

Leaning towards the solution in #7374.
Having direct control over the peers is the most versatile option, but also seems very complex to use from an API perspective.
Splitting peer slots per protocol (#7374) solves the same problem.

As such, I'm going to close this issue.
Feel free to re-open if the problem isn't solved by #7374.

@tomaka tomaka closed this as completed Nov 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request.
Projects
None yet
Development

No branches or pull requests

1 participant