Feature request: Allow extending libp2p.transports/libp2p.discovery instead of overriding it via flag

[mkg20001]

• Version: next
• Platform: *
• Subsystem: libp2p component

Type: feature

Severity: low

Description: Feature request: Allow extending libp2p.transports/libp2p.discovery instead of overriding it via flag

It would be good if there would be a flag to add custom transports in addition to the defaults, instead of fully overriding them.

This flag could be named libp2p.extendOnly or libp2p.extends

[lidel]

I had to override entire js-libp2p bundle used in Brave's js-ipfs just to add TCP transport.
I'd love to simplify this and switch to more granual extension mechanism suggested here.

[mkg20001]

I'm going to implement this in my current PR

[alanshaw]

Would it be better to pass the defaultOptions to the function so that you can just merge/replace as you please?

const ipfs = await IPFS.create({
  libp2p: ({ defaultOptions, peerInfo }) => {
    defaultOptions.transports.push(...)
    return new Libp2p(defaultOptions)
  }
})

[mkg20001]

While it does make sense in this use-case, I don't know if we should make libp2p an array and merge the function-results like I did in #2578, since I've now added the ability to add custom transports that have custom option available via the options object as part of the libp2p bundle, so one would need multiple entries, some of which might be objects, to fully control libp2p then.

[lidel]

I think we should keep this very simple and @alanshaw is on the right track.

Right now, if I want to add something before passing config to js-libp2p, I need to override entire defaultBundle.

What if we make a simple change and run custom function creating bundle against defaults from src/core/components/libp2p.js#L136 instead?

Something like this (in /src/core/components/libp2p.js):

module.exports = function libp2p (self, config) {
  const options = self._options || {}
  config = config || {}
-  // Always create libp2p via a bundle function
-  const createBundle = typeof options.libp2p === 'function'
-    ? options.libp2p
-    : defaultBundle
-
   const { datastore } = self._repo
   const peerInfo = self._peerInfo
   const peerBook = self._peerInfoBook
-  const libp2p = createBundle({ options, config, datastore, peerInfo, peerBook })
+  const libp2p = defaultBundle({ options, config, datastore, peerInfo, peerBook })

and later at the end of defaultBundle function:

+  let customBundle
+  if (typeof options.libp2p === 'function') {
+    customBundle = options.libp2p
+    delete options.libp2p
+  }
+
   const libp2pOptions = mergeOptions(libp2pDefaults, get(options, 'libp2p', {}))
+  if (customBundle) {
+    return customBundle({ libp2pOptions, config, datastore, peerInfo, peerBook })
+  }

  // Required inline to reduce startup time
  // Note: libp2p-nodejs gets replaced by libp2p-browser when webpacked/browserified
  const Node = require('../runtime/libp2p-nodejs')
  return new Node(libp2pOptions)

This way customBundle function gets libp2pOptions with defaults and can decide if those should be extended or replaced.

---

PS. I like the idea of having "drop-in" wrappers like stardust4ipfs, they make high level code easier to read. If we implement the above, those could still exist in the form of:

  libp2p: ({ datastore, peerInfo, peerBook, options, config }) => {
    options = require('stardust4ipfs')(options, config, peerInfo)
    options = require('brave4ipfs')(options, config, peerInfo, datastore, peerBook)
    return new Libp2p(options)
  }

We could add some sugar that enables us to run them sequentially to update config with the most common options:

  libp2p: [require('stardust4ipfs'), require('brave4ipfs')]

[mkg20001]

We could add some sugar that enables us to run them sequentially to update config with the most common options:

👍

options = require('stardust4ipfs')(options, config, peerInfo)

We should instead possibly pass the whole object as-is instead of splitting it up, so you can just directly do libp2p: require('stardust4ipfs') if you only need one extension

We should maybe not do return new Libp2p(options) but instead just return options for better chaining? Or no return at all and let new Libp2p() be handled outside of the "custom bundle function"?

This would also help enabling the libp2p: require('stardust4ipfs') usecase

[alanshaw]

Returning an instance allows you to subclass Libp2p. Please can we just do the simpliest thing and pass the libp2p options to the bundle function so you can extend/replace (like I suggested in #2579 (comment) and as @lidel fleshed out in #2579 (comment))?

[mkg20001]

So, I've created something that allows multiple methods of overriding the libp2p bundle

• libp2p: Function({ datastore, options, ...})
  • can directly modify options.libp2p and return undefined
  • can return new options object that either extends or overrides current options.libp2p by setting extend flag on it
  • can return a Libp2p instance
• libp2p: Object
  • can extend the libp2p object (by setting extend property) or override it

All of those can be chained, so one can do

{
  libp2p: [ // any of those is optional
    require('stardust4ipfs'),
    require('brave4ipfs'),
    {extend: true, transports: [MyCustomTransport]},
    ({options: {libp2p}}) => new Libp2p(libp2p)
  ]
}

If none of the Functions returns libp2p instance, then the default Node from ./runtime/libp2p-nodejs.js (or browser) will be called with the current value of options.libp2p

[mkg20001]

It's pushed in #2578

[alanshaw]

{
  libp2p: [ // any of those is optional
    require('stardust4ipfs'),
    require('brave4ipfs'),
    {extend: true, transports: [MyCustomTransport]},
    ({options: {libp2p}}) => new Libp2p(libp2p)
  ]
}

I get it, but this is too complicated. I really want to get the stardust change in but it should be a small focused change. If we're going to allow configuration like this then we should support it more generally for all the options in the constructor (which is a bigger proposal/change and separate PR and discussion). If you want this convenience right now you can easily create a module that accepts this array and spits out an options => Libp2p function.