Expose port mapping type

library/storage_thread_requests/requests/node_debug_request.nim

@@ -16,6 +16,7 @@ import ../../../storage/rest/json
 import ../../../storage/node
 
 from ../../../storage/storage import StorageServer, node
+import ../../../storage/nat
 import ../../../storage/discovery
 
 logScope:
@@ -69,6 +70,14 @@ proc getDebug(
           "unknown",
       "relayRunning":
         storage[].autoRelayService.isSome and storage[].autoRelayService.get.isRunning,
+      "portMapping":
+        if storage[].natMapper.isNone or
+            storage[].natMapper.get.portMappingType == NoMapping:
+          "none"
+        elif storage[].natMapper.get.portMappingType == UpnpMapping:
+          "upnp"
+        else:
+          "pmp",
     },
   }
 

storage/nat.nim

@@ -32,11 +32,16 @@ type NatConfig* = object
   of true: extIp*: IpAddress
   of false: nat*: NatStrategy
 
+type PortMappingType* = enum
+  NoMapping
+  UpnpMapping
+  PmpMapping
+
 type NatMapper* = ref object of RootObj
   natConfig*: NatConfig
   tcpPort*: Port
   discoveryPort*: Port
-  hasUpnpMapping: bool
+  portMappingType*: PortMappingType
 
 type MapNatPortsCtx = object
   natConfig: NatConfig
@@ -44,7 +49,7 @@ type MapNatPortsCtx = object
   discoveryPort: Port
   signal: ThreadSignalPtr
   result: Option[(Port, Port)]
-  hasUpnpMapping: bool
+  portMappingType: PortMappingType
 
 proc mapNatPortsThread(ctx: ptr MapNatPortsCtx) {.thread.} =
   if ctx.natConfig.hasExtIp:
@@ -57,7 +62,7 @@ proc mapNatPortsThread(ctx: ptr MapNatPortsCtx) {.thread.} =
   if upnpRes.isOk:
     let ports = upnpRes.value.mapPorts(ctx.tcpPort, ctx.discoveryPort)
     if ports.isSome:
-      ctx.hasUpnpMapping = true
+      ctx.portMappingType = UpnpMapping
       ctx.result = ports
       discard ctx.signal.fireSync()
       return
@@ -66,6 +71,7 @@ proc mapNatPortsThread(ctx: ptr MapNatPortsCtx) {.thread.} =
   if pmpRes.isOk:
     let ports = pmpRes.value.mapPorts(ctx.tcpPort, ctx.discoveryPort)
     if ports.isSome:
+      ctx.portMappingType = PmpMapping
       ctx.result = ports
 
   discard ctx.signal.fireSync()
