Election refactor follow up

nano/core_test/active_transactions.cpp

@@ -7,76 +7,93 @@
 
 using namespace std::chrono_literals;
 
+namespace nano
+{
 TEST (active_transactions, confirm_active)
 {
-	nano::system system (1);
-	auto & node1 = *system.nodes[0];
-	// Send and vote for a block before peering with node2
-	system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv);
-	auto send (system.wallet (0)->send_action (nano::test_genesis_key.pub, nano::public_key (), node1.config.receive_minimum.number ()));
-	system.deadline_set (5s);
-	while (!node1.active.empty () || !node1.block_confirmed_or_being_confirmed (node1.store.tx_begin_read (), send->hash ()))
-	{
-		ASSERT_NO_ERROR (system.poll ());
-	}
-	auto & node2 = *system.add_node (nano::node_config (nano::get_available_port (), system.logging));
+	nano::system system;
+	nano::node_flags node_flags;
+	node_flags.disable_request_loop = true;
+	auto & node1 = *system.add_node (node_flags);
+	nano::genesis genesis;
+	auto send (std::make_shared<nano::send_block> (genesis.hash (), nano::public_key (), nano::genesis_amount - 100, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (genesis.hash ())));
+	ASSERT_EQ (nano::process_result::progress, node1.process (*send).code);
+	nano::node_config node_config2 (nano::get_available_port (), system.logging);
+	node_config2.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
+	nano::node_flags node_flags2;
+	// The rep crawler would otherwise request confirmations in order to find representatives
+	node_flags2.disable_rep_crawler = true;
+	auto & node2 = *system.add_node (node_config2, node_flags2);
 	system.deadline_set (5s);
 	// Let node2 know about the block
 	while (node2.active.empty ())
 	{
 		node1.network.flood_block (send, nano::buffer_drop_policy::no_limiter_drop);
 		ASSERT_NO_ERROR (system.poll ());
 	}
-	while (node2.ledger.cache.cemented_count < 2 || !node2.active.empty ())
-	{
-		ASSERT_NO_ERROR (system.poll ());
-	}
-}
-
-TEST (active_transactions, confirm_frontier)
-{
-	nano::system system (1);
-	auto & node1 = *system.nodes[0];
-	// Send and vote for a block before peering with node2
+	// Save election to check request count afterwards
+	auto election = node2.active.election (send->qualified_root ());
+	ASSERT_NE (nullptr, election);
+	// Add key to node1
 	system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv);
-	auto send (system.wallet (0)->send_action (nano::test_genesis_key.pub, nano::public_key (), node1.config.receive_minimum.number ()));
-	system.deadline_set (5s);
-	while (!node1.active.empty () || !node1.block_confirmed_or_being_confirmed (node1.store.tx_begin_read (), send->hash ()))
+	// Add representative to disabled rep crawler
+	auto peers (node2.network.random_set (1));
+	ASSERT_FALSE (peers.empty ());
 	{
-		ASSERT_NO_ERROR (system.poll ());
+		nano::lock_guard<std::mutex> guard (node2.rep_crawler.probable_reps_mutex);
+		node2.rep_crawler.probable_reps.emplace (nano::test_genesis_key.pub, nano::genesis_amount, *peers.begin ());
 	}
-	auto & node2 = *system.add_node (nano::node_config (nano::get_available_port (), system.logging));
-	ASSERT_EQ (nano::process_result::progress, node2.process (*send).code);
-	system.deadline_set (5s);
 	while (node2.ledger.cache.cemented_count < 2 || !node2.active.empty ())
 	{
 		ASSERT_NO_ERROR (system.poll ());
 	}
+	// At least one confirmation request
+	ASSERT_GT (election->confirmation_request_count, 0);
+	// Blocks were cleared (except for not_an_account)
+	ASSERT_EQ (1, election->blocks.size ());
+}
 }
 
-TEST (active_transactions, confirm_dependent)
+namespace nano
+{
+TEST (active_transactions, confirm_frontier)
 {
 	nano::system system;
 	nano::node_flags node_flags;
 	node_flags.disable_request_loop = true;
 	auto & node1 = *system.add_node (node_flags);
+	nano::genesis genesis;
+	auto send (std::make_shared<nano::send_block> (genesis.hash (), nano::public_key (), nano::genesis_amount - 100, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (genesis.hash ())));
+	ASSERT_EQ (nano::process_result::progress, node1.process (*send).code);
+	nano::node_flags node_flags2;
+	// The rep crawler would otherwise request confirmations in order to find representatives
+	node_flags2.disable_rep_crawler = true;
+	auto & node2 = *system.add_node (node_flags2);
+	ASSERT_EQ (nano::process_result::progress, node2.process (*send).code);
+	system.deadline_set (5s);
+	while (node2.active.empty ())
+	{
+		ASSERT_NO_ERROR (system.poll ());
+	}
+	// Save election to check request count afterwards
+	auto election = node2.active.election (send->qualified_root ());
+	ASSERT_NE (nullptr, election);
+	// Add key to node1
 	system.wallet (0)->insert_adhoc (nano::test_genesis_key.prv);
-	auto send1 (system.wallet (0)->send_action (nano::test_genesis_key.pub, nano::public_key (), node1.config.receive_minimum.number ()));
-	auto send2 (system.wallet (0)->send_action (nano::test_genesis_key.pub, nano::public_key (), node1.config.receive_minimum.number ()));
-	auto send3 (system.wallet (0)->send_action (nano::test_genesis_key.pub, nano::public_key (), node1.config.receive_minimum.number ()));
-	nano::node_config node_config;
-	node_config.peering_port = nano::get_available_port ();
-	node_config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
-	auto & node2 = *system.add_node (node_config);
-	node2.process_local (send1);
-	node2.process_local (send2);
-	node2.process_active (send3);
+	// Add representative to disabled rep crawler
+	auto peers (node2.network.random_set (1));
+	ASSERT_FALSE (peers.empty ());
+	{
+		nano::lock_guard<std::mutex> guard (node2.rep_crawler.probable_reps_mutex);
+		node2.rep_crawler.probable_reps.emplace (nano::test_genesis_key.pub, nano::genesis_amount, *peers.begin ());
+	}
 	system.deadline_set (5s);
-	while (!node2.active.empty ())
+	while (node2.ledger.cache.cemented_count < 2 || !node2.active.empty ())
 	{
 		ASSERT_NO_ERROR (system.poll ());
 	}
-	ASSERT_EQ (4, node2.ledger.cache.cemented_count);
+	ASSERT_GT (election->confirmation_request_count, 0);
+}
 }
 
 TEST (active_transactions, adjusted_difficulty_priority)

nano/core_test/confirmation_solicitor.cpp

@@ -1,5 +1,6 @@
 #include <nano/core_test/testutil.hpp>
 #include <nano/lib/jsonconfig.hpp>
+#include <nano/node/confirmation_solicitor.hpp>
 #include <nano/node/testing.hpp>
 
 #include <gtest/gtest.h>
@@ -9,51 +10,49 @@ using namespace std::chrono_literals;
 TEST (confirmation_solicitor, batches)
 {
 	nano::system system;
-	nano::node_config node_config (nano::get_available_port (), system.logging);
-	node_config.enable_voting = false;
-	node_config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
 	nano::node_flags node_flags;
+	node_flags.disable_request_loop = true;
 	node_flags.disable_udp = false;
-	auto & node1 = *system.add_node (node_config, node_flags);
-	node_config.peering_port = nano::get_available_port ();
-	// To prevent races on the solicitor
+	auto & node1 = *system.add_node (node_flags);
+	// This tests instantiates a solicitor
 	node_flags.disable_request_loop = true;
-	auto & node2 = *system.add_node (node_config, node_flags);
+	auto & node2 = *system.add_node (node_flags);
 	// Solicitor will only solicit from this representative
 	auto channel1 (node2.network.udp_channels.create (node1.network.endpoint ()));
 	nano::representative representative (nano::test_genesis_key.pub, nano::genesis_amount, channel1);
-	// Lock active_transactions which uses the solicitor
+
+	std::vector<nano::representative> representatives{ representative };
+	nano::confirmation_solicitor solicitor (node2.network, node2.network_params.network);
+	solicitor.prepare (representatives);
+	// Ensure the representatives are correct
+	ASSERT_EQ (1, representatives.size ());
+	ASSERT_EQ (channel1, representatives.front ().channel);
+	ASSERT_EQ (nano::test_genesis_key.pub, representatives.front ().account);
+	auto send (std::make_shared<nano::send_block> (nano::genesis_hash, nano::keypair ().pub, nano::genesis_amount - 100, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (nano::genesis_hash)));
 	{
-		nano::lock_guard<std::mutex> active_guard (node2.active.mutex);
-		std::vector<nano::representative> representatives{ representative };
-		node2.active.solicitor.prepare (representatives);
-		// Ensure the representatives are correct
-		ASSERT_EQ (1, representatives.size ());
-		ASSERT_EQ (channel1, representatives.front ().channel);
-		ASSERT_EQ (nano::test_genesis_key.pub, representatives.front ().account);
-		auto send (std::make_shared<nano::send_block> (nano::genesis_hash, nano::keypair ().pub, nano::genesis_amount - 100, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (nano::genesis_hash)));
+		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));
-			ASSERT_FALSE (node2.active.solicitor.add (*election));
+			ASSERT_FALSE (solicitor.add (*election));
 		}
