Merge pull request #4276 from clemahieu/remove_priority_flush

Remove priority::flush and rename node::block_confirm to node::start_election

nano/core_test/confirmation_height.cpp

@@ -1118,7 +1118,7 @@ TEST (confirmation_height, conflict_rollback_cemented)
 					  .work (*system.work.generate (genesis_hash))
 					  .build_shared ();
 		ASSERT_EQ (nano::process_result::progress, node1->process (*fork1a).code);
-		ASSERT_TRUE (nano::test::confirm (*node1, { fork1a }));
+		nano::test::start_elections (system, *node1, { fork1a }, true);
 		ASSERT_TIMELY (5s, nano::test::confirmed (*node1, { fork1a }));
 
 		// create the other side of the fork on node2

nano/core_test/election.cpp

@@ -13,10 +13,8 @@ TEST (election, construction)
 {
 	nano::test::system system (1);
 	auto & node = *system.nodes[0];
-	node.block_confirm (nano::dev::genesis);
-	ASSERT_TIMELY (5s, node.active.election (nano::dev::genesis->qualified_root ()));
-	auto election = node.active.election (nano::dev::genesis->qualified_root ());
-	election->transition_active ();
+	auto election = std::make_shared<nano::election> (
+	node, nano::dev::genesis, [] (auto const &) {}, [] (auto const &) {}, nano::election_behavior::normal);
 }
 
 TEST (election, behavior)
@@ -288,7 +286,7 @@ TEST (election, continuous_voting)
 				 .build_shared ();
 
 	ASSERT_TRUE (nano::test::process (node1, { send1 }));
-	ASSERT_TIMELY (5s, nano::test::confirm (node1, { send1 }));
+	nano::test::start_elections (system, node1, { send1 }, true);
 	ASSERT_TIMELY (5s, nano::test::confirmed (node1, { send1 }));
 
 	node1.stats.clear ();

nano/core_test/election_scheduler.cpp

@@ -140,31 +140,3 @@ TEST (election_scheduler, no_vacancy)
 	ASSERT_TIMELY (5s, node.active.election (block2->qualified_root ()) != nullptr);
 	ASSERT_TRUE (node.scheduler.priority.empty ());
 }
-
-// Ensure that election_scheduler::flush terminates even if no elections can currently be queued e.g. shutdown or no active_transactions vacancy
-TEST (election_scheduler, flush_vacancy)
-{
-	nano::test::system system;
-	nano::node_config config = system.default_config ();
-	// No elections can be activated
-	config.active_elections_size = 0;
-	auto & node = *system.add_node (config);
-	nano::state_block_builder builder;
-	nano::keypair key;
-
-	auto send = builder.make_block ()
-				.account (nano::dev::genesis_key.pub)
-				.previous (nano::dev::genesis->hash ())
-				.representative (nano::dev::genesis_key.pub)
-				.link (key.pub)
-				.balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio)
-				.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
-				.work (*system.work.generate (nano::dev::genesis->hash ()))
-				.build_shared ();
-	ASSERT_EQ (nano::process_result::progress, node.process (*send).code);
-	node.scheduler.priority.activate (nano::dev::genesis_key.pub, node.store.tx_begin_read ());
-	// Ensure this call does not block, even though no elections can be activated.
-	node.scheduler.priority.flush ();
-	ASSERT_EQ (0, node.active.size ());
-	ASSERT_EQ (1, node.scheduler.priority.size ());
-}

nano/core_test/ledger.cpp

@@ -942,7 +942,7 @@ TEST (votes, add_one)
 	node1.work_generate_blocking (*send1);
 	auto transaction (node1.store.tx_begin_write ());
 	ASSERT_EQ (nano::process_result::progress, node1.ledger.process (transaction, *send1).code);
