Skip to content
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

Improve libp2p config #576

Closed
vasco-santos opened this issue Mar 2, 2020 · 12 comments
Closed

Improve libp2p config #576

vasco-santos opened this issue Mar 2, 2020 · 12 comments
Labels
dx Developer Experience exp/expert Having worked on the specific codebase is important exploration kind/enhancement A net-new feature or improvement to an existing feature kind/support A question or request for support P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked

Comments

@vasco-santos
Copy link
Member

  • Version: N/A
  • Platform: N/A
  • Subsystem: N/A

Type: Enhancement

Severity: N/A

Description:

The developer experience of the current libp2p config is one of the pain points for libp2p users, particularly new users. Moreover, there are a few inconsistencies in the config structure and the distance between the modules setup and the configuration for those modules causes some confusion among libp2p users.

Here follows a few ideas already discussed around this topic:

User friendly libp2p configs

  • Pre baked configs (libp2p-browser, libp2p-node, etc)
  • Potentially test against js-libp2p suite

Known issues

  • module configuration separated from the modules
  • DHT not part of content routing / peer routing
  • transports with discovery built-in cannot be added to the discovery modules
  • transports with encryption & multiplexing built-in
  • sane defaults
@vasco-santos vasco-santos added kind/enhancement A net-new feature or improvement to an existing feature kind/support A question or request for support exp/expert Having worked on the specific codebase is important exploration status/ready Ready to be worked P2 Medium: Good to have, but can wait until someone steps up dx Developer Experience labels Mar 2, 2020
@vasco-santos
Copy link
Member Author

go-libp2p uses default transports, multiplexers, encryption, listenAddr, among other things:

https://github.com/libp2p/go-libp2p/blob/v0.5.2/libp2p.go#L33-L55

We might consider a similar approach

@vasco-santos
Copy link
Member Author

We should also consider #202 (listen, announce and noAnnounce addresses here)

@jacobheun
Copy link
Contributor

Here are my thoughts on moving js-libp2p configuration to a functional configuration model. I think this would allow us to make libp2p more extensible and improve the UX around configuration.

The Problem

Currently js-libp2p is diffult to configure, esecially for new users. Due to its modularity there are a variety of options to configure and configuring things improperly can result in a node that doesn't do what is needed.

Requirements

  • Users should be able to select a default configuration for their environment.
  • Users should be alerted if they are missing any modules, and how to resolve that.
  • Users should be able to easily extend basic configurations.
  • Users should be able to fully customize libp2p

Considerations

This propsal will require internal updates to libp2p to ensure its API supports making it easily extendable. There are likely some internal order of operations issues that will need to be resolved as part of this work, as we'll need to ensure all internals like the upgrader and registrar are created prior to calling configuration functions.

The Proposal: functional configuration

This proposal aims to migrate Libp2p.create() away from an object based configuration, to a functional configuration. There are many advantages to this approach:

  • Modules will be able to more easily extend libp2p (dht adding itself to contentRouting, peerRouting and peerDiscovery)
    • Modules can export their libp2p configuration functions so users can more easily extend their nodes
  • js-libp2p can export a variety of configuration options, with runtime module includes
    • This will enable us to throw actional errors for users (ex: "Error: you are missing modules. You can fix this by running: npm install -S libp2p-tcp libp2p-mplex libp2p-secio libp2p-websockets")
    • This enables us to easily provide configurations based on a users runtime environment (nodejs, browser (web/extension/service worker))

The API

await Libp2p.create([peerInfo], ...functions)

The first param is an optional PeerInfo instance, so users can provide/customize their PeerId. All additional params are functions with the signature function(libp2p), where libp2p is the instance of the new libp2p node being configured.

Exporting Configurations

js-libp2p will export various configuration functions, and this set will likely grow over time. Functions will exist in the libp2p/src/configs directory. We can include a description for each configuration including its supported environments, modules required, and the features it includes.

Example Configurations

Nodejs
Includes all recommended nodejs modules such as: transports, muxers, crypto, pubsub, mdns, etc.
const { nodejs } = require('libp2p/src/configs')

