refactor: start moving discovery modules to waku/discovery

[Ivansete-status]

Description

We need to allow nwaku features to be accessible from both wakunode2 and libwaku. In this PR, we start moving discovery logic from wakunode2 app to a more generic location, waku/node/discovery_manager.

In future PRs, is likely that we will perform the following movements:

1. wakuDiscv5 and dynamicBootstrapNodes, see this, will get controlled only by DiscoveryManager.
2. The WakuNode object will contain a reference to discoveryManager, similarly to how it now contains a reference to peerManager. See:

   nwaku/waku/node/waku_node.nim

   Lines 84 to 85 in 4117fe6

   	WakuNode* = ref object
   	peerManager*: PeerManager

Notice that the discoveryManager module is aimed to contain the logic to handle the discovery of other peers plus the logic to make the "self" node discoverable by others.

Issue

migrate DiscV5 and DNS Discovery from app.nim to waku_node.nim: #2452

[github-actions]

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2587-rln-v2-false

Built from 8163a49

[gabrielmer]

I wonder if discovery should enter at the node layer, or if it belongs to the "middleware" layer we talked about when creating the factory.

Definitely love the idea of having a discovery manager :) Having the discovery logic in its own module is a great step 🤩

[NagyZoltanPeter]

Yes, I think it is a good step for restructure.
The only thing if sthg was changed in formatting as seemingly there are changes related only for that purpose without any meaningful change?
But it's ok, manageable.

[Ivansete-status]

I wonder if discovery should enter at the node layer, or if it belongs to the "middleware" layer we talked about when creating the factory.

Definitely love the idea of having a discovery manager :) Having the discovery logic in its own module is a great step 🤩

Very good point!
I think the DiscoveryManager component can be at the same level as PeerManager because both imply some management and interaction with other peers. Another option, to fulfill with your suggestion, would be to move the "discovery" logic one folder up, and then have waku/discovery_manager/ instead of waku/node/discovery_manager.

How do you feel about having the following? Does it sound correct?

 WakuNode* = ref object 
   peerManager*: PeerManager
   discManager*: DiscoveryManager   <------ 

[gabrielmer]

Very good point! I think the DiscoveryManager component can be at the same level as PeerManager because both imply some management and interaction with other peers. Another option, to fulfill with your suggestion, would be to move the "discovery" logic one folder up, and then have waku/discovery_manager/ instead of waku/node/discovery_manager.

How do you feel about having the following? Does it sound correct?

 WakuNode* = ref object 
   peerManager*: PeerManager
   discManager*: DiscoveryManager   <------ 

It's true that both the PeerManager and the DiscoveryManager imply some management/interaction with other peers, but what I'm not sure about is if they are at the same layer of abstraction.

There's a chance that the DiscoveryManager is actually some kind of upper level logic intended for the PeerManager to get populated, which in that case they shouldn't be together. But I do lack some background, so not sure if what I'm saying makes sense at all haha, but I think it's important to have that clear before adding it to the waku_node module.

@jm-clius what's your take on it?

[jm-clius]

While having a Discovery Manager makes sense from a code org POV, I wouldn't compose it into the node object. In fact, we've specifically moved discovery elements out of the node as we realised more and more that we needed the separate layer of abstraction (side note: we've also seriously considered moving peer management outside of the node object, even though it is indeed at a lower abstraction than the discovery manager). In general:

• a node is an object wrapping a libp2p switch, mounts libp2p protocols and manages the APIs to these protocols
• a peer manager is an object that manages a store of possible peers for the node and ensures that the node has the kind of connectivity it needs for every type of protocol.
• a discovery protocol may be any protocol/external provider of peers to the peer manager. These protocols may be discv5, DHTs, rendezvous, supplied lists, DNS lookups, etc. that have very little to do with libp2p transports and communications (in other words, it operates completely separately from the node and simply supplies the outcome of discovered peers to the peer manager).

The composition of the above is what makes a Waku application. Afaics the role of the node factory is exactly to assist with the composition of all these elements, so this is where the DiscoverManager would belong. IMO it would be a mistake to blur the lines of separation here.

[gabrielmer]

Looks amazing! Thanks so much! 😍

[jm-clius]

Thanks for this! The code organisation makes sense - also to include discovery functions in the factory from where it can be encapsulated by the bindings. Not sure yet I see the advantage why somewhat disparate objects would be composed into a single object, DiscoveryManager, though. Approving, as I assume it will make more sense in a future PR. :D