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.

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

upgrader: drop support for multistream simultaneous open #2557

Merged
merged 3 commits into from
Sep 12, 2023

Conversation

marten-seemann
Copy link
Contributor

@marten-seemann marten-seemann commented Sep 5, 2023

Fixes #2330. For libp2p/specs#573.

Turns out there wasn't even a test for this...

We can also merge multiformats/go-multistream#107 first, cut a new release, and then update go-multistream in this PR.

@@ -201,9 +201,7 @@ func (u *upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma

func (u *upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID, dir network.Direction) (sec.SecureConn, protocol.ID, bool, error) {
Copy link
Member

@sukunrt sukunrt Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need the third returned value(bool) right?

AFAICT it is used to indicate whether we are the server which the caller now has complete context of.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. This allowed a small refactor to simplify things a tiny bit.

@marten-seemann
Copy link
Contributor Author

Updated the go-multistream dependency to v0.5.0.

@marten-seemann marten-seemann merged commit c2f0e13 into master Sep 12, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider dropping multistream-select sim open
2 participants