mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-04-10 15:47:17 +02:00
Merge bitcoin/bitcoin#34451: rpc: fix race condition in gettxoutsetinfo
f3bf63ec4fkernel: acquire coinstats cursor and block info atomically (w0xlt)5e77072fa6rpc: fix race condition in gettxoutsetinfo (w0xlt) Pull request description: Fixes #34263 A `CHECK_NONFATAL` assertion failure could occur in `gettxoutsetinfo` when a new block was connected between capturing `pindex` and validating it in `GetUTXOStats()`. The fix removes the early `pindex` capture since `ComputeUTXOStats()` independently fetches the current best block under lock. The response now uses `stats.hashBlock` and `stats.nHeight` (the actual computed values) instead of the potentially stale `pindex`. ACKs for top commit: sedited: ACKf3bf63ec4ffjahr: utACKf3bf63ec4frkrux: Concept ACKf3bf63ec4ffor removal of the race condition. Tree-SHA512: c2d5cd5a1b4b4f1c22023c03970fea400a0b78005fa3d09d6567255615ab461c01b584d8a158651ee08769ec86fc4a1200f640ad58fdaa4879c335d90c891f6a
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -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<CCoinsStats> 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<CCoinsStats> 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");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user