Merge #19328: Add gettxoutsetinfo hash_type option

40506bf93f test: Test gettxouttsetinfo hash_type option (Fabian Jahr)
f17a4d1c4d rpc: Add hash_type NONE to gettxoutsetinfo (Fabian Jahr)
a712cf6f68 rpc: gettxoutsetinfo can specify hash_type (only legacy option for now) (Fabian Jahr)
605884ef21 refactor: Extract GetBogoSize function (Fabian Jahr)

Pull request description:

  This is another intermediate part of the Coinstats Index (tracked in #18000).

  Sjors suggested [here](https://github.com/bitcoin/bitcoin/pull/18000#issuecomment-641423019) that the part of the changes in #19145 that don't rely on the new `hash_type` muhash, i.e. that are for `hash_type=none`, could be merged separately from everything involving muhash. So these changes are extracted from #19145 here and can be merged without any other requirements.

  Building the index with no UTXO set hash is still valuable because `gettxoutsetinfo` can still be used to audit the `total_amount` for example. By itself this PR is not a huge improvement, `hash_type=none` is speeding up `gettxoutsetinfo` by about 10%, but it enables the implementation of an index on top of it in a follow-up and that means large parts of the index code of Coinstats Index can be merged while reviews for the hashing algorithm might take longer.

ACKs for top commit:
  MarcoFalke:
    ACK 40506bf93f 🖨
  Sjors:
    tACK 40506bf93f

Tree-SHA512: 3964c2b8eed427511b1aa9b2ef285dff27dc4d1537d72c3911e435b6e6b40912232da4acb3a09bd19a0372ddffa44103388d8a650169d95a4a727b970d210add

src/node/coinstats.cpp

@@ -8,13 +8,23 @@
 #include <coins.h>
 #include <hash.h>
 #include <serialize.h>
-#include <validation.h>
 #include <uint256.h>
 #include <util/system.h>
+#include <validation.h>
 
 #include <map>
 
-static void ApplyStats(CCoinsStats &stats, CHashWriter& ss, const uint256& hash, const std::map<uint32_t, Coin>& outputs)
+static uint64_t GetBogoSize(const CScript& scriptPubKey)
+{
+    return 32 /* txid */ +
+           4 /* vout index */ +
+           4 /* height + coinbase */ +
+           8 /* amount */ +
+           2 /* scriptPubKey len */ +
+           scriptPubKey.size() /* scriptPubKey */;
+}
+
+static void ApplyStats(CCoinsStats& stats, CHashWriter& ss, const uint256& hash, const std::map<uint32_t, Coin>& outputs)
 {
     assert(!outputs.empty());
     ss << hash;
@@ -26,26 +36,38 @@ static void ApplyStats(CCoinsStats &stats, CHashWriter& ss, const uint256& hash,
         ss << VARINT_MODE(output.second.out.nValue, VarIntMode::NONNEGATIVE_SIGNED);
         stats.nTransactionOutputs++;
         stats.nTotalAmount += output.second.out.nValue;
-        stats.nBogoSize += 32 /* txid */ + 4 /* vout index */ + 4 /* height + coinbase */ + 8 /* amount */ +
-                           2 /* scriptPubKey len */ + output.second.out.scriptPubKey.size() /* scriptPubKey */;
+        stats.nBogoSize += GetBogoSize(output.second.out.scriptPubKey);
     }
     ss << VARINT(0u);
 }
 
+static void ApplyStats(CCoinsStats& stats, std::nullptr_t, const uint256& hash, const std::map<uint32_t, Coin>& outputs)
+{
+    assert(!outputs.empty());
+    stats.nTransactions++;
+    for (const auto& output : outputs) {
+        stats.nTransactionOutputs++;
+        stats.nTotalAmount += output.second.out.nValue;
+        stats.nBogoSize += GetBogoSize(output.second.out.scriptPubKey);
+    }
+}
+
 //! Calculate statistics about the unspent transaction output set
-bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const std::function<void()>& interruption_point)
+template <typename T>
+static bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const std::function<void()>& interruption_point)
 {
     stats = CCoinsStats();
     std::unique_ptr<CCoinsViewCursor> pcursor(view->Cursor());
     assert(pcursor);
 
-    CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION);
     stats.hashBlock = pcursor->GetBestBlock();
     {
         LOCK(cs_main);
         stats.nHeight = LookupBlockIndex(stats.hashBlock)->nHeight;
     }
