Removed duplicates of announcedAddresses, extMultiaddresses (#3831)

Removing duplicates of multiaddresses for Enr.
Safe building Enr Record.

Co-authored-by: Copilot <copilot@github.com>

tests/factory/test_node_factory.nim

@@ -1,11 +1,19 @@
 {.used.}
 
-import testutils/unittests, chronos, libp2p/protocols/connectivity/relay/relay
+import
+  std/[options, sequtils],
+  testutils/unittests,
+  chronos,
+  libp2p/multiaddress,
+  libp2p/protocols/connectivity/relay/relay
+import eth/p2p/discoveryv5/enr
 
 import
   ../testlib/wakunode,
   waku/waku_node,
+  waku/waku_enr,
   waku/factory/node_factory,
+  waku/factory/internal_config,
   waku/factory/conf_builder/conf_builder,
   waku/factory/conf_builder/web_socket_conf_builder
 
@@ -38,6 +46,45 @@ suite "Node Factory":
       not node.wakuStore.isNil()
       not node.wakuArchive.isNil()
 
+  test "ENR configuration trims multiaddrs until record fits":
+    var conf = defaultTestWakuConf()
+    let bindIp = conf.endpointConf.p2pListenAddress
+    let bindPort = Port(30303)
+
+    let oversizedMultiaddrs = (0 .. 11).mapIt(
+      MultiAddress
+        .init(
+          "/dns4/very-long-logical-hostname-" & $it &
+            ".example.logos.dev.status.im/tcp/30303/wss"
+        )
+        .get()
+    )
+
+    let netConfig = NetConfig.init(
+      clusterId = conf.clusterId,
+      bindIp = bindIp,
+      bindPort = bindPort,
+      extMultiAddrs = oversizedMultiaddrs,
+      extMultiAddrsOnly = true,
+      wakuFlags = some(conf.wakuFlags),
+    ).valueOr:
+      raiseAssert error
+
+    let record = enrConfiguration(conf, netConfig).valueOr:
+      raiseAssert error
+
+    let typedRecord = record.toTyped()
+    require typedRecord.isOk()
+
+    let multiaddrsOpt = typedRecord.value.multiaddrs
+    require multiaddrsOpt.isSome()
+
+    let retainedMultiaddrs = multiaddrsOpt.get()
+    check:
+      retainedMultiaddrs.len < oversizedMultiaddrs.len
+      retainedMultiaddrs.len > 0
+      retainedMultiaddrs == oversizedMultiaddrs[0 ..< retainedMultiaddrs.len]
+
 asynctest "Set up a node with Filter enabled":
   var confBuilder = defaultTestWakuConfBuilder()
   confBuilder.filterServiceConf.withEnabled(true)

tests/test_waku_enr.nim

@@ -271,6 +271,44 @@ suite "Waku ENR - Multiaddresses":
       multiaddrs.contains(expectedAddr1)
       multiaddrs.contains(addr2)
 
+  test "encode and decode record with multiaddrs field deduplicates duplicate entries":
+    ## Given
+    let
+      enrSeqNum = 1u64
+      enrPrivKey = generatesecp256k1key()
+
+    let
+      addr1 = MultiAddress
+        .init(
+          "/ip4/127.0.0.1/tcp/80/ws/p2p/16Uiu2HAm4v86W3bmT1BiH6oSPzcsSr31iDQpSN5Qa882BCjjwgrD"
+        )
+        .get()
+      addr1NoPeerId = MultiAddress.init("/ip4/127.0.0.1/tcp/80/ws").get()
+      addr2 = MultiAddress.init("/ip4/127.0.0.1/tcp/443/wss").get()
+
+    ## When
+    var builder = EnrBuilder.init(enrPrivKey, seqNum = enrSeqNum)
+    builder.withMultiaddrs(@[addr1, addr1NoPeerId, addr2, addr2])
+
+    let recordRes = builder.build()
+
+    require recordRes.isOk()
+    let record = recordRes.tryGet()
+
+    let typedRecord = record.toTyped()
+    require typedRecord.isOk()
+
+    let multiaddrsOpt = typedRecord.value.multiaddrs
+
+    ## Then
+    check multiaddrsOpt.isSome()
+
+    let multiaddrs = multiaddrsOpt.get()
+    check:
+      multiaddrs.len == 2
+      multiaddrs.contains(addr1NoPeerId)
+      multiaddrs.contains(addr2)
+
 suite "Waku ENR - Relay static sharding":
   test "new relay shards object with single invalid shard id":
     ## Given

tests/test_waku_netconfig.nim

@@ -152,6 +152,31 @@ suite "Waku NetConfig":
       netConfig.announcedAddresses.len == 1 # DNS address
       netConfig.announcedAddresses[0] == dns4TcpEndPoint(dns4DomainName, extPort)
 
+  asyncTest "AnnouncedAddresses and enrMultiaddrs deduplicate dns4DomainName and extMultiAddrs overlap":
+    let
+      conf = defaultTestWakuConf()
+      dns4DomainName = "example.com"
+      extPort = Port(1234)
+      dns4Address = dns4TcpEndPoint(dns4DomainName, extPort)
+
+    let netConfigRes = NetConfig.init(
+      bindIp = conf.endpointConf.p2pListenAddress,
+      bindPort = conf.endpointConf.p2pTcpPort,
+      dns4DomainName = some(dns4DomainName),
+      extPort = some(extPort),
+      extMultiAddrs = @[dns4Address],
+    )
+
+    assert netConfigRes.isOk(), $netConfigRes.error
+
+    let netConfig = netConfigRes.get()
+
+    check:
+      netConfig.announcedAddresses.len == 1
+      netConfig.announcedAddresses[0] == dns4Address
+      netConfig.enrMultiAddrs.len == 1
+      netConfig.enrMultiAddrs[0] == dns4Address
+
   asyncTest "AnnouncedAddresses includes WebSocket addresses when enabled":
     var confBuilder = defaultTestWakuConfBuilder()
 

waku/factory/internal_config.nim

@@ -10,8 +10,8 @@ import
 
 import ../common/utils/nat, ../node/net_config, ../waku_enr, ../waku_core, ./waku_conf
 
-proc enrConfiguration*(
-    conf: WakuConf, netConfig: NetConfig
+proc tryBuildEnrRecord(
+    conf: WakuConf, netConfig: NetConfig, multiaddrs: seq[MultiAddress]
 ): Result[enr.Record, string] =
   var enrBuilder = EnrBuilder.init(conf.nodeKey)
 
@@ -22,7 +22,8 @@ proc enrConfiguration*(
   if netConfig.wakuFlags.isSome():
     enrBuilder.withWakuCapabilities(netConfig.wakuFlags.get())
 
-  enrBuilder.withMultiaddrs(netConfig.enrMultiaddrs)
+  if multiaddrs.len > 0:
+    enrBuilder.withMultiaddrs(multiaddrs)
 
   enrBuilder.withWakuRelaySharding(
     RelayShards(clusterId: conf.clusterId, shardIds: conf.subscribeShards)
@@ -30,11 +31,35 @@ proc enrConfiguration*(
     return err("could not initialize ENR with shards")
 
   let record = enrBuilder.build().valueOr:
-    error "failed to create enr record", error = error
     return err($error)
 
   return ok(record)
 
+proc enrConfiguration*(
+    conf: WakuConf, netConfig: NetConfig
+): Result[enr.Record, string] =
+  for retained in countdown(netConfig.enrMultiaddrs.len, 0):
+    let multiaddrs = netConfig.enrMultiaddrs[0 ..< retained]
+    let record = tryBuildEnrRecord(conf, netConfig, multiaddrs).valueOr:
+      if retained > 0:
+        warn "failed to create enr record, retrying with fewer multiaddrs",
+          error = error,
+          totalMultiaddrs = netConfig.enrMultiaddrs.len,
+          retainedMultiaddrs = retained - 1,
+          removedMultiaddr = multiaddrs[^1]
+        continue
+
+      error "failed to create enr record", error = error
+      return err($error)
+
+    if retained < netConfig.enrMultiaddrs.len:
+      warn "created enr record after trimming multiaddrs",
+        totalMultiaddrs = netConfig.enrMultiaddrs.len, retainedMultiaddrs = retained
+
+    return ok(record)
+
+  return err("failed to create enr record")
+
 proc dnsResolve*(
     domain: string, dnsAddrsNameServers: seq[IpAddress]
 ): Future[Result[string, string]] {.async.} =

waku/node/net_config.nim

@@ -156,12 +156,16 @@ proc init*(
   if extMultiAddrs.len > 0:
     announcedAddresses.add(extMultiAddrs)
 
+  announcedAddresses = announcedAddresses.deduplicate()
+
   let
     # enrMultiaddrs are just addresses which cannot be represented in ENR, as described in
     # https://rfc.vac.dev/spec/31/#many-connection-types
-    enrMultiaddrs = announcedAddresses.filterIt(
-      it.hasProtocol("dns4") or it.hasProtocol("dns6") or it.hasProtocol("ws") or
-        it.hasProtocol("wss")
+    enrMultiaddrs = deduplicate(
+      announcedAddresses.filterIt(
+        it.hasProtocol("dns4") or it.hasProtocol("dns6") or it.hasProtocol("ws") or
+          it.hasProtocol("wss")
+      )
     )
 
   ok(

waku/waku_enr/multiaddr.nim

@@ -74,7 +74,7 @@ func stripPeerId(multiaddr: MultiAddress): MultiAddress =
   return cleanAddr
 
 func withMultiaddrs*(builder: var EnrBuilder, multiaddrs: seq[MultiAddress]) =
-  let multiaddrs = multiaddrs.map(stripPeerId)
+  let multiaddrs = deduplicate(multiaddrs.map(stripPeerId))
   let value = encodeMultiaddrs(multiaddrs)
   builder.addFieldPair(MultiaddrEnrField, value)