Move logic from LookupUTXOStatsWithIndex to CoinStatsIndex::LookUpStats

The indexing codepath logic in node/coinstats.cpp is simple enough to be
moved into CoinStatsIndex::LookUpStats, avoiding an additional layer of
function calls. Callers are modified accordingly.

Also, add 2 missed BOOST_CHECKs to the coinstatsindex_initial_sync unit
test.

Author: dongcarl

src/index/coinstatsindex.cpp

@@ -316,28 +316,31 @@ static bool LookUpOne(const CDBWrapper& db, const CBlockIndex* block_index, DBVa
     return db.Read(DBHashKey(block_index->GetBlockHash()), result);
 }
 
-bool CoinStatsIndex::LookUpStats(const CBlockIndex* block_index, CCoinsStats& coins_stats) const
+std::optional<CCoinsStats> CoinStatsIndex::LookUpStats(const CBlockIndex* block_index) const
 {
+    CCoinsStats stats{Assert(block_index)->nHeight, block_index->GetBlockHash()};
+    stats.index_used = true;
+
     DBVal entry;
     if (!LookUpOne(*m_db, block_index, entry)) {
-        return false;
+        return std::nullopt;
     }
 
-    coins_stats.hashSerialized = entry.muhash;
-    coins_stats.nTransactionOutputs = entry.transaction_output_count;
-    coins_stats.nBogoSize = entry.bogo_size;
-    coins_stats.total_amount = entry.total_amount;
-    coins_stats.total_subsidy = entry.total_subsidy;
-    coins_stats.total_unspendable_amount = entry.total_unspendable_amount;
-    coins_stats.total_prevout_spent_amount = entry.total_prevout_spent_amount;
-    coins_stats.total_new_outputs_ex_coinbase_amount = entry.total_new_outputs_ex_coinbase_amount;
-    coins_stats.total_coinbase_amount = entry.total_coinbase_amount;
-    coins_stats.total_unspendables_genesis_block = entry.total_unspendables_genesis_block;
-    coins_stats.total_unspendables_bip30 = entry.total_unspendables_bip30;
-    coins_stats.total_unspendables_scripts = entry.total_unspendables_scripts;
-    coins_stats.total_unspendables_unclaimed_rewards = entry.total_unspendables_unclaimed_rewards;
-
-    return true;
+    stats.hashSerialized = entry.muhash;
+    stats.nTransactionOutputs = entry.transaction_output_count;
+    stats.nBogoSize = entry.bogo_size;
+    stats.total_amount = entry.total_amount;
+    stats.total_subsidy = entry.total_subsidy;
+    stats.total_unspendable_amount = entry.total_unspendable_amount;
+    stats.total_prevout_spent_amount = entry.total_prevout_spent_amount;
+    stats.total_new_outputs_ex_coinbase_amount = entry.total_new_outputs_ex_coinbase_amount;
+    stats.total_coinbase_amount = entry.total_coinbase_amount;
+    stats.total_unspendables_genesis_block = entry.total_unspendables_genesis_block;
+    stats.total_unspendables_bip30 = entry.total_unspendables_bip30;
+    stats.total_unspendables_scripts = entry.total_unspendables_scripts;
+    stats.total_unspendables_unclaimed_rewards = entry.total_unspendables_unclaimed_rewards;
+
+    return stats;
 }
 
 bool CoinStatsIndex::Init()

src/index/coinstatsindex.h

@@ -56,7 +56,7 @@ class CoinStatsIndex final : public BaseIndex
     explicit CoinStatsIndex(size_t n_cache_size, bool f_memory = false, bool f_wipe = false);
 
     // Look up stats for a specific block using CBlockIndex
-    bool LookUpStats(const CBlockIndex* block_index, node::CCoinsStats& coins_stats) const;
+    std::optional<node::CCoinsStats> LookUpStats(const CBlockIndex* block_index) const;
 };
 
 /// The global UTXO set hash object.

src/node/coinstats.cpp

