[kernel 3e/n] Decouple CDBWrapper and its kernel users from ArgsManager

src/Makefile.am

@@ -174,9 +174,11 @@ BITCOIN_CORE_H = \
   kernel/checks.h \
   kernel/coinstats.h \
   kernel/context.h \
+  kernel/dbwrapper_options.h \
   kernel/mempool_limits.h \
   kernel/mempool_options.h \
   kernel/mempool_persist.h \
+  kernel/txdb_options.h \
   kernel/validation_cache_sizes.h \
   key.h \
   key_io.h \
@@ -196,6 +198,7 @@ BITCOIN_CORE_H = \
   node/blockstorage.h \
   node/caches.h \
   node/chainstate.h \
+  node/chainstatemanager_args.h \
   node/coin.h \
   node/connection_types.h \
   node/context.h \
@@ -207,6 +210,7 @@ BITCOIN_CORE_H = \
   node/minisketchwrapper.h \
   node/psbt.h \
   node/transaction.h \
+  node/txdb_args.h \
   node/utxo_snapshot.h \
   node/validation_cache_args.h \
   noui.h \
@@ -380,6 +384,7 @@ libbitcoin_node_a_SOURCES = \
   node/blockstorage.cpp \
   node/caches.cpp \
   node/chainstate.cpp \
+  node/chainstatemanager_args.cpp \
   node/coin.cpp \
   node/connection_types.cpp \
   node/context.cpp \
@@ -392,6 +397,7 @@ libbitcoin_node_a_SOURCES = \
   node/minisketchwrapper.cpp \
   node/psbt.cpp \
   node/transaction.cpp \
+  node/txdb_args.cpp \
   node/validation_cache_args.cpp \
   noui.cpp \
   policy/fees.cpp \

src/bitcoin-chainstate.cpp

@@ -80,19 +80,30 @@ int main(int argc, char* argv[])
 
 
     // SETUP: Chainstate
+    node::CacheSizes cache_sizes;
+    cache_sizes.block_tree_db = 2 << 20;
+    cache_sizes.coins_db = 2 << 22;
+    cache_sizes.coins = (450 << 20) - (2 << 20) - (2 << 22);
+
     const ChainstateManager::Options chainman_opts{
         .chainparams = chainparams,
         .adjusted_time_callback = static_cast<int64_t (*)()>(GetTime),
+        .block_tree_db_opts = {
+            .cache_size = static_cast<size_t>(cache_sizes.block_tree_db),
+        },
+        .data_dir = abs_datadir,
+        .coins_view_db_opts = {
+            .cache_size = static_cast<size_t>(cache_sizes.coins_db),
+            .in_memory = false,
+            .wipe_existing = false,
+        },
+        .total_coinstip_cache_bytes = static_cast<size_t>(cache_sizes.coins),
     };
     ChainstateManager chainman{chainman_opts};
 
-    node::CacheSizes cache_sizes;
-    cache_sizes.block_tree_db = 2 << 20;
-    cache_sizes.coins_db = 2 << 22;
-    cache_sizes.coins = (450 << 20) - (2 << 20) - (2 << 22);
     node::ChainstateLoadOptions options;
     options.check_interrupt = [] { return false; };
-    auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options);
+    auto [status, error] = node::LoadChainstate(chainman, options);
     if (status != node::ChainstateLoadStatus::SUCCESS) {
         std::cerr << "Failed to load Chain state from your datadir." << std::endl;
         goto epilogue;
@@ -251,7 +262,6 @@ int main(int argc, char* argv[])
         for (CChainState* chainstate : chainman.GetAll()) {
             if (chainstate->CanFlushToDisk()) {
                 chainstate->ForceFlushStateToDisk();
-                chainstate->ResetCoinsViews();
             }
         }
     }

src/dbwrapper.cpp

@@ -8,6 +8,7 @@
 #include <logging.h>
 #include <random.h>
 #include <tinyformat.h>
+#include <util/check.h>
 #include <util/strencodings.h>
 #include <util/system.h>
 
@@ -127,48 +128,51 @@ static leveldb::Options GetOptions(size_t nCacheSize)
     return options;
 }
 
-CDBWrapper::CDBWrapper(const fs::path& path, size_t nCacheSize, bool fMemory, bool fWipe, bool obfuscate)
-    : m_name{fs::PathToString(path.stem())}
+CDBWrapper::CDBWrapper(const Options& opts)
+    : m_name{[&]() {
+        Assert(!opts.db_path.empty());
+        return fs::PathToString(opts.db_path.stem());
+    }()}
 {
     penv = nullptr;
     readoptions.verify_checksums = true;
     iteroptions.verify_checksums = true;
     iteroptions.fill_cache = false;
     syncoptions.sync = true;
-    options = GetOptions(nCacheSize);
+    options = GetOptions(opts.cache_size);
     options.create_if_missing = true;
-    if (fMemory) {
+    if (opts.in_memory) {
         penv = leveldb::NewMemEnv(leveldb::Env::Default());
         options.env = penv;
     } else {
-        if (fWipe) {
-            LogPrintf("Wiping LevelDB in %s\n", fs::PathToString(path));
-            leveldb::Status result = leveldb::DestroyDB(fs::PathToString(path), options);
+        if (opts.wipe_existing) {
+            LogPrintf("Wiping LevelDB in %s\n", fs::PathToString(opts.db_path));
+            leveldb::Status result = leveldb::DestroyDB(fs::PathToString(opts.db_path), options);
             dbwrapper_private::HandleError(result);
         }
-        TryCreateDirectories(path);
-        LogPrintf("Opening LevelDB in %s\n", fs::PathToString(path));
+        TryCreateDirectories(opts.db_path);
+        LogPrintf("Opening LevelDB in %s\n", fs::PathToString(opts.db_path));
     }
     // PathToString() return value is safe to pass to leveldb open function,
     // because on POSIX leveldb passes the byte string directly to ::open(), and
     // on Windows it converts from UTF-8 to UTF-16 before calling ::CreateFileW
     // (see env_posix.cc and env_windows.cc).
-    leveldb::Status status = leveldb::DB::Open(options, fs::PathToString(path), &pdb);
+    leveldb::Status status = leveldb::DB::Open(options, fs::PathToString(opts.db_path), &pdb);
     dbwrapper_private::HandleError(status);
     LogPrintf("Opened LevelDB successfully\n");
 
-    if (gArgs.GetBoolArg("-forcecompactdb", false)) {
-        LogPrintf("Starting database compaction of %s\n", fs::PathToString(path));
+    if (opts.do_compact) {
+        LogPrintf("Starting database compaction of %s\n", fs::PathToString(opts.db_path));
         pdb->CompactRange(nullptr, nullptr);
-        LogPrintf("Finished database compaction of %s\n", fs::PathToString(path));
+        LogPrintf("Finished database compaction of %s\n", fs::PathToString(opts.db_path));
     }
 
     // The base-case obfuscation key, which is a noop.
     obfuscate_key = std::vector<unsigned char>(OBFUSCATE_KEY_NUM_BYTES, '\000');
 
     bool key_exists = Read(OBFUSCATE_KEY_KEY, obfuscate_key);
 
-    if (!key_exists && obfuscate && IsEmpty()) {
+    if (!key_exists && opts.obfuscate_data && IsEmpty()) {
         // Initialize non-degenerate obfuscation if it won't upset
         // existing, non-obfuscated data.
         std::vector<unsigned char> new_key = CreateObfuscateKey();
@@ -177,10 +181,10 @@ CDBWrapper::CDBWrapper(const fs::path& path, size_t nCacheSize, bool fMemory, bo
         Write(OBFUSCATE_KEY_KEY, new_key);
         obfuscate_key = new_key;
 
-        LogPrintf("Wrote new obfuscate key for %s: %s\n", fs::PathToString(path), HexStr(obfuscate_key));
+        LogPrintf("Wrote new obfuscate key for %s: %s\n", fs::PathToString(opts.db_path), HexStr(obfuscate_key));
     }
 
-    LogPrintf("Using obfuscation key for %s: %s\n", fs::PathToString(path), HexStr(obfuscate_key));
+    LogPrintf("Using obfuscation key for %s: %s\n", fs::PathToString(opts.db_path), HexStr(obfuscate_key));
 }
 
 CDBWrapper::~CDBWrapper()

src/dbwrapper.h

@@ -5,6 +5,8 @@
 #ifndef BITCOIN_DBWRAPPER_H
 #define BITCOIN_DBWRAPPER_H
 
+#include <kernel/dbwrapper_options.h>
+
 #include <clientversion.h>
 #include <fs.h>
 #include <logging.h>
@@ -220,15 +222,9 @@ class CDBWrapper
     std::vector<unsigned char> CreateObfuscateKey() const;
 
 public:
-    /**
-     * @param[in] path        Location in the filesystem where leveldb data will be stored.
-     * @param[in] nCacheSize  Configures various leveldb cache settings.
-     * @param[in] fMemory     If true, use leveldb's memory environment.
-     * @param[in] fWipe       If true, remove all existing data.
-     * @param[in] obfuscate   If true, store data obfuscated via simple XOR. If false, XOR
-     *                        with a zero'd byte array.
-     */
-    CDBWrapper(const fs::path& path, size_t nCacheSize, bool fMemory = false, bool fWipe = false, bool obfuscate = false);
+    using Options = kernel::DBWrapperOpts;
+
+    CDBWrapper(const Options& opts);
     ~CDBWrapper();
 
     CDBWrapper(const CDBWrapper&) = delete;

src/index/base.cpp

@@ -44,9 +44,16 @@ CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash)
     return locator;
 }
 
-BaseIndex::DB::DB(const fs::path& path, size_t n_cache_size, bool f_memory, bool f_wipe, bool f_obfuscate) :
-    CDBWrapper(path, n_cache_size, f_memory, f_wipe, f_obfuscate)
-{}
+BaseIndex::DB::DB(const fs::path& path, size_t n_cache_size, bool f_memory, bool f_wipe, bool f_obfuscate)
+    : CDBWrapper{{
+          // TODO: Change this constructor to take CDBWrapper::Options
+          .db_path = path,
+          .cache_size = n_cache_size,
+          .in_memory = f_memory,
+          .wipe_existing = f_wipe,
+          .obfuscate_data = f_obfuscate,
+          .do_compact = gArgs.GetBoolArg("-forcecompactdb", false),
+      }} {}
 
 bool BaseIndex::DB::ReadBestBlock(CBlockLocator& locator) const
 {

src/init.cpp

@@ -40,11 +40,13 @@
 #include <node/blockstorage.h>
 #include <node/caches.h>
 #include <node/chainstate.h>
+#include <node/chainstatemanager_args.h>
 #include <node/context.h>
 #include <node/interface_ui.h>
 #include <node/mempool_args.h>
 #include <node/mempool_persist_args.h>
 #include <node/miner.h>
+#include <node/txdb_args.h>
 #include <node/validation_cache_args.h>
 #include <policy/feerate.h>
 #include <policy/fees.h>
@@ -108,6 +110,7 @@
 
 using kernel::DumpMempool;
 using kernel::ValidationCacheSizes;
+using kernel::nDefaultDbBatchSize;
 
 using node::ApplyArgsManOptions;
 using node::CacheSizes;
@@ -117,8 +120,8 @@ using node::DEFAULT_PRINTPRIORITY;
 using node::DEFAULT_STOPAFTERBLOCKIMPORT;
 using node::LoadChainstate;
 using node::MempoolPath;
-using node::ShouldPersistMempool;
 using node::NodeContext;
+using node::ShouldPersistMempool;
 using node::ThreadImport;
 using node::VerifyLoadedChainstate;
 using node::fPruneMode;
@@ -297,7 +300,6 @@ void Shutdown(NodeContext& node)
         for (CChainState* chainstate : node.chainman->GetAll()) {
             if (chainstate->CanFlushToDisk()) {
                 chainstate->ForceFlushStateToDisk();
-                chainstate->ResetCoinsViews();
             }
         }
     }
@@ -1400,16 +1402,28 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     for (bool fLoaded = false; !fLoaded && !ShutdownRequested();) {
         node.mempool = std::make_unique<CTxMemPool>(mempool_opts);
 
-        const ChainstateManager::Options chainman_opts{
+        const bool do_reindex = fReindex;
+
+        ChainstateManager::Options chainman_opts{
             .chainparams = chainparams,
             .adjusted_time_callback = GetAdjustedTime,
+            .block_tree_db_opts = {
+                .cache_size = static_cast<size_t>(cache_sizes.block_tree_db),
+                .wipe_existing = do_reindex,
+            },
+            .data_dir = gArgs.GetDataDirNet(),
+            .coins_view_db_opts = {
+                .cache_size = static_cast<size_t>(cache_sizes.coins_db), // FIXME
+                .in_memory = false,
+                .wipe_existing = do_reindex || fReindexChainState,
+            },
+            .total_coinstip_cache_bytes = static_cast<size_t>(cache_sizes.coins),
         };
-        node.chainman = std::make_unique<ChainstateManager>(chainman_opts);
-        ChainstateManager& chainman = *node.chainman;
+        ApplyArgsManOptions(args, chainman_opts);
 
         node::ChainstateLoadOptions options;
         options.mempool = Assert(node.mempool.get());
-        options.reindex = node::fReindex;
+        options.reindex = do_reindex;
         options.reindex_chainstate = fReindexChainState;
         options.prune = node::fPruneMode;
         options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
@@ -1431,8 +1445,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                 return std::make_tuple(node::ChainstateLoadStatus::FAILURE, _("Error opening block database"));
             }
         };
-        auto [status, error] = catch_exceptions([&]{ return LoadChainstate(chainman, cache_sizes, options); });
+        auto [status, error] = catch_exceptions([&] {
+            node.chainman = std::make_unique<ChainstateManager>(chainman_opts);
+            return LoadChainstate(*Assert(node.chainman), options);
+        });
         if (status == node::ChainstateLoadStatus::SUCCESS) {
+            ChainstateManager& chainman = *Assert(node.chainman);
             uiInterface.InitMessage(_("Verifying blocks…").translated);
             if (chainman.m_blockman.m_have_pruned && options.check_blocks > MIN_BLOCKS_TO_KEEP) {
                 LogPrintfCategory(BCLog::PRUNE, "pruned datadir may not have more than %d blocks; only checking available blocks\n",
@@ -1486,7 +1504,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     // ********************************************************* Step 8: start indexers
     if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
-        if (const auto error{WITH_LOCK(cs_main, return CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db)))}) {
+        if (const auto error{WITH_LOCK(cs_main, return CheckLegacyTxindex(chainman.m_blockman.m_block_tree_db))}) {
             return InitError(*error);
         }
 

src/kernel/chainstatemanager_opts.h

@@ -5,6 +5,8 @@
 #ifndef BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
 #define BITCOIN_KERNEL_CHAINSTATEMANAGER_OPTS_H
 
+#include <kernel/txdb_options.h>
+
 #include <cstdint>
 #include <functional>
 
@@ -20,6 +22,10 @@ namespace kernel {
 struct ChainstateManagerOpts {
     const CChainParams& chainparams;
     const std::function<int64_t()> adjusted_time_callback{nullptr};
+    BlockTreeDBOpts block_tree_db_opts;
+    const fs::path& data_dir;
+    CoinsViewDBOpts coins_view_db_opts;
+    size_t total_coinstip_cache_bytes;
 };
 
 } // namespace kernel

src/kernel/dbwrapper_options.h

@@ -0,0 +1,30 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_KERNEL_DBWRAPPER_OPTIONS_H
+#define BITCOIN_KERNEL_DBWRAPPER_OPTIONS_H
+
+#include <fs.h>
+
+#include <cstddef>
+
+namespace kernel {
+
+struct DBWrapperOpts {
+    //! Location in the filesystem where leveldb data will be stored.
+    fs::path db_path;
+    //! Configures various leveldb cache settings.
+    size_t cache_size;
+    //! If true, use leveldb's memory environment.
+    bool in_memory = false;
+    //! If true, remove all existing data.
+    bool wipe_existing = false;
+    //! If true, store data obfuscated via simple XOR. If false, XOR with a zero'd byte array.
+    bool obfuscate_data = false;
+    bool do_compact = false;
+};
+
+} // namespace kernel
+
+#endif // BITCOIN_KERNEL_DBWRAPPER_OPTIONS_H