Make event_process = false the default on derive(NetworkBehaviour)

[thomaseizinger]

With #2186, we are adding the until this day most-comprehensive example on how to use rust-libp2p.

This example is using event_process = false to integrate nicely with an eventloop-based design.

Back when it was added, we wanted to stay backwards compatible but at least in my view, setting event_process to false deemed to be very useful and "idiomatic".

Not having it the default is causing friction for users trying to follow our examples. (cc @whereistejas)

What are people's thoughts on making this the default with one of the next releases?

[wngr]

It depends a bit what how the "reference architecture" of a rust-libp2p based application looks like. The approach you advocated for and also proposed in #2186 seems to be that the "libp2p part" of an application is abstracted behind a interface (the Client) from the rest of the application.
An alternative approach is to embrace the abstractions proposed by libp2p, most importantly here the NetworkBehaviour: Custom behaviours are the building blocks of the application and those are hierarchically organized. The behaviours don't let their events bubble up, but rather handle and act on them on their own, in their own custom poll function.

I think here is a bit of an architectural mismatch between the intended (historical) architecture, and what makes sense (as learned by some of the production users of rust-libp2p).

Now, that being said, I personally very much favor abstracting the whole libp2p part of an application behind one interface, and handle everything in one giant event loop. That's why I wonder a bit, whether we shouldn't think about changing (or adding) the abstractions, rather than documenting historical complexity.

(On a side note: #1947 is also motivated by that, offering an opinionated (and easy way) to just ship some bytes from/to a peer..)

[thomaseizinger]

An alternative approach is to embrace the abstractions proposed by libp2p, most importantly here the NetworkBehaviour: Custom behaviours are the building blocks of the application and those are hierarchically organized. The behaviours don't let their events bubble up, but rather handle and act on them on their own, in their own custom poll function.

It is important to note that these approaches can be mixed and matched with each other. In my experience, starting with a flat composition of all your behaviours into one with event_process = false gets you going fast whilst providing a lot of flexibility.

As your application grows, so will the logic inside the eventloop. To stay on top of the complexity tyou can add layers of NetworkBehaviours in that use event_process = true or implement NetworkBehaviour manually. See for example this commit.

From this perspective, event_process = false seems like a better default to me, hence this issue.

Whether or not you are using a Client abstraction at the very top or put all your application logic into the eventloop is again a different story. However, I do believe that event_process = false on the top-most NetworkBehaviour is a "best practice" that should be followed. Otherwise you get into a mess where you need to pass stuff into a NetworkBehaviour that is annotated with #[behaviour(ignore)] just so you can access it within NetworkBehaviourEventProcess::inject_event.

[wngr]

Don't get me wrong, I agree with your proposal to make event_process = false the default. The point I tried to bring across was whether there is a place for another abstraction "next to the NetworkBehaviour". I think there is, I just don't know whether it's a complete new abstraction, or just a convenient wrapper.

[thomaseizinger]

The point I tried to bring across was whether there is a place for another abstraction "next to the NetworkBehaviour". I think there is, I just don't know whether it's a complete new abstraction, or just a convenient wrapper.

Got you! You might be interested in this discussion then :)

[mxinden]

👍 on event_process = false as the default.