Active elections loop tuning (#2306)

* Active elections tuning and removing per-block confirm_req

Co-authored-by: Srayman <[email protected]>

* Dont broadcast and request confirmation in the same loop for the same election

* Revert socket queue_size_max to original 128, to be increased via another PR

* Use non-doxygen in-definition

* Consistent loop initialization

* Move the alternating condition check upwards

* Fix incorrect documentation on max 16 representatives from rep_crawler

* Cleanup rep_crawler public methods

* Weight-ordered confirmation requests

* Rename confirm_frontiers to search_frontiers and split request_confirm into escalate/broadcast/confirm_req methods

* clang-format

* Add missing bundle insertion check and fix test

* More block broadcasting in tests

nano/core_test/active_transactions.cpp

@@ -22,15 +22,15 @@ TEST (active_transactions, bounded_active_elections)
 	{
 		node.process_active (send);
 		node.active.start (send);
+		ASSERT_NO_ERROR (system.poll ());
 		ASSERT_FALSE (node.active.empty ());
 		ASSERT_LE (node.active.size (), node.config.active_elections_size);
+		++count;
 		done = count > node.active.size ();
-		count++;
-		ASSERT_NO_ERROR (system.poll ());
 		auto previous_hash = send->hash ();
 		send = std::make_shared<nano::state_block> (nano::test_genesis_key.pub, previous_hash, nano::test_genesis_key.pub, nano::genesis_amount - count * nano::xrb_ratio, nano::test_genesis_key.pub, nano::test_genesis_key.prv, nano::test_genesis_key.pub, *system.work.generate (previous_hash));
-		//sleep this thread for the max delay between request loop rounds possible for such a small active_elections_size
-		std::this_thread::sleep_for (std::chrono::milliseconds (node.network_params.network.request_interval_ms + (node_config.active_elections_size * 20)));
+		//sleep this thread between request loop rounds
+		std::this_thread::sleep_for (std::chrono::milliseconds (2 * node.network_params.network.request_interval_ms));
 	}
 }
 
@@ -246,51 +246,50 @@ TEST (active_transactions, keep_local)
 	nano::system system;
 	nano::node_config node_config (24000, system.logging);
 	node_config.enable_voting = false;
-	node_config.active_elections_size = 3; //bound to 3, wont drop wallet created transactions, but good to test dropping remote
+	node_config.active_elections_size = 2; //bound to 2, wont drop wallet created transactions, but good to test dropping remote
 	// Disable frontier confirmation to allow the test to finish before
 	node_config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
-	auto & node1 = *system.add_node (node_config);
+	auto & node = *system.add_node (node_config);
 	auto & wallet (*system.wallet (0));
 	nano::genesis genesis;
 	//key 1/2 will be managed by the wallet
-	nano::keypair key1, key2, key3, key4;
+	nano::keypair key1, key2, key3, key4, key5, key6;
 	wallet.insert_adhoc (nano::test_genesis_key.prv);
-	wallet.insert_adhoc (key1.prv);
-	wallet.insert_adhoc (key2.prv);
-	auto send1 (wallet.send_action (nano::test_genesis_key.pub, key1.pub, node1.config.receive_minimum.number ()));
-	auto send2 (wallet.send_action (nano::test_genesis_key.pub, key2.pub, node1.config.receive_minimum.number ()));
-	auto send3 (wallet.send_action (nano::test_genesis_key.pub, key3.pub, node1.config.receive_minimum.number ()));
-	auto send4 (wallet.send_action (nano::test_genesis_key.pub, key4.pub, node1.config.receive_minimum.number ()));
+	auto send1 (wallet.send_action (nano::test_genesis_key.pub, key1.pub, node.config.receive_minimum.number ()));
+	auto send2 (wallet.send_action (nano::test_genesis_key.pub, key2.pub, node.config.receive_minimum.number ()));
+	auto send3 (wallet.send_action (nano::test_genesis_key.pub, key3.pub, node.config.receive_minimum.number ()));
+	auto send4 (wallet.send_action (nano::test_genesis_key.pub, key4.pub, node.config.receive_minimum.number ()));
+	auto send5 (wallet.send_action (nano::test_genesis_key.pub, key5.pub, node.config.receive_minimum.number ()));
+	auto send6 (wallet.send_action (nano::test_genesis_key.pub, key6.pub, node.config.receive_minimum.number ()));
 	system.deadline_set (10s);
-	while (node1.active.size () != 4)
+	// should not drop wallet created transactions
+	while (node.active.size () != 6)
 	{
 		ASSERT_NO_ERROR (system.poll ());
 	}
