Skip to content

Commit c2f2abd

Browse files
committed
Merge bitcoin/bitcoin#27125: refactor, kernel: Decouple ArgsManager from blockstorage
5ff63a0 refactor, blockstorage: Replace stopafterblockimport arg (TheCharlatan) 18e5ba7 refactor, blockstorage: Replace blocksdir arg (TheCharlatan) 02a0899 refactor, BlockManager: Replace fastprune from arg with options (TheCharlatan) a498d69 refactor/iwyu: Complete includes for blockmanager_args (TheCharlatan) f0bb102 refactor: Move functions to BlockManager methods (TheCharlatan) cfbb212 zmq: Pass lambda to zmq's ZMQPublishRawBlockNotifier (TheCharlatan) 8ed4ff8 refactor: Declare g_zmq_notification_interface as unique_ptr (TheCharlatan) Pull request description: The libbitcoin_kernel library should not rely on the `ArgsManager`, but rather use option structs that can be passed to the various classes it uses. This PR removes reliance on the `ArgsManager` from the `blockstorage.*` files. Like similar prior work, it uses the options struct in the `BlockManager` that can be populated with `ArgsManager` values. Some related prior work: bitcoin/bitcoin#26889 bitcoin/bitcoin#25862 bitcoin/bitcoin#25527 bitcoin/bitcoin#25487 Related PR removing blockstorage globals: bitcoin/bitcoin#25781 ACKs for top commit: ryanofsky: Code review ACK 5ff63a0. Since last ACK just added std::move and fixed commit title. Sorry for the noise! mzumsande: Code Review ACK 5ff63a0 Tree-SHA512: 4bde8fd140a40b97eca923e9016d85dcea6fad6fd199731f158376294af59c3e8b163a0725aa47b4be3519b61828044e0a042deea005e0c28de21d8b6c3e1ea7
2 parents 3ff67f7 + 5ff63a0 commit c2f2abd

33 files changed

+189
-169
lines changed

ci/test/06_script_b.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ if [ "${RUN_TIDY}" = "true" ]; then
5252
src/dbwrapper.cpp \
5353
src/init \
5454
src/kernel \
55+
src/node/blockmanager_args.cpp \
5556
src/node/chainstate.cpp \
5657
src/node/chainstatemanager_args.cpp \
5758
src/node/mempool_args.cpp \

src/bitcoin-chainstate.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ int main(int argc, char* argv[])
8989
};
9090
const node::BlockManager::Options blockman_opts{
9191
.chainparams = chainman_opts.chainparams,
92+
.blocks_dir = gArgs.GetBlocksDirPath(),
9293
};
9394
ChainstateManager chainman{chainman_opts, blockman_opts};
9495

src/index/base.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
#include <string>
2424
#include <utility>
2525

26-
using node::ReadBlockFromDisk;
27-
2826
constexpr uint8_t DB_BEST_BLOCK{'B'};
2927

