Response to ls message of multistream-select looks invalid

[tomaka]

It seems that in rust-libp2p the response to ls messages starts with the number of protocols.

rust-libp2p/misc/multistream-select/src/protocol.rs

Line 203 in 8cec457

let mut out_msg = Vec::from(uvi::encode::usize(ps.len(), &mut buf));

However the specs mention that it should start with the total size of the message:
https://github.com/multiformats/multistream-select#ls-example-in-detail

[romanb]

Our implementation probably predates multiformats/multistream-select#19 and thus at some point was actually spec-compliant 😇.

[romanb]

However the specs mention that it should start with the total size of the message: [..]

Note that all messages that get encode()ed are sent down a LengthDelimited sink, so that size is prefixed there. The ls response you mention looks correct according to the old spec but apparently that was never the implementation in the other projects, so the <varint-number-of-protocols> is what should apparently just not be sent.

[mxinden]

As far as I can tell ls is only used in the dialer_select_proto_parallel flow when negotiating with less than or equal 3 protocols. According to this comment go-libp2p is only using ls for debugging.

Does this imply that all rust-libp2p users interacting with go-libp2p users have at most 3 protocols to negotiate at a given point in time?

Any suggestions on an upgrade strategy to eventually be compliant with the spec while not breaking compatibility with the previous rust-libp2p version?

• We could (1) disable parallel negotiation, (2) change our ls message and (3) reenable parallel negotiation over multiple rust-libp2p versions.

• We could parse each ls response both in the old and the new format, hoping to not receive a message that would parse valid for both formats. Off the top of my head, given that protocols are both length delimited and newline delimited, this should work.

[romanb]

We could (1) disable parallel negotiation, (2) change our ls message and (3) reenable parallel negotiation over multiple rust-libp2p versions.

I think that would be my preference and the most elegant solution. By disabling here I assume you mean just temporarily no longer making the dialer choose this strategy while at the same time still responding properly to ls for existing rust-libp2p nodes. Then the ls encoding/decoding can be changed in a subsequent version and re-enabled as a choice for the dialer in yet another version. That shouldn't be too hard to keep track of and the releases can come relatively quickly.