-	node1.block_confirm (send1);
+	node1.start_election (send1);
 	ASSERT_TIMELY (5s, node1.active.election (send1->qualified_root ()));
 	auto election1 = node1.active.election (send1->qualified_root ());
 	ASSERT_EQ (1, election1->votes ().size ());
@@ -1043,7 +1043,7 @@ TEST (votes, add_old)
 	node1.work_generate_blocking (*send1);
 	auto transaction (node1.store.tx_begin_write ());
 	ASSERT_EQ (nano::process_result::progress, node1.ledger.process (transaction, *send1).code);
-	node1.block_confirm (send1);
+	node1.start_election (send1);
 	ASSERT_TIMELY (5s, node1.active.election (send1->qualified_root ()));
 	auto election1 = node1.active.election (send1->qualified_root ());
 	auto vote1 (std::make_shared<nano::vote> (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_min * 2, 0, std::vector<nano::block_hash>{ send1->hash () }));
@@ -1149,7 +1149,7 @@ TEST (votes, add_cooldown)
 	node1.work_generate_blocking (*send1);
 	auto transaction (node1.store.tx_begin_write ());
 	ASSERT_EQ (nano::process_result::progress, node1.ledger.process (transaction, *send1).code);
-	node1.block_confirm (send1);
+	node1.start_election (send1);
 	ASSERT_TIMELY (5s, node1.active.election (send1->qualified_root ()));
 	auto election1 = node1.active.election (send1->qualified_root ());
 	auto vote1 (std::make_shared<nano::vote> (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_min * 1, 0, std::vector<nano::block_hash>{ send1->hash () }));

nano/core_test/node.cpp

@@ -1329,7 +1329,7 @@ TEST (node, DISABLED_broadcast_elected)
 	{
 		auto block (node->block (node->latest (nano::dev::genesis_key.pub)));
 		ASSERT_NE (nullptr, block);
-		node->block_confirm (block);
+		node->start_election (block);
 		auto election (node->active.election (block->qualified_root ()));
 		ASSERT_NE (nullptr, election);
 		election->force_confirm ();
@@ -1397,9 +1397,9 @@ TEST (node, rep_self_vote)
 	ASSERT_EQ (nano::process_result::progress, node0->process (fund_big).code);
 	ASSERT_EQ (nano::process_result::progress, node0->process (open_big).code);
 	// Confirm both blocks, allowing voting on the upcoming block
-	node0->block_confirm (node0->block (open_big.hash ()));
-	auto election = node0->active.election (open_big.qualified_root ());
-	ASSERT_NE (nullptr, election);
+	node0->start_election (node0->block (open_big.hash ()));
+	std::shared_ptr<nano::election> election;
+	ASSERT_TIMELY (5s, election = node0->active.election (open_big.qualified_root ()));
 	election->force_confirm ();
 
 	system.wallet (0)->insert_adhoc (rep_big.prv);
@@ -1545,7 +1545,7 @@ TEST (node, bootstrap_fork_open)
 	// Confirm send0 to allow starting and voting on the following blocks
 	for (auto node : system.nodes)
 	{
-		node->block_confirm (node->block (node->latest (nano::dev::genesis_key.pub)));
+		node->start_election (node->block (node->latest (nano::dev::genesis_key.pub)));
 		ASSERT_TIMELY (1s, node->active.election (send0.qualified_root ()));
 		auto election = node->active.election (send0.qualified_root ());
 		ASSERT_NE (nullptr, election);
@@ -2147,9 +2147,9 @@ TEST (node, block_confirm)
 	ASSERT_TRUE (node1.ledger.block_or_pruned_exists (send1->hash ()));
 	ASSERT_TRUE (node2.ledger.block_or_pruned_exists (send1_copy->hash ()));
 	// Confirm send1 on node2 so it can vote for send2
-	node2.block_confirm (send1_copy);
-	auto election = node2.active.election (send1_copy->qualified_root ());
-	ASSERT_NE (nullptr, election);
+	node2.start_election (send1_copy);
+	std::shared_ptr<nano::election> election;
+	ASSERT_TIMELY (5s, election = node2.active.election (send1_copy->qualified_root ()));
 	ASSERT_TIMELY (10s, node1.active.recently_cemented.list ().size () == 1);
 }
 
@@ -2267,9 +2267,9 @@ TEST (node, local_votes_cache)
 		ASSERT_EQ (nano::process_result::progress, node.ledger.process (transaction, *send2).code);
 	}
 	// Confirm blocks to allow voting
