From ad9f5318f64e082706559f85897e17297e19642d Mon Sep 17 00:00:00 2001
From: Dimitrios Siganos <dimitris@siganos.org>
Date: Fri, 28 Jan 2022 03:21:07 +0000
Subject: [PATCH 01/10] socket_timeout unit tests

---
 nano/core_test/socket.cpp | 251 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 251 insertions(+)

diff --git a/nano/core_test/socket.cpp b/nano/core_test/socket.cpp
index eb5a8866a6..3d7a3d1df5 100644
--- a/nano/core_test/socket.cpp
+++ b/nano/core_test/socket.cpp
@@ -7,6 +7,8 @@
 
 #include <gtest/gtest.h>
 
+#include <boost/asio/read.hpp>
+
 #include <map>
 #include <memory>
 #include <utility>
@@ -605,3 +607,252 @@ TEST (socket, concurrent_writes)
 		t.join ();
 	}
 }
+
+/**
+ * Check that the socket correctly handles a tcp_io_timeout during tcp connect
+ * Steps:
+ *   set timeout to one second
+ *   do a tcp connect that will block for at least a few seconds at the tcp level
+ *   check that the connect returns error and that the correct counters have been incremented
+ */
+TEST (socket_timeout, connect)
+{
+	// create one node and set timeout to 1 second
+	nano::system system (1);
+	std::shared_ptr<nano::node> node = system.nodes[0];
+	node->config.tcp_io_timeout = std::chrono::seconds (1);
+
+	// try to connect to an IP address that most likely does not exist and will not reply
+	// we want the tcp stack to not receive a negative reply, we want it to see silence and to keep trying
+	// I use the un-routable IP address 10.255.254.253, which is likely to not exist
+	boost::asio::ip::tcp::endpoint endpoint (boost::asio::ip::make_address_v6 ("::ffff:10.255.254.253"), nano::get_available_port ());
+
+	// create a client socket and try to connect to the IP address that wil not respond
+	auto socket = std::make_shared<nano::client_socket> (*node);
+	std::atomic<bool> done = false;
+	boost::system::error_code ec;
+	socket->async_connect (endpoint, [&ec, &done] (boost::system::error_code const & ec_a) {
+		if (ec_a)
+		{
+			ec = ec_a;
+			done = true;
+		}
+	});
+
+	// check that the callback was called and we got an error
+	ASSERT_TIMELY (6s, done == true);
+	ASSERT_TRUE (ec);
+	ASSERT_EQ (1, node->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_connect_error, nano::stat::dir::in));
+
+	// check that the socket was closed due to tcp_io_timeout timeout
+	ASSERT_EQ (1, node->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_io_timeout_drop, nano::stat::dir::out));
+}
+
+TEST (socket_timeout, read)
+{
+	// create one node and set timeout to 1 second
+	nano::system system (1);
+	std::shared_ptr<nano::node> node = system.nodes[0];
+	node->config.tcp_io_timeout = std::chrono::seconds (2);
+
+	// create a server socket
+	boost::asio::ip::tcp::endpoint endpoint (boost::asio::ip::address_v6::loopback (), nano::get_available_port ());
+	boost::asio::ip::tcp::acceptor acceptor (system.io_ctx);
+	acceptor.open (endpoint.protocol ());
+	acceptor.bind (endpoint);
+	acceptor.listen (boost::asio::socket_base::max_listen_connections);
+
+	// asynchronously accept an incoming connection and create a newsock and do not send any data
+	boost::asio::ip::tcp::socket newsock (system.io_ctx);
+	acceptor.async_accept (newsock, [] (boost::system::error_code const & ec_a) {
+		debug_assert (!ec_a);
+	});
+
+	// create a client socket to connect and call async_read, which should time out
+	auto socket = std::make_shared<nano::client_socket> (*node);
+	std::atomic<bool> done = false;
+	boost::system::error_code ec;
+	socket->async_connect (endpoint, [&socket, &ec, &done] (boost::system::error_code const & ec_a) {
+		debug_assert (!ec_a);
+		auto buffer = std::make_shared<std::vector<uint8_t>> (1);
+		socket->async_read (buffer, 1, [&ec, &done] (boost::system::error_code const & ec_a, size_t size_a) {
+			if (ec_a)
+			{
+				ec = ec_a;
+				done = true;
+			}
+		});
+	});
+
+	// check that the callback was called and we got an error
+	ASSERT_TIMELY (10s, done == true);
+	ASSERT_TRUE (ec);
+	ASSERT_EQ (1, node->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_read_error, nano::stat::dir::in));
+
+	// check that the socket was closed due to tcp_io_timeout timeout
+	ASSERT_EQ (1, node->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_io_timeout_drop, nano::stat::dir::out));
+}
+
+TEST (socket_timeout, write)
+{
+	// create one node and set timeout to 1 second
+	nano::system system (1);
+	std::shared_ptr<nano::node> node = system.nodes[0];
+	node->config.tcp_io_timeout = std::chrono::seconds (2);
+
+	// create a server socket
+	boost::asio::ip::tcp::endpoint endpoint (boost::asio::ip::address_v6::loopback (), nano::get_available_port ());
+	boost::asio::ip::tcp::acceptor acceptor (system.io_ctx);
+	acceptor.open (endpoint.protocol ());
+	acceptor.bind (endpoint);
+	acceptor.listen (boost::asio::socket_base::max_listen_connections);
+
+	// asynchronously accept an incoming connection and create a newsock and do not receive any data
+	boost::asio::ip::tcp::socket newsock (system.io_ctx);
+	acceptor.async_accept (newsock, [] (boost::system::error_code const & ec_a) {
+		debug_assert (!ec_a);
+	});
+
+	// create a client socket and send lots of data to fill the socket queue on the local and remote side
+	// eventually, the all tcp queues should fill up and async_write will not be able to progress
+	// and the timeout should kick in and close the socket, which will cause the async_write to return an error
+	auto socket = std::make_shared<nano::client_socket> (*node);
+	std::atomic<bool> done = false;
+	boost::system::error_code ec;
+	socket->async_connect (endpoint, [&socket, &ec, &done] (boost::system::error_code const & ec_a) {
+		debug_assert (!ec_a);
+		auto buffer = std::make_shared<std::vector<uint8_t>> (128 * 1024);
+		for (auto i = 0; i < 1024; ++i)
+		{
+			socket->async_write (nano::shared_const_buffer{ buffer }, [&ec, &done] (boost::system::error_code const & ec_a, size_t size_a) {
+				if (ec_a)
+				{
+					ec = ec_a;
+					done = true;
+				}
+			});
+		}
+	});
+
+	// check that the callback was called and we got an error
+	ASSERT_TIMELY (10s, done == true);
+	ASSERT_TRUE (ec);
+	ASSERT_EQ (1, node->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_write_error, nano::stat::dir::in));
+
+	// check that the socket was closed due to tcp_io_timeout timeout
+	ASSERT_EQ (1, node->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_io_timeout_drop, nano::stat::dir::out));
+}
+
+TEST (socket_timeout, read_overlapped)
+{
+	// create one node and set timeout to 1 second
+	nano::system system (1);
+	std::shared_ptr<nano::node> node = system.nodes[0];
+	node->config.tcp_io_timeout = std::chrono::seconds (2);
+
+	// create a server socket
+	boost::asio::ip::tcp::endpoint endpoint (boost::asio::ip::address_v6::loopback (), nano::get_available_port ());
+	boost::asio::ip::tcp::acceptor acceptor (system.io_ctx);
+	acceptor.open (endpoint.protocol ());
+	acceptor.bind (endpoint);
+	acceptor.listen (boost::asio::socket_base::max_listen_connections);
+
+	// asynchronously accept an incoming connection and send one byte only
+	boost::asio::ip::tcp::socket newsock (system.io_ctx);
+	acceptor.async_accept (newsock, [&newsock] (boost::system::error_code const & ec_a) {
+		debug_assert (!ec_a);
+		auto buffer = std::make_shared<std::vector<uint8_t>> (1);
+		nano::async_write (newsock, nano::shared_const_buffer (buffer), [] (boost::system::error_code const & ec_a, size_t size_a) {
+			debug_assert (!ec_a);
+			debug_assert (size_a == 1);
+		});
+	});
+
+	// create a client socket to connect and call async_read twice, the second call should time out
+	auto socket = std::make_shared<nano::client_socket> (*node);
+	std::atomic<bool> done = false;
+	boost::system::error_code ec;
+	socket->async_connect (endpoint, [&socket, &ec, &done] (boost::system::error_code const & ec_a) {
+		debug_assert (!ec_a);
+		auto buffer = std::make_shared<std::vector<uint8_t>> (1);
+
+		socket->async_read (buffer, 1, [] (boost::system::error_code const & ec_a, size_t size_a) {
+			debug_assert (size_a == 1);
+		});
+
+		socket->async_read (buffer, 1, [&ec, &done] (boost::system::error_code const & ec_a, size_t size_a) {
+			debug_assert (size_a == 0);
+			if (ec_a)
+			{
+				ec = ec_a;
+				done = true;
+			}
+		});
+	});
+
+	// check that the callback was called and we got an error
+	ASSERT_TIMELY (10s, done == true);
+	ASSERT_TRUE (ec);
+	ASSERT_EQ (1, node->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_read_error, nano::stat::dir::in));
+
+	// check that the socket was closed due to tcp_io_timeout timeout
+	ASSERT_EQ (1, node->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_io_timeout_drop, nano::stat::dir::out));
+}
+
+TEST (socket_timeout, write_overlapped)
+{
+	// create one node and set timeout to 1 second
+	nano::system system (1);
+	std::shared_ptr<nano::node> node = system.nodes[0];
+	node->config.tcp_io_timeout = std::chrono::seconds (2);
+
+	// create a server socket
+	boost::asio::ip::tcp::endpoint endpoint (boost::asio::ip::address_v6::loopback (), nano::get_available_port ());
+	boost::asio::ip::tcp::acceptor acceptor (system.io_ctx);
+	acceptor.open (endpoint.protocol ());
+	acceptor.bind (endpoint);
+	acceptor.listen (boost::asio::socket_base::max_listen_connections);
+
+	// asynchronously accept an incoming connection and read 2 bytes only
+	boost::asio::ip::tcp::socket newsock (system.io_ctx);
+	auto buffer = std::make_shared<std::vector<uint8_t>> (1);
+	acceptor.async_accept (newsock, [&newsock, &buffer] (boost::system::error_code const & ec_a) {
+		debug_assert (!ec_a);
+		boost::asio::async_read (newsock, boost::asio::buffer (buffer->data (), buffer->size ()), [] (boost::system::error_code const & ec_a, size_t size_a) {
+			debug_assert (size_a == 1);
+		});
+	});
+
+	// create a client socket and send lots of data to fill the socket queue on the local and remote side
+	// eventually, the all tcp queues should fill up and async_write will not be able to progress
+	// and the timeout should kick in and close the socket, which will cause the async_write to return an error
+	auto socket = std::make_shared<nano::client_socket> (*node);
+	std::atomic<bool> done = false;
+	boost::system::error_code ec;
+	socket->async_connect (endpoint, [&socket, &ec, &done] (boost::system::error_code const & ec_a) {
+		debug_assert (!ec_a);
+		auto buffer1 = std::make_shared<std::vector<uint8_t>> (1);
+		auto buffer2 = std::make_shared<std::vector<uint8_t>> (128 * 1024);
+		socket->async_write (nano::shared_const_buffer{ buffer1 }, [] (boost::system::error_code const & ec_a, size_t size_a) {
+			debug_assert (size_a == 1);
+		});
+		for (auto i = 0; i < 1024; ++i)
+		{
+			socket->async_write (nano::shared_const_buffer{ buffer2 }, [&ec, &done] (boost::system::error_code const & ec_a, size_t size_a) {
+				if (ec_a)
+				{
+					ec = ec_a;
+					done = true;
+				}
+			});
+		}
+	});
+
+	// check that the callback was called and we got an error
+	ASSERT_TIMELY (10s, done == true);
+	ASSERT_TRUE (ec);
+	ASSERT_EQ (1, node->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_write_error, nano::stat::dir::in));
+
+	// check that the socket was closed due to tcp_io_timeout timeout
+	ASSERT_EQ (1, node->stats.count (nano::stat::type::tcp, nano::stat::detail::tcp_io_timeout_drop, nano::stat::dir::out));
+}

From ff04b78adc10d1b2cdbe7f89e0e88b128cb4dcb1 Mon Sep 17 00:00:00 2001
From: Dimitrios Siganos <dimitris@siganos.org>
Date: Thu, 27 Jan 2022 15:53:29 +0000
Subject: [PATCH 02/10] Rename socket::stop_timer to socket:set_last_completion

All stop timer does is to set a variable. It is a simply setter function.
It is confusing to call it stop_timer because it implies that it does
something more than just set a timestamp.
---
 nano/node/socket.cpp | 8 ++++----
 nano/node/socket.hpp | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/nano/node/socket.cpp b/nano/node/socket.cpp
index f64751f669..22071ceaf5 100644
--- a/nano/node/socket.cpp
+++ b/nano/node/socket.cpp
@@ -68,7 +68,7 @@ void nano::socket::async_connect (nano::tcp_endpoint const & endpoint_a, std::fu
 		}
 		else
 		{
-			this_l->stop_timer ();
+			this_l->set_last_completion ();
 		}
 		this_l->remote = endpoint_a;
 		callback (ec);
@@ -94,7 +94,7 @@ void nano::socket::async_read (std::shared_ptr<std::vector<uint8_t>> const & buf
 					else
 					{
 						this_l->node.stats.add (nano::stat::type::traffic_tcp, nano::stat::dir::in, size_a);
-						this_l->stop_timer ();
+						this_l->set_last_completion ();
 						this_l->update_last_receive_time ();
 					}
 					cbk (ec, size_a);
@@ -130,7 +130,7 @@ void nano::socket::async_write (nano::shared_const_buffer const & buffer_a, std:
 					else
 					{
 						this_l->node.stats.add (nano::stat::type::traffic_tcp, nano::stat::dir::out, size_a);
-						this_l->stop_timer ();
+						this_l->set_last_completion ();
 					}
 					if (cbk)
 					{
@@ -165,7 +165,7 @@ void nano::socket::start_timer (std::chrono::seconds deadline_a)
 	next_deadline = deadline_a.count ();
 }
 
-void nano::socket::stop_timer ()
+void nano::socket::set_last_completion ()
 {
 	last_completion_time_or_init = nano::seconds_since_epoch ();
 }
diff --git a/nano/node/socket.hpp b/nano/node/socket.hpp
index 4415d0c302..342e601466 100644
--- a/nano/node/socket.hpp
+++ b/nano/node/socket.hpp
@@ -135,7 +135,7 @@ class socket : public std::enable_shared_from_this<nano::socket>
 	std::atomic<bool> closed{ false };
 	void close_internal ();
 	void start_timer ();
-	void stop_timer ();
+	void set_last_completion ();
 	void update_last_receive_time ();
 	void checkup ();
 

From ed053faf177d648d68ab3327a88a19180cf064a7 Mon Sep 17 00:00:00 2001
From: Dimitrios Siganos <dimitris@siganos.org>
Date: Fri, 28 Jan 2022 20:49:28 +0000
Subject: [PATCH 03/10] Rename start_timer() to set_next_deadline()

---
 nano/node/bootstrap/bootstrap_connections.cpp |  2 +-
 nano/node/socket.cpp                          | 16 +++++++++-------
 nano/node/socket.hpp                          |  4 ++--
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/nano/node/bootstrap/bootstrap_connections.cpp b/nano/node/bootstrap/bootstrap_connections.cpp
index e5d504c773..e26a69ecf5 100644
--- a/nano/node/bootstrap/bootstrap_connections.cpp
+++ b/nano/node/bootstrap/bootstrap_connections.cpp
@@ -97,7 +97,7 @@ void nano::bootstrap_connections::pool_connection (std::shared_ptr<nano::bootstr
 	auto const & socket_l = client_a->socket;
 	if (!stopped && !client_a->pending_stop && !node.network.excluded_peers.check (client_a->channel->get_tcp_endpoint ()))
 	{
-		socket_l->start_timer (node.network_params.network.idle_timeout);
+		socket_l->set_next_deadline (node.network_params.network.idle_timeout);
 		// Push into idle deque
 		if (!push_front)
 		{
diff --git a/nano/node/socket.cpp b/nano/node/socket.cpp
index 22071ceaf5..2403bf7a14 100644
--- a/nano/node/socket.cpp
+++ b/nano/node/socket.cpp
@@ -58,7 +58,7 @@ void nano::socket::async_connect (nano::tcp_endpoint const & endpoint_a, std::fu
 	debug_assert (endpoint_type () == endpoint_type_t::client);
 	checkup ();
 	auto this_l (shared_from_this ());
-	start_timer ();
+	set_next_deadline ();
 	this_l->tcp_socket.async_connect (endpoint_a,
 	boost::asio::bind_executor (this_l->strand,
 	[this_l, callback = std::move (callback_a), endpoint_a] (boost::system::error_code const & ec) {
@@ -82,7 +82,7 @@ void nano::socket::async_read (std::shared_ptr<std::vector<uint8_t>> const & buf
 		auto this_l (shared_from_this ());
 		if (!closed)
 		{
-			start_timer ();
+			set_next_deadline ();
 			boost::asio::post (strand, boost::asio::bind_executor (strand, [buffer_a, callback = std::move (callback_a), size_a, this_l] () mutable {
 				boost::asio::async_read (this_l->tcp_socket, boost::asio::buffer (buffer_a->data (), size_a),
 				boost::asio::bind_executor (this_l->strand,
@@ -118,7 +118,7 @@ void nano::socket::async_write (nano::shared_const_buffer const & buffer_a, std:
 		boost::asio::post (strand, boost::asio::bind_executor (strand, [buffer_a, callback = std::move (callback_a), this_l = shared_from_this ()] () mutable {
 			if (!this_l->closed)
 			{
-				this_l->start_timer ();
+				this_l->set_next_deadline ();
 				nano::async_write (this_l->tcp_socket, buffer_a,
 				boost::asio::bind_executor (this_l->strand,
 				[buffer_a, cbk = std::move (callback), this_l] (boost::system::error_code ec, std::size_t size_a) {
@@ -155,12 +155,14 @@ void nano::socket::async_write (nano::shared_const_buffer const & buffer_a, std:
 	}
 }
 
-void nano::socket::start_timer ()
+/** sets the next deadline on this socket.
+ */
+void nano::socket::set_next_deadline ()
 {
-	start_timer (io_timeout);
+	set_next_deadline (io_timeout);
 }
 
-void nano::socket::start_timer (std::chrono::seconds deadline_a)
+void nano::socket::set_next_deadline (std::chrono::seconds deadline_a)
 {
 	next_deadline = deadline_a.count ();
 }
@@ -432,7 +434,7 @@ void nano::server_socket::on_connection (std::function<bool (std::shared_ptr<nan
 				// Make sure the new connection doesn't idle. Note that in most cases, the callback is going to start
 				// an IO operation immediately, which will start a timer.
 				new_connection->checkup ();
-				new_connection->start_timer (this_l->node.network_params.network.is_dev_network () ? this_l->node.network_params.network.socket_dev_idle_timeout : this_l->node.network_params.network.idle_timeout);
+				new_connection->set_next_deadline (this_l->node.network_params.network.is_dev_network () ? this_l->node.network_params.network.socket_dev_idle_timeout : this_l->node.network_params.network.idle_timeout);
 				this_l->node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_accept_success, nano::stat::dir::in);
 				this_l->connections_per_address.emplace (new_connection->remote.address (), new_connection);
 				if (cbk (new_connection, ec_a))
diff --git a/nano/node/socket.hpp b/nano/node/socket.hpp
index 342e601466..1c40f97d02 100644
--- a/nano/node/socket.hpp
+++ b/nano/node/socket.hpp
@@ -75,7 +75,7 @@ class socket : public std::enable_shared_from_this<nano::socket>
 	bool has_timed_out () const;
 	/** This can be called to change the maximum idle time, e.g. based on the type of traffic detected. */
 	void timeout_set (std::chrono::seconds io_timeout_a);
-	void start_timer (std::chrono::seconds deadline_a);
+	void set_next_deadline (std::chrono::seconds deadline_a);
 	void set_silent_connection_tolerance_time (std::chrono::seconds tolerance_time_a);
 	bool max () const
 	{
@@ -134,7 +134,7 @@ class socket : public std::enable_shared_from_this<nano::socket>
 	 error codes as the OS may have already completed the async operation. */
 	std::atomic<bool> closed{ false };
 	void close_internal ();
-	void start_timer ();
+	void set_next_deadline ();
 	void set_last_completion ();
 	void update_last_receive_time ();
 	void checkup ();

From f137dd02894b10b1d0eccc39bac96e7fb178aafa Mon Sep 17 00:00:00 2001
From: Dimitrios Siganos <dimitris@siganos.org>
Date: Fri, 28 Jan 2022 21:16:51 +0000
Subject: [PATCH 04/10] Remove network constant socket_dev_idle_timeout

We do not need it.
---
 nano/core_test/socket.cpp | 2 +-
 nano/lib/config.hpp       | 2 --
 nano/node/socket.cpp      | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/nano/core_test/socket.cpp b/nano/core_test/socket.cpp
index 3d7a3d1df5..73d4d617ec 100644
--- a/nano/core_test/socket.cpp
+++ b/nano/core_test/socket.cpp
@@ -360,7 +360,7 @@ TEST (socket, disconnection_of_silent_connections)
 	nano::node_config config;
 	// Increasing the timer timeout, so we don't let the connection to timeout due to the timer checker.
 	config.tcp_io_timeout = std::chrono::seconds::max ();
-	config.network_params.network.socket_dev_idle_timeout = std::chrono::seconds::max ();
+	config.network_params.network.idle_timeout = std::chrono::seconds::max ();
 	// Silent connections are connections open by external peers that don't contribute with any data.
 	config.network_params.network.silent_connection_tolerance_time = std::chrono::seconds{ 5 };
 
diff --git a/nano/lib/config.hpp b/nano/lib/config.hpp
index 2869d0fa36..ecf12d34eb 100644
--- a/nano/lib/config.hpp
+++ b/nano/lib/config.hpp
@@ -155,7 +155,6 @@ class network_constants
 																				: 47000;
 		request_interval_ms = is_dev_network () ? 20 : 500;
 		cleanup_period = is_dev_network () ? std::chrono::seconds (1) : std::chrono::seconds (60);
-		socket_dev_idle_timeout = std::chrono::seconds (2);
 		idle_timeout = is_dev_network () ? cleanup_period * 15 : cleanup_period * 2;
 		silent_connection_tolerance_time = std::chrono::seconds (120);
 		syn_cookie_cutoff = std::chrono::seconds (5);
@@ -190,7 +189,6 @@ class network_constants
 		return cleanup_period * 5;
 	}
 	/** Default maximum idle time for a socket before it's automatically closed */
-	std::chrono::seconds socket_dev_idle_timeout;
 	std::chrono::seconds idle_timeout;
 	std::chrono::seconds silent_connection_tolerance_time;
 	std::chrono::seconds syn_cookie_cutoff;
diff --git a/nano/node/socket.cpp b/nano/node/socket.cpp
index 2403bf7a14..c7da025fe8 100644
--- a/nano/node/socket.cpp
+++ b/nano/node/socket.cpp
@@ -434,7 +434,7 @@ void nano::server_socket::on_connection (std::function<bool (std::shared_ptr<nan
 				// Make sure the new connection doesn't idle. Note that in most cases, the callback is going to start
 				// an IO operation immediately, which will start a timer.
 				new_connection->checkup ();
-				new_connection->set_next_deadline (this_l->node.network_params.network.is_dev_network () ? this_l->node.network_params.network.socket_dev_idle_timeout : this_l->node.network_params.network.idle_timeout);
+				new_connection->set_next_deadline (this_l->node.network_params.network.idle_timeout);
 				this_l->node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_accept_success, nano::stat::dir::in);
 				this_l->connections_per_address.emplace (new_connection->remote.address (), new_connection);
 				if (cbk (new_connection, ec_a))

From aeb08e49f1209ea6b0da68e6e0f5d3fc127d0207 Mon Sep 17 00:00:00 2001
From: Dimitrios Siganos <dimitris@siganos.org>
Date: Fri, 28 Jan 2022 21:55:25 +0000
Subject: [PATCH 05/10] Rename update_last_receive_time() to
 set_last_receive_time()

To make it obvious it is setter function and to make it consistent with
other setter functions.
---
 nano/node/socket.cpp | 4 ++--
 nano/node/socket.hpp | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/nano/node/socket.cpp b/nano/node/socket.cpp
index c7da025fe8..284c7ff893 100644
--- a/nano/node/socket.cpp
+++ b/nano/node/socket.cpp
@@ -95,7 +95,7 @@ void nano::socket::async_read (std::shared_ptr<std::vector<uint8_t>> const & buf
 					{
 						this_l->node.stats.add (nano::stat::type::traffic_tcp, nano::stat::dir::in, size_a);
 						this_l->set_last_completion ();
-						this_l->update_last_receive_time ();
+						this_l->set_last_receive_time ();
 					}
 					cbk (ec, size_a);
 				}));
@@ -172,7 +172,7 @@ void nano::socket::set_last_completion ()
 	last_completion_time_or_init = nano::seconds_since_epoch ();
 }
 
-void nano::socket::update_last_receive_time ()
+void nano::socket::set_last_receive_time ()
 {
 	last_receive_time_or_init = nano::seconds_since_epoch ();
 }
diff --git a/nano/node/socket.hpp b/nano/node/socket.hpp
index 1c40f97d02..82f97580e5 100644
--- a/nano/node/socket.hpp
+++ b/nano/node/socket.hpp
@@ -136,7 +136,7 @@ class socket : public std::enable_shared_from_this<nano::socket>
 	void close_internal ();
 	void set_next_deadline ();
 	void set_last_completion ();
-	void update_last_receive_time ();
+	void set_last_receive_time ();
 	void checkup ();
 
 private:

From 2f18d6ce226db1ca897882d3b03b8f144b13f19f Mon Sep 17 00:00:00 2001
From: Dimitrios Siganos <dimitris@siganos.org>
Date: Sat, 29 Jan 2022 00:18:02 +0000
Subject: [PATCH 06/10] Minor code improvement - no functional change

---
 nano/node/socket.hpp | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/nano/node/socket.hpp b/nano/node/socket.hpp
index 82f97580e5..7445054354 100644
--- a/nano/node/socket.hpp
+++ b/nano/node/socket.hpp
@@ -12,15 +12,9 @@
 #include <memory>
 #include <vector>
 
-namespace boost
+namespace boost::asio::ip
 {
-namespace asio
-{
-	namespace ip
-	{
-		class network_v6;
-	}
-}
+class network_v6;
 }
 
 namespace nano

From 5a70f24cf259d98973f279fc82f7a7f06b2bfc1d Mon Sep 17 00:00:00 2001
From: Dimitrios Siganos <dimitris@siganos.org>
Date: Sat, 29 Jan 2022 01:12:57 +0000
Subject: [PATCH 07/10] Rename the nano::socket concept of deadline to timeout

A deadline makes people imagine a fixed point in time, a timestamp.
However, socket::next_deadline is a timeout value and not a deadline.
It signifies the seconds of inactivity that would cause a socket timeout.

Renames:
set_next_deadline          -> set_default_timeout
set_next_deadline(timeout) -> set_timeout(timeout)
io_timeout                 -> default_timeout
timeout_set                -> set_default_timeout_value
next_deadline              -> timeout
---
 nano/node/bootstrap/bootstrap_connections.cpp |  2 +-
 nano/node/bootstrap/bootstrap_server.cpp      |  4 +-
 nano/node/socket.cpp                          | 40 +++++++++++--------
 nano/node/socket.hpp                          | 12 +++---
 4 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/nano/node/bootstrap/bootstrap_connections.cpp b/nano/node/bootstrap/bootstrap_connections.cpp
index e26a69ecf5..12f2e7f2ad 100644
--- a/nano/node/bootstrap/bootstrap_connections.cpp
+++ b/nano/node/bootstrap/bootstrap_connections.cpp
@@ -97,7 +97,7 @@ void nano::bootstrap_connections::pool_connection (std::shared_ptr<nano::bootstr
 	auto const & socket_l = client_a->socket;
 	if (!stopped && !client_a->pending_stop && !node.network.excluded_peers.check (client_a->channel->get_tcp_endpoint ()))
 	{
-		socket_l->set_next_deadline (node.network_params.network.idle_timeout);
+		socket_l->set_timeout (node.network_params.network.idle_timeout);
 		// Push into idle deque
 		if (!push_front)
 		{
diff --git a/nano/node/bootstrap/bootstrap_server.cpp b/nano/node/bootstrap/bootstrap_server.cpp
index ae7caadcdd..87222c9c3a 100644
--- a/nano/node/bootstrap/bootstrap_server.cpp
+++ b/nano/node/bootstrap/bootstrap_server.cpp
@@ -179,7 +179,7 @@ void nano::bootstrap_server::stop ()
 void nano::bootstrap_server::receive ()
 {
 	// Increase timeout to receive TCP header (idle server socket)
-	socket->timeout_set (node->network_params.network.idle_timeout);
+	socket->set_default_timeout_value (node->network_params.network.idle_timeout);
 	auto this_l (shared_from_this ());
 	socket->async_read (receive_buffer, 8, [this_l] (boost::system::error_code const & ec, std::size_t size_a) {
 		// Set remote_endpoint
@@ -188,7 +188,7 @@ void nano::bootstrap_server::receive ()
 			this_l->remote_endpoint = this_l->socket->remote_endpoint ();
 		}
 		// Decrease timeout to default
-		this_l->socket->timeout_set (this_l->node->config.tcp_io_timeout);
+		this_l->socket->set_default_timeout_value (this_l->node->config.tcp_io_timeout);
 		// Receive header
 		this_l->receive_header_action (ec, size_a);
 	});
diff --git a/nano/node/socket.cpp b/nano/node/socket.cpp
index 284c7ff893..b5d211ecdd 100644
--- a/nano/node/socket.cpp
+++ b/nano/node/socket.cpp
@@ -40,10 +40,10 @@ nano::socket::socket (nano::node & node_a, endpoint_type_t endpoint_type_a) :
 	tcp_socket{ node_a.io_ctx },
 	node{ node_a },
 	endpoint_type_m{ endpoint_type_a },
-	next_deadline{ std::numeric_limits<uint64_t>::max () },
+	timeout{ std::numeric_limits<uint64_t>::max () },
 	last_completion_time_or_init{ nano::seconds_since_epoch () },
 	last_receive_time_or_init{ nano::seconds_since_epoch () },
-	io_timeout{ node_a.config.tcp_io_timeout },
+	default_timeout{ node_a.config.tcp_io_timeout },
 	silent_connection_tolerance_time{ node_a.network_params.network.silent_connection_tolerance_time }
 {
 }
@@ -58,7 +58,7 @@ void nano::socket::async_connect (nano::tcp_endpoint const & endpoint_a, std::fu
 	debug_assert (endpoint_type () == endpoint_type_t::client);
 	checkup ();
 	auto this_l (shared_from_this ());
-	set_next_deadline ();
+	set_default_timeout ();
 	this_l->tcp_socket.async_connect (endpoint_a,
 	boost::asio::bind_executor (this_l->strand,
 	[this_l, callback = std::move (callback_a), endpoint_a] (boost::system::error_code const & ec) {
@@ -82,7 +82,7 @@ void nano::socket::async_read (std::shared_ptr<std::vector<uint8_t>> const & buf
 		auto this_l (shared_from_this ());
 		if (!closed)
 		{
-			set_next_deadline ();
+			set_default_timeout ();
 			boost::asio::post (strand, boost::asio::bind_executor (strand, [buffer_a, callback = std::move (callback_a), size_a, this_l] () mutable {
 				boost::asio::async_read (this_l->tcp_socket, boost::asio::buffer (buffer_a->data (), size_a),
 				boost::asio::bind_executor (this_l->strand,
@@ -118,7 +118,7 @@ void nano::socket::async_write (nano::shared_const_buffer const & buffer_a, std:
 		boost::asio::post (strand, boost::asio::bind_executor (strand, [buffer_a, callback = std::move (callback_a), this_l = shared_from_this ()] () mutable {
 			if (!this_l->closed)
 			{
-				this_l->set_next_deadline ();
+				this_l->set_default_timeout ();
 				nano::async_write (this_l->tcp_socket, buffer_a,
 				boost::asio::bind_executor (this_l->strand,
 				[buffer_a, cbk = std::move (callback), this_l] (boost::system::error_code ec, std::size_t size_a) {
@@ -155,16 +155,21 @@ void nano::socket::async_write (nano::shared_const_buffer const & buffer_a, std:
 	}
 }
 
-/** sets the next deadline on this socket.
- */
-void nano::socket::set_next_deadline ()
+/** Call set_timeout with default_timeout as parameter */
+void nano::socket::set_default_timeout ()
 {
-	set_next_deadline (io_timeout);
+	set_timeout (default_timeout);
 }
 
-void nano::socket::set_next_deadline (std::chrono::seconds deadline_a)
+/** Set the current timeout of the socket in seconds
+ *  timeout occurs when the last socket completion is more than timeout seconds in the past
+ *  timeout always applies, the socket always has a timeout
+ *  to set infinite timeout, use std::numeric_limits<uint64_t>::max ()
+ *  the function checkup() checks for timeout on a regular interval
+ */
+void nano::socket::set_timeout (std::chrono::seconds timeout_a)
 {
-	next_deadline = deadline_a.count ();
+	timeout = timeout_a.count ();
 }
 
 void nano::socket::set_last_completion ()
@@ -190,7 +195,8 @@ void nano::socket::checkup ()
 				this_l->node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_silent_connection_drop, nano::stat::dir::in);
 				condition_to_disconnect = true;
 			}
-			if (this_l->next_deadline != std::numeric_limits<uint64_t>::max () && (now - this_l->last_completion_time_or_init) > this_l->next_deadline)
+
+			if (this_l->timeout != std::numeric_limits<uint64_t>::max () && (now - this_l->last_completion_time_or_init) > this_l->timeout)
 			{
 				this_l->node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_io_timeout_drop,
 				this_l->endpoint_type () == endpoint_type_t::server ? nano::stat::dir::in : nano::stat::dir::out);
@@ -224,9 +230,9 @@ bool nano::socket::has_timed_out () const
 	return timed_out;
 }
 
-void nano::socket::timeout_set (std::chrono::seconds io_timeout_a)
+void nano::socket::set_default_timeout_value (std::chrono::seconds timeout_a)
 {
-	io_timeout = io_timeout_a;
+	default_timeout = timeout_a;
 }
 
 void nano::socket::set_silent_connection_tolerance_time (std::chrono::seconds tolerance_time_a)
@@ -250,7 +256,7 @@ void nano::socket::close_internal ()
 {
 	if (!closed.exchange (true))
 	{
-		io_timeout = std::chrono::seconds (0);
+		default_timeout = std::chrono::seconds (0);
 		boost::system::error_code ec;
 
 		// Ignore error code for shutdown as it is best-effort
@@ -280,7 +286,7 @@ nano::server_socket::server_socket (nano::node & node_a, boost::asio::ip::tcp::e
 	local{ std::move (local_a) },
 	max_inbound_connections{ max_connections_a }
 {
-	io_timeout = std::chrono::seconds::max ();
+	default_timeout = std::chrono::seconds::max ();
 }
 
 void nano::server_socket::start (boost::system::error_code & ec_a)
@@ -434,7 +440,7 @@ void nano::server_socket::on_connection (std::function<bool (std::shared_ptr<nan
 				// Make sure the new connection doesn't idle. Note that in most cases, the callback is going to start
 				// an IO operation immediately, which will start a timer.
 				new_connection->checkup ();
-				new_connection->set_next_deadline (this_l->node.network_params.network.idle_timeout);
+				new_connection->set_timeout (this_l->node.network_params.network.idle_timeout);
 				this_l->node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_accept_success, nano::stat::dir::in);
 				this_l->connections_per_address.emplace (new_connection->remote.address (), new_connection);
 				if (cbk (new_connection, ec_a))
diff --git a/nano/node/socket.hpp b/nano/node/socket.hpp
index 7445054354..86e3519dd2 100644
--- a/nano/node/socket.hpp
+++ b/nano/node/socket.hpp
@@ -46,11 +46,13 @@ class socket : public std::enable_shared_from_this<nano::socket>
 		realtime,
 		realtime_response_server // special type for tcp channel response server
 	};
+
 	enum class endpoint_type_t
 	{
 		server,
 		client
 	};
+
 	/**
 	 * Constructor
 	 * @param node Owning node
@@ -68,8 +70,8 @@ class socket : public std::enable_shared_from_this<nano::socket>
 	/** Returns true if the socket has timed out */
 	bool has_timed_out () const;
 	/** This can be called to change the maximum idle time, e.g. based on the type of traffic detected. */
-	void timeout_set (std::chrono::seconds io_timeout_a);
-	void set_next_deadline (std::chrono::seconds deadline_a);
+	void set_default_timeout_value (std::chrono::seconds);
+	void set_timeout (std::chrono::seconds);
 	void set_silent_connection_tolerance_time (std::chrono::seconds tolerance_time_a);
 	bool max () const
 	{
@@ -116,11 +118,11 @@ class socket : public std::enable_shared_from_this<nano::socket>
 	/** The other end of the connection */
 	boost::asio::ip::tcp::endpoint remote;
 
-	std::atomic<uint64_t> next_deadline;
+	std::atomic<uint64_t> timeout;
 	std::atomic<uint64_t> last_completion_time_or_init;
 	std::atomic<uint64_t> last_receive_time_or_init;
 	std::atomic<bool> timed_out{ false };
-	std::atomic<std::chrono::seconds> io_timeout;
+	std::atomic<std::chrono::seconds> default_timeout;
 	std::chrono::seconds silent_connection_tolerance_time;
 	std::atomic<std::size_t> queue_size{ 0 };
 
@@ -128,7 +130,7 @@ class socket : public std::enable_shared_from_this<nano::socket>
 	 error codes as the OS may have already completed the async operation. */
 	std::atomic<bool> closed{ false };
 	void close_internal ();
-	void set_next_deadline ();
+	void set_default_timeout ();
 	void set_last_completion ();
 	void set_last_receive_time ();
 	void checkup ();

From 1bfcb73f260e750d83eec76d072fe444374899a6 Mon Sep 17 00:00:00 2001
From: Dimitrios Siganos <dimitris@siganos.org>
Date: Sat, 29 Jan 2022 01:14:34 +0000
Subject: [PATCH 08/10] Add some comments to nano::socket class

---
 nano/node/socket.cpp |  4 ++++
 nano/node/socket.hpp | 26 ++++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/nano/node/socket.cpp b/nano/node/socket.cpp
index b5d211ecdd..8abaa481e3 100644
--- a/nano/node/socket.cpp
+++ b/nano/node/socket.cpp
@@ -190,18 +190,22 @@ void nano::socket::checkup ()
 		{
 			uint64_t now (nano::seconds_since_epoch ());
 			auto condition_to_disconnect{ false };
+
+			// if this is a server socket, and no data is received for silent_connection_tolerance_time seconds then disconnect
 			if (this_l->endpoint_type () == endpoint_type_t::server && (now - this_l->last_receive_time_or_init) > this_l->silent_connection_tolerance_time.count ())
 			{
 				this_l->node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_silent_connection_drop, nano::stat::dir::in);
 				condition_to_disconnect = true;
 			}
 
+			// if there is no activity for timeout seconds then disconnect
 			if (this_l->timeout != std::numeric_limits<uint64_t>::max () && (now - this_l->last_completion_time_or_init) > this_l->timeout)
 			{
 				this_l->node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_io_timeout_drop,
 				this_l->endpoint_type () == endpoint_type_t::server ? nano::stat::dir::in : nano::stat::dir::out);
 				condition_to_disconnect = true;
 			}
+
 			if (condition_to_disconnect)
 			{
 				if (this_l->node.config.logging.network_timeout_logging ())
diff --git a/nano/node/socket.hpp b/nano/node/socket.hpp
index 86e3519dd2..7e5002440f 100644
--- a/nano/node/socket.hpp
+++ b/nano/node/socket.hpp
@@ -118,12 +118,38 @@ class socket : public std::enable_shared_from_this<nano::socket>
 	/** The other end of the connection */
 	boost::asio::ip::tcp::endpoint remote;
 
+	/** number of seconds of inactivity that causes a socket timeout
+	 *  activity is any successful connect, send or receive event
+	 */
 	std::atomic<uint64_t> timeout;
+
+	/** the timestamp (in seconds since epoch) of the last time there was successful activity on the socket
+	 *  activity is any successful connect, send or receive event
+	 */
 	std::atomic<uint64_t> last_completion_time_or_init;
+
+	/** the timestamp (in seconds since epoch) of the last time there was successful receive on the socket
+	 *  successful receive includes graceful closing of the socket by the peer (the read succeeds but returns 0 bytes)
+	 */
 	std::atomic<uint64_t> last_receive_time_or_init;
+
+	/** Flag that is set when cleanup decides to close the socket due to timeout.
+	 *  NOTE: Currently used by bootstrap_server::timeout() but I suspect that this and bootstrap_server::timeout() are not needed.
+	 */
 	std::atomic<bool> timed_out{ false };
+
+	/** the timeout value to use when calling set_default_timeout() */
 	std::atomic<std::chrono::seconds> default_timeout;
+
+	/** used in real time server sockets, number of seconds of no receive traffic that will cause the socket to timeout */
 	std::chrono::seconds silent_connection_tolerance_time;
+
+	/** Tracks number of blocks queued for delivery to the local socket send buffers.
+	 *  Under normal circumstances, this should be zero.
+	 *  Note that this is not the number of buffers queued to the peer, it is the number of buffers
+	 *  queued up to enter the local TCP send buffer
+	 *  socket buffer queue -> TCP send queue -> (network) -> TCP receive queue of peer
+	 */
 	std::atomic<std::size_t> queue_size{ 0 };
 
 	/** Set by close() - completion handlers must check this. This is more reliable than checking

From 799a0708e596748de27d060ece6ac14c0bd031d5 Mon Sep 17 00:00:00 2001
From: Dimitrios Siganos <dimitris@siganos.org>
Date: Sat, 29 Jan 2022 02:45:33 +0000
Subject: [PATCH 09/10] Remove needless code

---
 nano/node/socket.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nano/node/socket.cpp b/nano/node/socket.cpp
index 8abaa481e3..fe8981744d 100644
--- a/nano/node/socket.cpp
+++ b/nano/node/socket.cpp
@@ -199,7 +199,7 @@ void nano::socket::checkup ()
 			}
 
 			// if there is no activity for timeout seconds then disconnect
-			if (this_l->timeout != std::numeric_limits<uint64_t>::max () && (now - this_l->last_completion_time_or_init) > this_l->timeout)
+			if ((now - this_l->last_completion_time_or_init) > this_l->timeout)
 			{
 				this_l->node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_io_timeout_drop,
 				this_l->endpoint_type () == endpoint_type_t::server ? nano::stat::dir::in : nano::stat::dir::out);

From 85cdaec43cf27d3f6d717dad9b70fcc5b371df4d Mon Sep 17 00:00:00 2001
From: Dimitrios Siganos <dimitris@siganos.org>
Date: Sat, 29 Jan 2022 03:49:53 +0000
Subject: [PATCH 10/10] Use guard ifs to make nano::socket::async_write more
 readable

---
 nano/node/socket.cpp | 75 ++++++++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 34 deletions(-)

diff --git a/nano/node/socket.cpp b/nano/node/socket.cpp
index fe8981744d..966daafead 100644
--- a/nano/node/socket.cpp
+++ b/nano/node/socket.cpp
@@ -112,47 +112,54 @@ void nano::socket::async_read (std::shared_ptr<std::vector<uint8_t>> const & buf
 
 void nano::socket::async_write (nano::shared_const_buffer const & buffer_a, std::function<void (boost::system::error_code const &, std::size_t)> callback_a)
 {
-	if (!closed)
+	if (closed)
 	{
-		++queue_size;
-		boost::asio::post (strand, boost::asio::bind_executor (strand, [buffer_a, callback = std::move (callback_a), this_l = shared_from_this ()] () mutable {
-			if (!this_l->closed)
-			{
-				this_l->set_default_timeout ();
-				nano::async_write (this_l->tcp_socket, buffer_a,
-				boost::asio::bind_executor (this_l->strand,
-				[buffer_a, cbk = std::move (callback), this_l] (boost::system::error_code ec, std::size_t size_a) {
-					--this_l->queue_size;
-					if (ec)
-					{
-						this_l->node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_write_error, nano::stat::dir::in);
-					}
-					else
-					{
-						this_l->node.stats.add (nano::stat::type::traffic_tcp, nano::stat::dir::out, size_a);
-						this_l->set_last_completion ();
-					}
-					if (cbk)
-					{
-						cbk (ec, size_a);
-					}
-				}));
-			}
-			else if (callback)
-			{
+		if (callback_a)
+		{
+			node.background ([callback = std::move (callback_a)] () {
 				callback (boost::system::errc::make_error_code (boost::system::errc::not_supported), 0);
-			}
-		}));
+			});
+		}
+
+		return;
 	}
-	else
-	{
-		node.background ([callback = std::move (callback_a)] () {
+
+	++queue_size;
+
+	boost::asio::post (strand, boost::asio::bind_executor (strand, [buffer_a, callback = std::move (callback_a), this_l = shared_from_this ()] () mutable {
+		if (this_l->closed)
+		{
 			if (callback)
 			{
 				callback (boost::system::errc::make_error_code (boost::system::errc::not_supported), 0);
 			}
-		});
-	}
+
+			return;
+		}
+
+		this_l->set_default_timeout ();
+
+		nano::async_write (this_l->tcp_socket, buffer_a,
+		boost::asio::bind_executor (this_l->strand,
+		[buffer_a, cbk = std::move (callback), this_l] (boost::system::error_code ec, std::size_t size_a) {
+			--this_l->queue_size;
+
+			if (ec)
+			{
+				this_l->node.stats.inc (nano::stat::type::tcp, nano::stat::detail::tcp_write_error, nano::stat::dir::in);
+			}
+			else
+			{
+				this_l->node.stats.add (nano::stat::type::traffic_tcp, nano::stat::dir::out, size_a);
+				this_l->set_last_completion ();
+			}
+
+			if (cbk)
+			{
+				cbk (ec, size_a);
+			}
+		}));
+	}));
 }
 
 /** Call set_timeout with default_timeout as parameter */