Skip to content

Commit 7008087

Browse files
committed
Merge bitcoin#24410: [kernel 2a/n] Split hashing/index GetUTXOStats codepaths, decouple from coinstatsindex
664a14b coinstats: Move GetUTXOStats to rpc/blockchain (Carl Dong) f100687 kernel: Use ComputeUTXOStats in validation (Carl Dong) faa5238 style-only: Rearrange using decls after scripted-diff (Carl Dong) f329a92 scripted-diff: Move src/kernel/coinstats to kernel:: (Carl Dong) 0e54456 Use only kernel/coinstats.h in index/coinstatsindex.h (Carl Dong) 8097098 coinstats: Split node/coinstats.h to kernel/coinstats.h (Carl Dong) 35f73ce coinstats: Move hasher codepath to kernel/coinstats (Carl Dong) b7634fe Move logic from LookupUTXOStatsWithIndex to CoinStatsIndex::LookUpStats (Carl Dong) 1352e41 coinstats: Separate hasher/index lookup codepaths (Carl Dong) 524463d coinstats: Return purely out-param CCoinsStats (Carl Dong) 46eb9fc coinstats: Extract index_requested in-member to in-param (Carl Dong) a789f3f coinstats: Extract hash_type in-member to in-param (Carl Dong) 1022948 includes: Remove rpc/util.h -> node/coinstats.h (Carl Dong) 0848db9 fuzz: Remove useless GetUTXOStats fuzz case (Carl Dong) 52b1939 kernel: Remove unnecessary blockfilter{index,}.cpp (Carl Dong) Pull request description: Part of: bitcoin#24303 Depends on: bitcoin#24322 The `GetUTXOStats` function has 2 codepaths: - One which queries the `CoinStatsIndex` for the UTXO hash - One which actually performs the hashing For `libbitcoinkernel`, the only place where we call `GetUTXOStats` is in `PopulateAndValidateSnapshots`, which uses the `SHA256D` hash, and is therefore unable to use the `CoinStatsIndex` since that only provides `MuHash` hashes. Not that I think indices necessarily belong in `libbitcoinkernel` anyway. This PR separates these 2 aforementioned codepaths of `GetUTXOStats`, uses the hashing codepath in `PopulateAndValidateSnapshots`, and removes the need to link in `index/coinstatsindex.cpp` and `node/coinstats.cpp`. ----- Logistically, this PR: - Extracts out the `index_requested` and `hash_type` members of `CoinStats`, which served as "in-params" to `GetUTXOStats` embedded within the `CoinStats` struct. This allows `CoinStats` to only consist of "out-param" members, and be returned by `GetUTXOStats` without needing to be an "in-out" param - Introduce the purely virtual `UTXOHashers` class, with 3 implementations: `SHA256DHasher`, `MuHashHasher`, and `NullHasher`. These replace the existing template-based polymorphism. - Split `GetUTXOStats` into: - `CalculateUTXOStatsWithHasher(UTXOHasher&, ...)`, and - `LookupUTXOStatsWithIndex(CoinStatsIndex&, ...)` - Use `CalculateUTXOStatsWithHasher` directly where appropriate (`src/validation.cpp` and `src/fuzz`) - Move `GetUTXOStats` to `rpc/blockchain`, which is the only place that depends on `GetUTXOStats`'s weird fallback behaviour - Move `LookupUTXOStatsWithIndex` to `index/coinstatsindex` Code organization: - `src/` - `kernel/` → only contains the hashing codepath - `coinstats.cpp` → hashing codepath implementations - `coinstats.h` → header for `kernel/coinstats.cpp` - `index/` → only contains the index codepath - `coinstatsindex.cpp` → index codepath implementations - `coinstatsindex.h` - `validation.cpp` → only uses the hashing codepath - `rpc/blockchain.cpp` → uses both the hashing and index codepath, old `GetUTXOStats` fallback logic moved here as static - `test/fuzz/coins_view.cpp` → only uses the hashing codepath TODOs: - [x] Commit messages could be fleshed out more Would love any feedback! ACKs for top commit: laanwj: Code review ACK 664a14b Tree-SHA512: 18722c7bd279174d2d1881fec33ea04a9b261aae1c12e998cf434ef297d8ded47de69c526c8033a2ba7abc93ba3d2ff5faf4ce05e8888c725c31cf885ce3ef73
2 parents 8898906 + 664a14b commit 7008087

File tree

11 files changed

+146
-132
lines changed

11 files changed

+146
-132
lines changed

src/Makefile.am

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ BITCOIN_CORE_H = \
172172
interfaces/node.h \
173173
interfaces/wallet.h \
174174
kernel/chainstatemanager_opts.h \
175+
kernel/coinstats.h \
175176
key.h \
176177
key_io.h \
177178
logging.h \
@@ -191,7 +192,6 @@ BITCOIN_CORE_H = \
191192
node/caches.h \
192193
node/chainstate.h \
193194
node/coin.h \
194-
node/coinstats.h \
195195
node/context.h \
196196
node/miner.h \
197197
node/minisketchwrapper.h \
@@ -357,6 +357,7 @@ libbitcoin_node_a_SOURCES = \
357357
index/coinstatsindex.cpp \
358358
index/txindex.cpp \
359359
init.cpp \
360+
kernel/coinstats.cpp \
360361
mapport.cpp \
361362
net.cpp \
362363
netgroup.cpp \
@@ -365,7 +366,6 @@ libbitcoin_node_a_SOURCES = \
365366
node/caches.cpp \
366367
node/chainstate.cpp \
367368
node/coin.cpp \
368-
node/coinstats.cpp \
369369
node/context.cpp \
370370
node/interfaces.cpp \
371371
node/miner.cpp \
@@ -853,7 +853,6 @@ endif
853853
libbitcoinkernel_la_SOURCES = \
854854
kernel/bitcoinkernel.cpp \
855855
arith_uint256.cpp \
856-
blockfilter.cpp \
857856
chain.cpp \
858857
chainparamsbase.cpp \
859858
chainparams.cpp \
@@ -871,15 +870,12 @@ libbitcoinkernel_la_SOURCES = \
871870
flatfile.cpp \
872871
fs.cpp \
873872
hash.cpp \
874-
index/base.cpp \
875-
index/blockfilterindex.cpp \
876-
index/coinstatsindex.cpp \
877873
init/common.cpp \
874+
kernel/coinstats.cpp \
878875
key.cpp \
879876
logging.cpp \
880877
node/blockstorage.cpp \
881878
node/chainstate.cpp \
882-
node/coinstats.cpp \
883879
node/ui_interface.cpp \
884880
policy/feerate.cpp \
885881
policy/fees.cpp \

src/index/coinstatsindex.cpp

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@
1212
#include <undo.h>
1313
#include <validation.h>
1414

15-
using node::CCoinsStats;
16-
using node::GetBogoSize;
15+
using kernel::CCoinsStats;
16+
using kernel::GetBogoSize;
17+
using kernel::TxOutSer;
18+
1719
using node::ReadBlockFromDisk;
18-
using node::TxOutSer;
1920
using node::UndoReadFromDisk;
2021

2122
static constexpr uint8_t DB_BLOCK_HASH{'s'};
@@ -316,28 +317,31 @@ static bool LookUpOne(const CDBWrapper& db, const CBlockIndex* block_index, DBVa
316317
return db.Read(DBHashKey(block_index->GetBlockHash()), result);
317318
}
318319

319-
bool CoinStatsIndex::LookUpStats(const CBlockIndex* block_index, CCoinsStats& coins_stats) const
320+
std::optional<CCoinsStats> CoinStatsIndex::LookUpStats(const CBlockIndex* block_index) const
320321
{
322+
CCoinsStats stats{Assert(block_index)->nHeight, block_index->GetBlockHash()};
323+
stats.index_used = true;
324+
321325
DBVal entry;
322326
if (!LookUpOne(*m_db, block_index, entry)) {
323-
return false;
327+
return std::nullopt;
324328
}
325329

326-
coins_stats.hashSerialized = entry.muhash;
327-
coins_stats.nTransactionOutputs = entry.transaction_output_count;
328-
coins_stats.nBogoSize = entry.bogo_size;
329-
coins_stats.total_amount = entry.total_amount;
330-
coins_stats.total_subsidy = entry.total_subsidy;
331-
coins_stats.total_unspendable_amount = entry.total_unspendable_amount;
332-
coins_stats.total_prevout_spent_amount = entry.total_prevout_spent_amount;
333-
coins_stats.total_new_outputs_ex_coinbase_amount = entry.total_new_outputs_ex_coinbase_amount;
334-
coins_stats.total_coinbase_amount = entry.total_coinbase_amount;
335-
coins_stats.total_unspendables_genesis_block = entry.total_unspendables_genesis_block;
336-
coins_stats.total_unspendables_bip30 = entry.total_unspendables_bip30;
337-
coins_stats.total_unspendables_scripts = entry.total_unspendables_scripts;
338-
coins_stats.total_unspendables_unclaimed_rewards = entry.total_unspendables_unclaimed_rewards;
339-
340-
return true;
330+
stats.hashSerialized = entry.muhash;
331+
stats.nTransactionOutputs = entry.transaction_output_count;
332+
stats.nBogoSize = entry.bogo_size;
333+
stats.total_amount = entry.total_amount;
334+
stats.total_subsidy = entry.total_subsidy;
335+
stats.total_unspendable_amount = entry.total_unspendable_amount;
336+
stats.total_prevout_spent_amount = entry.total_prevout_spent_amount;
337+
stats.total_new_outputs_ex_coinbase_amount = entry.total_new_outputs_ex_coinbase_amount;
338+
stats.total_coinbase_amount = entry.total_coinbase_amount;
339+
stats.total_unspendables_genesis_block = entry.total_unspendables_genesis_block;
340+
stats.total_unspendables_bip30 = entry.total_unspendables_bip30;
341+
stats.total_unspendables_scripts = entry.total_unspendables_scripts;
342+
stats.total_unspendables_unclaimed_rewards = entry.total_unspendables_unclaimed_rewards;
343+
344+
return stats;
341345
}
342346