@@ -95,8 +101,8 @@ method mapNatPorts*(
       # Always sync hasUpnpMapping back, even on timeout or cancellation.
       # If the thread mapped ports just after the timeout, close() will
       # still clean them up on the router.
-      if ctx.hasUpnpMapping:
-        m.hasUpnpMapping = true
+      if ctx.portMappingType != NoMapping:
+        m.portMappingType = ctx.portMappingType
     freeShared(ctx)
     discard signal.close()
 
@@ -183,7 +189,7 @@ method handleNatStatus*(
 proc close*(m: NatMapper, device = UpnpDevice()) =
   # UPnP mappings are permanent (leaseDuration=0) and must be deleted explicitly.
   # NAT-PMP mappings expire automatically after NATPMP_LIFETIME seconds.
-  if not m.hasUpnpMapping:
+  if m.portMappingType != UpnpMapping:
     return
 
   # deletePortMapping requires the IGD control URL set during init

storage/rest/api.nim

@@ -41,6 +41,7 @@ import ../blockexchange
 import ../units
 import ../utils/options
 import ../utils/natsimulation
+import ../nat
 
 import ./coders
 import ./json
@@ -566,6 +567,7 @@ proc initDebugApi(
     conf: StorageConf,
     autonat: Option[AutonatV2Service],
     autoRelay: Option[AutoRelayService],
+    natMapper: Option[NatMapper],
     natRouter: Option[NatRouter],
     router: var RestRouter,
 ) =
@@ -595,6 +597,13 @@ proc initDebugApi(
             else:
               "unknown",
           "relayRunning": autoRelay.isSome and autoRelay.get.isRunning,
+          "portMapping":
+            if natMapper.isNone or natMapper.get.portMappingType == NoMapping:
+              "none"
+            elif natMapper.get.portMappingType == UpnpMapping:
+              "upnp"
+            else:
+              "pmp",
         },
       }
 
@@ -679,6 +688,7 @@ proc initRestApi*(
     repoStore: RepoStore,
     autonat: Option[AutonatV2Service],
     autoRelay: Option[AutoRelayService],
+    natMapper: Option[NatMapper],
     natRouter: Option[NatRouter],
     corsAllowedOrigin: ?string,
 ): RestRouter =
@@ -686,6 +696,6 @@ proc initRestApi*(
 
   initDataApi(node, repoStore, router)
   initNodeApi(node, conf, router)
-  initDebugApi(node, conf, autonat, autoRelay, natRouter, router)
+  initDebugApi(node, conf, autonat, autoRelay, natMapper, natRouter, router)
 
   return router

storage/storage.nim

@@ -58,7 +58,7 @@ type
     # Expose to make reachability accessible from rest api
     autonatService*: Option[AutonatV2Service]
     autoRelayService*: Option[AutoRelayService]
-    natMapper: Option[NatMapper]
+    natMapper*: Option[NatMapper]
     natRouter*: Option[NatRouter]
     isStarted: bool
 
@@ -424,7 +424,7 @@ proc new*(
     restServer = RestServerRef
       .new(
         storageNode.initRestApi(
-          config, repoStore, autonatService, autoRelayService, natRouter,
+          config, repoStore, autonatService, autoRelayService, natMapper, natRouter,
           config.apiCorsAllowedOrigin,
         ),
         initTAddress(config.apiBindAddress.get(), config.apiPort),

tests/integration/storageclient.nim

@@ -293,6 +293,17 @@ proc natRelayRunning*(
   except KeyError as e:
     return failure e.msg
 
+proc natPortMapping*(
+    client: StorageClient
+): Future[?!string] {.async: (raises: [CancelledError, HttpError]).} =
+  let info = await client.info()
+  if info.isErr:
+    return failure "Failed to get node info"
+  try:
+    return info.get()["nat"]["portMapping"].getStr().success
+  except KeyError as e:
+    return failure e.msg
+
 proc setNatFiltering*(
     client: StorageClient, filtering: string
 ): Future[?!void] {.async: (raises: [CancelledError, HttpError]).} =

tests/integration/storageconfig.nim

@@ -290,6 +290,14 @@ proc withListenIp*(
     config.addCliOption("--listen-ip", ip)
   return startConfig
 
+proc withListenPort*(
+    self: StorageConfigs, idx: int, port: int
+): StorageConfigs {.raises: [StorageConfigError].} =
+  self.checkBounds idx
+  var startConfig = self
+  startConfig.configs[idx].addCliOption("--listen-port", $port)
+  return startConfig
+
 proc withNatNumPeersToAsk*(
     self: StorageConfigs, numPeersToAsk: int
 ): StorageConfigs {.raises: [StorageConfigError].} =

tests/storage/testnat.nim

@@ -56,7 +56,7 @@ suite "NAT - NatMapper.close":
       tcpPort: Port(8080),
       discoveryPort: Port(8090),
     )
-    mapper.hasUpnpMapping = true
+    mapper.portMappingType = UpnpMapping
     let device = MockUpnpDevice()
     mapper.close(device)
     check device.deletedPorts ==
@@ -137,23 +137,3 @@ asyncchecksuite "NAT - handleNatStatus":
 
     check not autoRelay.isRunning
     check disc.announceAddrs == @[dialBack]
-
-suite "NAT - UPnP port mapping (requires NAT_TEST_UPNP=1)":
-  test "mapPorts and cleanup":
-    if getEnv("NAT_TEST_UPNP") != "1":
-      skip()
-      return
-
-    let res = UpnpDevice.init()
-    check res.isOk
-
-    let device = res.value
-    let ports = device.mapPorts(Port(8101), Port(8090))
-    check ports.isSome
-
-    let (tcp, udp) = ports.get()
-    check tcp == Port(8101)
-    check udp == Port(8090)
-
-    check device.deletePortMapping(Port(8101), NatIpProtocol.Tcp).isOk
-    check device.deletePortMapping(Port(8090), NatIpProtocol.Udp).isOk