libp2p / js-libp2p

The JavaScript Implementation of libp2p networking stack.
https://libp2p.github.io/js-libp2p/
Apache License 2.0
2.34k stars 446 forks source link

Dialling a peer and subscribing to a pubsub topic but "subscription-change" event does not always fire #2771

Open haydenyoung opened 1 month ago

haydenyoung commented 1 month ago

Severity:

Medium

Description:

OrbitDB has two peers connecting to one another and sharing messages using Libp2p and custom protocols. To initiate the message exchange, OrbitDB relies on the event subscription-change by attaching a listener to it. When fired, the listener will dial a protocol from peerA from peerB and initiate an exchange of data via streams.

When bootstrapping peerA's address in peerB, subscription-change fires reliably. However, when a discovery mechanism is used (we have been using mdns for local network discovery) and the peerB dials peerA manually, subscription-change becomes unreliable, sometimes firing, sometimes not.

Watching the pipePeerReadStream function in GossipSub, it appears that the stream doesn't contain anything and the data handling is completely skipped, giving the appearance that the event doesn't fire when, in fact, it does but it skips over much of the function because there is no data to read.

Steps to reproduce the error:

Prerequisites:

git clone https://github.com/orbitdb/orbit-db-pinner.git
cd orbit-db-pinner
git checkout pinner/new-refactor-2-helia-upgrade
npm i

To see a successful test run:

In terminal 1 start the relay:

npm run start:relay

In terminal 2 start the first pinner server:

npm run start:orbiter1

In terminal 3 start the second pinner server:

npm run start:orbiter2

In terminal 4, run the unit tests:

npm run test

or

npm run test:ci

All tests should run successfully.

To see subscription-change fail:

Kill Orbiter1 and Orbiter2

In src/lib/libp2p-config.js, uncomment this line and this line to enable local network discovery. Comment out the node bootstrapping. Config should now look like:

    peerDiscovery: [
      /*
      bootstrap({
        list: ['/ip4/127.0.0.1/tcp/54321/p2p/16Uiu2HAmBzKcgCfpJ4j4wJSLkKLbCVvnNBWPnhexrnJWJf1fDu5y']
      }),
      */
      mdns()
    ],

In src/daemon.js, add the following after line 55:

  libp2p.addEventListener('peer:discovery', async (event) => {
    const { id: peerId, multiaddrs } = event.detail

    if (peerId !== libp2p.peerId) {
      console.log("dialling", peerId.toString())
      await libp2p.peerStore.save(peerId, { multiaddrs })
      await libp2p.dial(peerId)
      console.log("dialled", peerId.toString())
    }
  })

In terminal 1 start the relay:

npm run start:relay

In terminal 2 start the first pinner server:

npm run start:orbiter1

In terminal 3 start the second pinner server:

npm run start:orbiter2

In terminal 4, run the unit tests:

npm run test

or

npm run test:ci

The dialling will ensure all relevant peers are connected in a similar fashion to when bootstrapping on initialization; Orbiter1, Orbiter2, Lander1, Lander2. However, subscription-change will fail which will result in timeout errors such as:

Error: Timeout of 30000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (~/orbitdb/voyager/test/e2e-browser.test.js)

The assumption would be that whether bootstrapped or dialled manually, the subscription-change event would fire regardless.