kernel: make BlockManagerOptions use kernel_LockedDirectory

Note: to ensure that blocks_dir is kept alive for the duration of
ChainstateManager, it is added as a parameter to
kernel_chainstate_manager_destroy. This introduces some asymmetry
where blocks_dir is a parameter to destroy, but not create,
ChainstateManager. This could be addressed by refactoring blocks_dir
out of BlockManagerOptions and adding it as a ctor parameter
to ChainstateManager, but is not done here to keep the changes
minimal.

src/bitcoin-chainstate.cpp

@@ -169,11 +169,14 @@ int main(int argc, char* argv[])
     ChainstateManagerOptions chainman_opts{context, data_dir};
     assert(chainman_opts);
     chainman_opts.SetWorkerThreads(4);
-    BlockManagerOptions blockman_opts{context, abs_datadir.string(), (abs_datadir / "blocks").string()};
+
+    LockedDirectory blocks_dir{abs_datadir / "blocks"};
+    assert(blocks_dir);
+    BlockManagerOptions blockman_opts{context, data_dir, blocks_dir};
     assert(blockman_opts);
     ChainstateLoadOptions chainstate_load_opts{};
 
-    auto chainman{std::make_unique<ChainMan>(context, chainman_opts, blockman_opts, chainstate_load_opts, data_dir)};
+    auto chainman{std::make_unique<ChainMan>(context, chainman_opts, blockman_opts, chainstate_load_opts, data_dir, blocks_dir)};
     if (!*chainman) {
         return 1;
     }

src/kernel/bitcoinkernel.cpp

@@ -787,24 +787,22 @@ void kernel_chainstate_manager_options_destroy(kernel_ChainstateManagerOptions*
     }
 }
 
