Skip to content

Commit b86aabc

Browse files
ryanofskyryanofsky
committed
refactor, kernel: Remove gArgs accesses from dbwrapper and txdb
The libbitcoin_kernel library should should not be trying to read options from the command line or make ArgsManager calls. Instead, it should get instead get options from simple function arguments and options structs. This commit removes gArgs accesses from dbwrapper and txdb modules by defining appropriate options structs, and is a followup to PR's #25290 #25487 #25527 which remove other ArgsManager uses from kernel modules. This commit does not change behavior in any way.
1 parent a75b779 commit b86aabc

18 files changed

+198
-84
lines changed

‎src/Makefile.am‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,10 @@ BITCOIN_CORE_H = \
197197
node/caches.h \
198198
node/chainstate.h \
199199
node/coin.h \
200+
node/coins_view_args.h \
200201
node/connection_types.h \
201202
node/context.h \
203+
node/database_args.h \
202204
node/eviction.h \
203205
node/interface_ui.h \
204206
node/mempool_args.h \
@@ -381,8 +383,10 @@ libbitcoin_node_a_SOURCES = \
381383
node/caches.cpp \
382384
node/chainstate.cpp \
383385
node/coin.cpp \
386+
node/coins_view_args.cpp \
384387
node/connection_types.cpp \
385388
node/context.cpp \
389+
node/database_args.cpp \
386390
node/eviction.cpp \
387391
node/interface_ui.cpp \
388392
node/interfaces.cpp \

‎src/dbwrapper.cpp‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -127,48 +127,48 @@ static leveldb::Options GetOptions(size_t nCacheSize)
127127
return options;
128128
}
129129

