Use chrono timestamps in ascending bootstrap code (#4687)

* Use unique ptr

* Use chrono timestamps

nano/core_test/bootstrap_ascending.cpp

@@ -264,9 +264,9 @@ TEST (bootstrap_ascending, config_serialization)
 	config1.requests_limit = 0x101;
 	config1.database_requests_limit = 0x102;
 	config1.pull_count = 0x103;
-	config1.timeout = 0x104;
+	config1.request_timeout = 0x104ms;
 	config1.throttle_coefficient = 0x105;
-	config1.throttle_wait = 0x106;
+	config1.throttle_wait = 0x106ms;
 	config1.block_wait_count = 0x107;
 	nano::tomlconfig toml1;
 	ASSERT_FALSE (config1.serialize (toml1));
@@ -281,7 +281,7 @@ TEST (bootstrap_ascending, config_serialization)
 	ASSERT_EQ (config1.requests_limit, config2.requests_limit);
 	ASSERT_EQ (config1.database_requests_limit, config2.database_requests_limit);
 	ASSERT_EQ (config1.pull_count, config2.pull_count);
-	ASSERT_EQ (config1.timeout, config2.timeout);
+	ASSERT_EQ (config1.request_timeout, config2.request_timeout);
 	ASSERT_EQ (config1.throttle_coefficient, config2.throttle_coefficient);
 	ASSERT_EQ (config1.throttle_wait, config2.throttle_wait);
 	ASSERT_EQ (config1.block_wait_count, config2.block_wait_count);

nano/lib/tomlconfig.hpp

@@ -143,6 +143,16 @@ public:
 		return *this;
 	}
 
+	/** Get chrono duration */
+	template <typename Duration>
+	tomlconfig & get_duration (std::string const & key, Duration & target)
+	{
+		uint64_t value;
+		get (key, value);
+		target = Duration{ value };
+		return *this;
+	}
+
 	/**
 	 * Get value of optional key. Use default value of data type if missing.
 	 */

nano/node/bootstrap/bootstrap_config.cpp

@@ -9,7 +9,7 @@ nano::error nano::account_sets_config::deserialize (nano::tomlconfig & toml)
 	toml.get ("consideration_count", consideration_count);
 	toml.get ("priorities_max", priorities_max);
 	toml.get ("blocking_max", blocking_max);
-	toml.get ("cooldown", cooldown);
+	toml.get_duration ("cooldown", cooldown);
 
 	return toml.get_error ();
 }
@@ -19,7 +19,7 @@ nano::error nano::account_sets_config::serialize (nano::tomlconfig & toml) const
 	toml.put ("consideration_count", consideration_count, "Limit the number of account candidates to consider and also the number of iterations.\ntype:uint64");
 	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, "Waiting time for an account to become available.\ntype:milliseconds");
+	toml.put ("cooldown", cooldown.count (), "Waiting time for an account to become available.\ntype:milliseconds");
 
 	return toml.get_error ();
 }