Nodejs Base
Only includes transports, muxers, and crypto. Does not include things like pubsub or mdns. Can be used for including/excluding desired modules.
const { nodejsBase } = require('libp2p/src/configs')

Browser
Includes all recommended browser modules such as: browser only transports, muxers, crypto, pubsub, etc.
const { browser } = require('libp2p/src/configs')

Browser Base
The same as Nodejs Base but with browser only transports.
const { browserBase } = require('libp2p/src/configs')

Pubsub
Can be combined with a Base configuration function to only add pubsub.
const { pubsub } = require('libp2p/src/configs')

Module Exported Configurations

Modules like the DHT, which require more advanced configuration and wiring into libp2p should export configuration functions allow users to more easily add it to their nodes. It is ideal for modules with more complex configuration to export at least 2 functions, a default and an advanced configuration.

For the DHT this might look like
const { dht } = require('libp2p-kad-dht/src/configs')
or
const { customDHT } = require('libp2p-kad-dht/src/configs')

The default dht function could simply be used by passing it to Libp2p.create, await libp2p.create(nodejsBase, dht).
The advanced function would take a custom config object and return a libp2p configuration function, await libp2p.create(nodejsBase, customDHT({ ...myDHTOptions })).

With the DHT we also really want to move this under the Peer and Content Routing components of libp2p, as well as moving RandomWalk into the PeerDiscovery system. This change would enable the DHT to handle the configuration itself, removing a lot of the complexity out of libp2p core. This allows libp2p to focus on being easily extendable.

Actionable Errors

When users are getting started, they should be provided with errors that are as actionable as possible. For example, if a user installs libp2p, npm install libp2p and then attempts to use the browser configuration function, they should receive an error telling them to install the necessary modules for that configuration if they are missing. This could look like:

$ npm install libp2p

const libp2p = require('libp2p')
const { browser } = require('libp2p/src/configs')

(async () => {
  // `Libp2p.create` will throw an actionable error: "Error: you are missing modules. You can fix this by running: npm install --save-prod libp2p-websockets libp2p-webrtc-star libp2p-mplex libp2p-secio libp2p-gossipsub"
  const libp2p = await Libp2p.create(browser)
  await libp2p.start()
})()

Running the above code would throw an error with a very specific message on all the modules that need to be installed. The user should be able to run the provided npm install command to resolve the issue.

@lidel
Copy link
Member

lidel commented Mar 4, 2020

Not sure how useful, but some related notes:

@hugomrdias
Copy link
Member

+1 on @jacobheun comment

gonna leave this here for a easier reference #578 (comment)

Its about having unified entry points for configs that automatically adjust to env.

Plus when implementing this don't forget about fringe envs like electron, react-native, etc.

@vasco-santos
Copy link
Member Author

@jacobheun this looks the type of config that we really need! 🚀

Just a few pieces missing in my head:

What type of configuration we offer in the event of Libp2p.create(). Provide a configuration with the bare minimum + websockets, so that it works both in node and browser?

What is your plan on configurations out of scope of subsystems? We would do the customBase function?

How would we do the Announce/... addresses think? We should probably need to rethink the config for the transports / dialer

@jacobheun
Copy link
Contributor

What type of configuration we offer in the event of Libp2p.create(). Provide a configuration with the bare minimum + websockets, so that it works both in node and browser?

IMO, we shouldn't have defaults that try to satisfy multiple environments. This is really where actionable error messages should kick in, to guide the user. As @hugomrdias mentions in #578 (comment), we can use browser build targeting via the pkg.browser field to select the browser default file instead of the node default. Unless users have pre installed modules this will error displaying the npm command to install the needed modules. I think it would be good to also print a Warning that the default configuration will be used and the user should check out [readme url] for configuration guidance.

What is your plan on configurations out of scope of subsystems? We would do the customBase function?

