Update & improve coverage: add mapport tests

cmake/script/CoverageInclude.cmake.in

@@ -41,6 +41,7 @@ list(APPEND LCOV_FILTER_COMMAND -p "src/crypto/ctaes")
 list(APPEND LCOV_FILTER_COMMAND -p "src/minisketch")
 list(APPEND LCOV_FILTER_COMMAND -p "src/secp256k1")
 list(APPEND LCOV_FILTER_COMMAND -p "depends")
+list(APPEND LCOV_FILTER_COMMAND -p "src/mapport_hooks.h")
 
 execute_process(
   COMMAND ${LCOV_COMMAND} --capture --initial --directory src --output-file baseline.info

src/mapport.cpp

@@ -17,6 +17,7 @@
 #include <random.h>
 #include <util/thread.h>
 #include <util/threadinterrupt.h>
+#include <mapport_hooks.h>
 
 #ifdef USE_UPNP
 #include <cstddef>  // workaround missing include in miniupnpc 2.3.3
@@ -35,6 +36,7 @@ static_assert(MINIUPNPC_API_VERSION >= 17, "miniUPnPc API version >= 17 assumed"
 #include <string>
 #include <thread>
 
+
 static CThreadInterrupt g_mapport_interrupt;
 static std::thread g_mapport_thread;
 static std::atomic_uint g_mapport_enabled_protos{MapPortProtoFlag::NONE};
@@ -79,7 +81,7 @@ static bool ProcessPCP()
         ret = false; // Set to true if any mapping succeeds.
 
         // IPv4
-        std::optional<CNetAddr> gateway4 = QueryDefaultGateway(NET_IPV4);
+        std::optional<CNetAddr> gateway4 = mapport_hooks::QueryDefaultGatewayFn(NET_IPV4);
         if (!gateway4) {
             LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Could not determine IPv4 default gateway\n");
         } else {
@@ -88,26 +90,26 @@ static bool ProcessPCP()
             // Open a port mapping on whatever local address we have toward the gateway.
             struct in_addr inaddr_any;
             inaddr_any.s_addr = htonl(INADDR_ANY);
-            auto res = PCPRequestPortMap(pcp_nonce, *gateway4, CNetAddr(inaddr_any), private_port, requested_lifetime, g_mapport_interrupt);
+            auto res = mapport_hooks::PCPRequestPortMapFn(pcp_nonce, *gateway4, CNetAddr(inaddr_any), private_port, requested_lifetime, g_mapport_interrupt);
             MappingError* pcp_err = std::get_if<MappingError>(&res);
             if (pcp_err && *pcp_err == MappingError::UNSUPP_VERSION) {
                 LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Got unsupported PCP version response, falling back to NAT-PMP\n");
-                res = NATPMPRequestPortMap(*gateway4, private_port, requested_lifetime, g_mapport_interrupt);
+                res = mapport_hooks::NATPMPRequestPortMapFn(*gateway4, private_port, requested_lifetime, g_mapport_interrupt);
             }
             handle_mapping(res);
         }
 
         // IPv6
-        std::optional<CNetAddr> gateway6 = QueryDefaultGateway(NET_IPV6);
+        std::optional<CNetAddr> gateway6 = mapport_hooks::QueryDefaultGatewayFn(NET_IPV6);
         if (!gateway6) {
             LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: Could not determine IPv6 default gateway\n");
         } else {
             LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "portmap: gateway [IPv6]: %s\n", gateway6->ToStringAddr());
 
             // Try to open pinholes for all routable local IPv6 addresses.
-            for (const auto &addr: GetLocalAddresses()) {
+            for (const auto &addr: mapport_hooks::GetLocalAddressesFn()) {
                 if (!addr.IsRoutable() || !addr.IsIPv6()) continue;
-                auto res = PCPRequestPortMap(pcp_nonce, *gateway6, addr, private_port, requested_lifetime, g_mapport_interrupt);
+                auto res = mapport_hooks::PCPRequestPortMapFn(pcp_nonce, *gateway6, addr, private_port, requested_lifetime, g_mapport_interrupt);
                 handle_mapping(res);
             }
         }

src/mapport_hooks.h

@@ -0,0 +1,40 @@
+// Copyright (c) 2025 The Bitcoin Knots developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#pragma once
+
+#include <common/netif.h>
+#include <common/pcp.h>
+#include <netaddress.h>
+#include <util/threadinterrupt.h>
+
+#include <optional>
+#include <variant>
+#include <vector>
+
+// Lightweight indirection hooks for mapport's network-dependent helpers.
+// These function-pointer hooks default to the real implementations in
+// production, and can be reassigned by unit tests at runtime to inject
+// deterministic behavior without performing any real network I/O.
+namespace mapport_hooks {
+using QueryDefaultGateway_t = std::optional<CNetAddr>(*)(Network);
+using PCPRequestPortMap_t   = std::variant<MappingResult, MappingError>(*)(
+    const PCPMappingNonce&, const CNetAddr&, const CNetAddr&, uint16_t, uint32_t, CThreadInterrupt&);
+using NATPMPRequestPortMap_t = std::variant<MappingResult, MappingError>(*)(
+    const CNetAddr&, uint16_t, uint32_t, CThreadInterrupt&);
+using GetLocalAddresses_t = std::vector<CNetAddr>(*)();
+
+// Defaults point to the real implementations. Tests may override these at runtime.
+inline QueryDefaultGateway_t QueryDefaultGatewayFn = QueryDefaultGateway;
+inline PCPRequestPortMap_t   PCPRequestPortMapFn = [](const PCPMappingNonce& nonce, const CNetAddr& gateway,
+                                                      const CNetAddr& bind, uint16_t port, uint32_t lifetime,
+                                                      CThreadInterrupt& interrupt) {
+    return PCPRequestPortMap(nonce, gateway, bind, port, lifetime, interrupt);
+};
+inline NATPMPRequestPortMap_t NATPMPRequestPortMapFn = [](const CNetAddr& gateway, uint16_t port,
+                                                          uint32_t lifetime, CThreadInterrupt& interrupt) {
+    return NATPMPRequestPortMap(gateway, port, lifetime, interrupt);
+};
+inline GetLocalAddresses_t GetLocalAddressesFn = [](){ return GetLocalAddresses(); };
+} // namespace mapport_hooks

src/mapport_testing.h

@@ -0,0 +1,12 @@
+// Copyright (c) 2025 The Bitcoin Knots developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#pragma once
+
+// Backward-compatibility header. Prefer including <mapport_hooks.h> directly.
+// This header provides a namespace alias so existing includes continue to work
+// without referencing the term "testing" in production symbols.
+#include <mapport_hooks.h>
+
+namespace mapport_testing = mapport_hooks;

src/test/CMakeLists.txt

@@ -102,6 +102,7 @@ add_executable(test_bitcoin
   system_tests.cpp
   timeoffsets_tests.cpp
   torcontrol_tests.cpp
+  mapport_tests.cpp
   transaction_tests.cpp
   translation_tests.cpp
   txdownload_tests.cpp

src/test/mapport_tests.cpp

@@ -0,0 +1,150 @@
+// Copyright (c) 2025 The Bitcoin Knots developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <boost/test/unit_test.hpp>
+
+#include <mapport.h>
+#include <chrono>
+#include <thread>
+#include <test/util/setup_common.h>
+#include <netaddress.h>
+#include <netbase.h>
+#include <util/threadinterrupt.h>
+#include <mapport_hooks.h>
+
+// Simple stub implementations matching the hook signatures (no network IO)
+static std::optional<CNetAddr> StubNoGateway(Network) { return std::nullopt; }
+static std::vector<CNetAddr> StubNoLocalAddrs() { return {}; }
+static std::variant<MappingResult, MappingError> StubPCPNoResources(
+    const PCPMappingNonce&, const CNetAddr&, const CNetAddr&, uint16_t, uint32_t, CThreadInterrupt&)
+{
+    return MappingError{MappingError::NO_RESOURCES};
+}
+static std::variant<MappingResult, MappingError> StubPMPNoResources(
+    const CNetAddr&, uint16_t, uint32_t, CThreadInterrupt&)
+{
+    return MappingError{MappingError::NO_RESOURCES};
+}
+
+struct MapportStubGuard {
+    // Save originals
+    decltype(mapport_hooks::QueryDefaultGatewayFn)  qdg_orig = mapport_hooks::QueryDefaultGatewayFn;
+    decltype(mapport_hooks::PCPRequestPortMapFn)    pcp_orig = mapport_hooks::PCPRequestPortMapFn;
+    decltype(mapport_hooks::NATPMPRequestPortMapFn) pmp_orig = mapport_hooks::NATPMPRequestPortMapFn;
+    decltype(mapport_hooks::GetLocalAddressesFn)    gla_orig = mapport_hooks::GetLocalAddressesFn;
+    ~MapportStubGuard() {
+        mapport_hooks::QueryDefaultGatewayFn  = qdg_orig;
+        mapport_hooks::PCPRequestPortMapFn    = pcp_orig;
+        mapport_hooks::NATPMPRequestPortMapFn = pmp_orig;
+        mapport_hooks::GetLocalAddressesFn    = gla_orig;
+    }
+};
+
+// These tests intentionally avoid enabling any mapping protocol in order to
+// exercise the control flow in mapport.cpp without performing any real
+// network operations. We rely on the fact that UPnP support is not compiled
+// in this build (USE_UPNP undefined). Enabling UPnP will still start the
+// mapport thread, but it won't attempt any UPnP work; we immediately
+// interrupt to keep the test fast and deterministic.
+
+BOOST_FIXTURE_TEST_SUITE(mapport_tests, BasicTestingSetup)
+
+BOOST_AUTO_TEST_CASE(start_stop_no_protocols)
+{
+    // Starting with both protocols disabled should be a quick no-op.
+    StartMapPort(false, false);
+
+    // Interrupt/Stop should be safe even if the thread was never started.
+    InterruptMapPort();
+    StopMapPort();
+
+    // Repeat to ensure idempotency of the stop path.
+    InterruptMapPort();
+    StopMapPort();
+}
+
+BOOST_AUTO_TEST_CASE(start_thread_with_upnp_only_then_interrupt)
+{
+    // Start the background thread by enabling only UPnP (no actual UPnP code
+    // will run in this build). Immediately interrupt and stop it to exercise
+    // ThreadMapPort(), StartThreadMapPort(), InterruptMapPort(), and StopMapPort().
+    StartMapPort(true, false);
+
+    // Give the thread a tiny slice to start. Not strictly necessary, but helps
+    // stabilize coverage across machines.
+    std::this_thread::sleep_for(std::chrono::milliseconds(1));
+
+    InterruptMapPort();
+    StopMapPort();
+
+    // Calling stop again should be a no-op.
+    StopMapPort();
+}
+
+BOOST_AUTO_TEST_CASE(repeated_start_calls_are_idempotent)
+{
+    // Start once with UPnP only, then start again. The second call should not
+    // start a second thread. Interrupt/stop will terminate the single thread.
+    StartMapPort(true, false);
+    StartMapPort(true, false);
+
+    InterruptMapPort();
+    StopMapPort();
+}
+
+BOOST_AUTO_TEST_CASE(toggle_enable_disable_sequences)
+{
+    // Sequence of toggles to exercise DispatchMapPort paths where
+    // current==NONE and enabled toggles between NONE and non-NONE.
+    StartMapPort(false, false); // No thread should be started.
+    StartMapPort(true, false);  // Start thread (UPnP-only path).
+
+    // Disable again while thread may be running. The thread will only exit
+    // after interrupt/stop.
+    StartMapPort(false, false);
+    InterruptMapPort();
+    StopMapPort();
+
+    // Final sanity: calls are safe repeatedly.
+    InterruptMapPort();
+    StopMapPort();
+}
+
+BOOST_AUTO_TEST_CASE(start_with_pcp_only_then_interrupt)
+{
+    // Avoid any real network. Force no default gateways and no local addresses.
+    MapportStubGuard guard;
+    mapport_hooks::QueryDefaultGatewayFn = &StubNoGateway;
+    mapport_hooks::GetLocalAddressesFn = &StubNoLocalAddrs;
+    mapport_hooks::PCPRequestPortMapFn = &StubPCPNoResources;
+    mapport_hooks::NATPMPRequestPortMapFn = &StubPMPNoResources;
+
+    StartMapPort(false, true);
+    std::this_thread::sleep_for(std::chrono::milliseconds(1));
+    InterruptMapPort();
+    StopMapPort();
+}
+
+BOOST_AUTO_TEST_CASE(enabling_another_protocol_does_not_switch)
+{
+    // Avoid network: no gateways so PCP does nothing.
+    MapportStubGuard guard;
+    mapport_hooks::QueryDefaultGatewayFn = [](Network){ return std::optional<CNetAddr>{}; };
+    mapport_hooks::GetLocalAddressesFn = [](){ return std::vector<CNetAddr>{}; };
+
+    // Start with PCP only, then enable UPnP in addition. According to
+    // DispatchMapPort(), enabling another protocol does not switch away from
+    // the currently used one; the dispatch should early-return.
+    StartMapPort(false, true); // Start PCP path.
+    std::this_thread::sleep_for(std::chrono::milliseconds(1));
+
+    // Enable UPnP while PCP is active.
+    StartMapPort(true, true);
+
+    // Clean up.
+    InterruptMapPort();
+    StopMapPort();
+}
+
+BOOST_AUTO_TEST_SUITE_END()