Remove election_winner_details container

nano/core_test/active_elections.cpp

@@ -1224,7 +1224,7 @@ TEST (active_elections, activate_inactive)
 	ASSERT_NE (nullptr, election);
 	election->force_confirm ();
 
-	ASSERT_TIMELY (5s, !node.confirming_set.exists (send2->hash ()));
+	ASSERT_TIMELY (5s, !node.confirming_set.contains (send2->hash ()));
 	ASSERT_TIMELY (5s, node.block_confirmed (send2->hash ()));
 	ASSERT_TIMELY (5s, node.block_confirmed (send->hash ()));
 
@@ -1398,6 +1398,8 @@ TEST (active_elections, bound_election_winners)
 	nano::node_config config = system.default_config ();
 	// Set election winner limit to a low value
 	config.active_elections.max_election_winners = 5;
+	// Large batch size would complicate this testcase
+	config.confirming_set.batch_size = 1;
 	auto & node = *system.add_node (config);
 
 	// Start elections for a couple of blocks, number of elections is larger than the election winner set limit
@@ -1411,22 +1413,12 @@ TEST (active_elections, bound_election_winners)
 		auto guard = node.ledger.tx_begin_write (nano::store::writer::testing);
 
 		// Ensure that when the number of election winners reaches the limit, AEC vacancy reflects that
+		// Confirming more elections should make the vacancy negative
 		ASSERT_TRUE (node.active.vacancy (nano::election_behavior::priority) > 0);
 
-		int index = 0;
-		for (; index < config.active_elections.max_election_winners; ++index)
-		{
-			auto election = node.vote_router.election (blocks[index]->hash ());
-			ASSERT_TRUE (election);
-			election->force_confirm ();
-		}
-
-		ASSERT_TIMELY_EQ (5s, node.active.vacancy (nano::election_behavior::priority), 0);
-
-		// Confirming more elections should make the vacancy negative
-		for (; index < blocks.size (); ++index)
+		for (auto const & block : blocks)
 		{
-			auto election = node.vote_router.election (blocks[index]->hash ());
+			auto election = node.vote_router.election (block->hash ());
 			ASSERT_TRUE (election);
 			election->force_confirm ();
 		}

nano/core_test/confirming_set.cpp

@@ -30,7 +30,7 @@ TEST (confirming_set, add_exists)
 	nano::confirming_set confirming_set{ config, ctx.ledger (), ctx.stats () };
 	auto send = ctx.blocks ()[0];
 	confirming_set.add (send->hash ());
-	ASSERT_TRUE (confirming_set.exists (send->hash ()));
+	ASSERT_TRUE (confirming_set.contains (send->hash ()));
 }
 
 TEST (confirming_set, process_one)
@@ -111,7 +111,6 @@ TEST (confirmation_callback, observer_callbacks)
 
 	ASSERT_EQ (2, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in));
 	ASSERT_EQ (3, node->ledger.cemented_count ());
-	ASSERT_EQ (0, node->active.election_winner_details_size ());
 }
 
 // The callback and confirmation history should only be updated after confirmation height is set (and not just after voting)
@@ -186,7 +185,6 @@ TEST (confirmation_callback, confirmed_history)
 	ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::inactive_conf_height, nano::stat::dir::out));
 	ASSERT_TIMELY_EQ (5s, 2, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in));
 	ASSERT_EQ (3, node->ledger.cemented_count ());
-	ASSERT_EQ (0, node->active.election_winner_details_size ());
 }
 
 TEST (confirmation_callback, dependent_election)
@@ -242,35 +240,10 @@ TEST (confirmation_callback, dependent_election)
 	// Wait for blocks to be confirmed in ledger, callbacks will happen after
 	ASSERT_TIMELY_EQ (5s, 3, node->stats.count (nano::stat::type::confirmation_height, nano::stat::detail::blocks_confirmed, nano::stat::dir::in));
 	// Once the item added to the confirming set no longer exists, callbacks have completed
-	ASSERT_TIMELY (5s, !node->confirming_set.exists (send2->hash ()));
+	ASSERT_TIMELY (5s, !node->confirming_set.contains (send2->hash ()));
 
 	ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_quorum, nano::stat::dir::out));
 	ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::active_conf_height, nano::stat::dir::out));
 	ASSERT_TIMELY_EQ (5s, 1, node->stats.count (nano::stat::type::confirmation_observer, nano::stat::detail::inactive_conf_height, nano::stat::dir::out));
 	ASSERT_EQ (4, node->ledger.cemented_count ());
