libp2p / js-libp2p

The JavaScript Implementation of libp2p networking stack.
https://libp2p.github.io/js-libp2p/
Other
2.33k stars 445 forks source link

Code in "Peer Discovery" section of Getting Started does not work #871

Closed lacker closed 3 years ago

lacker commented 3 years ago

Version: 0.30.2 Platform: Linux 4.15.0-132-generic #136-Ubuntu SMP Tue Jan 12 14:58:42 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux Node 10.23.1

Type:

Bug

Severity:

High

Steps to reproduce the error:

Run exactly the code provided in the "Peer Discovery" section of the Getting Started guide: https://github.com/libp2p/js-libp2p/blob/master/doc/GETTING_STARTED.md#peer-discovery Observe as it never discovers or connects to any nodes

It's possible that this code is not intended to work, that it is only similar to working code and there's something else I'd need to add to get it working. In that case I just suggest explaining in the instructions what else is needed. Or possibly there is something with my networking configuration. Or possibly there is just a current outage of some component of the network. I don't know which it is.

vasco-santos commented 3 years ago

Hey @lacker

Thanks for creating this issue. So yes, the addresses provided are mock addresses. It would probably make sense to change them to real production machines to avoid running into errors when that happens.

Would you like to submit a PR to update them according the multiaddrs used in the peer-discovery example?

lacker commented 3 years ago

The addresses look like they are two of the addresses included in the peer-discovery example, though, they don't look like mock addresses.

In the "Getting started" there is:

const bootstrapMultiaddrs = [
  '/dnsaddr/bootstrap.libp2p.io/p2p/QmbLHAnMoJPWSCR5Zhtx6BHJX9KiKNN6tpvbUcqanj75Nb',
  '/dnsaddr/bootstrap.libp2p.io/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN'
]

And in the "discovery mechanisms" example it uses:

const bootstrapers = [
  '/ip4/104.131.131.82/tcp/4001/p2p/QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ',
  '/dnsaddr/bootstrap.libp2p.io/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN',
  '/dnsaddr/bootstrap.libp2p.io/p2p/QmbLHAnMoJPWSCR5Zhtx6BHJX9KiKNN6tpvbUcqanj75Nb',
  '/dnsaddr/bootstrap.libp2p.io/p2p/QmZa1sAxajnQjVM8WjWXoMbmPd7NsWhfKsPkErzpm9wGkp',
  '/dnsaddr/bootstrap.libp2p.io/p2p/QmQCU2EcMqAqQPR2i9bChDtGNJchTbq5TbXJJ16u19uLTa',
  '/dnsaddr/bootstrap.libp2p.io/p2p/QmcZf59bWwK5XFi76CZX8cbJ4BhTzzA3gU1ZjYZcYW3dwt'
]
vasco-santos commented 3 years ago

@lacker yes you are right, I confused them. I will try locally the example

vasco-santos commented 3 years ago

I just tried it out and there is one issue with the discovery handler (node.on('peer:discovery')) as we changed it recently, but sadly we missed updating this doc.

const Libp2p = require('libp2p')
const WebSockets = require('libp2p-websockets')
const { NOISE } = require('libp2p-noise')
const MPLEX = require('libp2p-mplex')

const Bootstrap = require('libp2p-bootstrap')

// Known peers addresses
const bootstrapMultiaddrs = [
  '/dnsaddr/bootstrap.libp2p.io/p2p/QmbLHAnMoJPWSCR5Zhtx6BHJX9KiKNN6tpvbUcqanj75Nb',
  '/dnsaddr/bootstrap.libp2p.io/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN'
]

async function main () {
  const node = await Libp2p.create({
    modules: {
      transport: [WebSockets],
      connEncryption: [NOISE],
      streamMuxer: [MPLEX],
      peerDiscovery: [Bootstrap]
    },
    config: {
      peerDiscovery: {
        autoDial: true, // Auto connect to discovered peers (limited by ConnectionManager minConnections)
        // The `tag` property will be searched when creating the instance of your Peer Discovery service.
        // The associated object, will be passed to the service when it is instantiated.
        [Bootstrap.tag]: {
          enabled: true,
          list: bootstrapMultiaddrs // provide array of multiaddrs
        }
      }
    }
  })

  node.on('peer:discovery', (peerId) => {
    console.log('Discovered %s', peerId.toB58String()) // Log discovered peer
  })

  node.connectionManager.on('peer:connect', (connection) => {
    console.log('Connected to %s', connection.remotePeer.toB58String()) // Log connected peer
  })

  // start libp2p
  await node.start()
}

main()

Other than that, the code needs to be wrapped with an async function. Not sure if this was also the problem as the connected log was working for me. Let me know your thoughts

lacker commented 3 years ago

It seems part fixed. Now when I run the code you provide here, it "discovers" peers but never "connects" to any.

Discovered QmbLHAnMoJPWSCR5Zhtx6BHJX9KiKNN6tpvbUcqanj75Nb
Discovered QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN

Is that the intended behavior?

vasco-santos commented 3 years ago

Is that the intended behavior?

No it is not. This is strange, can you run https://github.com/libp2p/js-libp2p/tree/master/examples/discovery-mechanisms (example 1) ?

If that does not work, it would be useful to have some logs to understand what is going on. You can run your code from getting started with DEBUG=libp2p* node index.js

lacker commented 3 years ago

When I run that code it connects to just one of the nodes.

Connection established to: QmaCpDMGvV2BGHeYERUEnRQAwe3N8SzbUtfsmvsqQLuvuJ

When I look at the logs there is a suspicious error that occurs many times while trying to connect:

libp2p:connection-manager connecting to a peerStore stored peer QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN +35ms
  libp2p:dialer:err multiaddr /dnsaddr/bootstrap.libp2p.io/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN could not be resolved +10s
  libp2p:connection-manager:err could not connect to peerStore stored peer TypeError: recursiveMultiaddrs.flat is not a function
    at Dialer._resolve (/home/lacker/deck/node_modules/libp2p/src/dialer/index.js:245:39) +10s

And yeah, it looks like Array.flat was only added in Node 11, I'm using Node 10. I don't know if you intend to support Node 10 or not, so, your call whether it's better to avoid using flat or just say that only node 11+ is supported.

For me, I upgraded to node 14 and it is now working. So this is all a node-10-compatibility thing.

vasco-santos commented 3 years ago

@lacker so updating node completely solved this issue right?

We do not intend to support Node10 anymore as you can see in https://github.com/libp2p/js-libp2p/blob/master/package.json#L50 . We work on supporting active Node versions per https://nodejs.org/en/about/releases/

lacker commented 3 years ago

OK yeah node 14 works fine. Too bad it just silently gives weird errors on node 10, I guess node doesn't actually respect that line in the package.json? Dunno. Thanks for the help!