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

chore: add stardust to libp2p config and updated instructions on browser example #2831

Closed

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Mar 6, 2020

This PR adds libp2p-stardust transport to the ipfs browser runtime configuration.

Moreover, the stardust was added to the exchange-files example, in order to testing and provide an example for the community on how to use it.

Needs:

@vasco-santos vasco-santos force-pushed the chore/add-stardust-instructions-on-browser-example branch from 00489af to e918e0f Compare Mar 6, 2020
@vasco-santos
Copy link
Member Author

vasco-santos commented Mar 6, 2020

cc @mkg20001 (cannot ask you review on ipfs org)

@vasco-santos vasco-santos changed the title chore: add stardust instructions on browser example chore: add stardust to libp2p config and updated instructions on browser example Mar 9, 2020
@vasco-santos vasco-santos force-pushed the chore/add-stardust-instructions-on-browser-example branch from 903a1d8 to 3e88e65 Compare Mar 9, 2020
@vasco-santos vasco-santos force-pushed the chore/add-stardust-instructions-on-browser-example branch from 3e88e65 to 2da648c Compare Mar 9, 2020
@@ -942,6 +942,29 @@ const node = await IPFS.create({

The code above assumes you are running a local `signaling server` on port `9090`. Provide the correct values accordingly.

**Note:** We host a signalling server at star-signal.cloud.ipfs.team that can be used for practical demos and experimentation, it should not be used for apps in production. A libp2p-webrtc-star address, using the signalling server we provide, looks like:

/dns4/star-signal.cloud.ipfs.team/wss/p2p-webrtc-star/p2p/<your-peer-id>
Copy link
Contributor

@jacobheun jacobheun Mar 17, 2020

Choose a reason for hiding this comment

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

This address is going to end up changing when we update the deploy, which is in progress. This server is currently not stable and is old.

Copy link
Member Author

@vasco-santos vasco-santos Mar 17, 2020

Choose a reason for hiding this comment

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

Yeah, I will add this once we have the new server deployed


Note: The address above indicates WebSockets Secure, which can be accessed from both http and https.

#### Is there a more stable alternative to webrtc-star that offers a similar functionality?
Copy link
Contributor

@jacobheun jacobheun Mar 17, 2020

Choose a reason for hiding this comment

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

I don't think this is an accurate statement to make. It is an alternative but unless I am mistaken we have no performance measurements between the two to say stardust is more/less stable. Ideally, we should do some load testing on the rendezvous servers to make these comparisons.

config: {
Addresses: {
Swarm: [
'/dns4/ws-star.discovery.libp2p.io/tcp/443/wss/p2p-websocket-star'
Copy link
Contributor

@jacobheun jacobheun Mar 17, 2020

Choose a reason for hiding this comment

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

This address is wrong and is to websocket-star, it will need to change once there is a public test server to point to.

Copy link
Member Author

@vasco-santos vasco-santos Mar 17, 2020

Choose a reason for hiding this comment

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

I will put the own signaling instructions while we don't have this deployed

Copy link
Member

@lidel lidel left a comment

drive-by notes inline

packages/ipfs/test/core/libp2p.spec.js Outdated Show resolved Hide resolved
packages/ipfs/README.md Show resolved Hide resolved
config: {
Addresses: {
Swarm: [
`/ip4/0.0.0.0/tcp/5892/ws/p2p-stardust/p2p/${stardustServerId}`
Copy link
Member

@lidel lidel Mar 24, 2020

Choose a reason for hiding this comment

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

Just noticed that Address.Swarm notation is pretty different from one in webrtc-star and legacy ws-star where user just points at anonymous endpoint.

  • Is this because the stardust protocol requires prior knowledge of stardustServerId?
  • This is the main README, are we going to replace /ip4/0.0.0.0/tcp/5892 with a real server?
    (0.0.0.0 makes sense in examples where we run one on localhost, but here it may be confusing to users who try to copy&paste just to try it out)

Copy link
Member Author

@vasco-santos vasco-santos Mar 24, 2020

Choose a reason for hiding this comment

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

When the multiaddrs are added to libp2p peer for listening, js-ipfs adds a /p2p/{own_peer_id}.

In webrtc-star, it gets the valid multiaddr, however stardust multiaddr to listen must be /p2p/${stardustServerId}/p2p/{own_peer_id}. This is because stardust server is a libp2p node, while webrtc-star is not a libp2p node.

I thought about creating a fixture with the peer-id first, so that we have always the same id and not need this change. However, I think that this is not the best solution.

Anyway, the plan is to go as you asked. Once we have some servers deployed for experimenting, the server id issue will not be a problem anymore and we can replace this by the server address. The same will happen for webrtc-star.

Co-Authored-By: Marcin Rataj <lidel@lidel.org>
packages/ipfs/package.json Outdated Show resolved Hide resolved
@vasco-santos
Copy link
Member Author

vasco-santos commented Sep 17, 2020

We can close this as we will not move on with startdust

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

4 participants