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

upgrader: drop support for multistream simultaneous open #2557

Merged
merged 3 commits into from
Sep 12, 2023
Merged

upgrader: drop support for multistream simultaneous open #2557

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3cbce17
upgrader: drop support for multistream simultaneous open
marten-seemann Sep 5, 2023
260aedd
upgrader: simplify client / server logic
marten-seemann Sep 6, 2023
03ffbdd
update go-multistream to v0.5.0
marten-seemann Sep 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 10 additions & 15 deletions p2p/net/upgrader/upgrader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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.

isServer := dir == network.DirInbound
var st sec.SecureTransport
var err error
st, isServer, err = u.negotiateSecurity(ctx, conn, isServer)
st, err := u.negotiateSecurity(ctx, conn, isServer)
if err != nil {
return nil, "", false, err
}
Expand Down Expand Up @@ -308,41 +306,38 @@ func (u *upgrader) getSecurityByID(id protocol.ID) sec.SecureTransport {
return nil
}

func (u *upgrader) negotiateSecurity(ctx context.Context, insecure net.Conn, server bool) (sec.SecureTransport, bool, error) {
func (u *upgrader) negotiateSecurity(ctx context.Context, insecure net.Conn, server bool) (sec.SecureTransport, error) {
type result struct {
proto protocol.ID
iamserver bool
err error
proto protocol.ID
err error
}

done := make(chan result, 1)
go func() {
if server {
var r result
r.iamserver = true
r.proto, _, r.err = u.securityMuxer.Negotiate(insecure)
done <- r
return
}
var r result
r.proto, r.iamserver, r.err = mss.SelectWithSimopenOrFail(u.securityIDs, insecure)
r.proto, r.err = mss.SelectOneOf(u.securityIDs, insecure)
done <- r
}()

select {
case r := <-done:
if r.err != nil {
return nil, false, r.err
return nil, r.err
}
if s := u.getSecurityByID(r.proto); s != nil {
return s, r.iamserver, nil
return s, nil
}
return nil, false, fmt.Errorf("selected unknown security transport: %s", r.proto)
return nil, fmt.Errorf("selected unknown security transport: %s", r.proto)
case <-ctx.Done():
// We *must* do this. We have outstanding work on the connection
// and it's no longer safe to use.
// We *must* do this. We have outstanding work on the connection, and it's no longer safe to use.
insecure.Close()
<-done // wait to stop using the connection.
return nil, false, ctx.Err()
return nil, ctx.Err()
}
}