-	while (node1.active.size () != 0)
+	while (!node.active.empty ())
 	{
-		nano::lock_guard<std::mutex> active_guard (node1.active.mutex);
-		auto it (node1.active.roots.begin ());
-		while (!node1.active.roots.empty () && it != node1.active.roots.end ())
+		nano::lock_guard<std::mutex> active_guard (node.active.mutex);
+		auto it (node.active.roots.begin ());
+		while (!node.active.roots.empty () && it != node.active.roots.end ())
 		{
 			(it->election)->confirm_once ();
-			it = node1.active.roots.begin ();
+			it = node.active.roots.begin ();
 		}
 	}
-	auto open1 (std::make_shared<nano::state_block> (key3.pub, 0, key3.pub, nano::xrb_ratio, send3->hash (), key3.prv, key3.pub, *system.work.generate (key3.pub)));
-	node1.process_active (open1);
-	auto open2 (std::make_shared<nano::state_block> (key4.pub, 0, key4.pub, nano::xrb_ratio, send4->hash (), key4.prv, key4.pub, *system.work.generate (key4.pub)));
-	node1.process_active (open2);
-	//none are dropped since none are long_unconfirmed
-	system.deadline_set (10s);
-	while (node1.active.size () != 4)
-	{
-		ASSERT_NO_ERROR (system.poll ());
-	}
-	auto send5 (wallet.send_action (nano::test_genesis_key.pub, key1.pub, node1.config.receive_minimum.number ()));
-	node1.active.start (send5);
-	//drop two lowest non-wallet managed active_transactions before inserting a new into active as all are long_unconfirmed
+	auto open1 (std::make_shared<nano::state_block> (key1.pub, 0, key1.pub, node.config.receive_minimum.number (), send1->hash (), key1.prv, key1.pub, *system.work.generate (key1.pub)));
+	node.process_active (open1);
+	node.active.start (open1);
+	auto open2 (std::make_shared<nano::state_block> (key2.pub, 0, key2.pub, node.config.receive_minimum.number (), send2->hash (), key2.prv, key2.pub, *system.work.generate (key2.pub)));
+	node.process_active (open2);
+	node.active.start (open2);
+	auto open3 (std::make_shared<nano::state_block> (key3.pub, 0, key3.pub, node.config.receive_minimum.number (), send3->hash (), key3.prv, key3.pub, *system.work.generate (key3.pub)));
+	node.process_active (open3);
+	node.active.start (open3);
+	ASSERT_EQ (3, node.active.size ());
 	system.deadline_set (10s);
-	while (node1.active.size () != 3)
+	// bound elections, should drop after one loop
+	while (node.active.size () != node_config.active_elections_size)
 	{
 		ASSERT_NO_ERROR (system.poll ());
 	}
@@ -301,7 +300,7 @@ TEST (active_transactions, prioritize_chains)
 	nano::system system;
 	nano::node_config node_config (24000, system.logging);
 	node_config.enable_voting = false;
-	node_config.active_elections_size = 4; //bound to 3, wont drop wallet created transactions, but good to test dropping remote
+	node_config.active_elections_size = 4; //bound to 4, wont drop wallet created transactions, but good to test dropping remote
 	// Disable frontier confirmation to allow the test to finish before
 	node_config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
 	auto & node1 = *system.add_node (node_config);
@@ -352,34 +351,13 @@ TEST (active_transactions, prioritize_chains)
 		ASSERT_NO_ERROR (system.poll ());
 	}
 	system.deadline_set (10s);
-	bool done (false);
-	//wait for all to be long_unconfirmed
-	while (!done)
-	{
-		{
-			nano::lock_guard<std::mutex> guard (node1.active.mutex);
-			done = node1.active.long_unconfirmed_size == 4;
-		}
-		ASSERT_NO_ERROR (system.poll ());
-	}
 	std::this_thread::sleep_for (1s);
 	node1.process_active (open2);
 	system.deadline_set (10s);
 	while (node1.active.size () != 4)
 	{
 		ASSERT_NO_ERROR (system.poll ());
 	}
