Skip to content
This repository has been archived by the owner on Aug 19, 2022. It is now read-only.

handle TCP simultaneous open (option 4) #38

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

marten-seemann
Copy link
Collaborator

As described in libp2p/specs#227 (comment).

crypto.go Outdated Show resolved Hide resolved
conn.go Show resolved Hide resolved
conn.go Show resolved Hide resolved
@vyzo
Copy link
Contributor

vyzo commented Nov 25, 2019

Another concern here is that we need to signal the role to the multiplexer, at least in the case of yamux.
We might have to change the interface to return the role, or maybe we should change the transport upgrader to select based on the peer IDs.
@Stebalien thoughts?

@marten-seemann marten-seemann marked this pull request as ready for review November 26, 2019 07:57
@marten-seemann
Copy link
Collaborator Author

@vyzo I can see two ways to do implement the signaling:

  1. Add a IsClient() bool function the SecureConn interface, or
  2. change the function signature of SecureOutbound to SecureOutbound(context.Context, net.Conn, peer.ID) (SecureConn, bool /*is client */, error).

I think that 2. would be the cleaner solution. Option 1. would also add IsClient() to all interfaces that build on top of SecureConn, and it's not clear if that function actually makes sense on those interfaces. For example, a stream multiplexer might decide to initialize itself in client mode on the server side, and in server mode on the client side.

@vyzo
Copy link
Contributor

vyzo commented Nov 26, 2019

Agreed, option 2 is what I chose for multiselect/1.1.

@marten-seemann
Copy link
Collaborator Author

@vyzo Sounds good.
Where can I see the code for ms-1.1? Can you give me a link the corresponding PRs?

@vyzo
Copy link
Contributor

vyzo commented Nov 26, 2019

libp2p/go-libp2p#712 collects all the PRs together.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants