Limit failures for confirmed lazy bootstrap bulk pull (#3068)

* Limit failures for confirmed lazy bootstrap bulk pull

Considering that node always can start new attempt and cached votes don't include root to find out flipped votes

* Improve peer count calculation conditions

* Lower lazy bootstrap retry limit

* Lower lazy bootstrap retry limit

nano/core_test/bootstrap.cpp

@@ -969,6 +969,32 @@ TEST (bootstrap_processor, lazy_pruning_missing_block)
 	node2->stop ();
 }
 
+TEST (bootstrap_processor, lazy_cancel)
+{
+	nano::system system;
+	nano::node_config config (nano::get_available_port (), system.logging);
+	config.frontiers_confirmation = nano::frontiers_confirmation_mode::disabled;
+	nano::node_flags node_flags;
+	node_flags.disable_bootstrap_bulk_push_client = true;
+	auto node0 (system.add_node (config, node_flags));
+	nano::genesis genesis;
+	nano::keypair key1;
+	// Generating test chain
+	auto send1 (std::make_shared<nano::state_block> (nano::dev_genesis_key.pub, genesis.hash (), nano::dev_genesis_key.pub, nano::genesis_amount - nano::Gxrb_ratio, key1.pub, nano::dev_genesis_key.prv, nano::dev_genesis_key.pub, *node0->work_generate_blocking (genesis.hash ())));
+	// Start lazy bootstrap with last block in chain known
+	auto node1 (std::make_shared<nano::node> (system.io_ctx, nano::get_available_port (), nano::unique_path (), system.alarm, system.logging, system.work));
+	node1->network.udp_channels.insert (node0->network.endpoint (), node1->network_params.protocol.protocol_version);
+	node1->bootstrap_initiator.bootstrap_lazy (send1->hash (), true); // Start "confirmed" block bootstrap
+	{
+		auto lazy_attempt (node1->bootstrap_initiator.current_lazy_attempt ());
+		ASSERT_NE (nullptr, lazy_attempt);
+		ASSERT_EQ (send1->hash ().to_string (), lazy_attempt->id);
+	}
+	// Cancel failing lazy bootstrap
+	ASSERT_TIMELY (10s, !node1->bootstrap_initiator.in_progress ());
+	node1->stop ();
+}
+
 TEST (bootstrap_processor, wallet_lazy_frontier)
 {
 	nano::system system;

nano/node/bootstrap/bootstrap_bulk_pull.cpp

@@ -55,7 +55,7 @@ nano::bulk_pull_client::~bulk_pull_client ()
 
 void nano::bulk_pull_client::request ()
 {
-	debug_assert (!pull.head.is_zero () || pull.retry_limit != std::numeric_limits<unsigned>::max ());
+	debug_assert (!pull.head.is_zero () || pull.retry_limit <= connection->node->network_params.bootstrap.lazy_retry_limit);
 	expected = pull.head;
 	nano::bulk_pull req;
 	if (pull.head == pull.head_original && pull.attempts % 4 < 3)
@@ -219,7 +219,7 @@ void nano::bulk_pull_client::received_block (boost::system::error_code const & e
 			// Is block expected?
 			bool block_expected (false);
 			// Unconfirmed head is used only for lazy destinations if legacy bootstrap is not available, see nano::bootstrap_attempt::lazy_destinations_increment (...)
-			bool unconfirmed_account_head (connection->node->flags.disable_legacy_bootstrap && pull_blocks == 0 && pull.retry_limit != std::numeric_limits<unsigned>::max () && expected == pull.account_or_head && block->account () == pull.account_or_head);
+			bool unconfirmed_account_head (connection->node->flags.disable_legacy_bootstrap && pull_blocks == 0 && pull.retry_limit <= connection->node->network_params.bootstrap.lazy_retry_limit && expected == pull.account_or_head && block->account () == pull.account_or_head);
 			if (hash == expected || unconfirmed_account_head)
 			{
 				expected = block->previous ();

nano/node/bootstrap/bootstrap_connections.cpp

@@ -404,7 +404,7 @@ void nano::bootstrap_connections::requeue_pull (nano::pull_info const & pull_a,
 			attempt_l->pull_started ();
 			condition.notify_all ();
 		}
-		else if (attempt_l->mode == nano::bootstrap_mode::lazy && (pull.retry_limit == std::numeric_limits<unsigned>::max () || pull.attempts <= pull.retry_limit + (pull.processed / node.network_params.bootstrap.lazy_max_pull_blocks)))
+		else if (attempt_l->mode == nano::bootstrap_mode::lazy && (pull.attempts <= pull.retry_limit + (pull.processed / node.network_params.bootstrap.lazy_max_pull_blocks)))
 		{
 			debug_assert (pull.account_or_head == pull.head);
 			if (!attempt_l->lazy_processed_or_exists (pull.account_or_head.as_block_hash ()))

nano/node/bootstrap/bootstrap_lazy.cpp

@@ -34,7 +34,7 @@ void nano::bootstrap_attempt_lazy::lazy_start (nano::hash_or_account const & has
 	if (lazy_keys.size () < max_keys && lazy_keys.find (hash_or_account_a.as_block_hash ()) == lazy_keys.end () && !lazy_blocks_processed (hash_or_account_a.as_block_hash ()))
 	{
 		lazy_keys.insert (hash_or_account_a.as_block_hash ());
-		lazy_pulls.emplace_back (hash_or_account_a, confirmed ? std::numeric_limits<unsigned>::max () : node->network_params.bootstrap.lazy_retry_limit);
+		lazy_pulls.emplace_back (hash_or_account_a, confirmed ? lazy_retry_limit_confirmed () : node->network_params.bootstrap.lazy_retry_limit);
 		lock.unlock ();
 		condition.notify_all ();
 	}
@@ -65,7 +65,7 @@ void nano::bootstrap_attempt_lazy::lazy_requeue (nano::block_hash const & hash_a
 	{
 		lazy_blocks_erase (hash_a);
 		lock.unlock ();
-		node->bootstrap_initiator.connections->requeue_pull (nano::pull_info (hash_a, hash_a, previous_a, incremental_id, static_cast<nano::pull_info::count_t> (1), confirmed_a ? std::numeric_limits<unsigned>::max () : node->network_params.bootstrap.lazy_destinations_retry_limit));
+		node->bootstrap_initiator.connections->requeue_pull (nano::pull_info (hash_a, hash_a, previous_a, incremental_id, static_cast<nano::pull_info::count_t> (1), confirmed_a ? lazy_retry_limit_confirmed () : node->network_params.bootstrap.lazy_destinations_retry_limit));
 	}
 }
 
@@ -282,7 +282,7 @@ bool nano::bootstrap_attempt_lazy::process_block_lazy (std::shared_ptr<nano::blo
 		}
 		lazy_block_state_backlog_check (block_a, hash);
 		lock.unlock ();
-		nano::unchecked_info info (block_a, known_account_a, 0, nano::signature_verification::unknown, retry_limit == std::numeric_limits<unsigned>::max ());
+		nano::unchecked_info info (block_a, known_account_a, 0, nano::signature_verification::unknown, retry_limit > node->network_params.bootstrap.lazy_retry_limit);
 		node->block_processor.add (info);
 	}
 	// Force drop lazy bootstrap connection for long bulk_pull
@@ -504,6 +504,18 @@ bool nano::bootstrap_attempt_lazy::lazy_processed_or_exists (nano::block_hash co
 	return result;
 }
 
+unsigned nano::bootstrap_attempt_lazy::lazy_retry_limit_confirmed ()
+{
+	debug_assert (!mutex.try_lock ());
+	if (total_blocks % 1024 == 512 || peer_count == 0)
+	{
+		// Prevent too frequent network locks
+		peer_count = node->network.size ();
+	}
+	auto multiplier (node->flags.disable_legacy_bootstrap ? 2 : 1.25);
+	return multiplier * std::max (node->network_params.bootstrap.lazy_retry_limit, 2 * nano::narrow_cast<unsigned> (peer_count));
+}
+
 void nano::bootstrap_attempt_lazy::get_information (boost::property_tree::ptree & tree_a)
 {
 	nano::lock_guard<std::mutex> lock (mutex);

nano/node/bootstrap/bootstrap_lazy.hpp

@@ -37,7 +37,7 @@ public:
 	bool process_block (std::shared_ptr<nano::block>, nano::account const &, uint64_t, nano::bulk_pull::count_t, bool, unsigned) override;
 	void run () override;
 	void lazy_start (nano::hash_or_account const &, bool confirmed = true) override;
-	void lazy_add (nano::hash_or_account const &, unsigned = std::numeric_limits<unsigned>::max ());
+	void lazy_add (nano::hash_or_account const &, unsigned);
 	void lazy_add (nano::pull_info const &) override;
 	void lazy_requeue (nano::block_hash const &, nano::block_hash const &, bool) override;
 	bool lazy_finished ();
@@ -54,6 +54,7 @@ public:
 	void lazy_blocks_erase (nano::block_hash const &);
 	bool lazy_blocks_processed (nano::block_hash const &);
 	bool lazy_processed_or_exists (nano::block_hash const &) override;
+	unsigned lazy_retry_limit_confirmed ();
 	void get_information (boost::property_tree::ptree &) override;
 	std::unordered_set<size_t> lazy_blocks;
 	std::unordered_map<nano::block_hash, nano::lazy_state_backlog_item> lazy_state_backlog;
@@ -79,6 +80,7 @@ public:
 	lazy_destinations;
 	// clang-format on
 	std::atomic<size_t> lazy_blocks_count{ 0 };
+	size_t peer_count{ 0 };
 	std::atomic<bool> lazy_destinations_flushed{ false };
 	/** The maximum number of records to be read in while iterating over long lazy containers */
 	static uint64_t constexpr batch_read_size = 256;

nano/secure/common.cpp

@@ -172,7 +172,7 @@ nano::bootstrap_constants::bootstrap_constants (nano::network_constants & networ
 	lazy_max_pull_blocks = network_constants.is_dev_network () ? 2 : 512;
 	lazy_min_pull_blocks = network_constants.is_dev_network () ? 1 : 32;
 	frontier_retry_limit = network_constants.is_dev_network () ? 2 : 16;
-	lazy_retry_limit = network_constants.is_dev_network () ? 2 : frontier_retry_limit * 10;
+	lazy_retry_limit = network_constants.is_dev_network () ? 2 : frontier_retry_limit * 4;
 	lazy_destinations_retry_limit = network_constants.is_dev_network () ? 1 : frontier_retry_limit / 4;
 	gap_cache_bootstrap_start_interval = network_constants.is_dev_network () ? std::chrono::milliseconds (5) : std::chrono::milliseconds (30 * 1000);
 }