From 3e5dc610353ef16b0e7f391faaac77da6f4a0fab Mon Sep 17 00:00:00 2001
From: rkrux <rkrux.connect@gmail.com>
Date: Tue, 24 Mar 2026 16:22:58 +0530
Subject: [PATCH] rpc, refactor: gettxoutsetinfo race condition fix follow-ups

This patch addresses my own review comments from the review of PR 34451.
If these are found helpful, it makes sense to do them now after the previous
PR was merged and backported.

Pasting the comments below that also explains the changes:

- Move the pindex declaration below now that it is not used earlier.
- stats was being generated partially in both these ComputeUTXOStats functions,
which reads oddly to me. Now that the pcursor is also moved and passed to this
function, which reads oddly as well, I believe we can refactor this function
to completely build the stats inside this function. A side benefit is that by
removing the stats and pcursor arguments, the function signature becomes quite
similar to its namesake, which in turn becomes a straightforward wrapper of
this function.
---
 src/kernel/coinstats.cpp | 37 +++++++++++++++----------------------
 src/rpc/blockchain.cpp   |  2 +-
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/src/kernel/coinstats.cpp b/src/kernel/coinstats.cpp
index 4e30a876229..8af546db996 100644
--- a/src/kernel/coinstats.cpp
+++ b/src/kernel/coinstats.cpp
@@ -108,9 +108,17 @@ 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, std::unique_ptr<CCoinsViewCursor> pcursor)
+static std::optional<CCoinsStats> ComputeUTXOStats(T hash_obj, CCoinsView* view, node::BlockManager& blockman, const std::function<void()>& interruption_point)
 {
+    std::unique_ptr<CCoinsViewCursor> pcursor;
+    CBlockIndex* pindex;
+    {
+        LOCK(::cs_main);
+        pcursor = view->Cursor();
+        pindex = blockman.LookupBlockIndex(pcursor->GetBestBlock());
+    }
     assert(pcursor);
+    CCoinsStats stats{Assert(pindex)->nHeight, pindex->GetBlockHash()};
 
     Txid prevkey;
     std::map<uint32_t, Coin> outputs;
@@ -129,7 +137,7 @@ static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, c
             stats.coins_count++;
         } else {
             LogError("%s: unable to read value\n", __func__);
-            return false;
+            return std::nullopt;
         }
         pcursor->Next();
     }
@@ -141,42 +149,27 @@ static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, c
     FinalizeHash(hash_obj, stats);
 
     stats.nDiskSize = view->EstimateSize();
-
-    return true;
+    return stats;
 }
 
 std::optional<CCoinsStats> ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsView* view, node::BlockManager& blockman, const std::function<void()>& interruption_point)
 {
-    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 {
+    return [&]() -> std::optional<CCoinsStats> {
         switch (hash_type) {
         case(CoinStatsHashType::HASH_SERIALIZED): {
             HashWriter ss{};
-            return ComputeUTXOStats(view, stats, ss, interruption_point, std::move(pcursor));
+            return ComputeUTXOStats(ss, view, blockman, interruption_point);
         }
         case(CoinStatsHashType::MUHASH): {
             MuHash3072 muhash;
-            return ComputeUTXOStats(view, stats, muhash, interruption_point, std::move(pcursor));
+            return ComputeUTXOStats(muhash, view, blockman, interruption_point);
         }
         case(CoinStatsHashType::NONE): {
-            return ComputeUTXOStats(view, stats, nullptr, interruption_point, std::move(pcursor));
+            return ComputeUTXOStats(nullptr, view, blockman, interruption_point);
         }
         } // no default case, so the compiler can warn about missing cases
         assert(false);
     }();
-
-    if (!success) {
-        return std::nullopt;
-    }
-    return stats;
 }
 
 static void FinalizeHash(HashWriter& ss, CCoinsStats& stats)
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 64ca01e8f56..dd24390035b 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1065,7 +1065,6 @@ static RPCHelpMan gettxoutsetinfo()
 {
     UniValue ret(UniValue::VOBJ);
 
-    const CBlockIndex* pindex{nullptr};
     const CoinStatsHashType hash_type{ParseHashType(self.Arg<std::string_view>("hash_type"))};
     bool index_requested = request.params[2].isNull() || request.params[2].get_bool();
 
@@ -1082,6 +1081,7 @@ static RPCHelpMan gettxoutsetinfo()
         blockman = &active_chainstate.m_blockman;
     }
 
+    const CBlockIndex* pindex{nullptr};
     if (!request.params[1].isNull()) {
         if (!g_coin_stats_index) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Querying specific block heights requires coinstatsindex");