From 053f5d90b3c961ed8a95df482a69d35dde4b2206 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20W=C3=B3jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Sat, 17 May 2025 09:46:05 +0200 Subject: [PATCH] Merge pull request #4905 from pwojcikdev/fix-elections-sideband Fix missing sideband in election blocks --- nano/lib/thread_roles.cpp | 3 +++ nano/lib/thread_roles.hpp | 1 + nano/node/active_elections.cpp | 41 ++++++++++++++++++++++------------ nano/node/active_elections.hpp | 3 +++ nano/node/election.cpp | 9 ++------ nano/node/election.hpp | 2 +- 6 files changed, 37 insertions(+), 22 deletions(-) diff --git a/nano/lib/thread_roles.cpp b/nano/lib/thread_roles.cpp index af3bf9452..0f4d0f976 100644 --- a/nano/lib/thread_roles.cpp +++ b/nano/lib/thread_roles.cpp @@ -49,6 +49,9 @@ std::string nano::thread_role::get_string (nano::thread_role::name role) case nano::thread_role::name::aec_loop: thread_role_name_string = "AEC"; break; + case nano::thread_role::name::aec_notifications: + thread_role_name_string = "AEC notif"; + break; case nano::thread_role::name::wallet_actions: thread_role_name_string = "Wallet actions"; break; diff --git a/nano/lib/thread_roles.hpp b/nano/lib/thread_roles.hpp index 83c842d66..37e4d50a3 100644 --- a/nano/lib/thread_roles.hpp +++ b/nano/lib/thread_roles.hpp @@ -21,6 +21,7 @@ enum class name block_processing, ledger_notifications, aec_loop, + aec_notifications, wallet_actions, bootstrap_initiator, bootstrap_connections, diff --git a/nano/node/active_elections.cpp b/nano/node/active_elections.cpp index 1733bca16..d5282451a 100644 --- a/nano/node/active_elections.cpp +++ b/nano/node/active_elections.cpp @@ -29,7 +29,8 @@ nano::active_elections::active_elections (nano::node & node_a, nano::ledger_noti ledger_notifications{ ledger_notifications_a }, cementing_set{ cementing_set_a }, recently_confirmed{ config.confirmation_cache }, - recently_cemented{ config.confirmation_history_size } + recently_cemented{ config.confirmation_history_size }, + workers{ 1, nano::thread_role::name::aec_notifications } { count_by_behavior.fill (0); // Zero initialize array @@ -45,15 +46,16 @@ nano::active_elections::active_elections (nano::node & node_a, nano::ledger_noti results.push_back (result); } } - { - // TODO: This could be offloaded to a separate notification worker, profiling is needed + + // Notify observers about cemented blocks on a background thread + workers.post ([this, results = std::move (results)] () { auto transaction = node.ledger.tx_begin_read (); for (auto const & [status, votes] : results) { transaction.refresh_if_needed (); notify_observers (transaction, status, votes); } - } + }); }); // Notify elections about alternative (forked) blocks @@ -87,6 +89,8 @@ nano::active_elections::~active_elections () void nano::active_elections::start () { + workers.start (); + if (node.flags.disable_request_loop) { return; @@ -107,7 +111,12 @@ void nano::active_elections::stop () stopped = true; } condition.notify_all (); - nano::join_or_pass (thread); + if (thread.joinable ()) + { + thread.join (); + } + workers.stop (); + clear (); } @@ -182,8 +191,10 @@ auto nano::active_elections::block_cemented (std::shared_ptr const void nano::active_elections::notify_observers (nano::secure::transaction const & transaction, nano::election_status const & status, std::vector const & votes) const { - auto block = status.winner; - auto account = block->account (); + // Get block from ledger to ensure sideband is set (forked blocks may not have sideband) + auto const block = node.ledger.any.block_get (transaction, status.winner->hash ()); + release_assert (block != nullptr); // Block must exist in the ledger since it was cemented + auto const account = block->account (); switch (status.type) { @@ -209,6 +220,7 @@ void nano::active_elections::notify_observers (nano::secure::transaction const & } node.observers.account_balance.notify (account, false); + if (block->is_send ()) { node.observers.account_balance.notify (block->destination (), true); @@ -303,7 +315,7 @@ void nano::active_elections::tick_elections (nano::unique_lock & lo for (auto const & election : stale_elections) { node.logger.debug (nano::log::type::active_elections, "Bootstrapping account: {} with stale election with root: {}, blocks: {} (behavior: {}, state: {}, voters: {}, blocks: {}, duration: {}ms)", - election->account (), + election->account, election->qualified_root, fmt::join (election->blocks_hashes (), ", "), // TODO: Lazy eval to_string (election->behavior ()), @@ -312,7 +324,7 @@ void nano::active_elections::tick_elections (nano::unique_lock & lo election->block_count (), election->duration ().count ()); - node.bootstrap.prioritize (election->account ()); + node.bootstrap.prioritize (election->account); } } } @@ -324,7 +336,7 @@ void nano::active_elections::cleanup_election (nano::unique_lock & debug_assert (!election->confirmed () || recently_confirmed.exists (election->qualified_root)); // Keep track of election count by election type - debug_assert (count_by_behavior[election->behavior ()] > 0); + release_assert (count_by_behavior[election->behavior ()] > 0); count_by_behavior[election->behavior ()]--; auto blocks_l = election->blocks (); @@ -403,8 +415,8 @@ std::vector> nano::active_elections::list_active nano::election_insertion_result nano::active_elections::insert (std::shared_ptr const & block_a, nano::election_behavior election_behavior_a, erased_callback_t erased_callback_a) { - debug_assert (block_a); - debug_assert (block_a->has_sideband ()); + release_assert (block_a); + release_assert (block_a->has_sideband ()); nano::unique_lock lock{ mutex }; @@ -435,7 +447,7 @@ nano::election_insertion_result nano::active_elections::insert (std::shared_ptr< node.vote_router.connect (hash, result.election); // Keep track of election count by election type - debug_assert (count_by_behavior[result.election->behavior ()] >= 0); + release_assert (count_by_behavior[result.election->behavior ()] >= 0); count_by_behavior[result.election->behavior ()]++; // Skip passive phase for blocks without cached votes to avoid bootstrap delays @@ -495,7 +507,7 @@ nano::election_insertion_result nano::active_elections::insert (std::shared_ptr< if (result.inserted) { - debug_assert (result.election); + release_assert (result.election); // Notifications node.observers.active_started.notify (hash); @@ -646,6 +658,7 @@ nano::container_info nano::active_elections::container_info () const info.add ("recently_confirmed", recently_confirmed.container_info ()); info.add ("recently_cemented", recently_cemented.container_info ()); + info.add ("workers", workers.container_info ()); return info; } diff --git a/nano/node/active_elections.hpp b/nano/node/active_elections.hpp index 7a627275a..8f49010ef 100644 --- a/nano/node/active_elections.hpp +++ b/nano/node/active_elections.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -167,6 +168,8 @@ private: bool stopped{ false }; std::thread thread; + nano::thread_pool workers; + nano::interval bootstrap_stale_interval; friend class election; diff --git a/nano/node/election.cpp b/nano/node/election.cpp index 893b845c3..c59a0c76b 100644 --- a/nano/node/election.cpp +++ b/nano/node/election.cpp @@ -31,7 +31,8 @@ nano::election::election (nano::node & node_a, std::shared_ptr cons status (block_a), height (block_a->sideband ().height), root (block_a->root ()), - qualified_root (block_a->qualified_root ()) + qualified_root (block_a->qualified_root ()), + account (block_a->account ()) { last_votes.emplace (nano::account::null (), nano::vote_info{ std::chrono::steady_clock::now (), 0, block_a->hash () }); last_blocks.emplace (block_a->hash (), block_a); @@ -805,12 +806,6 @@ void nano::election::force_confirm () confirm_once (lock); } -nano::account nano::election::account () const -{ - nano::lock_guard guard{ mutex }; - return status.winner->account (); -} - std::unordered_set nano::election::blocks_hashes () const { nano::lock_guard guard{ mutex }; diff --git a/nano/node/election.hpp b/nano/node/election.hpp index 0db4b7ac7..9f3881cd8 100644 --- a/nano/node/election.hpp +++ b/nano/node/election.hpp @@ -143,8 +143,8 @@ public: // Information uint64_t const height; nano::root const root; nano::qualified_root const qualified_root; + nano::account const account; - nano::account account () const; std::vector votes_with_weight () const; nano::election_behavior behavior () const; nano::election_state state () const;