From ea9411e6afec3283c5fbccd2dc5f60e08f6741b8 Mon Sep 17 00:00:00 2001 From: gr0vity-dev Date: Mon, 5 Feb 2024 15:59:29 +0100 Subject: [PATCH] Fix race between election activation and confirmation_heigh_processor A block that is being processed by the confirmation_heigh_processor should not be inserted into the AEC --- nano/node/active_transactions.cpp | 2 +- nano/node/confirmation_height_processor.cpp | 5 ++++ nano/node/confirmation_height_processor.hpp | 1 + nano/node/confirmation_height_unbounded.cpp | 28 +++++++++++++++++++++ nano/node/confirmation_height_unbounded.hpp | 4 +++ 5 files changed, 39 insertions(+), 1 deletion(-) diff --git a/nano/node/active_transactions.cpp b/nano/node/active_transactions.cpp index a66f69119d..7cc9c59e29 100644 --- a/nano/node/active_transactions.cpp +++ b/nano/node/active_transactions.cpp @@ -428,7 +428,7 @@ nano::election_insertion_result nano::active_transactions::insert (std::shared_p auto const existing = roots.get ().find (root); if (existing == roots.get ().end ()) { - if (!recently_confirmed.exists (root)) + if (!recently_confirmed.exists (root) && !node.confirmation_height_processor.is_processing_or_confirmed (hash)) { result.inserted = true; auto observe_rep_cb = [&node = node] (auto const & rep_a) { diff --git a/nano/node/confirmation_height_processor.cpp b/nano/node/confirmation_height_processor.cpp index 4b9d9d82a6..4da9169ea1 100644 --- a/nano/node/confirmation_height_processor.cpp +++ b/nano/node/confirmation_height_processor.cpp @@ -234,6 +234,11 @@ bool nano::confirmation_height_processor::is_processing_block (nano::block_hash return is_processing_added_block (hash_a) || unbounded_processor.has_iterated_over_block (hash_a); } +bool nano::confirmation_height_processor::is_processing_or_confirmed (nano::block_hash const & hash_a) const +{ + return is_processing_added_block (hash_a) || unbounded_processor.has_iterated_or_confirmed (hash_a); +} + nano::block_hash nano::confirmation_height_processor::current () const { nano::lock_guard lk (mutex); diff --git a/nano/node/confirmation_height_processor.hpp b/nano/node/confirmation_height_processor.hpp index d31267833f..33ec1b9be7 100644 --- a/nano/node/confirmation_height_processor.hpp +++ b/nano/node/confirmation_height_processor.hpp @@ -41,6 +41,7 @@ class confirmation_height_processor final std::size_t awaiting_processing_size () const; bool is_processing_added_block (nano::block_hash const & hash_a) const; bool is_processing_block (nano::block_hash const &) const; + bool is_processing_or_confirmed (nano::block_hash const &) const; nano::block_hash current () const; /* diff --git a/nano/node/confirmation_height_unbounded.cpp b/nano/node/confirmation_height_unbounded.cpp index 8f50f39dcb..b6ef5357e9 100644 --- a/nano/node/confirmation_height_unbounded.cpp +++ b/nano/node/confirmation_height_unbounded.cpp @@ -459,7 +459,19 @@ void nano::confirmation_height_unbounded::clear_process_vars () implicit_receive_cemented_mapping_size = 0; { nano::lock_guard guard (block_cache_mutex); + // Move the hashes from block_cache to recently_confirmed + for (const auto & pair : block_cache) + { + const auto & block_hash = pair.first; + recently_confirmed.push_back (block_hash); + } block_cache.clear (); + + // Ensure recently_confirmed doesn't exceed max_recently_confirmed + while (recently_confirmed.size () > max_recently_confirmed) + { + recently_confirmed.pop_front (); + } } } @@ -469,6 +481,22 @@ bool nano::confirmation_height_unbounded::has_iterated_over_block (nano::block_h return block_cache.count (hash_a) == 1; } +bool nano::confirmation_height_unbounded::is_recently_confirmed (nano::block_hash const & hash_a) const +{ + nano::lock_guard guard (block_cache_mutex); + auto result = std::find (recently_confirmed.begin (), recently_confirmed.end (), hash_a) != recently_confirmed.end (); + return result; +} + +bool nano::confirmation_height_unbounded::has_iterated_or_confirmed (nano::block_hash const & hash_a) const +{ + nano::lock_guard guard (block_cache_mutex); + bool iterated_over = block_cache.count (hash_a) == 1; + bool recently_confirmed_status = std::find (recently_confirmed.begin (), recently_confirmed.end (), hash_a) != recently_confirmed.end (); + // Return true if either condition is met + return iterated_over || recently_confirmed_status; +} + uint64_t nano::confirmation_height_unbounded::block_cache_size () const { nano::lock_guard guard (block_cache_mutex); diff --git a/nano/node/confirmation_height_unbounded.hpp b/nano/node/confirmation_height_unbounded.hpp index ea3e7e52a1..b30587f13e 100644 --- a/nano/node/confirmation_height_unbounded.hpp +++ b/nano/node/confirmation_height_unbounded.hpp @@ -26,6 +26,8 @@ class confirmation_height_unbounded final void process (std::shared_ptr original_block); void cement_blocks (nano::write_guard &); bool has_iterated_over_block (nano::block_hash const &) const; + bool is_recently_confirmed (nano::block_hash const &) const; + bool has_iterated_or_confirmed (nano::block_hash const &) const; private: class confirmed_iterated_pair @@ -74,6 +76,8 @@ class confirmation_height_unbounded final mutable nano::mutex block_cache_mutex; std::unordered_map> block_cache; uint64_t block_cache_size () const; + static constexpr size_t max_recently_confirmed = 65536; + std::deque recently_confirmed; nano::timer timer;