Move vote generator calls into election code (#2688)

* Generate votes for:
    - started election (if prioritized)
    - changed winner
    - election prioritization
* Move votes generator from block processor to active transactions

nano/core_test/active_transactions.cpp

@@ -882,19 +882,19 @@ TEST (active_transactions, insertion_prioritization)
 		node.active.update_active_difficulty (lock);
 	};
 
-	ASSERT_TRUE (node.active.insert (blocks[2]).prioritized);
+	ASSERT_TRUE (node.active.insert (blocks[2]).election->prioritized ());
 	update_active_difficulty ();
-	ASSERT_FALSE (node.active.insert (blocks[3]).prioritized);
+	ASSERT_FALSE (node.active.insert (blocks[3]).election->prioritized ());
 	update_active_difficulty ();
-	ASSERT_TRUE (node.active.insert (blocks[1]).prioritized);
+	ASSERT_TRUE (node.active.insert (blocks[1]).election->prioritized ());
 	update_active_difficulty ();
-	ASSERT_FALSE (node.active.insert (blocks[4]).prioritized);
+	ASSERT_FALSE (node.active.insert (blocks[4]).election->prioritized ());
 	update_active_difficulty ();
-	ASSERT_TRUE (node.active.insert (blocks[0]).prioritized);
+	ASSERT_TRUE (node.active.insert (blocks[0]).election->prioritized ());
 	update_active_difficulty ();
-	ASSERT_FALSE (node.active.insert (blocks[5]).prioritized);
+	ASSERT_FALSE (node.active.insert (blocks[5]).election->prioritized ());
 	update_active_difficulty ();
-	ASSERT_FALSE (node.active.insert (blocks[6]).prioritized);
+	ASSERT_FALSE (node.active.insert (blocks[6]).election->prioritized ());
 }
 
 TEST (active_difficulty, less_than_one)

nano/core_test/confirmation_solicitor.cpp

@@ -33,12 +33,12 @@ TEST (confirmation_solicitor, batches)
 		nano::lock_guard<std::mutex> guard (node2.active.mutex);
 		for (size_t i (0); i < nano::network::confirm_req_hashes_max; ++i)
 		{
-			auto election (std::make_shared<nano::election> (node2, send, nullptr));
+			auto election (std::make_shared<nano::election> (node2, send, nullptr, false));
 			ASSERT_FALSE (solicitor.add (*election));
 		}
 		ASSERT_EQ (1, solicitor.max_confirm_req_batches);
 		// Reached the maximum amount of requests for the channel
-		auto election (std::make_shared<nano::election> (node2, send, nullptr));
+		auto election (std::make_shared<nano::election> (node2, send, nullptr, false));
 		ASSERT_TRUE (solicitor.add (*election));
 		// Broadcasting should be immediate
 		ASSERT_EQ (0, node2.stats.count (nano::stat::type::message, nano::stat::detail::publish, nano::stat::dir::out));

nano/core_test/node.cpp

@@ -1238,7 +1238,6 @@ TEST (node, fork_publish)
 		// Wait until the genesis rep activated & makes vote
 		while (election->last_votes_size () != 2)
 		{
-			node1.block_processor.generator.add (send1->hash ());
 			node1.vote_processor.flush ();
 			ASSERT_NO_ERROR (system.poll ());
 		}
@@ -1886,7 +1885,6 @@ TEST (node, rep_self_vote)
 	ASSERT_EQ (nano::process_result::progress, node0->process (*block0).code);
 	auto & active (node0->active);
 	auto election1 = active.insert (block0);
-	node0->block_processor.generator.add (block0->hash ());
 	system.deadline_set (1s);
 	// Wait until representatives are activated & make vote
 	while (election1.election->last_votes_size () != 3)
@@ -2905,6 +2903,8 @@ TEST (node, vote_republish)
 	}
 }
 
