Add address policy filter

storage/conf.nim

@@ -53,7 +53,7 @@ export
   DefaultQuotaBytes, DefaultBlockTtl, DefaultBlockInterval, DefaultNumBlocksPerInterval,
   DefaultBlockRetries
 
-const DefaultNatScheduleInterval* = 5.minutes
+const DefaultNatScheduleInterval* = 2.minutes
 
 type ThreadCount* = distinct Natural
 

storage/storage.nim

@@ -313,6 +313,11 @@ proc new*(
     switchBuilder = switchBuilder.withObservedAddrManager(
       ObservedAddrManager.new(minCount = observedAddrMinCount)
     )
+    # libp2p keeps the private address in peerInfo.addrs.
+    # Since Autonat V2 uses the observed public address,
+    # we can filter the private addresses to keep only the dialable
+    # addresses.
+    switchBuilder = switchBuilder.withAddressPolicy(dialableAddressPolicy)
 
   var natRouter: Option[NatRouter]
   let switch =
@@ -469,9 +474,14 @@ proc new*(
       maxNumRelays = config.natMaxRelays,
       client = relayClient,
       onReservation = proc(addresses: seq[MultiAddress]) {.gcsafe, raises: [].} =
-        info "Relay reservation updated", addresses
+        # A relay server is required to have a public extip, so its
+        # circuit addresses always include a public one. The relay's reservation
+        # response can also carry loopback/private addresses:
+        # they are never dialable by a remote peer, so drop them.
+        let publicAddrs = addresses.filterIt(it.hasPublicRelayTransport())
+        info "Relay reservation updated", addresses = publicAddrs
         # relay addresses are for download traffic only, not DHT routing
-        discovery.announceRelayAddrs(addresses),
+        discovery.announceRelayAddrs(publicAddrs),
       rng = random.Rng.instance(),
     )
 

storage/utils/addrutils.nim

@@ -14,6 +14,7 @@ import std/strutils
 import std/options
 
 import pkg/libp2p
+import pkg/libp2p/wire
 import pkg/stew/endians2
 
 func remapAddr*(
@@ -58,9 +59,29 @@ func getTcpPort*(ma: MultiAddress): Option[Port] =
     return Port.none
   some(Port(fromBytesBE(uint16, portBytes.get())))
 
+proc hasPublicRelayTransport*(ma: MultiAddress): bool =
+  ## True when ``ma`` is a circuit address whose relay is publicly dialable.
+  ## A circuit address is <relay wire addr>/p2p/<relayId>/p2p-circuit; the part
+  ## before /p2p/ is the relay's wire address, which isPublicMA can check.
+  ## Unlike libp2p's publicRoutableAddressPolicy we drop non-public relays: our
+  ## relay path only runs on a genuine NAT, where the relay is always public.
+  let relayWireStr = ($ma).split("/p2p/")[0]
+  let relayWireAddr = MultiAddress.init(relayWireStr).valueOr:
+    return false
+  relayWireAddr.isPublicMA()
+
+proc dialableAddressPolicy*(ma: MultiAddress): bool {.gcsafe, raises: [].} =
+  # Use with switchBuilder.withAddressPolicy.
+  # Filter the peerInfo.addrs updated by libp2p without
+  # declaring another address mapper.
+  if ma.isCircuitRelayMA():
+    ma.hasPublicRelayTransport()
+  else:
+    ma.isPublicMA()
+
 proc getMultiAddrWithIpAndTcpPort*(ip: IpAddress, port: Port): MultiAddress =
   ## Creates a MultiAddress with the specified IP address and TCP port
-  ## 
+  ##
   ## Parameters:
   ##   - ip: A valid IP address (IPv4 or IPv6)
   ##   - port: The TCP port number

tests/storage/testaddrutils.nim

@@ -35,3 +35,43 @@ suite "addrutils - remapAddr":
     let ma = MultiAddress.init("/ip4/1.2.3.4/tcp/5000").expect("valid")
     let remapped = ma.remapAddr(ip = some(parseIpAddress("8.8.8.8")))
     check remapped == MultiAddress.init("/ip4/8.8.8.8/tcp/5000").expect("valid")
+
+suite "addrutils - hasPublicRelayTransport":
+  const relayId = "16Uiu2HAkyRvHo1AyyQY1xiHC8QbYjXCHkZbneVC8dBtJjp1SZcGD"
+
+  proc circuitAddr(relayIp: string): MultiAddress =
+    MultiAddress.init("/ip4/" & relayIp & "/tcp/8070/p2p/" & relayId & "/p2p-circuit").expect(
+      "valid"
+    )
+
+  test "true when the relay has a public ip":
+    check circuitAddr("204.168.234.45").hasPublicRelayTransport()
+
+  test "false when the relay is loopback":
+    check not circuitAddr("127.0.0.1").hasPublicRelayTransport()
+
+  test "false when the relay is a private ip":
+    check not circuitAddr("172.17.0.1").hasPublicRelayTransport()
+
+suite "addrutils - dialableAddressPolicy":
+  const relayId = "16Uiu2HAkyRvHo1AyyQY1xiHC8QbYjXCHkZbneVC8dBtJjp1SZcGD"
+
+  proc circuitAddr(relayIp: string): MultiAddress =
+    MultiAddress.init("/ip4/" & relayIp & "/tcp/8070/p2p/" & relayId & "/p2p-circuit").expect(
+      "valid"
+    )
+
+  test "keeps a public direct address":
+    check MultiAddress.init("/ip4/204.168.234.45/tcp/8070").expect("valid").dialableAddressPolicy()
+
+  test "drops a loopback direct address":
+    check not MultiAddress.init("/ip4/127.0.0.1/tcp/8070").expect("valid").dialableAddressPolicy()
+
+  test "drops a private direct address":
+    check not MultiAddress.init("/ip4/192.168.100.103/tcp/8070").expect("valid").dialableAddressPolicy()
+
+  test "keeps a circuit address through a public relay":
+    check circuitAddr("204.168.234.45").dialableAddressPolicy()
+
+  test "drops a circuit address through a private relay":
+    check not circuitAddr("172.17.0.1").dialableAddressPolicy()