Not sure I follow what you mean. Is this in regards to configuring systems that aren't included in the base? If so, there are two options as I see it:

  1. Use a base config and then supply your own configuration function: Libp2p.create(browserBase, myCustomConfigFn)
  2. Fully customize your node with a single, or multiple functions: Libp2p.create(myConfigurationFn)

I think in most instances users will use the 1st option when customizing, but users that need very fine control could use option 2.

How would we do the Announce/... addresses think? We should probably need to rethink the config for the transports / dialer

Configuring this should be easy once we add support for this, as it will just involve setting the listen/announce/noAnnounce addresses on the pertinent system (PeerStore/TransportManager/etc). For defaults, we would really only set default listening addresses and likely when we setup each transport. I imagine we will also include sample functions for joining the public network (stardust/webrtc-star/etc) for both browser and nodejs, which would include announce addresses.

@mikeal
Copy link
Contributor

mikeal commented Mar 11, 2020

I have a related inquiry that might help in this thread.

In a week or so I’m going to want to use libp2p to get me a connection between two peers. I need to write some JS code that starts a peer and returns me something I can put in another peer in order to get a connection between them. I need code that will work in the browser and in Node.js and allow peers in each environment to connect to each other.

What is the smallest amount of code I can write to do that?

Back in June 2018 I found the configuration process so daunting that I wrote https://github.com/mikeal/libp2p-simple . With all the refactors that have happened this is probably much easier now, or is it?

@achingbrain
Copy link
Member

Just registering my view that the current libp2p config is full of gotchas.

For example, I can't use [WebRTC.tag] for both transport and peer discovery config.

This looks consistent and intuitive (IMO, once it was clear that WebRTCStar.tag is the key the peer discovery setup searches for):

libp2p: {
  // other config here...
  config: {
    peerDiscovery: {
      [WebRTCStar.tag]: {
        enabled: true
      }
    },
    transport: {
      [WebRTCStar.tag]: {
        wrtc
      }
    }
  }
}

but it explodes with:

Error: no WebRTC support
    at ClassIsWrapper.createListener (/Users/alex/test/node_modules/libp2p-webrtc-star/src/index.js:198:21)
    at TransportManager.listen (/Users/alex/test/node_modules/libp2p/src/transport-manager.js:151:36)
    at Libp2p._onStarting (/Users/alex/test/node_modules/libp2p/src/index.js:440:33)
    at Libp2p.start (/Users/alex/test/node_modules/libp2p/src/index.js:225:18)
    at start (/Users/alex/test/node_modules/ipfs/src/core/components/start.js:78:18)
    at async main (/Users/alex/test/index.js:14:16) {
  code: 'ERR_NO_WEBRTC_SUPPORT'
}

because WebRTCStar.tag is 'webRTCStar' but the transport config is looking for the key 'WebRTCStar'.

This works but looks weird:

libp2p: {
  // other config here...
  config: {
    peerDiscovery: {
      [WebRTCStar.tag]: {
        enabled: true
      }
    },
    transport: {
      WebRTCStar: {
        wrtc
      }
    }
  }
}

It'd be great if this stuff could be consistent in any future version. Just making the config keys case-insensitive would do it.

@vasco-santos
Copy link
Member Author

We should also takes this into account for this endeavour:

XPosting @jacobheun comment on #798 (comment)

Yeah, I'd really like to get rid of this whole boot delay logic. In reality we should probably have some async boot function and when that finishes these other systems initiate, but I think we can revisit this when we update the config logic. I'll review the module

@vasco-santos
Copy link
Member Author

Discussion on pre configuration, builds and repo structure relevant for this:

libp2p/js-libp2p-relay-server#2 (comment)
libp2p/js-libp2p-relay-server#2 (comment)

@maschad
Copy link
Member

maschad commented Sep 28, 2023

@maschad maschad closed this as completed Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx Developer Experience exp/expert Having worked on the specific codebase is important exploration kind/enhancement A net-new feature or improvement to an existing feature kind/support A question or request for support P2 Medium: Good to have, but can wait until someone steps up status/ready Ready to be worked
Projects
None yet
Development

No branches or pull requests

7 participants