130-
CDBWrapper::CDBWrapper(const fs::path& path, size_t nCacheSize, bool fMemory, bool fWipe, bool obfuscate)
131-
: m_name{fs::PathToString(path.stem())}
130+
CDBWrapper::CDBWrapper(const DBParams& params)
131+
: m_name{fs::PathToString(params.path.stem())}
132132
{
133133
penv = nullptr;
134134
readoptions.verify_checksums = true;
135135
iteroptions.verify_checksums = true;
136136
iteroptions.fill_cache = false;
137137
syncoptions.sync = true;
138-
options = GetOptions(nCacheSize);
138+
options = GetOptions(params.cache_bytes);
139139
options.create_if_missing = true;
140-
if (fMemory) {
140+
if (params.memory_only) {
141141
penv = leveldb::NewMemEnv(leveldb::Env::Default());
142142
options.env = penv;
143143
} else {
144-
if (fWipe) {
145-
LogPrintf("Wiping LevelDB in %s\n", fs::PathToString(path));
146-
leveldb::Status result = leveldb::DestroyDB(fs::PathToString(path), options);
144+
if (params.wipe_data) {
145+
LogPrintf("Wiping LevelDB in %s\n", fs::PathToString(params.path));
146+
leveldb::Status result = leveldb::DestroyDB(fs::PathToString(params.path), options);
147147
dbwrapper_private::HandleError(result);
148148
}
149-
TryCreateDirectories(path);
150-
LogPrintf("Opening LevelDB in %s\n", fs::PathToString(path));
149+
TryCreateDirectories(params.path);
150+
LogPrintf("Opening LevelDB in %s\n", fs::PathToString(params.path));
151151
}
152152
// PathToString() return value is safe to pass to leveldb open function,
153153
// because on POSIX leveldb passes the byte string directly to ::open(), and
154154
// on Windows it converts from UTF-8 to UTF-16 before calling ::CreateFileW
155155
// (see env_posix.cc and env_windows.cc).
156-
leveldb::Status status = leveldb::DB::Open(options, fs::PathToString(path), &pdb);
156+
leveldb::Status status = leveldb::DB::Open(options, fs::PathToString(params.path), &pdb);
157157
dbwrapper_private::HandleError(status);
158158
LogPrintf("Opened LevelDB successfully\n");
159159

160-
if (gArgs.GetBoolArg("-forcecompactdb", false)) {
161-
LogPrintf("Starting database compaction of %s\n", fs::PathToString(path));
160+
if (params.options.force_compact) {
161+
LogPrintf("Starting database compaction of %s\n", fs::PathToString(params.path));
162162
pdb->CompactRange(nullptr, nullptr);
163-
LogPrintf("Finished database compaction of %s\n", fs::PathToString(path));
163+
LogPrintf("Finished database compaction of %s\n", fs::PathToString(params.path));
164164
}
165165

166166
// The base-case obfuscation key, which is a noop.
167167
obfuscate_key = std::vector<unsigned char>(OBFUSCATE_KEY_NUM_BYTES, '\000');
168168

169169
bool key_exists = Read(OBFUSCATE_KEY_KEY, obfuscate_key);
170170

171-
if (!key_exists && obfuscate && IsEmpty()) {
171+
if (!key_exists && params.obfuscate && IsEmpty()) {
172172
// Initialize non-degenerate obfuscation if it won't upset
173173
// existing, non-obfuscated data.
174174
std::vector<unsigned char> new_key = CreateObfuscateKey();
@@ -177,10 +177,10 @@ CDBWrapper::CDBWrapper(const fs::path& path, size_t nCacheSize, bool fMemory, bo
177177
Write(OBFUSCATE_KEY_KEY, new_key);
178178
obfuscate_key = new_key;
179179

180-
LogPrintf("Wrote new obfuscate key for %s: %s\n", fs::PathToString(path), HexStr(obfuscate_key));
180+
LogPrintf("Wrote new obfuscate key for %s: %s\n", fs::PathToString(params.path), HexStr(obfuscate_key));
181181
}
182182

183-
LogPrintf("Using obfuscation key for %s: %s\n", fs::PathToString(path), HexStr(obfuscate_key));
183+
LogPrintf("Using obfuscation key for %s: %s\n", fs::PathToString(params.path), HexStr(obfuscate_key));
184184
}
185185

186186
CDBWrapper::~CDBWrapper()

‎src/dbwrapper.h‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,29 @@ class Env;
3131
static const size_t DBWRAPPER_PREALLOC_KEY_SIZE = 64;
3232
static const size_t DBWRAPPER_PREALLOC_VALUE_SIZE = 1024;
3333

34+
//! User-controlled performance and debug options.
35+
struct DBOptions {
36+
//! Compact database on startup.
37+
bool force_compact = false;
38+
};
39+
40+
//! Application-specific storage settings.
41+
struct DBParams {
42+
//! Location in the filesystem where leveldb data will be stored.
43+
fs::path path;
44+
//! Configures various leveldb cache settings.
45+
size_t cache_bytes;
46+
//! If true, use leveldb's memory environment.
47+
bool memory_only = false;
48+
//! If true, remove all existing data.
49+
bool wipe_data = false;
50+
//! If true, store data obfuscated via simple XOR. If false, XOR with a
51+
//! zero'd byte array.
52+
bool obfuscate = false;
53+
//! Passed-through options.
54+
DBOptions options;
55+
};
56+
3457
class dbwrapper_error : public std::runtime_error
3558
{
3659
public:
@@ -220,15 +243,7 @@ class CDBWrapper
220243
std::vector<unsigned char> CreateObfuscateKey() const;
221244

222245
public:
223-
/**
224-
* @param[in] path Location in the filesystem where leveldb data will be stored.
225-
* @param[in] nCacheSize Configures various leveldb cache settings.
226-
* @param[in] fMemory If true, use leveldb's memory environment.
227-
* @param[in] fWipe If true, remove all existing data.
228-
* @param[in] obfuscate If true, store data obfuscated via simple XOR. If false, XOR
229-
* with a zero'd byte array.
230-
*/
231-
CDBWrapper(const fs::path& path, size_t nCacheSize, bool fMemory = false, bool fWipe = false, bool obfuscate = false);
246+
CDBWrapper(const DBParams& params);
232247
~CDBWrapper();
233248

234249
CDBWrapper(const CDBWrapper&) = delete;

‎src/index/base.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <kernel/chain.h>
99
#include <node/blockstorage.h>
1010
#include <node/context.h>
11+
#include <node/database_args.h>
1112
#include <node/interface_ui.h>
1213
#include <shutdown.h>
1314
#include <tinyformat.h>
@@ -44,8 +45,13 @@ CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash)
4445
return locator;
4546
}
4647

47-
BaseIndex::DB::DB(const fs::path& path, size_t n_cache_size, bool f_memory, bool f_wipe, bool f_obfuscate) :
48-
CDBWrapper(path, n_cache_size, f_memory, f_wipe, f_obfuscate)
48+
BaseIndex::DB::DB(const fs::path& path, size_t n_cache_size, bool f_memory, bool f_wipe, bool f_obfuscate) : CDBWrapper{DBParams{
49+
.path = path,
50+
.cache_bytes = n_cache_size,
51+
.memory_only = f_memory,
52+
.wipe_data = f_wipe,
53+
.obfuscate = f_obfuscate,
54+
.options{[] { DBOptions options; node::ReadDatabaseArgs(gArgs, options); return options; }()}}}
4955
{}
5056

5157
bool BaseIndex::DB::ReadBestBlock(CBlockLocator& locator) const

‎src/init.cpp‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@
4040
#include <node/blockstorage.h>
4141
#include <node/caches.h>
4242
#include <node/chainstate.h>
43+
#include <node/coins_view_args.h>
4344
#include <node/context.h>
45+
#include <node/database_args.h>
4446
#include <node/interface_ui.h>
4547
#include <node/mempool_args.h>
4648
#include <node/mempool_persist_args.h>
@@ -1400,10 +1402,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
14001402
for (bool fLoaded = false; !fLoaded && !ShutdownRequested();) {
14011403
node.mempool = std::make_unique<CTxMemPool>(mempool_opts);
14021404

1403-
const ChainstateManager::Options chainman_opts{
1405+
ChainstateManager::Options chainman_opts{
14041406
.chainparams = chainparams,
14051407
.adjusted_time_callback = GetAdjustedTime,
1408+
.datadir = args.GetDataDirNet(),
14061409
};
1410+
node::ReadDatabaseArgs(args, chainman_opts.block_tree_db);
1411+
node::ReadDatabaseArgs(args, chainman_opts.coins_db);
1412+
node::ReadCoinsViewArgs(args, chainman_opts.coins_view);
14071413
node.chainman = std::make_unique<ChainstateManager>(chainman_opts);
14081414
ChainstateManager& chainman = *node.chainman;
14091415

‎src/kernel/chainstatemanager_opts.h‎

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

8+
#include <dbwrapper.h>
9+
#include <txdb.h>
10+
811
#include <cstdint>
912
#include <functional>
1013

@@ -20,6 +23,10 @@ namespace kernel {
2023
struct ChainstateManagerOpts {
2124
const CChainParams& chainparams;
2225
const std::function<int64_t()> adjusted_time_callback{nullptr};
26+
fs::path datadir;
27+
DBOptions block_tree_db;
28+
DBOptions coins_db;
29+
CoinsViewOptions coins_view;
2330
};
2431

2532
} // namespace kernel

‎src/node/chainstate.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,12 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
4141
// new CBlockTreeDB tries to delete the existing file, which
4242
// fails if it's still open from the previous loop. Close it first:
4343
pblocktree.reset();
44-
pblocktree.reset(new CBlockTreeDB(cache_sizes.block_tree_db, options.block_tree_db_in_memory, options.reindex));
44+
pblocktree.reset(new CBlockTreeDB{DBParams{
45+
.path = chainman.m_options.datadir / "blocks" / "index",
46+
.cache_bytes = static_cast<size_t>(cache_sizes.block_tree_db),
47+
.memory_only = options.block_tree_db_in_memory,
48+
.wipe_data = options.reindex,
49+
.options{chainman.m_options.block_tree_db}}});
4550

4651
if (options.reindex) {
4752
pblocktree->WriteReindexing(true);

‎src/node/coins_view_args.cpp‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright (c) 2022 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <node/coins_view_args.h>
6+
7+
#include <txdb.h>
8+
#include <util/system.h>
9+
10+
namespace node {
11+
void ReadCoinsViewArgs(const ArgsManager& args, CoinsViewOptions& options)
12+
{
13+
if (auto value = args.GetIntArg("-dbbatchsize")) options.batch_write_bytes = *value;
14+
if (auto value = args.GetIntArg("-dbcrashratio")) options.simulate_crash_ratio = *value;
15+
}
16+
} // namespace node

‎src/node/coins_view_args.h‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) 2022 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#ifndef BITCOIN_NODE_COINS_VIEW_ARGS_H
6+
#define BITCOIN_NODE_COINS_VIEW_ARGS_H
7+
8+
#include <fs.h>
9+
10+
class ArgsManager;
11+
struct CoinsViewOptions;
12+
13+
namespace node {
14+
void ReadCoinsViewArgs(const ArgsManager& args, CoinsViewOptions& options);
15+
} // namespace node
16+
17+
#endif // BITCOIN_NODE_COINS_VIEW_ARGS_H

‎src/node/database_args.cpp‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright (c) 2022 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <node/database_args.h>
6+
7+
#include <dbwrapper.h>
8+
#include <util/system.h>
9+
10+
namespace node {
11+
void ReadDatabaseArgs(const ArgsManager& args, DBOptions& options)
12+
{
13+
// Settings here apply to all databases (chainstate, blocks, and index
14+
// databases), but it'd be easy to parse database-specific options by adding
15+
// a database_type string or enum parameter to this function.
16+
if (auto value = args.GetBoolArg("-forcecompactdb")) options.force_compact = *value;
17+
}
18+
} // namespace node

0 commit comments

Comments
 (0)