diff --git a/src/Makefile.am b/src/Makefile.am index d3eabbf2f3..559382876e 100644 --- a/src/Makefile.am +++ b/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 \ diff --git a/src/index/coinstatsindex.cpp b/src/index/coinstatsindex.cpp index 8321b7f17b..682550e1d6 100644 --- a/src/index/coinstatsindex.cpp +++ b/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; diff --git a/src/init.cpp b/src/init.cpp index ea2ca1590d..ef5a9399e6 100644 --- a/src/init.cpp +++ b/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__); diff --git a/src/node/coinstats.cpp b/src/node/coinstats.cpp index 8e82a284bd..f41adb160f 100644 --- a/src/node/coinstats.cpp +++ b/src/node/coinstats.cpp @@ -11,8 +11,8 @@ #include #include #include -// #include #include +#include #include #include @@ -83,7 +83,9 @@ static void ApplyStats(CCoinsStats& stats, const uint256& hash, const std::mapsecond.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(); diff --git a/src/node/coinstats.h b/src/node/coinstats.h index bcd3e55acd..dc4d062154 100644 --- a/src/node/coinstats.h +++ b/src/node/coinstats.h @@ -24,9 +24,10 @@ 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}; @@ -34,7 +35,8 @@ struct CCoinsStats 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 total_amount{0}; //! The number of coins contained. uint64_t coins_count{0}; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 63720810ee..e9020fcfae 100644 --- a/src/rpc/blockchain.cpp +++ b/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(stats.nTransactions)); ret.pushKV("disk_size", stats.nDiskSize); diff --git a/src/test/fuzz/addition_overflow.cpp b/src/test/fuzz/addition_overflow.cpp index 158ce6e756..8b435431dd 100644 --- a/src/test/fuzz/addition_overflow.cpp +++ b/src/test/fuzz/addition_overflow.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include diff --git a/src/test/fuzz/crypto_chacha20_poly1305_aead.cpp b/src/test/fuzz/crypto_chacha20_poly1305_aead.cpp index 1f122082b2..2f6469e589 100644 --- a/src/test/fuzz/crypto_chacha20_poly1305_aead.cpp +++ b/src/test/fuzz/crypto_chacha20_poly1305_aead.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include diff --git a/src/test/fuzz/integer.cpp b/src/test/fuzz/integer.cpp index 5754cd34c5..30351c398a 100644 --- a/src/test/fuzz/integer.cpp +++ b/src/test/fuzz/integer.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/fuzz/pow.cpp b/src/test/fuzz/pow.cpp index 02beb6eb37..fd0ca0644a 100644 --- a/src/test/fuzz/pow.cpp +++ b/src/test/fuzz/pow.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 8053d890b9..94b6ca76e5 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -3,6 +3,8 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include + +#include #include FuzzedSock::FuzzedSock(FuzzedDataProvider& fuzzed_data_provider) diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 2600c15884..bc0d48c24d 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -217,17 +218,6 @@ template } } -template -[[ nodiscard ]] bool AdditionOverflow(const T i, const T j) noexcept -{ - static_assert(std::is_integral::value, "Integral required."); - if (std::numeric_limits::is_signed) { - return (i > 0 && j > std::numeric_limits::max() - i) || - (i < 0 && j < std::numeric_limits::min() - i); - } - return std::numeric_limits::max() - i < j; -} - [[ nodiscard ]] inline bool ContainsSpentInput(const CTransaction& tx, const CCoinsViewCache& inputs) noexcept { for (const CTxIn& tx_in : tx.vin) { diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index bdac179816..dd84391585 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -15,6 +15,7 @@ #include #include // For MessageSign(), MessageVerify(), MESSAGE_MAGIC #include +#include #include #include #include @@ -1449,6 +1450,38 @@ BOOST_AUTO_TEST_CASE(test_IsDigit) BOOST_CHECK_EQUAL(IsDigit(9), false); } +/* Check for overflow */ +template +static void TestAddMatrixOverflow() +{ + constexpr T MAXI{std::numeric_limits::max()}; + BOOST_CHECK(!CheckedAdd(T{1}, MAXI)); + BOOST_CHECK(!CheckedAdd(MAXI, MAXI)); + BOOST_CHECK_EQUAL(0, CheckedAdd(T{0}, T{0}).value()); + BOOST_CHECK_EQUAL(MAXI, CheckedAdd(T{0}, MAXI).value()); + BOOST_CHECK_EQUAL(MAXI, CheckedAdd(T{1}, MAXI - 1).value()); +} + +/* Check for overflow or underflow */ +template +static void TestAddMatrix() +{ + TestAddMatrixOverflow(); + constexpr T MINI{std::numeric_limits::min()}; + constexpr T MAXI{std::numeric_limits::max()}; + BOOST_CHECK(!CheckedAdd(T{-1}, MINI)); + BOOST_CHECK(!CheckedAdd(MINI, MINI)); + BOOST_CHECK_EQUAL(MINI, CheckedAdd(T{0}, MINI).value()); + BOOST_CHECK_EQUAL(MINI, CheckedAdd(T{-1}, MINI + 1).value()); + BOOST_CHECK_EQUAL(-1, CheckedAdd(MINI, MAXI).value()); +} + +BOOST_AUTO_TEST_CASE(util_overflow) +{ + TestAddMatrixOverflow(); + TestAddMatrix(); +} + BOOST_AUTO_TEST_CASE(test_ParseInt32) { int32_t n; diff --git a/src/util/overflow.h b/src/util/overflow.h new file mode 100644 index 0000000000..5982af8d04 --- /dev/null +++ b/src/util/overflow.h @@ -0,0 +1,31 @@ +// Copyright (c) 2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_OVERFLOW_H +#define BITCOIN_UTIL_OVERFLOW_H + +#include +#include + +template +[[nodiscard]] bool AdditionOverflow(const T i, const T j) noexcept +{ + static_assert(std::is_integral::value, "Integral required."); + if (std::numeric_limits::is_signed) { + return (i > 0 && j > std::numeric_limits::max() - i) || + (i < 0 && j < std::numeric_limits::min() - i); + } + return std::numeric_limits::max() - i < j; +} + +template +[[nodiscard]] std::optional CheckedAdd(const T i, const T j) noexcept +{ + if (AdditionOverflow(i, j)) { + return std::nullopt; + } + return i + j; +} + +#endif // BITCOIN_UTIL_OVERFLOW_H