From 62af77926656643282cbdb992b29b052819a6ff5 Mon Sep 17 00:00:00 2001 From: Arnaud Date: Tue, 5 May 2026 21:28:01 +0400 Subject: [PATCH] Rebase and cleanup --- storage/blockexchange/peers/peercontext.nim | 2 +- storage/manifest/coders.nim | 4 +- storage/nat.nim | 29 ++++---- storage/storage.nim | 34 +++------- storage/stores/networkstore.nim | 2 + storage/utils/addrutils.nim | 35 ---------- storage/utils/natutils.nim | 50 ++++++-------- tests/integration/1_minute/testnat.nim | 1 - tests/integration/storageconfig.nim | 8 +++ .../blockexchange/discovery/testdiscovery.nim | 4 +- tests/storage/testnatutils.nim | 68 +------------------ 11 files changed, 56 insertions(+), 181 deletions(-) diff --git a/storage/blockexchange/peers/peercontext.nim b/storage/blockexchange/peers/peercontext.nim index 9e7c80f6..d81a4161 100644 --- a/storage/blockexchange/peers/peercontext.nim +++ b/storage/blockexchange/peers/peercontext.nim @@ -7,7 +7,7 @@ ## This file may not be copied, modified, or distributed except according to ## those terms. -import std/math +import std/[math, options] import pkg/libp2p import pkg/chronos diff --git a/storage/manifest/coders.nim b/storage/manifest/coders.nim index 19664e64..a774a96a 100644 --- a/storage/manifest/coders.nim +++ b/storage/manifest/coders.nim @@ -13,9 +13,7 @@ import times {.push raises: [].} -import std/tables -import std/sequtils -import std/options +import std/[tables, options] import pkg/libp2p import pkg/questionable diff --git a/storage/nat.nim b/storage/nat.nim index 863b1e68..9e4e591b 100644 --- a/storage/nat.nim +++ b/storage/nat.nim @@ -16,7 +16,7 @@ import import pkg/chronos import pkg/chronicles import pkg/libp2p -import pkg/libp2p/protocols/connectivity/autonat/types +import pkg/libp2p/protocols/connectivity/autonatv2/service import pkg/libp2p/services/autorelayservice import ./utils @@ -58,7 +58,7 @@ logScope: type NatMapper* = ref object of RootObj method mapNatPorts*(m: NatMapper): Option[(Port, Port)] {.base, gcsafe, raises: [].} = - raiseAssert "mapNatPorts not implemented" + none((Port, Port)) type DefaultNatMapper* = ref object of NatMapper natConfig*: NatConfig @@ -303,18 +303,13 @@ proc findReachableNodes*(bootstrapNodes: seq[SignedPeerRecord]): seq[SignedPeerR proc nattedPorts*(natConfig: NatConfig, tcpPort, udpPort: Port): Option[(Port, Port)] = if natConfig.hasExtIp: - return none((Port, Port)) # manual setup, no port mapping needed - setupNat(natConfig.nat, tcpPort, udpPort, "storage") + return none((Port, Port)) + let clientId = "storage" + return setupNat(natConfig.nat, tcpPort, udpPort, clientId) method mapNatPorts*(m: DefaultNatMapper): Option[(Port, Port)] {.gcsafe, raises: [].} = nattedPorts(m.natConfig, m.tcpPort, m.discoveryPort) -proc hasPublicIp*(addrs: seq[MultiAddress]): bool = - for addr in addrs: - let (ip, _) = getAddressAndPort(addr) - if ip.isSome and isGlobalUnicast(ip.get): - return true - proc handleNatStatus*( networkReachability: NetworkReachability, dialBackAddr: Opt[MultiAddress], @@ -340,18 +335,20 @@ proc handleNatStatus*( let discAddr = dialBackAddr.get.remapAddr(protocol = some("udp"), port = some(discoveryPort)) discovery.updateAnnounceRecord(@[dialBackAddr.get]) - discovery.updateDhtRecord(@[dialBackAddr.get, discAddr]) + discovery.updateDhtRecord(@[discAddr]) # TODO: switch DHT to server mode of NotReachable: var hasPortMapping = false - if dialBackAddr.isSome: + if dialBackAddr.isNone: + warn "Got empty dialback address in AutoNat when node is Reachable" + else: let maybePorts = mapper.mapNatPorts() if maybePorts.isSome: let (tcpPort, udpPort) = maybePorts.get() - let announceAddr = dialBackAddr.get.remapAddr(port = some(tcpPort)) - let discAddr = + let announceAddress = dialBackAddr.get.remapAddr(port = some(tcpPort)) + let discoveryAddrs = dialBackAddr.get.remapAddr(protocol = some("udp"), port = some(udpPort)) # TODO: Try a dial me to make sure we are reachable @@ -360,8 +357,8 @@ proc handleNatStatus*( if not await autoRelayService.stop(switch): debug "AutoRelayService stop method returned false" - discovery.updateAnnounceRecord(@[announceAddr]) - discovery.updateDhtRecord(@[announceAddr, discAddr]) + discovery.updateAnnounceRecord(@[announceAddress]) + discovery.updateDhtRecord(@[discoveryAddrs]) hasPortMapping = true diff --git a/storage/storage.nim b/storage/storage.nim index 5e7ce9f7..69b9bfea 100644 --- a/storage/storage.nim +++ b/storage/storage.nim @@ -41,7 +41,6 @@ import ./namespaces import ./storagetypes import ./logutils import ./nat -import ./utils/natutils logScope: topics = "storage node" @@ -88,29 +87,16 @@ proc start*(s: StorageServer) {.async.} = else: getBestLocalAddress(s.config.listenIp) - if announceIp.isNone: - # We should have an IP, even at private IP - raise newException(StorageError, "Unable to determine an IP address to announce") - - # Remap switch addresses to the resolved IP (replaces 0.0.0.0 or :: with the actual address), - # keeping unique entries only. - let announceAddrs = s.storageNode.switch.peerInfo.addrs - .mapIt(it.remapAddr(ip = announceIp, port = none(Port))) - .deduplicate() - let discoveryAddrs = - @[getMultiAddrWithIPAndUDPPort(announceIp.get, s.config.discoveryPort)] - s.storageNode.discovery.updateDhtRecord(announceAddrs & discoveryAddrs) - s.storageNode.discovery.updateAnnounceRecord(announceAddrs) - - var hasPublicAddr = false - for announceAddr in announceAddrs: - let (maybeIp, _) = getAddressAndPort(announceAddr) - if maybeIp.isSome and maybeIp.get.isGlobalUnicast(): - hasPublicAddr = true - break - - if not hasPublicAddr: - warn "Unable to determine a public IP address. This node will only be reachable on a private network." + if announceIp.isSome: + let ip = announceIp.get + let announceAddrs = s.storageNode.switch.peerInfo.addrs + .mapIt(it.remapAddr(ip = some(ip), port = none(Port))) + .deduplicate() + let discAddr = getMultiAddrWithIPAndUDPPort(ip, s.config.discoveryPort) + s.storageNode.discovery.updateAnnounceRecord(announceAddrs) + s.storageNode.discovery.updateDhtRecord(announceAddrs & @[discAddr]) + else: + warn "Unable to determine a local IP address to announce" await s.storageNode.start() diff --git a/storage/stores/networkstore.nim b/storage/stores/networkstore.nim index 8c781f6b..6767f042 100644 --- a/storage/stores/networkstore.nim +++ b/storage/stores/networkstore.nim @@ -9,6 +9,8 @@ {.push raises: [].} +import std/options + import pkg/chronos import pkg/libp2p import pkg/questionable/results diff --git a/storage/utils/addrutils.nim b/storage/utils/addrutils.nim index 09891cfa..72d80ad3 100644 --- a/storage/utils/addrutils.nim +++ b/storage/utils/addrutils.nim @@ -74,38 +74,3 @@ proc getMultiAddrWithIpAndTcpPort*(ip: IpAddress, port: Port): MultiAddress = return MultiAddress.init(ipFamily & $ip & "/tcp/" & $port).expect( "Failed to construct multiaddress with IP and TCP port" ) - -proc getAddressAndPort*( - ma: MultiAddress -): tuple[ip: Option[IpAddress], port: Option[Port]] = - try: - # Try IPv4 first - let ipv4Result = ma[multiCodec("ip4")] - let ip = - if ipv4Result.isOk: - let ipBytes = ipv4Result.get().protoArgument().expect("Invalid IPv4 format") - let ipArray = [ipBytes[0], ipBytes[1], ipBytes[2], ipBytes[3]] - some(IpAddress(family: IPv4, address_v4: ipArray)) - else: - # Try IPv6 if IPv4 not found - let ipv6Result = ma[multiCodec("ip6")] - if ipv6Result.isOk: - let ipBytes = ipv6Result.get().protoArgument().expect("Invalid IPv6 format") - var ipArray: array[16, byte] - for i in 0 .. 15: - ipArray[i] = ipBytes[i] - some(IpAddress(family: IPv6, address_v6: ipArray)) - else: - none(IpAddress) - - # Get TCP Port - let portResult = ma[multiCodec("tcp")] - let port = - if portResult.isOk: - let portBytes = portResult.get().protoArgument().expect("Invalid port format") - some(Port(fromBytesBE(uint16, portBytes))) - else: - none(Port) - (ip: ip, port: port) - except Exception: - (ip: none(IpAddress), port: none(Port)) diff --git a/storage/utils/natutils.nim b/storage/utils/natutils.nim index 04cc57f9..b808d915 100644 --- a/storage/utils/natutils.nim +++ b/storage/utils/natutils.nim @@ -1,6 +1,6 @@ {.push raises: [].} -import std/[net, tables, hashes, options], pkg/results, chronos, chronicles +import std/[net, options], pkg/results, chronos, chronicles import pkg/libp2p @@ -9,54 +9,42 @@ type NatStrategy* = enum NatUpnp NatPmp -func isGlobalUnicast*(address: TransportAddress): bool = - if address.isGlobal() and address.isUnicast(): true else: false +proc getRouteIpv4*(): Result[IpAddress, cstring] = + let + publicAddress = TransportAddress( + family: AddressFamily.IPv4, address_v4: [1'u8, 1, 1, 1], port: Port(0) + ) + route = getBestRoute(publicAddress) -func isGlobalUnicast*(address: IpAddress): bool = - let a = initTAddress(address, Port(0)) - a.isGlobalUnicast() - -proc getRoute(publicAddress: TransportAddress): Result[IpAddress, cstring] = - let route = getBestRoute(publicAddress) - - if route.source.family == AddressFamily.None or route.source.isUnspecified(): - err("No best route found") + if route.source.isUnspecified(): + err("No best ipv4 route found") else: let ip = try: route.source.address() except ValueError as e: - # This should not occur really. error "Address conversion error", exception = e.name, msg = e.msg return err("Invalid IP address") ok(ip) -proc getRouteIpv4*(): Result[IpAddress, cstring] = - # Avoiding Exception with initTAddress and can't make it work with static. - # Note: `publicAddress` is only used an "example" IP to find the best route, - # no data is send over the network to this IP! - let publicAddress = TransportAddress( - family: AddressFamily.IPv4, address_v4: [1'u8, 1, 1, 1], port: Port(0) - ) - - return getRoute(publicAddress) - proc getRouteIpv6*(): Result[IpAddress, cstring] = - # Note: `googleDnsIpv6` is only used as an "example" IP to find the best route, - # no data is sent over the network to this IP! const googleDnsIpv6 = TransportAddress( family: AddressFamily.IPv6, - # 2001:4860:4860::8888 address_v6: [32'u8, 1, 72, 96, 72, 96, 0, 0, 0, 0, 0, 0, 0, 0, 136, 136], port: Port(0), ) + let route = getBestRoute(googleDnsIpv6) + if route.source.isUnspecified(): + return err("No best ipv6 route found") + try: + ok(route.source.address()) + except ValueError as e: + error "Address conversion error", exception = e.name, msg = e.msg + err("Invalid IP address") - return getRoute(googleDnsIpv6) - -# If bindIp is a anyLocal address (0.0.0.0 or ::), -# the function will find the best ip address. -# Otherwise, it will just return the ip as it is. proc getBestLocalAddress*(bindIp: IpAddress): Option[IpAddress] = + ## If bindIp is anyLocal (0.0.0.0 or ::), finds the best local IP via routing table. + ## Otherwise returns bindIp as-is. let bindAddress = initTAddress(bindIp, Port(0)) if bindAddress.isAnyLocal(): let ip = diff --git a/tests/integration/1_minute/testnat.nim b/tests/integration/1_minute/testnat.nim index 4aa13bce..50c0467d 100644 --- a/tests/integration/1_minute/testnat.nim +++ b/tests/integration/1_minute/testnat.nim @@ -1,6 +1,5 @@ import std/json import std/options -import std/sequtils import pkg/chronos import pkg/questionable/results diff --git a/tests/integration/storageconfig.nim b/tests/integration/storageconfig.nim index 240d44a2..691daf34 100644 --- a/tests/integration/storageconfig.nim +++ b/tests/integration/storageconfig.nim @@ -321,3 +321,11 @@ proc withNatScheduleInterval*( for config in startConfig.configs.mitems: config.addCliOption("--nat-schedule-interval", $scheduleInterval) return startConfig + +proc withExtIp*( + self: StorageConfigs, ip = "127.0.0.1" +): StorageConfigs {.raises: [StorageConfigError].} = + var startConfig = self + for config in startConfig.configs.mitems: + config.addCliOption("--nat", "extip:" & ip) + return startConfig diff --git a/tests/storage/blockexchange/discovery/testdiscovery.nim b/tests/storage/blockexchange/discovery/testdiscovery.nim index d467361e..471f9e88 100644 --- a/tests/storage/blockexchange/discovery/testdiscovery.nim +++ b/tests/storage/blockexchange/discovery/testdiscovery.nim @@ -1,6 +1,4 @@ -import std/sequtils -import std/sugar -import std/tables +import std/[sequtils, sugar, tables, options] import pkg/chronos diff --git a/tests/storage/testnatutils.nim b/tests/storage/testnatutils.nim index fcc48561..08a6621e 100644 --- a/tests/storage/testnatutils.nim +++ b/tests/storage/testnatutils.nim @@ -1,67 +1 @@ -import std/[unittest, net, options] -import pkg/chronos -import ../../storage/utils/natutils - -suite "isGlobalUnicast": - test "localhost IPv4 is not global unicast": - check not isGlobalUnicast(parseIpAddress("127.0.0.1")) - - test "unspecified IPv4 is not global unicast": - check not isGlobalUnicast(parseIpAddress("0.0.0.0")) - - test "link-local IPv4 is not global unicast": - check not isGlobalUnicast(parseIpAddress("169.254.1.1")) - - test "private IPv4 is not global unicast": - check not isGlobalUnicast(parseIpAddress("10.0.0.1")) - - test "public IPv4 is global unicast": - check isGlobalUnicast(parseIpAddress("8.8.8.8")) - - test "localhost IPv6 is not global unicast": - check not isGlobalUnicast(parseIpAddress("::1")) - - test "unspecified IPv6 is not global unicast": - check not isGlobalUnicast(parseIpAddress("::")) - - test "link-local IPv6 is not global unicast": - check not isGlobalUnicast(parseIpAddress("fe80::1")) - - test "private IPv6 is not global unicast": - check not isGlobalUnicast(parseIpAddress("fc00::1")) - - test "public IPv6 is global unicast": - check isGlobalUnicast(parseIpAddress("2606:4700::1")) - -suite "getRoute": - test "getRouteIpv4 returns a valid IPv4": - let res = getRouteIpv4() - - check res.isOk - check res.get().family == IpAddressFamily.IPv4 - - test "getRouteIpv6 returns a valid IPv6": - let res = getRouteIpv6() - # If the machine does not have a global route because - # it is not configured for IPv6, the test will fail - # because it didn't find the best route. In that case, - # we can just skip the test, because it is not a problem - # with the test itself but the machine configuration. - if res.isErr: - check res.error == "No best route found" - else: - check res.get().family == IpAddressFamily.IPv6 - -suite "getBestLocalAddress": - test "specific IPv4 is returned as it is": - let ip = parseIpAddress("192.168.1.1") - check getBestLocalAddress(ip) == some(ip) - - test "specific IPv6 is returned as it is": - let ip = parseIpAddress("2606:4700::1") - check getBestLocalAddress(ip) == some(ip) - - test "0.0.0.0 resolves to a local IPv4": - let res = getBestLocalAddress(parseIpAddress("0.0.0.0")) - check res.isSome - check res.get().family == IpAddressFamily.IPv4 +discard