Provide better testing and add separate upnp and pcp test on miniupnpd

.github/workflows/ci-reusable.yml

@@ -69,9 +69,13 @@ jobs:
         run: make -j${ncpu} testLibstorage
 
       ## Part 4 Tests ##
-      - name: NAT integration tests
-        if: matrix.tests == 'nat-integration'
-        run: make testNatIntegration
+      - name: NAT UPnP integration tests
+        if: matrix.tests == 'nat-upnp-integration'
+        run: make testNatUpnpIntegration
+
+      - name: NAT PCP integration tests
+        if: matrix.tests == 'nat-pcp-integration'
+        run: make testNatPcpIntegration
 
   status:
     if: always()

Makefile

@@ -87,7 +87,8 @@ endif
 	testAll \
 	testIntegration \
 	testLibstorage \
-	testNatIntegration \
+	testNatUpnpIntegration \
+	testNatPcpIntegration \
 	update
 
 ifeq ($(NIM_PARAMS),)
@@ -149,11 +150,15 @@ testIntegration: | build deps
 	echo -e $(BUILD_MSG) "build/$@" && \
 		$(ENV_SCRIPT) nim testIntegration $(TEST_PARAMS) $(NIM_PARAMS) build.nims
 
-# Builds and runs the UPnP NAT integration test inside a miniupnpd container
 DOCKER := $(or $(shell which podman 2>/dev/null), $(shell which docker 2>/dev/null))
-testNatIntegration:
+
+testNatUpnpIntegration:
 	$(DOCKER) build -t miniupnpd-test -f tests/integration/nat/Dockerfile .
-	$(DOCKER) run --rm --cap-add NET_ADMIN miniupnpd-test
+	$(DOCKER) run --rm --cap-add NET_ADMIN -e DEBUG=$(DEBUG) miniupnpd-test
+
+testNatPcpIntegration:
+	$(DOCKER) build -t miniupnpd-test -f tests/integration/nat/Dockerfile .
+	$(DOCKER) run --rm --cap-add NET_ADMIN -e DEBUG=$(DEBUG) -e TEST_PCP=1 miniupnpd-test
 
 # Builds a C example that uses the libstorage C library and runs it
 testLibstorage: | build deps

build.nims

@@ -85,13 +85,22 @@ task testNatPortMapping, "Run UPnP NAT integration test (requires miniupnpd cont
   putEnv("STORAGE_INTEGRATION_TEST_INCLUDES", "nat/testnatupnp.nim")
   test "testIntegration", outName = "testIntegrationNat"
 
-# Used to build the testing binarie in Docker
-task buildNatPortMappingBinaries, "Build UPnP NAT test binaries without running them":
+task testNatPcpMapping, "Run PCP NAT integration test (requires miniupnpd container)":
+  buildBinary "storage",
+    outName = "storage",
+    params = "-d:chronicles_runtime_filtering -d:chronicles_log_level=TRACE"
+  putEnv("STORAGE_INTEGRATION_TEST_INCLUDES", "nat/testnatpcp.nim")
+  test "testIntegration", outName = "testIntegrationNatPcp"
+
+# Used to build the testing binaries in Docker
+task buildNatPortMappingBinaries, "Build UPnP and PCP NAT test binaries without running them":
   buildBinary "storage",
     outName = "storage",
     params = "-d:chronicles_runtime_filtering -d:chronicles_log_level=TRACE"
   putEnv("STORAGE_INTEGRATION_TEST_INCLUDES", "nat/testnatupnp.nim")
   buildBinary "testIntegration", outName = "testIntegrationNat", srcDir = "tests/"
+  putEnv("STORAGE_INTEGRATION_TEST_INCLUDES", "nat/testnatpcp.nim")
+  buildBinary "testIntegration", outName = "testIntegrationNatPcp", srcDir = "tests/"
 
 task build, "build Logos Storage binary":
   storageTask()

tests/integration/1_minute/testnat.nim

