Improve active elections update loop

nano/node/active_elections.cpp

@@ -53,9 +53,18 @@ nano::active_elections::active_elections (nano::node & node_a, nano::ledger_noti
 		// 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)
+			for (auto const & [election, status, votes] : results)
 			{
 				transaction.refresh_if_needed ();
+
+				// Dependent elections are implicitly confirmed when their block is cemented
+				// TODO: Should this be the case? Maybe just cancel the election and issue block cemented notification?
+				if (election)
+				{
+					node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::confirm_dependent);
+					election->try_confirm (status.winner->hash ()); // TODO: This should either confirm or cancel the election
+				}
+
 				notify_observers (transaction, status, votes);
 			}
 		});
@@ -165,27 +174,6 @@ auto nano::active_elections::insert (std::shared_ptr<nano::block> const & block,
 
 			node.vote_router.connect (hash, result.election);
 
-			auto should_activate_immediately = [&] () {
-				// Broadcast votes immediately for priority elections
-				if (behavior == nano::election_behavior::priority)
-				{
-					return true;
-				}
-				// Skip passive phase for blocks without cached votes to avoid bootstrap delays
-				if (!node.vote_cache.contains (hash))
-				{
-					return true;
-				}
-				return false;
-			};
-
-			bool activate_immediately = should_activate_immediately ();
-			if (activate_immediately)
-			{
-				node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::activate_immediately);
-				result.election->transition_active ();
-			}
-
 			node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::started);
 			node.stats.inc (nano::stat::type::active_elections_started, to_stat_detail (behavior));
 
@@ -193,11 +181,10 @@ auto nano::active_elections::insert (std::shared_ptr<nano::block> const & block,
 			nano::log::arg{ "behavior", behavior },
 			nano::log::arg{ "election", result.election });
 
-			node.logger.debug (nano::log::type::active_elections, "Started new election for root: {} with blocks: {} (behavior: {}, active immediately: {})",
+			node.logger.debug (nano::log::type::active_elections, "Started new election for root: {} with blocks: {} (behavior: {})",
 			root,
 			fmt::join (result.election->blocks_hashes (), ", "), // TODO: Lazy eval
-			to_string (behavior),
+			to_string (behavior));
-			activate_immediately);
 		}
 		else
 		{
@@ -231,6 +218,23 @@ auto nano::active_elections::insert (std::shared_ptr<nano::block> const & block,
 	{
 		release_assert (result.election);
 
+		auto should_activate_immediately = [&] () {
+			// Skip passive phase for blocks without cached votes to avoid bootstrap delays
+			if (!node.vote_cache.contains (hash))
+			{
+				return true;
+			}
+			return false;
+		};
+
+		// Transition to active (broadcasting votes, sending confirm reqs) state if needed
+		bool activate_immediately = should_activate_immediately ();
+		if (activate_immediately)
+		{
+			node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::activate_immediately);
+			result.election->transition_active ();
+		}
+
 		// Notifications
 		node.observers.active_started.notify (hash);
 		vacancy_updated.notify ();
@@ -382,12 +386,7 @@ auto nano::active_elections::block_cemented (std::shared_ptr<nano::block> const
 	debug_assert (node.block_confirmed (block->hash ()));
 
 	// Dependent elections are implicitly confirmed when their block is cemented
-	auto dependend_election = election_impl (block->qualified_root ());
+	auto election = election_impl (block->qualified_root ());
-	if (dependend_election)
-	{
-		node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::confirm_dependent);
-		dependend_election->try_confirm (block->hash ()); // TODO: This should either confirm or cancel the election
-	}
 
 	nano::election_status status;
 	std::vector<nano::vote_with_weight_info> votes;
@@ -401,7 +400,7 @@ auto nano::active_elections::block_cemented (std::shared_ptr<nano::block> const
 		votes = source_election->votes_with_weight ();
 		status.type = nano::election_status_type::active_confirmed_quorum;
 	}
-	else if (dependend_election)
+	else if (election)
 	{
 		status.type = nano::election_status_type::active_confirmation_height;
 	}
@@ -420,7 +419,7 @@ auto nano::active_elections::block_cemented (std::shared_ptr<nano::block> const
 	nano::log::arg{ "confirmation_root", confirmation_root },
 	nano::log::arg{ "source_election", source_election });
 
-	return { status, votes };
+	return { election, status, votes };
 }
 
 void nano::active_elections::notify_observers (nano::secure::transaction const & transaction, nano::election_status const & status, std::vector<nano::vote_with_weight_info> const & votes) const
@@ -461,11 +460,31 @@ void nano::active_elections::notify_observers (nano::secure::transaction const &
 	}
 }
 
