chore: discv5 re-org clean-up (#1823)

- Remove discv5 from node and it's builder
- Require a record to init a node (was previously using discv5 record).
- Fix various tests that assumed nodes with discv5.
- Fix ENR creation
- Fix wakubridge
- Fix networkmonitor

apps/networkmonitor/networkmonitor.nim

@@ -160,6 +160,7 @@ proc populateInfoFromIp(allPeersRef: CustomPeersTableRef,
 # crawls the network discovering peers and trying to connect to them
 # metrics are processed and exposed
 proc crawlNetwork(node: WakuNode,
+                  wakuDiscv5: WakuDiscoveryV5,
                   restClient: RestClientRef,
                   conf: NetworkMonitorConf,
                   allPeersRef: CustomPeersTableRef) {.async.} =
@@ -167,10 +168,10 @@ proc crawlNetwork(node: WakuNode,
   let crawlInterval = conf.refreshInterval * 1000
   while true:
     # discover new random nodes
-    let discoveredNodes = await node.wakuDiscv5.protocol.queryRandom()
+    let discoveredNodes = await wakuDiscv5.protocol.queryRandom()
 
     # nodes are nested into bucket, flat it
-    let flatNodes = node.wakuDiscv5.protocol.routingTable.buckets.mapIt(it.nodes).flatten()
+    let flatNodes = wakuDiscv5.protocol.routingTable.buckets.mapIt(it.nodes).flatten()
 
     # populate metrics related to capabilities as advertised by the ENR (see waku field)
     setDiscoveredPeersCapabilities(flatNodes)
@@ -242,27 +243,56 @@ proc getBootstrapFromDiscDns(conf: NetworkMonitorConf): Result[seq[enr.Record],
   except CatchableError:
     error("failed discovering peers from DNS")
 
-proc initAndStartNode(conf: NetworkMonitorConf): Result[WakuNode, string] =
+proc initAndStartApp(conf: NetworkMonitorConf): Result[(WakuNode, WakuDiscoveryV5), string] =
+  let bindIp = try:
+    ValidIpAddress.init("0.0.0.0")
+  except CatchableError:
+    return err("could not start node: " & getCurrentExceptionMsg())
+
+  let extIp = try:
+    ValidIpAddress.init("127.0.0.1")
+  except CatchableError:
+    return err("could not start node: " & getCurrentExceptionMsg())
+
   let
     # some hardcoded parameters
     rng = keys.newRng()
-    nodeKey = crypto.PrivateKey.random(Secp256k1, rng[])[]
+    key = crypto.PrivateKey.random(Secp256k1, rng[])[]
     nodeTcpPort = Port(60000)
     nodeUdpPort = Port(9000)
     flags = CapabilitiesBitfield.init(lightpush = false, filter = false, store = false, relay = true)
 
-  try:
-    let
-      bindIp = ValidIpAddress.init("0.0.0.0")
-      extIp = ValidIpAddress.init("127.0.0.1")
+  var builder = EnrBuilder.init(key)
 
-    var builder = WakuNodeBuilder.init()
-    builder.withNodeKey(nodeKey)
-    ? builder.withNetworkConfigurationDetails(bindIp, nodeTcpPort)
-    let node = ? builder.build()
+  builder.withIpAddressAndPorts(
+      ipAddr = some(extIp),
+      tcpPort = some(nodeTcpPort),
+      udpPort = some(nodeUdpPort),
+  )
+  builder.withWakuCapabilities(flags)
+
+  let recordRes = builder.build()
+  let record =
+    if recordRes.isErr():
+      return err("cannot build record: " & $recordRes.error)
+    else: recordRes.get()
+
+  var nodeBuilder = WakuNodeBuilder.init()
+
+  nodeBuilder.withNodeKey(key)
+  nodeBuilder.withRecord(record)
+  let res = nodeBuilder.withNetworkConfigurationDetails(bindIp, nodeTcpPort)
+  if res.isErr():
+    return err("node building error" & $res.error)
+
+  let nodeRes = nodeBuilder.build()
+  let node =
+    if nodeRes.isErr():
+      return err("node building error" & $res.error)
+    else: nodeRes.get()
 
   var discv5BootstrapEnrsRes = getBootstrapFromDiscDns(conf)
-    if not discv5BootstrapEnrsRes.isOk():
+  if discv5BootstrapEnrsRes.isErr():
     error("failed discovering peers from DNS")
   var discv5BootstrapEnrs = discv5BootstrapEnrsRes.get()
 
@@ -270,16 +300,24 @@ proc initAndStartNode(conf: NetworkMonitorConf): Result[WakuNode, string] =
   for enrUri in conf.bootstrapNodes:
     addBootstrapNode(enrUri, discv5BootstrapEnrs)
 
-    # mount discv5
-    node.wakuDiscv5 = WakuDiscoveryV5.new(
-        some(extIp), some(nodeTcpPort), some(nodeUdpPort),
-        bindIp, nodeUdpPort, discv5BootstrapEnrs, false,
-        keys.PrivateKey(nodeKey.skkey), flags, @[], node.rng, @[])
+  # discv5
+  let discv5Conf = WakuDiscoveryV5Config(
+    discv5Config: none(DiscoveryConfig),
+    address: bindIp,
+    port: nodeUdpPort,
+    privateKey: keys.PrivateKey(key.skkey),
+    bootstrapRecords: discv5BootstrapEnrs,
+    autoupdateRecord: false
+  )
 
-    node.wakuDiscv5.protocol.open()
-    return ok(node)
+  let wakuDiscv5 = WakuDiscoveryV5.new(node.rng, discv5Conf, some(record))
+
+  try:
+    wakuDiscv5.protocol.open()
   except CatchableError:
-    error("could not start node")
+    return err("could not start node: " & getCurrentExceptionMsg())
+
+  ok((node, wakuDiscv5))
 
 proc startRestApiServer(conf: NetworkMonitorConf,
                         allPeersInfo: CustomPeersTableRef,
@@ -366,12 +404,12 @@ when isMainModule:
   let restClient = clientRest.get()
 
   # start waku node
-  let nodeRes = initAndStartNode(conf)
+  let nodeRes = initAndStartApp(conf)
   if nodeRes.isErr():
     error "could not start node"
     quit 1
 
-  let node = nodeRes.get()
+  let (node, discv5) = nodeRes.get()
 
   waitFor node.mountRelay()
 
@@ -380,6 +418,6 @@ when isMainModule:
 
   # spawn the routine that crawls the network
   # TODO: split into 3 routines (discovery, connections, ip2location)
-  asyncSpawn crawlNetwork(node, restClient, conf, allPeersInfo)
+  asyncSpawn crawlNetwork(node, discv5, restClient, conf, allPeersInfo)
 
   runForever()

apps/wakubridge/wakubridge.nim

@@ -24,6 +24,7 @@ import
   # Waku v2 imports
   libp2p/crypto/crypto,
   libp2p/nameresolving/nameresolver,
+  ../../waku/v2/waku_enr,
   ../../waku/v2/waku_core,
   ../../waku/v2/waku_store,
   ../../waku/v2/waku_filter,
@@ -228,10 +229,15 @@ proc new*(T: type WakuBridge,
                               topics: topicInterest)
   nodev1.configureWaku(wakuConfig)
 
+  var builder = EnrBuilder.init(nodev2Key)
+  builder.withIpAddressAndPorts(nodev2ExtIp, nodev2ExtPort, none(Port))
+  let record = builder.build().tryGet()
+
   # Setup Waku v2 node
   let nodev2 = block:
       var builder = WakuNodeBuilder.init()
       builder.withNodeKey(nodev2Key)
+      builder.withRecord(record)
       builder.withNetworkConfigurationDetails(nodev2BindIp, nodev2BindPort, nodev2ExtIp, nodev2ExtPort).tryGet()
       builder.withSwitchConfiguration(nameResolver=nameResolver)
       builder.build().tryGet()

apps/wakunode2/app.nim

@@ -220,7 +220,7 @@ proc setupDiscoveryV5*(app: App): WakuDiscoveryV5 =
 
   let discv5Conf = WakuDiscoveryV5Config(
     discv5Config: some(discv5Config),
-    address: app.netConf.bindIp,
+    address: app.conf.listenAddress,
     port: discv5UdpPort,
     privateKey: keys.PrivateKey(app.key.skkey),
     bootstrapRecords: discv5BootstrapEnrs,
@@ -504,7 +504,7 @@ proc startApp*(app: App): Future[AppResult[void]] {.async.} =
     let res = app.wakuDiscv5.get().start()
 
     if res.isErr():
-      return err("failed to start waku discovery v5: " & res.error)
+      return err("failed to start waku discovery v5: " & $res.error)
 
     asyncSpawn app.wakuDiscv5.get().searchLoop(app.node.peerManager, some(app.record))
 

apps/wakunode2/internal_config.nim

@@ -83,8 +83,8 @@ proc createRecord*(conf: WakuNodeConf, netConf: NetConfig, key: crypto.PrivateKe
   var builder = EnrBuilder.init(key)
 
   builder.withIpAddressAndPorts(
-      ipAddr = netConf.extIp,
-      tcpPort = netConf.extPort,
+      ipAddr = netConf.enrIp,
+      tcpPort = netConf.enrPort,
       udpPort = netConf.discv5UdpPort,
   )
 

tests/v2/testlib/wakunode.nim

@@ -51,12 +51,34 @@ proc newTestWakuNode*(nodeKey: crypto.PrivateKey,
     dns4DomainName = dns4DomainName,
     discv5UdpPort = discv5UdpPort,
   )
+  let netConf =
     if netConfigRes.isErr():
       raise newException(Defect, "Invalid network configuration: " & $netConfigRes.error)
+    else:
+      netConfigRes.get()
+
+  var enrBuilder = EnrBuilder.init(nodeKey)
+
+  enrBuilder.withIpAddressAndPorts(
+      ipAddr = netConf.enrIp,
+      tcpPort = netConf.enrPort,
+      udpPort = netConf.discv5UdpPort,
+  )
+  if netConf.wakuFlags.isSome():
+    enrBuilder.withWakuCapabilities(netConf.wakuFlags.get())
+  enrBuilder.withMultiaddrs(netConf.enrMultiaddrs)
+
+  let recordRes = enrBuilder.build()
+  let record =
+    if recordRes.isErr():
+      raise newException(Defect, "Invalid record: " & $recordRes.error)
+    else:
+      recordRes.get()
 
   var builder = WakuNodeBuilder.init()
   builder.withRng(rng())
   builder.withNodeKey(nodeKey)
+  builder.withRecord(record)
   builder.withNetworkConfiguration(netConfigRes.get())
   builder.withPeerStorage(peerStorage, capacity = peerStoreCapacity)
   builder.withSwitchConfiguration(

tests/wakubridge/test_wakubridge.nim

@@ -19,6 +19,7 @@ import
   ../../waku/v1/protocol/waku_protocol,
   ../../waku/v2/waku_core,
   ../../waku/v2/waku_node,
+  ../../waku/v2/waku_enr,
   ../../waku/v2/utils/compat,
   ../test_helpers
 
@@ -54,9 +55,16 @@ procSuite "WakuBridge":
 
     # Waku v2 node
     v2NodeKey = crypto.PrivateKey.random(Secp256k1, cryptoRng[])[]
+
+  var builder = EnrBuilder.init(v2NodeKey)
+  builder.withIpAddressAndPorts(none(ValidIpAddress), none(Port), none(Port))
+  let record = builder.build().tryGet()
+
+  let
     v2Node = block:
       var builder = WakuNodeBuilder.init()
       builder.withNodeKey(v2NodeKey)
+      builder.withRecord(record)
       builder.withNetworkConfigurationDetails(ValidIpAddress.init("0.0.0.0"), Port(62203)).tryGet()
       builder.build().tryGet()
 

waku/v2/node/builder.nim

@@ -77,8 +77,7 @@ proc withNetworkConfigurationDetails*(builder: var WakuNodeBuilder,
           wsEnabled: bool = false,
           wssEnabled: bool = false,
           wakuFlags = none(CapabilitiesBitfield),
-          dns4DomainName = none(string),
-          discv5UdpPort = none(Port)): WakuNodeBuilderResult {.
+          dns4DomainName = none(string)): WakuNodeBuilderResult {.
   deprecated: "use 'builder.withNetworkConfiguration()' instead".} =
   let netConfig = ? NetConfig.init(
     bindIp = bindIp,
@@ -91,7 +90,6 @@ proc withNetworkConfigurationDetails*(builder: var WakuNodeBuilder,
     wssEnabled = wssEnabled,
     wakuFlags = wakuFlags,
     dns4DomainName = dns4DomainName,
-    discv5UdpPort = discv5UdpPort,
   )
   builder.withNetworkConfiguration(netConfig)
   ok()
@@ -144,6 +142,9 @@ proc build*(builder: WakuNodeBuilder): Result[WakuNode, string] =
   if builder.netConfig.isNone():
     return err("network configuration is required")
 
+  if builder.record.isNone():
+    return err("node record is required")
+
   # fallbck to max connections if not set
   var maxRelayPeers: int
   if builder.maxRelayPeers.isNone():
@@ -181,9 +182,8 @@ proc build*(builder: WakuNodeBuilder): Result[WakuNode, string] =
   var node: WakuNode
   try:
     node = WakuNode.new(
-      nodeKey = builder.nodeKey.get(),
       netConfig = builder.netConfig.get(),
-      enr = builder.record,
+      enr = builder.record.get(),
       switch = switch,
       peerManager = peerManager,
       rng = rng,

waku/v2/node/waku_node.nim

@@ -37,7 +37,6 @@ import
   ../waku_lightpush/client as lightpush_client,
   ../waku_enr,
   ../waku_dnsdisc,
-  ../waku_discv5,
   ../waku_peer_exchange,
   ./config,
   ./peer_manager,
@@ -101,37 +100,10 @@ type
     enr*: enr.Record
     libp2pPing*: Ping
     rng*: ref rand.HmacDrbgContext
-    wakuDiscv5*: WakuDiscoveryV5
     rendezvous*: RendezVous
     announcedAddresses* : seq[MultiAddress]
     started*: bool # Indicates that node has started listening
 
-proc getEnr*(netConfig: NetConfig,
-             wakuDiscV5 = none(WakuDiscoveryV5),
-             nodeKey: crypto.PrivateKey): Result[enr.Record, string] =
-  if wakuDiscV5.isSome():
-    return ok(wakuDiscV5.get().protocol.getRecord())
-
-  var builder = EnrBuilder.init(nodeKey, seqNum = 1)
-
-  builder.withIpAddressAndPorts(
-    ipAddr = netConfig.enrIp,
-    tcpPort = netConfig.enrPort,
-    udpPort = netConfig.discv5UdpPort
-  )
-
-  if netConfig.wakuFlags.isSome():
-    builder.withWakuCapabilities(netConfig.wakuFlags.get())
-
-  if netConfig.enrMultiAddrs.len > 0:
-    builder.withMultiaddrs(netConfig.enrMultiAddrs)
-
-  let recordRes = builder.build()
-  if recordRes.isErr():
-    return err($recordRes.error)
-
-  return ok(recordRes.get())
-
 proc getAutonatService*(rng: ref HmacDrbgContext): AutonatService =
   ## AutonatService request other peers to dial us back
   ## flagging us as Reachable or NotReachable.
@@ -156,11 +128,9 @@ proc getAutonatService*(rng: ref HmacDrbgContext): AutonatService =
   return autonatService
 
 proc new*(T: type WakuNode,
-          nodeKey: crypto.PrivateKey,
           netConfig: NetConfig,
-          enr: Option[enr.Record],
+          enr: enr.Record,
           switch: Switch,
-          wakuDiscv5 = none(WakuDiscoveryV5),
           peerManager: PeerManager,
           # TODO: make this argument required after tests are updated
           rng: ref HmacDrbgContext = crypto.newRng()
@@ -169,23 +139,12 @@ proc new*(T: type WakuNode,
 
   info "Initializing networking", addrs= $netConfig.announcedAddresses
 
-  let enr =
-    if enr.isNone():
-      let nodeEnrRes = getEnr(netConfig, wakuDiscv5, nodekey)
-
-      if nodeEnrRes.isErr():
-        raise newException(Defect, "failed to generate the node ENR record: " & $nodeEnrRes.error)
-
-      nodeEnrRes.get()
-    else: enr.get()
-
   return WakuNode(
     peerManager: peerManager,
     switch: switch,
     rng: rng,
     enr: enr,
     announcedAddresses: netConfig.announcedAddresses,
-    wakuDiscv5: if wakuDiscV5.isSome(): wakuDiscV5.get() else: nil,
   )
 
 proc peerInfo*(node: WakuNode): PeerInfo =