Decay bootstrap blocking set entries (#4844)

nano/core_test/bootstrap.cpp

@@ -178,6 +178,83 @@ TEST (account_sets, saturate_priority)
 	ASSERT_EQ (sets.priority (account), nano::bootstrap::account_sets::priority_max);
 }
 
+TEST (account_sets, decay_blocking)
+{
+	using namespace std::chrono_literals;
+
+	nano::test::system system;
+	nano::account_sets_config config;
+	config.blocking_decay = 1s;
+	nano::bootstrap::account_sets sets{ config, system.stats };
+
+	// Test empty set
+	ASSERT_EQ (0, sets.decay_blocking ());
+
+	// Create test accounts and timestamps
+	nano::account account1{ 1 };
+	nano::account account2{ 2 };
+	nano::account account3{ 3 };
+
+	auto now = std::chrono::steady_clock::now ();
+
+	// Add first account
+	sets.priority_up (account1);
+	sets.block (account1, random_hash (), now);
+	ASSERT_TRUE (sets.blocked (account1));
+	ASSERT_EQ (1, sets.blocked_size ());
+
+	// Decay before timeout should not remove entry
+	ASSERT_EQ (0, sets.decay_blocking (now));
+	ASSERT_TRUE (sets.blocked (account1));
+	ASSERT_EQ (1, sets.blocked_size ());
+
+	// Add second account after 500ms
+	now += 500ms;
+	sets.priority_up (account2);
+	sets.block (account2, random_hash (), now);
+	ASSERT_TRUE (sets.blocked (account2));
+	ASSERT_EQ (2, sets.blocked_size ());
+
+	// Add third account after another 500ms
+	now += 500ms;
+	sets.priority_up (account3);
+	sets.block (account3, random_hash (), now);
+	ASSERT_TRUE (sets.blocked (account3));
+	ASSERT_EQ (3, sets.blocked_size ());
+
+	// Decay at 1.5s - should remove first two accounts
+	now += 500ms;
+	ASSERT_EQ (2, sets.decay_blocking (now));
+	ASSERT_FALSE (sets.blocked (account1));
+	ASSERT_FALSE (sets.blocked (account2));
+	ASSERT_TRUE (sets.blocked (account3));
+	ASSERT_EQ (1, sets.blocked_size ());
+
+	// Reinsert second account
+	auto hash2 = random_hash ();
+	sets.priority_up (account2);
+	sets.block (account2, hash2, now);
+	ASSERT_TRUE (sets.blocked (account2));
+	ASSERT_EQ (2, sets.blocked_size ());
+
+	// Immediate decay should not affect reinserted account
+	ASSERT_EQ (0, sets.decay_blocking (now));
+	ASSERT_TRUE (sets.blocked (account2));
+
+	// Decay at 2s - should remove account3 but keep reinserted account2
+	now += 500ms;
+	ASSERT_EQ (1, sets.decay_blocking (now));
+	ASSERT_FALSE (sets.blocked (account3));
+	ASSERT_TRUE (sets.blocked (account2));
+	ASSERT_EQ (1, sets.blocked_size ());
+
+	// Final decay after another second - should remove remaining account
+	now += 1s;
+	ASSERT_EQ (1, sets.decay_blocking (now));
+	ASSERT_FALSE (sets.blocked (account2));
+	ASSERT_EQ (0, sets.blocked_size ());
+}
+
 /*
  * bootstrap
  */

nano/lib/stats_enums.hpp

@@ -549,6 +549,8 @@ enum class detail
 	deprioritize,
 	deprioritize_failed,
 	sync_dependencies,
+	decay_blocking,
+	blocking_decayed,
 	dependency_synced,
 
 	request_blocks,

nano/node/bootstrap/account_sets.cpp

@@ -117,7 +117,7 @@ void nano::bootstrap::account_sets::priority_erase (nano::account const & accoun
 	}
 }
 
-void nano::bootstrap::account_sets::block (nano::account const & account, nano::block_hash const & dependency)
+void nano::bootstrap::account_sets::block (nano::account const & account, nano::block_hash const & dependency, std::chrono::steady_clock::time_point now)
 {
 	debug_assert (!account.is_zero ());
 
@@ -128,7 +128,7 @@ void nano::bootstrap::account_sets::block (nano::account const & account, nano::
 		stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::block);
 
 		debug_assert (blocking.get<tag_account> ().count (account) == 0);
-		blocking.get<tag_account> ().insert ({ account, dependency });
+		blocking.get<tag_account> ().insert ({ account, dependency, now });
 		trim_overflow ();
 	}
 	else
@@ -311,6 +311,32 @@ void nano::bootstrap::account_sets::sync_dependencies ()
 	trim_overflow ();
 }
 