-kernel_BlockManagerOptions* kernel_block_manager_options_create(const kernel_Context* context_, const char* data_dir, size_t data_dir_len, const char* blocks_dir, size_t blocks_dir_len)
+kernel_BlockManagerOptions* kernel_block_manager_options_create(const kernel_Context* context_, const kernel_LockedDirectory* data_dir_, const kernel_LockedDirectory* blocks_dir_)
 {
     try {
-        fs::path abs_blocks_dir{fs::absolute(fs::PathFromString({blocks_dir, blocks_dir_len}))};
-        fs::create_directories(abs_blocks_dir);
-        fs::path abs_data_dir{fs::absolute(fs::PathFromString({data_dir, data_dir_len}))};
-        fs::create_directories(abs_data_dir);
+        const auto data_dir{cast_const_locked_directory(data_dir_)};
+        const auto blocks_dir{cast_const_locked_directory(blocks_dir_)};
         auto context{cast_const_context(context_)};
         if (!context) {
             return nullptr;
         }
         kernel::CacheSizes cache_sizes{DEFAULT_KERNEL_CACHE};
         return reinterpret_cast<kernel_BlockManagerOptions*>(new node::BlockManager::Options{
             .chainparams = *context->m_chainparams,
-            .blocks_dir = abs_blocks_dir,
+            .blocks_dir = blocks_dir->path,
             .notifications = *context->m_notifications,
             .block_tree_db_params = DBParams{
-                .path = abs_data_dir / "blocks" / "index",
+                .path = data_dir->path / "blocks" / "index",
                 .cache_bytes = cache_sizes.block_tree_db,
             }});
     } catch (const std::exception& e) {
@@ -887,12 +885,14 @@ kernel_ChainstateManager* kernel_chainstate_manager_create(
     try {
         const auto& chainstate_load_opts{*cast_const_chainstate_load_options(chainstate_load_opts_)};
         const LockedDirectory data_dir{chainman_opts->datadir};
+        const LockedDirectory blocks_dir{blockman_opts->blocks_dir};
 
         auto cleanup_chainman = [&]() {
             kernel_chainstate_manager_destroy(
                 reinterpret_cast<kernel_ChainstateManager*>(chainman),
                 context_,
-                reinterpret_cast<const kernel_LockedDirectory*>(&data_dir));
+                reinterpret_cast<const kernel_LockedDirectory*>(&data_dir),
+                reinterpret_cast<const kernel_LockedDirectory*>(&blocks_dir));
         };
 
         if (blockman_opts->block_tree_db_params.wipe_data && !chainstate_load_opts.wipe_chainstate_db) {
@@ -934,7 +934,8 @@ kernel_ChainstateManager* kernel_chainstate_manager_create(
 void kernel_chainstate_manager_destroy(
     kernel_ChainstateManager* chainman_,
     const kernel_Context* context_,
-    const kernel_LockedDirectory* data_dir_)
+    const kernel_LockedDirectory* data_dir_,
+    const kernel_LockedDirectory* blocks_dir_)
 {
     if (!chainman_) return;
 

src/kernel/bitcoinkernel.h

@@ -850,20 +850,16 @@ BITCOINKERNEL_API void kernel_chainstate_manager_options_destroy(kernel_Chainsta
  * @param[in] context          Non-null, the created options will associate with this kernel context
  *                             for the duration of their lifetime. The same context needs to be used
  *                             when instantiating the chainstate manager.
- * @param[in] data_directory   Non-null, path string of the directory containing the chainstate data.
+ * @param[in] data_directory   Non-null, directory containing the chainstate data.
  *                             This is usually the same as the data directory used for the chainstate
- *                             manager options. If the directory does not exist yet, it will be created.
- * @param[in] blocks_directory Non-null, path string of the directory containing the block data. If
- *                             the directory does not exist yet, it will be created.
+ *                             manager options.
+ * @param[in] blocks_directory Non-null, directory containing the block data.
  * @return                     The allocated block manager options, or null on error.
  */
 BITCOINKERNEL_API kernel_BlockManagerOptions* BITCOINKERNEL_WARN_UNUSED_RESULT kernel_block_manager_options_create(
     const kernel_Context* context,
-    const char* data_directory,
-    size_t data_directory_len,
-    const char* blocks_directory,
-    size_t blocks_directory_len
-) BITCOINKERNEL_ARG_NONNULL(1, 2);
+    const kernel_LockedDirectory* data_directory,
+    const kernel_LockedDirectory* blocks_directory) BITCOINKERNEL_ARG_NONNULL(1, 2, 3);
 
 /**
  * @brief Sets wipe block tree db in the block manager options.
@@ -1001,7 +997,8 @@ BITCOINKERNEL_API bool BITCOINKERNEL_WARN_UNUSED_RESULT kernel_chainstate_manage
 BITCOINKERNEL_API void kernel_chainstate_manager_destroy(
     kernel_ChainstateManager* chainstate_manager,
     const kernel_Context* context,
-    const kernel_LockedDirectory* data_dir);
+    const kernel_LockedDirectory* data_dir,
+    const kernel_LockedDirectory* blocks_dir) BITCOINKERNEL_ARG_NONNULL(1, 2, 3, 4);
 
 ///@}
 

src/kernel/bitcoinkernel_wrapper.h

@@ -398,6 +398,7 @@ class LockedDirectory
     /** Check whether this LockedDirectory object is valid. */
     explicit operator bool() const noexcept { return bool{m_directory}; }
 
+    friend class BlockManagerOptions;
     friend class ChainMan;
     friend class ChainstateManagerOptions;
 };
@@ -444,8 +445,8 @@ class BlockManagerOptions
     std::unique_ptr<kernel_BlockManagerOptions, Deleter> m_options;
 
 public:
-    BlockManagerOptions(const Context& context, const std::string& data_dir, const std::string& blocks_dir) noexcept
-        : m_options{kernel_block_manager_options_create(context.m_context.get(), data_dir.c_str(), data_dir.length(), blocks_dir.c_str(), blocks_dir.length())}
+    BlockManagerOptions(const Context& context, const LockedDirectory& data_dir, const LockedDirectory& blocks_dir) noexcept
+        : m_options{kernel_block_manager_options_create(context.m_context.get(), data_dir.m_directory.get(), blocks_dir.m_directory.get())}
     {
     }
 
@@ -627,21 +628,24 @@ class ChainMan
     kernel_ChainstateManager* m_chainman;
     const Context& m_context;
     const LockedDirectory& m_data_dir;
+    const LockedDirectory& m_blocks_dir;
 
 public:
     ChainMan(
         const Context& context,
         const ChainstateManagerOptions& chainman_opts,
         const BlockManagerOptions& blockman_opts,
         ChainstateLoadOptions& chainstate_load_opts,
-        const LockedDirectory& data_dir) noexcept // TODO: data_dir is also a member of (opaque) chainman_opts, should ideally be de-duplicated.
+        const LockedDirectory& data_dir,            // TODO: data_dir is also a member of (opaque) chainman_opts, should ideally be de-duplicated.
+        const LockedDirectory& blocks_dir) noexcept // TODO: blocks_dir is also a member of (opaque) blockman_opts, should ideally be de-duplicated.
         : m_chainman{kernel_chainstate_manager_create(
               context.m_context.get(),
               chainman_opts.m_options.get(),
               blockman_opts.m_options.get(),
               chainstate_load_opts.m_options.get())},
           m_context{context},
-          m_data_dir{data_dir}
+          m_data_dir{data_dir},
+          m_blocks_dir{blocks_dir}
     {
     }
 
@@ -715,7 +719,7 @@ class ChainMan
 
     ~ChainMan()
     {
-        kernel_chainstate_manager_destroy(m_chainman, m_context.m_context.get(), m_data_dir.m_directory.get());
+        kernel_chainstate_manager_destroy(m_chainman, m_context.m_context.get(), m_data_dir.m_directory.get(), m_blocks_dir.m_directory.get());
     }
 };
 

src/test/kernel/test_kernel.cpp

@@ -388,11 +388,13 @@ void chainman_test()
     ChainstateManagerOptions chainman_opts{context, data_dir};
     assert(chainman_opts);
     chainman_opts.SetWorkerThreads(4);
-    BlockManagerOptions blockman_opts{context, test_directory.m_directory.string(), (test_directory.m_directory / "blocks").string()};
+    LockedDirectory blocks_dir{test_directory.m_directory / "blocks"};
+    assert(blocks_dir);
+    BlockManagerOptions blockman_opts{context, data_dir, blocks_dir};
     assert(blockman_opts);
     ChainstateLoadOptions chainstate_load_opts{};
 
-    ChainMan chainman{context, chainman_opts, blockman_opts, chainstate_load_opts, data_dir};
+    ChainMan chainman{context, chainman_opts, blockman_opts, chainstate_load_opts, data_dir, blocks_dir};
     assert(chainman);
 }
 
@@ -406,7 +408,9 @@ std::unique_ptr<ChainMan> create_chainman(TestDirectory& test_directory,
     LockedDirectory data_dir{test_directory.m_directory.string()};
     ChainstateManagerOptions chainman_opts{context, data_dir};
     assert(chainman_opts);
-    BlockManagerOptions blockman_opts{context, test_directory.m_directory.string(), (test_directory.m_directory / "blocks").string()};
+    LockedDirectory blocks_dir{test_directory.m_directory / "blocks"};
+    assert(blocks_dir);
+    BlockManagerOptions blockman_opts{context, data_dir, blocks_dir};
     assert(blockman_opts);
     ChainstateLoadOptions chainstate_load_opts{};
 
@@ -424,7 +428,7 @@ std::unique_ptr<ChainMan> create_chainman(TestDirectory& test_directory,
         chainstate_load_opts.SetChainstateDbInMemory(chainstate_db_in_memory);
     }
 
-    auto chainman{std::make_unique<ChainMan>(context, chainman_opts, blockman_opts, chainstate_load_opts, data_dir)};
+    auto chainman{std::make_unique<ChainMan>(context, chainman_opts, blockman_opts, chainstate_load_opts, data_dir, blocks_dir)};
     assert(chainman);
     return chainman;
 }