Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove usage of the ls command in multistream-select #2925

Closed
dignifiedquire opened this issue Sep 21, 2022 · 2 comments · Fixed by #2934
Closed

Remove usage of the ls command in multistream-select #2925

dignifiedquire opened this issue Sep 21, 2022 · 2 comments · Fixed by #2934

Comments

@dignifiedquire
Copy link
Member

dignifiedquire commented Sep 21, 2022 •
edited
Loading

Summary

Currently, if there are more than 3 protocols listed, multistream-select uses a parallel approach which relies on the ls command.
This command has been removed from go-multistream and so all dials to kubo or other nodes using newer go-libp2p will fail in this case.

References

Possible Solution

Make it either configurable or remove the usage of the ls command.
I have a patch that just removes the parallel version, which is the simplest version. I am not sure how it could be avoided to not use ls.

Would you like to work on fixing this bug?

Yes
@mxinden mxinden mentioned this issue Sep 21, 2022
Merged
mxinden added a commit to mxinden/multistream-select that referenced this issue Sep 22, 2022
@mxinden
README.md: Mark ls as optional
34bad78 
go-multistream removed `ls` support in
multiformats/go-multistream#76. This breaks
compatibility with the multistream implementation in Rust see
libp2p/rust-libp2p#2925.

This commit updates the specification after the fact, explicitly making `ls`
support optional.
@mxinden mxinden mentioned this issue Sep 22, 2022
Merged
@mxinden
Copy link
Member

mxinden commented Sep 22, 2022

I have a patch that just removes the parallel version, which is the simplest version. I am not sure how it could be avoided to not use ls.

So far no one raised objections. In case no-one does on multiformats/multistream-select#22, I am in favor of your change and would appreciate a pull request.

dignifiedquire added a commit to dignifiedquire/rust-libp2p that referenced this issue Sep 23, 2022
@dignifiedquire
fix(multistream-select): remove parallel dialing optimization
d326bee 
This is to avoid the usage of the now optional `ls` command, and stay compatible with go-multistream.

Closes libp2p#2925
@dignifiedquire dignifiedquire mentioned this issue Sep 23, 2022
Merged
4 tasks
@dignifiedquire
Copy link
Member Author

@mxinden created #2934

mxinden pushed a commit that referenced this issue Sep 29, 2022
@dignifiedquire
misc/multistream-select/: Remove parallel dialing optimization (#2934)
c71115d 
This is to avoid the usage of the now optional `ls` command, and
stay compatible with go-multistream.

Closes #2925
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

multistream-select: Remove parallel dialing optimization dignifiedquire/rust-libp2p
2 participants
@dignifiedquire @mxinden