-    ss << stats.hashBlock;
+
+    PrepareHash(hash_obj, stats);
+
     uint256 prevkey;
     std::map<uint32_t, Coin> outputs;
     while (pcursor->Valid()) {
@@ -54,7 +76,7 @@ bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const std::function<void
         Coin coin;
         if (pcursor->GetKey(key) && pcursor->GetValue(coin)) {
             if (!outputs.empty() && key.hash != prevkey) {
-                ApplyStats(stats, ss, prevkey, outputs);
+                ApplyStats(stats, hash_obj, prevkey, outputs);
                 outputs.clear();
             }
             prevkey = key.hash;
@@ -66,9 +88,38 @@ bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const std::function<void
         pcursor->Next();
     }
     if (!outputs.empty()) {
-        ApplyStats(stats, ss, prevkey, outputs);
+        ApplyStats(stats, hash_obj, prevkey, outputs);
     }
-    stats.hashSerialized = ss.GetHash();
+
+    FinalizeHash(hash_obj, stats);
+
     stats.nDiskSize = view->EstimateSize();
     return true;
 }
+
+bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, CoinStatsHashType hash_type, const std::function<void()>& interruption_point)
+{
+    switch (hash_type) {
+    case(CoinStatsHashType::HASH_SERIALIZED): {
+        CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION);
+        return GetUTXOStats(view, stats, ss, interruption_point);
+    }
+    case(CoinStatsHashType::NONE): {
+        return GetUTXOStats(view, stats, nullptr, interruption_point);
+    }
+    } // no default case, so the compiler can warn about missing cases
+    assert(false);
+}
+
+// The legacy hash serializes the hashBlock
+static void PrepareHash(CHashWriter& ss, CCoinsStats& stats)
+{
+    ss << stats.hashBlock;
+}
+static void PrepareHash(std::nullptr_t, CCoinsStats& stats) {}
+
+static void FinalizeHash(CHashWriter& ss, CCoinsStats& stats)
+{
+    stats.hashSerialized = ss.GetHash();
+}
+static void FinalizeHash(std::nullptr_t, CCoinsStats& stats) {}

src/node/coinstats.h

@@ -14,6 +14,11 @@
 
 class CCoinsView;
 
+enum class CoinStatsHashType {
+    HASH_SERIALIZED,
+    NONE,
+};
+
 struct CCoinsStats
 {
     int nHeight{0};
@@ -30,6 +35,6 @@ struct CCoinsStats
 };
 
 //! Calculate statistics about the unspent transaction output set
-bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const std::function<void()>& interruption_point = {});
+bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const CoinStatsHashType hash_type, const std::function<void()>& interruption_point = {});
 
 #endif // BITCOIN_NODE_COINSTATS_H

src/rpc/blockchain.cpp

@@ -975,7 +975,9 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request)
             RPCHelpMan{"gettxoutsetinfo",
                 "\nReturns statistics about the unspent transaction output set.\n"
                 "Note this call may take some time.\n",
-                {},
+                {
+                    {"hash_type", RPCArg::Type::STR, /* default */ "hash_serialized_2", "Which UTXO set hash should be calculated. Options: 'hash_serialized_2' (the legacy algorithm), 'none'."},
+                },
                 RPCResult{
                     RPCResult::Type::OBJ, "", "",
                     {
@@ -984,7 +986,7 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request)
                         {RPCResult::Type::NUM, "transactions", "The number of transactions with unspent outputs"},
                         {RPCResult::Type::NUM, "txouts", "The number of unspent transaction outputs"},
                         {RPCResult::Type::NUM, "bogosize", "A meaningless metric for UTXO set size"},
-                        {RPCResult::Type::STR_HEX, "hash_serialized_2", "The serialized hash"},
+                        {RPCResult::Type::STR_HEX, "hash_serialized_2", "The serialized hash (only present if 'hash_serialized_2' hash_type is chosen)"},
                         {RPCResult::Type::NUM, "disk_size", "The estimated size of the chainstate on disk"},
                         {RPCResult::Type::STR_AMOUNT, "total_amount", "The total amount"},
                     }},
@@ -999,14 +1001,18 @@ static UniValue gettxoutsetinfo(const JSONRPCRequest& request)
     CCoinsStats stats;
     ::ChainstateActive().ForceFlushStateToDisk();
 
+    const CoinStatsHashType hash_type = ParseHashType(request.params[0], CoinStatsHashType::HASH_SERIALIZED);
+
     CCoinsView* coins_view = WITH_LOCK(cs_main, return &ChainstateActive().CoinsDB());