3028
constexpr auto SYNC_LOG_INTERVAL{30s};
@@ -159,8 +157,6 @@ void BaseIndex::ThreadSync()
159157
SetSyscallSandboxPolicy(SyscallSandboxPolicy::TX_INDEX);
160158
const CBlockIndex* pindex = m_best_block_index.load();
161159
if (!m_synced) {
162-
auto& consensus_params = Params().GetConsensus();
163-
164160
std::chrono::steady_clock::time_point last_log_time{0s};
165161
std::chrono::steady_clock::time_point last_locator_write_time{0s};
166162
while (true) {
@@ -207,7 +203,7 @@ void BaseIndex::ThreadSync()
207203

208204
CBlock block;
209205
interfaces::BlockInfo block_info = kernel::MakeBlockInfo(pindex);
210-
if (!ReadBlockFromDisk(block, pindex, consensus_params)) {
206+
if (!m_chainstate->m_blockman.ReadBlockFromDisk(block, *pindex)) {
211207
FatalError("%s: Failed to read block %s from disk",
212208
__func__, pindex->GetBlockHash().ToString());
213209
return;

src/index/blockfilterindex.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@
1212
#include <util/fs_helpers.h>
1313
#include <validation.h>
1414

15-
using node::UndoReadFromDisk;
16-
1715
/* The index database stores three items for each block: the disk location of the encoded filter,
1816
* its dSHA256 hash, and the header. Those belonging to blocks on the active chain are indexed by
1917
* height, and those belonging to blocks that have been reorganized out of the active chain are
@@ -223,7 +221,7 @@ bool BlockFilterIndex::CustomAppend(const interfaces::BlockInfo& block)
223221
// pindex variable gives indexing code access to node internals. It
224222
// will be removed in upcoming commit
225223
const CBlockIndex* pindex = WITH_LOCK(cs_main, return m_chainstate->m_blockman.LookupBlockIndex(block.hash));
226-
if (!UndoReadFromDisk(block_undo, pindex)) {
224+
if (!m_chainstate->m_blockman.UndoReadFromDisk(block_undo, *pindex)) {
227225
return false;
228226
}
229227

src/index/coinstatsindex.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,6 @@ using kernel::CCoinsStats;
1919
using kernel::GetBogoSize;
2020
using kernel::TxOutSer;
2121

22-
using node::ReadBlockFromDisk;
23-
using node::UndoReadFromDisk;
24-
2522
static constexpr uint8_t DB_BLOCK_HASH{'s'};
2623
static constexpr uint8_t DB_BLOCK_HEIGHT{'t'};
2724
static constexpr uint8_t DB_MUHASH{'M'};
@@ -125,7 +122,7 @@ bool CoinStatsIndex::CustomAppend(const interfaces::BlockInfo& block)
125122
// pindex variable gives indexing code access to node internals. It
126123
// will be removed in upcoming commit
127124
const CBlockIndex* pindex = WITH_LOCK(cs_main, return m_chainstate->m_blockman.LookupBlockIndex(block.hash));
128-
if (!UndoReadFromDisk(block_undo, pindex)) {
125+
if (!m_chainstate->m_blockman.UndoReadFromDisk(block_undo, *pindex)) {
129126
return false;
130127
}
131128

@@ -282,12 +279,11 @@ bool CoinStatsIndex::CustomRewind(const interfaces::BlockKey& current_tip, const
282279
LOCK(cs_main);
283280
const CBlockIndex* iter_tip{m_chainstate->m_blockman.LookupBlockIndex(current_tip.hash)};
284281
const CBlockIndex* new_tip_index{m_chainstate->m_blockman.LookupBlockIndex(new_tip.hash)};
285-
const auto& consensus_params{Params().GetConsensus()};
286282

287283
do {
288284
CBlock block;
289285

290-
if (!ReadBlockFromDisk(block, iter_tip, consensus_params)) {
286+
if (!m_chainstate->m_blockman.ReadBlockFromDisk(block, *iter_tip)) {
291287
return error("%s: Failed to read block %s from disk",
292288
__func__, iter_tip->GetBlockHash().ToString());
293289
}
@@ -409,7 +405,7 @@ bool CoinStatsIndex::ReverseBlock(const CBlock& block, const CBlockIndex* pindex
409405

410406
// Ignore genesis block
411407
if (pindex->nHeight > 0) {
412-
if (!UndoReadFromDisk(block_undo, pindex)) {
408+
if (!m_chainstate->m_blockman.UndoReadFromDisk(block_undo, *pindex)) {
413409
return false;
414410
}
415411

src/index/txindex.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
#include <node/blockstorage.h>
1111
#include <validation.h>
1212

13-
using node::OpenBlockFile;
14-
1513
constexpr uint8_t DB_TXINDEX{'t'};
1614

1715
std::unique_ptr<TxIndex> g_txindex;
@@ -80,7 +78,7 @@ bool TxIndex::FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRe
8078
return false;
8179
}
8280

83-
CAutoFile file(OpenBlockFile(postx, true), SER_DISK, CLIENT_VERSION);
81+
CAutoFile file(m_chainstate->m_blockman.OpenBlockFile(postx, true), SER_DISK, CLIENT_VERSION);
8482
if (file.IsNull()) {
8583
return error("%s: OpenBlockFile failed", __func__);
8684
}

src/init.cpp

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -112,22 +112,23 @@
112112
#include <zmq/zmqrpc.h>
113113
#endif
114114

115+
using kernel::DEFAULT_STOPAFTERBLOCKIMPORT;
115116
using kernel::DumpMempool;
116117
using kernel::ValidationCacheSizes;
117118

118119
using node::ApplyArgsManOptions;
120+
using node::BlockManager;
119121
using node::CacheSizes;
120122
using node::CalculateCacheSizes;
121123
using node::DEFAULT_PERSIST_MEMPOOL;
122124
using node::DEFAULT_PRINTPRIORITY;
123-
using node::DEFAULT_STOPAFTERBLOCKIMPORT;
125+
using node::fReindex;
124126
using node::LoadChainstate;
125127
using node::MempoolPath;
126-
using node::ShouldPersistMempool;
127128
using node::NodeContext;
129+
using node::ShouldPersistMempool;
128130
using node::ThreadImport;
129131
using node::VerifyLoadedChainstate;
130-
using node::fReindex;
131132

132133
static constexpr bool DEFAULT_PROXYRANDOMIZE{true};
133134
static constexpr bool DEFAULT_REST_ENABLE{false};
@@ -328,9 +329,8 @@ void Shutdown(NodeContext& node)
328329

329330
#if ENABLE_ZMQ
330331
if (g_zmq_notification_interface) {
331-
UnregisterValidationInterface(g_zmq_notification_interface);
332-
delete g_zmq_notification_interface;
333-
g_zmq_notification_interface = nullptr;
332+
UnregisterValidationInterface(g_zmq_notification_interface.get());
333+
g_zmq_notification_interface.reset();
334334
}
335335
#endif
336336

@@ -1038,8 +1038,9 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
10381038
if (const auto error{ApplyArgsManOptions(args, chainman_opts_dummy)}) {
10391039
return InitError(*error);
10401040
}
1041-
node::BlockManager::Options blockman_opts_dummy{
1041+
BlockManager::Options blockman_opts_dummy{
10421042
.chainparams = chainman_opts_dummy.chainparams,
1043+
.blocks_dir = args.GetBlocksDirPath(),
10431044
};
10441045
if (const auto error{ApplyArgsManOptions(args, blockman_opts_dummy)}) {
10451046
return InitError(*error);
@@ -1425,10 +1426,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
14251426
}
14261427

14271428
#if ENABLE_ZMQ
1428-
g_zmq_notification_interface = CZMQNotificationInterface::Create();
1429+
g_zmq_notification_interface = CZMQNotificationInterface::Create(
1430+
[&chainman = node.chainman](CBlock& block, const CBlockIndex& index) {
1431+
assert(chainman);
1432+
return chainman->m_blockman.ReadBlockFromDisk(block, index);
1433+
});
14291434

14301435
if (g_zmq_notification_interface) {
1431-
RegisterValidationInterface(g_zmq_notification_interface);
1436+
RegisterValidationInterface(g_zmq_notification_interface.get());
14321437
}
14331438
#endif
14341439

@@ -1443,8 +1448,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
14431448
};
14441449
Assert(!ApplyArgsManOptions(args, chainman_opts)); // no error can happen, already checked in AppInitParameterInteraction
14451450

1446-
node::BlockManager::Options blockman_opts{
1451+
BlockManager::Options blockman_opts{
14471452
.chainparams = chainman_opts.chainparams,
1453+
.blocks_dir = args.GetBlocksDirPath(),
14481454
};
14491455
Assert(!ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction
14501456

@@ -1674,7 +1680,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
16741680
}
16751681

16761682
chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &chainman, &args] {
1677-
ThreadImport(chainman, vImportFiles, args, ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{});
1683+
ThreadImport(chainman, vImportFiles, ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{});
16781684
});
16791685

16801686
// Wait for genesis block to be processed

src/kernel/blockmanager_opts.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,26 @@
55
#ifndef BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
66
#define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H
77

8+
#include <util/fs.h>
9+
10+
#include <cstdint>
11+
812
class CChainParams;
913

1014
namespace kernel {
1115

16+
static constexpr bool DEFAULT_STOPAFTERBLOCKIMPORT{false};
17+
1218
/**
1319
* An options struct for `BlockManager`, more ergonomically referred to as
1420
* `BlockManager::Options` due to the using-declaration in `BlockManager`.
1521
*/
1622
struct BlockManagerOpts {
1723
const CChainParams& chainparams;
1824
uint64_t prune_target{0};
25+
bool fast_prune{false};
26+
bool stop_after_block_import{DEFAULT_STOPAFTERBLOCKIMPORT};
27+
const fs::path blocks_dir;
1928
};
2029

2130
} // namespace kernel

src/net_processing.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@
5151
#include <optional>
5252
#include <typeinfo>
5353

54-
using node::ReadBlockFromDisk;
55-
using node::ReadRawBlockFromDisk;
56-
5754
/** How long to cache transactions in mapRelay for normal relay */
5855
static constexpr auto RELAY_TX_CACHE_TIME = 15min;
5956
/** How long a transaction has to be in the mempool before it can unconditionally be relayed (even when not in mapRelay). */
@@ -2189,15 +2186,15 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv&
21892186
// Fast-path: in this case it is possible to serve the block directly from disk,
21902187
// as the network format matches the format on disk
21912188
std::vector<uint8_t> block_data;
2192-
if (!ReadRawBlockFromDisk(block_data, pindex->GetBlockPos(), m_chainparams.MessageStart())) {
2189+
if (!m_chainman.m_blockman.ReadRawBlockFromDisk(block_data, pindex->GetBlockPos(), m_chainparams.MessageStart())) {
21932190
assert(!"cannot load block from disk");
21942191
}
21952192
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::BLOCK, Span{block_data}));
21962193
// Don't set pblock as we've sent the block
21972194
} else {
21982195
// Send block from disk
21992196
std::shared_ptr<CBlock> pblockRead = std::make_shared<CBlock>();
2200-
if (!ReadBlockFromDisk(*pblockRead, pindex, m_chainparams.GetConsensus())) {
2197+
if (!m_chainman.m_blockman.ReadBlockFromDisk(*pblockRead, *pindex)) {
22012198
assert(!"cannot load block from disk");
22022199
}
22032200
pblock = pblockRead;
@@ -3889,7 +3886,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
38893886

38903887
if (pindex->nHeight >= m_chainman.ActiveChain().Height() - MAX_BLOCKTXN_DEPTH) {
38913888
CBlock block;
3892-
bool ret = ReadBlockFromDisk(block, pindex, m_chainparams.GetConsensus());
3889+
const bool ret{m_chainman.m_blockman.ReadBlockFromDisk(block, *pindex)};
38933890
assert(ret);
38943891

38953892
SendBlockTransactions(pfrom, *peer, block, req);
@@ -5546,7 +5543,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
55465543
m_connman.PushMessage(pto, std::move(cached_cmpctblock_msg.value()));
55475544
} else {
55485545
CBlock block;
5549-
bool ret = ReadBlockFromDisk(block, pBestIndex, consensusParams);
5546+
const bool ret{m_chainman.m_blockman.ReadBlockFromDisk(block, *pBestIndex)};
55505547
assert(ret);
55515548
CBlockHeaderAndShortTxIDs cmpctblock{block};
55525549
m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock));

src/node/blockmanager_args.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,14 @@
55
#include <node/blockmanager_args.h>
66

77
#include <common/args.h>
8+
#include <node/blockstorage.h>
9+
#include <tinyformat.h>
10+
#include <util/translation.h>
811
#include <validation.h>
912

13+
#include <cstdint>
14+
#include <optional>
15+
1016
namespace node {
1117
std::optional<bilingual_str> ApplyArgsManOptions(const ArgsManager& args, BlockManager::Options& opts)
1218
{
@@ -25,6 +31,9 @@ std::optional<bilingual_str> ApplyArgsManOptions(const ArgsManager& args, BlockM
2531
}
2632
opts.prune_target = nPruneTarget;
2733

34+
if (auto value{args.GetBoolArg("-fastprune")}) opts.fast_prune = *value;
35+
if (auto value{args.GetBoolArg("-stopafterblockimport")}) opts.stop_after_block_import = *value;
36+
2837
return std::nullopt;
2938
}
3039
} // namespace node

0 commit comments

Comments
 (0)