From 6eb09fd6141f4c96dae3e1fe1a1f1946c91d0131 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 18 Apr 2025 18:12:46 -0400 Subject: [PATCH] test: Add unit test coverage for Init and Shutdown code Currently this code is not called in unit tests. Calling should make it possible to write tests for things like IPC exceptions being thrown during shutdown. --- src/common/args.cpp | 8 ++++++ src/common/args.h | 6 +--- src/net.cpp | 6 ++++ src/net.h | 1 + src/netbase.h | 31 ++++++++++++++------- src/rpc/server.cpp | 6 ++++ src/rpc/server.h | 1 + src/test/CMakeLists.txt | 1 + src/test/node_init_tests.cpp | 51 ++++++++++++++++++++++++++++++++++ src/test/util/setup_common.cpp | 11 ++++++++ 10 files changed, 107 insertions(+), 15 deletions(-) create mode 100644 src/test/node_init_tests.cpp diff --git a/src/common/args.cpp b/src/common/args.cpp index 71dcd1ac10c..6a79b6c6f17 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -589,6 +589,14 @@ void ArgsManager::AddHiddenArgs(const std::vector& names) } } +void ArgsManager::ClearArgs() +{ + LOCK(cs_args); + m_settings = {}; + m_available_args.clear(); + m_network_only_args.clear(); +} + void ArgsManager::CheckMultipleCLIArgs() const { LOCK(cs_args); diff --git a/src/common/args.h b/src/common/args.h index 6c5ac48ae3e..da19cbda66f 100644 --- a/src/common/args.h +++ b/src/common/args.h @@ -359,11 +359,7 @@ protected: /** * Clear available arguments */ - void ClearArgs() { - LOCK(cs_args); - m_available_args.clear(); - m_network_only_args.clear(); - } + void ClearArgs(); /** * Check CLI command args diff --git a/src/net.cpp b/src/net.cpp index 217d9a89037..77ea23e6657 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -266,6 +266,12 @@ std::optional GetLocalAddrForPeer(CNode& node) return std::nullopt; } +void ClearLocal() +{ + LOCK(g_maplocalhost_mutex); + return mapLocalHost.clear(); +} + // learn a new local address bool AddLocal(const CService& addr_, int nScore) { diff --git a/src/net.h b/src/net.h index 4cb4cb906e2..06bd7b1e875 100644 --- a/src/net.h +++ b/src/net.h @@ -158,6 +158,7 @@ enum /** Returns a local address that we should advertise to this peer. */ std::optional GetLocalAddrForPeer(CNode& node); +void ClearLocal(); bool AddLocal(const CService& addr, int nScore = LOCAL_NONE); bool AddLocal(const CNetAddr& addr, int nScore = LOCAL_NONE); void RemoveLocal(const CService& addr); diff --git a/src/netbase.h b/src/netbase.h index b2cc172e536..41b3ca8fdb0 100644 --- a/src/netbase.h +++ b/src/netbase.h @@ -121,6 +121,13 @@ public: m_reachable.clear(); } + void Reset() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + AssertLockNotHeld(m_mutex); + LOCK(m_mutex); + m_reachable = DefaultNets(); + } + [[nodiscard]] bool Contains(Network net) const EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { AssertLockNotHeld(m_mutex); @@ -142,17 +149,21 @@ public: } private: - mutable Mutex m_mutex; - - std::unordered_set m_reachable GUARDED_BY(m_mutex){ - NET_UNROUTABLE, - NET_IPV4, - NET_IPV6, - NET_ONION, - NET_I2P, - NET_CJDNS, - NET_INTERNAL + static std::unordered_set DefaultNets() + { + return { + NET_UNROUTABLE, + NET_IPV4, + NET_IPV6, + NET_ONION, + NET_I2P, + NET_CJDNS, + NET_INTERNAL + }; }; + + mutable Mutex m_mutex; + std::unordered_set m_reachable GUARDED_BY(m_mutex){DefaultNets()}; }; extern ReachableNets g_reachable_nets; diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 8631ae0adfe..722577cc218 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -327,6 +327,12 @@ void SetRPCWarmupStatus(const std::string& newStatus) rpcWarmupStatus = newStatus; } +void SetRPCWarmupStarting() +{ + LOCK(g_rpc_warmup_mutex); + fRPCInWarmup = true; +} + void SetRPCWarmupFinished() { LOCK(g_rpc_warmup_mutex); diff --git a/src/rpc/server.h b/src/rpc/server.h index d4b48f2418f..19bd54dfc4f 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -29,6 +29,7 @@ void RpcInterruptionPoint(); * immediately with RPC_IN_WARMUP. */ void SetRPCWarmupStatus(const std::string& newStatus); +void SetRPCWarmupStarting(); /* Mark warmup as done. RPC calls will be processed from now on. */ void SetRPCWarmupFinished(); diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 6ce33621af8..e891566bc1f 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -64,6 +64,7 @@ add_executable(test_bitcoin net_peer_eviction_tests.cpp net_tests.cpp netbase_tests.cpp + node_init_tests.cpp node_warnings_tests.cpp orphanage_tests.cpp pcp_tests.cpp diff --git a/src/test/node_init_tests.cpp b/src/test/node_init_tests.cpp new file mode 100644 index 00000000000..802e3176099 --- /dev/null +++ b/src/test/node_init_tests.cpp @@ -0,0 +1,51 @@ +// Copyright (c) 2025 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include + +#include +#include + +using node::NodeContext; + +BOOST_FIXTURE_TEST_SUITE(node_init_tests, BasicTestingSetup) + +//! Custom implementation of interfaces::Init for testing. +class TestInit : public interfaces::Init +{ +public: + TestInit(NodeContext& node) : m_node(node) + { + InitContext(m_node); + m_node.init = this; + } + std::unique_ptr makeChain() override { return interfaces::MakeChain(m_node); } + std::unique_ptr makeWalletLoader(interfaces::Chain& chain) override + { + return MakeWalletLoader(chain, *Assert(m_node.args)); + } + NodeContext& m_node; +}; + +BOOST_AUTO_TEST_CASE(init_test) +{ + // Clear state set by BasicTestingSetup that AppInitMain assumes is unset. + LogInstance().DisconnectTestLogger(); + m_node.args->SetConfigFilePath({}); + + // Prevent the test from trying to listen on ports 8332 and 8333. + m_node.args->ForceSetArg("-server", "0"); + m_node.args->ForceSetArg("-listen", "0"); + + // Run through initialization and shutdown code. + TestInit init{m_node}; + BOOST_CHECK(AppInitInterfaces(m_node)); + BOOST_CHECK(AppInitMain(m_node)); + Interrupt(m_node); + Shutdown(m_node); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 76a42d19ea2..f3ccab0580e 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -115,6 +115,14 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts) if (!EnableFuzzDeterminism()) { SeedRandomForTest(SeedRand::FIXED_SEED); } + + // Reset globals + fDiscover = true; + fListen = true; + SetRPCWarmupStarting(); + g_reachable_nets.Reset(); + ClearLocal(); + m_node.shutdown_signal = &m_interrupt; m_node.shutdown_request = [this]{ return m_interrupt(); }; m_node.args = &gArgs; @@ -214,7 +222,10 @@ BasicTestingSetup::~BasicTestingSetup() } else { fs::remove_all(m_path_root); } + // Clear all arguments except for -datadir, which GUI tests currently rely + // on to be set even after the testing setup is destroyed. gArgs.ClearArgs(); + gArgs.ForceSetArg("-datadir", fs::PathToString(m_path_root)); } ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)