Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

🌟 feat: add websocket bootstrapers to the config #740

Merged
merged 4 commits into from
May 7, 2017

Conversation

daviddias
Copy link
Member

@daviddias daviddias commented Jan 30, 2017

Added the bootstrappers @lgierth created to the Bootstraper list. This is still not 100% working as it requires some updates throughout the dialing code (DNS addrs need to be valid wss/tcp).

Assignee: @dryajov

@daviddias daviddias added the status/in-progress In progress label Jan 30, 2017
@daviddias daviddias changed the title [WIP] feat: add websocket bootstrapers to the config feat: add websocket bootstrapers to the config Feb 1, 2017
@daviddias daviddias added exp/expert Having worked on the specific codebase is important help wanted Seeking public contribution on this issue labels Feb 13, 2017
@dryajov
Copy link
Member

dryajov commented Feb 13, 2017

As far as I understand, this:

Make the WebSockets transport DNS aware

requires something similar to this - libp2p/js-libp2p-webrtc-star#84 here https://github.com/libp2p/js-libp2p-websockets?

@daviddias
Copy link
Member Author

@dryajov exactly :)

@ghost
Copy link

ghost commented Feb 14, 2017

Also, you'll probably need an Origin header, unless the browser sets it for you.

@daviddias daviddias added status/ready Ready to be worked and removed status/in-progress In progress help wanted Seeking public contribution on this issue labels Mar 13, 2017
"/ip4/104.236.151.122/tcp/4001/ipfs/QmSoLju6m7xTh3DuokvT3886QRYqxAzb1kShaanJgW36yx"
"/ip4/104.236.151.122/tcp/4001/ipfs/QmSoLju6m7xTh3DuokvT3886QRYqxAzb1kShaanJgW36yx",
"/dns4/strawberry.i.ipfs.io/wss/ipfs/QmWyLSnMHW2H6bmCG9e9PQq4ARve94JduvGjbutUuzx4a8",
"/dns4/blueberry.i.ipfs.io/wss/ipfs/QmVcj9MATxGTAFoQSbrJvZ9Fbs4Jzvrxy9hyJeRwbW8NeA"
Copy link
Member Author

Choose a reason for hiding this comment

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

@lgierth I need a green light from you on these.

Note, it is the only way browser nodes will connect to bootstrapers, it would be extremely useful.

Copy link

Choose a reason for hiding this comment

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

Take these:

/dns4/ams-1.bootstrap.libp2p.io/tcp/443/wss/ipfs/QmSoLer265NRgSp2LA3dPaeykiS1J6DifTC88f5uVQKNAd
/dns4/sfo-1.bootstrap.libp2p.io/tcp/443/wss/ipfs/QmSoLju6m7xTh3DuokvT3886QRYqxAzb1kShaanJgW36yx
/dns4/lon-1.bootstrap.libp2p.io/tcp/443/wss/ipfs/QmSoLMeWqB7YGVLJN3pNLQpmmEk35v6wYtsMGLzSr5QBU3
/dns4/sfo-2.bootstrap.libp2p.io/tcp/443/wss/ipfs/QmSoLnSGccFuZQJzRadHn95W2CrSFmZuTdDWP8HXaHca9z
/dns4/sfo-3.bootstrap.libp2p.io/tcp/443/wss/ipfs/QmSoLPppuBtQSGwKDZT2M73ULpjvfd3aZ6ha4oFGL1KrGM
/dns4/sgp-1.bootstrap.libp2p.io/tcp/443/wss/ipfs/QmSoLSafTMBsPKadTEgaXctDQVcqN88CNLHXMkTNwMKPnu
/dns4/nyc-1.bootstrap.libp2p.io/tcp/443/wss/ipfs/QmSoLueR4xBeUbY9WZ9xGUUxunbKWcrNFTDAadQJmocnWm
/dns4/nyc-2.bootstrap.libp2p.io/tcp/443/wss/ipfs/QmSoLV4Bbm51jM9C4gDYZQ9Cy3U6aXMJDAbzgu2fzaDs64

These addresses will be alot slimer once we have /dnsaddr (see go-multiaddr-dns, need to write it up further).

Copy link
Member Author

Choose a reason for hiding this comment

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

The above actually don't work because they don't have valid SSL certs.

Tried with the fruits, they work with Node.js, but in the browser I get this:

image

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems that the gateway didn't like it, perhaps the origin? @lgierth how can I help you debug what happens on that node?

Copy link
Member Author

Choose a reason for hiding this comment

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

@lgierth I want to have my websockets bootstrappeeeers \o/

Copy link

Choose a reason for hiding this comment

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

On it, on it. Waiting on DNS verification for the cert now

Copy link

Choose a reason for hiding this comment

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

They're all working now \o/

@daviddias
Copy link
Member Author

Also, you'll probably need an Origin header, unless the browser sets it for you.

The browser does that automatically, using where the app has been loaded. Shall we enable websockets dials from anywhere?

@daviddias daviddias force-pushed the feat/add-bootstrapers-through-dns branch from 8c883de to 5b94de4 Compare March 21, 2017 18:07
@daviddias daviddias assigned ghost and daviddias Mar 21, 2017
@daviddias daviddias changed the title feat: add websocket bootstrapers to the config 🌟 feat: add websocket bootstrapers to the config Mar 21, 2017
@daviddias daviddias force-pushed the feat/add-bootstrapers-through-dns branch from 5b94de4 to 06cb6fe Compare March 23, 2017 15:24
@daviddias
Copy link
Member Author

