kernel: acquire coinstats cursor and block info atomically
What the developer wrote
Authored by w0xlt
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.
What changed, and why it matters
This change fixes a timing bug in how Bitcoin Core calculates statistics about all unspent coins (the UTXO set). Previously, the code could read the current best block, then briefly allow a new block to be connected, and only then start scanning the coin database. That could make the final report say it was for block X while the actual data came from block X+1. The fix captures both the block pointer and the database cursor while holding the same global lock, so they stay consistent. It is a correctness fix for an RPC/administrative feature, not a direct theft or remote-code-execution vulnerability.
Backport to maintained release branches if coinstats consistency is relied upon; otherwise treat as routine correctness fix. No emergency deployment required.
Security signals we found
Race condition between block index lookup and UTXO cursor acquisition
Inconsistent coin statistics report (block metadata vs. UTXO snapshot)
Use of global cs_main lock to atomicize multi-step state reads
Correctness fix for gettxoutset RPC / coinstats index data
Evidence from the diff
ComputeUTXOStats in src/kernel/coinstats.cpp previously called blockman.LookupBlockIndex(view->GetBestBlock()) under cs_main, then dropped the lock and later called view->Cursor() inside the templated ComputeUTXOStats helper. Because cs_main was released in between, a new block could be connected, advancing the view’s best block and changing the cursor’s snapshot. The patch moves view->Cursor() into the same cs_main critical section as LookupBlockIndex, passing the acquired cursor down to the helper so the reported block hash/height and the iterated UTXO set are atomically consistent.
Changed components
src/kernel/coinstats.cppComputeUTXOStatsRPC gettxoutsetinfo / coinstats indexInspect captured patch +11 / −6
diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp
index d287ec4b..49b51d64 100644
--- a/src/kernel/coinstats.cpp
+++ b/src/kernel/coinstats.cpp
@@ -109,9 +109,8 @@ static void ApplyStats(CCoinsStats& stats, const std::map<uint32_t, Coin>& outpu
//! Calculate statistics about the unspent transaction output set
template <typename T>
-static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const std::function<void()>& interruption_point)
+static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const std::function<void()>& interruption_point, std::unique_ptr<CCoinsViewCursor> pcursor)
{
- std::unique_ptr<CCoinsViewCursor> pcursor(view->Cursor());
assert(pcursor);
Txid prevkey;
@@ -149,21 +148,27 @@ static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, c
std::optional<CCoinsStats> ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsView* view, node::BlockManager& blockman, const std::function<void()>& interruption_point)
{
- CBlockIndex* pindex = WITH_LOCK(::cs_main, return blockman.LookupBlockIndex(view->GetBestBlock()));
+ std::unique_ptr<CCoinsViewCursor> 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);
Why this scored 26/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.