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

Extract Waku Relay from @waku/core #1151

Closed
fryorcraken opened this issue Feb 7, 2023 · 3 comments · Fixed by #1316
Closed

Extract Waku Relay from @waku/core #1151

fryorcraken opened this issue Feb 7, 2023 · 3 comments · Fixed by #1316
Assignees

Comments

@fryorcraken
Copy link
Collaborator

fryorcraken commented Feb 7, 2023

This is a change request

Problem

Waku Relay is based on libp2p-gossipsub. We use @chainsafe/libp2p-gossipsub.

This dependency has some issues:

Also, we currently recommend the usage of light push/filter/store in browser clients.

Relay in the browser has known scalability issues: #905.
Also, relay does not provide true anonymity, which is why protocols such as dandelion++ are being studied:

Proposed Solutions

Extract Waku Relay from @waku/core in a @waku/relay package.
For now, we can keep this in the same repo (js-waku). We can move it outside if it create developer experience friction.

Also move the createRelayNode function from @waku/create to @waku/relay.

End result should be that neither @waku/core nor @waku/create depend on @chainsafe/libp2p-gossipsub.

Do a deprecation phase as railgun use relay.

  1. Extract to @waku/relay but @waku/create still depends on it and expose createRelayNode, deprecate createRelayNode in @waku/create in JSDoc comment.
  2. Move createRelayNode to @waku/relay
@danisharora099
Copy link
Collaborator

danisharora099 commented Apr 27, 2023

Do a deprecation phase as railgun use relay.

Extract to @waku/relay but @waku/create still depends on it and expose createRelayNode, deprecate createRelayNode in @waku/create in JSDoc comment.
Move createRelayNode to @waku/relay

Do you mean that relay depend on libp2p-gossipsub while we go through the deprecation phase?


Exposing createRelayNode and createFullNode functions from @waku/relay proxied through @waku/create temporarily might not be possible as relay already depends on create (ref: 3fb768d) for the creation of FullNode and getting helpers like defaultPeerDiscovery. Doing otherwise leads to cases of cyclic dependencies.

Is it possible to flag the release with a "breaking change" label, and inform Railgun?

Please share if there are any alternative solutions in mind.

WIP PR: #1316

@fryorcraken
Copy link
Collaborator Author

Do a deprecation phase as railgun use relay.
Extract to @waku/relay but @waku/create still depends on it and expose createRelayNode, deprecate createRelayNode in @waku/create in JSDoc comment.
Move createRelayNode to @waku/relay

Do you mean that relay depend on libp2p-gossipsub while we go through the deprecation phase?

I would expect relay to always depend on libp2p-gossipsub.
During the deprecation phase, @waku/create can depend on @waku/relay.

Then, we can remove @waku/create dependency on @waku/relay (and hence libp2p-gossipsub and then createRelayNode function is moved somewhere (probably @waku/relay if we can avoid cyclic dependencies or maybe a new package, not sure).

Exposing createRelayNode and createFullNode functions from @waku/relay proxied through @waku/create temporarily might not be possible as relay already depends on create (ref: 3fb768d) for the creation of FullNode and getting helpers like
defaultPeerDiscovery. Doing otherwise leads to cases of cyclic dependencies.

It would be best if no library depend on @waku/create apart from the test packages.
The aim of @waku/create is to provide easy default value. It's aimed to be convenience over performance.
When a developer decide to move to performance over convenience, they should be able to remove their dependency on @waku/create.

You have already moved createRelayNode and createFullNode to @waku/relay, hence the potential cyclic dependency.
The proposal was to not move theses functions over and keep them in @waku/create.

I understand that defaultLibp2p is needed and is currently in @waku/create, hence the dependency.

We already know that defaultLibp2p is a tech debt, currently tracked with #1159

Knowing that, then doing the phased approach and leaving the create* functions in @waku/create sounds like the best solution, until #1159 is done.

Is it possible to flag the release with a "breaking change" label, and inform Railgun?

Looks like the phasing remains the best approach.

@danisharora099
Copy link
Collaborator

danisharora099 commented May 2, 2023

You have already moved createRelayNode and createFullNode to @waku/relay, hence the potential cyclic dependency. The proposal was to not move theses functions over and keep them in @waku/create.

I understand that defaultLibp2p is needed and is currently in @waku/create, hence the dependency.

We already know that defaultLibp2p is a tech debt, currently tracked with #1159

I was under the impression that this was a pre-requisite as mentioned in the original issue description:

Also move the createRelayNode function from @waku/create to @waku/relay.

Do a deprecation phase as railgun use relay.

Extract to @waku/relay but @waku/create still depends on it and expose createRelayNode, deprecate createRelayNode in @waku/create in JSDoc comment.
Move createRelayNode to @waku/relay

But according to the following, it makes sense. 👍

Knowing that, then doing the phased approach and leaving the create* functions in @waku/create sounds like the best solution, until #1159 is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants