Bisected election backtracking (#2778)

* Add ledger::backtrack limited to 128 jumps

* Bisect election dependencies when activating

* Addressing special case where the winner may not have a loaded sideband

* Simplify ledger::backtrack by specifying desired number of jumps instead. Limit is placed on the caller

* Perform DB transactions without holding the active mutex by batch processing

* Add a debug_assert on getting the previous block since it happens in the same tx

* Add a pessimistic fallback by also starting an election for the first unconfirmed block

* Set empty sideband, required by recent change (thanks Serg!)

* Check if first unconfirmed block is not being processed by conf height

* Already in active_transactions

* Use std::min<uint64_t> to avoid overflow (Wes review)

* Use owns_lock from mutex (Wes)

nano/core_test/confirmation_solicitor.cpp

@@ -29,6 +29,7 @@ TEST (confirmation_solicitor, batches)
 	ASSERT_EQ (nano::test_genesis_key.pub, representatives.front ().account);
 	ASSERT_TIMELY (3s, node2.network.size () == 1);
 	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)));
+	send->sideband_set ({});
 	{
 		nano::lock_guard<std::mutex> guard (node2.active.mutex);
 		for (size_t i (0); i < nano::network::confirm_req_hashes_max; ++i)
@@ -71,6 +72,7 @@ TEST (confirmation_solicitor, different_hash)
 	ASSERT_EQ (nano::test_genesis_key.pub, representatives.front ().account);
 	ASSERT_TIMELY (3s, node2.network.size () == 1);
 	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)));
+	send->sideband_set ({});
 	{
 		nano::lock_guard<std::mutex> guard (node2.active.mutex);
 		auto election (std::make_shared<nano::election> (node2, send, nullptr, false));

nano/core_test/election.cpp

@@ -17,3 +17,82 @@ TEST (election, construction)
 	election->transition_passive ();
 	ASSERT_FALSE (election->idle ());
 }
+
+namespace nano
+{
+TEST (election, bisect_dependencies)
+{
+	nano::system system;
+	nano::node_flags flags;
+	flags.disable_request_loop = true;
+	auto & node = *system.add_node (flags);
+	nano::genesis genesis;
+	nano::confirmation_height_info conf_info;
+	ASSERT_FALSE (node.store.confirmation_height_get (node.store.tx_begin_read (), nano::test_genesis_key.pub, conf_info));
+	ASSERT_EQ (1, conf_info.height);
+	std::vector<std::shared_ptr<nano::block>> blocks;
+	blocks.push_back (nullptr); // idx == height
+	blocks.push_back (genesis.open);
+	nano::block_builder builder;
+	auto amount = nano::genesis_amount;
+	for (int i = 0; i < 299; ++i)
+	{
+		auto latest = blocks.back ();
+		blocks.push_back (builder.state ()
+		                  .previous (latest->hash ())
+		                  .account (nano::test_genesis_key.pub)
+		                  .representative (nano::test_genesis_key.pub)
+		                  .balance (--amount)
+		                  .link (nano::test_genesis_key.pub)
+		                  .sign (nano::test_genesis_key.prv, nano::test_genesis_key.pub)
+		                  .work (*system.work.generate (latest->hash ()))
+		                  .build ());
+		ASSERT_EQ (nano::process_result::progress, node.process (*blocks.back ()).code);
+	}
+	ASSERT_EQ (301, blocks.size ());
+	ASSERT_TRUE (node.active.empty ());
+	{
+		auto election = node.active.insert (blocks.back ()).election;
+		ASSERT_NE (nullptr, election);
+		ASSERT_EQ (300, election->blocks.begin ()->second->sideband ().height);
+		nano::unique_lock<std::mutex> lock (node.active.mutex);
+		election->activate_dependencies ();
+		node.active.activate_dependencies (lock);
+	}
+	// The first dependency activation also starts an election for the first unconfirmed block
+	ASSERT_EQ (3, node.active.size ());
+	{
+		auto election = node.active.election (blocks[2]->qualified_root ());
+		ASSERT_NE (nullptr, election);
+		ASSERT_EQ (2, election->blocks.begin ()->second->sideband ().height);
+	}
+
+	auto check_height_and_activate_next = [&node, &blocks](uint64_t height_a) {
+		auto election = node.active.election (blocks[height_a]->qualified_root ());
+		ASSERT_NE (nullptr, election);
+		ASSERT_EQ (height_a, election->blocks.begin ()->second->sideband ().height);
+		nano::unique_lock<std::mutex> lock (node.active.mutex);
+		election->activate_dependencies ();
+		node.active.activate_dependencies (lock);
+	};
+	check_height_and_activate_next (300 - 128); // ensure limited by 128 jumps
+	ASSERT_EQ (4, node.active.size ());
+	check_height_and_activate_next (87);
+	ASSERT_EQ (5, node.active.size ());
+	check_height_and_activate_next (44);
+	ASSERT_EQ (6, node.active.size ());
+	check_height_and_activate_next (23);
+	ASSERT_EQ (7, node.active.size ());
+	check_height_and_activate_next (12);
+	ASSERT_EQ (8, node.active.size ());
+	check_height_and_activate_next (7);
+	ASSERT_EQ (9, node.active.size ());
+	check_height_and_activate_next (4);
+	ASSERT_EQ (10, node.active.size ());
+	check_height_and_activate_next (3);
+	ASSERT_EQ (10, node.active.size ()); // height 2 already inserted initially, no more blocks to activate
+	check_height_and_activate_next (2);
+	ASSERT_EQ (10, node.active.size ()); // conf height is 1, no more blocks to activate
+	ASSERT_EQ (node.active.blocks.size (), node.active.roots.size ());
+}
+}

nano/core_test/ledger.cpp

@@ -3132,3 +3132,54 @@ TEST (ledger, epoch_2_upgrade_callback)
 	auto latest = upgrade_epoch (pool, ledger, nano::epoch::epoch_2);
 	ASSERT_TRUE (cb_hit);
 }
+
+TEST (ledger, backtrack)
+{
+	nano::genesis genesis;
+	nano::stat stats;
+	nano::logger_mt logger;
+	auto store = nano::make_store (logger, nano::unique_path ());
+	ASSERT_TRUE (!store->init_error ());
+	bool cb_hit = false;
+	nano::ledger ledger (*store, stats, nano::generate_cache (), [&cb_hit]() {
+		cb_hit = true;
+	});
+	{
+		auto transaction (store->tx_begin_write ());
+		store->initialize (transaction, genesis, ledger.cache);
+	}
+	nano::work_pool pool (std::numeric_limits<unsigned>::max ());
+	std::vector<std::shared_ptr<nano::block>> blocks;
+	blocks.push_back (nullptr); // idx == height
+	blocks.push_back (genesis.open);
+	auto amount = nano::genesis_amount;
+	for (auto i = 0; i < 300; ++i)
+	{
+		nano::block_builder builder;
+		std::error_code ec;
+		auto latest = blocks.back ();
+		blocks.push_back (builder.state ()
+		                  .previous (latest->hash ())
+		                  .account (nano::test_genesis_key.pub)
+		                  .representative (nano::test_genesis_key.pub)
+		                  .balance (--amount)
+		                  .link (nano::test_genesis_key.pub)
+		                  .sign (nano::test_genesis_key.prv, nano::test_genesis_key.pub)
+		                  .work (*pool.generate (latest->hash ()))
+		                  .build (ec));
+		ASSERT_FALSE (ec);
+		ASSERT_EQ (nano::process_result::progress, ledger.process (store->tx_begin_write (), *blocks.back ()).code);
+	}
+	ASSERT_EQ (302, blocks.size ());
+	ASSERT_EQ (301, blocks[301]->sideband ().height);
+	auto transaction (store->tx_begin_read ());
+	auto block_100 = ledger.backtrack (transaction, blocks[300], 200);
+	ASSERT_NE (nullptr, block_100);
+	ASSERT_EQ (*block_100, *blocks[100]);
+	ASSERT_NE (nullptr, ledger.backtrack (transaction, blocks[10], 10));
+	ASSERT_NE (ledger.backtrack (transaction, blocks[10], 1), ledger.backtrack (transaction, blocks[11], 2));
+	ASSERT_EQ (ledger.backtrack (transaction, blocks[1], 0), ledger.backtrack (transaction, blocks[1], 1));
+	ASSERT_NE (ledger.backtrack (transaction, blocks[2], 0), ledger.backtrack (transaction, blocks[2], 1));
+	ASSERT_EQ (nullptr, ledger.backtrack (transaction, nullptr, 0));
+	ASSERT_EQ (nullptr, ledger.backtrack (transaction, nullptr, 10));
+}

nano/node/active_transactions.cpp

@@ -256,6 +256,7 @@ void nano::active_transactions::request_confirm (nano::unique_lock<std::mutex> &
 	solicitor.flush ();
 	generator_session.flush ();
 	lock_a.lock ();
+	activate_dependencies (lock_a);
 
 	// This is updated after the loop to ensure slow machines don't do the full check often
 	if (check_all_elections_l)
@@ -294,6 +295,81 @@ void nano::active_transactions::frontiers_confirmation (nano::unique_lock<std::m
 	}
 }
 
+void nano::active_transactions::activate_dependencies (nano::unique_lock<std::mutex> & lock_a)
+{
+	debug_assert (lock_a.owns_lock ());
+	decltype (pending_dependencies) pending_l;
+	pending_l.swap (pending_dependencies);
+	lock_a.unlock ();
+	// Store blocks to activate when the lock is re-acquired, adding the hash of the original election as a dependency
+	std::vector<std::pair<std::shared_ptr<nano::block>, nano::block_hash>> activate_l;
+	{
+		auto transaction = node.store.tx_begin_read ();
+		for (auto const & entry_l : pending_l)
+		{
+			auto const & block_l (entry_l.first);
+			auto const height_l (entry_l.second);
+			bool escalated_l (false);
+			auto previous_hash_l (block_l->previous ());
+			if (!previous_hash_l.is_zero ())
+			{
+				/* Insert first unconfirmed block (pessimistic) and bisect the chain (likelihood) */
+				auto account (node.ledger.account (transaction, block_l->hash ()));
+				nano::confirmation_height_info conf_info_l;
+				if (!node.store.confirmation_height_get (transaction, account, conf_info_l))
+				{
+					if (height_l > conf_info_l.height + 1 && !conf_info_l.frontier.is_zero ())
+					{
+						auto successor_hash_l = node.store.block_successor (transaction, conf_info_l.frontier);
+						if (!confirmation_height_processor.is_processing_block (successor_hash_l))
+						{
+							auto successor_l = node.store.block_get (transaction, successor_hash_l);
+							debug_assert (successor_l != nullptr);
+							if (successor_l != nullptr)
+							{
+								activate_l.emplace_back (successor_l, block_l->hash ());
+							}
+						}
+					}
+					if (height_l > conf_info_l.height + 2)
+					{
+						auto const jumps_l = std::min<uint64_t> (128, (height_l - conf_info_l.height) / 2);
+						auto backtracked_l (node.ledger.backtrack (transaction, block_l, jumps_l));
+						if (backtracked_l != nullptr)
+						{
+							activate_l.emplace_back (backtracked_l, block_l->hash ());
+						}
+					}
+				}
+			}
+			/* If previous block not existing/not commited yet, block_source can cause segfault for state blocks
+				So source check can be done only if previous != nullptr or previous is 0 (open account) */
+			if (previous_hash_l.is_zero () || node.ledger.block_exists (previous_hash_l))
+			{
+				auto source_hash_l (node.ledger.block_source (transaction, *block_l));
+				if (!source_hash_l.is_zero () && source_hash_l != previous_hash_l && blocks.find (source_hash_l) == blocks.end ())
+				{
+					auto source_l (node.store.block_get (transaction, source_hash_l));
+					if (source_l != nullptr && !node.block_confirmed_or_being_confirmed (transaction, source_hash_l))
+					{
+						activate_l.emplace_back (source_l, block_l->hash ());
+					}
+				}
+			}
+		}
+	}
+	lock_a.lock ();
+	for (auto const & entry_l : activate_l)
+	{
+		auto election = insert_impl (entry_l.first);
+		if (election.inserted)
+		{
+			election.election->transition_active ();
+			election.election->dependent_blocks.insert (entry_l.second);
+		}
+	}
+}
+
 void nano::active_transactions::request_loop ()
 {
 	nano::unique_lock<std::mutex> lock (mutex);

nano/node/active_transactions.hpp

@@ -207,6 +207,8 @@ class active_transactions final
 	void frontiers_confirmation (nano::unique_lock<std::mutex> &);
 	nano::account next_frontier_account{ 0 };
 	std::chrono::steady_clock::time_point next_frontier_check{ std::chrono::steady_clock::now () };
+	void activate_dependencies (nano::unique_lock<std::mutex> &);
+	std::vector<std::pair<std::shared_ptr<nano::block>, uint64_t>> pending_dependencies;
 	nano::condition_variable condition;
 	bool started{ false };
 	std::atomic<bool> stopped{ false };
@@ -262,14 +264,17 @@ class active_transactions final
 	bool inactive_votes_bootstrap_check (std::vector<nano::account> const &, nano::block_hash const &, bool &);
 	boost::thread thread;
 
+	friend class election;
+	friend std::unique_ptr<container_info_component> collect_container_info (active_transactions &, const std::string &);
+
 	friend class active_transactions_dropped_cleanup_Test;
 	friend class active_transactions_vote_replays_Test;
 	friend class confirmation_height_prioritize_frontiers_Test;
 	friend class confirmation_height_prioritize_frontiers_overwrite_Test;
 	friend class active_transactions_confirmation_consistency_Test;
 	friend class active_transactions_vote_generator_session_Test;
 	friend class node_vote_by_hash_bundle_Test;
-	friend std::unique_ptr<container_info_component> collect_container_info (active_transactions &, const std::string &);
+	friend class election_bisect_dependencies_Test;
 };
 
 std::unique_ptr<container_info_component> collect_container_info (active_transactions & active_transactions, const std::string & name);

nano/node/election.cpp

@@ -27,7 +27,8 @@ nano::election::election (nano::node & node_a, std::shared_ptr<nano::block> bloc
 confirmation_action (confirmation_action_a),
 prioritized_m (prioritized_a),
 node (node_a),
-status ({ block_a, 0, std::chrono::duration_cast<std::chrono::milliseconds> (std::chrono::system_clock::now ().time_since_epoch ()), std::chrono::duration_values<std::chrono::milliseconds>::zero (), 0, 1, 0, nano::election_status_type::ongoing })
+status ({ block_a, 0, std::chrono::duration_cast<std::chrono::milliseconds> (std::chrono::system_clock::now ().time_since_epoch ()), std::chrono::duration_values<std::chrono::milliseconds>::zero (), 0, 1, 0, nano::election_status_type::ongoing }),
+height (block_a->sideband ().height)
 {
 	last_votes.emplace (node.network_params.random.not_an_account, nano::vote_info{ std::chrono::steady_clock::now (), 0, block_a->hash () });
 	blocks.emplace (block_a->hash (), block_a);
@@ -210,46 +211,8 @@ bool nano::election::confirmed () const
 
 void nano::election::activate_dependencies ()
 {
-	auto transaction = node.store.tx_begin_read ();
-	bool escalated_l (false);
-	std::shared_ptr<nano::block> previous_l;
-	auto previous_hash_l (status.winner->previous ());
-	if (!previous_hash_l.is_zero () && node.active.blocks.find (previous_hash_l) == node.active.blocks.end ())
-	{
-		previous_l = node.store.block_get (transaction, previous_hash_l);
-		if (previous_l != nullptr && !node.block_confirmed_or_being_confirmed (transaction, previous_hash_l))
-		{
-			auto election = node.active.insert_impl (previous_l);
-			if (election.inserted)
-			{
-				election.election->transition_active ();
-				escalated_l = true;
-			}
-		}
-	}
-	/* If previous block not existing/not commited yet, block_source can cause segfault for state blocks
-				So source check can be done only if previous != nullptr or previous is 0 (open account) */
-	if (previous_hash_l.is_zero () || previous_l != nullptr)
-	{
-		auto source_hash_l (node.ledger.block_source (transaction, *status.winner));
-		if (!source_hash_l.is_zero () && source_hash_l != previous_hash_l && node.active.blocks.find (source_hash_l) == node.active.blocks.end ())
-		{
-			auto source_l (node.store.block_get (transaction, source_hash_l));
-			if (source_l != nullptr && !node.block_confirmed_or_being_confirmed (transaction, source_hash_l))
-			{
-				auto election = node.active.insert_impl (source_l);
-				if (election.inserted)
-				{
-					election.election->transition_active ();
-					escalated_l = true;
-				}
-			}
-		}
-	}
-	if (escalated_l)
-	{
-		update_dependent ();
-	}
+	debug_assert (!node.active.mutex.try_lock ());
+	node.active.pending_dependencies.emplace_back (status.winner, height);
 }
 
 void nano::election::broadcast_block (nano::confirmation_solicitor & solicitor_a)
@@ -266,7 +229,7 @@ void nano::election::broadcast_block (nano::confirmation_solicitor & solicitor_a
 bool nano::election::transition_time (nano::confirmation_solicitor & solicitor_a)
 {
 	debug_assert (!node.active.mutex.try_lock ());
-	nano::unique_lock<std::mutex> lock (timepoints_mutex);
+	nano::lock_guard<std::mutex> guard (timepoints_mutex);
 	bool result = false;
 	switch (state_m)
 	{
@@ -293,9 +256,7 @@ bool nano::election::transition_time (nano::confirmation_solicitor & solicitor_a
 			if (base_latency () * active_broadcasting_duration_factor < std::chrono::steady_clock::now () - state_start)
 			{
 				state_change (nano::election::state_t::broadcasting, nano::election::state_t::backtracking);
-				lock.unlock ();
 				activate_dependencies ();
-				lock.lock ();
 			}
 			break;
 		case nano::election::state_t::backtracking:

nano/node/election.hpp

@@ -111,5 +111,8 @@ class election final : public std::enable_shared_from_this<nano::election>
 	std::unordered_map<nano::block_hash, nano::uint128_t> last_tally;
 	std::unordered_set<nano::block_hash> dependent_blocks;
 	std::chrono::seconds late_blocks_delay{ 5 };
+	uint64_t const height;
+
+	friend class election_bisect_dependencies_Test;
 };
 }

nano/secure/ledger.cpp

@@ -1178,6 +1178,19 @@ std::shared_ptr<nano::block> nano::ledger::forked_block (nano::transaction const
 	return result;
 }
 
+std::shared_ptr<nano::block> nano::ledger::backtrack (nano::transaction const & transaction_a, std::shared_ptr<nano::block> const & start_a, uint64_t jumps_a)
+{
+	auto block = start_a;
+	while (jumps_a > 0 && block != nullptr && !block->previous ().is_zero ())
+	{
+		block = store.block_get (transaction_a, block->previous ());
+		debug_assert (block != nullptr);
+		--jumps_a;
+	}
+	debug_assert (block == nullptr || block->previous ().is_zero () || jumps_a == 0);
+	return block;
+}
+
 bool nano::ledger::block_confirmed (nano::transaction const & transaction_a, nano::block_hash const & hash_a) const
 {
 	auto confirmed (false);

nano/secure/ledger.hpp

@@ -25,6 +25,7 @@ class ledger final
 	nano::uint128_t weight (nano::account const &);
 	std::shared_ptr<nano::block> successor (nano::transaction const &, nano::qualified_root const &);
 	std::shared_ptr<nano::block> forked_block (nano::transaction const &, nano::block const &);
+	std::shared_ptr<nano::block> backtrack (nano::transaction const &, std::shared_ptr<nano::block> const &, uint64_t);
 	bool block_confirmed (nano::transaction const & transaction_a, nano::block_hash const & hash_a) const;
 	bool block_not_confirmed_or_not_exists (nano::block const & block_a) const;
 	nano::block_hash latest (nano::transaction const &, nano::account const &);