+namespace nano
+{
 TEST (node, vote_by_hash_bundle)
 {
 	// Keep max_hashes above system to ensure it is kept in scope as votes can be added during system destruction
@@ -2925,7 +2925,7 @@ TEST (node, vote_by_hash_bundle)
 	for (int i = 1; i <= 200; i++)
 	{
 		nano::block_hash hash (i);
-		system.nodes[0]->block_processor.generator.add (hash);
+		system.nodes[0]->active.generator.add (hash);
 	}
 
 	// Verify that bundling occurs. While reaching 12 should be common on most hardware in release mode,
@@ -2936,6 +2936,7 @@ TEST (node, vote_by_hash_bundle)
 		ASSERT_NO_ERROR (system.poll ());
 	}
 }
+}
 
 TEST (node, vote_by_hash_republish)
 {

nano/node/active_transactions.cpp

@@ -16,6 +16,7 @@ using namespace std::chrono;
 
 nano::active_transactions::active_transactions (nano::node & node_a, nano::confirmation_height_processor & confirmation_height_processor_a) :
 confirmation_height_processor (confirmation_height_processor_a),
+generator (node_a.config, node_a.store, node_a.wallets, node_a.vote_processor, node_a.votes_cache, node_a.network),
 node (node_a),
 multipliers_cb (20, 1.),
 trended_active_difficulty (node_a.network_params.network.publish_thresholds.epoch_1),
@@ -72,7 +73,7 @@ void nano::active_transactions::confirm_prioritized_frontiers (nano::transaction
 
 		size_t elections_count (0);
 		nano::unique_lock<std::mutex> lk (mutex);
-		auto start_elections_for_prioritized_frontiers = [&transaction_a, &elections_count, max_elections, &lk, &representative, this](prioritize_num_uncemented & cementable_frontiers) {
+		auto start_elections_for_prioritized_frontiers = [&transaction_a, &elections_count, max_elections, &lk, this](prioritize_num_uncemented & cementable_frontiers) {
 			while (!cementable_frontiers.empty () && !this->stopped && elections_count < max_elections)
 			{
 				auto cementable_account_front_it = cementable_frontiers.get<tag_uncemented> ().begin ();
@@ -97,11 +98,6 @@ void nano::active_transactions::confirm_prioritized_frontiers (nano::transaction
 							{
 								election_insert_result.election->transition_active ();
 								++elections_count;
-								// Calculate votes for local representatives
-								if (election_insert_result.prioritized && representative)
-								{
-									this->node.block_processor.generator.add (info.head);
-								}
 							}
 						}
 					}
@@ -211,8 +207,6 @@ void nano::active_transactions::request_confirm (nano::unique_lock<std::mutex> &
 	nano::confirmation_solicitor solicitor (node.network, node.network_params.network);
 	solicitor.prepare (node.rep_crawler.principal_representatives (std::numeric_limits<size_t>::max ()));
 
-	bool const representative_l (node.config.enable_voting && node.wallets.rep_counts ().voting > 0);
-	std::vector<nano::block_hash> hashes_generation_l;
 	auto & sorted_roots_l (roots.get<tag_difficulty> ());
 	auto const election_ttl_cutoff_l (std::chrono::steady_clock::now () - election_time_to_live);
 	bool const check_all_elections_l (std::chrono::steady_clock::now () - last_check_all_elections > check_all_elections_period);
@@ -232,14 +226,9 @@ void nano::active_transactions::request_confirm (nano::unique_lock<std::mutex> &
 		auto & election_l (i->election);
 		bool const confirmed_l (election_l->confirmed ());
 
-		// Queue vote generation for newly prioritized elections
-		if (!i->prioritized && unconfirmed_count_l < prioritized_cutoff)
+		if (!election_l->prioritized () && unconfirmed_count_l < prioritized_cutoff)
 		{
-			sorted_roots_l.modify (i, [](nano::conflict_info & info_a) { info_a.prioritized = true; });
-			if (representative_l && !confirmed_l)
-			{
-				hashes_generation_l.push_back (election_l->status.winner->hash ());
-			}
+			election_l->prioritize_election ();
 		}
 
 		unconfirmed_count_l += !confirmed_l;
@@ -256,10 +245,6 @@ void nano::active_transactions::request_confirm (nano::unique_lock<std::mutex> &
 	}
 	lock_a.unlock ();
 	solicitor.flush ();
-	for (auto const & hash_l : hashes_generation_l)
-	{
-		node.block_processor.generator.add (hash_l);
-	}
 	lock_a.lock ();
 
 	// This is updated after the loop to ensure slow machines don't do the full check often
@@ -500,6 +485,7 @@ void nano::active_transactions::stop ()
 	{
 		thread.join ();
 	}
+	generator.stop ();
 	lock.lock ();
 	roots.clear ();
 }
@@ -517,10 +503,10 @@ nano::election_insertion_result nano::active_transactions::insert_impl (std::sha
 			{
 				result.inserted = true;
 				auto hash (block_a->hash ());
-				result.election = nano::make_shared<nano::election> (node, block_a, confirmation_action_a);
 				auto difficulty (block_a->difficulty ());
-				result.prioritized = roots.size () < prioritized_cutoff || difficulty > last_prioritized_difficulty.value_or (0);
-				roots.get<tag_root> ().emplace (nano::conflict_info{ root, difficulty, difficulty, result.election, result.prioritized });
+				bool prioritized = roots.size () < prioritized_cutoff || difficulty > last_prioritized_difficulty.value_or (0);
+				result.election = nano::make_shared<nano::election> (node, block_a, confirmation_action_a, prioritized);
+				roots.get<tag_root> ().emplace (nano::conflict_info{ root, difficulty, difficulty, result.election });
 				blocks.emplace (hash, result.election);
 				add_adjust_difficulty (hash);
 				result.election->insert_inactive_votes_cache (hash);
@@ -1089,5 +1075,6 @@ std::unique_ptr<nano::container_info_component> nano::collect_container_info (ac
 	composite->add_component (std::make_unique<container_info_leaf> (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_leaf> (container_info{ "priority_cementable_frontiers_count", active_transactions.priority_cementable_frontiers_size (), sizeof (nano::cementable_account) }));
 	composite->add_component (std::make_unique<container_info_leaf> (container_info{ "inactive_votes_cache_count", active_transactions.inactive_votes_cache_size (), sizeof (nano::gap_information) }));
+	composite->add_component (collect_container_info (active_transactions.generator, "generator"));
 	return composite;
 }

nano/node/active_transactions.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <nano/lib/numbers.hpp>
+#include <nano/node/voting.hpp>
 #include <nano/secure/common.hpp>
 
 #include <boost/circular_buffer.hpp>
@@ -37,7 +38,6 @@ class conflict_info final
 	uint64_t difficulty;
 	uint64_t adjusted_difficulty;
 	std::shared_ptr<nano::election> election;
-	bool prioritized;
 };
 
 class cementable_account final
@@ -70,7 +70,6 @@ class election_insertion_result final
 public:
 	std::shared_ptr<nano::election> election;
 	bool inserted{ false };
-	bool prioritized{ false };
 };
 
 // Core class for determining consensus
@@ -153,6 +152,7 @@ class active_transactions final
 private:
 	std::mutex election_winner_details_mutex;
 	std::unordered_map<nano::block_hash, std::shared_ptr<nano::election>> election_winner_details;
+	nano::vote_generator generator;
 
 	// Call action with confirmed block, may be different than what we started with
 	// clang-format off
@@ -224,6 +224,7 @@ class active_transactions final
 	friend class confirmation_height_prioritize_frontiers_Test;
 	friend class confirmation_height_prioritize_frontiers_overwrite_Test;
 	friend class active_transactions_confirmation_consistency_Test;
+	friend class node_vote_by_hash_bundle_Test;
 	friend std::unique_ptr<container_info_component> collect_container_info (active_transactions &, const std::string &);
 };
 

nano/node/blockprocessor.cpp

@@ -10,9 +10,6 @@
 std::chrono::milliseconds constexpr nano::block_processor::confirmation_request_delay;
 
 nano::block_processor::block_processor (nano::node & node_a, nano::write_database_queue & write_database_queue_a) :
-generator (node_a.config, node_a.store, node_a.wallets, node_a.vote_processor, node_a.votes_cache, node_a.network),
-stopped (false),
-active (false),
 next_log (std::chrono::steady_clock::now ()),
 node (node_a),
 write_database_queue (write_database_queue_a),
@@ -40,7 +37,6 @@ nano::block_processor::~block_processor ()
 
 void nano::block_processor::stop ()
 {
-	generator.stop ();
 	{
 		nano::lock_guard<std::mutex> lock (mutex);
 		stopped = true;
@@ -292,11 +288,6 @@ void nano::block_processor::process_live (nano::block_hash const & hash_a, std::
 	{
 		node.network.flood_block (block_a, nano::buffer_drop_policy::no_limiter_drop);
 	}
-	if (election.prioritized && node.config.enable_voting && node.wallets.rep_counts ().voting > 0)
-	{
-		// Announce our weighted vote to the network
-		generator.add (hash_a);
-	}
 }
 
 nano::process_return nano::block_processor::process_one (nano::write_transaction const & transaction_a, nano::unchecked_info info_a, const bool watch_work_a, const bool first_publish_a)
@@ -501,6 +492,5 @@ std::unique_ptr<nano::container_info_component> nano::collect_container_info (bl
 	composite->add_component (collect_container_info (block_processor.state_block_signature_verification, "state_block_signature_verification"));
 	composite->add_component (std::make_unique<container_info_leaf> (container_info{ "blocks", blocks_count, sizeof (decltype (block_processor.blocks)::value_type) }));
 	composite->add_component (std::make_unique<container_info_leaf> (container_info{ "forced", forced_count, sizeof (decltype (block_processor.forced)::value_type) }));
-	composite->add_component (collect_container_info (block_processor.generator, "generator"));
 	return composite;
 }

nano/node/blockprocessor.hpp

@@ -2,7 +2,6 @@
 
 #include <nano/lib/blocks.hpp>
 #include <nano/node/state_block_signature_verification.hpp>
-#include <nano/node/voting.hpp>
 #include <nano/secure/common.hpp>
 
 #include <boost/multi_index/hashed_index.hpp>
@@ -45,7 +44,6 @@ class block_processor final
 	void process_blocks ();
 	nano::process_return process_one (nano::write_transaction const &, nano::unchecked_info, const bool = false, const bool = false);
 	nano::process_return process_one (nano::write_transaction const &, std::shared_ptr<nano::block>, const bool = false);
-	nano::vote_generator generator;
 	std::atomic<bool> flushing{ false };
 	// Delay required for average network propagartion before requesting confirmation
 	static std::chrono::milliseconds constexpr confirmation_request_delay{ 1500 };
@@ -56,8 +54,8 @@ class block_processor final
 	void process_live (nano::block_hash const &, std::shared_ptr<nano::block>, const bool = false, const bool = false);
 	void requeue_invalid (nano::block_hash const &, nano::unchecked_info const &);
 	void process_verified_state_blocks (std::deque<nano::unchecked_info> &, std::vector<int> const &, std::vector<nano::block_hash> const &, std::vector<nano::signature> const &);
-	bool stopped;
-	bool active;
+	bool stopped{ false };
+	bool active{ false };
 	bool awaiting_write{ false };
 	std::chrono::steady_clock::time_point next_log;
 	std::deque<nano::unchecked_info> blocks;

nano/node/election.cpp

@@ -22,15 +22,20 @@ nano::election_vote_result::election_vote_result (bool replay_a, bool processed_
 	processed = processed_a;
 }
 
-nano::election::election (nano::node & node_a, std::shared_ptr<nano::block> block_a, std::function<void(std::shared_ptr<nano::block>)> const & confirmation_action_a) :
+nano::election::election (nano::node & node_a, std::shared_ptr<nano::block> block_a, std::function<void(std::shared_ptr<nano::block>)> const & confirmation_action_a, bool prioritized_a) :
 confirmation_action (confirmation_action_a),
 state_start (std::chrono::steady_clock::now ()),
 node (node_a),
-status ({ block_a, 0, std::chrono::duration_cast<std::chrono::milliseconds> (std::chrono::system_clock::now ().time_since_epoch ()), std::chrono::duration_values<std::chrono::milliseconds>::zero (), 0, 1, 0, nano::election_status_type::ongoing })
+status ({ block_a, 0, std::chrono::duration_cast<std::chrono::milliseconds> (std::chrono::system_clock::now ().time_since_epoch ()), std::chrono::duration_values<std::chrono::milliseconds>::zero (), 0, 1, 0, nano::election_status_type::ongoing }),
+prioritized_m (prioritized_a)
 {
 	last_votes.emplace (node.network_params.random.not_an_account, nano::vote_info{ std::chrono::steady_clock::now (), 0, block_a->hash () });
 	blocks.emplace (block_a->hash (), block_a);
 	update_dependent ();
+	if (prioritized_a)
+	{
+		generate_votes (block_a->hash ());
+	}
 }
 
 void nano::election::confirm_once (nano::election_status_type type_a)
@@ -354,11 +359,8 @@ void nano::election::confirm_if_quorum ()
 	}
 	if (sum >= node.config.online_weight_minimum.number () && winner_hash_l != status_winner_hash_l)
 	{
-		if (node.config.enable_voting && node.wallets.rep_counts ().voting > 0)
-		{
-			node.votes_cache.remove (status_winner_hash_l);
-			node.block_processor.generator.add (winner_hash_l);
-		}
+		remove_votes (status_winner_hash_l);
+		generate_votes (winner_hash_l);
 		node.block_processor.force (block_l);
 		status.winner = block_l;
 		update_dependent ();
@@ -576,3 +578,39 @@ void nano::election::insert_inactive_votes_cache (nano::block_hash const & hash_
 		confirm_if_quorum ();
 	}
 }
+
+bool nano::election::prioritized () const
+{
+	return prioritized_m;
+}
+
+void nano::election::prioritize_election ()
+{
+	debug_assert (!node.active.mutex.try_lock ());
+	debug_assert (!prioritized_m);
+	prioritized_m = true;
+	generate_votes (status.winner->hash ());
+}
+
+void nano::election::generate_votes (nano::block_hash const & hash_a)
+{
+	if (node.config.enable_voting && node.wallets.rep_counts ().voting > 0)
+	{
+		node.active.generator.add (hash_a);
+	}
+}
+
+void nano::election::remove_votes (nano::block_hash const & hash_a)
+{
+	if (node.config.enable_voting && node.wallets.rep_counts ().voting > 0)
+	{
+		// Remove votes from election
+		auto list_generated_votes (node.votes_cache.find (hash_a));
+		for (auto const & vote : list_generated_votes)
+		{
+			last_votes.erase (vote->account);
+		}
+		// Clear votes cache
+		node.votes_cache.remove (hash_a);
+	}
+}

nano/node/election.hpp

@@ -62,9 +62,13 @@ class election final : public std::enable_shared_from_this<nano::election>
 	void broadcast_block (nano::confirmation_solicitor &);
 	void send_confirm_req (nano::confirmation_solicitor &);
 	void activate_dependencies ();
+	// Calculate votes for local representatives
+	void generate_votes (nano::block_hash const &);
+	void remove_votes (nano::block_hash const &);
+	std::atomic<bool> prioritized_m = { false };
 
 public:
-	election (nano::node &, std::shared_ptr<nano::block>, std::function<void(std::shared_ptr<nano::block>)> const &);
+	election (nano::node &, std::shared_ptr<nano::block>, std::function<void(std::shared_ptr<nano::block>)> const &, bool);
 	nano::election_vote_result vote (nano::account, uint64_t, nano::block_hash);
 	nano::tally_t tally ();
 	// Check if we have vote quorum
@@ -78,6 +82,8 @@ class election final : public std::enable_shared_from_this<nano::election>
 	void update_dependent ();
 	void adjust_dependent_difficulty ();
 	void insert_inactive_votes_cache (nano::block_hash const &);
+	bool prioritized () const;
+	void prioritize_election ();
 	// Erase all blocks from active and, if not confirmed, clear digests from network filters
 	void cleanup ();
 

nano/node/node.cpp

@@ -1112,11 +1112,6 @@ void nano::node::block_confirm (std::shared_ptr<nano::block> block_a)
 	{
 		election.election->transition_active ();
 	}
-	// Calculate votes for local representatives
-	if (election.prioritized && config.enable_voting && wallets.rep_counts ().voting > 0 && active.active (*block_a))
-	{
-		block_processor.generator.add (block_a->hash ());
-	}
 }
 
 bool nano::node::block_confirmed_or_being_confirmed (nano::transaction const & transaction_a, nano::block_hash const & hash_a)