refactor: pubsub #467

vasco-santos · 2019-10-23T12:16:10Z

This PR aims to add the pubsub subsystem into js-libp2p.

In order to make it possible, the peer-store was created, as well as the registrar.

The peer-store will replace peer-book, using the same name as go-libp2p, being this the initial step for libp2p/js-libp2p#453, as well as for the new version of peer-store, which we will be working on after the refactor.

The registrar aims to be a bridge between all subsystems and js-libp2p (peer-store + connection-manager). Before this refactor, js-libp2p subsystems like pubsub and dht were responsible for tracking new peers that appear in the network and try to dial them using their respective protocols. Other than that, each subsystem was responsible for managing the connections and dials they were doing. This way, we aim to centralize this logic and enable subsystems to only care about their work, and leave the dialing and connection management for who should take care of them. It is also important pointing out that this is the first step to enable us to enhance the connection-manager. More context on libp2p/notes#13

Needs:

refactor: async migration ChainSafe/js-libp2p-gossipsub#49

package.json

src/index.js

src/peer-store.js

src/registrar.js

jacobheun · 2019-11-06T16:01:52Z

src/pubsub.js


-    peers: promisify((topic, callback) => {
+    getPeersSubscribed: (topic) => {


Would we ever have pubsub peers that aren't subscribed, if so, are those different? Just trying to get a better understanding of what this is expected to return. Might be good to add some jsdocs to the methods missing them.

We can have peers connected with pubsub, but that did not subscribe the topic provided.

I will add jsdocs here and more concrete stuff like examples in the docs PR

test/pubsub/implementations.node.js

test/pubsub/operation.node.js

Co-Authored-By: Jacob Heun <[email protected]>

jacobheun · 2019-11-15T12:28:58Z

test/pubsub/operation.node.js

+  })
+
+  // TODO: Needs identify push
+  describe.skip('pubsub started after connect', () => {


This suite should be able to be run now.

jacobheun · 2019-11-15T12:29:56Z

test/registrar/registrar.spec.js

-      }
-      throw new Error('should fail to register a protocol if the onDisconnect handler is not provided')
-    })
+    // TODO: not valid topology


Did you want to add something for this?

jacobheun · 2019-11-15T12:30:58Z

src/upgrader.js

@@ -186,7 +186,7 @@ class Upgrader {
          const { stream, protocol } = await mss.handle(Array.from(this.protocols.keys()))
          log('%s: incoming stream opened on %s', direction, protocol)
          connection.addStream(stream, protocol)
-          this._onStream({ connection, stream, protocol })
+          this._onStream({ connection, stream, protocol, remotePeer })


There shouldn't be a need to pass the remotePeer, as it's on the connection, connection.remotePeer. Same with the other two changes below

jacobheun

1 minor thing with the Topology check, otherwise this looks good

jacobheun · 2019-11-15T14:27:54Z

src/registrar.js

+    assert(
+      Topology.isTopology(topology) ||
+      MulticodecTopology.isMulticodecTopology(topology),
+      'topology must be an instance of interfaces/topology')


is-class should handle the base class extension check, so we should just need to check that it is a Topology, Topology.isTopology(topology)

Cool, that works :)

jacobheun

🚀

* feat: peer-store v0 * chore: apply suggestions from code review Co-Authored-By: Jacob Heun <[email protected]> * chore: address review * refactor: pubsub subsystem * chore: address review * chore: use topology interface * chore: address review * chore: address review * chore: simplify tests

jacobheun reviewed Oct 24, 2019

View reviewed changes

vasco-santos force-pushed the refactor/pubsub branch 8 times, most recently from 0fb7dc1 to 09cae28 Compare November 1, 2019 16:41

vasco-santos requested a review from jacobheun November 1, 2019 16:49

vasco-santos force-pushed the refactor/pubsub branch from 09cae28 to d5aba97 Compare November 1, 2019 18:23

This was referenced Nov 1, 2019

feat: peer store #470

Merged

feat: registrar #471

Merged

vasco-santos force-pushed the refactor/pubsub branch from d5aba97 to 692f6a6 Compare November 2, 2019 14:53

jacobheun force-pushed the refactor/pubsub branch from 692f6a6 to 4be379e Compare November 4, 2019 15:06

vasco-santos mentioned this pull request Nov 5, 2019

refactor: async libp2p/js-libp2p-pubsub#26

Merged

1 task

vasco-santos force-pushed the refactor/pubsub branch 5 times, most recently from 0158558 to 221f613 Compare November 6, 2019 15:14

vasco-santos requested review from jacobheun and removed request for jacobheun November 6, 2019 15:40

jacobheun reviewed Nov 6, 2019

View reviewed changes

vasco-santos force-pushed the refactor/pubsub branch from 959c4ad to 9b68696 Compare November 14, 2019 14:33

vasco-santos and others added 5 commits November 14, 2019 15:33

feat: peer-store v0

dca4727

chore: apply suggestions from code review

ec5319a

Co-Authored-By: Jacob Heun <[email protected]>

chore: address review

751bc00

refactor: pubsub subsystem

dcd127b

chore: address review

cdfd023

vasco-santos force-pushed the refactor/pubsub branch 2 times, most recently from b48661e to 3355780 Compare November 14, 2019 15:13

chore: use topology interface

4cc9736

vasco-santos force-pushed the refactor/pubsub branch from 3355780 to 4cc9736 Compare November 14, 2019 18:13

jacobheun reviewed Nov 15, 2019

View reviewed changes

chore: address review

340edf5

jacobheun approved these changes Nov 15, 2019

View reviewed changes

vasco-santos force-pushed the refactor/pubsub branch 2 times, most recently from 1ca2d87 to b33aca0 Compare November 15, 2019 15:13

chore: address review

6240dc0

vasco-santos force-pushed the refactor/pubsub branch from b33aca0 to 6240dc0 Compare November 15, 2019 15:15

chore: simplify tests

9b9ac54

jacobheun approved these changes Nov 15, 2019

View reviewed changes

jacobheun merged commit 392c440 into refactor/async-await Nov 15, 2019

jacobheun deleted the refactor/pubsub branch November 15, 2019 15:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: pubsub #467

refactor: pubsub #467

vasco-santos commented Oct 23, 2019 •

edited

Loading

jacobheun Nov 6, 2019

vasco-santos Nov 6, 2019

jacobheun Nov 15, 2019

jacobheun Nov 15, 2019

jacobheun Nov 15, 2019

jacobheun left a comment

jacobheun Nov 15, 2019

vasco-santos Nov 15, 2019

jacobheun left a comment


		peers: promisify((topic, callback) => {
		getPeersSubscribed: (topic) => {

refactor: pubsub #467

refactor: pubsub #467

Conversation

vasco-santos commented Oct 23, 2019 • edited Loading

jacobheun Nov 6, 2019

Choose a reason for hiding this comment

vasco-santos Nov 6, 2019

Choose a reason for hiding this comment

jacobheun Nov 15, 2019

Choose a reason for hiding this comment

jacobheun Nov 15, 2019

Choose a reason for hiding this comment

jacobheun Nov 15, 2019

Choose a reason for hiding this comment

jacobheun left a comment

Choose a reason for hiding this comment

jacobheun Nov 15, 2019

Choose a reason for hiding this comment

vasco-santos Nov 15, 2019

Choose a reason for hiding this comment

jacobheun left a comment

Choose a reason for hiding this comment

vasco-santos commented Oct 23, 2019 •

edited

Loading