-	node.block_confirm (send2);
-	auto election = node.active.election (send2->qualified_root ());
-	ASSERT_NE (nullptr, election);
+	node.start_election (send2);
+	std::shared_ptr<nano::election> election;
+	ASSERT_TIMELY (5s, election = node.active.election (send2->qualified_root ()));
 	election->force_confirm ();
 	ASSERT_TIMELY (3s, node.ledger.cache.cemented_count == 3);
 	system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
@@ -2573,7 +2573,7 @@ TEST (node, vote_by_hash_bundle)
 	}
 
 	// Confirming last block will confirm whole chain and allow us to generate votes for those blocks later
-	ASSERT_TIMELY (5s, nano::test::confirm (node, { blocks.back () }));
+	nano::test::start_elections (system, node, { blocks.back () }, true);
 	ASSERT_TIMELY (5s, nano::test::confirmed (node, { blocks.back () }));
 
 	system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
@@ -2752,7 +2752,7 @@ TEST (node, epoch_conflict_confirm)
 	ASSERT_TRUE (nano::test::process (node1, { send, send2, open }));
 
 	// Confirm open block in node1 to allow generating votes
-	ASSERT_TIMELY (5s, nano::test::confirm (node1, { open }));
+	nano::test::start_elections (system, node1, { open }, true);
 	ASSERT_TIMELY (5s, nano::test::confirmed (node1, { open }));
 
 	// Process initial blocks on node0
@@ -2766,7 +2766,7 @@ TEST (node, epoch_conflict_confirm)
 	ASSERT_TIMELY (5s, nano::test::exists (node1, { change, epoch_open }));
 
 	// Confirm initial blocks in node1 to allow generating votes later
-	ASSERT_TIMELY (5s, nano::test::confirm (node1, { change, epoch_open, send2 }));
+	nano::test::start_elections (system, node1, { change, epoch_open, send2 }, true);
 	ASSERT_TIMELY (5s, nano::test::confirmed (node1, { change, epoch_open, send2 }));
 
 	// Start elections for node0 for conflicting change and epoch_open blocks (those two blocks have the same root)
@@ -3481,7 +3481,7 @@ TEST (node, rollback_vote_self)
 
 	// Process and mark the first 2 blocks as confirmed to allow voting
 	ASSERT_TRUE (nano::test::process (node, { send1, open }));
-	ASSERT_TIMELY (5s, nano::test::confirm (node, { send1, open }));
+	nano::test::start_elections (system, node, { send1, open }, true);
 	ASSERT_TIMELY (5s, node.ledger.cache.cemented_count == 3);
 
 	// wait until the rep weights have caught up with the weight transfer
@@ -3768,7 +3768,7 @@ TEST (node, dependency_graph)
 
 	// Start an election for the first block of the dependency graph, and ensure all blocks are eventually confirmed
 	system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
