ethereum / consensus-specs

Ethereum Proof-of-Stake Consensus Specifications
Creative Commons Zero v1.0 Universal
3.59k stars 985 forks source link

[Discussion] Mplex future #3490

Open jxs opened 1 year ago

jxs commented 1 year ago

Mplex is currently the main multiplexing protocol supported by all Ethereum consensus clients, while yamux MAY be supported: https://github.com/ethereum/consensus-specs/blob/e3a939e439d6c05356c9c29c5cd347384180bc01/specs/phase0/p2p-interface.md?plain=1#L164-L167

Libp2p is deprecating Mplex on libp2p ¹ and making yamux the main and the only multiplexer for TCP.

With that in mind, should we update the spec to swap Mplex to MAY support and yamux to MUST support?

AgeManning commented 1 year ago

Yep, I think the plan is to deprecate mplex once all the client teams have full support and prioritize it. Hopefully we can get this done before the next hard fork.

nisdas commented 1 year ago

Other clients can chime in, but I think we might only get yamux fully supported and tested after deneb across all clients. So we will have to maintain mplex for a while. For that reason mplex will still need to be a MUST for now.

arnetheduck commented 1 year ago

+1 for the hf after deneb. Practically, I envision clients will first support yamux then prefer it then mplex will finally be removed, resulting in a gradual migration.

p-shahi commented 1 year ago

Protocol Labs is deprecating Mplex on libp2p ¹ and making yamux the main and the only multiplexer for TCP.

@jxs nit but an important one, any chance you can change s/"Protocol Labs"/"different implementations like go, rust, and js-libp2p are" as PL =/= the different implementations

arnetheduck commented 1 year ago

nit

this is a libp2p spec deprecation first and foremost and PL maintains the spec - implementations follow, but they are not driving the deprecation in this case

jxs commented 1 year ago

Protocol Labs is deprecating Mplex on libp2p ¹ and making yamux the main and the only multiplexer for TCP.

@jxs nit but an important one, any chance you can change s/"Protocol Labs"/"different implementations like go, rust, and js-libp2p are" as PL =/= the different implementations

Thanks for the suggestion Prithvi, updated it

Menduist commented 1 year ago

Besides requiring stable yamux support in all implementation, the underlying libp2ps require a clever mechanism (which is not part of the specs) to have the same performance as mplex with more than 256kb in flight on a specific stream. AFAIK, only go supports this mechanism: https://github.com/libp2p/go-yamux/pull/54

Using yamux without this mechanism would cause an extra ~500ms delay on block propagation when they are >256kb, which we can expect from deneb

As long as this is not resolved, we shouldn't switch, imo mplex is harder to work with, because of the lack of stream backpressure, but we already dealt with that, so let's not "move fast and break things" just to please some urge to clean a github repo

Menduist commented 1 year ago

And FWIW I agree with @arnetheduck, how can you say that the push to switch to yamux is coming from the libp2p implementations, when more than half of them don't have even a stable yamux yet @p-shahi? (my definition of stable being "ran in prod for a few months")

While I appreciate the fact that thanks to the deprecation, things are starting to move towards yamux, this is very done very hastily, without consideration for networks with multiple libp2ps, and a big step back in term of libp2p interoperability.

(sorry for the off-topic)