hide chaindata_env of TempChainData, TestDatabaseContext and DataStore

silkworm/capi/silkworm_test.cpp

@@ -45,8 +45,8 @@ using namespace silkworm::db;
 struct CApiTest : public db::test_util::TestDatabaseContext {
     TemporaryDirectory tmp_dir;
     SilkwormSettings settings{.log_verbosity = SilkwormLogLevel::SILKWORM_LOG_NONE};
-    mdbx::env env{mdbx_env()};
-    std::filesystem::path env_path() { return mdbx_env().get_path(); }
+    mdbx::env env{*chaindata_rw()};
+    const std::filesystem::path& env_path() { return chaindata_dir_path(); }
 };
 
 //! Utility to copy `src` C-string to `dst` fixed-size char array

silkworm/db/data_store.hpp

@@ -57,11 +57,6 @@ class DataStore {
         return {store_.chaindata_rw(), store_.repository(blocks::kBlocksRepositoryName)};
     }
 
-    // TODO: remove this, use RXAccess instead
-    mdbx::env chaindata_env() const { return store_.chaindata_env(); }
-    // TODO: remove this, use RXAccess instead
-    mdbx::env* chaindata_env_ptr() { return store_.chaindata_env_ptr(); }
-
     db::ROAccess chaindata() const { return store_.chaindata(); }
     db::RWAccess chaindata_rw() const { return store_.chaindata_rw(); }
 

silkworm/db/datastore/data_store.hpp

@@ -39,15 +39,6 @@ class DataStore {
             entry.second->close();
     }
 
-    // TODO: remove this, use RXAccess instead
-    mdbx::env chaindata_env() const {
-        return chaindata_env_;  // NOLINT(cppcoreguidelines-slicing)
-    }
-    // TODO: remove this, use RXAccess instead
-    mdbx::env* chaindata_env_ptr() {
-        return &chaindata_env_;  // NOLINT(cppcoreguidelines-slicing)
-    }
-
     db::ROAccess chaindata() const { return db::ROAccess{chaindata_env_}; }
     db::RWAccess chaindata_rw() const { return db::RWAccess{chaindata_env_}; }
     snapshots::SnapshotRepository& repository(const EntityName& name) const { return *repositories_.at(name); }

silkworm/db/kv/api/local_cursor_test.cpp

@@ -41,7 +41,7 @@ struct LocalCursorTest : public ContextTestBase, TestDatabaseContext {
 
 TEST_CASE_METHOD(LocalCursorTest, "LocalCursor::open_cursor", "[db][kv][api][local_cursor]") {
     spawn_and_wait([&]() -> Task<void> {
-        ROTxnManaged txn{mdbx_env()};
+        ROTxnManaged txn = chaindata().start_ro_tx();
         LocalCursor cursor{txn, ++last_cursor_id};
 
         CHECK_NOTHROW(co_await cursor.open_cursor(table::kHeadersName, /*is_dup_sorted=*/false));
@@ -51,7 +51,7 @@ TEST_CASE_METHOD(LocalCursorTest, "LocalCursor::open_cursor", "[db][kv][api][loc
 
 TEST_CASE_METHOD(LocalCursorTest, "LocalCursor::close_cursor", "[db][kv][api][local_cursor]") {
     spawn_and_wait([&]() -> Task<void> {
-        ROTxnManaged txn{mdbx_env()};
+        ROTxnManaged txn = chaindata().start_ro_tx();
         LocalCursor cursor{txn, ++last_cursor_id};
         REQUIRE_NOTHROW(co_await cursor.open_cursor(table::kHeadersName, /*is_dup_sorted=*/false));
         REQUIRE(cursor.cursor_id() > 0);
@@ -68,7 +68,7 @@ static auto decode_header(ByteView data_view) {
 
 TEST_CASE_METHOD(LocalCursorTest, "LocalCursor::first", "[db][kv][api][local_cursor]") {
     spawn_and_wait([&]() -> Task<void> {
-        ROTxnManaged txn{mdbx_env()};
+        ROTxnManaged txn = chaindata().start_ro_tx();
         LocalCursor cursor{txn, ++last_cursor_id};
         REQUIRE_NOTHROW(co_await cursor.open_cursor(table::kHeadersName, /*is_dup_sorted=*/false));
 
@@ -83,7 +83,7 @@ TEST_CASE_METHOD(LocalCursorTest, "LocalCursor::first", "[db][kv][api][local_cur
 
 TEST_CASE_METHOD(LocalCursorTest, "LocalCursor::last", "[db][kv][api][local_cursor]") {
     spawn_and_wait([&]() -> Task<void> {
-        ROTxnManaged txn{mdbx_env()};
+        ROTxnManaged txn = chaindata().start_ro_tx();
         LocalCursor cursor{txn, ++last_cursor_id};
         REQUIRE_NOTHROW(co_await cursor.open_cursor(table::kHeadersName, /*is_dup_sorted=*/false));
 
@@ -98,7 +98,7 @@ TEST_CASE_METHOD(LocalCursorTest, "LocalCursor::last", "[db][kv][api][local_curs
 
 TEST_CASE_METHOD(LocalCursorTest, "LocalCursor::next", "[db][kv][api][local_cursor]") {
     spawn_and_wait([&]() -> Task<void> {
-        ROTxnManaged txn{mdbx_env()};
+        ROTxnManaged txn = chaindata().start_ro_tx();
         LocalCursor cursor{txn, ++last_cursor_id};
         REQUIRE_NOTHROW(co_await cursor.open_cursor(table::kHeadersName, /*is_dup_sorted=*/false));
 
@@ -116,7 +116,7 @@ TEST_CASE_METHOD(LocalCursorTest, "LocalCursor::next", "[db][kv][api][local_curs
 
 TEST_CASE_METHOD(LocalCursorTest, "LocalCursor::previous", "[db][kv][api][local_cursor]") {
     spawn_and_wait([&]() -> Task<void> {
-        ROTxnManaged txn{mdbx_env()};
+        ROTxnManaged txn = chaindata().start_ro_tx();
         LocalCursor cursor{txn, ++last_cursor_id};
         REQUIRE_NOTHROW(co_await cursor.open_cursor(table::kHeadersName, /*is_dup_sorted=*/false));
 

silkworm/db/test_util/temp_chain_data.hpp

@@ -48,15 +48,11 @@ class TempChainData {
 
     const db::EnvConfig& chaindata_env_config() const { return chaindata_env_config_; }
 
-    // TODO: make private and use RXAccess
-    virtual mdbx::env env() const {
-        return *env_;  // NOLINT(cppcoreguidelines-slicing)
+    virtual db::ROAccess chaindata() const {
+        return db::ROAccess{*env_};
     }
-    db::ROAccess chaindata() const {
-        return db::ROAccess{env()};
-    }
-    db::RWAccess chaindata_rw() const {
-        return db::RWAccess{env()};
+    virtual db::RWAccess chaindata_rw() const {
+        return db::RWAccess{*env_};
     }
 
     mdbx::txn& txn() const { return *txn_; }
@@ -99,13 +95,16 @@ class TempChainDataStore : public TempChainData {
         txn_.reset();
     }
 
-    mdbx::env env() const override {
-        return data_store_.chaindata_env();
-    }
-
     db::DataStore& operator*() { return data_store_; }
     db::DataStore* operator->() { return &data_store_; }
 
+    db::ROAccess chaindata() const override {
+        return data_store_.chaindata();
+    }
+    db::RWAccess chaindata_rw() const override {
+        return data_store_.chaindata_rw();
+    }
+
     db::DataModelFactory data_model_factory() {
         return db::DataModelFactory{data_store_.ref()};
     }

silkworm/db/test_util/test_database_context.cpp

@@ -189,6 +189,10 @@ TestDatabaseContext::TestDatabaseContext()
     : chaindata_dir_path_{TemporaryDirectory::get_unique_temporary_path()},
       env_{std::make_unique<mdbx::env_managed>(initialize_test_database(chaindata_dir_path_))} {}
 
+TestDatabaseContext::TestDatabaseContext(const TemporaryDirectory& tmp_dir)
+    : chaindata_dir_path_{DataDirectory{tmp_dir.path()}.chaindata().path()},
+      env_{std::make_unique<mdbx::env_managed>(initialize_test_database(chaindata_dir_path_))} {}
+
 silkworm::ChainConfig TestDatabaseContext::get_chain_config() const {
     ROTxnManaged txn = chaindata().start_ro_tx();
     auto chain_config = read_chain_config(txn);

silkworm/db/test_util/test_database_context.hpp

@@ -36,6 +36,7 @@ void populate_blocks(db::RWTxn& txn, const std::filesystem::path& tests_dir, InM
 class TestDatabaseContext {
   public:
     TestDatabaseContext();
+    explicit TestDatabaseContext(const TemporaryDirectory& tmp_dir);
 
     virtual ~TestDatabaseContext() {
         if (env_) {
@@ -44,18 +45,15 @@ class TestDatabaseContext {
         }
     }
 
-    // TODO: make private and use RXAccess
-    virtual mdbx::env mdbx_env() const {
-        return *env_;  // NOLINT(cppcoreguidelines-slicing)
+    virtual db::ROAccess chaindata() const {
+        return db::ROAccess{*env_};
     }
-    db::ROAccess chaindata() const {
-        return db::ROAccess{mdbx_env()};
-    }
-    db::RWAccess chaindata_rw() const {
-        return db::RWAccess{mdbx_env()};
+    virtual db::RWAccess chaindata_rw() const {
+        return db::RWAccess{*env_};
     }
 
     silkworm::ChainConfig get_chain_config() const;
+    const std::filesystem::path& chaindata_dir_path() const { return chaindata_dir_path_; }
 
   protected:
     mdbx::env_managed move_env() {
@@ -71,7 +69,8 @@ class TestDatabaseContext {
 class TestDataStore : public TestDatabaseContext {
   public:
     explicit TestDataStore(const TemporaryDirectory& tmp_dir)
-        : data_store_{
+        : TestDatabaseContext{tmp_dir},
+          data_store_{
               move_env(),
               blocks::make_blocks_repository(
                   DataDirectory{tmp_dir.path(), true}.snapshots().path()),
@@ -82,13 +81,16 @@ class TestDataStore : public TestDatabaseContext {
         std::filesystem::remove_all(chaindata_dir_path_);
     }
 
-    mdbx::env mdbx_env() const override {
-        return data_store_.chaindata_env();
-    }
-
     db::DataStore& operator*() { return data_store_; }
     db::DataStore* operator->() { return &data_store_; }
 
+    db::ROAccess chaindata() const override {
+        return data_store_.chaindata();
+    }
+    db::RWAccess chaindata_rw() const override {
+        return data_store_.chaindata_rw();
+    }
+
     db::DataModelFactory data_model_factory() {
         return db::DataModelFactory{data_store_.ref()};
     }

silkworm/node/execution/active_direct_service_test.cpp

@@ -48,11 +48,10 @@ class ActiveDirectServiceForTest : public ActiveDirectService {
 struct ActiveDirectServiceTest : public TaskRunner {
     explicit ActiveDirectServiceTest()
         : log_guard{log::Level::kNone},
-          settings{make_node_settings_from_temp_chain_data(tmp_chaindata)},
-          dba{tmp_chaindata.env()} {
+          settings{make_node_settings_from_temp_chain_data(tmp_chaindata)} {
         tmp_chaindata.add_genesis_data();
         tmp_chaindata.commit_txn();
-        mock_execution_engine = std::make_unique<NiceMock<MockExecutionEngine>>(executor(), settings, dba);
+        mock_execution_engine = std::make_unique<NiceMock<MockExecutionEngine>>(executor(), settings, tmp_chaindata.chaindata_rw());
         direct_service = std::make_unique<ActiveDirectServiceForTest>(*mock_execution_engine, execution_context);
         execution_context_thread = std::thread{[this]() {
             direct_service->execution_loop();
@@ -68,7 +67,6 @@ struct ActiveDirectServiceTest : public TaskRunner {
     SetLogVerbosityGuard log_guard;
     TempChainData tmp_chaindata;
     NodeSettings settings;
-    db::RWAccess dba;
     std::unique_ptr<MockExecutionEngine> mock_execution_engine;
     boost::asio::io_context execution_context;
     std::unique_ptr<ActiveDirectServiceForTest> direct_service;

silkworm/node/execution/direct_service_test.cpp

@@ -40,18 +40,16 @@ using silkworm::test_util::TaskRunner;
 struct DirectServiceTest : public TaskRunner {
     explicit DirectServiceTest()
         : log_guard{log::Level::kNone},
-          settings{make_node_settings_from_temp_chain_data(tmp_chaindata)},
-          dba{tmp_chaindata.env()} {
+          settings{make_node_settings_from_temp_chain_data(tmp_chaindata)} {
         tmp_chaindata.add_genesis_data();
         tmp_chaindata.commit_txn();
-        mock_execution_engine = std::make_unique<MockExecutionEngine>(executor(), settings, dba);
+        mock_execution_engine = std::make_unique<MockExecutionEngine>(executor(), settings, tmp_chaindata.chaindata_rw());
         direct_service = std::make_unique<DirectService>(*mock_execution_engine);
     }
 
     SetLogVerbosityGuard log_guard;
     TempChainData tmp_chaindata;
     NodeSettings settings;
-    db::RWAccess dba;
     std::unique_ptr<MockExecutionEngine> mock_execution_engine;
     std::unique_ptr<DirectService> direct_service;
 };

silkworm/node/stagedsync/execution_engine_test.cpp

@@ -66,13 +66,13 @@ TEST_CASE("ExecutionEngine Integration Test", "[node][execution][execution_engin
     db::DataModelFactory data_model_factory = db_context.data_model_factory();
 
     auto node_settings = NodeSettings{
-        .data_directory = std::make_unique<DataDirectory>(db_context.mdbx_env().get_path(), false),
+        .data_directory = std::make_unique<DataDirectory>(tmp_dir.path(), false),
         .chain_config = db_context.get_chain_config(),
         .parallel_fork_tracking_enabled = false,
         .keep_db_txn_open = true,
     };
 
-    RWAccess db_access{db_context.mdbx_env()};
+    RWAccess db_access = db_context.chaindata_rw();
 
     ExecutionEngineForTest exec_engine{
         runner.executor(),
@@ -831,7 +831,7 @@ TEST_CASE("ExecutionEngine") {
         data_model_factory,
         /* log_timer_factory = */ std::nullopt,
         make_stages_factory(node_settings, data_model_factory),
-        RWAccess{context.env()},
+        context.chaindata_rw(),
     };
     exec_engine.open();
 

silkworm/sync/internals/body_sequence_test.cpp

@@ -449,7 +449,7 @@ TEST_CASE("body downloading", "[silkworm][sync][BodySequence]") {
         BlockHeader header2;
         header2.number = 2;
         header2.parent_hash = header1_hash;
-        db::RWTxnManaged txn2{context.env()};
+        db::RWTxnManaged txn2 = context.chaindata_rw().start_rw_tx();
         db::write_canonical_header_hash(txn2, header2.hash().bytes, 1);
         db::write_canonical_header(txn2, header2);
         db::write_header(txn2, header2, true);