dependency injection during boot time

[pgte]

Some libp2p modules require or would benefit from knowing the peerId and other values. It happens that, at the time the declarative options are created, the peerId is not yet known. As an example, libp2p-websocket-star would require the PeerId object to prevent spoofing. It happens that this value is not known before boot.

Here I propose a modules definition form that is backwards compatible and can be summed up to:

If a module is a function, libp2p assumes it is a constructor and executes that function, which should return a the module instance.

This would allow to declare config like this:

{
  modules: {
    transport: [
      wrtcStar // instance,
      ({peerId}) => new WSStar({id: peerId})
    ]
  }
}

[pgte]

One problem with this setting is the discovery. Usually, a discovery module is extracted from the transport module, something like this:

const wstar = new WRTCStar()
{
  modules: {
    transport: [
      wstar
    ],
    discovery: [ wstar.discovery ]
  }
}

One option to mitigate this would be to implicitely add the discovery module if the transport instance contains a .discovery attribute.

Another workaround would be to cache it like this:

let wsstar // caches the WSStar object instance
const createWSStar = ({peerId}) => (wsstar ? wsstar : wsstar = new WSStar({id: peerId}))

const libp2pOptions = {
  modules: {
    transport: [
      createWSStar
    ],
    discovery: [
      (options) => createWSStar(options).discovery
    ]
  }
}

Thoughts?

[pgte]

/cc @diasdavid @mkg20001

[pgte]

(Probably should replace peerId with peerInfo, but not really relevant for the overall discussion of the DI method here).

[dryajov]

Another approach might be passing an instance of swarm to the transport during initialization, the peerId/peerInfo can be extracted from it. We already do that for DHT and Circuit.

[daviddias]

Let's continue evaluating this, I need to think more about it but I am in the same page that we need DI.

[dryajov]

overall, 👍 for DI, should make things a lot more testable and such!

[mkg20001]

Currently I'm using a .setSwarm(swarm) function after transport creation to hack around this limitation in libp2p/js-libp2p-websocket-star#43
Something similar could be implemented. Like if (typeof transport._setSwarm === 'function') transport._setSwarm(swarm) after transport creation in libp2p.
Or even better: Just use the currently mostly-unused option object and add the swarm there

[mkg20001]

PR #160

[dryajov]

@mkg20001 why not just pass swarm as a constructor argument?

[mkg20001]

Because usually transports get created before the libp2p object gets created (afaik)

[dryajov]

@mkg20001 you're right, swarm gets created after the protos get instantiated.

[daviddias]

Here is what I'm thinking the right way to go about this.

Each libp2p module should expose:

// regular class that specifies the args that the module needs. Example:
const switch = new Switch(options, peerInfo, peerBook)

// static method that lets the instance be created with the "backbone". The "backbone" is an 
// object created inside js-libp2p that will have references to all the things that can be used by
// the libp2p internals.
const switch = Switch.create(options, backbone)

// method that lets us attach the "backbone" after the instance was created.
const switch = new Switch(options)
switch.attach(backbone)
// This way we can have our cake and eat it too. It will modules like *-star transports to
// be initiated so that we can access their discovery service reference before libp2p is 
// created, but then attach the backbone so that they can have access to things like the
// switch do p2p-circuit dials.

Note: I'm calling libp2p-switch to the libp2p-swarm as part of the rename endeavour https://github.com/libp2p/js-libp2p-swarm/issues/40

[richardschneider]

Why re-invent the wheel? If we we want IOC/DI then let's find one that has been well tested and used.

Fast search of NPM came up with https://www.npmjs.com/package/inversify

[daviddias]

Fair point, however, I don't see how that module will help us. It does way more things than what we need and it is TypeScript only.

We don't want full DI with a TypeSystem (although that would be great), we are looking to add support for libp2p modules to use libp2p itself.

[dryajov]

I'll leave this here for consideration - https://github.com/dryajov/opium, it's pure es6 (needs to be updated to remote all the babel stuff that is not needed anymore). But its a full fledged IoC library.

[jacobheun]

Reviewing this, it seems like the original problem that needed to be solved with DI was the boot time dependency injection, specifically knowing the peerInfo so that it could be handed off to modules, like transports.

Using Libp2p exclusively, this should be doable. One of the big problems was trying to customize libp2p within IPFS. The latest version of IPFS now supports passing a function that allows you to generate your libp2p bundle on ipfs boot time, which should resolve a lot of these issues. There is an example of that at https://github.com/ipfs/js-ipfs/tree/master/examples/custom-libp2p.

Are there current issues that this doesn't solve for that DI is needed for?

[daviddias]

@pgte any final remarks before closing this one?