-
Notifications
You must be signed in to change notification settings - Fork 1.1k
P2P handshake handling #2306
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
P2P handshake handling #2306
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2306 +/- ##
==========================================
- Coverage 65.81% 65.63% -0.19%
==========================================
Files 119 122 +3
Lines 9446 9532 +86
==========================================
+ Hits 6217 6256 +39
- Misses 2538 2575 +37
- Partials 691 701 +10 |
Hey @prestonvanloon, this usage of the libp2p API is not correct, that's why you're observing weird effects. The job of the connection manager is to keep the connection count in check to avoid killing commercial routers. What you want to do is attach your notifiee directly to the host's network:
You can:
Once you do that, here's the key: you want to open the stream via the The conn doesn't know about multistream negotiation, and will open a raw stream for you but will not negotiate the protocol. The call to So the result is that you have a raw stream with no protocol pinned on it on the wire, therefore the other end doesn't notify the stream handler for that protocol. I'm sorry our APIs are a bit messy here. I'm taking this issue as an experience report and once we settle the core refactor, we'll embark on a mission to make our APIs more cohesive and intuitive. |
shared/p2p/negotiation.go
Outdated
ConnectedF: func(net inet.Network, conn inet.Conn) { | ||
log.Debug("Checking connection to peer") | ||
|
||
s, err := h.NewStream(context.Background(), conn.RemotePeer(), handshakeProtocol) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@raulk, this is blocking / never opens the stream. Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you try to create the stream in a goroutine? "never block the callback" is the problem I think you're seeing here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works, thanks!
Thanks for the feedback @raulk. I'm still unable to create a stream between the peers. I'm seeing that host.NewStream is blocking indefinitely on both sides. |
Co-Authored-By: prestonvanloon <[email protected]>
The basic idea of this PR is to implement the following:
On new peer connection, send a "handshake" message to the peer. The peer should also respond with a "handshake" message. If the two peers do not agree on some of the information in the handshake, they should disconnect and not try to reconnect with each other.
Banning peers is blocked by libp2p/go-libp2p#274