-
-	ASSERT_EQ (0, node->active.election_winner_details_size ());
-}
-
-TEST (confirmation_callback, election_winner_details_clearing_node_process_confirmed)
-{
-	// Make sure election_winner_details is also cleared if the block never enters the confirmation height processor from node::process_confirmed
-	nano::test::system system (1);
-	auto node = system.nodes.front ();
-
-	nano::block_builder builder;
-	auto send = builder
-				.send ()
-				.previous (nano::dev::genesis->hash ())
-				.destination (nano::dev::genesis_key.pub)
-				.balance (nano::dev::constants.genesis_amount - nano::Knano_ratio)
-				.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
-				.work (*system.work.generate (nano::dev::genesis->hash ()))
-				.build ();
-	// Add to election_winner_details. Use an unrealistic iteration so that it should fall into the else case and do a cleanup
-	node->active.add_election_winner_details (send->hash (), nullptr);
-	nano::election_status election;
-	election.winner = send;
-	node->process_confirmed (election, 1000000);
-	ASSERT_EQ (0, node->active.election_winner_details_size ());
 }

nano/core_test/ledger_confirm.cpp

@@ -752,29 +752,6 @@ TEST (ledger_confirm, observers)
 	ASSERT_EQ (2, node1->ledger.cemented_count ());
 }
 
-TEST (ledger_confirm, election_winner_details_clearing_node_process_confirmed)
-{
-	// Make sure election_winner_details is also cleared if the block never enters the confirmation height processor from node::process_confirmed
-	nano::test::system system (1);
-	auto node = system.nodes.front ();
-
-	nano::block_builder builder;
-	auto send = builder
-				.send ()
-				.previous (nano::dev::genesis->hash ())
-				.destination (nano::dev::genesis_key.pub)
-				.balance (nano::dev::constants.genesis_amount - nano::Knano_ratio)
-				.sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
-				.work (*system.work.generate (nano::dev::genesis->hash ()))
-				.build ();
-	// Add to election_winner_details. Use an unrealistic iteration so that it should fall into the else case and do a cleanup
-	node->active.add_election_winner_details (send->hash (), nullptr);
-	nano::election_status election;
-	election.winner = send;
-	node->process_confirmed (election, 1000000);
-	ASSERT_EQ (0, node->active.election_winner_details_size ());
-}
-
 TEST (ledger_confirm, pruned_source)
 {
 	nano::test::system system;

nano/lib/stats_enums.hpp

@@ -93,6 +93,7 @@ enum class type
 	message_processor,
 	message_processor_overfill,
 	message_processor_type,
+	process_confirmed,
 
 	_last // Must be the last enum
 };
@@ -134,6 +135,8 @@ enum class detail
 	cemented,
 	cooldown,
 	empty,
+	done,
+	retry,
 
 	// processing queue
 	queue,
@@ -249,6 +252,8 @@ enum class detail
 	generate_vote_final,
 	broadcast_block_initial,
 	broadcast_block_repeat,
+	confirm_once,
+	confirm_once_failed,
 
 	// election types
 	manual,
@@ -407,6 +412,7 @@ enum class detail
 	// active_elections
 	started,
 	stopped,
+	confirm_dependent,
 
 	// unchecked
 	put,

nano/node/active_elections.cpp

