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

feat: rendezvous protocol initial implementation #6

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

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented May 29, 2020

This PR adds the rendezvous protocol server implementation using the latest js-libp2p.

Spec: https://github.com/libp2p/specs/tree/master/rendezvous

This PR contains the server implementations, as well as some hardening and performance considerations. Take into account that this was a full rewrite of the module, so you do not need to have into account the older code.

Github actions CI only allows services to operate in linux. Other environments are not being used

@vasco-santos
Copy link
Member Author

vasco-santos commented May 29, 2020

@jacobheun this should be ready for an initial review on the structure, API and rpc parts. I also left a few questions that would like to discuss ( @vyzo would like your feedback on them as well)

@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 251a7e5 to f2fc803 Compare Jun 2, 2020
Copy link

@jacobheun jacobheun left a comment

Added some initial comments.

One thing I think would be valuable here is to write a separate readme on how this will work with js-libp2p, as I think that will help flush out the API here a bit better. Assume we have a single rendezvous server, a relay, and a few nodes who want to find one another on a certain namespace. libp2p should start by connecting to the rendezvous server and ask for nodes in the relay space (if they aren't dialable) so that they can add that as a listener. The nodes should also query for peers they want to connect to on the same namespace. Once a node as a dialable address (relay or otherwise), it should register on its target namespace.

We should consider what the responsibilities are internally for libp2p and what they are for the application.

  • Who is responsible for querying in the first place? Does libp2p handle this, or does the application need to account for it?
  • After a query is made, who is responsible for determining if we need more records? (cookie reuse)
  • Who is responsible for unregistering? Does libp2p do this on shutdown?

I think the use cases may vary quite a bit so it's reasonable for application logic to handle a lot of this, but clarifying that in usage guides and showcasing how to do those things is valuable and will make sure we've provided the support to do that.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/constants.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/rendezvous-point/index.js Outdated Show resolved Hide resolved
src/rendezvous-point/index.js Outdated Show resolved Hide resolved
src/rendezvous-point/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch 4 times, most recently from b23fc7a to b2156be Compare Jul 7, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch 2 times, most recently from 4ca76ec to 60713c0 Compare Jul 8, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 60713c0 to b8bf044 Compare Jul 9, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 9913bc8 to b080670 Compare Jul 13, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 6e58f5b to ebb22d1 Compare Jul 13, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 5931f0e to 9765a95 Compare Jul 15, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from ab46cf0 to 3ab6b6c Compare Jul 22, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch 2 times, most recently from b5dac88 to 28541c7 Compare Jul 27, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 28541c7 to 7e3c541 Compare Jul 27, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch 5 times, most recently from be5778f to 4fae672 Compare Dec 22, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 4fae672 to 83cd4b7 Compare Dec 24, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from b9fda1e to 9bf5bcb Compare Dec 24, 2020
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from dd3ce31 to 264ce2a Compare Dec 28, 2020
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
strategy:
matrix:
os: [ubuntu-latest]
Copy link
Member Author

@vasco-santos vasco-santos Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github actions does not support running services (mysql) when running tests on other OS... We can move into travis, but I feel we can just run it in ubuntu as I don't expect people will run a Rendezvous server in Windows/MacOS

- run: yarn
- run: npx nyc --reporter=lcov aegir test -t node -- --bail
- uses: codecov/codecov-action@v1
test-chrome:
Copy link
Member Author

@vasco-santos vasco-santos Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept the memory datastore previously implemented in the repo. It might be useful for small tests without spinning up MySQL, as well as to run the tests in the browser.

The browser tests are not useful as no one should run this in the browser, and I can remove them.

For example, for testing the libp2p client in libp2p core, it is easier to use the memory datastore, in order to not have to spin a MySQL db for running the tests

src/constants.js Outdated
@@ -0,0 +1,5 @@
'use strict'

exports.PROTOCOL_MULTICODEC = '/rendezvous/1.0.0'
Copy link
Member Author

@vasco-santos vasco-santos Jan 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch 2 times, most recently from c883455 to 54c0948 Compare Jan 4, 2021
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 54c0948 to 01ec7bc Compare Jan 4, 2021
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 212d754 to 5f025d3 Compare Jan 12, 2021
@vasco-santos vasco-santos mentioned this pull request Jan 13, 2021
1 task
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 8baf618 to 2840251 Compare Jan 18, 2021
@carsonfarmer
Copy link

carsonfarmer commented May 19, 2021

How likely is this to be merged any time soon? Looking really good!

@vasco-santos
Copy link
Member Author

vasco-santos commented May 21, 2021

How likely is this to be merged any time soon? Looking really good!

Thanks for reaching out. Apologies for the lack of visibility on the state of this. This is currently blocked on a couple of things:

It will likely not happen in the near future, unless we have more specific requirements from the community that push this work. The priority for this is being tracked in this project PR: protocol/web3-dev-team#67

@vasco-santos
Copy link
Member Author

vasco-santos commented Jun 24, 2021

As a status update on the Rendezvous Protocol, there is a new breaking change to the protocol that needs to be accounted for this PR libp2p/specs#338

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

Successfully merging this pull request may close these issues.

None yet

3 participants