diff options
author | Carl Dong <contact@carldong.me> | 2022-02-15 18:37:32 -0500 |
---|---|---|
committer | Carl Dong <contact@carldong.me> | 2022-05-23 14:50:35 -0400 |
commit | 524463daf6a10b20a4e20116a68101a684929eda (patch) | |
tree | 257ecace292cba8614f8b1e4592f524561293016 /src | |
parent | 46eb9fc56a296a2acea10ec7e5bf7b1827f73c45 (diff) | |
download | bitcoin-524463daf6a10b20a4e20116a68101a684929eda.tar.xz |
coinstats: Return purely out-param CCoinsStats
In previous commits in this patchset, we removed all in-param members of
CCoinsStats. Now that that's done, we can modify GetUTXOStats to return
an optional CCoinsStats instead of a status bool. Callers are modified
accordingly.
In rpc/blockchain.cpp, we discover that GetUTXOStats' status bool when
getting UTXO stats for pprev was not checked for error. We fix this as
well.
Diffstat (limited to 'src')
-rw-r--r-- | src/node/coinstats.cpp | 38 | ||||
-rw-r--r-- | src/node/coinstats.h | 10 | ||||
-rw-r--r-- | src/rpc/blockchain.cpp | 25 | ||||
-rw-r--r-- | src/validation.cpp | 8 |
4 files changed, 48 insertions, 33 deletions
diff --git a/src/node/coinstats.cpp b/src/node/coinstats.cpp index 13f1041a51..8fc53fe172 100644 --- a/src/node/coinstats.cpp +++ b/src/node/coinstats.cpp @@ -9,6 +9,7 @@ #include <crypto/muhash.h> #include <hash.h> #include <index/coinstatsindex.h> +#include <optional> #include <serialize.h> #include <uint256.h> #include <util/overflow.h> @@ -144,22 +145,31 @@ static bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats& return true; } -bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats& stats, CoinStatsHashType hash_type, const std::function<void()>& interruption_point, const CBlockIndex* pindex, bool index_requested) +std::optional<CCoinsStats> GetUTXOStats(CCoinsView* view, BlockManager& blockman, CoinStatsHashType hash_type, const std::function<void()>& interruption_point, const CBlockIndex* pindex, bool index_requested) { - switch (hash_type) { - case(CoinStatsHashType::HASH_SERIALIZED): { - CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION); - return GetUTXOStats(view, blockman, stats, ss, interruption_point, pindex, hash_type, index_requested); - } - case(CoinStatsHashType::MUHASH): { - MuHash3072 muhash; - return GetUTXOStats(view, blockman, stats, muhash, interruption_point, pindex, hash_type, index_requested); - } - case(CoinStatsHashType::NONE): { - return GetUTXOStats(view, blockman, stats, nullptr, interruption_point, pindex, hash_type, index_requested); + CCoinsStats stats{}; + + bool success = [&]() -> bool { + switch (hash_type) { + case(CoinStatsHashType::HASH_SERIALIZED): { + CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION); + return GetUTXOStats(view, blockman, stats, ss, interruption_point, pindex, hash_type, index_requested); + } + case(CoinStatsHashType::MUHASH): { + MuHash3072 muhash; + return GetUTXOStats(view, blockman, stats, muhash, interruption_point, pindex, hash_type, index_requested); + } + case(CoinStatsHashType::NONE): { + return GetUTXOStats(view, blockman, stats, nullptr, interruption_point, pindex, hash_type, index_requested); + } + } // no default case, so the compiler can warn about missing cases + assert(false); + }(); + + if (!success) { + return std::nullopt; } - } // no default case, so the compiler can warn about missing cases - assert(false); + return stats; } // The legacy hash serializes the hashBlock diff --git a/src/node/coinstats.h b/src/node/coinstats.h index adc67eb0a6..36cbf19f29 100644 --- a/src/node/coinstats.h +++ b/src/node/coinstats.h @@ -71,11 +71,11 @@ struct CCoinsStats { * * @param[in] index_requested Signals if the coinstatsindex should be used (when available). */ -bool GetUTXOStats(CCoinsView* view, node::BlockManager& blockman, - CCoinsStats& stats, CoinStatsHashType hash_type, - const std::function<void()>& interruption_point = {}, - const CBlockIndex* pindex = nullptr, - bool index_requested = true); +std::optional<CCoinsStats> GetUTXOStats(CCoinsView* view, node::BlockManager& blockman, + CoinStatsHashType hash_type, + const std::function<void()>& interruption_point = {}, + const CBlockIndex* pindex = nullptr, + bool index_requested = true); uint64_t GetBogoSize(const CScript& script_pub_key); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 98c2deb4bf..97983cea5a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -862,7 +862,6 @@ static RPCHelpMan gettxoutsetinfo() const CBlockIndex* pindex{nullptr}; const CoinStatsHashType hash_type{request.params[0].isNull() ? CoinStatsHashType::HASH_SERIALIZED : ParseHashType(request.params[0].get_str())}; - CCoinsStats stats{}; bool index_requested = request.params[2].isNull() || request.params[2].get_bool(); NodeContext& node = EnsureAnyNodeContext(request.context); @@ -903,7 +902,9 @@ static RPCHelpMan gettxoutsetinfo() } } - if (GetUTXOStats(coins_view, *blockman, stats, hash_type, node.rpc_interruption_point, pindex, index_requested)) { + const std::optional<CCoinsStats> maybe_stats = GetUTXOStats(coins_view, *blockman, hash_type, node.rpc_interruption_point, pindex, index_requested); + if (maybe_stats.has_value()) { + const CCoinsStats& stats = maybe_stats.value(); ret.pushKV("height", (int64_t)stats.nHeight); ret.pushKV("bestblock", stats.hashBlock.GetHex()); ret.pushKV("txouts", (int64_t)stats.nTransactionOutputs); @@ -923,9 +924,12 @@ static RPCHelpMan gettxoutsetinfo() ret.pushKV("total_unspendable_amount", ValueFromAmount(stats.total_unspendable_amount)); CCoinsStats prev_stats{}; - if (pindex->nHeight > 0) { - GetUTXOStats(coins_view, *blockman, prev_stats, hash_type, node.rpc_interruption_point, pindex->pprev, index_requested); + const std::optional<CCoinsStats> maybe_prev_stats = GetUTXOStats(coins_view, *blockman, hash_type, node.rpc_interruption_point, pindex->pprev, index_requested); + if (!maybe_prev_stats) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set"); + } + prev_stats = maybe_prev_stats.value(); } UniValue block_info(UniValue::VOBJ); @@ -2285,7 +2289,7 @@ UniValue CreateUTXOSnapshot( const fs::path& temppath) { std::unique_ptr<CCoinsViewCursor> pcursor; - CCoinsStats stats{}; + std::optional<CCoinsStats> maybe_stats; const CBlockIndex* tip; { @@ -2305,19 +2309,20 @@ UniValue CreateUTXOSnapshot( chainstate.ForceFlushStateToDisk(); - if (!GetUTXOStats(&chainstate.CoinsDB(), chainstate.m_blockman, stats, CoinStatsHashType::HASH_SERIALIZED, node.rpc_interruption_point)) { + maybe_stats = GetUTXOStats(&chainstate.CoinsDB(), chainstate.m_blockman, CoinStatsHashType::HASH_SERIALIZED, node.rpc_interruption_point); + if (!maybe_stats) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Unable to read UTXO set"); } pcursor = chainstate.CoinsDB().Cursor(); - tip = CHECK_NONFATAL(chainstate.m_blockman.LookupBlockIndex(stats.hashBlock)); + tip = CHECK_NONFATAL(chainstate.m_blockman.LookupBlockIndex(maybe_stats->hashBlock)); } LOG_TIME_SECONDS(strprintf("writing UTXO snapshot at height %s (%s) to file %s (via %s)", tip->nHeight, tip->GetBlockHash().ToString(), fs::PathToString(path), fs::PathToString(temppath))); - SnapshotMetadata metadata{tip->GetBlockHash(), stats.coins_count, tip->nChainTx}; + SnapshotMetadata metadata{tip->GetBlockHash(), maybe_stats->coins_count, tip->nChainTx}; afile << metadata; @@ -2339,11 +2344,11 @@ UniValue CreateUTXOSnapshot( afile.fclose(); UniValue result(UniValue::VOBJ); - result.pushKV("coins_written", stats.coins_count); + result.pushKV("coins_written", maybe_stats->coins_count); result.pushKV("base_hash", tip->GetBlockHash().ToString()); result.pushKV("base_height", tip->nHeight); result.pushKV("path", path.u8string()); - result.pushKV("txoutset_hash", stats.hashSerialized.ToString()); + result.pushKV("txoutset_hash", maybe_stats->hashSerialized.ToString()); // Cast required because univalue doesn't have serialization specified for // `unsigned int`, nChainTx's type. result.pushKV("nchaintx", uint64_t{tip->nChainTx}); diff --git a/src/validation.cpp b/src/validation.cpp index 5fdf0398df..40c2618db9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5095,22 +5095,22 @@ bool ChainstateManager::PopulateAndValidateSnapshot( assert(coins_cache.GetBestBlock() == base_blockhash); - CCoinsStats stats{}; auto breakpoint_fnc = [] { /* TODO insert breakpoint here? */ }; // As above, okay to immediately release cs_main here since no other context knows // about the snapshot_chainstate. CCoinsViewDB* snapshot_coinsdb = WITH_LOCK(::cs_main, return &snapshot_chainstate.CoinsDB()); - if (!GetUTXOStats(snapshot_coinsdb, m_blockman, stats, CoinStatsHashType::HASH_SERIALIZED, breakpoint_fnc)) { + const std::optional<CCoinsStats> maybe_stats = GetUTXOStats(snapshot_coinsdb, m_blockman, CoinStatsHashType::HASH_SERIALIZED, breakpoint_fnc); + if (!maybe_stats.has_value()) { LogPrintf("[snapshot] failed to generate coins stats\n"); return false; } // Assert that the deserialized chainstate contents match the expected assumeutxo value. - if (AssumeutxoHash{stats.hashSerialized} != au_data.hash_serialized) { + if (AssumeutxoHash{maybe_stats->hashSerialized} != au_data.hash_serialized) { LogPrintf("[snapshot] bad snapshot content hash: expected %s, got %s\n", - au_data.hash_serialized.ToString(), stats.hashSerialized.ToString()); + au_data.hash_serialized.ToString(), maybe_stats->hashSerialized.ToString()); return false; } |