@@ -24,7 +24,7 @@ multinodesuite "AutoNAT detection":
   )
   test "node is reachable when using bootstrap node on same network", natConfig:
     let node2 = clients()[1]
-    await node2.client.checkNatStatus("Reachable")
+    await node2.client.checkReachable()
 
   let endpointIndependentConfig = NodeConfigs(
     clients: StorageConfigs
@@ -39,7 +39,7 @@ multinodesuite "AutoNAT detection":
   # EIF = Endpoint Independent Filtering
   test "node with simulated EIF nat is detected as reachable", endpointIndependentConfig:
     let node2 = clients()[1]
-    await node2.client.checkNatStatus("Reachable")
+    await node2.client.checkReachable()
 
   let autonatConfig = NodeConfigs(
     clients: StorageConfigs
@@ -55,7 +55,7 @@ multinodesuite "AutoNAT detection":
   test "node with simulated APDF nat is detected as not reachable and starts relay",
     autonatConfig:
     let node2 = clients()[1]
-    await node2.client.checkNatStatus("NotReachable")
+    await node2.client.checkNotReachable()
 
   let transitionConfig = NodeConfigs(
     clients: StorageConfigs
@@ -73,11 +73,11 @@ multinodesuite "AutoNAT detection":
     transitionConfig:
     let node2 = clients()[1]
 
-    await node2.client.checkNatStatus("NotReachable")
+    await node2.client.checkNotReachable()
 
     check (await node2.client.setNatFiltering("endpoint-independent")).isOk
 
-    await node2.client.checkNatStatus("Reachable")
+    await node2.client.checkReachable()
 
   let natToSimConfig = NodeConfigs(
     clients: StorageConfigs
@@ -94,11 +94,26 @@ multinodesuite "AutoNAT detection":
     natToSimConfig:
     let node2 = clients()[1]
 
-    await node2.client.checkNatStatus("Reachable")
+    await node2.client.checkReachable()
 
     check (await node2.client.setNatFiltering("address-and-port-dependent")).isOk
 
-    await node2.client.checkNatStatus("NotReachable")
+    await node2.client.checkNotReachable()
+
+  let doubleNatConfig = NodeConfigs(
+    clients: StorageConfigs
+      .init(nodes = 2)
+      .withRelay(0)
+      .withNatSimulation(idx = 1, "double-nat")
+      .withNatNumPeersToAsk(1)
+      .withNatMinConfidence(0.5)
+      .withNatScheduleInterval(5.seconds)
+      .withNatMaxQueueSize(1).some
+  )
+  test "node behind double NAT is detected as not reachable and starts relay",
+    doubleNatConfig:
+    let node2 = clients()[1]
+    await node2.client.checkNotReachable()
 
   let multiNatConfig = NodeConfigs(
     clients: StorageConfigs
@@ -117,5 +132,5 @@ multinodesuite "AutoNAT detection":
     let node2 = clients()[1]
     let node3 = clients()[2]
 
-    await node2.client.checkNatStatus("NotReachable")
-    await node3.client.checkNatStatus("NotReachable")
+    await node2.client.checkNotReachable()
+    await node3.client.checkNotReachable()

tests/integration/multinodes.nim

@@ -215,7 +215,7 @@ template multinodesuite*(suiteName: string, body: untyped) =
         failAndTeardownOnError "failed to start client nodes":
           # Only the first node (bootstrap) gets a known extip. Other nodes use
           # nat=auto so AutoNAT can run and determine their reachability.
-          clients = clients.withExtIp(0)
+          clients = clients.withExtIp(0).withAutonatServer(0)
           for config in clients.configs:
             let node = await startClientNode(config)
             running.add RunningNode(role: Role.Client, node: node)

tests/integration/nat/Dockerfile

@@ -9,7 +9,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
     && rm -rf /var/lib/apt/lists/*
 
 # Build miniupnpd with stub redirector: no iptables/nftables needed, always
-# returns success for port mapping requests — safe for isolated test containers.
+# returns success for port mapping requests.
 COPY tests/integration/nat/miniupnpd_stub_rdr.c /tmp/stub_rdr.c
 RUN git clone --depth=1 --branch miniupnpd_2_3_9 \
         https://github.com/miniupnp/miniupnp.git /tmp/miniupnp \

tests/integration/nat/docker-entrypoint.sh

@@ -5,24 +5,41 @@ RUNDIR=/tmp/miniupnpd
 mkdir -p "$RUNDIR"
 
 LAN_IF=$(ip route show default | awk '/default/{print $5; exit}')
+LAN_IP=$(ip -4 addr show "$LAN_IF" | awk '/inet /{print $2; exit}' | cut -d/ -f1)
 
 ip link add plum-wan type dummy
 ip addr add 1.2.3.4/24 dev plum-wan
 ip link set plum-wan up
 
-cat > "$RUNDIR/miniupnpd.conf" << EOF
+start_miniupnpd() {
+  local enable_pcp_pmp=$1
+  cat > "$RUNDIR/miniupnpd.conf" << EOF
 ext_ifname=plum-wan
 listening_ip=$LAN_IF
-enable_pcp_pmp=no
+enable_pcp_pmp=$enable_pcp_pmp
 port=0
 allow 1024-65535 0.0.0.0/0 1024-65535
 EOF
+  miniupnpd -d -f "$RUNDIR/miniupnpd.conf" > "$RUNDIR/miniupnpd.log" 2>&1 &
+  sleep 1
+}
+
+if [[ "${TEST_PCP:-0}" == "1" ]]; then
+  # PCP requires the UDP source IP to match the client_address in the MAP request.
+  # Point the default route at LAN_IP so libplum uses it as both gateway and PCP target.
+  ip route replace default via "$LAN_IP" dev "$LAN_IF"
+  start_miniupnpd yes
+  failed=0
+  DEBUG=${DEBUG:-0} /app/build/testIntegrationNatPcp || failed=1
+else
+  start_miniupnpd no
+  failed=0
+  DEBUG=${DEBUG:-0} /app/build/testIntegrationNat || failed=1
+fi
 
 if [[ "${DEBUG:-0}" == "1" ]]; then
-  miniupnpd -d -f "$RUNDIR/miniupnpd.conf" &
-else
-  miniupnpd -d -f "$RUNDIR/miniupnpd.conf" > /dev/null 2>&1 &
+  echo "--- miniupnpd log ---"
+  cat "$RUNDIR/miniupnpd.log" 2>/dev/null || true
 fi
-sleep 1
 
-/app/build/testIntegrationNat
+[ $failed -eq 0 ] || exit 1

tests/integration/nathelper.nim

@@ -19,16 +19,31 @@ proc checkNatStatus*(
       let info = (await client.info()).get
       let nat = info["nat"]
       let addrs = info["addrs"].getElems.mapIt(it.getStr)
-      nat["reachability"].getStr() == reachability and
-        nat["clientMode"].getBool() == clientMode and
-        nat["relayRunning"].getBool() == relayRunning and
-        addrs.anyIt("p2p-circuit" in it) == relayRunning,
+      let r = nat["reachability"].getStr()
+      let cm = nat["clientMode"].getBool()
+      let rr = nat["relayRunning"].getBool()
+      let ha = addrs.anyIt("p2p-circuit" in it)
+      let pm = nat["portMapping"].getStr()
+
+      # It is important to check all the conditions together to avoid race
+      # (new autonat iteration)
+      # So we add a checkoint for better debug in case of failures
+      checkpoint(
+        "reachability=" & r & " (want " & reachability & ")" & " clientMode=" & $cm &
+          " (want " & $clientMode & ")" & " relayRunning=" & $rr & " (want " &
+          $relayRunning & ")" & " p2p-circuit=" & $ha & " (want " & $relayRunning & ")" &
+          " portMapping=" & pm
+      )
+      r == reachability and cm == clientMode and rr == relayRunning and
+        ha == relayRunning,
     timeout = RelayTimeout,
     pollInterval = PollInterval,
   )
 
-proc checkNatStatus*(client: StorageClient, reachability: string) {.async.} =
-  let notReachable = reachability == "NotReachable"
+proc checkReachable*(client: StorageClient) {.async.} =
+  await client.checkNatStatus("Reachable", relayRunning = false, clientMode = false)
+
+proc checkNotReachable*(client: StorageClient, relayRunning = true) {.async.} =
   await client.checkNatStatus(
-    reachability, relayRunning = notReachable, clientMode = notReachable
+    "NotReachable", relayRunning = relayRunning, clientMode = true
   )

tests/integration/storageconfig.nim

@@ -282,22 +282,6 @@ proc withStorageQuota*(
     config.addCliOption("--storage-quota", $quota)
   return startConfig
 
-proc withListenIp*(
-    self: StorageConfigs, ip: string
-): StorageConfigs {.raises: [StorageConfigError].} =
-  var startConfig = self
-  for config in startConfig.configs.mitems:
-    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].} =
@@ -330,30 +314,6 @@ proc withNatScheduleInterval*(
     config.addCliOption("--nat-schedule-interval", $scheduleInterval)
   return startConfig
 
-proc withNatPortMappingDiscoverTimeout*(
-    self: StorageConfigs, timeout: int
-): StorageConfigs {.raises: [StorageConfigError].} =
-  var startConfig = self
-  for config in startConfig.configs.mitems:
-    config.addCliOption("--nat-port-mapping-discover-timeout", $timeout)
-  return startConfig
-
-proc withNatPortMappingTimeout*(
-    self: StorageConfigs, timeout: int
-): StorageConfigs {.raises: [StorageConfigError].} =
-  var startConfig = self
-  for config in startConfig.configs.mitems:
-    config.addCliOption("--nat-port-mapping-timeout", $timeout)
-  return startConfig
-
-proc withNatPortMappingRecheckPeriod*(
-    self: StorageConfigs, timeout: int
-): StorageConfigs {.raises: [StorageConfigError].} =
-  var startConfig = self
-  for config in startConfig.configs.mitems:
-    config.addCliOption("--nat-port-mapping-recheck-period", $timeout)
-  return startConfig
-
 proc withExtIp*(
     self: StorageConfigs, idx: int, ip = "127.0.0.1"
 ): StorageConfigs {.raises: [StorageConfigError].} =
@@ -363,27 +323,13 @@ proc withExtIp*(
   startConfig.configs[idx].addCliOption("--nat", "extip:" & ip)
   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
-
 proc withRelay*(
     self: StorageConfigs, idx: int
 ): StorageConfigs {.raises: [StorageConfigError].} =
   self.checkBounds idx
 
   var startConfig = self
-  startConfig.configs[idx].addCliOption("--relay")
-  return startConfig
-
-proc withRelay*(self: StorageConfigs): StorageConfigs {.raises: [StorageConfigError].} =
-  var startConfig = self
-  for config in startConfig.configs.mitems:
-    config.addCliOption("--relay")
+  startConfig.configs[idx].addCliOption("--relay-server")
   return startConfig
 
 # For testing, a node with extip (not behind nat) is a bootstrap node
@@ -408,10 +354,10 @@ proc withNatSimulation*(
   startConfig.configs[idx].addCliOption("--nat-simulation", filtering)
   return startConfig
 
-proc withNatSimulation*(
-    self: StorageConfigs, filtering: string
+proc withAutonatServer*(
+    self: StorageConfigs, idx: int
 ): StorageConfigs {.raises: [StorageConfigError].} =
+  self.checkBounds idx
   var startConfig = self
-  for config in startConfig.configs.mitems:
-    config.addCliOption("--nat-simulation", filtering)
+  startConfig.configs[idx].addCliOption("--autonat-server")
   return startConfig

tests/nat/testnatpcp.nim

@@ -0,0 +1,90 @@
+import std/[json, strutils, sequtils]
+import pkg/chronos
+import pkg/questionable/results
+
+import ../integration/multinodes
+import ../integration/storageclient
+import ../integration/storageconfig
+
+import ../integration/nathelper
+
+multinodesuite "AutoNAT PCP port mapping":
+  let pcpConfig = NodeConfigs(
+    clients: StorageConfigs
+      .init(nodes = 2)
+      .withRelay(0)
+      .withNatSimulation(idx = 1, "address-and-port-dependent")
+      .withNatNumPeersToAsk(1)
+      .withNatMinConfidence(0.5)
+      .withNatScheduleInterval(10.seconds)
+      .withNatMaxQueueSize(1).some
+  )
+
+  test "node behind NAT maps ports via PCP and exposes mapping in debug info", pcpConfig:
+    let node2 = clients()[1]
+
+    await node2.client.checkNotReachable(relayRunning = false)
+
+    check eventuallySafe(
+      block:
+        let res = await node2.client.natPortMapping()
+        res.isOk and res.get == "pcp",
+      timeout = RelayTimeout,
+      pollInterval = PollInterval,
+    )
+
+    await node2.client.checkReachable()
+
+    await node2.stop()
+
+  let relayFallbackConfig = NodeConfigs(
+    clients: StorageConfigs
+      .init(nodes = 2)
+      .withRelay(0)
+      .withNatSimulation(idx = 1, "double-nat")
+      .withNatNumPeersToAsk(1)
+      .withNatMinConfidence(0.5)
+      .withNatScheduleInterval(10.seconds)
+      # Increase the max queue to trigger the AutoNat 2 times
+      .withNatMaxQueueSize(2).some
+  )
+
+  test "node behind double NAT falls back to relay after PCP mapping does not help",
+    relayFallbackConfig:
+    let node2 = clients()[1]
+
+    await node2.client.checkNotReachable(relayRunning = false)
+
+    check eventuallySafe(
+      block:
+        let res = await node2.client.natPortMapping()
+        res.isOk and res.get == "pcp",
+      timeout = RelayTimeout,
+      pollInterval = PollInterval,
+    )
+
+    await node2.client.checkNotReachable()
+
+  test "reachable node downloads content uploaded by node behind NAT after PCP mapping",
+    pcpConfig:
+    let node1 = clients()[0]
+    let node2 = clients()[1]
+
+    check eventuallySafe(
+      block:
+        let res = await node2.client.natPortMapping()
+        res.isOk and res.get == "pcp",
+      timeout = RelayTimeout,
+      pollInterval = PollInterval,
+    )
+
+    let content = "content uploaded by nat node"
+    let cid = (await node2.client.upload(content)).get
+
+    check eventuallySafe(
+      (await node1.client.download(cid)).isOk,
+      timeout = RelayTimeout,
+      pollInterval = PollInterval,
+    )
+
+    check (await node1.client.download(cid)).get == content

tests/nat/testnatupnp.nim

@@ -8,30 +8,23 @@ import ../integration/storageconfig
 
 import ../integration/nathelper
 
-const DetectionTimeout = 15_000
-
 multinodesuite "AutoNAT UPnP port mapping":
   let upnpConfig = NodeConfigs(
     clients: StorageConfigs
       .init(nodes = 2)
       .withRelay(0)
       .withNatSimulation(idx = 1, "address-and-port-dependent")
-      .withListenPort(idx = 1, 8102)
       .withNatNumPeersToAsk(1)
       .withNatMinConfidence(0.5)
       .withNatScheduleInterval(10.seconds)
-      .withNatMaxQueueSize(1)
-      .withLogFile()
-      .withLogLevel(idx = 1, LogLevel.DEBUG).some
+      .withNatMaxQueueSize(1).some
   )
 
   test "node behind NAT maps ports via UPnP and exposes mapping in debug info",
     upnpConfig:
     let node2 = clients()[1]
 
-    await node2.client.checkNatStatus(
-      "NotReachable", relayRunning = false, clientMode = true
-    )
+    await node2.client.checkNotReachable(relayRunning = false)
 
     check eventuallySafe(
       block:
@@ -41,13 +34,58 @@ multinodesuite "AutoNAT UPnP port mapping":
       pollInterval = PollInterval,
     )
 
-    await node2.client.checkNatStatus(
-      "NotReachable", relayRunning = false, clientMode = true
-    )
-
-    let announceAddrs =
-      (await node2.client.info()).get["announceAddresses"].getElems.mapIt(it.getStr)
-    let tcpAddr = announceAddrs.filterIt(it.startsWith("/ip4/") and "/tcp/" in it)
-    check tcpAddr.len > 0
+    await node2.client.checkReachable()
 
     await node2.stop()
+
+  let relayFallbackConfig = NodeConfigs(
+    clients: StorageConfigs
+      .init(nodes = 2)
+      .withRelay(0)
+      .withNatSimulation(idx = 1, "double-nat")
+      .withNatNumPeersToAsk(1)
+      .withNatMinConfidence(0.5)
+      .withNatScheduleInterval(10.seconds)
+      # Increase the max queue to trigger the AutoNat 2 times
+      .withNatMaxQueueSize(2).some
+  )
+
+  test "node behind double NAT falls back to relay after UPnP mapping does not help",
+    relayFallbackConfig:
+    let node2 = clients()[1]
+
+    await node2.client.checkNotReachable(relayRunning = false)
+
+    check eventuallySafe(
+      block:
+        let res = await node2.client.natPortMapping()
+        res.isOk and res.get == "upnp",
+      timeout = RelayTimeout,
+      pollInterval = PollInterval,
+    )
+
+    await node2.client.checkNotReachable()
+
+  test "reachable node downloads content uploaded by node behind NAT after UPnP mapping",
+    upnpConfig:
+    let node1 = clients()[0]
+    let node2 = clients()[1]
+
+    check eventuallySafe(
+      block:
+        let res = await node2.client.natPortMapping()
+        res.isOk and res.get == "upnp",
+      timeout = RelayTimeout,
+      pollInterval = PollInterval,
+    )
+
+    let content = "content uploaded by nat node"
+    let cid = (await node2.client.upload(content)).get
+
+    check eventuallySafe(
+      (await node1.client.download(cid)).isOk,
+      timeout = RelayTimeout,
+      pollInterval = PollInterval,
+    )
+
+    check (await node1.client.download(cid)).get == content

tests/storage/testaddrutils.nim

@@ -3,6 +3,23 @@ import pkg/libp2p/multiaddress
 import ../asynctest
 import ../../storage/utils/addrutils
 
+suite "addrutils - getTcpPort":
+  test "extracts port from ipv4 tcp address":
+    let ma = MultiAddress.init("/ip4/1.2.3.4/tcp/5000").expect("valid")
+    check getTcpPort(ma) == some(Port(5000))
+
+  test "extracts port from ipv6 tcp address":
+    let ma = MultiAddress.init("/ip6/::1/tcp/8080").expect("valid")
+    check getTcpPort(ma) == some(Port(8080))
+
+  test "returns none for udp address":
+    let ma = MultiAddress.init("/ip4/1.2.3.4/udp/5000").expect("valid")
+    check getTcpPort(ma) == Port.none
+
+  test "extracts port 0":
+    let ma = MultiAddress.init("/ip4/0.0.0.0/tcp/0").expect("valid")
+    check getTcpPort(ma) == some(Port(0))
+
 suite "addrutils - remapAddr":
   test "replaces protocol tcp with udp":
     let ma = MultiAddress.init("/ip4/1.2.3.4/tcp/5000").expect("valid")

tests/storage/testnat.nim

@@ -14,11 +14,11 @@ import ../../storage/discovery
 import ../../storage/rng
 import ../../storage/utils
 
-type MockNatMapper = ref object of NatMapper
+type MockNatPortMapper = ref object of NatPortMapper
   mappedPorts: Option[(Port, Port, MappingProtocol)]
 
 method mapNatPorts*(
-    m: MockNatMapper
+    m: MockNatPortMapper
 ): Future[Option[(Port, Port, MappingProtocol)]] {.
     async: (raises: [CancelledError]), gcsafe
 .} =
@@ -49,7 +49,7 @@ asyncchecksuite "NAT - handleNatStatus":
   test "handleNatStatus announces mapped address when NotReachable and UPnP succeeds":
     let dialBack = MultiAddress.init("/ip4/1.2.3.4/tcp/8080").expect("valid")
     let mapper =
-      MockNatMapper(mappedPorts: some((Port(9000), Port(9001), MappingProtocol.UPnP)))
+      MockNatPortMapper(mappedPorts: some((Port(9000), Port(9001), MappingProtocol.UPnP)))
 
     await mapper.handleNatStatus(
       NotReachable, Opt.some(dialBack), discoveryPort, disc, sw, autoRelay
@@ -61,7 +61,7 @@ asyncchecksuite "NAT - handleNatStatus":
     check disc.protocol.clientMode
 
   test "handleNatStatus starts autoRelay when NotReachable and UPnP failed":
-    let mapper = MockNatMapper(mappedPorts: none((Port, Port, MappingProtocol)))
+    let mapper = MockNatPortMapper(mappedPorts: none((Port, Port, MappingProtocol)))
 
     await mapper.handleNatStatus(
       NotReachable, Opt.none(MultiAddress), discoveryPort, disc, sw, autoRelay
@@ -72,7 +72,7 @@ asyncchecksuite "NAT - handleNatStatus":
 
   test "handleNatStatus starts autoRelay when NotReachable and mapping fails":
     let dialBack = MultiAddress.init("/ip4/1.2.3.4/tcp/8080").expect("valid")
-    let mapper = MockNatMapper(mappedPorts: none((Port, Port, MappingProtocol)))
+    let mapper = MockNatPortMapper(mappedPorts: none((Port, Port, MappingProtocol)))
 
     await mapper.handleNatStatus(
       NotReachable, Opt.some(dialBack), discoveryPort, disc, sw, autoRelay
@@ -83,7 +83,7 @@ asyncchecksuite "NAT - handleNatStatus":
     check disc.protocol.clientMode
 
   test "handleNatStatus does not announce address when Reachable and no dialBackAddr":
-    let mapper = MockNatMapper(mappedPorts: none((Port, Port, MappingProtocol)))
+    let mapper = MockNatPortMapper(mappedPorts: none((Port, Port, MappingProtocol)))
 
     await mapper.handleNatStatus(
       Reachable, Opt.none(MultiAddress), discoveryPort, disc, sw, autoRelay
@@ -95,7 +95,7 @@ asyncchecksuite "NAT - handleNatStatus":
 
   test "handleNatStatus stops relay and announces dialBackAddr when Reachable":
     let dialBack = MultiAddress.init("/ip4/1.2.3.4/tcp/8080").expect("valid")
-    let mapper = MockNatMapper(mappedPorts: none((Port, Port, MappingProtocol)))
+    let mapper = MockNatPortMapper(mappedPorts: none((Port, Port, MappingProtocol)))
 
     discard await autorelayservice.setup(autoRelay, sw)
     await mapper.handleNatStatus(

tests/storage/testnatsimulation.nim

@@ -1,8 +1,11 @@
+import std/net
 import pkg/chronos
+import pkg/libp2p/wire
 
 import ./helpers
 import ../asynctest
 import ../../storage/rng
+import ../../storage/nat
 import ../../storage/utils/natsimulation
 
 const flags = {ServerFlags.ReuseAddr}
@@ -52,7 +55,7 @@ asyncchecksuite "NatTransport - Address-Dependent Filtering":
   var bootstrap, thirdNode, natNode: Switch
 
   setup:
-    let router = NatRouter.new(AddressDependent)
+    let router = NatRouter.new(AddressDependent, dropTimeout = 1.seconds)
     bootstrap = newSwitch(Rng.instance())
     thirdNode = newSwitch(Rng.instance())
     natNode = newNatSwitch(router, Rng.instance())
@@ -85,7 +88,7 @@ asyncchecksuite "NatTransport - Address-and-Port-Dependent Filtering":
   var bootstrap, thirdNode, natNode: Switch
 
   setup:
-    let router = NatRouter.new(AddressAndPortDependent)
+    let router = NatRouter.new(AddressAndPortDependent, dropTimeout = 1.seconds)
     bootstrap = newSwitch(Rng.instance())
     thirdNode = newSwitch(Rng.instance())
     natNode = newNatSwitch(router, Rng.instance())
@@ -113,3 +116,59 @@ asyncchecksuite "NatTransport - Address-and-Port-Dependent Filtering":
     await natNode.connect(bootstrap.peerInfo.peerId, bootstrap.peerInfo.addrs)
     expect(LPError):
       await thirdNode.connect(natNode.peerInfo.peerId, natNode.peerInfo.addrs)
+
+asyncchecksuite "NatTransport - Double NAT":
+  var bootstrap, natNode: Switch
+  var router: NatRouter
+
+  setup:
+    router = NatRouter.new(DoubleNat, dropTimeout = 1.seconds)
+    bootstrap = newSwitch(Rng.instance())
+    natNode = newNatSwitch(router, Rng.instance())
+    await bootstrap.start()
+    await natNode.start()
+
+  teardown:
+    await bootstrap.stop()
+    await natNode.stop()
+
+  test "bootstrap cannot connect to nat node regardless of port mapping":
+    let actualPort = initTAddress(natNode.peerInfo.addrs[0]).get().port
+    let natMapper = NatPortMapper()
+    natMapper.activeTcpPort = some(actualPort)
+    router.natMapper = some(natMapper)
+
+    expect(LPError):
+      await bootstrap.connect(natNode.peerInfo.peerId, natNode.peerInfo.addrs)
+
+asyncchecksuite "NatTransport - Port Mapping":
+  var bootstrap, natNode: Switch
+  var router: NatRouter
+
+  setup:
+    router = NatRouter.new(AddressAndPortDependent, dropTimeout = 1.seconds)
+    bootstrap = newSwitch(Rng.instance())
+    natNode = newNatSwitch(router, Rng.instance())
+    await bootstrap.start()
+    await natNode.start()
+
+  teardown:
+    await bootstrap.stop()
+    await natNode.stop()
+
+  test "bootstrap can connect to nat node when port mapping matches listen port":
+    let actualPort = initTAddress(natNode.peerInfo.addrs[0]).get().port
+    let natMapper = NatPortMapper()
+    natMapper.activeTcpPort = some(actualPort)
+    router.natMapper = some(natMapper)
+
+    await bootstrap.connect(natNode.peerInfo.peerId, natNode.peerInfo.addrs)
+    check bootstrap.isConnected(natNode.peerInfo.peerId)
+
+  test "bootstrap cannot connect to nat node when port mapping does not match":
+    let natMapper = NatPortMapper()
+    natMapper.activeTcpPort = some(Port(1))
+    router.natMapper = some(natMapper)
+
+    expect(LPError):
+      await bootstrap.connect(natNode.peerInfo.peerId, natNode.peerInfo.addrs)

tools/scripts/ci-job-matrix.sh

@@ -117,10 +117,13 @@ libstorage_test () {
   job
 }
 
-# outputs a NAT integration test job
+# outputs NAT integration test jobs
 # Linux-only: miniupnpd is a Linux daemon, network namespace manipulation requires Linux
-nat_integration_test () {
-  job_tests="nat-integration"
+nat_integration_tests () {
+  job_tests="nat-upnp-integration"
+  job_includes=""
+  job
+  job_tests="nat-pcp-integration"
   job_includes=""
   job
 }
@@ -131,7 +134,7 @@ all_tests () {
   integration_test
   libstorage_test
   if [ "$job_os" = "linux" ]; then
-    nat_integration_test
+    nat_integration_tests
   fi
 }