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

Faulty peer disconnection notification logic #130

Closed
vyzo opened this issue Dec 12, 2018 · 5 comments
Closed

Faulty peer disconnection notification logic #130

vyzo opened this issue Dec 12, 2018 · 5 comments
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@vyzo
Copy link
Collaborator

vyzo commented Dec 12, 2018

If there are multiple connections to the same peer, then the disconnect notification is emitted even though the peer is still connected, breaking gossipsub's peer tracking.
See the logs in vacp2p/nim-libp2p#8

@vyzo
Copy link
Collaborator Author

vyzo commented Dec 12, 2018

Also of note that the peer connection event is emitted twice as well.

@vyzo vyzo added the kind/bug A bug in existing code (including security flaws) label Dec 12, 2018
@vyzo
Copy link
Collaborator Author

vyzo commented Dec 12, 2018

related: #117

@vyzo
Copy link
Collaborator Author

vyzo commented Dec 13, 2018

Upon reflection, the issue here is that our peer management is completely incapable of handling multiple connections to the same peer.
The connection notification is emitted twice, which results in two sets of readers and writers, each of which can emit a peerDead event on failure.
So once the host closes one of the two connections, we are guaranteed to declare the peer dead even if the peer is still connected with the other connection.

We will need to completely rework on peer management logic it seems.

@vyzo
Copy link
Collaborator Author

vyzo commented Dec 13, 2018

cc @cheatfate

@vyzo
Copy link
Collaborator Author

vyzo commented Dec 14, 2018

Source issue confirmed as fixed by @cheatfate; closing.

@vyzo vyzo closed this as completed Dec 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

1 participant