-    if (GetUTXOStats(coins_view, stats, RpcInterruptionPoint)) {
+    if (GetUTXOStats(coins_view, stats, hash_type, RpcInterruptionPoint)) {
         ret.pushKV("height", (int64_t)stats.nHeight);
         ret.pushKV("bestblock", stats.hashBlock.GetHex());
         ret.pushKV("transactions", (int64_t)stats.nTransactions);
         ret.pushKV("txouts", (int64_t)stats.nTransactionOutputs);
         ret.pushKV("bogosize", (int64_t)stats.nBogoSize);
-        ret.pushKV("hash_serialized_2", stats.hashSerialized.GetHex());
+        if (hash_type == CoinStatsHashType::HASH_SERIALIZED) {
+            ret.pushKV("hash_serialized_2", stats.hashSerialized.GetHex());
+        }
         ret.pushKV("disk_size", stats.nDiskSize);
         ret.pushKV("total_amount", ValueFromAmount(stats.nTotalAmount));
     } else {
@@ -2317,7 +2323,7 @@ UniValue dumptxoutset(const JSONRPCRequest& request)
 
         ::ChainstateActive().ForceFlushStateToDisk();
 
-        if (!GetUTXOStats(&::ChainstateActive().CoinsDB(), stats, RpcInterruptionPoint)) {
+        if (!GetUTXOStats(&::ChainstateActive().CoinsDB(), stats, CoinStatsHashType::NONE, RpcInterruptionPoint)) {
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set");
         }
 
@@ -2378,7 +2384,7 @@ static const CRPCCommand commands[] =
     { "blockchain",         "getmempoolinfo",         &getmempoolinfo,         {} },
     { "blockchain",         "getrawmempool",          &getrawmempool,          {"verbose"} },
     { "blockchain",         "gettxout",               &gettxout,               {"txid","n","include_mempool"} },
-    { "blockchain",         "gettxoutsetinfo",        &gettxoutsetinfo,        {} },
+    { "blockchain",         "gettxoutsetinfo",        &gettxoutsetinfo,        {"hash_type"} },
     { "blockchain",         "pruneblockchain",        &pruneblockchain,        {"height"} },
     { "blockchain",         "savemempool",            &savemempool,            {} },
     { "blockchain",         "verifychain",            &verifychain,            {"checklevel","nblocks"} },

src/rpc/util.cpp

@@ -113,6 +113,23 @@ std::vector<unsigned char> ParseHexO(const UniValue& o, std::string strKey)
     return ParseHexV(find_value(o, strKey), strKey);
 }
 
+CoinStatsHashType ParseHashType(const UniValue& param, const CoinStatsHashType default_type)
+{
+    if (param.isNull()) {
+        return default_type;
+    } else {
+        std::string hash_type_input = param.get_str();
+
+        if (hash_type_input == "hash_serialized_2") {
+            return CoinStatsHashType::HASH_SERIALIZED;
+        } else if (hash_type_input == "none") {
+            return CoinStatsHashType::NONE;
+        } else {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%d is not a valid hash_type", hash_type_input));
+        }
+    }
+}
+
 std::string HelpExampleCli(const std::string& methodname, const std::string& args)
 {
     return "> bitcoin-cli " + methodname + " " + args + "\n";

src/rpc/util.h

@@ -5,6 +5,7 @@
 #ifndef BITCOIN_RPC_UTIL_H
 #define BITCOIN_RPC_UTIL_H
 
+#include <node/coinstats.h>
 #include <node/transaction.h>
 #include <outputtype.h>
 #include <protocol.h>
@@ -77,6 +78,8 @@ extern uint256 ParseHashO(const UniValue& o, std::string strKey);
 extern std::vector<unsigned char> ParseHexV(const UniValue& v, std::string strName);
 extern std::vector<unsigned char> ParseHexO(const UniValue& o, std::string strKey);
 
+CoinStatsHashType ParseHashType(const UniValue& param, const CoinStatsHashType default_type);
+
 extern CAmount AmountFromValue(const UniValue& value);
 extern std::string HelpExampleCli(const std::string& methodname, const std::string& args);
 extern std::string HelpExampleRpc(const std::string& methodname, const std::string& args);

src/test/fuzz/coins_view.cpp

@@ -278,7 +278,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
             CCoinsStats stats;
             bool expected_code_path = false;
             try {
-                (void)GetUTXOStats(&coins_view_cache, stats);
+                (void)GetUTXOStats(&coins_view_cache, stats, CoinStatsHashType::HASH_SERIALIZED);
             } catch (const std::logic_error&) {
                 expected_code_path = true;
             }

test/functional/rpc_blockchain.py

@@ -241,6 +241,17 @@ class BlockchainTest(BitcoinTestFramework):
         del res['disk_size'], res3['disk_size']
         assert_equal(res, res3)
 
+        self.log.info("Test hash_type option for gettxoutsetinfo()")
+        # Adding hash_type 'hash_serialized_2', which is the default, should
+        # not change the result.
+        res4 = node.gettxoutsetinfo(hash_type='hash_serialized_2')
+        del res4['disk_size']
+        assert_equal(res, res4)
+
+        # hash_type none should not return a UTXO set hash.
+        res5 = node.gettxoutsetinfo(hash_type='none')
+        assert 'hash_serialized_2' not in res5
+
     def _test_getblockheader(self):
         node = self.nodes[0]