Rename misleading bool

storage/nat.nim

@@ -50,7 +50,7 @@ type NatPortMapper* = ref object of RootObj
   recheckPeriod*: int
   portMapping*: Option[PortMapping]
   plumInitialized: bool
-  closed: bool
+  stopped: bool
 
 # libplum seams, extracted as methods so tests can override them without I/O.
 
@@ -95,7 +95,7 @@ method mapNatPorts*(
 ): Future[Option[(Port, Port, MappingProtocol)]] {.
     async: (raises: [CancelledError]), base, gcsafe
 .} =
-  if m.closed or m.natConfig.hasExtIp:
+  if m.stopped or m.natConfig.hasExtIp:
     return none((Port, Port, MappingProtocol))
 
   # If both mappings are still live, return the stored ports without recreating.
@@ -116,7 +116,7 @@ method mapNatPorts*(
 
   let tcpRes = await m.createMappingFor(TCP, m.tcpPort.uint16)
 
-  if m.closed:
+  if m.stopped:
     # Double check in case the node is stopping
     return none((Port, Port, MappingProtocol))
 
@@ -126,7 +126,7 @@ method mapNatPorts*(
 
   let udpRes = await m.createMappingFor(UDP, m.discoveryPort.uint16)
 
-  if m.closed:
+  if m.stopped:
     # Double check in case the node is stopping
     return none((Port, Port, MappingProtocol))
 
@@ -155,9 +155,14 @@ proc close*(m: NatPortMapper) =
     discard cleanup()
     m.plumInitialized = false
 
+proc start*(m: NatPortMapper) =
+  ## Re-enable AutoNAT-driven port mapping after a previous stop, so a restarted
+  ## node maps its ports again instead of staying disabled.
+  m.stopped = false
+
 proc stop*(m: NatPortMapper) =
   ## Ensure that any future AutoNAT callback does not re-initialize libplum.
-  m.closed = true
+  m.stopped = true
   m.close()
 
 method handleNatStatus*(
@@ -169,7 +174,7 @@ method handleNatStatus*(
     switch: Switch,
     autoRelayService: AutoRelayService,
 ) {.async: (raises: [CancelledError]), base, gcsafe.} =
-  if m.closed:
+  if m.stopped:
     return
 
   case networkReachability
@@ -206,7 +211,7 @@ method handleNatStatus*(
 
       let maybePorts = await m.mapNatPorts()
 
-      if m.closed:
+      if m.stopped:
         # Double check in case the node is stopping
         return
 

storage/storage.nim

@@ -95,6 +95,9 @@ proc start*(s: StorageServer) {.async.} =
     for t in s.storageNode.switch.transports:
       t.networkReachability = NetworkReachability.NotReachable
 
+  if s.natMapper.isSome:
+    s.natMapper.get.start()
+
   # When listenPort is 0 the OS assigns a random port. For UDP, the port
   # doesn't change so there is no need to update it.
   if s.natMapper.isSome and s.config.listenPort == Port(0):

tests/storage/testnatreaction.nim

@@ -33,8 +33,10 @@ type MockMapNatPortMapper = ref object of NatPortMapper
   live: bool
   createAttempts: seq[PlumProtocol]
   destroyed: seq[cint]
+  initAttempts: int
 
 method initPlum(m: MockMapNatPortMapper): Result[void, string] {.gcsafe.} =
+  inc m.initAttempts
   ok()
 
 method hasLivePortMapping(m: MockMapNatPortMapper): bool {.gcsafe.} =
@@ -195,6 +197,24 @@ asyncchecksuite "NAT reaction - port mapping":
     check not autoRelay.isRunning
     check disc.announceAddrs == newSeq[MultiAddress]()
 
+  test "handleNatStatus retries the port mapping on the next NotReachable after a failure":
+    # A failed mapping must not disable the mapper: close() resets plum so the
+    # next AutoNAT iteration re-runs discover and tries again.
+    let mapper = MockMapNatPortMapper(
+      tcpResult: Result[MappingResult, string].err("tcp mapping failed")
+    )
+
+    autorelayservice.setup(autoRelay, sw)
+    await mapper.handleNatStatus(
+      NotReachable, Opt.none(MultiAddress), discoveryPort, disc, sw, autoRelay
+    )
+    await mapper.handleNatStatus(
+      NotReachable, Opt.none(MultiAddress), discoveryPort, disc, sw, autoRelay
+    )
+
+    check mapper.initAttempts == 2
+    check mapper.createAttempts == @[PlumProtocol.TCP, PlumProtocol.TCP]
+
 asyncchecksuite "NAT reaction - address announcing":
   var sw: Switch
   var key: PrivateKey
@@ -343,3 +363,18 @@ asyncchecksuite "NAT - mapNatPorts":
     check (await mapper.mapNatPorts()) ==
       some((Port(9000), Port(9001), MappingProtocol.UPnP))
     check mapper.createAttempts.len == 0
+
+  test "does not map after stop, maps again after start":
+    let mapper = MockMapNatPortMapper(
+      tcpResult: mappingOk(cint(1), 9000), udpResult: mappingOk(cint(2), 9001)
+    )
+
+    mapper.stop()
+
+    check (await mapper.mapNatPorts()).isNone
+    check mapper.createAttempts.len == 0
+
+    mapper.start()
+
+    check (await mapper.mapNatPorts()) ==
+      some((Port(9000), Port(9001), MappingProtocol.UPnP))