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

Awesome Endeavour: Circuit Relay #830

Merged
merged 19 commits into from Nov 8, 2017

Conversation

Projects
None yet
3 participants
@dryajov
Member

dryajov commented Apr 13, 2017

We're getting close to completing the JS circuit implementation, this issue is to track further progress. Main work is being done here - libp2p/js-libp2p-circuit#9 libp2p/js-libp2p-circuit#14.

Assumptions (needs validation)

  1. All addresses are encapsulated with p2p-circuit by default to make them dialable over circuit relay
  2. If an explicit relay address is present don't add defaults, use that as the address instead
  3. Out of all available transports relay is dialed last

Outstanding work (this will be kept up to date with specific tasks as they come up):

  • Complete the relay spec libp2p/specs#15
  • Verify and complete integration points in js-ipfs and libp2p
    • Finalize integrating with libp2p
    • Finalize integrating with js-ipfs
  • More test coverage and integration tests (WIP)
    • this is the latest coverage report for unit tests, lots more is needed but we're getting there
=============================== Coverage summary ===============================
Statements   : 71.03% ( 255/359 )
Branches     : 54.96% ( 72/131 )
Functions    : 70.83% ( 17/24 )
Lines        : 71.23% ( 250/351 )
================================================================================
  • Cleanup implementation
    • cleanup pull stream read/write logic
    • cleanup relay.js
    • cleanup listener.js

Outstanding PRs:

Name Ready for Review Reviewed Merged Published
libp2p-tcp
libp2p-websockets
libp2p-webrtc-star
libp2p-webrtc-direct
libp2p-circuit
libp2p-swarm
libp2p
ipfs

@diasdavid @lgierth @whyrusleeping @dignifiedquire

NOTE: I'm currently bringing this issue up to date with all the latest developments - this is still in WIP.

EDIT: This issue is up to date!

@dryajov dryajov added the in progress label Apr 13, 2017

@dryajov dryajov changed the title from feat: adding circuit relay to [WIP] feat: adding circuit relay Apr 13, 2017

@diasdavid diasdavid referenced this pull request Apr 13, 2017

Closed

feat/v0.1.0 #9

48 of 53 tasks complete

@dryajov dryajov referenced this pull request Apr 16, 2017

Closed

WIP - Awesome Relay Endeavour #833

8 of 9 tasks complete

@diasdavid diasdavid changed the title from [WIP] feat: adding circuit relay to feat: adding circuit relay May 19, 2017

@diasdavid diasdavid added ready and removed in progress labels Jun 16, 2017

@diasdavid diasdavid changed the title from feat: adding circuit relay to Awesome Endeavour: Circuit Relay Jul 8, 2017

@dryajov dryajov referenced this pull request Aug 16, 2017

Merged

v0.1.0 #14

48 of 48 tasks complete

@dryajov dryajov requested a review from diasdavid Aug 29, 2017

@dryajov dryajov changed the title from Awesome Endeavour: Circuit Relay to WIP: Circuit Relay Aug 29, 2017

@diasdavid diasdavid changed the title from WIP: Circuit Relay to Awesome Endeavour: Circuit Relay Sep 1, 2017

@dryajov

This comment has been minimized.

Show comment
Hide comment
@dryajov

dryajov Sep 1, 2017

Member

@diasdavid

Here are the action items discussed last week:

  • Clean up libp2p tests
  • Add swarm tests
  • Work on go-js interop tests
  • Prepare demo that showcases circuit
    • Use the files transfer example
Member

dryajov commented Sep 1, 2017

@diasdavid

Here are the action items discussed last week:

  • Clean up libp2p tests
  • Add swarm tests
  • Work on go-js interop tests
  • Prepare demo that showcases circuit
    • Use the files transfer example

@diasdavid diasdavid referenced this pull request Sep 4, 2017

Closed

⚡️ v0.26.0 RELEASE 🚀 #986

16 of 16 tasks complete
@dryajov

This comment has been minimized.

Show comment
Hide comment
@dryajov

dryajov Sep 5, 2017

Member

A quick update on whats outstanding:

  • mafmt broke with the latest changes for webrtc addresses (mafmt master), I'm working on resolving it. The only address thats broken is the webrtc one, so it isn't really blocking the rest of the effort.
  • interop tests in js-ipfs. I've got some basic tests set up in test/core that work, but need to get additional interop tests set up:
test go-relay js-relay notes
go<->go
go<->js-cli
go<->js-browser
js-cli<->go
js-cli<->js-cli
js-cli<->js-browser
js-browser<->go
js-browser<->js-cli
js-browser<->js-browser

EDIT: I'll be adding the missing tests shortly. The reason they are not added yet, is because currently spawning nodes in browser runs is extremely cumbersome. Some of this has been captured in this issue - #1030.

Member

dryajov commented Sep 5, 2017

A quick update on whats outstanding:

  • mafmt broke with the latest changes for webrtc addresses (mafmt master), I'm working on resolving it. The only address thats broken is the webrtc one, so it isn't really blocking the rest of the effort.
  • interop tests in js-ipfs. I've got some basic tests set up in test/core that work, but need to get additional interop tests set up:
test go-relay js-relay notes
go<->go
go<->js-cli
go<->js-browser
js-cli<->go
js-cli<->js-cli
js-cli<->js-browser
js-browser<->go
js-browser<->js-cli
js-browser<->js-browser

EDIT: I'll be adding the missing tests shortly. The reason they are not added yet, is because currently spawning nodes in browser runs is extremely cumbersome. Some of this has been captured in this issue - #1030.

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Sep 11, 2017

Member

As an update to everyone following this PR, check @dryajov demoing it on the IPFS All Hands! => https://youtu.be/chAXj_vsR2s?t=25m01s

Member

diasdavid commented Sep 11, 2017

As an update to everyone following this PR, check @dryajov demoing it on the IPFS All Hands! => https://youtu.be/chAXj_vsR2s?t=25m01s

@dryajov

This comment has been minimized.

Show comment
Hide comment
@dryajov

dryajov Sep 14, 2017

Member

This are the notes (from memory) of a discussion held between @vyzo @Stebalien @diasdavid @dryajov on Sept. 14 regarding releasing go and js circuit:

Go:

  • Go circuit is merged and awaiting go ipfs release - rc candidate is coming soon
  • Go circuit does not announce circuit addresses to the DHT, but it would be nice to have a flag that would allow switching that functionality on/off - handy for experimenting once circuit is released.
    • @diasdavid should we add something like that to the JS implementation - probably not needed immediately since we're not announcing peers on the DHT just yet?

Js:

  • Interop testing is happening - see table in #830 (comment).
  • multiformats/js-mafmt#16 is no longer needed as it will be handled in the filter function of each transport.
  • @diasdavid to review and release libp2p-circuit to npm
  • @dryajov to finish up testing
  • common goal is to have circuit ready for allhands next week (Sept. 18)

Feel free to update this comment if I missed anything.

Member

dryajov commented Sep 14, 2017

This are the notes (from memory) of a discussion held between @vyzo @Stebalien @diasdavid @dryajov on Sept. 14 regarding releasing go and js circuit:

Go:

  • Go circuit is merged and awaiting go ipfs release - rc candidate is coming soon
  • Go circuit does not announce circuit addresses to the DHT, but it would be nice to have a flag that would allow switching that functionality on/off - handy for experimenting once circuit is released.
    • @diasdavid should we add something like that to the JS implementation - probably not needed immediately since we're not announcing peers on the DHT just yet?

Js:

  • Interop testing is happening - see table in #830 (comment).
  • multiformats/js-mafmt#16 is no longer needed as it will be handled in the filter function of each transport.
  • @diasdavid to review and release libp2p-circuit to npm
  • @dryajov to finish up testing
  • common goal is to have circuit ready for allhands next week (Sept. 18)

Feel free to update this comment if I missed anything.

@ReisenB ReisenB referenced this pull request Sep 15, 2017

Open

State of IPFS #1

@dryajov

This comment has been minimized.

Show comment
Hide comment
@dryajov

dryajov Sep 18, 2017

Member

An update on the current state of circuit:

The good news:

  • I've added most interop tests concerning go <-> js in the cli, browser is bit more complex, but I have a browser <-> circuit <-> browser test in core which passes
  • The changes discussed during the last call and captured in the comment above have been taken care of and are reflected in the table in the first comment
  • Manual testing was also done and the issues that were identified have been all taken care of
  • In general coding is complete and things just need a good review

The not so good news:

Running tests with stuff linked locally breaks the test suit in several places, basically some tests will fail when ran as part of the whole suit but work when ran isolated, I'm not sure whats going on and I haven't yet tried recreating this inside a docker container, so it might be my setup or just the crazy linking thats going on underneath. Here are my raw notes of things that I've seen fail:

 - interop repo (test/interop/repo.js) tests are failing with latest go ipfs master...
 - Tests in interface-ipfs-core are failing i.e. - "Connecting two peers with one address each"
 - some before each hooks in core/files-sharding.spec.js ("should be able to add dir without sharding") fail when running as part of the suit
 - bootstrap.spec.js fails when running as part of the suit
- init.spec.js fails when ran as part of the suit
-  gateway tests are failing when ran as part of the suit
- bitswap.spec.js fails when ran as part of the suit
Member

dryajov commented Sep 18, 2017

An update on the current state of circuit:

The good news:

  • I've added most interop tests concerning go <-> js in the cli, browser is bit more complex, but I have a browser <-> circuit <-> browser test in core which passes
  • The changes discussed during the last call and captured in the comment above have been taken care of and are reflected in the table in the first comment
  • Manual testing was also done and the issues that were identified have been all taken care of
  • In general coding is complete and things just need a good review

The not so good news:

Running tests with stuff linked locally breaks the test suit in several places, basically some tests will fail when ran as part of the whole suit but work when ran isolated, I'm not sure whats going on and I haven't yet tried recreating this inside a docker container, so it might be my setup or just the crazy linking thats going on underneath. Here are my raw notes of things that I've seen fail:

 - interop repo (test/interop/repo.js) tests are failing with latest go ipfs master...
 - Tests in interface-ipfs-core are failing i.e. - "Connecting two peers with one address each"
 - some before each hooks in core/files-sharding.spec.js ("should be able to add dir without sharding") fail when running as part of the suit
 - bootstrap.spec.js fails when running as part of the suit
- init.spec.js fails when ran as part of the suit
-  gateway tests are failing when ran as part of the suit
- bitswap.spec.js fails when ran as part of the suit
@dryajov

This comment has been minimized.

Show comment
Hide comment
@dryajov

dryajov Sep 25, 2017

Member

To help move this issue further, here is break down of where things are for the reviewers:

  • Coding is done across all modules and awaiting review
  • Interop tests for the js-go on the cli, have been added under test/interop
  • A simple browser test (browser<->relay<->browser) has also been added using an external node
    • more complex browser tests are required (#1030)
  • The table on the first comment reflects the current state of the PRs
  • The issues mentioned in the previous comment are still present and being troubleshooted. It would be nice to get a second set of eyes to eliminate npm linking issues
Member

dryajov commented Sep 25, 2017

To help move this issue further, here is break down of where things are for the reviewers:

  • Coding is done across all modules and awaiting review
  • Interop tests for the js-go on the cli, have been added under test/interop
  • A simple browser test (browser<->relay<->browser) has also been added using an external node
    • more complex browser tests are required (#1030)
  • The table on the first comment reflects the current state of the PRs
  • The issues mentioned in the previous comment are still present and being troubleshooted. It would be nice to get a second set of eyes to eliminate npm linking issues
@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Oct 13, 2017

Member

@dryajov started reviewing and merging, got blocked due to aegir update. Could you help with that too?

Member

diasdavid commented Oct 13, 2017

@dryajov started reviewing and merging, got blocked due to aegir update. Could you help with that too?

@dryajov

This comment has been minimized.

Show comment
Hide comment
@dryajov

dryajov Oct 13, 2017

Member

@diasdavid I'll update aegir to latest in circuit.

Member

dryajov commented Oct 13, 2017

@diasdavid I'll update aegir to latest in circuit.

@diasdavid diasdavid added P0 - Critical and removed ready labels Oct 17, 2017

@dryajov

This comment has been minimized.

Show comment
Hide comment
@dryajov

dryajov Oct 18, 2017

Member

From a conversation with @diasdavid, here is what needs to happen next to get circuit into js-ipfs!

The bellow modules need to be released first as everything else depends on them:

  • libp2p-tcp and libp2p-webrtc-direct have been released. yay! ❤️
  • libp2p-websockets
    • updated to latest aegir and awaiting review/release
  • libp2p-webrtc-start
    • updated to latest aegir and awaiting review/release

Once the above modules are released, we can start moving the next batch in the order listed:

  • libp2p-circuit
    • updated to the newest aegir and awaiting a review from @VictorBjelkholm ⭐️
  • libp2p-swarm
    • upgraded to the newest aegir and is awaiting libp2p-circuit release
  • libp2p
    • upgraded to the newest aegir and awaiting libp2p-swarm
  • js-ipfs
    • needs updating to newest aegir
    • needs libp2p released
Member

dryajov commented Oct 18, 2017

From a conversation with @diasdavid, here is what needs to happen next to get circuit into js-ipfs!

The bellow modules need to be released first as everything else depends on them:

  • libp2p-tcp and libp2p-webrtc-direct have been released. yay! ❤️
  • libp2p-websockets
    • updated to latest aegir and awaiting review/release
  • libp2p-webrtc-start
    • updated to latest aegir and awaiting review/release

Once the above modules are released, we can start moving the next batch in the order listed:

  • libp2p-circuit
    • updated to the newest aegir and awaiting a review from @VictorBjelkholm ⭐️
  • libp2p-swarm
    • upgraded to the newest aegir and is awaiting libp2p-circuit release
  • libp2p
    • upgraded to the newest aegir and awaiting libp2p-swarm
  • js-ipfs
    • needs updating to newest aegir
    • needs libp2p released

@diasdavid diasdavid added the ready label Oct 18, 2017

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Oct 22, 2017

Member

@dryajov what's the update on this #830 (comment) ?

Member

diasdavid commented Oct 22, 2017

@dryajov what's the update on this #830 (comment) ?

@dryajov

This comment has been minimized.

Show comment
Hide comment
@dryajov

dryajov Oct 22, 2017

Member

@diasdavid I'll update that table shortly.

Member

dryajov commented Oct 22, 2017

@diasdavid I'll update that table shortly.

Show outdated Hide outdated .aegir.js
Show outdated Hide outdated package.json

diasdavid and others added some commits Nov 8, 2017

Show outdated Hide outdated .aegir.js

dryajov and others added some commits Nov 8, 2017

@richardschneider

This comment has been minimized.

Show comment
Hide comment
@richardschneider

richardschneider Nov 8, 2017

Contributor
pull.asyncMap((val, cb) => glob(path.join(val, '/**/*'), cb)),

will most likely fail on Windows. The path.join will give back slashes.

Contributor

richardschneider commented on 36afc59 Nov 8, 2017

pull.asyncMap((val, cb) => glob(path.join(val, '/**/*'), cb)),

will most likely fail on Windows. The path.join will give back slashes.

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Nov 8, 2017

Member

@richardschneider true. Thanks for catching that. Let's do the Windows changes in another PR though :)

Member

diasdavid replied Nov 8, 2017

@richardschneider true. Thanks for catching that. Let's do the Windows changes in another PR though :)

This comment has been minimized.

Show comment
Hide comment
@richardschneider

richardschneider Nov 8, 2017

Contributor

No worries. I was just starting to do windows-interop on js-ipfs and I saw all the changes you are about to push. So I just sat back and watched.

Contributor

richardschneider replied Nov 8, 2017

No worries. I was just starting to do windows-interop on js-ipfs and I saw all the changes you are about to push. So I just sat back and watched.

diasdavid added some commits Nov 8, 2017

@diasdavid diasdavid merged commit 104ef1e into master Nov 8, 2017

0 of 6 checks passed

Node Security 16 vulnerabilities found
Details
ci/circleci Your tests are queued behind your running builds
Details
continuous-integration/appveyor/branch Waiting for AppVeyor build to complete
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@wafflebot wafflebot bot removed the in progress label Nov 8, 2017

@diasdavid diasdavid deleted the feat/add-relay branch Nov 8, 2017

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Nov 8, 2017

Member

@dryajov please go ahead and create a PR enabling Circuit Relay interop tests

Member

diasdavid commented Nov 8, 2017

@dryajov please go ahead and create a PR enabling Circuit Relay interop tests

@diasdavid

This comment has been minimized.

Show comment
Hide comment
@diasdavid

diasdavid Nov 8, 2017

Member

The work continues on #1063

Member

diasdavid commented Nov 8, 2017

The work continues on #1063

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment