From 360ef3d5646a680ed0d2d78c1680679e8ea594a9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20W=C3=B3jcik?=
 <3044353+pwojcikdev@users.noreply.github.com>
Date: Thu, 23 May 2024 23:51:21 +0200
Subject: [PATCH] Use read/write transaction variant in vote generator (#4639)

---
 nano/node/vote_generator.cpp | 46 +++++++++++++++++++++++++++---------
 nano/node/vote_generator.hpp | 11 ++++-----
 nano/store/write_queue.hpp   |  1 -
 3 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/nano/node/vote_generator.cpp b/nano/node/vote_generator.cpp
index bf0d86dbc2..a0619d7ceb 100644
--- a/nano/node/vote_generator.cpp
+++ b/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;
-	{
-		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 });
+	std::deque<candidate_t> verified;
 
+	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 ();
diff --git a/nano/node/vote_generator.hpp b/nano/node/vote_generator.hpp
index bc1a2914ab..6557274a39 100644
--- a/nano/node/vote_generator.hpp
+++ b/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 @@ class vote_generator final
 	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);
-	/**
-	 * 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 &);
+	bool should_vote (transaction_variant_t const &, nano::root const &, nano::block_hash const &) 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
diff --git a/nano/store/write_queue.hpp b/nano/store/write_queue.hpp
index e01a6bf360..8875b2db35 100644
--- a/nano/store/write_queue.hpp
+++ b/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
 };