no peer:connect event with autoDial

[olivier-nerot]

With libp2p V0.28, peer:connect events seems not to be fired

const peerid = ...;
const libp2p = await Libp2p.create({
        peerId: peerid,
        addresses: {
          listen: ['/ip4/0.0.0.0/tcp/0'],
        },
        modules: {
          transport: [TCP],
          streamMuxer: [Mplex],
          peerDiscovery: [MulticastDNS],
        },
        config: {
          peerDiscovery: {
            autoDial: true,
            [MulticastDNS.tag]: { // or mdns: ?
              interval: 1000,
              enabled: true,
            },
          },
        },
      });

      libp2p.on('peer:discovery', (peerId) => {
        debug('Discovered: %O', peerId);
        // libp2p.dial(peerId);
      });

      libp2p.connectionManager.on('peer:connect', (connection) => { 
        debug('peer connected %O',connection);
      });

      libp2p.start();

With the same setup, if I launch libp2p.start() from another node, the peer:discovery is well logged, but not peer:connect. I thought the autoDial:true should make a peer dials other discovered peers automatically.
I've tried also to call libp2p.dial(peerId) into the peer:discovery event, but peerId has no multiaddrs value, and have not found how to dial a discovered peer.

The documentation is sometimes confusing with this discovery process (for example, some explains to use mdns, other tells to use [MulticastDNS.tag]in config)

[jacobheun]

This is a usability bug. We should be throwing an error on Libp2p.create as you're not supplying any connection encryption, you need either libp2p-noise or libp2p-secio in your configuration https://github.com/libp2p/js-libp2p/blob/master/doc/CONFIGURATION.md#basic-setup.

If you're intentionally not using crypto for development/testing purposes you can use Plaintext, const Plaintext = require('libp2p/src/insecure/plaintext'), in place of Secio or Noise but this shouldn't be used outside of testing.

[olivier-nerot]

Thanks ! yes, an error thrown would help. I've not seen either that encryption was mandatory. This should be added in documentation.

Dealing with this setup, to better understand, does [MulticastDNS.tag] or mdns be used ? And If autoDial is false, how is it possible to dial a peer on peer:discovery event, as peerid has no multiaddrs whereas dial() requires them ?

[vasco-santos]

@olivier-nerot I will have a look into the docs to see where we have these inconsistencies and also add the error and docs for the required encryption.

Dealing with this setup, to better understand, does [MulticastDNS.tag] or mdns be used ?

We aim to use MulticastDNS.tag to be specific on where the tags come from in the discovery modules

https://github.com/libp2p/js-libp2p-mdns/blob/master/src/index.js#L104

Its value is mdns and that's why it will work either way.

And If autoDial is false, how is it possible to dial a peer on peer:discovery event, as peerid has no multiaddrs whereas dial() requires them ?

When a peer:discovery event happens, the discovered peer data is stored in the PeerStore libp2p/js-libp2p/v0.28.0/src/index.js#L485-L493. libp2p.dial accepts a multiaddr and PeerId as a target to dial. If you provide a multiaddr, it will be used for the dial, while if you provide a PeerId, we will look into the PeerStore.AddressBook if we know any multiaddrs for the provided peer. If we do, we just try to dial using the known multiaddrs. You can read more about the PeerStore flows in libp2p/js-libp2p/v0.28.0/src/peer-store

[olivier-nerot]

Thanks a lot. I understand it better. To help, you have MulticastDNS.tag in the Customizing Peer Discovery section of Configuration.md and mdns in the 2.js example
When I have gathered a working sample, I will push it here if it may help.

[vasco-santos]

I created #665 that solves what was discussed here. I hope the visibility for the modules being required is better now.

Per #576 we have to get the configuration more obvious and easy for everyone, and not require users to read a bunch of documents for getting started.