Skip to content

Commit 1c77e2e

Browse files
ryanofskyryanofsky
committed
refactor, kernel: Remove gArgs accesses from dbwrapper and txdb
Code in the libbitcoin_kernel library should not be calling ArgsManager methods or trying to read options from the command line. Instead it should just get options values from simple structs and function arguments that are passed in externally. 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 calls from kernel modules. This PR does not change behavior in any way.
1 parent 6db2c00 commit 1c77e2e

19 files changed

+190
-74
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/bitcoin-chainstate.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ int main(int argc, char* argv[])
8383
const ChainstateManager::Options chainman_opts{
8484
.chainparams = chainparams,
8585
.adjusted_time_callback = NodeClock::now,
86+
.datadir = abs_datadir,
8687
};
8788
ChainstateManager chainman{chainman_opts};
8889

‎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: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#ifndef BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
66
#define BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
77

8+
#include <dbwrapper.h>
9+
#include <txdb.h>
810
#include <util/time.h>
911

1012
#include <cstdint>
@@ -22,6 +24,10 @@ namespace kernel {
2224
struct ChainstateManagerOpts {
2325
const CChainParams& chainparams;
2426
const std::function<NodeClock::time_point()> adjusted_time_callback{nullptr};
27+
fs::path datadir;
28+
DBOptions block_tree_db;
29+
DBOptions coins_db;
30+
CoinsViewOptions coins_view;
2531
};
2632

2733
} // 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

0 commit comments

Comments
 (0)