@@ -32,9 +32,9 @@ nano::error nano::bootstrap_ascending_config::deserialize (nano::tomlconfig & to
 	toml.get ("requests_limit", requests_limit);
 	toml.get ("database_requests_limit", database_requests_limit);
 	toml.get ("pull_count", pull_count);
-	toml.get ("timeout", timeout);
+	toml.get_duration ("timeout", request_timeout);
 	toml.get ("throttle_coefficient", throttle_coefficient);
-	toml.get ("throttle_wait", throttle_wait);
+	toml.get_duration ("throttle_wait", throttle_wait);
 	toml.get ("block_wait_count", block_wait_count);
 
 	if (toml.has_key ("account_sets"))
@@ -51,9 +51,9 @@ nano::error nano::bootstrap_ascending_config::serialize (nano::tomlconfig & toml
 	toml.put ("requests_limit", requests_limit, "Request limit to ascending bootstrap after which requests will be dropped.\nNote: changing to unlimited (0) is not recommended.\ntype:uint64");
 	toml.put ("database_requests_limit", database_requests_limit, "Request limit for accounts from database after which requests will be dropped.\nNote: changing to unlimited (0) is not recommended as this operation competes for resources on querying the database.\ntype:uint64");
 	toml.put ("pull_count", pull_count, "Number of requested blocks for ascending bootstrap request.\ntype:uint64");
-	toml.put ("timeout", timeout, "Timeout in milliseconds for incoming ascending bootstrap messages to be processed.\ntype:milliseconds");
+	toml.put ("timeout", request_timeout.count (), "Timeout in milliseconds for incoming ascending bootstrap messages to be processed.\ntype:milliseconds");
 	toml.put ("throttle_coefficient", throttle_coefficient, "Scales the number of samples to track for bootstrap throttling.\ntype:uint64");
-	toml.put ("throttle_wait", throttle_wait, "Length of time to wait between requests when throttled.\ntype:milliseconds");
+	toml.put ("throttle_wait", throttle_wait.count (), "Length of time to wait between requests when throttled.\ntype:milliseconds");
 	toml.put ("block_wait_count", block_wait_count, "Asending bootstrap will wait while block processor has more than this many blocks queued.\ntype:uint64");
 
 	nano::tomlconfig account_sets_l;

nano/node/bootstrap/bootstrap_config.hpp

@@ -17,7 +17,7 @@ public:
 	std::size_t consideration_count{ 4 };
 	std::size_t priorities_max{ 256 * 1024 };
 	std::size_t blocking_max{ 256 * 1024 };
-	nano::millis_t cooldown{ 1000 * 3 };
+	std::chrono::milliseconds cooldown{ 1000 * 3 };
 };
 
 class bootstrap_ascending_config final
@@ -30,9 +30,9 @@ public:
 	std::size_t requests_limit{ 64 };
 	std::size_t database_requests_limit{ 1024 };
 	std::size_t pull_count{ nano::bootstrap_server::max_blocks };
-	nano::millis_t timeout{ 1000 * 3 };
+	std::chrono::milliseconds request_timeout{ 1000 * 5 };
 	std::size_t throttle_coefficient{ 16 };
-	nano::millis_t throttle_wait{ 100 };
+	std::chrono::milliseconds throttle_wait{ 100 };
 	std::size_t block_wait_count{ 1000 };
 
 	nano::account_sets_config account_sets;

nano/node/bootstrap_ascending/account_sets.cpp

@@ -114,25 +114,36 @@ void nano::bootstrap_ascending::account_sets::unblock (nano::account const & acc
 	}
 }
 
