libp2p / specs

Technical specifications for the libp2p networking stack
https://libp2p.io
1.56k stars 273 forks source link

Pubsub protocol underspecified #425

Closed wemeetagain closed 2 years ago

wemeetagain commented 2 years ago

Pubsub implementations currently have the unspecified behavior of opening two streams per peer (one inbound, one outbound), and treating each stream as unidirectional.

This behavior, as well as the behavior surrounding how to handle inconsistencies between the protocol negotiated between the inbound and outbound stream should be specified.

Or alternatively, a single-stream approach (as most/all? other defined protocols) should be specified, and implementations changed to suit.

vyzo commented 2 years ago

We are not going to change the implementation(s) at this point.

Feel free to submit pr for the unidirectional stream behaviour.

mxinden commented 2 years ago

Or alternatively, a single-stream approach (as most/all? other defined protocols) should be specified, and implementations changed to suit.

Agreed with @vyzo that at this point I don't think changing to a single-stream approach would be worth the large effort.

At some point we might even consider going the opposite route, enabling implementations to use more than one stream, thus decreasing the degree of head-of-line blocking across concurrent messages.

This behavior, as well as the behavior surrounding how to handle inconsistencies between the protocol negotiated between the inbound and outbound stream should be specified.

Would you mind proposing a change to the specification @wemeetagain?

wemeetagain commented 2 years ago

Yeah I can make a PR

mxinden commented 2 years ago

Friendly ping. @wemeetagain are you still interested proposing a patch?

wemeetagain commented 2 years ago

Thanks for the ping. Will try to take a look this week and submit something, anything :crossed_fingers:

mxinden commented 2 years ago

With https://github.com/libp2p/specs/pull/442 merged, I am closing here. Thanks @wemeetagain for raising this.