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

@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 June 2, 2020 12:37
Copy link

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

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

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 July 7, 2020 12:41
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch 2 times, most recently from 4ca76ec to 60713c0 Compare July 8, 2020 15:50
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 60713c0 to b8bf044 Compare July 9, 2020 08:10
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 9913bc8 to b080670 Compare July 13, 2020 12:37
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 6e58f5b to ebb22d1 Compare July 13, 2020 15:45
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 5931f0e to 9765a95 Compare July 15, 2020 16:25
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from ab46cf0 to 3ab6b6c Compare July 22, 2020 18:06
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch 2 times, most recently from b5dac88 to 28541c7 Compare July 27, 2020 13:51
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 28541c7 to 7e3c541 Compare July 27, 2020 14:07
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch 2 times, most recently from be5778f to 4fae672 Compare December 22, 2020 19:06
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 4fae672 to 83cd4b7 Compare December 24, 2020 10:06
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from b9fda1e to 9bf5bcb Compare December 24, 2020 16:15
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from dd3ce31 to 264ce2a Compare December 28, 2020 21:57
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
strategy:
matrix:
os: [ubuntu-latest]
Copy link
Member Author

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 Dec 24, 2020

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

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 January 4, 2021 15:54
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 54c0948 to 01ec7bc Compare January 4, 2021 15:58
@vasco-santos vasco-santos force-pushed the feat/rendezvous-protocol-full-implementation branch from 212d754 to 5f025d3 Compare January 12, 2021 12:54
@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 January 18, 2021 10:32
@carsonfarmer
Copy link

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

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

@nonchip
Copy link

nonchip commented Dec 30, 2022

unless we have more specific requirements from the community that push this work

@vasco-santos well one of those requirements is the fact libp2p/js-libp2p#385 exists, the -stars are all deprecated and only work half the time anyway, this project is supposed to replace them, while js-libp2p claims to wanna work in a browser...

not sure how "web3" (:vomiting_face:) relates to this is or what other additional requirements have to be tracked to give "the only transport that actually works in a browser" priority, but actually being able to run libp2p and connect to enough nodes more than 5% of the time to even get a single pubsub message through on the same host even would be kinda nice.

@p-shahi
Copy link
Member

p-shahi commented Feb 1, 2023

@nonchip good news

the -stars are all deprecated and only work half the time anyway, this project is supposed to replace them, while js-libp2p claims to wanna work in a browser...

there are two solutions for js-libp2p to work in the browser. The first is js-libp2p-webtransport. Here's an example of using webtransport to fetch a file from Kubo https://github.com/libp2p/js-libp2p-webtransport/tree/main/examples/fetch-file-from-kubo

the other solution is native support for WebRTC. There are two parts to this. The first is browser-to-server connectivity via js-libp2p-webrtc which enables connecting to server nodes (i.e. a rust-libp2p or go-libp2p node running WebRTC). It's implemented per this specification: https://github.com/libp2p/specs/tree/master/webrtc#browser-to-public-server. This work is largely completed in js-libp2p and rust-libp2p. The go-libp2p pull request is currently in progress:

The second part of WebRTC is browser-to-browser: libp2p/js-libp2p#1462 ; work on this is in progress and we hope to deliver by the end of February. Here's the specification: libp2p/specs#497

So rest assured, there are plenty of ways of getting js-libp2p to work in the browser with support for native WebTransport and WebRTC.

@nonchip
Copy link

nonchip commented Feb 1, 2023

@p-shahi thanks!

The second part of WebRTC is browser-to-browser: libp2p/js-libp2p#1462 ; work on this is in progress and we hope to deliver by the end of February. Here's the specification: libp2p/specs#497

that's the bit i personally am relying on most because i'm building a 100% selfcontained single-file html "app" which can't expect a server ideally.

so far I'm using webrtc-star for both transport and discovery (the latter being "enhanced" by pubsub-peer-discovery since i'm heavily relying on pubsub anyway), but it's sadly very unreliable as soon as it gets "in the wild", apparently having issues with connectivity in general (dialling all kinds of invalid addresses), both gossip- and floodsub being unable to get anything through, and on top of all that apparently chrome has a bug where it just never deletes any closed webrtc connections so after like 3 minutes of seeing all kinds of things like ipfs nodes it simply refuses to let libp2p open more connections.

after weeks of careful tweaking it works like 50%ish reliably if i'm putting 4-6 nodes on the same non-public webrtc-star server, but any more or less than that, or if i just let them talk to each other for longer than like half an hour, and it still fails.

and with the various deprecations and stuff (and also the official webrtc-star server just crashing every few hours) i doubt i can improve my current codebase more with those dependencies, and the news of a better interoperable and specified implementation to come soon sounds definitely great!


Edit: i hope you forgive my frustration above (also @vasco-santos), and that you'll see why this project was total chaos for me, and then seeing a statement like "nah p2p in the browser won't happen for a while in js-libp2p" when it seemingly just never did anything (the 5% i mentioned weren't an understatement, took me weeks and careful tweaking of the "network size" to even be able to test it, and i can't rely on a fixed network size anyway, i wanna deploy that to users :P) was rather disheartening.

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.

5 participants