From fdcf0b17cbc5afd8982d1b8ff02f80b5defeb2f0 Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Wed, 19 Apr 2023 10:27:37 -0300 Subject: [PATCH 1/5] Remove store dependency from unchecked_map --- nano/core_test/block_store.cpp | 6 ++--- nano/core_test/ledger.cpp | 4 ++-- nano/core_test/unchecked_map.cpp | 39 ++++++++------------------------ nano/node/node.cpp | 2 +- nano/node/unchecked_map.cpp | 4 +--- nano/node/unchecked_map.hpp | 10 ++------ 6 files changed, 19 insertions(+), 46 deletions(-) diff --git a/nano/core_test/block_store.cpp b/nano/core_test/block_store.cpp index 52396d51ef..20ca3689db 100644 --- a/nano/core_test/block_store.cpp +++ b/nano/core_test/block_store.cpp @@ -447,9 +447,7 @@ TEST (block_store, empty_bootstrap) { nano::test::system system{}; nano::logger_mt logger; - auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants); - nano::unchecked_map unchecked{ *store, system.stats, false }; - ASSERT_TRUE (!store->init_error ()); + nano::unchecked_map unchecked{ system.stats, false }; size_t count = 0; unchecked.for_each ([&count] (nano::unchecked_key const & key, nano::unchecked_info const & info) { ++count; @@ -961,6 +959,7 @@ TEST (block_store, pruned_random) ASSERT_EQ (hash1, random_hash); } +/* This test has no code to test. namespace nano { namespace lmdb @@ -1008,6 +1007,7 @@ namespace lmdb } } } + */ TEST (block_store, state_block) { diff --git a/nano/core_test/ledger.cpp b/nano/core_test/ledger.cpp index 666707d145..3fc61244c3 100644 --- a/nano/core_test/ledger.cpp +++ b/nano/core_test/ledger.cpp @@ -5541,7 +5541,7 @@ TEST (ledger, migrate_lmdb_to_rocksdb) boost::asio::ip::address_v6 address (boost::asio::ip::make_address_v6 ("::ffff:127.0.0.1")); uint16_t port = 100; nano::lmdb::store store{ logger, path / "data.ldb", nano::dev::constants }; - nano::unchecked_map unchecked{ store, system.stats, false }; + nano::unchecked_map unchecked{ system.stats, false }; nano::ledger ledger{ store, system.stats, nano::dev::constants }; nano::work_pool pool{ nano::dev::network_params.network, std::numeric_limits::max () }; @@ -5582,7 +5582,7 @@ TEST (ledger, migrate_lmdb_to_rocksdb) ASSERT_FALSE (error); nano::rocksdb::store rocksdb_store{ logger, path / "rocksdb", nano::dev::constants }; - nano::unchecked_map rocksdb_unchecked{ rocksdb_store, system.stats, false }; + nano::unchecked_map rocksdb_unchecked{ system.stats, false }; auto rocksdb_transaction (rocksdb_store.tx_begin_read ()); nano::pending_info pending_info{}; diff --git a/nano/core_test/unchecked_map.cpp b/nano/core_test/unchecked_map.cpp index c90f9d2bc9..b2012c4f5c 100644 --- a/nano/core_test/unchecked_map.cpp +++ b/nano/core_test/unchecked_map.cpp @@ -2,7 +2,7 @@ #include #include #include -#include +#include #include #include #include @@ -19,13 +19,10 @@ class context { public: context () : - store{ nano::make_store (logger, nano::unique_path (), nano::dev::constants) }, - unchecked{ *store, stats, false } + unchecked{ stats, false } { } - nano::logger_mt logger; nano::stats stats; - std::unique_ptr store; nano::unchecked_map unchecked; }; std::shared_ptr block () @@ -58,10 +55,7 @@ TEST (unchecked_map, put_one) TEST (block_store, one_bootstrap) { nano::test::system system{}; - nano::logger_mt logger{}; - auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants); - nano::unchecked_map unchecked{ *store, system.stats, false }; - ASSERT_TRUE (!store->init_error ()); + nano::unchecked_map unchecked{ system.stats, false }; nano::block_builder builder; auto block1 = builder .send () @@ -72,12 +66,11 @@ TEST (block_store, one_bootstrap) .work (5) .build_shared (); unchecked.put (block1->hash (), nano::unchecked_info{ block1 }); - auto check_block_is_listed = [&] (nano::transaction const & transaction_a, nano::block_hash const & block_hash_a) { + auto check_block_is_listed = [&] (nano::block_hash const & block_hash_a) { return unchecked.get (block_hash_a).size () > 0; }; // Waits for the block1 to get saved in the database - ASSERT_TIMELY (10s, check_block_is_listed (store->tx_begin_read (), block1->hash ())); - auto transaction = store->tx_begin_read (); + ASSERT_TIMELY (10s, check_block_is_listed (block1->hash ())); std::vector dependencies; unchecked.for_each ([&dependencies] (nano::unchecked_key const & key, nano::unchecked_info const & info) { dependencies.push_back (key.key ()); @@ -95,10 +88,7 @@ TEST (block_store, one_bootstrap) TEST (unchecked, simple) { nano::test::system system{}; - nano::logger_mt logger{}; - auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants); - nano::unchecked_map unchecked{ *store, system.stats, false }; - ASSERT_TRUE (!store->init_error ()); + nano::unchecked_map unchecked{ system.stats, false }; nano::block_builder builder; auto block = builder .send () @@ -139,10 +129,7 @@ TEST (unchecked, multiple) // Don't test this in rocksdb mode GTEST_SKIP (); } - nano::logger_mt logger{}; - auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants); - nano::unchecked_map unchecked{ *store, system.stats, false }; - ASSERT_TRUE (!store->init_error ()); + nano::unchecked_map unchecked{ system.stats, false }; nano::block_builder builder; auto block = builder .send () @@ -172,10 +159,7 @@ TEST (unchecked, multiple) TEST (unchecked, double_put) { nano::test::system system{}; - nano::logger_mt logger{}; - auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants); - nano::unchecked_map unchecked{ *store, system.stats, false }; - ASSERT_TRUE (!store->init_error ()); + nano::unchecked_map unchecked{ system.stats, false }; nano::block_builder builder; auto block = builder .send () @@ -206,10 +190,7 @@ TEST (unchecked, double_put) TEST (unchecked, multiple_get) { nano::test::system system{}; - nano::logger_mt logger{}; - auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants); - nano::unchecked_map unchecked{ *store, system.stats, false }; - ASSERT_TRUE (!store->init_error ()); + nano::unchecked_map unchecked{ system.stats, false }; // Instantiates three blocks nano::block_builder builder; auto block1 = builder @@ -248,7 +229,7 @@ TEST (unchecked, multiple_get) // count the number of blocks in the unchecked table by counting them one by one // we cannot trust the count() method if the backend is rocksdb - auto count_unchecked_blocks_one_by_one = [&store, &unchecked] () { + auto count_unchecked_blocks_one_by_one = [&unchecked] () { size_t count = 0; unchecked.for_each ([&count] (nano::unchecked_key const & key, nano::unchecked_info const & info) { ++count; diff --git a/nano/node/node.cpp b/nano/node/node.cpp index 26018241a6..cb519f63ed 100644 --- a/nano/node/node.cpp +++ b/nano/node/node.cpp @@ -159,7 +159,7 @@ nano::node::node (boost::asio::io_context & io_ctx_a, boost::filesystem::path co logger (config_a.logging.min_time_between_log_output), store_impl (nano::make_store (logger, application_path_a, network_params.ledger, flags.read_only, true, config_a.rocksdb_config, config_a.diagnostics_config.txn_tracking, config_a.block_processor_batch_max_time, config_a.lmdb_config, config_a.backup_before_upgrade)), store (*store_impl), - unchecked{ store, stats, flags.disable_block_processor_unchecked_deletion }, + unchecked{ stats, flags.disable_block_processor_unchecked_deletion }, wallets_store_impl (std::make_unique (application_path_a / "wallets.ldb", config_a.lmdb_config)), wallets_store (*wallets_store_impl), gap_cache (*this), diff --git a/nano/node/unchecked_map.cpp b/nano/node/unchecked_map.cpp index bc12e9f7f7..64b2b9cf0c 100644 --- a/nano/node/unchecked_map.cpp +++ b/nano/node/unchecked_map.cpp @@ -4,10 +4,8 @@ #include #include #include -#include -nano::unchecked_map::unchecked_map (nano::store & store, nano::stats & stats, bool const & disable_delete) : - store{ store }, +nano::unchecked_map::unchecked_map (nano::stats & stats, bool const & disable_delete) : stats{ stats }, disable_delete{ disable_delete }, thread{ [this] () { run (); } } diff --git a/nano/node/unchecked_map.hpp b/nano/node/unchecked_map.hpp index cfcb8b59d0..b76e8ccb44 100644 --- a/nano/node/unchecked_map.hpp +++ b/nano/node/unchecked_map.hpp @@ -3,7 +3,7 @@ #include #include #include -#include +#include #include #include @@ -18,16 +18,11 @@ namespace mi = boost::multi_index; namespace nano { class stats; -class store; -class transaction; -class unchecked_info; -class unchecked_key; -class write_transaction; class unchecked_map { public: - unchecked_map (nano::store &, nano::stats &, bool const & do_delete); + unchecked_map (nano::stats &, bool const & do_delete); ~unchecked_map (); void put (nano::hash_or_account const & dependency, nano::unchecked_info const & info); @@ -56,7 +51,6 @@ class unchecked_map void query_impl (nano::block_hash const & hash); private: // Dependencies - nano::store & store; nano::stats & stats; private: From f5ec339a10d0d783292c12a2c1811ff5645d9f87 Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Wed, 19 Apr 2023 12:56:19 -0300 Subject: [PATCH 2/5] Remove unused transaction objects from json_handler.cpp --- nano/node/json_handler.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index 61f7357e9e..d4f6fa64e3 100644 --- a/nano/node/json_handler.cpp +++ b/nano/node/json_handler.cpp @@ -4114,7 +4114,6 @@ void nano::json_handler::unchecked () if (!ec) { boost::property_tree::ptree unchecked; - auto transaction (node.store.tx_begin_read ()); node.unchecked.for_each ( [&unchecked, &json_block_l] (nano::unchecked_key const & key, nano::unchecked_info const & info) { if (json_block_l) @@ -4137,7 +4136,6 @@ void nano::json_handler::unchecked () void nano::json_handler::unchecked_clear () { node.workers.push_task (create_worker_task ([] (std::shared_ptr const & rpc_l) { - auto transaction (rpc_l->node.store.tx_begin_write ({ tables::unchecked })); rpc_l->node.unchecked.clear (); rpc_l->response_l.put ("success", ""); rpc_l->response_errors (); @@ -4195,7 +4193,6 @@ void nano::json_handler::unchecked_keys () if (!ec) { boost::property_tree::ptree unchecked; - auto transaction (node.store.tx_begin_read ()); node.unchecked.for_each ( key, [&unchecked, json_block_l] (nano::unchecked_key const & key, nano::unchecked_info const & info) { From 2500dfb9021fae126bcbe33347b8efce9cfae0fe Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Wed, 19 Apr 2023 13:00:13 -0300 Subject: [PATCH 3/5] Remove the unchecked_store from the store classes Kept the lmdb::unchecked_store because it contains the LMDB file handler that will be used by the database upgrade code (to be added). --- nano/core_test/block_store.cpp | 2 + nano/node/CMakeLists.txt | 3 -- nano/node/blockprocessor.cpp | 2 +- nano/node/lmdb/lmdb.cpp | 3 +- nano/node/lmdb/unchecked_store.cpp | 52 ---------------------- nano/node/lmdb/unchecked_store.hpp | 38 ++++------------ nano/node/rocksdb/rocksdb.cpp | 2 - nano/node/rocksdb/rocksdb.hpp | 7 +-- nano/node/rocksdb/unchecked_store.cpp | 52 ---------------------- nano/node/rocksdb/unchecked_store.hpp | 28 ------------ nano/secure/store.cpp | 16 ------- nano/secure/store.hpp | 62 --------------------------- 12 files changed, 15 insertions(+), 252 deletions(-) delete mode 100644 nano/node/lmdb/unchecked_store.cpp delete mode 100644 nano/node/rocksdb/unchecked_store.cpp delete mode 100644 nano/node/rocksdb/unchecked_store.hpp diff --git a/nano/core_test/block_store.cpp b/nano/core_test/block_store.cpp index 20ca3689db..5aee62d2cb 100644 --- a/nano/core_test/block_store.cpp +++ b/nano/core_test/block_store.cpp @@ -2436,6 +2436,7 @@ namespace nano { // This thest ensures the tombstone_count is increased when there is a delete. The tombstone_count is part of a flush // logic bound to the way RocksDB is used by the node. +/* The unchecked table was deprecated and it is being removed, rewrite this test using another table TEST (rocksdb_block_store, tombstone_count) { if (!nano::rocksdb_config::using_rocksdb_in_tests ()) @@ -2469,6 +2470,7 @@ TEST (rocksdb_block_store, tombstone_count) store->unchecked.del (store->tx_begin_write (), nano::unchecked_key (previous, block->hash ())); ASSERT_TIMELY (5s, store->tombstone_map.at (nano::tables::unchecked).num_since_last_flush.load () == 1); } + */ } namespace nano diff --git a/nano/node/CMakeLists.txt b/nano/node/CMakeLists.txt index 30c373c31e..6016ac9654 100644 --- a/nano/node/CMakeLists.txt +++ b/nano/node/CMakeLists.txt @@ -137,7 +137,6 @@ add_library( lmdb/version_store.hpp lmdb/version_store.cpp lmdb/unchecked_store.hpp - lmdb/unchecked_store.cpp lmdb/lmdb.hpp lmdb/lmdb.cpp lmdb/lmdb_env.hpp @@ -197,8 +196,6 @@ add_library( rocksdb/pending_store.cpp rocksdb/pruned_store.hpp rocksdb/pruned_store.cpp - rocksdb/unchecked_store.hpp - rocksdb/unchecked_store.cpp rocksdb/version_store.hpp rocksdb/version_store.cpp rocksdb/rocksdb.hpp diff --git a/nano/node/blockprocessor.cpp b/nano/node/blockprocessor.cpp index 82817bd186..6be01128a7 100644 --- a/nano/node/blockprocessor.cpp +++ b/nano/node/blockprocessor.cpp @@ -222,7 +222,7 @@ auto nano::block_processor::process_batch (nano::unique_lock & lock { std::deque processed; auto scoped_write_guard = write_database_queue.wait (nano::writer::process_batch); - auto transaction (node.store.tx_begin_write ({ tables::accounts, tables::blocks, tables::frontiers, tables::pending, tables::unchecked })); + auto transaction (node.store.tx_begin_write ({ tables::accounts, tables::blocks, tables::frontiers, tables::pending })); nano::timer timer_l; lock_a.lock (); timer_l.start (); diff --git a/nano/node/lmdb/lmdb.cpp b/nano/node/lmdb/lmdb.cpp index 2ecb3a2370..a9d324fae3 100644 --- a/nano/node/lmdb/lmdb.cpp +++ b/nano/node/lmdb/lmdb.cpp @@ -45,7 +45,6 @@ nano::lmdb::store::store (nano::logger_mt & logger_a, boost::filesystem::path co frontier_store, account_store, pending_store, - unchecked_store, online_weight_store, pruned_store, peer_store, @@ -63,7 +62,7 @@ nano::lmdb::store::store (nano::logger_mt & logger_a, boost::filesystem::path co peer_store{ *this }, confirmation_height_store{ *this }, final_vote_store{ *this }, - unchecked_store{ *this }, + unchecked_store{}, version_store{ *this }, logger (logger_a), env (error, path_a, nano::mdb_env::options::make ().set_config (lmdb_config_a).set_use_no_mem_init (true)), diff --git a/nano/node/lmdb/unchecked_store.cpp b/nano/node/lmdb/unchecked_store.cpp deleted file mode 100644 index e969fffd80..0000000000 --- a/nano/node/lmdb/unchecked_store.cpp +++ /dev/null @@ -1,52 +0,0 @@ -#include -#include -#include - -nano::lmdb::unchecked_store::unchecked_store (nano::lmdb::store & store_a) : - store (store_a){}; - -void nano::lmdb::unchecked_store::clear (nano::write_transaction const & transaction_a) -{ - auto status = store.drop (transaction_a, tables::unchecked); - store.release_assert_success (status); -} - -void nano::lmdb::unchecked_store::put (nano::write_transaction const & transaction_a, nano::hash_or_account const & dependency, nano::unchecked_info const & info) -{ - auto status = store.put (transaction_a, tables::unchecked, nano::unchecked_key{ dependency, info.block->hash () }, info); - store.release_assert_success (status); -} - -bool nano::lmdb::unchecked_store::exists (nano::transaction const & transaction_a, nano::unchecked_key const & key) -{ - nano::mdb_val value; - auto status = store.get (transaction_a, tables::unchecked, key, value); - release_assert (store.success (status) || store.not_found (status)); - return store.success (status); -} - -void nano::lmdb::unchecked_store::del (nano::write_transaction const & transaction_a, nano::unchecked_key const & key_a) -{ - auto status (store.del (transaction_a, tables::unchecked, key_a)); - store.release_assert_success (status); -} - -nano::store_iterator nano::lmdb::unchecked_store::end () const -{ - return nano::store_iterator (nullptr); -} - -nano::store_iterator nano::lmdb::unchecked_store::begin (nano::transaction const & transaction) const -{ - return store.make_iterator (transaction, tables::unchecked); -} - -nano::store_iterator nano::lmdb::unchecked_store::lower_bound (nano::transaction const & transaction, nano::unchecked_key const & key) const -{ - return store.make_iterator (transaction, tables::unchecked, key); -} - -size_t nano::lmdb::unchecked_store::count (nano::transaction const & transaction_a) -{ - return store.count (transaction_a, tables::unchecked); -} diff --git a/nano/node/lmdb/unchecked_store.hpp b/nano/node/lmdb/unchecked_store.hpp index 3f6b9029a1..7a491befff 100644 --- a/nano/node/lmdb/unchecked_store.hpp +++ b/nano/node/lmdb/unchecked_store.hpp @@ -1,36 +1,16 @@ #pragma once -#include - #include -namespace nano +namespace nano::lmdb { -namespace lmdb +class unchecked_store { - class store; - class unchecked_store : public nano::unchecked_store - { - private: - nano::lmdb::store & store; - - public: - unchecked_store (nano::lmdb::store & store_a); - - void clear (nano::write_transaction const & transaction_a) override; - void put (nano::write_transaction const & transaction_a, nano::hash_or_account const & dependency, nano::unchecked_info const & info_a) override; - bool exists (nano::transaction const & transaction_a, nano::unchecked_key const & unchecked_key_a) override; - void del (nano::write_transaction const & transaction_a, nano::unchecked_key const & key_a) override; - nano::store_iterator end () const override; - nano::store_iterator begin (nano::transaction const & transaction_a) const override; - nano::store_iterator lower_bound (nano::transaction const & transaction_a, nano::unchecked_key const & key_a) const override; - size_t count (nano::transaction const & transaction_a) override; - - /** - * Unchecked bootstrap blocks info. - * nano::block_hash -> nano::unchecked_info - */ - MDB_dbi unchecked_handle{ 0 }; - }; -} +public: + /** + * Unchecked bootstrap blocks info. + * nano::block_hash -> nano::unchecked_info + */ + MDB_dbi unchecked_handle{ 0 }; +}; } diff --git a/nano/node/rocksdb/rocksdb.cpp b/nano/node/rocksdb/rocksdb.cpp index 1ee667f054..c779752278 100644 --- a/nano/node/rocksdb/rocksdb.cpp +++ b/nano/node/rocksdb/rocksdb.cpp @@ -67,7 +67,6 @@ nano::rocksdb::store::store (nano::logger_mt & logger_a, boost::filesystem::path frontier_store, account_store, pending_store, - unchecked_store, online_weight_store, pruned_store, peer_store, @@ -80,7 +79,6 @@ nano::rocksdb::store::store (nano::logger_mt & logger_a, boost::filesystem::path frontier_store{ *this }, account_store{ *this }, pending_store{ *this }, - unchecked_store{ *this }, online_weight_store{ *this }, pruned_store{ *this }, peer_store{ *this }, diff --git a/nano/node/rocksdb/rocksdb.hpp b/nano/node/rocksdb/rocksdb.hpp index 60ee133349..7edafc5dea 100644 --- a/nano/node/rocksdb/rocksdb.hpp +++ b/nano/node/rocksdb/rocksdb.hpp @@ -13,7 +13,6 @@ #include #include #include -#include #include #include @@ -33,8 +32,8 @@ class rocksdb_block_store_tombstone_count_Test; namespace rocksdb { /** - * rocksdb implementation of the block store - */ + * rocksdb implementation of the block store + */ class store : public nano::store { private: @@ -47,7 +46,6 @@ namespace rocksdb nano::rocksdb::peer_store peer_store; nano::rocksdb::pending_store pending_store; nano::rocksdb::pruned_store pruned_store; - nano::rocksdb::unchecked_store unchecked_store; nano::rocksdb::version_store version_store; public: @@ -60,7 +58,6 @@ namespace rocksdb friend class nano::rocksdb::peer_store; friend class nano::rocksdb::pending_store; friend class nano::rocksdb::pruned_store; - friend class nano::rocksdb::unchecked_store; friend class nano::rocksdb::version_store; explicit store (nano::logger_mt &, boost::filesystem::path const &, nano::ledger_constants & constants, nano::rocksdb_config const & = nano::rocksdb_config{}, bool open_read_only = false); diff --git a/nano/node/rocksdb/unchecked_store.cpp b/nano/node/rocksdb/unchecked_store.cpp deleted file mode 100644 index 3e8c266ea8..0000000000 --- a/nano/node/rocksdb/unchecked_store.cpp +++ /dev/null @@ -1,52 +0,0 @@ -#include -#include -#include - -nano::rocksdb::unchecked_store::unchecked_store (nano::rocksdb::store & store_a) : - store (store_a){}; - -void nano::rocksdb::unchecked_store::clear (nano::write_transaction const & transaction_a) -{ - auto status = store.drop (transaction_a, tables::unchecked); - store.release_assert_success (status); -} - -void nano::rocksdb::unchecked_store::put (nano::write_transaction const & transaction_a, nano::hash_or_account const & dependency, nano::unchecked_info const & info) -{ - auto status = store.put (transaction_a, tables::unchecked, nano::unchecked_key{ dependency, info.block->hash () }, info); - store.release_assert_success (status); -} - -bool nano::rocksdb::unchecked_store::exists (nano::transaction const & transaction_a, nano::unchecked_key const & key) -{ - nano::rocksdb_val value; - auto status = store.get (transaction_a, tables::unchecked, key, value); - release_assert (store.success (status) || store.not_found (status)); - return store.success (status); -} - -void nano::rocksdb::unchecked_store::del (nano::write_transaction const & transaction_a, nano::unchecked_key const & key_a) -{ - auto status (store.del (transaction_a, tables::unchecked, key_a)); - store.release_assert_success (status); -} - -nano::store_iterator nano::rocksdb::unchecked_store::end () const -{ - return nano::store_iterator (nullptr); -} - -nano::store_iterator nano::rocksdb::unchecked_store::begin (nano::transaction const & transaction) const -{ - return store.make_iterator (transaction, tables::unchecked); -} - -nano::store_iterator nano::rocksdb::unchecked_store::lower_bound (nano::transaction const & transaction, nano::unchecked_key const & key) const -{ - return store.make_iterator (transaction, tables::unchecked, key); -} - -size_t nano::rocksdb::unchecked_store::count (nano::transaction const & transaction_a) -{ - return store.count (transaction_a, tables::unchecked); -} diff --git a/nano/node/rocksdb/unchecked_store.hpp b/nano/node/rocksdb/unchecked_store.hpp deleted file mode 100644 index 35942c551c..0000000000 --- a/nano/node/rocksdb/unchecked_store.hpp +++ /dev/null @@ -1,28 +0,0 @@ -#pragma once - -#include - -namespace nano -{ -namespace rocksdb -{ - class store; - class unchecked_store : public nano::unchecked_store - { - private: - nano::rocksdb::store & store; - - public: - unchecked_store (nano::rocksdb::store & store_a); - - void clear (nano::write_transaction const & transaction_a) override; - void put (nano::write_transaction const & transaction_a, nano::hash_or_account const & dependency, nano::unchecked_info const & info_a) override; - bool exists (nano::transaction const & transaction_a, nano::unchecked_key const & unchecked_key_a) override; - void del (nano::write_transaction const & transaction_a, nano::unchecked_key const & key_a) override; - nano::store_iterator end () const override; - nano::store_iterator begin (nano::transaction const & transaction_a) const override; - nano::store_iterator lower_bound (nano::transaction const & transaction_a, nano::unchecked_key const & key_a) const override; - size_t count (nano::transaction const & transaction_a) override; - }; -} -} diff --git a/nano/secure/store.cpp b/nano/secure/store.cpp index e6fd573bfa..cc3e727660 100644 --- a/nano/secure/store.cpp +++ b/nano/secure/store.cpp @@ -112,7 +112,6 @@ nano::store::store ( nano::frontier_store & frontier_store_a, nano::account_store & account_store_a, nano::pending_store & pending_store_a, - nano::unchecked_store & unchecked_store_a, nano::online_weight_store & online_weight_store_a, nano::pruned_store & pruned_store_a, nano::peer_store & peer_store_a, @@ -124,7 +123,6 @@ nano::store::store ( frontier (frontier_store_a), account (account_store_a), pending (pending_store_a), - unchecked (unchecked_store_a), online_weight (online_weight_store_a), pruned (pruned_store_a), peer (peer_store_a), @@ -155,20 +153,6 @@ void nano::store::initialize (nano::write_transaction const & transaction_a, nan frontier.put (transaction_a, hash_l, constants.genesis->account ()); } -auto nano::unchecked_store::equal_range (nano::transaction const & transaction, nano::block_hash const & dependency) -> std::pair -{ - nano::unchecked_key begin_l{ dependency, 0 }; - nano::unchecked_key end_l{ nano::block_hash{ dependency.number () + 1 }, 0 }; - // Adjust for edge case where number () + 1 wraps around. - auto end_iter = begin_l.previous < end_l.previous ? lower_bound (transaction, end_l) : end (); - return std::make_pair (lower_bound (transaction, begin_l), std::move (end_iter)); -} - -auto nano::unchecked_store::full_range (nano::transaction const & transaction) -> std::pair -{ - return std::make_pair (begin (transaction), end ()); -} - std::optional nano::account_store::get (const nano::transaction & transaction, const nano::account & account) { nano::account_info info; diff --git a/nano/secure/store.hpp b/nano/secure/store.hpp index a360c5508e..8d16eb81e2 100644 --- a/nano/secure/store.hpp +++ b/nano/secure/store.hpp @@ -103,22 +103,6 @@ class db_val static_assert (std::is_standard_layout::value, "Standard layout is required"); } - db_val (nano::unchecked_info const & val_a) : - buffer (std::make_shared> ()) - { - { - nano::vectorstream stream (*buffer); - val_a.serialize (stream); - } - convert_buffer_to_value (); - } - - db_val (nano::unchecked_key const & val_a) : - db_val (sizeof (val_a), const_cast (&val_a)) - { - static_assert (std::is_standard_layout::value, "Standard layout is required"); - } - db_val (nano::confirmation_height_info const & val_a) : buffer (std::make_shared> ()) { @@ -222,25 +206,6 @@ class db_val return result; } - explicit operator nano::unchecked_info () const - { - nano::bufferstream stream (reinterpret_cast (data ()), size ()); - nano::unchecked_info result; - bool error (result.deserialize (stream)); - (void)error; - debug_assert (!error); - return result; - } - - explicit operator nano::unchecked_key () const - { - nano::unchecked_key result; - debug_assert (size () == sizeof (result)); - static_assert (sizeof (nano::unchecked_key::previous) + sizeof (nano::pending_key::hash) == sizeof (result), "Packed class"); - std::copy (reinterpret_cast (data ()), reinterpret_cast (data ()) + sizeof (result), reinterpret_cast (&result)); - return result; - } - explicit operator nano::uint128_union () const { return convert (); @@ -745,26 +710,6 @@ class confirmation_height_store virtual void for_each_par (std::function, nano::store_iterator)> const &) const = 0; }; -/** - * Manages unchecked storage and iteration - */ -class unchecked_store -{ -public: - using iterator = nano::store_iterator; - - virtual void clear (nano::write_transaction const &) = 0; - virtual void put (nano::write_transaction const &, nano::hash_or_account const & dependency, nano::unchecked_info const &) = 0; - std::pair equal_range (nano::transaction const & transaction, nano::block_hash const & dependency); - std::pair full_range (nano::transaction const & transaction); - virtual bool exists (nano::transaction const & transaction_a, nano::unchecked_key const & unchecked_key_a) = 0; - virtual void del (nano::write_transaction const &, nano::unchecked_key const &) = 0; - virtual iterator begin (nano::transaction const &) const = 0; - virtual iterator lower_bound (nano::transaction const &, nano::unchecked_key const &) const = 0; - virtual iterator end () const = 0; - virtual size_t count (nano::transaction const &) = 0; -}; - /** * Manages final vote storage and iteration */ @@ -821,7 +766,6 @@ class block_store virtual uint64_t account_height (nano::transaction const & transaction_a, nano::block_hash const & hash_a) const = 0; }; -class unchecked_map; /** * Store manager */ @@ -836,7 +780,6 @@ class store nano::frontier_store &, nano::account_store &, nano::pending_store &, - nano::unchecked_store &, nano::online_weight_store &, nano::pruned_store &, nano::peer_store &, @@ -861,9 +804,6 @@ class store static int constexpr version_minimum{ 14 }; static int constexpr version_current{ 21 }; -private: - unchecked_store & unchecked; - public: online_weight_store & online_weight; pruned_store & pruned; @@ -890,8 +830,6 @@ class store virtual nano::read_transaction tx_begin_read () const = 0; virtual std::string vendor_get () const = 0; - - friend class unchecked_map; }; std::unique_ptr make_store (nano::logger_mt & logger, boost::filesystem::path const & path, nano::ledger_constants & constants, bool open_read_only = false, bool add_db_postfix = false, nano::rocksdb_config const & rocksdb_config = nano::rocksdb_config{}, nano::txn_tracking_config const & txn_tracking_config_a = nano::txn_tracking_config{}, std::chrono::milliseconds block_processor_batch_max_time_a = std::chrono::milliseconds (5000), nano::lmdb_config const & lmdb_config_a = nano::lmdb_config{}, bool backup_before_upgrade = false); From d8a2f4e9cefe9cc4cafb0486e2767c72220bb8db Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Wed, 19 Apr 2023 14:01:37 -0300 Subject: [PATCH 4/5] Update and reactivate the rocksdb tombstone_count test The same test can be performed using a different RocksDB table. --- nano/core_test/block_store.cpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/nano/core_test/block_store.cpp b/nano/core_test/block_store.cpp index 5aee62d2cb..71132bbd09 100644 --- a/nano/core_test/block_store.cpp +++ b/nano/core_test/block_store.cpp @@ -2436,14 +2436,13 @@ namespace nano { // This thest ensures the tombstone_count is increased when there is a delete. The tombstone_count is part of a flush // logic bound to the way RocksDB is used by the node. -/* The unchecked table was deprecated and it is being removed, rewrite this test using another table TEST (rocksdb_block_store, tombstone_count) { if (!nano::rocksdb_config::using_rocksdb_in_tests ()) { GTEST_SKIP (); } - nano::test::system system{}; + nano::test::system system; nano::logger_mt logger; auto store = std::make_unique (logger, nano::unique_path (), nano::dev::constants); ASSERT_TRUE (!store->init_error ()); @@ -2457,20 +2456,18 @@ TEST (rocksdb_block_store, tombstone_count) .work (5) .build_shared (); // Enqueues a block to be saved in the database - auto previous = block->previous (); - store->unchecked.put (store->tx_begin_write (), previous, nano::unchecked_info (block)); - nano::unchecked_key key{ previous, block->hash () }; + nano::account account{ 1 }; + store->account.put (store->tx_begin_write (), account, nano::account_info{}); auto check_block_is_listed = [&] (nano::transaction const & transaction_a) { - return store->unchecked.exists (transaction_a, key); + return store->account.exists (transaction_a, account); }; // Waits for the block to get saved ASSERT_TIMELY (5s, check_block_is_listed (store->tx_begin_read ())); - ASSERT_EQ (store->tombstone_map.at (nano::tables::unchecked).num_since_last_flush.load (), 0); - // Perorms a delete and checks for the tombstone counter - store->unchecked.del (store->tx_begin_write (), nano::unchecked_key (previous, block->hash ())); - ASSERT_TIMELY (5s, store->tombstone_map.at (nano::tables::unchecked).num_since_last_flush.load () == 1); + ASSERT_EQ (store->tombstone_map.at (nano::tables::accounts).num_since_last_flush.load (), 0); + // Performs a delete operation and checks for the tombstone counter + store->account.del (store->tx_begin_write (), account); + ASSERT_TIMELY (5s, store->tombstone_map.at (nano::tables::accounts).num_since_last_flush.load () == 1); } - */ } namespace nano From f3b9eb91235f2b36c7bf656cfad9bd5b7f96ec0d Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Wed, 19 Apr 2023 14:22:09 -0300 Subject: [PATCH 5/5] Remove a deprecated test for the removed unchecked table --- nano/core_test/block_store.cpp | 50 ---------------------------------- 1 file changed, 50 deletions(-) diff --git a/nano/core_test/block_store.cpp b/nano/core_test/block_store.cpp index 71132bbd09..2f3ca5ca32 100644 --- a/nano/core_test/block_store.cpp +++ b/nano/core_test/block_store.cpp @@ -959,56 +959,6 @@ TEST (block_store, pruned_random) ASSERT_EQ (hash1, random_hash); } -/* This test has no code to test. -namespace nano -{ -namespace lmdb -{ - // Databases need to be dropped in order to convert to dupsort compatible - TEST (block_store, DISABLED_change_dupsort) // Unchecked is no longer dupsort table - { - nano::test::system system{}; - auto path (nano::unique_path ()); - nano::logger_mt logger{}; - nano::lmdb::store store{ logger, path, nano::dev::constants }; - nano::unchecked_map unchecked{ store, system.stats, false }; - auto transaction (store.tx_begin_write ()); - ASSERT_EQ (0, mdb_drop (store.env.tx (transaction), store.unchecked_store.unchecked_handle, 1)); - ASSERT_EQ (0, mdb_dbi_open (store.env.tx (transaction), "unchecked", MDB_CREATE, &store.unchecked_store.unchecked_handle)); - nano::block_builder builder; - auto send1 = builder - .send () - .previous (0) - .destination (0) - .balance (0) - .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) - .work (0) - .build_shared (); - auto send2 = builder - .send () - .previous (1) - .destination (0) - .balance (0) - .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub) - .work (0) - .build_shared (); - ASSERT_NE (send1->hash (), send2->hash ()); - unchecked.put (send1->hash (), nano::unchecked_info (send1)); - unchecked.put (send1->hash (), nano::unchecked_info (send2)); - ASSERT_EQ (0, mdb_drop (store.env.tx (transaction), store.unchecked_store.unchecked_handle, 0)); - mdb_dbi_close (store.env, store.unchecked_store.unchecked_handle); - ASSERT_EQ (0, mdb_dbi_open (store.env.tx (transaction), "unchecked", MDB_CREATE | MDB_DUPSORT, &store.unchecked_store.unchecked_handle)); - unchecked.put (send1->hash (), nano::unchecked_info (send1)); - unchecked.put (send1->hash (), nano::unchecked_info (send2)); - ASSERT_EQ (0, mdb_drop (store.env.tx (transaction), store.unchecked_store.unchecked_handle, 1)); - ASSERT_EQ (0, mdb_dbi_open (store.env.tx (transaction), "unchecked", MDB_CREATE | MDB_DUPSORT, &store.unchecked_store.unchecked_handle)); - unchecked.put (send1->hash (), nano::unchecked_info (send1)); - unchecked.put (send1->hash (), nano::unchecked_info (send2)); - } -} -} - */ - TEST (block_store, state_block) { nano::logger_mt logger;