@@ -25,36 +25,24 @@ nano::active_elections::active_elections (nano::node & node_a, nano::confirming_
 	confirming_set{ confirming_set_a },
 	block_processor{ block_processor_a },
 	recently_confirmed{ config.confirmation_cache },
-	recently_cemented{ config.confirmation_history_size },
-	election_time_to_live{ node_a.network_params.network.is_dev_network () ? 0s : 2s }
+	recently_cemented{ config.confirmation_history_size }
 {
 	count_by_behavior.fill (0); // Zero initialize array
 
-	confirming_set.batch_cemented.add ([this] (nano::confirming_set::cemented_notification const & notification) {
+	confirming_set.batch_cemented.add ([this] (auto const & cemented) {
+		auto transaction = node.ledger.tx_begin_read ();
+		for (auto const & [block, confirmation_root, source_election] : cemented)
 		{
-			auto transaction = node.ledger.tx_begin_read ();
-			for (auto const & [block, confirmation_root] : notification.cemented)
-			{
-				transaction.refresh_if_needed ();
-
-				block_cemented_callback (transaction, block, confirmation_root);
-			}
-		}
-		for (auto const & hash : notification.already_cemented)
-		{
-			block_already_cemented_callback (hash);
+			transaction.refresh_if_needed ();
+			block_cemented (transaction, block, confirmation_root, source_election);
 		}
 	});
 
 	// Notify elections about alternative (forked) blocks
 	block_processor.block_processed.add ([this] (auto const & result, auto const & context) {
-		switch (result)
+		if (result == nano::block_status::fork)
 		{
-			case nano::block_status::fork:
-				publish (context.block);
-				break;
-			default:
-				break;
+			publish (context.block);
 		}
 	});
 }
@@ -91,41 +79,48 @@ void nano::active_elections::stop ()
 	clear ();
 }
 
-void nano::active_elections::block_cemented_callback (nano::secure::transaction const & transaction, std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root)
+void nano::active_elections::block_cemented (nano::secure::transaction const & transaction, std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root, std::shared_ptr<nano::election> const & source_election)
 {
 	debug_assert (node.block_confirmed (block->hash ()));
 
-	if (auto election_l = election (block->qualified_root ()))
+	// Dependent elections are implicitly confirmed when their block is cemented
+	auto dependend_election = election (block->qualified_root ());
+	if (dependend_election)
 	{
-		election_l->try_confirm (block->hash ());
+		node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::confirm_dependent);
+		dependend_election->try_confirm (block->hash ());
 	}
-	auto election = remove_election_winner_details (block->hash ());
+
 	nano::election_status status;
 	std::vector<nano::vote_with_weight_info> votes;
 	status.winner = block;
-	if (election)
-	{
-		status = election->get_status ();
-		votes = election->votes_with_weight ();
-	}
-	if (block->hash () == confirmation_root)
+
+	// Check if the currently cemented block was part of an election that triggered the confirmation
+	if (source_election && source_election->qualified_root == block->qualified_root ())
 	{
+		status = source_election->get_status ();
+		debug_assert (status.winner->hash () == block->hash ());
+		votes = source_election->votes_with_weight ();
 		status.type = nano::election_status_type::active_confirmed_quorum;
 	}
-	else if (election)
+	else if (dependend_election)
 	{
 		status.type = nano::election_status_type::active_confirmation_height;
 	}
 	else
 	{
 		status.type = nano::election_status_type::inactive_confirmation_height;
 	}
+
 	recently_cemented.put (status);
 
 	node.stats.inc (nano::stat::type::active_elections, nano::stat::detail::cemented);
 	node.stats.inc (nano::stat::type::active_elections_cemented, to_stat_detail (status.type));
 
-	node.logger.trace (nano::log::type::active_elections, nano::log::detail::active_cemented, nano::log::arg{ "election", election });
+	node.logger.trace (nano::log::type::active_elections, nano::log::detail::active_cemented,
+	nano::log::arg{ "block", block },
+	nano::log::arg{ "confirmation_root", confirmation_root },
+	nano::log::arg{ "source_election", source_election });
 
 	notify_observers (transaction, status, votes);
 
@@ -185,39 +180,6 @@ void nano::active_elections::activate_successors (nano::secure::transaction cons
 	}
 }
 
-void nano::active_elections::add_election_winner_details (nano::block_hash const & hash_a, std::shared_ptr<nano::election> const & election_a)
-{
-	nano::lock_guard<nano::mutex> guard{ election_winner_details_mutex };
-	election_winner_details.emplace (hash_a, election_a);
-}
-
-std::shared_ptr<nano::election> nano::active_elections::remove_election_winner_details (nano::block_hash const & hash_a)
-{
-	std::shared_ptr<nano::election> result;
-	{
-		nano::lock_guard<nano::mutex> guard{ election_winner_details_mutex };
-		auto existing = election_winner_details.find (hash_a);
-		if (existing != election_winner_details.end ())
-		{
-			result = existing->second;
-			election_winner_details.erase (existing);
-		}
-	}
-
-	vacancy_update ();
-
-	return result;
-}
-
-void nano::active_elections::block_already_cemented_callback (nano::block_hash const & hash_a)
-{
-	// Depending on timing there is a situation where the election_winner_details is not reset.
-	// This can happen when a block wins an election, and the block is confirmed + observer
-	// called before the block hash gets added to election_winner_details. If the block is confirmed
-	// callbacks have already been done, so we can safely just remove it.
-	remove_election_winner_details (hash_a);
-}
-
 int64_t nano::active_elections::limit (nano::election_behavior behavior) const
 {
 	switch (behavior)
@@ -265,7 +227,7 @@ int64_t nano::active_elections::vacancy (nano::election_behavior behavior) const
 	};
 
 	auto election_winners_vacancy = [this] () -> int64_t {
-		return static_cast<int64_t> (config.max_election_winners) - static_cast<int64_t> (election_winner_details_size ());
+		return static_cast<int64_t> (config.max_election_winners) - static_cast<int64_t> (confirming_set.size ());
 	};
 
 	return std::min (election_vacancy (behavior), election_winners_vacancy ());
@@ -572,12 +534,6 @@ bool nano::active_elections::publish (std::shared_ptr<nano::block> const & block
 	return result;
 }
 
-std::size_t nano::active_elections::election_winner_details_size () const
-{
-	nano::lock_guard<nano::mutex> guard{ election_winner_details_mutex };
-	return election_winner_details.size ();
-}
-
 void nano::active_elections::clear ()
 {
 	// TODO: Call erased_callback for each election
@@ -595,7 +551,6 @@ nano::container_info nano::active_elections::container_info () const
 
 	nano::container_info info;
 	info.put ("roots", roots.size ());
-	info.put ("election_winner_details", election_winner_details_size ());
 	info.put ("normal", static_cast<std::size_t> (count_by_behavior[nano::election_behavior::priority]));
 	info.put ("hinted", static_cast<std::size_t> (count_by_behavior[nano::election_behavior::hinted]));
 	info.put ("optimistic", static_cast<std::size_t> (count_by_behavior[nano::election_behavior::optimistic]));

nano/node/active_elections.hpp

@@ -123,10 +123,6 @@ class active_elections final
 	int64_t vacancy (nano::election_behavior behavior) const;
 	std::function<void ()> vacancy_update{ [] () {} };
 
-	std::size_t election_winner_details_size () const;
-	void add_election_winner_details (nano::block_hash const &, std::shared_ptr<nano::election> const &);
-	std::shared_ptr<nano::election> remove_election_winner_details (nano::block_hash const &);
-
 	nano::container_info container_info () const;
 
 private:
@@ -139,8 +135,7 @@ class active_elections final
 	std::vector<std::shared_ptr<nano::election>> list_active_impl (std::size_t) const;
 	void activate_successors (nano::secure::transaction const &, std::shared_ptr<nano::block> const & block);
 	void notify_observers (nano::secure::transaction const &, nano::election_status const & status, std::vector<nano::vote_with_weight_info> const & votes) const;
-	void block_cemented_callback (nano::secure::transaction const &, std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root);
-	void block_already_cemented_callback (nano::block_hash const & hash);
+	void block_cemented (nano::secure::transaction const &, std::shared_ptr<nano::block> const & block, nano::block_hash const & confirmation_root, std::shared_ptr<nano::election> const & source_election);
 
 private: // Dependencies
 	active_elections_config const & config;
@@ -157,12 +152,6 @@ class active_elections final
 	mutable nano::mutex mutex{ mutex_identifier (mutexes::active) };
 
 private:
-	mutable nano::mutex election_winner_details_mutex{ mutex_identifier (mutexes::election_winner_details) };
-	std::unordered_map<nano::block_hash, std::shared_ptr<nano::election>> election_winner_details;
-
-	// Maximum time an election can be kept active if it is extending the container
-	std::chrono::seconds const election_time_to_live;
-
 	/** Keeps track of number of elections by election behavior (normal, hinted, optimistic) */
 	nano::enum_array<nano::election_behavior, int64_t> count_by_behavior{};