Merge bitcoin#25222: refactor: Pass reference to LookUpStats

faa3d38 refactor: Pass reference to LookUpStats (MacroFake)

Pull request description:

  I find it confusing to have an interface that accepts nullptr, but immediately crashes the program when someone does pass nullptr.

  Fix that.

  Also some include fixups.

ACKs for top commit:
  aureleoules:
    ACK faa3d38

Tree-SHA512: f90b649e9991e137b83a9899258ee73605719c081a6b789ac27fe7fe73eb70fbb41d89479bcd536d5c3ad788a5795de8451bc1b94e5c9267dcf9636d9e4a1109

Author: MacroFake

src/index/coinstatsindex.cpp

@@ -6,6 +6,7 @@
 #include <coins.h>
 #include <crypto/muhash.h>
 #include <index/coinstatsindex.h>
+#include <kernel/coinstats.h>
 #include <node/blockstorage.h>
 #include <serialize.h>
 #include <txdb.h>
@@ -322,13 +323,13 @@ static bool LookUpOne(const CDBWrapper& db, const interfaces::BlockKey& block, D
     return db.Read(DBHashKey(block.hash), result);
 }
 
-std::optional<CCoinsStats> CoinStatsIndex::LookUpStats(const CBlockIndex* block_index) const
+std::optional<CCoinsStats> CoinStatsIndex::LookUpStats(const CBlockIndex& block_index) const
 {
-    CCoinsStats stats{Assert(block_index)->nHeight, block_index->GetBlockHash()};
+    CCoinsStats stats{block_index.nHeight, block_index.GetBlockHash()};
     stats.index_used = true;
 
     DBVal entry;
-    if (!LookUpOne(*m_db, {block_index->GetBlockHash(), block_index->nHeight}, entry)) {
+    if (!LookUpOne(*m_db, {block_index.GetBlockHash(), block_index.nHeight}, entry)) {
         return std::nullopt;
     }
 

src/index/coinstatsindex.h

@@ -5,11 +5,14 @@
 #ifndef BITCOIN_INDEX_COINSTATSINDEX_H
 #define BITCOIN_INDEX_COINSTATSINDEX_H
 
-#include <chain.h>
 #include <crypto/muhash.h>
-#include <flatfile.h>
 #include <index/base.h>
-#include <kernel/coinstats.h>
+
+class CBlockIndex;
+class CDBBatch;
+namespace kernel {
+struct CCoinsStats;
+}
 
 /**
  * CoinStatsIndex maintains statistics on the UTXO set.
@@ -56,7 +59,7 @@ class CoinStatsIndex final : public BaseIndex
     explicit CoinStatsIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory = false, bool f_wipe = false);
 
     // Look up stats for a specific block using CBlockIndex
-    std::optional<kernel::CCoinsStats> LookUpStats(const CBlockIndex* block_index) const;
+    std::optional<kernel::CCoinsStats> LookUpStats(const CBlockIndex& block_index) const;
 };
 
 /// The global UTXO set hash object.

src/rpc/blockchain.cpp

@@ -833,9 +833,9 @@ static std::optional<kernel::CCoinsStats> GetUTXOStats(CCoinsView* view, node::B
     // Use CoinStatsIndex if it is requested and available and a hash_type of Muhash or None was requested
     if ((hash_type == kernel::CoinStatsHashType::MUHASH || hash_type == kernel::CoinStatsHashType::NONE) && g_coin_stats_index && index_requested) {
         if (pindex) {
-            return g_coin_stats_index->LookUpStats(pindex);
+            return g_coin_stats_index->LookUpStats(*pindex);
         } else {
-            CBlockIndex* block_index = WITH_LOCK(::cs_main, return blockman.LookupBlockIndex(view->GetBestBlock()));
+            CBlockIndex& block_index = *CHECK_NONFATAL(WITH_LOCK(::cs_main, return blockman.LookupBlockIndex(view->GetBestBlock())));
             return g_coin_stats_index->LookUpStats(block_index);
         }
     }

src/test/coinstatsindex_tests.cpp

@@ -5,6 +5,7 @@
 #include <chainparams.h>
 #include <index/coinstatsindex.h>
 #include <interfaces/chain.h>
+#include <kernel/coinstats.h>
 #include <test/util/setup_common.h>
 #include <test/util/validation.h>
 #include <util/time.h>
@@ -38,7 +39,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
     }
 
     // CoinStatsIndex should not be found before it is started.
-    BOOST_CHECK(!coin_stats_index.LookUpStats(block_index));
+    BOOST_CHECK(!coin_stats_index.LookUpStats(*block_index));
 
     // BlockUntilSyncedToCurrentChain should return false before CoinStatsIndex
     // is started.
@@ -54,10 +55,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));
+    BOOST_CHECK(coin_stats_index.LookUpStats(*genesis_block_index));
 
     // Check that CoinStatsIndex updates with new blocks.
-    BOOST_CHECK(coin_stats_index.LookUpStats(block_index));
+    BOOST_CHECK(coin_stats_index.LookUpStats(*block_index));
 
     const CScript script_pub_key{CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG};
     std::vector<CMutableTransaction> noTxns;
@@ -71,7 +72,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
         LOCK(cs_main);
         new_block_index = m_node.chainman->ActiveChain().Tip();
     }
-    BOOST_CHECK(coin_stats_index.LookUpStats(new_block_index));
+    BOOST_CHECK(coin_stats_index.LookUpStats(*new_block_index));
 
     BOOST_CHECK(block_index != new_block_index);