-		ASSERT_EQ (1, node2.active.solicitor.max_confirm_req_batches);
+		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));
-		ASSERT_TRUE (node2.active.solicitor.add (*election));
+		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));
-		ASSERT_FALSE (node2.active.solicitor.broadcast (*election));
-		system.deadline_set (5s);
-		while (node2.stats.count (nano::stat::type::message, nano::stat::detail::publish, nano::stat::dir::out) < 1)
-		{
-			ASSERT_NO_ERROR (system.poll ());
-		}
+		ASSERT_FALSE (solicitor.broadcast (*election));
+	}
+	system.deadline_set (5s);
+	while (node2.stats.count (nano::stat::type::message, nano::stat::detail::publish, nano::stat::dir::out) < 1)
+	{
+		ASSERT_NO_ERROR (system.poll ());
 	}
 	// From rep crawler
 	ASSERT_EQ (1, node2.stats.count (nano::stat::type::message, nano::stat::detail::confirm_req, nano::stat::dir::out));
 	system.deadline_set (5s);
-	node2.active.solicitor.flush ();
+	solicitor.flush ();
 	while (node2.stats.count (nano::stat::type::message, nano::stat::detail::confirm_req, nano::stat::dir::out) < 2)
 	{
 		ASSERT_NO_ERROR (system.poll ());

nano/node/active_transactions.cpp

@@ -1,8 +1,11 @@
 #include <nano/lib/threading.hpp>
 #include <nano/node/active_transactions.hpp>
 #include <nano/node/confirmation_height_processor.hpp>
+#include <nano/node/confirmation_solicitor.hpp>
 #include <nano/node/election.hpp>
 #include <nano/node/node.hpp>
+#include <nano/node/repcrawler.hpp>
+#include <nano/secure/blockstore.hpp>
 
 #include <boost/format.hpp>
 #include <boost/variant/get.hpp>
@@ -16,7 +19,6 @@ confirmation_height_processor (confirmation_height_processor_a),
 node (node_a),
 multipliers_cb (20, 1.),
 trended_active_difficulty (node_a.network_params.network.publish_threshold),
-solicitor (node_a.network, node_a.network_params.network),
 election_time_to_live (node_a.network_params.network.is_test_network () ? 0s : 2s),
 thread ([this]() {
 	nano::thread_role::set (nano::thread_role::name::request_loop);
@@ -225,6 +227,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock<std::mutex> &
 
 	// Only representatives ready to receive batched confirm_req
 	lock_a.unlock ();
+	nano::confirmation_solicitor solicitor (node.network, node.network_params.network);
 	solicitor.prepare (node.rep_crawler.representatives (node.network_params.protocol.tcp_realtime_protocol_version_min));
 	lock_a.lock ();
 
@@ -244,7 +247,8 @@ void nano::active_transactions::request_confirm (nano::unique_lock<std::mutex> &
 	for (auto i = sorted_roots_l.begin (), n = sorted_roots_l.end (); i != n; ++count_l)
 	{
 		auto & election_l (i->election);
-		if ((count_l >= node.config.active_elections_size && election_l->election_start < election_ttl_cutoff_l && !node.wallets.watcher->is_watched (i->root)) || election_l->transition_time (saturated_l))
+		bool const overflow_l (count_l >= node.config.active_elections_size && election_l->election_start < election_ttl_cutoff_l && !node.wallets.watcher->is_watched (i->root));
+		if (overflow_l || election_l->transition_time (solicitor, saturated_l))
 		{
 			election_l->clear_blocks ();
 			i = sorted_roots_l.erase (i);

nano/node/active_transactions.hpp

@@ -1,12 +1,6 @@
 #pragma once
 
 #include <nano/lib/numbers.hpp>
-#include <nano/node/confirmation_solicitor.hpp>
-#include <nano/node/election.hpp>
-#include <nano/node/gap_cache.hpp>
-#include <nano/node/repcrawler.hpp>
-#include <nano/node/transport/transport.hpp>
-#include <nano/secure/blockstore.hpp>
 #include <nano/secure/common.hpp>
 
 #include <boost/circular_buffer.hpp>
@@ -142,7 +136,6 @@ class active_transactions final
 	size_t inactive_votes_cache_size ();
 	size_t election_winner_details_size ();
 	void add_election_winner_details (nano::block_hash const &, std::shared_ptr<nano::election> const &);
-	nano::confirmation_solicitor solicitor;
 
 private:
 	std::mutex election_winner_details_mutex;

nano/node/blockprocessor.cpp

@@ -1,5 +1,6 @@
 #include <nano/lib/timer.hpp>
 #include <nano/node/blockprocessor.hpp>
+#include <nano/node/election.hpp>
 #include <nano/node/node.hpp>
 #include <nano/secure/blockstore.hpp>
 

nano/node/election.cpp

@@ -1,3 +1,4 @@
+#include <nano/node/confirmation_solicitor.hpp>
 #include <nano/node/election.hpp>
 #include <nano/node/node.hpp>
 
@@ -148,13 +149,14 @@ bool nano::election::state_change (nano::election::state_t expected_a, nano::ele
 	return result;
 }
 
-void nano::election::send_confirm_req ()
+void nano::election::send_confirm_req (nano::confirmation_solicitor & solicitor_a)
 {
 	if (last_req + std::chrono::seconds (15) < std::chrono::steady_clock::now ())
 	{
-		if (!node.active.solicitor.add (*this))
+		if (!solicitor_a.add (*this))
 		{
 			last_req = std::chrono::steady_clock::now ();
+			++confirmation_request_count;
 		}
 	}
 }
@@ -242,18 +244,18 @@ void nano::election::activate_dependencies ()
 	}
 }
 
-void nano::election::broadcast_block ()
+void nano::election::broadcast_block (nano::confirmation_solicitor & solicitor_a)
 {
 	if (base_latency () * 5 < std::chrono::steady_clock::now () - last_block)
 	{
-		if (!node.active.solicitor.broadcast (*this))
+		if (!solicitor_a.broadcast (*this))
 		{
 			last_block = std::chrono::steady_clock::now ();
 		}
 	}
 }
 
-bool nano::election::transition_time (bool const saturated_a)
+bool nano::election::transition_time (nano::confirmation_solicitor & solicitor_a, bool const saturated_a)
 {
 	debug_assert (!node.active.mutex.try_lock ());
 	nano::unique_lock<std::mutex> lock (timepoints_mutex);
@@ -271,8 +273,8 @@ bool nano::election::transition_time (bool const saturated_a)
 			break;
 		}
 		case nano::election::state_t::active:
-			broadcast_block ();
-			send_confirm_req ();
+			broadcast_block (solicitor_a);
+			send_confirm_req (solicitor_a);
 			if (base_latency () * active_duration_factor < std::chrono::steady_clock::now () - state_start)
 			{
 				state_change (nano::election::state_t::active, nano::election::state_t::backtracking);
@@ -281,8 +283,8 @@ bool nano::election::transition_time (bool const saturated_a)
 			}
 			break;
 		case nano::election::state_t::backtracking:
-			broadcast_block ();
-			send_confirm_req ();
+			broadcast_block (solicitor_a);
+			send_confirm_req (solicitor_a);
 			break;
 		case nano::election::state_t::confirmed:
 			if (base_latency () * (saturated_a ? confirmed_duration_factor_saturated : confirmed_duration_factor) < std::chrono::steady_clock::now () - state_start)

nano/node/election.hpp

@@ -12,27 +12,8 @@
 namespace nano
 {
 class channel;
+class confirmation_solicitor;
 class node;
-enum class election_status_type : uint8_t
-{
-	ongoing = 0,
-	active_confirmed_quorum = 1,
-	active_confirmation_height = 2,
-	inactive_confirmation_height = 3,
-	stopped = 5
-};
-class election_status final
-{
-public:
-	std::shared_ptr<nano::block> winner;
-	nano::amount tally;
-	std::chrono::milliseconds election_end;
-	std::chrono::milliseconds election_duration;
-	unsigned confirmation_request_count;
-	unsigned block_count;
-	unsigned voter_count;
-	election_status_type type;
-};
 class vote_info final
 {
 public:
@@ -80,8 +61,8 @@ class election final : public std::enable_shared_from_this<nano::election>
 
 	bool valid_change (nano::election::state_t, nano::election::state_t) const;
 	bool state_change (nano::election::state_t, nano::election::state_t);
-	void broadcast_block ();
-	void send_confirm_req ();
+	void broadcast_block (nano::confirmation_solicitor &);
+	void send_confirm_req (nano::confirmation_solicitor &);
 	void activate_dependencies ();
 
 public:
@@ -101,7 +82,7 @@ class election final : public std::enable_shared_from_this<nano::election>
 	void insert_inactive_votes_cache (nano::block_hash const &);
 
 public: // State transitions
-	bool transition_time (bool const saturated);
+	bool transition_time (nano::confirmation_solicitor &, bool const saturated);
 	void transition_passive ();
 	void transition_active ();