merge bitcoin#23411: Avoid integer overflow in ApplyStats when activating snapshot

Author: kwvg

src/Makefile.am

@@ -323,6 +323,7 @@ BITCOIN_CORE_H = \
   util/macros.h \
   util/message.h \
   util/moneystr.h \
+  util/overflow.h \
   util/ranges.h \
   util/readwritefile.h \
   util/underlying.h \

src/index/coinstatsindex.cpp

@@ -321,7 +321,7 @@ bool CoinStatsIndex::LookUpStats(const CBlockIndex* block_index, CCoinsStats& co
     coins_stats.hashSerialized = entry.muhash;
     coins_stats.nTransactionOutputs = entry.transaction_output_count;
     coins_stats.nBogoSize = entry.bogo_size;
-    coins_stats.nTotalAmount = entry.total_amount;
+    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;

src/init.cpp

@@ -885,7 +885,9 @@ static void PeriodicStats(ArgsManager& args, const CTxMemPool& mempool)
         statsClient.gauge("utxoset.txOutputs", stats.nTransactionOutputs, 1.0f);
         statsClient.gauge("utxoset.dbSizeBytes", stats.nDiskSize, 1.0f);
         statsClient.gauge("utxoset.blockHeight", stats.nHeight, 1.0f);
-        statsClient.gauge("utxoset.totalAmount", (double)stats.nTotalAmount / (double)COIN, 1.0f);
+        if (stats.total_amount.has_value()) {
+            statsClient.gauge("utxoset.totalAmount", (double)stats.total_amount.value() / (double)COIN, 1.0f);
+        }
     } else {
         // something went wrong
         LogPrintf("%s: GetUTXOStats failed\n", __func__);

src/node/coinstats.cpp

@@ -11,8 +11,8 @@
 #include <index/coinstatsindex.h>
 #include <serialize.h>
 #include <uint256.h>
-// #include <util/system.h>
 #include <util/check.h>
+#include <util/overflow.h>
 #include <util/system.h>
 #include <validation.h>
 
@@ -83,7 +83,9 @@ static void ApplyStats(CCoinsStats& stats, const uint256& hash, const std::map<u
     stats.nTransactions++;
     for (auto it = outputs.begin(); it != outputs.end(); ++it) {
         stats.nTransactionOutputs++;
-        stats.nTotalAmount += it->second.out.nValue;
+        if (stats.total_amount.has_value()) {
+            stats.total_amount = CheckedAdd(*stats.total_amount, it->second.out.nValue);
+        }
         stats.nBogoSize += GetBogoSize(it->second.out.scriptPubKey);
     }
 }
@@ -94,7 +96,9 @@ static void ApplyStats(CCoinsStats& stats, std::nullptr_t, const uint256& hash,
     stats.nTransactions++;
     for (const auto& output : outputs) {
         stats.nTransactionOutputs++;
-        stats.nTotalAmount += output.second.out.nValue;
+        if (stats.total_amount.has_value()) {
+            stats.total_amount = CheckedAdd(*stats.total_amount, output.second.out.nValue);
+        }
         stats.nBogoSize += GetBogoSize(output.second.out.scriptPubKey);
     }
 }
@@ -107,11 +111,9 @@ static bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats&
     assert(pcursor);
 
     if (!pindex) {
-        {
-            LOCK(cs_main);
-            assert(std::addressof(g_chainman.m_blockman) == std::addressof(blockman));
-            pindex = blockman.LookupBlockIndex(view->GetBestBlock());
-        }
+        LOCK(cs_main);
+        assert(std::addressof(g_chainman.m_blockman) == std::addressof(blockman));
+        pindex = blockman.LookupBlockIndex(view->GetBestBlock());
     }
     stats.nHeight = Assert(pindex)->nHeight;
     stats.hashBlock = pindex->GetBlockHash();

src/node/coinstats.h

@@ -24,17 +24,19 @@ enum class CoinStatsHashType {
     NONE,
 };
 
-struct CCoinsStats
-{
-    CoinStatsHashType m_hash_type;
+struct CCoinsStats {
+    //! Which hash type to use
+    const CoinStatsHashType m_hash_type;
+
     int nHeight{0};
     uint256 hashBlock{};
     uint64_t nTransactions{0};
     uint64_t nTransactionOutputs{0};
     uint64_t nBogoSize{0};
     uint256 hashSerialized{};
     uint64_t nDiskSize{0};
-    CAmount nTotalAmount{0};
+    //! The total amount, or nullopt if an overflow occurred calculating it
+    std::optional<CAmount> total_amount{0};
 
     //! The number of coins contained.
     uint64_t coins_count{0};

src/rpc/blockchain.cpp

@@ -1423,9 +1423,10 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request)
             ret.pushKV("hash_serialized_2", stats.hashSerialized.GetHex());
         }
         if (hash_type == CoinStatsHashType::MUHASH) {
-              ret.pushKV("muhash", stats.hashSerialized.GetHex());
+            ret.pushKV("muhash", stats.hashSerialized.GetHex());
         }
-        ret.pushKV("total_amount", ValueFromAmount(stats.nTotalAmount));
+        CHECK_NONFATAL(stats.total_amount.has_value());
+        ret.pushKV("total_amount", ValueFromAmount(stats.total_amount.value()));
         if (!stats.index_used) {
             ret.pushKV("transactions", static_cast<int64_t>(stats.nTransactions));
             ret.pushKV("disk_size", stats.nDiskSize);

src/test/fuzz/addition_overflow.cpp

@@ -5,6 +5,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <util/overflow.h>
 
 #include <cstdint>
 #include <string>

src/test/fuzz/crypto_chacha20_poly1305_aead.cpp

@@ -7,6 +7,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <util/overflow.h>
 
 #include <cassert>
 #include <cstdint>

src/test/fuzz/integer.cpp

@@ -27,6 +27,7 @@
 #include <uint256.h>
 #include <util/check.h>
 #include <util/moneystr.h>
+#include <util/overflow.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/system.h>

src/test/fuzz/pow.cpp

@@ -9,6 +9,7 @@
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
+#include <util/overflow.h>
 
 #include <cstdint>
 #include <optional>