+bool nano::active_elections::trigger (nano::qualified_root const & root)
+{
+	bool triggered = false;
+	{
+		nano::lock_guard<nano::mutex> guard{ mutex };
+		if (auto election = index.election (root))
+		{
+			triggered = index.trigger (election);
+		}
+	}
+	if (triggered)
+	{
+		condition.notify_all ();
+	}
+	return triggered;
+}
+
 void nano::active_elections::tick_elections (nano::unique_lock<nano::mutex> & lock)
 {
 	debug_assert (lock.owns_lock ());
 
-	auto const election_list = list_active_impl ();
+	auto const now = std::chrono::steady_clock::now ();
+	auto const cutoff = now - node.network_params.network.aec_loop_interval;
+	auto const election_list = index.list (cutoff, now);
+	debug_assert (!election_list.empty ()); // Shouldn't be called if there are no elections to process
 
 	lock.unlock ();
 
@@ -510,26 +529,34 @@ void nano::active_elections::tick_elections (nano::unique_lock<nano::mutex> & lo
 	}
 }
 
+bool nano::active_elections::predicate () const
+{
+	debug_assert (!mutex.try_lock ());
+
+	auto cutoff = std::chrono::steady_clock::now () - node.network_params.network.aec_loop_interval;
+	return index.any (cutoff);
+}
+
 void nano::active_elections::run ()
 {
 	nano::unique_lock<nano::mutex> lock{ mutex };
 	while (!stopped)
 	{
-		auto const stamp = std::chrono::steady_clock::now ();
+		if (predicate ())
-
+		{
 			node.stats.inc (nano::stat::type::active, nano::stat::detail::loop);
 
 			tick_elections (lock);
 			debug_assert (!lock.owns_lock ());
 			lock.lock ();
-
+		}
-		auto const min_sleep = node.network_params.network.aec_loop_interval / 2;
+		else
-		auto const wakeup = std::max (stamp + node.network_params.network.aec_loop_interval, std::chrono::steady_clock::now () + min_sleep);
+		{
-
+			condition.wait_for (lock, node.network_params.network.aec_loop_interval / 2, [this] {
-		condition.wait_until (lock, wakeup, [this, wakeup] {
+				return stopped || predicate ();
-			return stopped || std::chrono::steady_clock::now () >= wakeup;
 			});
 		}
+	}
 }
 
 int64_t nano::active_elections::limit (nano::election_behavior behavior) const

nano/node/active_elections.hpp

@@ -80,6 +80,9 @@ public:
 	// Notify this container about a new block (potential fork)
 	bool publish (std::shared_ptr<nano::block> const &);
 
+	// Trigger an immediate election update (e.g. after it is confirmed)
+	bool trigger (nano::qualified_root const &);
+
 	/// Is the root of this block in the roots container
 	bool active (nano::block const &) const;
 	bool active (nano::qualified_root const &) const;
@@ -111,13 +114,20 @@ public: // Events
 	nano::observer_set<> vacancy_updated;
 
 private:
+	bool predicate () const;
 	void run ();
 	void tick_elections (nano::unique_lock<nano::mutex> &);
 
 	// Erase all blocks from active and, if not confirmed, clear digests from network filters
 	void erase_election (nano::unique_lock<nano::mutex> & lock_a, std::shared_ptr<nano::election>);
 
-	using block_cemented_result = std::pair<nano::election_status, std::vector<nano::vote_with_weight_info>>;
+	struct block_cemented_result
+	{
+		std::shared_ptr<nano::election> election;
+		nano::election_status status;
+		std::vector<nano::vote_with_weight_info> votes;
+	};
+
 	block_cemented_result block_cemented (std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root, std::shared_ptr<nano::election> const & source_election);
 	void notify_observers (nano::secure::transaction const &, nano::election_status const & status, std::vector<nano::vote_with_weight_info> const & votes) const;
 
@@ -152,22 +162,8 @@ private:
 	nano::interval bootstrap_stale_interval;
 	nano::interval warning_interval;
 
-	friend class election;
-
 public: // Tests
 	void clear ();
-
-	friend class node_fork_storm_Test;
-	friend class system_block_sequence_Test;
-	friend class node_mass_block_new_Test;
-	friend class active_elections_vote_replays_Test;
-	friend class frontiers_confirmation_prioritize_frontiers_Test;
-	friend class frontiers_confirmation_prioritize_frontiers_max_optimistic_elections_Test;
-	friend class confirmation_height_prioritize_frontiers_overwrite_Test;
-	friend class active_elections_confirmation_consistency_Test;
-	friend class node_deferred_dependent_elections_Test;
-	friend class active_elections_pessimistic_elections_Test;
-	friend class frontiers_confirmation_expired_optimistic_elections_removal_Test;
 };
 
 nano::stat::type to_stat_type (nano::election_state);

nano/node/election.cpp

@@ -78,6 +78,8 @@ void nano::election::confirm_once (nano::unique_lock<nano::mutex> & lock)
 
 		lock.unlock ();
 
+		node.active.trigger (qualified_root);
+
 		node.election_workers.post ([status_l, confirmation_action_l = confirmation_action] () {
 			if (confirmation_action_l)
 			{