343347
bool CoinStatsIndex::Init()

src/index/coinstatsindex.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
#include <crypto/muhash.h>
1010
#include <flatfile.h>
1111
#include <index/base.h>
12-
#include <node/coinstats.h>
12+
#include <kernel/coinstats.h>
1313

1414
/**
1515
* CoinStatsIndex maintains statistics on the UTXO set.
@@ -56,7 +56,7 @@ class CoinStatsIndex final : public BaseIndex
5656
explicit CoinStatsIndex(size_t n_cache_size, bool f_memory = false, bool f_wipe = false);
5757

5858
// Look up stats for a specific block using CBlockIndex
59-
bool LookUpStats(const CBlockIndex* block_index, node::CCoinsStats& coins_stats) const;
59+
std::optional<kernel::CCoinsStats> LookUpStats(const CBlockIndex* block_index) const;
6060
};
6161

6262
/// The global UTXO set hash object.

src/node/coinstats.cpp renamed to src/kernel/coinstats.cpp

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
1-
// Copyright (c) 2010 Satoshi Nakamoto
2-
// Copyright (c) 2009-2021 The Bitcoin Core developers
1+
// Copyright (c) 2022 The Bitcoin Core developers
32
// Distributed under the MIT software license, see the accompanying
43
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
54

6-
#include <node/coinstats.h>
5+
#include <kernel/coinstats.h>
76

87
#include <coins.h>
98
#include <crypto/muhash.h>
109
#include <hash.h>
11-
#include <index/coinstatsindex.h>
1210
#include <serialize.h>
1311
#include <uint256.h>
1412
#include <util/overflow.h>
@@ -17,7 +15,12 @@
1715

1816
#include <map>
1917

20-
namespace node {
18+
namespace kernel {
19+
20+
CCoinsStats::CCoinsStats(int block_height, const uint256& block_hash)
21+
: nHeight(block_height),
22+
hashBlock(block_hash) {}
23+
2124
// Database-independent metric indicating the UTXO set size
2225
uint64_t GetBogoSize(const CScript& script_pub_key)
2326
{
@@ -93,24 +96,11 @@ static void ApplyStats(CCoinsStats& stats, const uint256& hash, const std::map<u
9396

9497
//! Calculate statistics about the unspent transaction output set
9598
template <typename T>
96-
static bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats& stats, T hash_obj, const std::function<void()>& interruption_point, const CBlockIndex* pindex)
99+
static bool ComputeUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const std::function<void()>& interruption_point)
97100
{
98101
std::unique_ptr<CCoinsViewCursor> pcursor(view->Cursor());
99102
assert(pcursor);
100103

101-
if (!pindex) {
102-
LOCK(cs_main);
103-
pindex = blockman.LookupBlockIndex(view->GetBestBlock());
104-
}
105-
stats.nHeight = Assert(pindex)->nHeight;
106-
stats.hashBlock = pindex->GetBlockHash();
107-
108-
// Use CoinStatsIndex if it is requested and available and a hash_type of Muhash or None was requested
109-
if ((stats.m_hash_type == CoinStatsHashType::MUHASH || stats.m_hash_type == CoinStatsHashType::NONE) && g_coin_stats_index && stats.index_requested) {
110-
stats.index_used = true;
111-
return g_coin_stats_index->LookUpStats(pindex, stats);
112-
}
113-
114104
PrepareHash(hash_obj, stats);
115105

116106
uint256 prevkey;
@@ -141,25 +131,36 @@ static bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats&
141131
FinalizeHash(hash_obj, stats);
142132

143133
stats.nDiskSize = view->EstimateSize();
134+
144135
return true;
145136
}
146137

147-
bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats& stats, const std::function<void()>& interruption_point, const CBlockIndex* pindex)
138+
std::optional<CCoinsStats> ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsView* view, node::BlockManager& blockman, const std::function<void()>& interruption_point)
148139
{
149-
switch (stats.m_hash_type) {
150-
case(CoinStatsHashType::HASH_SERIALIZED): {
151-
CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION);
152-
return GetUTXOStats(view, blockman, stats, ss, interruption_point, pindex);
153-
}
154-
case(CoinStatsHashType::MUHASH): {
155-
MuHash3072 muhash;
156-
return GetUTXOStats(view, blockman, stats, muhash, interruption_point, pindex);
157-
}
158-
case(CoinStatsHashType::NONE): {
159-
return GetUTXOStats(view, blockman, stats, nullptr, interruption_point, pindex);
140+
CBlockIndex* pindex = WITH_LOCK(::cs_main, return blockman.LookupBlockIndex(view->GetBestBlock()));
141+
CCoinsStats stats{Assert(pindex)->nHeight, pindex->GetBlockHash()};
142+
143+
bool success = [&]() -> bool {
144+
switch (hash_type) {
145+
case(CoinStatsHashType::HASH_SERIALIZED): {
146+
CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION);
147+
return ComputeUTXOStats(view, stats, ss, interruption_point);
148+
}
149+
case(CoinStatsHashType::MUHASH): {
150+
MuHash3072 muhash;
151+
return ComputeUTXOStats(view, stats, muhash, interruption_point);
152+
}
153+
case(CoinStatsHashType::NONE): {
154+
return ComputeUTXOStats(view, stats, nullptr, interruption_point);
155+
}
156+
} // no default case, so the compiler can warn about missing cases
157+
assert(false);
158+
}();
159+
160+
if (!success) {
161+
return std::nullopt;
160162
}
161-
} // no default case, so the compiler can warn about missing cases
162-
assert(false);
163+
return stats;
163164
}
164165

165166
// The legacy hash serializes the hashBlock
@@ -182,4 +183,5 @@ static void FinalizeHash(MuHash3072& muhash, CCoinsStats& stats)
182183
stats.hashSerialized = out;
183184
}
184185
static void FinalizeHash(std::nullptr_t, CCoinsStats& stats) {}
185-
} // namespace node
186+
187+
} // namespace kernel

src/node/coinstats.h renamed to src/kernel/coinstats.h

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
// Copyright (c) 2010 Satoshi Nakamoto
2-
// Copyright (c) 2009-2021 The Bitcoin Core developers
1+
// Copyright (c) 2022 The Bitcoin Core developers
32
// Distributed under the MIT software license, see the accompanying
43
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
54

6-
#ifndef BITCOIN_NODE_COINSTATS_H
7-
#define BITCOIN_NODE_COINSTATS_H
5+
#ifndef BITCOIN_KERNEL_COINSTATS_H
6+
#define BITCOIN_KERNEL_COINSTATS_H
87

98
#include <chain.h>
109
#include <coins.h>
@@ -20,17 +19,14 @@ namespace node {
2019
class BlockManager;
2120
} // namespace node
2221

23-
namespace node {
22+
namespace kernel {
2423
enum class CoinStatsHashType {
2524
HASH_SERIALIZED,
2625
MUHASH,
2726
NONE,
2827
};
2928

3029
struct CCoinsStats {
31-
//! Which hash type to use
32-
const CoinStatsHashType m_hash_type;
33-
3430
int nHeight{0};
3531
uint256 hashBlock{};
3632
uint64_t nTransactions{0};
@@ -44,8 +40,6 @@ struct CCoinsStats {
4440
//! The number of coins contained.
4541
uint64_t coins_count{0};
4642

47-
//! Signals if the coinstatsindex should be used (when available).
48-
bool index_requested{true};
4943
//! Signals if the coinstatsindex was used to retrieve the statistics.
5044
bool index_used{false};
5145

@@ -70,15 +64,15 @@ struct CCoinsStats {
7064
//! Total cumulative amount of coins lost due to unclaimed miner rewards up to and including this block
7165
CAmount total_unspendables_unclaimed_rewards{0};
7266

73-
CCoinsStats(CoinStatsHashType hash_type) : m_hash_type(hash_type) {}
67+
CCoinsStats() = default;
68+
CCoinsStats(int block_height, const uint256& block_hash);
7469
};
7570

76-
//! Calculate statistics about the unspent transaction output set
77-
bool GetUTXOStats(CCoinsView* view, node::BlockManager& blockman, CCoinsStats& stats, const std::function<void()>& interruption_point = {}, const CBlockIndex* pindex = nullptr);
78-
7971
uint64_t GetBogoSize(const CScript& script_pub_key);
8072

8173
CDataStream TxOutSer(const COutPoint& outpoint, const Coin& coin);
82-
} // namespace node
8374

84-
#endif // BITCOIN_NODE_COINSTATS_H
75+
std::optional<CCoinsStats> ComputeUTXOStats(CoinStatsHashType hash_type, CCoinsView* view, node::BlockManager& blockman, const std::function<void()>& interruption_point = {});
76+
} // namespace kernel
77+
78+
#endif // BITCOIN_KERNEL_COINSTATS_H

0 commit comments

Comments
 (0)