-	//wait for all to be long_unconfirmed
-	done = false;
-	system.deadline_set (10s);
-	while (!done)
-	{
-		{
-			nano::lock_guard<std::mutex> guard (node1.active.mutex);
-			done = node1.active.long_unconfirmed_size == 4;
-		}
-		ASSERT_NO_ERROR (system.poll ());
-	}
 	size_t seen (0);
 	{
 		auto it (node1.active.roots.get<1> ().begin ());

nano/core_test/confirmation_height.cpp

@@ -895,7 +895,7 @@ TEST (confirmation_height, prioritize_frontiers)
 	transaction.refresh ();
 	node->active.prioritize_frontiers_for_confirmation (transaction, std::chrono::seconds (1), std::chrono::seconds (1));
 	ASSERT_TRUE (priority_orders_match (node->active.priority_wallet_cementable_frontiers, std::array<nano::account, num_accounts>{ key3.pub, nano::genesis_account, key4.pub, key1.pub, key2.pub }));
-	node->active.confirm_frontiers (transaction);
+	node->active.search_frontiers (transaction);
 
 	// Check that the active transactions roots contains the frontiers
 	system.deadline_set (std::chrono::seconds (10));

nano/core_test/node.cpp

@@ -205,7 +205,7 @@ TEST (node, node_receive_quorum)
 			nano::lock_guard<std::mutex> guard (system.nodes[0]->active.mutex);
 			auto info (system.nodes[0]->active.roots.find (nano::qualified_root (previous, previous)));
 			ASSERT_NE (system.nodes[0]->active.roots.end (), info);
-			done = info->election->confirmation_request_count > nano::active_transactions::minimum_confirmation_request_count;
+			done = info->election->confirmation_request_count > 2;
 		}
 		ASSERT_NO_ERROR (system.poll ());
 	}
@@ -850,11 +850,21 @@ TEST (node_config, v17_v18_upgrade)
 	auto upgraded (false);
 	nano::node_config config;
 	config.logging.init (path);
+
+	// Initial values for configs that should be upgraded
+	config.active_elections_size = 50000;
+	config.vote_generator_delay = 500ms;
+
 	// These config options should not be present
 	ASSERT_FALSE (tree.get_optional_child ("backup_before_upgrade"));
 	ASSERT_FALSE (tree.get_optional_child ("work_watcher_period"));
 
 	config.deserialize_json (upgraded, tree);
+
+	// These configs should have been upgraded
+	ASSERT_EQ (100, tree.get<unsigned> ("vote_generator_delay"));
+	ASSERT_EQ (10000, tree.get<unsigned long long> ("active_elections_size"));
+
 	// The config options should be added after the upgrade
 	ASSERT_TRUE (!!tree.get_optional_child ("backup_before_upgrade"));
 	ASSERT_TRUE (!!tree.get_optional_child ("work_watcher_period"));
@@ -878,25 +888,29 @@ TEST (node_config, v18_values)
 
 	// Check config is correct
 	{
+		tree.put ("active_elections_size", 10000);
 		tree.put ("vote_generator_delay", 100);
 		tree.put ("backup_before_upgrade", true);
 		tree.put ("work_watcher_period", 5);
 	}
 
 	config.deserialize_json (upgraded, tree);
 	ASSERT_FALSE (upgraded);
+	ASSERT_EQ (config.active_elections_size, 10000);
 	ASSERT_EQ (config.vote_generator_delay.count (), 100);
 	ASSERT_EQ (config.backup_before_upgrade, true);
 	ASSERT_EQ (config.work_watcher_period.count (), 5);
 
 	// Check config is correct with other values
+	tree.put ("active_elections_size", 5);
 	tree.put ("vote_generator_delay", std::numeric_limits<unsigned long>::max () - 100);
 	tree.put ("backup_before_upgrade", false);
 	tree.put ("work_watcher_period", 999);
 
 	upgraded = false;
 	config.deserialize_json (upgraded, tree);
 	ASSERT_FALSE (upgraded);
+	ASSERT_EQ (config.active_elections_size, 5);
 	ASSERT_EQ (config.vote_generator_delay.count (), std::numeric_limits<unsigned long>::max () - 100);
 	ASSERT_EQ (config.backup_before_upgrade, false);
 	ASSERT_EQ (config.work_watcher_period.count (), 999);
@@ -2315,7 +2329,7 @@ TEST (node, confirm_quorum)
 			nano::lock_guard<std::mutex> guard (system.nodes[0]->active.mutex);
 			auto info (system.nodes[0]->active.roots.find (nano::qualified_root (send1->hash (), send1->hash ())));
 			ASSERT_NE (system.nodes[0]->active.roots.end (), info);
-			done = info->election->confirmation_request_count > nano::active_transactions::minimum_confirmation_request_count;
+			done = info->election->confirmation_request_count > 2;
 		}
 		ASSERT_NO_ERROR (system.poll ());
 	}

nano/lib/config.hpp

@@ -69,7 +69,7 @@ class network_constants
 		default_rpc_port = is_live_network () ? 7076 : is_beta_network () ? 55000 : 45000;
 		default_ipc_port = is_live_network () ? 7077 : is_beta_network () ? 56000 : 46000;
 		default_websocket_port = is_live_network () ? 7078 : is_beta_network () ? 57000 : 47000;
-		request_interval_ms = is_test_network () ? (is_sanitizer_build ? 100 : 20) : 16000;
+		request_interval_ms = is_test_network () ? (is_sanitizer_build ? 100 : 20) : 500;
 	}
 
 	/** Network work thresholds. ~5 seconds of work for the live network */