From 5e77072fa60110a00d2bff31798d58b6c10bd3da Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Tue, 27 Jan 2026 12:57:32 -0800 Subject: [PATCH 1/5] rpc: fix race condition in gettxoutsetinfo Fix an assertion failure in gettxoutsetinfo (issue #34263) caused by capturing the best block before releasing cs_main, then checking it against a potentially newer best block in GetUTXOStats(). Remove the early pindex capture since ComputeUTXOStats() independently fetches the current best block under lock. Use stats.hashBlock and stats.nHeight (the actual computed values) instead of the potentially stale pindex when building the response. --- src/rpc/blockchain.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index d97b7c6c15c6..c631a93674aa 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1080,7 +1080,6 @@ static RPCHelpMan gettxoutsetinfo() LOCK(::cs_main); coins_view = &active_chainstate.CoinsDB(); blockman = &active_chainstate.m_blockman; - pindex = blockman->LookupBlockIndex(coins_view->GetBestBlock()); } if (!request.params[1].isNull()) { @@ -1104,7 +1103,7 @@ static RPCHelpMan gettxoutsetinfo() // If a specific block was requested and the index has already synced past that height, we can return the // data already even though the index is not fully synced yet. - if (pindex->nHeight > summary.best_block_height) { + if (pindex && pindex->nHeight > summary.best_block_height) { throw JSONRPCError(RPC_INTERNAL_ERROR, strprintf("Unable to get data because coinstatsindex is still syncing. Current height: %d", summary.best_block_height)); } } @@ -1130,8 +1129,9 @@ static RPCHelpMan gettxoutsetinfo() ret.pushKV("disk_size", stats.nDiskSize); } else { CCoinsStats prev_stats{}; - if (pindex->nHeight > 0) { - const std::optional maybe_prev_stats = GetUTXOStats(coins_view, *blockman, hash_type, node.rpc_interruption_point, pindex->pprev, index_requested); + if (stats.nHeight > 0) { + const CBlockIndex& block_index = *CHECK_NONFATAL(WITH_LOCK(::cs_main, return blockman->LookupBlockIndex(stats.hashBlock))); + const std::optional maybe_prev_stats = GetUTXOStats(coins_view, *blockman, hash_type, node.rpc_interruption_point, block_index.pprev, index_requested); if (!maybe_prev_stats) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set"); } From f3bf63ec4f028cf9ee0820226f44fcbe26d358c9 Mon Sep 17 00:00:00 2001 From: w0xlt <94266259+w0xlt@users.noreply.github.com> Date: Tue, 3 Feb 2026 11:18:35 -0800 Subject: [PATCH 2/5] kernel: acquire coinstats cursor and block info atomically Acquire the cursor and block index under the same cs_main lock to eliminate a potential race where a new block could be connected between capturing the block info and acquiring the cursor, causing the reported stats to reference a different block than the one being iterated. --- src/kernel/coinstats.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp index d287ec4be6e3..49b51d64f30c 100644 --- a/src/kernel/coinstats.cpp +++ b/src/kernel/coinstats.cpp @@ -109,9 +109,8 @@ static void ApplyStats(CCoinsStats& stats, const std::map& outpu //! Calculate statistics about the unspent transaction output set template -static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const std::function& interruption_point) +static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const std::function& interruption_point, std::unique_ptr pcursor) { - std::unique_ptr pcursor(view->Cursor()); assert(pcursor); Txid prevkey; @@ -149,21 +148,27 @@ static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, c std::optional ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsView* view, node::BlockManager& blockman, const std::function& interruption_point) { - CBlockIndex* pindex = WITH_LOCK(::cs_main, return blockman.LookupBlockIndex(view->GetBestBlock())); + std::unique_ptr pcursor; + CBlockIndex* pindex; + { + LOCK(::cs_main); + pcursor = view->Cursor(); + pindex = blockman.LookupBlockIndex(pcursor->GetBestBlock()); + } CCoinsStats stats{Assert(pindex)->nHeight, pindex->GetBlockHash()}; bool success = [&]() -> bool { switch (hash_type) { case(CoinStatsHashType::HASH_SERIALIZED): { HashWriter ss{}; - return ComputeUTXOStats(view, stats, ss, interruption_point); + return ComputeUTXOStats(view, stats, ss, interruption_point, std::move(pcursor)); } case(CoinStatsHashType::MUHASH): { MuHash3072 muhash; - return ComputeUTXOStats(view, stats, muhash, interruption_point); + return ComputeUTXOStats(view, stats, muhash, interruption_point, std::move(pcursor)); } case(CoinStatsHashType::NONE): { - return ComputeUTXOStats(view, stats, nullptr, interruption_point); + return ComputeUTXOStats(view, stats, nullptr, interruption_point, std::move(pcursor)); } } // no default case, so the compiler can warn about missing cases assert(false); From 498b6eb6b5e8aceb372d3097df2715d9c7fc416b Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Wed, 18 Mar 2026 10:43:28 +0100 Subject: [PATCH 3/5] cmake: Migrate away from deprecated SQLite3 target CMake version 4.3 deprecated the imported target `Sqlite::Sqlite3`. Use the preferred name `Sqlite3::Sqlite3` instead and provide an alias for older versions of CMake. Also define the same alias when using vcpkg. --- CMakeLists.txt | 4 ++++ src/wallet/CMakeLists.txt | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 518e02776d44..26e5fede0eb9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -117,8 +117,12 @@ if(ENABLE_WALLET) if(VCPKG_TARGET_TRIPLET) # Use of the `unofficial::` namespace is a vcpkg package manager convention. find_package(unofficial-sqlite3 CONFIG REQUIRED) + add_library(SQLite3::SQLite3 ALIAS unofficial::sqlite3::sqlite3) else() find_package(SQLite3 3.7.17 REQUIRED) + if(NOT TARGET SQLite3::SQLite3) # CMake < 4.3 + add_library(SQLite3::SQLite3 ALIAS SQLite::SQLite3) + endif() endif() endif() cmake_dependent_option(BUILD_WALLET_TOOL "Build bitcoin-wallet tool." ${BUILD_TESTS} "ENABLE_WALLET" OFF) diff --git a/src/wallet/CMakeLists.txt b/src/wallet/CMakeLists.txt index 8ec381df5a8c..36fd3ef95aec 100644 --- a/src/wallet/CMakeLists.txt +++ b/src/wallet/CMakeLists.txt @@ -38,8 +38,7 @@ target_link_libraries(bitcoin_wallet PRIVATE core_interface bitcoin_common - $ - $ + SQLite3::SQLite3 univalue Boost::headers $ From 99996f6c06dff19cd99131d0469b84c01c216cbb Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 18 Mar 2026 16:14:16 +0100 Subject: [PATCH 4/5] test: Fix intermittent issue in feature_assumeutxo.py --- test/functional/feature_assumeutxo.py | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 7420dfe02a01..90f597de176d 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -38,6 +38,7 @@ assert_equal, assert_not_equal, assert_raises_rpc_error, + dumb_sync_blocks, ensure_for, sha256sum_file, try_rpc, @@ -631,30 +632,17 @@ def check_tx_counts(final: bool) -> None: PAUSE_HEIGHT = FINAL_HEIGHT - 40 - self.log.info("Restarting node to stop at height %d", PAUSE_HEIGHT) - self.restart_node(1, extra_args=[ - f"-stopatheight={PAUSE_HEIGHT}", *self.extra_args[1]]) - - # Upon restart during snapshot tip sync, the node must remain in 'limited' mode. + self.log.info(f"Sync node up to height {PAUSE_HEIGHT}") + # During snapshot tip sync, the node must remain in 'limited' mode. self.assert_only_network_limited_service(n1) - - # Finally connect the nodes and let them sync. - # - # Set `wait_for_connect=False` to avoid a race between performing connection - # assertions and the -stopatheight tripping. - self.connect_nodes(0, 1, wait_for_connect=False) - - n1.wait_until_stopped(timeout=5) + dumb_sync_blocks(src=n0, dst=n1, height=PAUSE_HEIGHT) self.log.info("Checking that blocks are segmented on disk") assert self.has_blockfile(n1, "00000"), "normal blockfile missing" assert self.has_blockfile(n1, "00001"), "assumed blockfile missing" assert not self.has_blockfile(n1, "00002"), "too many blockfiles" - self.log.info("Restarted node before snapshot validation completed, reloading...") - self.restart_node(1, extra_args=self.extra_args[1]) - - # Upon restart, the node must remain in 'limited' mode + # The node must remain in 'limited' mode self.assert_only_network_limited_service(n1) # Send snapshot block to n1 out of order. This makes the test less From faf71d6cb49b8a668c417d80bc56277f30cbf2e1 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 18 Mar 2026 16:19:10 +0100 Subject: [PATCH 5/5] test: [refactor] Use verbosity=0 named arg This is less confusing than the verbose=0 alias. --- test/functional/test_framework/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 497a6d2d7ba2..2eb891cf6c9a 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -729,7 +729,7 @@ def dumb_sync_blocks(*, src, dst, height=None): height = height or src.getblockcount() for i in range(dst.getblockcount() + 1, height + 1): block_hash = src.getblockhash(i) - block = src.getblock(blockhash=block_hash, verbose=0) + block = src.getblock(blockhash=block_hash, verbosity=0) dst.submitblock(block) assert_equal(dst.getblockcount(), height)