Rework the way one combines NetworkBehaviours together

[tomaka]

The NetworkBehaviour custom derive is a bit weird in terms of API (especially when it comes to the event handling trait and having to override a poll() function), and is also imprecise in terms of ordering of operations.

For example, addresses_of_peers is supposed to return the addresses in a specific order, but it is unclear to the user how to influence this order.

I think a better solution would be to make the trait easy to implement manually, so that everything is clear.
Additionally, we would provide in libp2p itself some common combinations to easily get started, such as Ping+Identify+Kademlia+Floodsub+mDNS.

[tomaka]

I think a better solution would be to make the trait easy to implement manually, so that everything is clear.
Additionally, we would provide in libp2p itself some common combinations to easily get started, such as Ping+Identify+Kademlia+Floodsub+mDNS.

Alternatively, a different (and IMO better) long term vision for combining network behaviours together would be to do something like that:

let behaviour = Ping::new().join(Identify::new()).join(Kademlia::new());
let swarm = Swarm::new(behaviour, ...);

At the moment the problem with that is that we cannot easily express the type of the Swarm. It would be extremely long and annoying to do so. This can be solved:

• Either with impl Trait in the future.
• Or by adding a BoxedNetworkBehaviour type similar to BoxedTransport and BoxedStreamMuxer.

So the first step for this issue would be to create a BoxedNetworkBehaviour that takes as little template parameters as possible (normally just the substream and event type) and makes it possible to abstract over a NetworkBehaviour.

[tomaka]

Another possibility would be to specialize the Swarm a bit more.
Instead of having only NetworkBehaviour, we would have several DiscoveryBehaviour, PubsubBehaviour, etc. traits with a mandatory output event type and a mandatory set of methods to implement. This makes it possible to combine behaviours in a straight-forward way.

[tomaka]

Digging up this issue.

So the first step for this issue would be to create a BoxedNetworkBehaviour that takes as little template parameters as possible (normally just the substream and event type) and makes it possible to abstract over a NetworkBehaviour.

I think this is the way to go, not only for grouping together network behaviours but also as a general direction.

One major pain points of the API of rust-libp2p right now in my opinion is the fact that being generic over a NetworkBehaviour requires you to copy-paste most of these trait bounds into your code:

rust-libp2p/swarm/src/lib.rs

Lines 177 to 206 in 084c7d0

	where TBehaviour: NetworkBehaviour<ProtocolsHandler = THandler>,
	TMuxer: StreamMuxer + Send + Sync + 'static,
	<TMuxer as StreamMuxer>::OutboundSubstream: Send + 'static,
	<TMuxer as StreamMuxer>::Substream: Send + 'static,
	TTransport: Transport<Output = (TConnInfo, TMuxer)> + Clone,
	TTransport::Error: Send + 'static,
	TTransport::Listener: Send + 'static,
	TTransport::ListenerUpgrade: Send + 'static,
	TTransport::Dial: Send + 'static,
	THandlerErr: error::Error,
	THandler: IntoProtocolsHandler + Send + 'static,
	<THandler as IntoProtocolsHandler>::Handler: ProtocolsHandler<InEvent = TInEvent, OutEvent = TOutEvent, Substream = Substream<TMuxer>, Error = THandlerErr> + Send + 'static,
	<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::InEvent: Send + 'static,
	<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::OutEvent: Send + 'static,
	<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::Error: Send + 'static,
	<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::OutboundOpenInfo: Send + 'static, // TODO: shouldn't be necessary
	<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::InboundProtocol: InboundUpgrade<Substream<TMuxer>> + Send + 'static,
	<<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::InboundProtocol as UpgradeInfo>::Info: Send + 'static,
	<<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::InboundProtocol as UpgradeInfo>::InfoIter: Send + 'static,
	<<<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::InboundProtocol as UpgradeInfo>::InfoIter as IntoIterator>::IntoIter: Send + 'static,
	<<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::InboundProtocol as InboundUpgrade<Substream<TMuxer>>>::Error: Send + 'static,
	<<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::InboundProtocol as InboundUpgrade<Substream<TMuxer>>>::Future: Send + 'static,
	<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::OutboundProtocol: OutboundUpgrade<Substream<TMuxer>> + Send + 'static,
	<<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::OutboundProtocol as UpgradeInfo>::Info: Send + 'static,
	<<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::OutboundProtocol as UpgradeInfo>::InfoIter: Send + 'static,
	<<<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::OutboundProtocol as UpgradeInfo>::InfoIter as IntoIterator>::IntoIter: Send + 'static,
	<<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::OutboundProtocol as OutboundUpgrade<Substream<TMuxer>>>::Future: Send + 'static,
	<<<THandler as IntoProtocolsHandler>::Handler as ProtocolsHandler>::OutboundProtocol as OutboundUpgrade<Substream<TMuxer>>>::Error: Send + 'static,
	<NodeHandlerWrapper<<THandler as IntoProtocolsHandler>::Handler> as NodeHandler>::OutboundOpenInfo: Send + 'static, // TODO: shouldn't be necessary
	TConnInfo: ConnectionInfo<PeerId = PeerId> + fmt::Debug + Clone + Send + 'static,

[tomaka]

So the first step for this issue would be to create a BoxedNetworkBehaviour that takes as little template parameters as possible (normally just the substream and event type) and makes it possible to abstract over a NetworkBehaviour.

I ended up implementing that (in a local branch).
The problem is that you then cannot call the methods of the NetworkBehaviour.

I think that doing this would also be a good idea:

Instead of having only NetworkBehaviour, we would have several DiscoveryBehaviour, PubsubBehaviour, etc. traits with a mandatory output event type and a mandatory set of methods to implement. This makes it possible to combine behaviours in a straight-forward way.

[mxinden]

Related pull request: #1405

[thomaseizinger]

I don't think we will be doing this any time soon, closing as stale.