From 8be54e3b19677b02e19d054a4a5b2f1968bb1c46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 11 Jan 2026 23:57:55 +0100 Subject: [PATCH 1/7] test: cover IBD exit conditions Add a unit test that exercises the `IsInitialBlockDownload()` decision matrix by varying the cached latch, `BlockManager::LoadingBlocks()`, and tip work/recency inputs. This documents the current latching behavior and provides a baseline for later refactors. --- .../validation_chainstatemanager_tests.cpp | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 24122bd31579..d0b731b3716c 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -143,11 +143,11 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); // Reset IBD state so IsInitialBlockDownload() returns true and causes - // MaybeRebalancesCaches() to prioritize the snapshot chainstate, giving it + // MaybeRebalanceCaches() to prioritize the snapshot chainstate, giving it // more cache space than the snapshot chainstate. Calling ResetIbd() is // necessary because m_cached_finished_ibd is already latched to true before - // the test starts due to the test setup. After ResetIbd() is called. - // IsInitialBlockDownload will return true because at this point the active + // the test starts due to the test setup. After ResetIbd() is called, + // IsInitialBlockDownload() will return true because at this point the active // chainstate has a null chain tip. static_cast(manager).ResetIbd(); @@ -163,6 +163,43 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) BOOST_CHECK_CLOSE(double(c2.m_coinsdb_cache_size_bytes), max_cache * 0.95, 1); } +BOOST_FIXTURE_TEST_CASE(chainstatemanager_ibd_exit_after_loading_blocks, ChainTestingSetup) +{ + CBlockIndex tip; + ChainstateManager& chainman{*Assert(m_node.chainman)}; + auto apply{[&](bool cached_finished_ibd, bool loading_blocks, bool tip_exists, bool enough_work, bool tip_recent) { + LOCK(::cs_main); + chainman.ResetChainstates(); + chainman.InitializeChainstate(m_node.mempool.get()); + + const auto recent_time{Now() - chainman.m_options.max_tip_age}; + + chainman.m_cached_finished_ibd.store(cached_finished_ibd, std::memory_order_relaxed); + chainman.m_blockman.m_importing = loading_blocks; + if (tip_exists) { + tip.nChainWork = chainman.MinimumChainWork() - (enough_work ? 0 : 1); + tip.nTime = (recent_time - (tip_recent ? 0h : 100h)).time_since_epoch().count(); + chainman.ActiveChain().SetTip(tip); + } else { + assert(!chainman.ActiveChain().Tip()); + } + }}; + + for (const bool cached_finished_ibd : {false, true}) { + for (const bool loading_blocks : {false, true}) { + for (const bool tip_exists : {false, true}) { + for (const bool enough_work : {false, true}) { + for (const bool tip_recent : {false, true}) { + apply(cached_finished_ibd, loading_blocks, tip_exists, enough_work, tip_recent); + const bool expected_ibd = !cached_finished_ibd && (loading_blocks || !tip_exists || !enough_work || !tip_recent); + BOOST_CHECK_EQUAL(chainman.IsInitialBlockDownload(), expected_ibd); + } + } + } + } + } +} + struct SnapshotTestSetup : TestChain100Setup { // Run with coinsdb on the filesystem to support, e.g., moving invalidated // chainstate dirs to "*_invalid". From 8d531c6210eb05bc424c971f621bb0b688ff70e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Mon, 12 Jan 2026 00:13:04 +0100 Subject: [PATCH 2/7] validation: invert `m_cached_finished_ibd` to `m_cached_is_ibd` Rename and invert the internal IBD latch so the cached value directly matches `IsInitialBlockDownload()` (true while in IBD, then latched to false). This is a behavior-preserving refactor to avoid double negatives. --- src/test/fuzz/block_index_tree.cpp | 2 +- src/test/util/validation.cpp | 4 ++-- src/test/validation_chainstatemanager_tests.cpp | 12 ++++++------ src/validation.cpp | 8 ++++---- src/validation.h | 8 +++----- 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/test/fuzz/block_index_tree.cpp b/src/test/fuzz/block_index_tree.cpp index 131ec93533f7..a4bbe9584000 100644 --- a/src/test/fuzz/block_index_tree.cpp +++ b/src/test/fuzz/block_index_tree.cpp @@ -207,7 +207,7 @@ FUZZ_TARGET(block_index_tree, .init = initialize_block_index_tree) chainman.nBlockSequenceId = 2; chainman.ActiveChain().SetTip(*genesis); chainman.ActiveChainstate().setBlockIndexCandidates.clear(); - chainman.m_cached_finished_ibd = false; + chainman.m_cached_is_ibd = true; blockman.m_blocks_unlinked.clear(); blockman.m_have_pruned = false; blockman.CleanupForFuzzing(); diff --git a/src/test/util/validation.cpp b/src/test/util/validation.cpp index bb9c6db22551..7e09597ea34e 100644 --- a/src/test/util/validation.cpp +++ b/src/test/util/validation.cpp @@ -32,14 +32,14 @@ void TestChainstateManager::DisableNextWrite() void TestChainstateManager::ResetIbd() { - m_cached_finished_ibd = false; + m_cached_is_ibd = true; assert(IsInitialBlockDownload()); } void TestChainstateManager::JumpOutOfIbd() { Assert(IsInitialBlockDownload()); - m_cached_finished_ibd = true; + m_cached_is_ibd = false; Assert(!IsInitialBlockDownload()); } diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index d0b731b3716c..c576b89fb33a 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -145,7 +145,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) // Reset IBD state so IsInitialBlockDownload() returns true and causes // MaybeRebalanceCaches() to prioritize the snapshot chainstate, giving it // more cache space than the snapshot chainstate. Calling ResetIbd() is - // necessary because m_cached_finished_ibd is already latched to true before + // necessary because m_cached_is_ibd is already latched to false before // the test starts due to the test setup. After ResetIbd() is called, // IsInitialBlockDownload() will return true because at this point the active // chainstate has a null chain tip. @@ -167,14 +167,14 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_ibd_exit_after_loading_blocks, ChainTe { CBlockIndex tip; ChainstateManager& chainman{*Assert(m_node.chainman)}; - auto apply{[&](bool cached_finished_ibd, bool loading_blocks, bool tip_exists, bool enough_work, bool tip_recent) { + auto apply{[&](bool cached_is_ibd, bool loading_blocks, bool tip_exists, bool enough_work, bool tip_recent) { LOCK(::cs_main); chainman.ResetChainstates(); chainman.InitializeChainstate(m_node.mempool.get()); const auto recent_time{Now() - chainman.m_options.max_tip_age}; - chainman.m_cached_finished_ibd.store(cached_finished_ibd, std::memory_order_relaxed); + chainman.m_cached_is_ibd.store(cached_is_ibd, std::memory_order_relaxed); chainman.m_blockman.m_importing = loading_blocks; if (tip_exists) { tip.nChainWork = chainman.MinimumChainWork() - (enough_work ? 0 : 1); @@ -185,13 +185,13 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_ibd_exit_after_loading_blocks, ChainTe } }}; - for (const bool cached_finished_ibd : {false, true}) { + for (const bool cached_is_ibd : {false, true}) { for (const bool loading_blocks : {false, true}) { for (const bool tip_exists : {false, true}) { for (const bool enough_work : {false, true}) { for (const bool tip_recent : {false, true}) { - apply(cached_finished_ibd, loading_blocks, tip_exists, enough_work, tip_recent); - const bool expected_ibd = !cached_finished_ibd && (loading_blocks || !tip_exists || !enough_work || !tip_recent); + apply(cached_is_ibd, loading_blocks, tip_exists, enough_work, tip_recent); + const bool expected_ibd = cached_is_ibd && (loading_blocks || !tip_exists || !enough_work || !tip_recent); BOOST_CHECK_EQUAL(chainman.IsInitialBlockDownload(), expected_ibd); } } diff --git a/src/validation.cpp b/src/validation.cpp index b6da6d2d8d6f..3b10237e5ba6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1939,7 +1939,7 @@ void Chainstate::InitCoinsCache(size_t cache_size_bytes) m_coins_views->InitCache(); } -// Note that though this is marked const, we may end up modifying `m_cached_finished_ibd`, which +// Note that though this is marked const, we may end up modifying `m_cached_is_ibd`, which // is a performance-related implementation detail. This function must be marked // `const` so that `CValidationInterface` clients (which are given a `const Chainstate*`) // can call it. @@ -1947,11 +1947,11 @@ void Chainstate::InitCoinsCache(size_t cache_size_bytes) bool ChainstateManager::IsInitialBlockDownload() const { // Optimization: pre-test latch before taking the lock. - if (m_cached_finished_ibd.load(std::memory_order_relaxed)) + if (!m_cached_is_ibd.load(std::memory_order_relaxed)) return false; LOCK(cs_main); - if (m_cached_finished_ibd.load(std::memory_order_relaxed)) + if (!m_cached_is_ibd.load(std::memory_order_relaxed)) return false; if (m_blockman.LoadingBlocks()) { return true; @@ -1967,7 +1967,7 @@ bool ChainstateManager::IsInitialBlockDownload() const return true; } LogInfo("Leaving InitialBlockDownload (latching to false)"); - m_cached_finished_ibd.store(true, std::memory_order_relaxed); + m_cached_is_ibd.store(false, std::memory_order_relaxed); return false; } diff --git a/src/validation.h b/src/validation.h index e4b1e555bdde..2c518bad1512 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1030,13 +1030,11 @@ class ChainstateManager ValidationCache m_validation_cache; /** - * Whether initial block download has ended and IsInitialBlockDownload - * should return false from now on. + * Whether initial block download (IBD) is ongoing. * - * Mutable because we need to be able to mark IsInitialBlockDownload() - * const, which latches this for caching purposes. + * Once set to false, IsInitialBlockDownload() will keep returning false. */ - mutable std::atomic m_cached_finished_ibd{false}; + mutable std::atomic_bool m_cached_is_ibd{true}; /** * Every received block is assigned a unique and increasing identifier, so we From b9c0ab3b75a19d7a1f7c01762374ce85f2d0d7be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 11 Jan 2026 23:56:21 +0100 Subject: [PATCH 3/7] chain: add `CChain::IsTipRecent` helper Factor the chain tip work/recency check out of `ChainstateManager::IsInitialBlockDownload()` into a reusable `CChain::IsTipRecent()` helper, and annotate it as requiring `cs_main` since it's reading mutable state. Also introduce a local `chainman_ref` in the kernel import-blocks wrapper to reduce repetition and keep follow-up diffs small. `IsInitialBlockDownload` returns were also unified to make the followup move clean. Co-authored-by: Patrick Strateman Co-authored-by: Martin Zumsande --- src/chain.h | 9 +++++++++ src/kernel/bitcoinkernel.cpp | 3 ++- src/validation.cpp | 21 ++++----------------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/chain.h b/src/chain.h index 466276c01a7d..e94308aa7f8f 100644 --- a/src/chain.h +++ b/src/chain.h @@ -420,6 +420,15 @@ class CChain return int(vChain.size()) - 1; } + /** Check whether this chain's tip exists, has enough work, and is recent. */ + bool IsTipRecent(const arith_uint256& min_chain_work, std::chrono::seconds max_tip_age) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main) + { + const auto tip{Tip()}; + return tip && + tip->nChainWork >= min_chain_work && + tip->Time() >= Now() - max_tip_age; + } + /** Set/initialize a chain with a given tip. */ void SetTip(CBlockIndex& block); diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp index bbcfd66b9301..b061a1854836 100644 --- a/src/kernel/bitcoinkernel.cpp +++ b/src/kernel/bitcoinkernel.cpp @@ -1054,7 +1054,8 @@ int btck_chainstate_manager_import_blocks(btck_ChainstateManager* chainman, cons import_files.emplace_back(std::string{block_file_paths_data[i], block_file_paths_lens[i]}.c_str()); } } - node::ImportBlocks(*btck_ChainstateManager::get(chainman).m_chainman, import_files); + auto& chainman_ref{*btck_ChainstateManager::get(chainman).m_chainman}; + node::ImportBlocks(chainman_ref, import_files); } catch (const std::exception& e) { LogError("Failed to import blocks: %s", e.what()); return -1; diff --git a/src/validation.cpp b/src/validation.cpp index 3b10237e5ba6..1b3b09cbabee 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1947,25 +1947,12 @@ void Chainstate::InitCoinsCache(size_t cache_size_bytes) bool ChainstateManager::IsInitialBlockDownload() const { // Optimization: pre-test latch before taking the lock. - if (!m_cached_is_ibd.load(std::memory_order_relaxed)) - return false; + if (!m_cached_is_ibd.load(std::memory_order_relaxed)) return false; LOCK(cs_main); - if (!m_cached_is_ibd.load(std::memory_order_relaxed)) - return false; - if (m_blockman.LoadingBlocks()) { - return true; - } - CChain& chain{ActiveChain()}; - if (chain.Tip() == nullptr) { - return true; - } - if (chain.Tip()->nChainWork < MinimumChainWork()) { - return true; - } - if (chain.Tip()->Time() < Now() - m_options.max_tip_age) { - return true; - } + if (!m_cached_is_ibd.load(std::memory_order_relaxed)) return false; + if (m_blockman.LoadingBlocks()) return true; + if (!ActiveChain().IsTipRecent(MinimumChainWork(), m_options.max_tip_age)) return true; LogInfo("Leaving InitialBlockDownload (latching to false)"); m_cached_is_ibd.store(false, std::memory_order_relaxed); return false; From 557b41a38ccf2929ca1e5271db1701e5fbe781af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 11 Jan 2026 23:57:13 +0100 Subject: [PATCH 4/7] validation: make `IsInitialBlockDownload()` lock-free `ChainstateManager::IsInitialBlockDownload()` is queried on hot paths and previously acquired `cs_main` internally, contributing to lock contention. Cache the IBD status in `m_cached_is_ibd`, and introduce `ChainstateManager::UpdateIBDStatus()` to latch it once block loading has finished and the current chain tip has enough work and is recent. Call the updater after tip updates and after `ImportBlocks()` completes. Since `IsInitialBlockDownload()` no longer updates the cache, drop `mutable` from `m_cached_is_ibd` and only update it from `UpdateIBDStatus()` under `cs_main`. Update the new unit test to showcase the new `UpdateIBDStatus()`. Co-authored-by: Patrick Strateman Co-authored-by: Martin Zumsande --- src/init.cpp | 1 + src/kernel/bitcoinkernel.cpp | 1 + .../validation_chainstatemanager_tests.cpp | 1 + src/validation.cpp | 32 +++++++++++-------- src/validation.h | 19 +++++++++-- 5 files changed, 38 insertions(+), 16 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index f2af858eb46c..a245ebfbd76a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1966,6 +1966,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) ScheduleBatchPriority(); // Import blocks and ActivateBestChain() ImportBlocks(chainman, vImportFiles); + WITH_LOCK(::cs_main, chainman.UpdateIBDStatus()); if (args.GetBoolArg("-stopafterblockimport", DEFAULT_STOPAFTERBLOCKIMPORT)) { LogInfo("Stopping after block import"); if (!(Assert(node.shutdown_request))()) { diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp index b061a1854836..a6a9c325dd16 100644 --- a/src/kernel/bitcoinkernel.cpp +++ b/src/kernel/bitcoinkernel.cpp @@ -1056,6 +1056,7 @@ int btck_chainstate_manager_import_blocks(btck_ChainstateManager* chainman, cons } auto& chainman_ref{*btck_ChainstateManager::get(chainman).m_chainman}; node::ImportBlocks(chainman_ref, import_files); + WITH_LOCK(::cs_main, chainman_ref.UpdateIBDStatus()); } catch (const std::exception& e) { LogError("Failed to import blocks: %s", e.what()); return -1; diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index c576b89fb33a..41dfdcd16201 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -183,6 +183,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_ibd_exit_after_loading_blocks, ChainTe } else { assert(!chainman.ActiveChain().Tip()); } + chainman.UpdateIBDStatus(); }}; for (const bool cached_is_ibd : {false, true}) { diff --git a/src/validation.cpp b/src/validation.cpp index 1b3b09cbabee..09675b2a3d61 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1939,23 +1939,15 @@ void Chainstate::InitCoinsCache(size_t cache_size_bytes) m_coins_views->InitCache(); } -// Note that though this is marked const, we may end up modifying `m_cached_is_ibd`, which -// is a performance-related implementation detail. This function must be marked -// `const` so that `CValidationInterface` clients (which are given a `const Chainstate*`) -// can call it. +// This function must be marked `const` so that `CValidationInterface` clients +// (which are given a `const Chainstate*`) can call it. +// +// It is lock-free and depends on `m_cached_is_ibd`, which is latched by +// `UpdateIBDStatus()`. // bool ChainstateManager::IsInitialBlockDownload() const { - // Optimization: pre-test latch before taking the lock. - if (!m_cached_is_ibd.load(std::memory_order_relaxed)) return false; - - LOCK(cs_main); - if (!m_cached_is_ibd.load(std::memory_order_relaxed)) return false; - if (m_blockman.LoadingBlocks()) return true; - if (!ActiveChain().IsTipRecent(MinimumChainWork(), m_options.max_tip_age)) return true; - LogInfo("Leaving InitialBlockDownload (latching to false)"); - m_cached_is_ibd.store(false, std::memory_order_relaxed); - return false; + return m_cached_is_ibd.load(std::memory_order_relaxed); } void Chainstate::CheckForkWarningConditions() @@ -2999,6 +2991,7 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra } m_chain.SetTip(*pindexDelete->pprev); + m_chainman.UpdateIBDStatus(); UpdateTip(pindexDelete->pprev); // Let wallets know transactions went from 1-confirmed to @@ -3128,6 +3121,7 @@ bool Chainstate::ConnectTip( } // Update m_chain & related variables. m_chain.SetTip(*pindexNew); + m_chainman.UpdateIBDStatus(); UpdateTip(pindexNew); const auto time_6{SteadyClock::now()}; @@ -3331,6 +3325,15 @@ static SynchronizationState GetSynchronizationState(bool init, bool blockfiles_i return SynchronizationState::INIT_DOWNLOAD; } +void ChainstateManager::UpdateIBDStatus() +{ + if (!m_cached_is_ibd.load(std::memory_order_relaxed)) return; + if (m_blockman.LoadingBlocks()) return; + if (!CurrentChainstate().m_chain.IsTipRecent(MinimumChainWork(), m_options.max_tip_age)) return; + LogInfo("Leaving InitialBlockDownload (latching to false)"); + m_cached_is_ibd.store(false, std::memory_order_relaxed); +} + bool ChainstateManager::NotifyHeaderTip() { bool fNotify = false; @@ -4614,6 +4617,7 @@ bool Chainstate::LoadChainTip() return false; } m_chain.SetTip(*pindex); + m_chainman.UpdateIBDStatus(); tip = m_chain.Tip(); // Make sure our chain tip before shutting down scores better than any other candidate diff --git a/src/validation.h b/src/validation.h index 2c518bad1512..438aab3e7841 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1032,9 +1032,11 @@ class ChainstateManager /** * Whether initial block download (IBD) is ongoing. * - * Once set to false, IsInitialBlockDownload() will keep returning false. + * This value is used for lock-free IBD checks, and latches from true to + * false once block loading has finished and the current chain tip has + * enough work and is recent. */ - mutable std::atomic_bool m_cached_is_ibd{true}; + std::atomic_bool m_cached_is_ibd{true}; /** * Every received block is assigned a unique and increasing identifier, so we @@ -1155,6 +1157,19 @@ class ChainstateManager CBlockIndex* ActiveTip() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChain().Tip(); } //! @} + /** + * Update and possibly latch the IBD status. + * + * If block loading has finished and the current chain tip has enough work + * and is recent, set `m_cached_is_ibd` to false. This function never sets + * the flag back to true. + * + * This should be called after operations that may affect IBD exit + * conditions (e.g. after updating the active chain tip, or after + * `ImportBlocks()` finishes). + */ + void UpdateIBDStatus() EXCLUSIVE_LOCKS_REQUIRED(cs_main); + node::BlockMap& BlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { AssertLockHeld(::cs_main); From 34bed0ed8c449a3834927cec3447dbe6c74edf3d Mon Sep 17 00:00:00 2001 From: woltx <94266259+w0xlt@users.noreply.github.com> Date: Mon, 19 Jan 2026 01:53:40 -0800 Subject: [PATCH 5/7] test: use IP_PORTRANGE_HIGH on FreeBSD for dynamic port allocation On FreeBSD, the default ephemeral port range (10000-65535) overlaps with the test framework's static port range (11000-26000), causing intermittent "address already in use" failures when tests use dynamic port allocation (port=0). Add a helper function that sets the IP_PORTRANGE/IPV6_PORTRANGE socket option to IP_PORTRANGE_HIGH before binding, which requests ports from the high range (49152-65535) instead. This range does not overlap with the test framework's static ports. Constants from FreeBSD's netinet/in.h and netinet6/in6.h: - IP_PORTRANGE = 19 (for IPv4 sockets) - IPV6_PORTRANGE = 14 (for IPv6 sockets) - IP_PORTRANGE_HIGH = 1 Fixes: bitcoin/bitcoin#34331 Co-Authored-By: Vasil Dimov Co-Authored-By: MarcoFalke <*~=\`'#}+{/-|&$^_@721217.xyz> --- test/functional/test_framework/netutil.py | 19 +++++++++++++++++++ test/functional/test_framework/socks5.py | 7 ++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/test/functional/test_framework/netutil.py b/test/functional/test_framework/netutil.py index 75577b0ca84d..5504029a7662 100644 --- a/test/functional/test_framework/netutil.py +++ b/test/functional/test_framework/netutil.py @@ -181,3 +181,22 @@ def format_addr_port(addr, port): return f"[{addr}]:{port}" else: return f"{addr}:{port}" + + +def set_ephemeral_port_range(sock): + '''On FreeBSD, set socket to use the high ephemeral port range (49152-65535). + + FreeBSD's default ephemeral port range (10000-65535) overlaps with the test + framework's static port range starting at TEST_RUNNER_PORT_MIN (default=11000). + Using IP_PORTRANGE_HIGH avoids this overlap when binding to port 0 for dynamic + port allocation. + ''' + if sys.platform.startswith('freebsd'): + # Constants from FreeBSD's netinet/in.h and netinet6/in6.h + IP_PORTRANGE = 19 + IPV6_PORTRANGE = 14 + IP_PORTRANGE_HIGH = 1 # Same value for both IPv4 and IPv6 + if sock.family == socket.AF_INET6: + sock.setsockopt(socket.IPPROTO_IPV6, IPV6_PORTRANGE, IP_PORTRANGE_HIGH) + else: + sock.setsockopt(socket.IPPROTO_IP, IP_PORTRANGE, IP_PORTRANGE_HIGH) diff --git a/test/functional/test_framework/socks5.py b/test/functional/test_framework/socks5.py index 711734cf98ec..085c5a2e3241 100644 --- a/test/functional/test_framework/socks5.py +++ b/test/functional/test_framework/socks5.py @@ -11,7 +11,8 @@ import logging from .netutil import ( - format_addr_port + format_addr_port, + set_ephemeral_port_range, ) logger = logging.getLogger("TestFramework.socks5") @@ -202,6 +203,10 @@ def __init__(self, conf): self.conf = conf self.s = socket.socket(conf.af) self.s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + # When using dynamic port allocation (port=0), ensure we don't get a + # port that conflicts with the test framework's static port range. + if conf.addr[1] == 0: + set_ephemeral_port_range(self.s) self.s.bind(conf.addr) # When port=0, the OS assigns an available port. Update conf.addr # to reflect the actual bound address so callers can use it. From 2845f10a2be0fee13b2772d24e948052243782b8 Mon Sep 17 00:00:00 2001 From: node Date: Tue, 20 Jan 2026 15:31:56 -0800 Subject: [PATCH 6/7] test: extend FreeBSD ephemeral port range fix to P2P listeners The previous commit added set_ephemeral_port_range() to avoid port conflicts on FreeBSD by requesting ports from the high ephemeral range (49152-65535) instead of the default range which overlaps with the test framework's static port range. That fix was applied to the SOCKS5 server but not to P2P listeners created via NetworkThread.create_listen_server(). This commit extends the fix to cover P2P listeners as well. When port=0 is requested (dynamic allocation), we now: 1. Manually create a socket with the appropriate address family 2. Call set_ephemeral_port_range() to configure the port range 3. Bind and listen on the socket 4. Pass the pre-configured socket to asyncio's create_server() This ensures that dynamically allocated ports for P2P listeners also come from the high range on FreeBSD, avoiding conflicts with the test framework's static port assignments. Co-Authored-By: Vasil Dimov --- test/functional/test_framework/p2p.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 986eaf1e88e3..8102da7a7f7c 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -22,9 +22,11 @@ import asyncio from collections import defaultdict +import ipaddress from io import BytesIO import logging import platform +import socket import struct import sys import threading @@ -76,6 +78,9 @@ MAGIC_BYTES, sha256, ) +from test_framework.netutil import ( + set_ephemeral_port_range, +) from test_framework.util import ( assert_not_equal, MAX_NODES, @@ -793,8 +798,22 @@ def peer_protocol(): # connections, we can accomplish this by providing different # `proto` functions - listener = await cls.network_event_loop.create_server(peer_protocol, addr, port) - port = listener.sockets[0].getsockname()[1] + if port == 0: + # Manually create the socket in order to set the range to be + # used for the port before the bind() call. + if ipaddress.ip_address(addr).version == 4: + address_family = socket.AF_INET + else: + address_family = socket.AF_INET6 + s = socket.socket(address_family) + set_ephemeral_port_range(s) + s.bind((addr, 0)) + s.listen() + listener = await cls.network_event_loop.create_server(peer_protocol, sock=s) + port = listener.sockets[0].getsockname()[1] + else: + listener = await cls.network_event_loop.create_server(peer_protocol, addr, port) + logger.debug("Listening server on %s:%d should be started" % (addr, port)) cls.listeners[(addr, port)] = listener From fad042235bd6054d99d3f5a07529276b0138b484 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 13 Jan 2026 18:16:04 +0100 Subject: [PATCH 7/7] refactor: Remove remaining std::bind, check via clang-tidy --- src/.clang-tidy | 1 + src/qt/splashscreen.cpp | 12 +++++++++--- src/qt/transactiontablemodel.cpp | 4 +++- src/wallet/wallet.cpp | 4 +++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/.clang-tidy b/src/.clang-tidy index f54e07facae4..cd42491a1ec4 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -10,6 +10,7 @@ bugprone-unhandled-self-assignment, bugprone-unused-return-value, misc-unused-using-decls, misc-no-recursion, +modernize-avoid-bind, modernize-deprecated-headers, modernize-use-default-member-init, modernize-use-emplace, diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index 237df5f6e84c..553f6789435b 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -180,8 +180,12 @@ static void ShowProgress(SplashScreen *splash, const std::string &title, int nPr void SplashScreen::subscribeToCoreSignals() { // Connect signals to client - m_handler_init_message = m_node->handleInitMessage(std::bind(InitMessage, this, std::placeholders::_1)); - m_handler_show_progress = m_node->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); + m_handler_init_message = m_node->handleInitMessage([this](const std::string& message) { + InitMessage(this, message); + }); + m_handler_show_progress = m_node->handleShowProgress([this](const std::string& title, int nProgress, bool resume_possible) { + ShowProgress(this, title, nProgress, resume_possible); + }); m_handler_init_wallet = m_node->handleInitWallet([this]() { handleLoadWallet(); }); } @@ -190,7 +194,9 @@ void SplashScreen::handleLoadWallet() #ifdef ENABLE_WALLET if (!WalletModel::isWalletEnabled()) return; m_handler_load_wallet = m_node->walletLoader().handleLoadWallet([this](std::unique_ptr wallet) { - m_connected_wallet_handlers.emplace_back(wallet->handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2, false))); + m_connected_wallet_handlers.emplace_back(wallet->handleShowProgress([this](const std::string& title, int nProgress) { + ShowProgress(this, title, nProgress, /*resume_possible=*/false); + })); m_connected_wallets.emplace_back(std::move(wallet)); }); #endif diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 28734e3edd9b..6c09c4d7a891 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -712,7 +712,9 @@ void TransactionTablePriv::DispatchNotifications() void TransactionTableModel::subscribeToCoreSignals() { // Connect signals to wallet - m_handler_transaction_changed = walletModel->wallet().handleTransactionChanged(std::bind(&TransactionTablePriv::NotifyTransactionChanged, priv, std::placeholders::_1, std::placeholders::_2)); + m_handler_transaction_changed = walletModel->wallet().handleTransactionChanged([this](const Txid& hash, ChangeType status) { + priv->NotifyTransactionChanged(hash, status); + }); m_handler_show_progress = walletModel->wallet().handleShowProgress([this](const std::string&, int progress) { priv->m_loading = progress < 100; priv->DispatchNotifications(); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index de260f8e49d7..d12d3877e3eb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3543,7 +3543,9 @@ void CWallet::ConnectScriptPubKeyManNotifiers() { for (const auto& spk_man : GetActiveScriptPubKeyMans()) { spk_man->NotifyCanGetAddressesChanged.connect(NotifyCanGetAddressesChanged); - spk_man->NotifyFirstKeyTimeChanged.connect(std::bind(&CWallet::MaybeUpdateBirthTime, this, std::placeholders::_2)); + spk_man->NotifyFirstKeyTimeChanged.connect([this](const ScriptPubKeyMan*, int64_t time) { + MaybeUpdateBirthTime(time); + }); } }