-void nano::bootstrap_ascending::account_sets::timestamp (const nano::account & account, bool reset)
+void nano::bootstrap_ascending::account_sets::timestamp_set (const nano::account & account)
 {
-	const nano::millis_t tstamp = reset ? 0 : nano::milliseconds_since_epoch ();
-
 	auto iter = priorities.get<tag_account> ().find (account);
 	if (iter != priorities.get<tag_account> ().end ())
 	{
-		priorities.get<tag_account> ().modify (iter, [tstamp] (auto & entry) {
+		priorities.get<tag_account> ().modify (iter, [] (auto & entry) {
-			entry.timestamp = tstamp;
+			entry.timestamp = std::chrono::steady_clock::now ();
 		});
 	}
 }
 
+void nano::bootstrap_ascending::account_sets::timestamp_reset (const nano::account & account)
+{
+	auto iter = priorities.get<tag_account> ().find (account);
+	if (iter != priorities.get<tag_account> ().end ())
+	{
+		priorities.get<tag_account> ().modify (iter, [] (auto & entry) {
+			entry.timestamp = {};
+		});
+	}
+}
+
+// Returns false if the account is busy
 bool nano::bootstrap_ascending::account_sets::check_timestamp (const nano::account & account) const
 {
 	auto iter = priorities.get<tag_account> ().find (account);
 	if (iter != priorities.get<tag_account> ().end ())
 	{
-		if (nano::milliseconds_since_epoch () - iter->timestamp < config.cooldown)
+		auto const cutoff = std::chrono::steady_clock::now () - config.cooldown;
+		if (iter->timestamp > cutoff)
 		{
 			return false;
 		}
@@ -241,14 +252,3 @@ std::unique_ptr<nano::container_info_component> nano::bootstrap_ascending::accou
 	composite->add_component (std::make_unique<container_info_leaf> (container_info{ "blocking", blocking.size (), sizeof (decltype (blocking)::value_type) }));
 	return composite;
 }
-
-/*
- * priority_entry
- */
-
-nano::bootstrap_ascending::account_sets::priority_entry::priority_entry (nano::account account_a, float priority_a) :
-	account{ account_a },
-	priority{ priority_a }
-{
-	id = nano::bootstrap_ascending::generate_id ();
-}

nano/node/bootstrap_ascending/account_sets.hpp

@@ -41,7 +41,8 @@ namespace bootstrap_ascending
 		void priority_down (nano::account const & account);
 		void block (nano::account const & account, nano::block_hash const & dependency);
 		void unblock (nano::account const & account, std::optional<nano::block_hash> const & hash = std::nullopt);
-		void timestamp (nano::account const & account, bool reset = false);
+		void timestamp_set (nano::account const & account);
+		void timestamp_reset (nano::account const & account);
 
 		nano::account next ();
 
@@ -68,12 +69,11 @@ namespace bootstrap_ascending
 	private:
 		struct priority_entry
 		{
-			nano::account account{ 0 };
+			nano::account account;
-			float priority{ 0 };
+			float priority;
-			nano::millis_t timestamp{ 0 };
-			nano::bootstrap_ascending::id_t id{ 0 }; // Uniformly distributed, used for random querying
 
-			priority_entry (nano::account account, float priority);
+			id_t id{ generate_id () }; // Uniformly distributed, used for random querying
+			std::chrono::steady_clock::time_point timestamp{};
 		};
 
 		struct blocking_entry

nano/node/bootstrap_ascending/service.cpp

@@ -141,7 +141,7 @@ void nano::bootstrap_ascending::service::inspect (secure::transaction const & tx
 			// If we've inserted any block in to an account, unmark it as blocked
 			accounts.unblock (account);
 			accounts.priority_up (account);
-			accounts.timestamp (account, /* reset timestamp */ true);
+			accounts.timestamp_reset (account);
 
 			if (block.is_send ())
 			{
@@ -230,7 +230,7 @@ nano::account nano::bootstrap_ascending::service::wait_available_account ()
 		auto account = available_account ();
 		if (!account.is_zero ())
 		{
-			accounts.timestamp (account);
+			accounts.timestamp_set (account);
 			return account;
 		}
 		else
@@ -246,7 +246,6 @@ bool nano::bootstrap_ascending::service::request (nano::account & account, std::
 	async_tag tag{};
 	tag.id = nano::bootstrap_ascending::generate_id ();
 	tag.account = account;
-	tag.time = nano::milliseconds_since_epoch ();
 
 	// Check if the account picked has blocks, if it does, start the pull from the highest block
 	auto info = ledger.store.account.get (ledger.store.tx_begin_read (), account);
@@ -323,8 +322,14 @@ void nano::bootstrap_ascending::service::run_timeouts ()
 		scoring.sync (network.list ());
 		scoring.timeout ();
 		throttle.resize (compute_throttle_size ());
+
+		auto const cutoff = std::chrono::steady_clock::now () - config.bootstrap_ascending.request_timeout;
+		auto should_timeout = [cutoff] (async_tag const & tag) {
+			return tag.timestamp < cutoff;
+		};
+
 		auto & tags_by_order = tags.get<tag_sequenced> ();
-		while (!tags_by_order.empty () && nano::time_difference (tags_by_order.front ().time, nano::milliseconds_since_epoch ()) > config.bootstrap_ascending.timeout)
+		while (!tags_by_order.empty () && should_timeout (tags_by_order.front ()))
 		{
 			auto tag = tags_by_order.front ();
 			tags_by_order.pop_front ();
@@ -350,7 +355,7 @@ void nano::bootstrap_ascending::service::process (nano::asc_pull_ack const & mes
 		tags_by_id.erase (iterator);
 
 		// Track bootstrap request response time
-		stats.sample (nano::stat::sample::bootstrap_tag_duration, nano::milliseconds_since_epoch () - tag.time, { 0, config.bootstrap_ascending.timeout });
+		stats.sample (nano::stat::sample::bootstrap_tag_duration, nano::log::milliseconds_delta (tag.timestamp), { 0, config.bootstrap_ascending.request_timeout.count () });
 
 		scoring.received_message (channel);
 

nano/node/bootstrap_ascending/service.hpp

@@ -82,8 +82,9 @@ namespace bootstrap_ascending
 			query_type type{ query_type::invalid };
 			nano::bootstrap_ascending::id_t id{ 0 };
 			nano::hash_or_account start{ 0 };
-			nano::millis_t time{ 0 };
 			nano::account account{ 0 };
+
+			std::chrono::steady_clock::time_point timestamp{ std::chrono::steady_clock::now () };
 		};
 
 	public: // Events

nano/node/message_processor.cpp

@@ -1,4 +1,5 @@
 #include <nano/lib/thread_roles.hpp>
+#include <nano/node/bootstrap_ascending/service.hpp>
 #include <nano/node/message_processor.hpp>
 #include <nano/node/node.hpp>
 

nano/node/node.cpp

@@ -4,6 +4,7 @@
 #include <nano/lib/tomlconfig.hpp>
 #include <nano/lib/utility.hpp>
 #include <nano/node/active_elections.hpp>
+#include <nano/node/bootstrap_ascending/service.hpp>
 #include <nano/node/common.hpp>
 #include <nano/node/confirming_set.hpp>
 #include <nano/node/daemonconfig.hpp>
@@ -216,7 +217,8 @@ nano::node::node (std::shared_ptr<boost::asio::io_context> io_ctx_a, std::filesy
 	aggregator{ *aggregator_impl },
 	wallets (wallets_store.init_error (), *this),
 	backlog{ nano::backlog_population_config (config), scheduler, ledger, stats },
-	ascendboot{ config, block_processor, ledger, network, stats },
+	ascendboot_impl{ std::make_unique<nano::bootstrap_ascending::service> (config, block_processor, ledger, network, stats) },
+	ascendboot{ *ascendboot_impl },
 	websocket{ config.websocket_config, observers, wallets, ledger, io_ctx, logger },
 	epoch_upgrader{ *this, ledger, store, network_params, logger },
 	local_block_broadcaster_impl{ std::make_unique<nano::local_block_broadcaster> (config.local_block_broadcaster, *this, block_processor, network, confirming_set, stats, logger, !flags.disable_block_processor_republishing) },

nano/node/node.hpp

@@ -12,7 +12,6 @@
 #include <nano/node/bootstrap/bootstrap.hpp>
 #include <nano/node/bootstrap/bootstrap_attempt.hpp>
 #include <nano/node/bootstrap/bootstrap_server.hpp>
-#include <nano/node/bootstrap_ascending/service.hpp>
 #include <nano/node/distributed_work_factory.hpp>
 #include <nano/node/epoch_upgrader.hpp>
 #include <nano/node/fwd.hpp>
@@ -62,6 +61,10 @@ namespace transport
 {
 	class tcp_listener;
 }
+namespace bootstrap_ascending
+{
+	class service;
+}
 namespace rocksdb
 {
 } // Declare a namespace rocksdb inside nano so all references to the rocksdb library need to be globally scoped e.g. ::rocksdb::Slice
@@ -209,7 +212,8 @@ public:
 	nano::request_aggregator & aggregator;
 	nano::wallets wallets;
 	nano::backlog_population backlog;
-	nano::bootstrap_ascending::service ascendboot;
+	std::unique_ptr<nano::bootstrap_ascending::service> ascendboot_impl;
+	nano::bootstrap_ascending::service & ascendboot;
 	nano::websocket_server websocket;
 	nano::epoch_upgrader epoch_upgrader;
 	std::unique_ptr<nano::local_block_broadcaster> local_block_broadcaster_impl;