-	node.block_confirm (gen_send1);
+	node.start_election (gen_send1);
 
 	ASSERT_NO_ERROR (system.poll_until_true (15s, [&] {
 		// Not many blocks should be active simultaneously
@@ -3957,7 +3957,7 @@ TEST (node, dependency_graph_frontier)
 	system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
 
 	ASSERT_TIMELY (10s, node2.active.active (gen_send1->qualified_root ()));
-	node1.block_confirm (gen_send1);
+	node1.start_election (gen_send1);
 
 	ASSERT_TIMELY (15s, node1.ledger.cache.cemented_count == node1.ledger.cache.block_count);
 	ASSERT_TIMELY (15s, node2.ledger.cache.cemented_count == node2.ledger.cache.block_count);

nano/core_test/optimistic_scheduler.cpp

@@ -24,7 +24,7 @@ TEST (optimistic_scheduler, activate_one)
 	auto & [account, blocks] = chains.front ();
 
 	// Confirm block towards at the beginning the chain, so gap between confirmation and account frontier is larger than `gap_threshold`
-	ASSERT_TRUE (nano::test::confirm (node, { blocks.at (11) }));
+	nano::test::start_elections (system, node, { blocks.at (11) }, true);
 	ASSERT_TIMELY (5s, nano::test::confirmed (node, { blocks.at (11) }));
 
 	// Ensure unconfirmed account head block gets activated
@@ -93,7 +93,7 @@ TEST (optimistic_scheduler, under_gap_threshold)
 	auto & [account, blocks] = chains.front ();
 
 	// Confirm block towards the end of the chain, so gap between confirmation and account frontier is less than `gap_threshold`
-	ASSERT_TRUE (nano::test::confirm (node, { blocks.at (55) }));
+	nano::test::start_elections (system, node, { blocks.at (55) }, true);
 	ASSERT_TIMELY (5s, nano::test::confirmed (node, { blocks.at (55) }));
 
 	// Manually trigger backlog scan

nano/core_test/request_aggregator.cpp

@@ -283,9 +283,9 @@ TEST (request_aggregator, split)
 		request.emplace_back (block->hash (), block->root ());
 	}
 	// Confirm all blocks
-	node.block_confirm (blocks.back ());
-	auto election (node.active.election (blocks.back ()->qualified_root ()));
-	ASSERT_NE (nullptr, election);
+	node.start_election (blocks.back ());
+	std::shared_ptr<nano::election> election;
+	ASSERT_TIMELY (5s, election = node.active.election (blocks.back ()->qualified_root ()));
 	election->force_confirm ();
 	ASSERT_TIMELY (5s, max_vbh + 2 == node.ledger.cache.cemented_count);
 	ASSERT_EQ (max_vbh + 1, request.size ());
@@ -488,7 +488,7 @@ TEST (request_aggregator, cannot_vote)
 	ASSERT_EQ (0, node.stats.count (nano::stat::type::message, nano::stat::detail::confirm_ack, nano::stat::dir::out));
 
 	// With an ongoing election
-	node.block_confirm (send2);
+	node.start_election (send2);
 	node.aggregator.add (dummy_channel, request);
 	ASSERT_EQ (1, node.aggregator.size ());
 	ASSERT_TIMELY (3s, node.aggregator.empty ());
@@ -501,9 +501,9 @@ TEST (request_aggregator, cannot_vote)
 	ASSERT_EQ (0, node.stats.count (nano::stat::type::message, nano::stat::detail::confirm_ack, nano::stat::dir::out));
 
 	// Confirm send1
-	node.block_confirm (send1);
-	auto election (node.active.election (send1->qualified_root ()));
-	ASSERT_NE (nullptr, election);
+	node.start_election (send1);
+	std::shared_ptr<nano::election> election;
+	ASSERT_TIMELY (5s, election = node.active.election (send1->qualified_root ()));
 	election->force_confirm ();
 	ASSERT_TIMELY (3s, node.ledger.dependents_confirmed (node.store.tx_begin_read (), *send2));
 	node.aggregator.add (dummy_channel, request);

nano/core_test/vote_processor.cpp

@@ -13,8 +13,8 @@ TEST (vote_processor, codes)
 {
 	nano::test::system system (1);
 	auto & node (*system.nodes[0]);
-	nano::keypair key;
-	auto vote (std::make_shared<nano::vote> (key.pub, key.prv, nano::vote::timestamp_min * 1, 0, std::vector<nano::block_hash>{ nano::dev::genesis->hash () }));
+	auto blocks = nano::test::setup_chain (system, node, 1, nano::dev::genesis_key, false);
+	auto vote (std::make_shared<nano::vote> (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::vote::timestamp_min * 1, 0, std::vector<nano::block_hash>{ blocks[0]->hash () }));
 	auto vote_invalid = std::make_shared<nano::vote> (*vote);
 	vote_invalid->signature.bytes[0] ^= 1;
 	auto channel (std::make_shared<nano::transport::inproc::channel> (node, node));
@@ -29,8 +29,9 @@ TEST (vote_processor, codes)
 	ASSERT_EQ (nano::vote_code::indeterminate, node.vote_processor.vote_blocking (vote, channel));
 
 	// First vote from an account for an ongoing election
-	node.block_confirm (nano::dev::genesis);
-	ASSERT_NE (nullptr, node.active.election (nano::dev::genesis->qualified_root ()));
+	node.start_election (blocks[0]);
+	std::shared_ptr<nano::election> election;
+	ASSERT_TIMELY (5s, election = node.active.election (blocks[0]->qualified_root ()));
 	ASSERT_EQ (nano::vote_code::vote, node.vote_processor.vote_blocking (vote, channel));
 
 	// Processing the same vote is a replay
@@ -40,7 +41,7 @@ TEST (vote_processor, codes)
 	ASSERT_EQ (nano::vote_code::invalid, node.vote_processor.vote_blocking (vote_invalid, channel));
 
 	// Once the election is removed (confirmed / dropped) the vote is again indeterminate
-	node.active.erase (*nano::dev::genesis);
+	node.active.erase (*blocks[0]);
 	ASSERT_EQ (nano::vote_code::indeterminate, node.vote_processor.vote_blocking (vote, channel));
 }
 
@@ -256,13 +257,13 @@ TEST (vote_processor, local_broadcast_without_a_representative)
 	ASSERT_EQ (nano::process_result::progress, node.process_local (send).value ().code);
 	ASSERT_TIMELY (10s, !node.active.empty ());
 	ASSERT_EQ (node.config.vote_minimum, node.weight (nano::dev::genesis_key.pub));
-	node.block_confirm (send);
+	node.start_election (send);
 	// Process a vote without a representative
 	auto vote = std::make_shared<nano::vote> (nano::dev::genesis_key.pub, nano::dev::genesis_key.prv, nano::milliseconds_since_epoch (), nano::vote::duration_max, std::vector<nano::block_hash>{ send->hash () });
 	ASSERT_EQ (nano::vote_code::vote, node.active.vote (vote));
 	// Make sure the vote was processed.
-	auto election (node.active.election (send->qualified_root ()));
-	ASSERT_NE (nullptr, election);
+	std::shared_ptr<nano::election> election;
+	ASSERT_TIMELY (5s, election = node.active.election (send->qualified_root ()));
 	auto votes (election->votes ());
 	auto existing (votes.find (nano::dev::genesis_key.pub));
 	ASSERT_NE (votes.end (), existing);

nano/core_test/wallet.cpp

@@ -1182,8 +1182,8 @@ TEST (wallet, search_receivable)
 	// Pending search should start an election
 	ASSERT_TRUE (node.active.empty ());
 	ASSERT_FALSE (wallet.search_receivable (wallet.wallets.tx_begin_read ()));
-	auto election = node.active.election (send->qualified_root ());
-	ASSERT_NE (nullptr, election);
+	std::shared_ptr<nano::election> election;
+	ASSERT_TIMELY (5s, election = node.active.election (send->qualified_root ()));
 
 	// Erase the key so the confirmation does not trigger an automatic receive
 	wallet.store.erase (node.wallets.tx_begin_write (), nano::dev::genesis->account ());

nano/core_test/wallets.cpp

@@ -222,8 +222,8 @@ TEST (wallets, search_receivable)
 		{
 			node.wallets.search_receivable (wallet_id);
 		}
-		auto election = node.active.election (send->qualified_root ());
-		ASSERT_NE (nullptr, election);
+		std::shared_ptr<nano::election> election;
+		ASSERT_TIMELY (5s, election = node.active.election (send->qualified_root ()));
 
 		// Erase the key so the confirmation does not trigger an automatic receive
 		wallet->store.erase (node.wallets.tx_begin_write (), nano::dev::genesis->account ());

nano/node/json_handler.cpp

@@ -1205,7 +1205,7 @@ void nano::json_handler::block_confirm ()
 				// Start new confirmation for unconfirmed (or not being confirmed) block
 				if (!node.confirmation_height_processor.is_processing_block (hash))
 				{
-					node.block_confirm (std::move (block_l));
+					node.start_election (std::move (block_l));
 				}
 			}
 			else

nano/node/node.cpp

@@ -1263,17 +1263,9 @@ void nano::node::add_initial_peers ()
 	}
 }
 