@dryajov merged your websockets PR, looks good.

Here it is still failing though, see:

» jsipfs daemon
Initializing daemon...
Swarm listening on /ip4/127.0.0.1/tcp/4003/ws/ipfs/QmdsZZEuqzAxRJy2NNFrXEeUzmyMkj8mgn4ybmh9Cfa5bH
Swarm listening on /ip4/127.0.0.1/tcp/4002/ipfs/QmdsZZEuqzAxRJy2NNFrXEeUzmyMkj8mgn4ybmh9Cfa5bH
Swarm listening on /ip4/192.168.1.135/tcp/4002/ipfs/QmdsZZEuqzAxRJy2NNFrXEeUzmyMkj8mgn4ybmh9Cfa5bH
/Users/koruza/code/js-ipfs/node_modules/multiaddr/src/codec.js:54
      throw ParseError('invalid address: ' + str)
      ^

Error: Error parsing address: invalid address: /dns4/strawberry.i.ipfs.io/wss/ipfs

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

Ah, weird. Let me check it out.

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

@diasdavid I see mafmt 2.1.6 being pulled in by most other dependencies. Wonder if thats the issue? Still investigating.

 yarn list|grep mafmt
│  ├─ mafmt@^2.1.6
│  ├─ mafmt@^2.1.6
│  ├─ mafmt@^2.1.6
│  ├─ mafmt@^2.1.6
│  ├─ mafmt@^2.1.6
│  ├─ mafmt@^2.1.6
├─ mafmt@2.1.7

@daviddias
Copy link
Member Author

oh, it might be indeed. It should pick the latest patch though :( I guess we've to bump a minor and update everywhere manually .

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

@diasdavid yeah, maybe worth making some of this deps as peerDependencies, so that only the top package pulls it in.

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

btw, I can help updating some of this - wanna split them up?

@dignifiedquire
Copy link
Member

dignifiedquire commented Mar 23, 2017

@diasdavid you can just explicitly bump the version on the packages instead of releasing a minor, so that the minimum is the version you want

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

@dignifiedquire not sure if I follow :) What do you mean by

you can just explicitly bump the version on the packages instead of releasing a minor, so that the minimum is the version you want

Just bump and release each dep that pulls in mafmt? I though thats what @diasdavid is doing.

Also, are you guys aware of any tool like https://github.com/lerna/lerna for non monorepos? I've been resisting the idea of monorepos for some time, but I can definitely see a use case for it with the amount of modules in IPFS/libp2p/multformats/ipld.

I don't think that it makes sense to merge all IPFS and friends under one big monorepo, but it be still nice to:

a) manage all related code in github (and other VSCs) groups (pull/push/rebase across multiple repos)
b) update package versions at once across all the repos in the group or groups of repos
c) release all or some packages at once

If you're already doing that I'd love to hear what you're using for it :)

@dignifiedquire
Copy link
Member

I mean just changing the dependency in the package.json to "mafmt": "^2.1.7" so that the minimum is the new one

@dignifiedquire
Copy link
Member

re lerna, yes this was discussed before (I actually have a monorepo to make linking easier on my local machine) ref ipfs/community#174

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

@dignifiedquire ah, you're right. That should work.

Thanks for the monorepo pointer, gonna read that thread.

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

@diasdavid actually, the issue is something else, the bug seems to be in railing here - https://github.com/libp2p/js-libp2p-railing/blob/master/src/index.js#L26, the split is being done incorrectly and you end up with the wrong multiaddr - /dns4/strawberry.i.ipfs.io/wss/ipfs, note the dangling /ipfs. We should use decapsulate instead I think, I'll fix and PR in a sec.

@daviddias
Copy link
Member Author

@dryajov awesome, great catch! Makes sense :)

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

here is the PR for railing - libp2p/js-libp2p-bootstrap#53

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

@diasdavid there is one more issue with websockets. I'm looking into it right now, should be able to fix it quickly.

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

here is the PR to address the remaining issues in websockets - libp2p/js-libp2p-websockets#57

@dryajov
Copy link
Member

dryajov commented Mar 23, 2017

@daviddias
Copy link
Member Author

awesome work @dryajov ! :)

@daviddias daviddias force-pushed the feat/add-bootstrapers-through-dns branch from c347ecc to d5d2fe1 Compare March 29, 2017 05:35
@daviddias
Copy link
Member Author

Update

  • Adjust the nginx configs so that we can dial on websockets
  • Fix the transfer-files example (Chrome is not giving FilesList as an array anymore)
  • Transfer a file from a browser node to a gateway
  • Diagnose why the Websocket connections to the gateways die up so quickly

@daviddias
Copy link
Member Author

bring this up

@lgierth did you had the chance to configure ws timeouts on the bootstrapers?

@daviddias
Copy link
Member Author

I'm seeing an improve on stability of the websocket bootrapers, in fact, I don't see any going down anymore :)

Just pushed the last finishing touches to merge this PR, will merge once CI is green

@daviddias daviddias force-pushed the feat/add-bootstrapers-through-dns branch from f72e5ef to 3b680a7 Compare May 7, 2017 09:43
@daviddias
Copy link
Member Author

There are some failing pubsub tests but it seems they also fail on master, which tells me some dependency was updated which is causing a breaking change.

All tests pass locally and on Circle, which makes it especially hard to debug.

With regards to this PR, all of the respective functionality is sound.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/expert Having worked on the specific codebase is important
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants