Skip to content
This repository has been archived by the owner on Aug 23, 2019. It is now read-only.

Revisit Swarm - multitransport + upgrades - https://github.com/diasdavid/node-ipfs-swarm/issues/8 #10

Merged
merged 18 commits into from
Sep 23, 2015

Conversation

daviddias
Copy link
Member

Finished refactoring swarm, we still have some path to 'support all the transports in the world', but at least now it has a notion of what it looks like.

I believe also code became more simpler and less magical, but nevertheless handling more than one transport isn't easier than handling just one.

It does what we need to advance the rest of libp2p

  • handle several transports
  • capable of performing stream muxing
  • identify

Also, with the process and the organization, I had the chance to create more code documents and describe what has to be done next, so other people can contribute more too :)


# Description

ipfs-swarm is an abstraction for the network layer on IPFS. It offers an API to open streams between peers on a specific protocol.
libp2p-swarm is connection abstraction that is able to leverage several transports and connection upgrades (such as congestion control, encrypt a channel, multiplex several streams in one connection, and more. It does this by bringing protocol multiplexing to the application level (instead of the traditional Port level) using multicodec and multistream.
Copy link
Member

Choose a reason for hiding this comment

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

libp2p-swarm is a connection abstraction that is able to leverage several transports and connection upgrades, such as congestion control, channel encryption, multiplexing several streams in one connection, and more. It does this by bringing protocol multiplexing to the application level (instead of the traditional Port level) using multicodec and multistream.

@RichardLitt
Copy link
Member

Coverage issue:

14:53 ~/src/node-ipfs-swarm $ npm run coverage

> libp2p-swarm@0.4.1 coverage /Users/richard/src/node-ipfs-swarm
> lab -t 100 tests/*-test.js



  ..........bimbas
Caught exception: TypeError: Cannot read property 'listener' of undefined
do I get back
.

11 tests complete
Test duration: 686 ms
No global variable leaks detected
Coverage: 88.80% (44/393)
src/swarm.js missing coverage on line(s): 10, 11, 62, 96, 97, 99, 107, 113, 116, 134, 135, 146, 147, 149, 165, 166, 178, 179, 185, 186, 196, 199, 216, 217, 279, 280, 291, 292
src/identify/index.js missing coverage on line(s): 25, 26, 118, 122, 133, 137, 138, 139, 140, 141, 148, 149, 150, 151, 155, 156
Code coverage below threshold: 88.80 < 100


npm ERR! Darwin 14.4.0
npm ERR! argv "/Users/richard/.nvm/versions/node/v0.12.0/bin/node" "/Users/richard/.nvm/versions/node/v0.12.0/bin/npm" "run" "coverage"
npm ERR! node v0.12.0
npm ERR! npm  v2.5.1
npm ERR! code ELIFECYCLE
npm ERR! libp2p-swarm@0.4.1 coverage: `lab -t 100 tests/*-test.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the libp2p-swarm@0.4.1 coverage script 'lab -t 100 tests/*-test.js'.
npm ERR! This is most likely a problem with the libp2p-swarm package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     lab -t 100 tests/*-test.js
npm ERR! You can get their info via:
npm ERR!     npm owner ls libp2p-swarm
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /Users/richard/src/node-ipfs-swarm/npm-debug.log

@RichardLitt
Copy link
Member

Running npm run laf changes some files. Did you precommit this right?

14:56 ~/src/node-ipfs-swarm $ git status
On branch revisit
Your branch is up-to-date with 'origin/revisit'.
nothing to commit, working directory clean
14:56 ~/src/node-ipfs-swarm $ npm run laf

> libp2p-swarm@0.4.1 laf /Users/richard/src/node-ipfs-swarm
> standard --format

14:56 ~/src/node-ipfs-swarm $ git status
On branch revisit
Your branch is up-to-date with 'origin/revisit'.
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

    modified:   src/identify/index.js
    modified:   src/swarm.js
    modified:   tests/swarm-test.js

no changes added to commit (use "git add" and/or "git commit -a")
14:56 ~/src/node-ipfs-swarm $ git diff
diff --git a/src/identify/index.js b/src/identify/index.js
index a37b4a0..7a6abc6 100644
--- a/src/identify/index.js
+++ b/src/identify/index.js
@@ -39,14 +39,14 @@ function identify (muxedConns, peerInfoSelf, socket, conn, muxer) {
         }
         console.log('do I get back')

-        // TODO: Pass the new discovered info about the peer that contacted us
-        // to something like the Kademlia Router, so the peerInfo for this peer
-        // is fresh
-        //   - before this was exectued through a event emitter
-        // self.emit('peer-update', {
-        //   peerId: peerId,
-        //   listenAddrs: msg.listenAddrs.map(function (mhb) {return multiaddr(mhb)})
-        // })
+      // TODO: Pass the new discovered info about the peer that contacted us
+      // to something like the Kademlia Router, so the peerInfo for this peer
+      // is fresh
+      //   - before this was exectued through a event emitter
+      // self.emit('peer-update', {
+      //   peerId: peerId,
+      //   listenAddrs: msg.listenAddrs.map(function (mhb) {return multiaddr(mhb)})
+      // })
       })

       var mh = getMultiaddr(socket)
diff --git a/src/swarm.js b/src/swarm.js
index 7284738..1c764c9 100644
--- a/src/swarm.js
+++ b/src/swarm.js
@@ -67,9 +67,7 @@ function Swarm (peerInfo) {
     })
   }

-  self.addUpgrade = function (ConnUpgrade, options) {
-
-  }
+  self.addUpgrade = function (ConnUpgrade, options) {}

   self.addStreamMuxer = function (name, StreamMuxer, options) {
     self.muxers[name] = {
@@ -260,7 +258,7 @@ function Swarm (peerInfo) {
     // we pass muxedConns so that identify can access the socket of the other
     // peer
     self.handleProtocol(identify.protoId,
-        identify.getHandlerFunction(self.peerInfo, self.muxedConns))
+      identify.getHandlerFunction(self.peerInfo, self.muxedConns))
   }

   self.handleProtocol = function (protocol, handlerFunction) {
@@ -300,7 +298,7 @@ function Swarm (peerInfo) {
         muxer.on('stream', userProtocolMuxer)
       })
     } else {
-       // if no stream muxer, then
+      // if no stream muxer, then
       userProtocolMuxer(conn)
     }
14:56 ~/src/node-ipfs-swarm $ 

@RichardLitt
Copy link
Member

Why didn't you just delete swarm-old.js?

@daviddias
Copy link
Member Author

On: #10 (comment)

Yes, I did :) https://github.com/diasdavid/node-ipfs-swarm/blob/revisit/package.json#L25-L29 but for sanity, I use normal standard (which just checks for compliance), standard --format (npm run laf) formats some other little bits, that might even be standard compliant, but that is just how standard prefers.

@daviddias
Copy link
Member Author

@RichardLitt
Copy link
Member

On #10, yeah, it's in tests/.

Maybe change the code so that laf doesn't change anything? Kind of weird to have a script change stuff.

@daviddias
Copy link
Member Author

Maybe change the code so that laf doesn't change anything? Kind of weird to have a script change stuff.

laf is not part of pre-commit though, lint is, laf is a convenience, for people that don't have a editor that checks the style and helps formatting it for them.

@RichardLitt
Copy link
Member

So you're saying that if someone else commits, they should run laf, in which case there will be these unnecessary code changes with files not associated to their commits? I'm not sure that's smart.

@daviddias
Copy link
Member Author

They don't need to run laf, just npm run lint should not return any errors and if they don't feel like going by codestyle errors one by one, they can run standard --format (which laf(lint and format) is just an alias to) and see if everything gets solved for them. It is just a convenience script, not part of the pre-commit process or anything.

Is it too confusing? I can always take it off :)

@RichardLitt
Copy link
Member

Heh. It's only confusing that they're going to see changes that aren't theirs, or related to their code. That's... not right?

@daviddias
Copy link
Member Author

laf be gone! :) thanks @RichardLitt

@RichardLitt
Copy link
Member

:P Probably smarter.

@daviddias
Copy link
Member Author

good to merge then?

@RichardLitt
Copy link
Member

After deletion of dupe comment, yeah. Or before. 📦

daviddias added a commit that referenced this pull request Sep 23, 2015
@daviddias daviddias merged commit 4d9d8c9 into master Sep 23, 2015
@daviddias daviddias deleted the revisit branch September 23, 2015 22:09
@RichardLitt
Copy link
Member

👍

@jbenet
Copy link

jbenet commented Sep 23, 2015

@diasdavid looks like everything's all set, but lmk if you need my feedback here/elsewhere.

@daviddias
Copy link
Member Author

Thank you @jbenet! Feedback is always welcome :)

@RichardLitt gave me some awesome feedback at lightspeed, when you see him in person, give a ^5 for me :)

@daviddias daviddias mentioned this pull request Sep 28, 2015
65 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants