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

go-js Interop: Make the dialling/listening flow independent from identify #446

Closed
daviddias opened this issue Jun 10, 2016 · 3 comments
Closed
Assignees

Comments

@daviddias
Copy link
Member

Currently, there is a deadlock caused when connecting a go-ipfs and a js-ipfs node together, this is caused by how Identify is implemented. The tl;dr; is that go-ipfs executes the identify on the dialer side and blocks until that call is finished and js-ipfs won't 'handle' protocols until it finishes it's identify on the listening side causing a deadlock.

This issue revealed some other underlying issues that came from both implementations, since both implementations tried to hide/patch the problem that comes from having connections open (dialed or accepted), that were not identified (yet), causing issues for how protocols like bitswap, that depend on having the connection identified, to work.

In order to properly fix this issue, we need to remove this dependency/assumption on the protocols above libp2p and give them the ability to execute identify, if it didn't happen until then. By doing so, we keep our design principle of not making assumptions intact.

go-libp2p dialing flow
  1. nodeA dials to nodeB
  2. nodeA does the multistream to secio and spdy/yamux handshakes
  3. nodeA runs the Identify protocol
  4. nodeA blocks until Identify is finished. This means that no stream on this connection will be open until Identify finishes
js-libp2p dialing flow
  1. nodeA dials to nodeB
  2. nodeA does the multistream to secio and spdy/yamux handshakes
  3. nodeB runs the identify protocol. Then can open streams too on this conn
  4. nodeA opens as many streams as it wants

Issues with current design.

  • go-ipfs makes the dialer block on Identify, however, the dialer doesn't really require to run identify, because it is the only node in the connection that has the information of the other peer from the beginning.
  • lack of interop (main one)
  • protocols above are depending on the fact that Identify happened
  • go-ipfs never finishes its identify with js

Solution to mitigate this in the short term

Design changes to mitigate these issues completely.

  • add a .setPeerInfo() to the conn object, so that we can attach the Information we have about the peer in the other side
  • add a .getPeerInfo() to conn object that will retrieve the PeerInfo or execute the identify to figure it out. This will make it so that any other protocol that wants to learn its peerId, can execute it on demand, rather than hoping for it to be there. <- This is the main change that will make identify independent.
  • add a .setMuxer() to the conn object, so that the conn Object knows how to open a stream to run identify on it
  • Create a 'state' machine to signal the dialing state to a given peer
  • Extract identify to become its own module (while we are at it)

Other notes:

  • There was a decision to deprecate the protocols section on Identify, around last Summer, since it would be redundant to a multistream ls. Although it is a redundant operation, the fact is that identify is an operation that is 'almost' mandatory in the IPFS/libp2p context, which means that we can piggy back the protocols the other node supports in that round trip, so that we can save another round trip to figure out which protocols are supported. That being said, let's keep the protocols that being supported on the protobuf https://github.com/diasdavid/js-libp2p-swarm/blob/master/src/identify.proto#L24.
@dignifiedquire
Copy link
Member

@diasdavid thanks for writing this up :)

@daviddias
Copy link
Member Author

daviddias commented Jun 11, 2016

Update: There is one more quick implementation wise. go-libp2p does identify one way only, while js-libp2p does identify both ways and the listener waits to receive the information first before sending to the dialer. Since go-libp2p never sends nothing, it causes a deadlock there too.

Since we are in less of a rush, instead of going through the quick fix (hacky), I'll just refactoring these all of these things at once.

@daviddias
Copy link
Member Author

daviddias commented Jul 3, 2016

This is done now :)

I'll keep this issue open till I write the segment on libp2p spec to cover this nuances and how we can improve

@daviddias daviddias self-assigned this Sep 7, 2016
@daviddias daviddias transferred this issue from libp2p/js-libp2p-switch Aug 22, 2019
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

No branches or pull requests

3 participants