+size_t nano::bootstrap::account_sets::decay_blocking (std::chrono::steady_clock::time_point now)
+{
+	stats.inc (nano::stat::type::bootstrap_account_sets, nano::stat::detail::decay_blocking);
+
+	auto const cutoff = now - config.blocking_decay;
+
+	// Erase all entries that are older than the cutoff
+	size_t result = 0;
+	for (auto it = blocking.get<tag_timestamp> ().begin (); it != blocking.get<tag_timestamp> ().end ();)
+	{
+		if (it->timestamp <= cutoff)
+		{
+			it = blocking.get<tag_timestamp> ().erase (it);
+			++result;
+		}
+		else
+		{
+			break; // Entries are sorted by timestamp, no need to continue
+		}
+	}
+
+	stats.add (nano::stat::type::bootstrap_account_sets, nano::stat::detail::blocking_decayed, result);
+
+	return result;
+}
+
 bool nano::bootstrap::account_sets::blocked (nano::account const & account) const
 {
 	return blocking.get<tag_account> ().contains (account);

nano/node/bootstrap/account_sets.hpp

@@ -13,6 +13,7 @@
 #include <boost/multi_index/sequenced_index.hpp>
 #include <boost/multi_index_container.hpp>
 
+#include <chrono>
 #include <random>
 
 namespace mi = boost::multi_index;
@@ -38,7 +39,7 @@ class account_sets
 	void priority_set (nano::account const & account, double priority = priority_initial);
 	void priority_erase (nano::account const & account);
 
-	void block (nano::account const & account, nano::block_hash const & dependency);
+	void block (nano::account const & account, nano::block_hash const & dependency, std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now ());
 	void unblock (nano::account const & account, std::optional<nano::block_hash> const & hash = std::nullopt);
 
 	void timestamp_set (nano::account const & account);
@@ -54,6 +55,11 @@ class account_sets
 	 */
 	void sync_dependencies ();
 
+	/**
+	 * Should be called periodically to remove old entries from the blocking set
+	 */
+	size_t decay_blocking (std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now ());
+
 	struct priority_result
 	{
 		nano::account account;
@@ -92,16 +98,19 @@ class account_sets
 	{
 		nano::account account;
 		double priority;
+
 		unsigned fails{ 0 };
-		std::chrono::steady_clock::time_point timestamp{};
+		std::chrono::steady_clock::time_point timestamp{}; // Use for cooldown, set to current time when this account is sampled
 		id_t id{ generate_id () }; // Uniformly distributed, used for random querying
 	};
 
 	struct blocking_entry
 	{
 		nano::account account;
 		nano::block_hash dependency;
-		nano::account dependency_account{ 0 };
+		std::chrono::steady_clock::time_point timestamp; // Used for decaying old entries
+
+		nano::account dependency_account{ 0 }; // Account that contains the dependency block, fetched via a background dependency walker
 		id_t id{ generate_id () }; // Uniformly distributed, used for random querying
 	};
 
@@ -112,6 +121,7 @@ class account_sets
 	class tag_dependency {};
 	class tag_dependency_account {};
 	class tag_priority {};
+	class tag_timestamp {};
 
 	// Tracks the ongoing account priorities
 	using ordered_priorities = boost::multi_index_container<priority_entry,
@@ -137,7 +147,9 @@ class account_sets
 		mi::ordered_non_unique<mi::tag<tag_dependency_account>,
 			mi::member<blocking_entry, nano::account, &blocking_entry::dependency_account>>,
 		mi::ordered_unique<mi::tag<tag_id>,
-			mi::member<blocking_entry, id_t, &blocking_entry::id>>
+			mi::member<blocking_entry, id_t, &blocking_entry::id>>,
+		mi::ordered_non_unique<mi::tag<tag_timestamp>,
+			mi::member<blocking_entry, std::chrono::steady_clock::time_point, &blocking_entry::timestamp>>
 	>>;
 	// clang-format on
 

nano/node/bootstrap/bootstrap_config.cpp

@@ -11,6 +11,7 @@ nano::error nano::account_sets_config::deserialize (nano::tomlconfig & toml)
 	toml.get ("priorities_max", priorities_max);
 	toml.get ("blocking_max", blocking_max);
 	toml.get_duration ("cooldown", cooldown);
+	toml.get_duration ("blocking_decay", blocking_decay);
 
 	return toml.get_error ();
 }
@@ -21,6 +22,7 @@ nano::error nano::account_sets_config::serialize (nano::tomlconfig & toml) const
 	toml.put ("priorities_max", priorities_max, "Cutoff size limit for the priority list.\ntype:uint64");
 	toml.put ("blocking_max", blocking_max, "Cutoff size limit for the blocked accounts from the priority list.\ntype:uint64");
 	toml.put ("cooldown", cooldown.count (), "Waiting time for an account to become available.\ntype:milliseconds");
+	toml.put ("blocking_decay", blocking_decay.count (), "Time to wait before removing an account from the blocked list.\ntype:seconds");
 
 	return toml.get_error ();
 }

nano/node/bootstrap/bootstrap_config.hpp

@@ -4,6 +4,8 @@
 #include <nano/lib/timer.hpp>
 #include <nano/node/bootstrap/bootstrap_server.hpp>
 
+using namespace std::chrono_literals;
+
 namespace nano
 {
 class tomlconfig;
@@ -19,6 +21,7 @@ class account_sets_config final
 	std::size_t priorities_max{ 256 * 1024 };
 	std::size_t blocking_max{ 256 * 1024 };
 	std::chrono::milliseconds cooldown{ 1000 * 3 };
+	std::chrono::seconds blocking_decay{ 15min };
 };
 
 class frontier_scan_config final

nano/node/bootstrap/bootstrap_service.cpp

@@ -757,11 +757,14 @@ void nano::bootstrap_service::cleanup_and_sync ()
 
 	throttle.resize (compute_throttle_size ());
 
+	accounts.decay_blocking ();
+
 	auto const now = std::chrono::steady_clock::now ();
 	auto should_timeout = [&] (async_tag const & tag) {
 		return tag.cutoff < now;
 	};
 
+	// Erase timed out requests
 	auto & tags_by_order = tags.get<tag_sequenced> ();
 	while (!tags_by_order.empty () && should_timeout (tags_by_order.front ()))
 	{
@@ -771,7 +774,8 @@ void nano::bootstrap_service::cleanup_and_sync ()
 		tags_by_order.pop_front ();
 	}
 
-	if (sync_dependencies_interval.elapse (60s))
+	// Reinsert known dependencies into the priority set
+	if (sync_dependencies_interval.elapse (nano::is_dev_run () ? 1s : 60s))
 	{
 		stats.inc (nano::stat::type::bootstrap, nano::stat::detail::sync_dependencies);
 		accounts.sync_dependencies ();