Use read/write transaction variant in vote generator (#4639)

nano/node/vote_generator.cpp

@@ -38,20 +38,29 @@ nano::vote_generator::vote_generator (nano::node_config const & config_a, nano::
 
 nano::vote_generator::~vote_generator ()
 {
-	stop ();
+	debug_assert (stopped);
+	debug_assert (!thread.joinable ());
 }
 
-bool nano::vote_generator::should_vote (secure::write_transaction const & transaction, nano::root const & root_a, nano::block_hash const & hash_a)
+bool nano::vote_generator::should_vote (transaction_variant_t const & transaction_variant, nano::root const & root_a, nano::block_hash const & hash_a) const
 {
-	auto block = ledger.any.block_get (transaction, hash_a);
 	bool should_vote = false;
+	std::shared_ptr<nano::block> block;
 	if (is_final)
 	{
+		debug_assert (std::holds_alternative<nano::secure::write_transaction> (transaction_variant));
+		auto const & transaction = std::get<nano::secure::write_transaction> (transaction_variant);
+
+		block = ledger.any.block_get (transaction, hash_a);
 		should_vote = block != nullptr && ledger.dependents_confirmed (transaction, *block) && ledger.store.final_vote.put (transaction, block->qualified_root (), hash_a);
 		debug_assert (block == nullptr || root_a == block->root ());
 	}
 	else
 	{
+		debug_assert (std::holds_alternative<nano::secure::read_transaction> (transaction_variant));
+		auto const & transaction = std::get<nano::secure::read_transaction> (transaction_variant);
+
+		block = ledger.any.block_get (transaction, hash_a);
 		should_vote = block != nullptr && ledger.dependents_confirmed (transaction, *block);
 	}
 
@@ -94,24 +103,39 @@ void nano::vote_generator::add (const root & root, const block_hash & hash)
 
 void nano::vote_generator::process_batch (std::deque<queue_entry_t> & batch)
 {
-	std::deque<candidate_t> candidates_new;
+	std::deque<candidate_t> verified;
-	{
-		auto guard = ledger.store.write_queue.wait (is_final ? nano::store::writer::voting_final : nano::store::writer::voting);
-		auto transaction = ledger.tx_begin_write ({ tables::final_votes });
 
+	auto verify_batch = [this, &verified] (auto && transaction_variant, auto && batch) {
 		for (auto & [root, hash] : batch)
 		{
-			if (should_vote (transaction, root, hash))
+			if (should_vote (transaction_variant, root, hash))
 			{
-				candidates_new.emplace_back (root, hash);
+				verified.emplace_back (root, hash);
 			}
 		}
+	};
+
+	if (is_final)
+	{
+		auto guard = ledger.store.write_queue.wait (nano::store::writer::voting_final);
+		transaction_variant_t transaction_variant{ ledger.tx_begin_write ({ tables::final_votes }) };
+
+		verify_batch (transaction_variant, batch);
+
 		// Commit write transaction
 	}
-	if (!candidates_new.empty ())
+	else
+	{
+		transaction_variant_t transaction_variant{ ledger.tx_begin_read () };
+
+		verify_batch (transaction_variant, batch);
+	}
+
+	// Submit verified candidates to the main processing thread
+	if (!verified.empty ())
 	{
 		nano::unique_lock<nano::mutex> lock{ mutex };
-		candidates.insert (candidates.end (), candidates_new.begin (), candidates_new.end ());
+		candidates.insert (candidates.end (), verified.begin (), verified.end ());
 		if (candidates.size () >= nano::network::confirm_ack_hashes_max)
 		{
 			lock.unlock ();

nano/node/vote_generator.hpp

@@ -17,6 +17,7 @@
 #include <condition_variable>
 #include <deque>
 #include <thread>
+#include <variant>
 
 namespace mi = boost::multi_index;
 
@@ -36,6 +37,7 @@ namespace nano::secure
 {
 class transaction;
 class write_transaction;
+class read_transaction;
 }
 namespace nano::transport
 {
@@ -67,18 +69,15 @@ public:
 	std::unique_ptr<container_info_component> collect_container_info (std::string const & name) const;
 
 private:
+	using transaction_variant_t = std::variant<nano::secure::read_transaction, nano::secure::write_transaction>;
+
 	void run ();
 	void broadcast (nano::unique_lock<nano::mutex> &);
 	void reply (nano::unique_lock<nano::mutex> &, request_t &&);
 	void vote (std::vector<nano::block_hash> const &, std::vector<nano::root> const &, std::function<void (std::shared_ptr<nano::vote> const &)> const &);
 	void broadcast_action (std::shared_ptr<nano::vote> const &) const;
 	void process_batch (std::deque<queue_entry_t> & batch);
-	/**
+	bool should_vote (transaction_variant_t const &, nano::root const &, nano::block_hash const &) const;
-	 * Check if block is eligible for vote generation
-	 * @param transaction : needs `tables::final_votes` lock
-	 * @return: Should vote
-	 */
-	bool should_vote (secure::write_transaction const &, nano::root const &, nano::block_hash const &);
 
 private:
 	std::function<void (std::shared_ptr<nano::vote> const &, std::shared_ptr<nano::transport::channel> &)> reply_action; // must be set only during initialization by using set_reply_action

nano/store/write_queue.hpp

@@ -14,7 +14,6 @@ enum class writer
 	confirmation_height,
 	process_batch,
 	pruning,
-	voting,
 	voting_final,
 	testing // Used in tests to emulate a write lock
 };