Network bridge should give out peer communication objects

[rphmeier]

The Network Bridge subsystem currently works by informing protocols about connected peers. All outgoing communication to peers happens via a NetworkBridgeMessage::SendMessage. That has these problems:

• Communication can bottleneck at the overseer. All network traffic has to flow through this one point.
• It doesn't allow for any type of per-peer flow control

Substrate's networking APIs have been improving on per-peer traffic management utilities exposed to the higher layer, as in paritytech/substrate#6692 . Previously, the only API that was available was NetworkService::write_notification which just wrote into an UnboundedSender and thus didn't allow for any strong higher level logic.

What are these connection objects in practice? As I understand, based on conversations with @tomaka, they will be something like a libp2p substream. The intent is for the higher-level protocols to be in charge of buffering messages for peers. So the "raw" sender should be wrapped in something like this:

struct BufferedPeerSender<T> {
  buf: [T], // Vec, ArrayVec, something like that
  sender: PeerSender,
}

The buffered type is expected to be a T: Encode and the sender will have utility functions on it for clearing outdated messages, in case the buffer is full. The goal is to be able to synchronously buffer messages and asynchronously send them. This utility is not meant to be used directly by higher-level code, as it comes with many difficulties (I explored how complicated it gets in this chain of comments: paritytech/substrate#6591 (comment)).

As a follow-on to this issue, we'll want to create some higher-level wrappers around this buffer management for e.g. point-to-point and gossip communication that removes most of the complexity from users' view.

[rphmeier]

This issue is currently in the Design phase, but will move to implementation after a small guide PR is included.

I'd like to avoid leaking these networking details like buffer management to the guide too much at this stage. So within the guide, it makes most sense to me to just state that each peer-sender can be used to send messages to peers and then use language like "send the peer a message using its sender". At the guide level, we care about protocol more than implementation.

[tomaka]

Linking here the very rough draft I suggested for using send_notifications (which has now been renamed to prepare_notification): https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=427a0f40cd2d3e9bf9c3f5a228111045

[tomaka]

If I understand correctly, the idea would be to remove NetworkBridgeMessage::SendMessage altogether and add a third field to NetworkBridgeEvent::PeerConnected that makes it possible to send messages.

Since we should not put any generic on NetworkBridgeEvent itself, I would suggest that this third field should be something like MessageSenderPrototype (important part being ...Prototype) that needs to be turned into a MessageSender<T> before it can be used.

This would unfortunately introduce a small ambiguity as to whether the substream is open, as noted here, so PeerDisconnected events should still exist and be the authority in this regard.

[rphmeier]

@tomaka confirmed, that is the idea and agreed on both points. I like calling it Prototype because it needs to be combined with a buffer before it can be used as a sender. Either that or RawMessageSender.

[tomaka]

This should now be implementable now that paritytech/substrate#6803 is merged.

[rphmeier]

We should not yet remove the SendMessage logic that uses write_notification under the hood, but should instead introduce this alongside that and refactor the individual protocols to stop using it.