@@ -187,33 +187,15 @@ static void FinalizeHash(MuHash3072& muhash, CCoinsStats& stats)
 }
 static void FinalizeHash(std::nullptr_t, CCoinsStats& stats) {}
 
-std::optional<CCoinsStats> LookupUTXOStatsWithIndex(CoinStatsIndex& coin_stats_index, const CBlockIndex* pindex)
-{
-    CCoinsStats stats{Assert(pindex)->nHeight, pindex->GetBlockHash()};
-
-    stats.index_used = true;
-    if (!coin_stats_index.LookUpStats(pindex, stats)) {
-        return std::nullopt;
-    }
-
-    return stats;
-}
-
-std::optional<CCoinsStats> LookupUTXOStatsWithIndex(CoinStatsIndex& coin_stats_index, CCoinsView* view, BlockManager& blockman)
-{
-    CBlockIndex* pindex = WITH_LOCK(::cs_main, return blockman.LookupBlockIndex(view->GetBestBlock()));
-
-    return LookupUTXOStatsWithIndex(coin_stats_index, pindex);
-}
-
 std::optional<CCoinsStats> GetUTXOStats(CCoinsView* view, BlockManager& blockman, CoinStatsHashType hash_type, const std::function<void()>& interruption_point, const CBlockIndex* pindex, bool index_requested)
 {
     // Use CoinStatsIndex if it is requested and available and a hash_type of Muhash or None was requested
     if ((hash_type == CoinStatsHashType::MUHASH || hash_type == CoinStatsHashType::NONE) && g_coin_stats_index && index_requested) {
         if (pindex) {
-            return LookupUTXOStatsWithIndex(*g_coin_stats_index, pindex);
+            return g_coin_stats_index->LookUpStats(pindex);
         } else {
-            return LookupUTXOStatsWithIndex(*g_coin_stats_index, view, blockman);
+            CBlockIndex* block_index = WITH_LOCK(::cs_main, return blockman.LookupBlockIndex(view->GetBestBlock()));
+            return g_coin_stats_index->LookUpStats(block_index);
         }
     }
 

src/test/coinstatsindex_tests.cpp

@@ -33,15 +33,14 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
 {
     CoinStatsIndex coin_stats_index{1 << 20, true};
 
-    CCoinsStats coin_stats{};
     const CBlockIndex* block_index;
     {
         LOCK(cs_main);
         block_index = m_node.chainman->ActiveChain().Tip();
     }
 
     // CoinStatsIndex should not be found before it is started.
-    BOOST_CHECK(!coin_stats_index.LookUpStats(block_index, coin_stats));
+    BOOST_CHECK(!coin_stats_index.LookUpStats(block_index));
 
     // BlockUntilSyncedToCurrentChain should return false before CoinStatsIndex
     // is started.
@@ -57,10 +56,10 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
         LOCK(cs_main);
         genesis_block_index = m_node.chainman->ActiveChain().Genesis();
     }
-    BOOST_CHECK(coin_stats_index.LookUpStats(genesis_block_index, coin_stats));
+    BOOST_CHECK(coin_stats_index.LookUpStats(genesis_block_index));
 
     // Check that CoinStatsIndex updates with new blocks.
-    coin_stats_index.LookUpStats(block_index, coin_stats);
+    BOOST_CHECK(coin_stats_index.LookUpStats(block_index));
 
     const CScript script_pub_key{CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG};
     std::vector<CMutableTransaction> noTxns;
@@ -69,13 +68,12 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
     // Let the CoinStatsIndex to catch up again.
     BOOST_CHECK(coin_stats_index.BlockUntilSyncedToCurrentChain());
 
-    CCoinsStats new_coin_stats{};
     const CBlockIndex* new_block_index;
     {
         LOCK(cs_main);
         new_block_index = m_node.chainman->ActiveChain().Tip();
     }
-    coin_stats_index.LookUpStats(new_block_index, new_coin_stats);
+    BOOST_CHECK(coin_stats_index.LookUpStats(new_block_index));
 
     BOOST_CHECK(block_index != new_block_index);