Improve readability for vote handling in active_transactions

nano/node/active_transactions.cpp

@@ -448,68 +448,90 @@ nano::election_insertion_result nano::active_transactions::insert_impl (nano::un
 // Validate a vote and apply it to the current election if one exists
 nano::vote_code nano::active_transactions::vote (std::shared_ptr<nano::vote> const & vote_a)
 {
-	nano::vote_code result{ nano::vote_code::indeterminate };
-	// If all hashes were recently confirmed then it is a replay
-	unsigned recently_confirmed_counter (0);
+	std::vector<std::pair<std::shared_ptr<nano::election>, nano::block_hash>> active_elections_hashes;
+	std::vector<nano::block_hash> inactive_hashes;
 
-	std::vector<std::pair<std::shared_ptr<nano::election>, nano::block_hash>> process;
-	std::vector<nano::block_hash> inactive; // Hashes that should be added to inactive vote cache
+	categorize_hashes (vote_a, active_elections_hashes, inactive_hashes);
+	handle_inactive_votes (inactive_hashes, vote_a);
+	auto result = handle_active_votes (active_elections_hashes, vote_a);
 
+	return result;
+}
+
+void nano::active_transactions::categorize_hashes (std::shared_ptr<nano::vote> const & vote_a, std::vector<std::pair<std::shared_ptr<nano::election>, nano::block_hash>> & active_elections_hashes, std::vector<nano::block_hash> & inactive_hashes)
+{
+	nano::unique_lock<nano::mutex> lock{ mutex };
+	for (auto const & hash : vote_a->hashes)
 	{
-		nano::unique_lock<nano::mutex> lock{ mutex };
-		for (auto const & hash : vote_a->hashes)
+		auto existing = blocks.find (hash);
+		if (existing != blocks.end ())
 		{
-			auto existing (blocks.find (hash));
-			if (existing != blocks.end ())
-			{
-				process.emplace_back (existing->second, hash);
-			}
-			else if (!recently_confirmed.exists (hash))
-			{
-				inactive.emplace_back (hash);
-			}
-			else
-			{
-				++recently_confirmed_counter;
-			}
+			active_elections_hashes.emplace_back (existing->second, hash);
+		}
+		else if (!recently_confirmed.exists (hash))
+		{
+			inactive_hashes.emplace_back (hash);
 		}
 	}
+}
 
-	// Process inactive votes outside of the critical section
-	for (auto & hash : inactive)
+void nano::active_transactions::handle_inactive_votes (std::vector<nano::block_hash> & inactive_hashes, std::shared_ptr<nano::vote> const & vote_a)
+{
+	for (auto & hash : inactive_hashes)
 	{
 		add_inactive_vote_cache (hash, vote_a);
 	}
+}
 
-	if (!process.empty ())
+nano::vote_code nano::active_transactions::handle_active_votes (std::vector<std::pair<std::shared_ptr<nano::election>, nano::block_hash>> & active_elections_hashes, std::shared_ptr<nano::vote> const & vote_a)
+{
+	auto result = nano::vote_code::indeterminate;
+
+	if (!active_elections_hashes.empty ())
 	{
-		bool replay (false);
-		bool processed (false);
-		for (auto const & [election, block_hash] : process)
+		bool replay = false;
+		bool processed = false;
+		for (auto const & [election, block_hash] : active_elections_hashes)
 		{
-			auto const result_l = election->vote (vote_a->account, vote_a->timestamp (), block_hash);
-			processed = processed || result_l.processed;
-			replay = replay || result_l.replay;
+			auto const result = election->vote (vote_a->account, vote_a->timestamp (), block_hash);
+			processed = processed || result.processed;
+			replay = replay || result.replay;
 		}
 
-		// Republish vote if it is new and the node does not host a principal representative (or close to)
 		if (processed)
-		{
-			auto const reps (node.wallets.reps ());
-			if (!reps.have_half_rep () && !reps.exists (vote_a->account))
-			{
-				node.network.flood_vote (vote_a, 0.5f);
-			}
-		}
+			republish_vote_if_needed (vote_a);
+
 		result = replay ? nano::vote_code::replay : nano::vote_code::vote;
 	}
-	else if (recently_confirmed_counter == vote_a->hashes.size ())
+
+	if (result == nano::vote_code::indeterminate && no_new_votes_present (vote_a))
 	{
 		result = nano::vote_code::replay;
 	}
 	return result;
 }
 
+void nano::active_transactions::republish_vote_if_needed (std::shared_ptr<nano::vote> const & vote_a)
+{
+	auto const reps = node.wallets.reps ();
+	if (!reps.have_half_rep () && !reps.exists (vote_a->account))
+	{
+		node.network.flood_vote (vote_a, 0.5f);
+	}
+}
+
+bool nano::active_transactions::no_new_votes_present (std::shared_ptr<nano::vote> const & vote_a)
+{
+	for (const auto & hash : vote_a->hashes)
+	{
+		if (!recently_confirmed.exists (hash))
+		{
+			return false; // At least one hash is not recently confirmed, so it's not a replay.
+		}
+	}
+	return true; // All hashes have been recently confirmed, it's a replay.
+}
+
 bool nano::active_transactions::active (nano::qualified_root const & root_a) const
 {
 	nano::lock_guard<nano::mutex> lock{ mutex };

nano/node/active_transactions.hpp

@@ -197,6 +197,11 @@ class active_transactions final
 	 * TODO: Should be moved to `vote_cache` class
 	 */
 	void add_inactive_vote_cache (nano::block_hash const & hash, std::shared_ptr<nano::vote> vote);
+	void categorize_hashes (std::shared_ptr<nano::vote> const &, std::vector<std::pair<std::shared_ptr<nano::election>, nano::block_hash>> &, std::vector<nano::block_hash> &);
+	void handle_inactive_votes (std::vector<nano::block_hash> &, std::shared_ptr<nano::vote> const &);
+	nano::vote_code handle_active_votes (std::vector<std::pair<std::shared_ptr<nano::election>, nano::block_hash>> & process, std::shared_ptr<nano::vote> const & vote_a);
+	void republish_vote_if_needed (std::shared_ptr<nano::vote> const &);
+	bool no_new_votes_present (std::shared_ptr<nano::vote> const &);
 
 private: // Dependencies
 	nano::confirmation_height_processor & confirmation_height_processor;