Skip to content
This repository has been archived by the owner on Jul 21, 2023. It is now read-only.

fix stream ids to match mplex protocol spec #83

Closed
jacobheun opened this issue Oct 1, 2018 · 7 comments
Closed

fix stream ids to match mplex protocol spec #83

jacobheun opened this issue Oct 1, 2018 · 7 comments
Labels
kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up

Comments

@jacobheun
Copy link
Contributor

In regards to ipfs/js-ipfs#1601, it looks like streams id's are being set incorrectly. We need to make sure that stream id's match the spec, where initiator ids are odd and receiver id's are even. The js implementation currently has them backwards.

Spec: https://github.com/libp2p/mplex#protocol

@jacobheun jacobheun added status/ready Ready to be worked P2 Medium: Good to have, but can wait until someone steps up kind/bug A bug in existing code (including security flaws) labels Oct 1, 2018
@dryajov
Copy link
Member

dryajov commented Oct 1, 2018

@jacobheun, there is already an issue documenting it here - libp2p/mplex#3. I'm not sure why the fix to the spec wasn't merged (wording?). Bottom line, the spec was incorrect, since it was reverse engineered from the existing js code, and the id sequencing was interpreted incorrectly. I believe this fix has been already applied to the Go implementation (the js one is correct).

@dryajov
Copy link
Member

dryajov commented Oct 1, 2018

Also, I've heard that this fixes the multiple interop issues with Go (@Stebalien is this the still the case?).

@Stebalien
Copy link
Member

This should have been fixed and I believe the release has been fixed. Is this still an issue?

@Stebalien
Copy link
Member

Ah, I need to merge that spec PR (and possibly update it). I'll take a look.

@dryajov
Copy link
Member

dryajov commented Oct 1, 2018

Thanks, I think we never nailed down the correct wording, happy to wrap that up if its still an issue.

@Stebalien
Copy link
Member

I merged that PR and submitted a new one with some final edits. Mind taking a look? libp2p/mplex#5

@jacobheun
Copy link
Contributor Author

@dryajov ah, that's great, thanks! Closing this.

@ghost ghost removed the status/ready Ready to be worked label Oct 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

3 participants