waku-org / js-waku

JavaScript implementation of Waku v2
https://js.waku.org
Apache License 2.0
168 stars 42 forks source link

bug: node doesn't discover new peers that are started after the initial peer discovery phase #1860

Open fbarbu15 opened 8 months ago

fbarbu15 commented 8 months ago

This is a bug report

This works fine with wakuorg/nwaku:v0.24.0 but fails with wakuorg/nwaku:v0.25.0 and harbor.status.im/wakuorg/nwaku:latest A wild guess is that this issue is a by-product of https://github.com/waku-org/nwaku/pull/2332#discussion_r1466087883

Steps

  1. Start nwaku1 with peer exchange enabled
  2. Bootstrap nwaku2 based on nwaku1
  3. Start js-waku
    waku = await createLightNode({
      libp2p: {
        peerDiscovery: [
          bootstrap({ list: [(await nwaku2.getMultiaddrWithId()).toString()] }),
          wakuPeerExchangeDiscovery(pubsubTopic)
        ]
      }
    });
    await waku.start();
  4. Wait for js-waku to discover nwaku1 via peer exchange
  5. After nwaku1 was discovered start nwaku3 similar to nwaku2
  6. Wait for js-waku to discover nwaku3 via peer exchange

Actual Results

Expected Results

Here's the full test to make this reproduce easily:

import { bootstrap } from "@libp2p/bootstrap";
import type { PeerId } from "@libp2p/interface";
import type { LightNode } from "@waku/interfaces";
import { wakuPeerExchangeDiscovery } from "@waku/peer-exchange";
import { createLightNode, Tags } from "@waku/sdk";
import { Logger, singleShardInfoToPubsubTopic } from "@waku/utils";
import Sinon from "sinon";

import {
  afterEachCustom,
  beforeEachCustom,
  delay,
  makeLogFileName,
  ServiceNode,
  tearDownNodes
} from "../../src/index.js";

export const log = new Logger("test:pe");
const pubsubTopic = [singleShardInfoToPubsubTopic({ clusterId: 0, shard: 2 })];

describe("Peer Exchange", function () {
  this.timeout(200_000);
  let waku: LightNode;
  let nwaku1: ServiceNode;
  let nwaku2: ServiceNode;
  let nwaku3: ServiceNode;

  beforeEachCustom(this, async () => {
    nwaku1 = new ServiceNode(makeLogFileName(this.ctx) + "1");
    nwaku2 = new ServiceNode(makeLogFileName(this.ctx) + "2");
    await nwaku1.start({
      pubsubTopic: pubsubTopic,
      discv5Discovery: true,
      peerExchange: true,
      relay: true
    });
    await nwaku2.start({
      pubsubTopic: pubsubTopic,
      discv5Discovery: true,
      peerExchange: true,
      discv5BootstrapNode: (await nwaku1.info()).enrUri,
      relay: true
    });
  });

  afterEachCustom(this, async () => {
    await tearDownNodes([nwaku1, nwaku2, nwaku3], waku);
  });

  it.only("new peer added after a peer was already found", async function () {
    console.log("BOOTSTRAPPING JS_WAKU BASED ON NWAKU2");
    waku = await createLightNode({
      libp2p: {
        peerDiscovery: [
          bootstrap({ list: [(await nwaku2.getMultiaddrWithId()).toString()] }),
          wakuPeerExchangeDiscovery(pubsubTopic)
        ]
      }
    });
    await waku.start();
    console.log("NWAKU2 PEERS AT THE BEGINNING:");
    console.log(await nwaku2.peers());
    console.log("JS-WAKU PEERS AT THE BEGINNING:");
    console.log(await waku.connectionManager.getPeersByDiscovery());

    Sinon.spy((waku as any).connectionManager, "dialPeer");
    const pxPeersDiscovered = new Set<PeerId>();
    await new Promise<void>((resolve) => {
      waku.libp2p.addEventListener("peer:discovery", (evt) => {
        return void (async () => {
          const peerId = evt.detail.id;
          const peer = await waku.libp2p.peerStore.get(peerId);
          const tags = Array.from(peer.tags.keys());
          if (tags.includes(Tags.PEER_EXCHANGE)) {
            pxPeersDiscovered.add(peerId);
            if (pxPeersDiscovered.size === 1) {
              resolve();
            }
          }
        })();
      });
    });

    console.log("NWAKU2 PEERS BEFORE START OF NWAKU3:");
    console.log(await nwaku2.peers());
    console.log("JS-WAKU PEERS BEFORE START OF NWAKU3:");
    console.log(
      (await waku.connectionManager.getPeersByDiscovery()).DISCOVERED[
        Tags.PEER_EXCHANGE
      ][0]
    );

    nwaku3 = new ServiceNode(makeLogFileName(this) + "3");
    await nwaku3.start({
      pubsubTopic: pubsubTopic,
      discv5Discovery: true,
      peerExchange: true,
      discv5BootstrapNode: (await nwaku1.info()).enrUri,
      relay: true,
      lightpush: true,
      filter: true
    });

    await delay(60000);

    console.log("NWAKU2 PEERS AFTER 60S OF START OF NWAKU3:");
    console.log(await nwaku2.peers());
    console.log("JS-WAKU PEERS AFTER 60S OF START OF NWAKU3:");
    console.log(
      (await waku.connectionManager.getPeersByDiscovery()).DISCOVERED[
        Tags.PEER_EXCHANGE
      ][0]
    );
    console.log(
      (await waku.connectionManager.getPeersByDiscovery()).DISCOVERED[
        Tags.PEER_EXCHANGE
      ][1]
    );
  });
});

Or link with the actual test from github that caught this . To be added

Here's comparative logs nwaku 24 vs 25: test_with_nwaku_0_25.zip test_with_nwaku_0_24.zip

weboko commented 6 months ago

Moving to TODO for now as reliability and release are more important now. Next in line to be in Priority.