feat: add libp2p factory config option with example #1470

jacobheun · 2018-07-26T17:59:37Z

This adds the ability to pass a libp2p generator function to the ipfs configuration. This makes it easier for users to add custom modules to libp2p that require some startup time properties, like peerInfo (libp2p/js-libp2p#222).

I have also included an example of how to do this. I think this makes complex libp2p configuration much cleaner and easier to do.

Tests have been added for the old configuration options and the new generator option. I isolated them to avoid spinning up a full ipfs node.

alanshaw · 2018-07-29T22:06:41Z

README.md

- `libp2p` (object) add custom modules to the libp2p stack of your node
+- `libp2p` (object or function(ipfs, config)) add custom modules to the libp2p stack of your node
+
+    The libp2p option allows you to build your libp2p node by configuration, or via a generator. If you are looking to just modify the below options, using the object format is the quickest way to get the default features of libp2p. If you need to create a more customized libp2p node, such as with custom transports or peer/content routers that need some of the ipfs data on startup, a generator is a great way to achieve this.


I'm wondering if "generator" might be confused with an actual JS generator function? IMHO it would be more appropriate to call it a factory function.

Yes I agree that could get confusing, I will update the references to factory.

I think this happened but then got changed back?

wow, I spaced on that one. Came back from vacation saw some inconsistencies between factory/generator and changed everything the wrong way. Everything should be factory now, aside from the branch name.

alanshaw · 2018-07-29T22:07:03Z

README.md

+
+    The libp2p option allows you to build your libp2p node by configuration, or via a generator. If you are looking to just modify the below options, using the object format is the quickest way to get the default features of libp2p. If you need to create a more customized libp2p node, such as with custom transports or peer/content routers that need some of the ipfs data on startup, a generator is a great way to achieve this.
+
+    You can see the generator in action in the [custom libp2p example](examples/custom-libp2p).


💯 for adding an example too!

alanshaw · 2018-07-29T22:23:09Z

examples/custom-libp2p/index.js

+const libp2pGenerator = (_ipfsNode, _ipfsConfig) => {
+  // Set convenience variables to clearly showcase some of the useful things that are available
+  const peerInfo = _ipfsNode._peerInfo
+  const peerBook = _ipfsNode._peerBook


hmm, _peerInfo and _peerBook presumably are pefixed with underscores because they're considered private APIs - I don't think we should encourage people to use them like this!

Would it be simpler and also still solve the issue (libp2p/js-libp2p#222) to just allow passing peerInfo and peerBook to transports in the same way we do for peer discovery modules? https://github.com/libp2p/js-libp2p/blob/master/src/index.js#L185

I see the value in potentially being able to subclass Libp2p and also passing in the IPFS config, but if we don't have a use case for those two features I'd be tempted to go for a simpler approach for now to directly address @pgte's issue and which is also beneficial to people who use libp2p directly too.

Ah, you might have to ignore that last comment (apologies it's late and I should be asleep already), I just looked at the code being used https://github.com/ipfs-shipyard/peer-star-app/blob/master/src/transport/ipfs.js#L26 and the issue is not being able to use the same instance in both transport and peerDiscovery.

Let me re-review this tomorrow.

For _peerInfo and _peerBook it would probably be better to create some getter methods for those so it's clearer what users should be doing. What do you think?

For some additional context on another problem with the existing approach that a factory will help easily solve:
I am currently working on peer and content delegation for libp2p. The content delegate needs the peerInfo of the node, but also has options for other parameters, https://github.com/libp2p/js-libp2p-delegated-content-routing/blob/011adc9e90aefe786cc6103fbd3ebb9e007268b7/src/index.js#L41. It's possible that this could be done via the libp2p configuration, but I worry that the configuration is going to get annoyingly complicated for users trying to do anything more complex. As libp2p grows and more modules are introduced or created by users, I think that having more flexibility over the nodes creation is going to be really valuable for users.

Unless you think it's valuable to have these accessible outside of libp2p node creation context I'd probably have these passed separately to the factory function rather than exposing a new public API. I'd rather not have to support people randomly mutating the peer info/book.

I'll update it to pass those into the generator.

I've updated the generator to take a single object argument which includes the peerInfo, peerBook, ipfs node options and ipfs node config. The example has also been updated and I fixed the conflicts with master. Should be ready for another look over.

pgte

Looks good to me!

refactor: generator to factory

alanshaw · 2018-08-17T13:00:17Z

@jacobheun I've rebased the branch, just waiting for CI

daviddias · 2018-08-22T07:14:58Z

examples/custom-libp2p/index.js

+// Now that we have our custom factory, let's start up the ipfs node!
+const node = new IPFS({
+  libp2p: libp2pFactory
+})


Instead of calling it Factory, let's just reuse the wording around libp2p bundles (used in https://github.com/libp2p/js-libp2p#creating-your-own-libp2p-bundle, the website, slidedecks and other places).

Also, it would be great that instead of passing a factory, we could pass the libp2p instance itself.

I can switch up the wording to be libp2p bundle compliant.

For passing the libp2p instance, we can't currently do that due to needing the PeerInfo on creation, which is created on ipfs pre-start. When libp2p gets its state machine update we could make creation and startup more flexible so that PeerInfo could be provided to libp2p before it is started and then it could in turn bootstrap (either directly or via event hooks) all of its dependents that require PeerInfo of PeerId data.

I've updated the language to be more bundle centric, and added a link to the example in the main examples readme since it wasn't there yet.

alanshaw · 2018-08-24T20:47:32Z

CI green - @diasdavid is it 👍by you now?

daviddias

I see a bunch of times that the factory keyword is used. I understand that is indeed the design pattern. If you feel that the docs are clean enough and not add more unnecessary terms to the language already used in libp2p, then LGTM :)

daviddias · 2018-08-25T05:25:19Z

README.md

@@ -301,8 +301,11 @@ Modify the default IPFS node config. This object will be *merged* with the defau
 | Type | Default |
 |------|---------|
 | object | [`libp2p-nodejs.js`](https://github.com/ipfs/js-ipfs/blob/master/src/core/runtime/libp2p-nodejs.js) in Node.js, [`libp2p-browser.js`](https://github.com/ipfs/js-ipfs/blob/master/src/core/runtime/libp2p-browser.js) in browsers |
+| function | [`libp2p factory`](examples/custom-libp2p) |


Was leaving the word factory here intentional?

alanshaw · 2018-08-27T12:17:02Z

Ah sorry I only looked at the commit with the change - @jacobheun would you mind changing the rest as well?

jacobheun · 2018-08-28T11:53:13Z

Remainder of the factory language has been updated to bundle.

ghost assigned jacobheun Jul 26, 2018

ghost added the status/in-progress In progress label Jul 26, 2018

jacobheun requested review from daviddias and alanshaw July 26, 2018 18:01

jacobheun mentioned this pull request Jul 27, 2018

getting peerId (or peerInfo) instance in transport libp2p/js-libp2p#222

Closed

jacobheun requested a review from pgte July 27, 2018 12:40

jacobheun force-pushed the feat/libp2p-generator branch from fa7f5d0 to deab67d Compare July 27, 2018 15:38

alanshaw suggested changes Jul 29, 2018

View reviewed changes

jacobheun force-pushed the feat/libp2p-generator branch from 4ff95d6 to 31b1e61 Compare July 30, 2018 15:06

pgte approved these changes Aug 6, 2018

View reviewed changes

alanshaw mentioned this pull request Aug 9, 2018

⚡️ v0.32.0 RELEASE 🚀 #1497

Closed

22 tasks

jacobheun force-pushed the feat/libp2p-generator branch 3 times, most recently from a43a1f2 to 585c882 Compare August 16, 2018 17:37

jacobheun changed the title ~~feat: add libp2p generator config option with example~~ feat: add libp2p factory config option with example Aug 16, 2018

alanshaw approved these changes Aug 17, 2018

View reviewed changes

jacobheun added 4 commits August 17, 2018 13:59

feat: add libp2p generator config option with example

06a6f2f

docs: factory instead of generator

6ec62a5

feat: create getters for peerInfo and peerBook

3417b6f

refactor: dont expose new getters, use options

289b12e

refactor: generator to factory

alanshaw force-pushed the feat/libp2p-generator branch from 585c882 to 289b12e Compare August 17, 2018 12:59

ghost assigned alanshaw Aug 17, 2018

daviddias suggested changes Aug 22, 2018

View reviewed changes

docs: update libp2p example to use bundle language

bf92fb3

daviddias approved these changes Aug 25, 2018

View reviewed changes

docs: update language from factory to bundle

6480119

alanshaw merged commit 46222e1 into master Aug 30, 2018

ghost removed the status/in-progress In progress label Aug 30, 2018

alanshaw deleted the feat/libp2p-generator branch August 30, 2018 07:23

alanshaw mentioned this pull request Sep 10, 2018

preparations for js-ipfs 0.32 peer-base/peer-base#52

Closed

This was referenced Sep 15, 2021

[Snyk] Fix for 1 vulnerabilities rtpro/js-ipfs#45

Open

[Snyk] Fix for 1 vulnerabilities awaisab172/js-ipfs#33

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add libp2p factory config option with example #1470

feat: add libp2p factory config option with example #1470

jacobheun commented Jul 26, 2018 •

edited

Loading

alanshaw Jul 29, 2018

jacobheun Jul 30, 2018

alanshaw Aug 16, 2018

jacobheun Aug 16, 2018

alanshaw Jul 29, 2018

alanshaw Jul 29, 2018

alanshaw Jul 29, 2018

jacobheun Jul 30, 2018

alanshaw Aug 1, 2018

jacobheun Aug 9, 2018

jacobheun Aug 9, 2018

pgte left a comment

alanshaw commented Aug 17, 2018

daviddias Aug 22, 2018

jacobheun Aug 22, 2018

jacobheun Aug 22, 2018

alanshaw commented Aug 24, 2018

daviddias left a comment

daviddias Aug 25, 2018

alanshaw commented Aug 27, 2018

jacobheun commented Aug 28, 2018


		The libp2p option allows you to build your libp2p node by configuration, or via a generator. If you are looking to just modify the below options, using the object format is the quickest way to get the default features of libp2p. If you need to create a more customized libp2p node, such as with custom transports or peer/content routers that need some of the ipfs data on startup, a generator is a great way to achieve this.

		You can see the generator in action in the [custom libp2p example](examples/custom-libp2p).

feat: add libp2p factory config option with example #1470

feat: add libp2p factory config option with example #1470

Conversation

jacobheun commented Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgte left a comment

Choose a reason for hiding this comment

alanshaw commented Aug 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanshaw commented Aug 24, 2018

daviddias left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanshaw commented Aug 27, 2018

jacobheun commented Aug 28, 2018

jacobheun commented Jul 26, 2018 •

edited

Loading