From 0dd6ee1318f01b37bd3ab765458654167f92e102 Mon Sep 17 00:00:00 2001 From: clemahieu Date: Tue, 25 Feb 2020 10:02:04 +0000 Subject: [PATCH] Remove dropped_election_cache in preparation for election refactor. (#2590) * Remove dropped_election_cache in preparation for election refactor. * Removing unused parameter. --- nano/core_test/active_transactions.cpp | 68 ------------------------- nano/node/active_transactions.cpp | 69 +------------------------- nano/node/active_transactions.hpp | 13 +---- nano/node/blockprocessor.cpp | 2 +- 4 files changed, 3 insertions(+), 149 deletions(-) diff --git a/nano/core_test/active_transactions.cpp b/nano/core_test/active_transactions.cpp index 64eca0e7..ffa3c493 100644 --- a/nano/core_test/active_transactions.cpp +++ b/nano/core_test/active_transactions.cpp @@ -254,7 +254,6 @@ TEST (active_transactions, keep_local) { ASSERT_NO_ERROR (system.poll ()); } - ASSERT_EQ (0, node.active.dropped_elections_cache_size ()); while (!node.active.empty ()) { nano::lock_guard active_guard (node.active.mutex); @@ -276,7 +275,6 @@ TEST (active_transactions, keep_local) { ASSERT_NO_ERROR (system.poll ()); } - ASSERT_EQ (1, node.active.dropped_elections_cache_size ()); } TEST (active_transactions, prioritize_chains) @@ -571,72 +569,6 @@ TEST (active_transactions, update_difficulty) } } -TEST (active_transactions, restart_dropped) -{ - nano::system system; - nano::node_config node_config (nano::get_available_port (), system.logging); - node_config.enable_voting = false; - node_config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled; - auto & node = *system.add_node (node_config); - nano::genesis genesis; - auto send1 (std::make_shared (nano::test_genesis_key.pub, genesis.hash (), nano::test_genesis_key.pub, nano::genesis_amount - nano::xrb_ratio, nano::test_genesis_key.pub, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (genesis.hash ()))); - // Process only in ledger and emulate dropping the election - ASSERT_EQ (nano::process_result::progress, node.process (*send1).code); - { - nano::lock_guard guard (node.active.mutex); - node.active.add_dropped_elections_cache (send1->qualified_root ()); - } - uint64_t difficulty1 (0); - nano::work_validate (*send1, &difficulty1); - // Generate higher difficulty work - auto work2 (*system.work.generate (send1->root (), difficulty1)); - uint64_t difficulty2 (0); - nano::work_validate (send1->root (), work2, &difficulty2); - ASSERT_GT (difficulty2, difficulty1); - // Process the same block with updated work - auto send2 (std::make_shared (*send1)); - send2->block_work_set (work2); - node.process_active (send2); - // Wait until the block is in elections - system.deadline_set (5s); - bool done{ false }; - while (!done) - { - { - nano::lock_guard guard (node.active.mutex); - auto existing (node.active.roots.find (send2->qualified_root ())); - done = existing != node.active.roots.end (); - if (done) - { - ASSERT_EQ (difficulty2, existing->difficulty); - } - } - ASSERT_NO_ERROR (system.poll ()); - } - std::shared_ptr block; - while (block == nullptr) - { - ASSERT_NO_ERROR (system.poll ()); - block = node.store.block_get (node.store.tx_begin_read (), send1->hash ()); - } - ASSERT_EQ (work2, block->block_work ()); - // Drop election - node.active.erase (*send2); - // Try to restart election with the lower difficulty block, should not work since the block as lower work - node.process_active (send1); - system.deadline_set (5s); - while (node.block_processor.size () > 0) - { - ASSERT_NO_ERROR (system.poll ()); - } - ASSERT_TRUE (node.active.empty ()); - // Verify the block was not updated in the ledger - { - auto block (node.store.block_get (node.store.tx_begin_read (), send1->hash ())); - ASSERT_EQ (work2, block->block_work ()); - } -} - TEST (active_transactions, vote_replays) { nano::system system; diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index 57a0c257..2ca075d9 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -323,7 +323,6 @@ void nano::active_transactions::request_confirm (nano::unique_lock & { election_l->stop (); inactive_l.insert (root_l); - add_dropped_elections_cache (root_l); } // Attempt obtaining votes else if (election_l->skip_delay || election_l->election_start < cutoff_l) @@ -672,7 +671,7 @@ bool nano::active_transactions::active (nano::block const & block_a) return active (block_a.qualified_root ()); } -void nano::active_transactions::update_difficulty (std::shared_ptr block_a, boost::optional opt_transaction_a) +void nano::active_transactions::update_difficulty (std::shared_ptr block_a) { nano::unique_lock lock (mutex); auto existing_election (roots.get ().find (block_a->qualified_root ())); @@ -695,41 +694,6 @@ void nano::active_transactions::update_difficulty (std::shared_ptr adjust_difficulty (block_a->hash ()); } } - else if (opt_transaction_a.is_initialized ()) - { - // Only guaranteed to immediately restart the election if the new block is received within 60s of dropping it - constexpr std::chrono::seconds recently_dropped_cutoff{ 60s }; - if (find_dropped_elections_cache (block_a->qualified_root ()) > std::chrono::steady_clock::now () - recently_dropped_cutoff) - { - lock.unlock (); - nano::block_sideband existing_sideband; - auto hash (block_a->hash ()); - auto existing_block (node.store.block_get (*opt_transaction_a, hash, &existing_sideband)); - release_assert (existing_block != nullptr); - nano::confirmation_height_info confirmation_height_info; - release_assert (!node.store.confirmation_height_get (*opt_transaction_a, node.store.block_account (*opt_transaction_a, hash), confirmation_height_info)); - bool confirmed = (confirmation_height_info.height >= existing_sideband.height); - if (!confirmed && existing_block->block_work () != block_a->block_work ()) - { - uint64_t existing_difficulty; - uint64_t new_difficulty; - if (!nano::work_validate (nano::work_version::work_1, *block_a, &new_difficulty) && !nano::work_validate (nano::work_version::work_1, *existing_block, &existing_difficulty)) - { - if (new_difficulty > existing_difficulty) - { - // Re-writing the block is necessary to avoid the same work being received later to force restarting the election - // The existing block is re-written, not the arriving block, as that one might not have gone through a full signature check - existing_block->block_work_set (block_a->block_work ()); - node.store.block_put (*opt_transaction_a, hash, *existing_block, existing_sideband); - - // Restart election for the upgraded block, previously dropped from elections - lock.lock (); - insert_impl (existing_block); - } - } - } - } - } } void nano::active_transactions::adjust_difficulty (nano::block_hash const & hash_a) @@ -1108,36 +1072,6 @@ bool nano::active_transactions::inactive_votes_bootstrap_check (std::vector guard (mutex); - return dropped_elections_cache.size (); -} - -void nano::active_transactions::add_dropped_elections_cache (nano::qualified_root const & root_a) -{ - debug_assert (!mutex.try_lock ()); - dropped_elections_cache.get ().emplace_back (nano::election_timepoint{ std::chrono::steady_clock::now (), root_a }); - if (dropped_elections_cache.size () > dropped_elections_cache_max) - { - dropped_elections_cache.get ().pop_front (); - } -} - -std::chrono::steady_clock::time_point nano::active_transactions::find_dropped_elections_cache (nano::qualified_root const & root_a) -{ - debug_assert (!mutex.try_lock ()); - auto existing (dropped_elections_cache.get ().find (root_a)); - if (existing != dropped_elections_cache.get ().end ()) - { - return existing->time; - } - else - { - return std::chrono::steady_clock::time_point{}; - } -} - size_t nano::active_transactions::election_winner_details_size () { nano::lock_guard guard (mutex); @@ -1170,6 +1104,5 @@ std::unique_ptr nano::collect_container_info (ac composite->add_component (std::make_unique (container_info{ "priority_wallet_cementable_frontiers_count", active_transactions.priority_wallet_cementable_frontiers_size (), sizeof (nano::cementable_account) })); composite->add_component (std::make_unique (container_info{ "priority_cementable_frontiers_count", active_transactions.priority_cementable_frontiers_size (), sizeof (nano::cementable_account) })); composite->add_component (std::make_unique (container_info{ "inactive_votes_cache_count", active_transactions.inactive_votes_cache_size (), sizeof (nano::gap_information) })); - composite->add_component (std::make_unique (container_info{ "dropped_elections_count", active_transactions.dropped_elections_cache_size (), sizeof (nano::election_timepoint) })); return composite; } diff --git a/nano/node/active_transactions.hpp b/nano/node/active_transactions.hpp index 0ac75f1d..ac99181d 100644 --- a/nano/node/active_transactions.hpp +++ b/nano/node/active_transactions.hpp @@ -97,7 +97,7 @@ public: // Is the root of this block in the roots container bool active (nano::block const &); bool active (nano::qualified_root const &); - void update_difficulty (std::shared_ptr, boost::optional = boost::none); + void update_difficulty (std::shared_ptr); void adjust_difficulty (nano::block_hash const &); void update_active_difficulty (nano::unique_lock &); uint64_t active_difficulty (); @@ -139,9 +139,6 @@ public: size_t inactive_votes_cache_size (); std::unordered_map> election_winner_details; size_t election_winner_details_size (); - void add_dropped_elections_cache (nano::qualified_root const &); - std::chrono::steady_clock::time_point find_dropped_elections_cache (nano::qualified_root const &); - size_t dropped_elections_cache_size (); nano::confirmation_solicitor solicitor; private: @@ -207,14 +204,6 @@ private: // clang-format on static size_t constexpr inactive_votes_cache_max{ 16 * 1024 }; bool inactive_votes_bootstrap_check (std::vector const &, nano::block_hash const &, bool &); - // clang-format off - boost::multi_index_container>, - mi::hashed_unique, - mi::member>>> - dropped_elections_cache; - // clang-format on static size_t constexpr dropped_elections_cache_max{ 32 * 1024 }; boost::thread thread; diff --git a/nano/node/blockprocessor.cpp b/nano/node/blockprocessor.cpp index 37fe2a11..54031933 100644 --- a/nano/node/blockprocessor.cpp +++ b/nano/node/blockprocessor.cpp @@ -466,7 +466,7 @@ nano::process_return nano::block_processor::process_one (nano::write_transaction node.logger.try_log (boost::str (boost::format ("Old for: %1%") % hash.to_string ())); } queue_unchecked (transaction_a, hash); - node.active.update_difficulty (info_a.block, transaction_a); + node.active.update_difficulty (info_a.block); node.stats.inc (nano::stat::type::ledger, nano::stat::detail::old); break; }