From f0a7e4b4258d3bf7216cd95f157dd28156d46dbf Mon Sep 17 00:00:00 2001 From: Arnaud Date: Thu, 4 Jun 2026 18:51:00 +0400 Subject: [PATCH] Update nim libplum and provide multiple fixes --- storage/nat.nim | 26 ++++------ storage/presets.nim | 1 - storage/storage.nim | 52 ++++++++++--------- .../integration/5_minutes/testnatdownload.nim | 16 +++--- tests/integration/multinodes.nim | 9 +++- tests/storage/testdiscovery.nim | 2 +- tests/storage/testnat.nim | 10 ++-- tests/storage/testnatsimulation.nim | 4 +- vendor/nim-libplum | 2 +- 9 files changed, 64 insertions(+), 58 deletions(-) diff --git a/storage/nat.nim b/storage/nat.nim index a0b175b0..6abcea63 100644 --- a/storage/nat.nim +++ b/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: diff --git a/storage/presets.nim b/storage/presets.nim index a8600ac2..81d441a6 100644 --- a/storage/presets.nim +++ b/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", diff --git a/storage/storage.nim b/storage/storage.nim index 5632d94e..25fe2bf5 100644 --- a/storage/storage.nim +++ b/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( diff --git a/tests/integration/5_minutes/testnatdownload.nim b/tests/integration/5_minutes/testnatdownload.nim index 95554326..89299760 100644 --- a/tests/integration/5_minutes/testnatdownload.nim +++ b/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 diff --git a/tests/integration/multinodes.nim b/tests/integration/multinodes.nim index 75fdb9d1..d5419255 100644 --- a/tests/integration/multinodes.nim +++ b/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: diff --git a/tests/storage/testdiscovery.nim b/tests/storage/testdiscovery.nim index bba63d64..a32d4401 100644 --- a/tests/storage/testdiscovery.nim +++ b/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": diff --git a/tests/storage/testnat.nim b/tests/storage/testnat.nim index 888d0b2f..7d0e7275 100644 --- a/tests/storage/testnat.nim +++ b/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 ) diff --git a/tests/storage/testnatsimulation.nim b/tests/storage/testnatsimulation.nim index b1de0b38..f2079c8a 100644 --- a/tests/storage/testnatsimulation.nim +++ b/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() diff --git a/vendor/nim-libplum b/vendor/nim-libplum index bf0ace8d..0ca0361f 160000 --- a/vendor/nim-libplum +++ b/vendor/nim-libplum @@ -1 +1 @@ -Subproject commit bf0ace8da2715b6aed1e1ed9f33614c8e3b83893 +Subproject commit 0ca0361f50452147d52c7d613c5d6d27a7ac3471