-std::shared_ptr<nano::election> nano::node::block_confirm (std::shared_ptr<nano::block> const & block_a)
+void nano::node::start_election (std::shared_ptr<nano::block> const & block)
 {
-	scheduler.priority.manual (block_a);
-	scheduler.priority.flush ();
-	auto election = active.election (block_a->qualified_root ());
-	if (election != nullptr)
-	{
-		election->transition_active ();
-		return election;
-	}
-	return {};
+	scheduler.priority.manual (block);
 }
 
 bool nano::node::block_confirmed (nano::block_hash const & hash_a)

nano/node/node.hpp

@@ -121,11 +121,7 @@ class node final : public std::enable_shared_from_this<nano::node>
 	boost::optional<uint64_t> work_generate_blocking (nano::work_version const, nano::root const &, uint64_t, boost::optional<nano::account> const & = boost::none);
 	void work_generate (nano::work_version const, nano::root const &, uint64_t, std::function<void (boost::optional<uint64_t>)>, boost::optional<nano::account> const & = boost::none, bool const = false);
 	void add_initial_peers ();
-	/*
-	 * Starts an election for the block, DOES NOT confirm it
-	 * TODO: Rename to `start_election`
-	 */
-	std::shared_ptr<nano::election> block_confirm (std::shared_ptr<nano::block> const &);
+	void start_election (std::shared_ptr<nano::block> const & block);
 	bool block_confirmed (nano::block_hash const &);
 	bool block_confirmed_or_being_confirmed (nano::block_hash const &);
 	void do_rpc_callback (boost::asio::ip::tcp::resolver::iterator i_a, std::string const &, uint16_t, std::shared_ptr<std::string> const &, std::shared_ptr<std::string> const &, std::shared_ptr<boost::asio::ip::tcp::resolver> const &);

nano/node/scheduler/priority.cpp

@@ -69,14 +69,6 @@ bool nano::scheduler::priority::activate (nano::account const & account_a, nano:
 	return false; // Not activated
 }
 
-void nano::scheduler::priority::flush ()
-{
-	nano::unique_lock<nano::mutex> lock{ mutex };
-	condition.wait (lock, [this] () {
-		return stopped || empty_locked () || node.active.vacancy () <= 0;
-	});
-}
-
 void nano::scheduler::priority::notify ()
 {
 	condition.notify_all ();
@@ -133,7 +125,11 @@ void nano::scheduler::priority::run ()
 				manual_queue.pop_front ();
 				lock.unlock ();
 				stats.inc (nano::stat::type::election_scheduler, nano::stat::detail::insert_manual);
-				node.active.insert (block, election_behavior);
+				auto result = node.active.insert (block, election_behavior);
+				if (result.election != nullptr)
+				{
+					result.election->transition_active ();
+				}
 			}
 			else if (priority_queue_predicate ())
 			{