Update nim libplum and provide multiple fixes

storage/nat.nim

@@ -64,12 +64,12 @@ method mapNatPorts*(
   if not m.plumInitialized:
     # 5s matches the old NatPortMappingTimeout used with miniupnpc/libnatpmp.
     let plumLogLevel =
-      if getEnv("DEBUG") == "1": PLUM_LOG_LEVEL_VERBOSE else: PLUM_LOG_LEVEL_NONE
+      if getEnv("DEBUG") == "1": PlumLogLevel.Verbose else: PlumLogLevel.None
     let res = init(
       logLevel = plumLogLevel,
-      discoverTimeout = m.discoverTimeout,
-      mappingTimeout = m.mappingTimeout,
-      recheckPeriod = m.recheckPeriod,
+      discoverTimeout = m.discoverTimeout.int32,
+      mappingTimeout = m.mappingTimeout.int32,
+      recheckPeriod = m.recheckPeriod.int32,
     )
     if res.isErr:
       warn "Failed to initialize plum", msg = res.error
@@ -149,10 +149,8 @@ method handleNatStatus*(
       return
 
     if autoRelayService.isRunning:
-      if not await autoRelayService.stop(switch):
-        debug "AutoRelayService stop method returned false"
-      else:
-        debug "AutoRelayService stopped"
+      await autoRelayService.stop(switch)
+      debug "AutoRelayService stopped"
 
     # Update the record first, then flip to server mode: otherwise the node
     # briefly serves DHT queries with the previous (possibly empty) record.
@@ -201,10 +199,8 @@ method handleNatStatus*(
 
         if autoRelayService.isRunning:
           # Here we stop the relay because the node *should* be reachable
-          if not await autoRelayService.stop(switch):
-            debug "AutoRelayService returned an issue when trying to stop"
-          else:
-            debug "AutoRelayService stopped"
+          await autoRelayService.stop(switch)
+          debug "AutoRelayService stopped"
 
         # Note that we update the DHT records but we don't set the client mode
         # to false because we are not sure the node is reachable.
@@ -221,10 +217,8 @@ method handleNatStatus*(
     if not hasPortMapping and not autoRelayService.isRunning:
       debug "No port mapping found let's start autorelay"
 
-      if not await autoRelayService.setup(switch):
-        warn "Unable to start autorelay service"
-      else:
-        debug "AutoRelayService started"
+      await autoRelayService.start(switch)
+      debug "AutoRelayService started"
 
 proc reachabilityStr*(autonat: Option[AutonatV2Service]): string =
   if autonat.isSome:

storage/presets.nim

@@ -53,7 +53,6 @@ proc `bootstrapNodes`*(self: NetworkPreset): seq[SignedPeerRecord] =
     result.add(parse(SignedPeerRecord, record).tryGet())
 
 const NetworkPresets* = [
-  NetworkPreset.init("none", "No bootstrap nodes", @[]),
   NetworkPreset.init(
     "logos.test",
     "Logos testnet",

storage/storage.nim

@@ -181,7 +181,7 @@ proc stop*(s: StorageServer) {.async.} =
 
   if s.autoRelayService.isSome and s.autoRelayService.get.isRunning:
     proc stopAutoRelay(): Future[void] {.async: (raises: []).} =
-      discard await noCancel s.autoRelayService.get.stop(s.storageNode.switch)
+      await noCancel s.autoRelayService.get.stop(s.storageNode.switch)
 
     futures.add(stopAutoRelay())
 
@@ -277,6 +277,25 @@ proc new*(
     .withSignedPeerRecord(true)
     .withCircuitRelay(relay)
 
+  let bootstrapNodes =
+    if config.noBootstrapNode:
+      # Sanity checks that the user isn't doing anything funny.
+      if config.bootstrapNodes.len > 0:
+        error "Cannot specify bootstrap nodes when using no-bootstrap flag"
+        raise newException(
+          ValueError, "Cannot specify bootstrap nodes when using no-bootstrap flag"
+        )
+
+      warn "Node has been marked with --no-bootstrap-node and will NOT be bootstrapped"
+      seq[SignedPeerRecord](@[])
+    elif config.bootstrapNodes.len > 0:
+      warn "Overriding network preset using custom bootstrap nodes",
+        nodes = config.bootstrapNodes
+      config.bootstrapNodes
+    else:
+      info "Bootstrapping node using a predefined network", network = $config.network
+      config.network.bootstrapNodes
+
   var autonatConfig = none(AutonatV2ServiceConfig)
   if config.autonatServer:
     info "AutoNAT server enabled"
@@ -301,8 +320,14 @@ proc new*(
         enableAddressMapper = false,
       )
     )
+    # At the first AutoNAT probe, the only identify observations available come
+    # from the bootstrap nodes, so requiring more observations than there are
+    # bootstrap nodes would make the threshold unreachable. The floor of 1
+    # covers the case where the bootstrap list is empty.
+    let observedAddrMinCount =
+      max(1, min(config.natObservedAddrMinCount, bootstrapNodes.len))
     switchBuilder = switchBuilder.withObservedAddrManager(
-      ObservedAddrManager.new(minCount = config.natObservedAddrMinCount)
+      ObservedAddrManager.new(minCount = observedAddrMinCount)
     )
 
   var natRouter: Option[NatRouter]
@@ -323,8 +348,6 @@ proc new*(
         .build()
 
   var taskPool: Taskpool
-  autonatClient.setup(switch)
-  switch.mount(autonatClient)
 
   # AutoNAT's first reachability probe fires immediately on start.
   # Wired via withAutonatV2 it lands in switch.services and runs at switch.start,
@@ -382,25 +405,6 @@ proc new*(
     error "Failed to initialize discovery datastore",
       path = providersPath, err = discoveryStoreRes.error.msg
 
-  let bootstrapNodes =
-    if config.noBootstrapNode:
-      # Sanity checks that the user isn't doing anything funny.
-      if config.bootstrapNodes.len > 0:
-        error "Cannot specify bootstrap nodes when using no-bootstrap flag"
-        raise newException(
-          ValueError, "Cannot specify bootstrap nodes when using no-bootstrap flag"
-        )
-
-      warn "Node has been marked with --no-bootstrap-node and will NOT be bootstrapped"
-      seq[SignedPeerRecord](@[])
-    elif config.bootstrapNodes.len > 0:
-      warn "Overriding network preset using custom bootstrap nodes",
-        nodes = config.bootstrapNodes
-      config.bootstrapNodes
-    else:
-      info "Bootstrapping node using a predefined network", network = $config.network
-      config.network.bootstrapNodes
-
   let
     discoveryStore =
       Datastore(discoveryStoreRes.expect("Should create discovery datastore!"))
@@ -411,7 +415,6 @@ proc new*(
       bindPort = config.discoveryPort,
       bootstrapNodes = bootstrapNodes,
       discoveryPort = config.discoveryPort,
-      bootstrapNodes = config.bootstrapNodes,
       store = discoveryStore,
     )
 
@@ -491,6 +494,7 @@ proc new*(
       rng = random.Rng.instance(),
     )
 
+    relayService.setup(switch)
     autoRelayService = some(relayService)
 
     natMapper = some(

tests/integration/5_minutes/testnatdownload.nim

@@ -51,12 +51,16 @@ multinodesuite "NAT download":
       pollInterval = PollInterval,
     )
 
-    # Verify natNode advertises a relay circuit address. seed has never dialed
-    # natNode, so APDF blocks any direct inbound connection from seed — the
-    # only reachable address is the p2p-circuit one.
-    let info = (await natNode.client.info()).get
-    let addrs = info["addrs"].getElems.mapIt(it.getStr)
-    check addrs.anyIt("p2p-circuit" in it)
+    # relayRunning only means the service started: the reservation itself
+    # takes a few more seconds, so we have to poll.
+    proc advertisesCircuitAddr(): Future[bool] {.async.} =
+      let info = (await natNode.client.info()).get
+      let addrs = info["addrs"].getElems.mapIt(it.getStr)
+      return addrs.anyIt("p2p-circuit" in it)
+
+    check eventuallySafe(
+      await advertisesCircuitAddr(), timeout = RelayTimeout, pollInterval = PollInterval
+    )
 
     let content = "content seeded from nat node"
     let cid = (await natNode.client.upload(content)).get

tests/integration/multinodes.nim

@@ -127,8 +127,13 @@ template multinodesuite*(suiteName: string, body: untyped) =
           lastUsedStorageApiPort = apiPort
           lastUsedStorageDiscPort = discPort
 
-        for bootstrapNode in bootstrapNodes:
-          config.addCliOption("--bootstrap-node", bootstrapNode)
+        if bootstrapNodes.len == 0:
+          # Without this flag the node would bootstrap on the default
+          # network preset.
+          config.addCliOption("--no-bootstrap-node")
+        else:
+          for bootstrapNode in bootstrapNodes:
+            config.addCliOption("--bootstrap-node", bootstrapNode)
 
         config.addCliOption("--data-dir", datadir)
       except StorageConfigError as e:

tests/storage/testdiscovery.nim

@@ -20,7 +20,7 @@ suite "Discovery - SPR record logic":
     udpPort = Port(8090)
 
   setup:
-    key = PrivateKey.random(Rng.instance[]).get()
+    key = PrivateKey.random(Rng.instance()).get()
     disc = Discovery.new(key, announceAddrs = @[])
 
   test "updateRecordsAndSpr sets the SPR with both TCP and UDP addresses":

tests/storage/testnat.nim

@@ -35,7 +35,7 @@ asyncchecksuite "NAT - handleNatStatus":
   setup:
     autoRelay =
       AutoRelayService.new(1, relayClientModule.RelayClient.new(), nil, Rng.instance())
-    key = PrivateKey.random(Rng.instance[]).get()
+    key = PrivateKey.random(Rng.instance()).get()
     disc = Discovery.new(key, announceAddrs = @[])
     sw = newStandardSwitch()
     await sw.start()
@@ -44,7 +44,7 @@ asyncchecksuite "NAT - handleNatStatus":
     await sw.stop()
 
     if autoRelay.isRunning:
-      discard await autoRelay.stop(sw)
+      await autoRelay.stop(sw)
 
   let discoveryPort = Port(8090)
 
@@ -54,7 +54,7 @@ asyncchecksuite "NAT - handleNatStatus":
       mappedPorts: some((Port(9000), Port(9001), MappingProtocol.UPnP))
     )
 
-    discard await autorelayservice.setup(autoRelay, sw)
+    autorelayservice.setup(autoRelay, sw)
     await mapper.handleNatStatus(
       NotReachable, Opt.some(dialBack), discoveryPort, disc, sw, autoRelay
     )
@@ -89,7 +89,7 @@ asyncchecksuite "NAT - handleNatStatus":
   test "handleNatStatus does nothing when Reachable and no dialBackAddr":
     let mapper = MockNatPortMapper(mappedPorts: none((Port, Port, MappingProtocol)))
 
-    discard await autorelayservice.setup(autoRelay, sw)
+    autorelayservice.setup(autoRelay, sw)
     disc.protocol.clientMode = true
     await mapper.handleNatStatus(
       Reachable, Opt.none(MultiAddress), discoveryPort, disc, sw, autoRelay
@@ -104,7 +104,7 @@ asyncchecksuite "NAT - handleNatStatus":
     let mapper = MockNatPortMapper(mappedPorts: none((Port, Port, MappingProtocol)))
 
     disc.protocol.clientMode = true
-    discard await autorelayservice.setup(autoRelay, sw)
+    autorelayservice.setup(autoRelay, sw)
     await mapper.handleNatStatus(
       Reachable, Opt.some(dialBack), discoveryPort, disc, sw, autoRelay
     )

tests/storage/testnatsimulation.nim

@@ -26,7 +26,7 @@ proc newSwitch(rng: Rng): Switch =
   SwitchBuilder
     .new()
     .withRng(rng)
-    .withPrivateKey(PrivateKey.random(rng[]).get())
+    .withPrivateKey(PrivateKey.random(rng).get())
     .withAddresses(@[MultiAddress.init(listenAddr).get()])
     .withTcpTransport(flags)
     .withNoise()
@@ -37,7 +37,7 @@ proc newNatSwitch(router: NatRouter, rng: Rng): Switch =
   SwitchBuilder
     .new()
     .withRng(rng)
-    .withPrivateKey(PrivateKey.random(rng[]).get())
+    .withPrivateKey(PrivateKey.random(rng).get())
     .withAddresses(@[MultiAddress.init(listenAddr).get()])
     .withNatTransport(router, flags)
     .withNoise()

vendor/nim-libplum

@@ -1 +1 @@
-Subproject commit bf0ace8da2715b6aed1e1ed9f33614c8e3b83893
+Subproject commit 0ca0361f50452147d52c7d613c5d6d27a7ac3471