Merge pull request #4938 from pwojcikdev/tcp-server-tests

More `tcp_server` tests

nano/core_test/network.cpp

@@ -310,7 +310,7 @@ TEST (network, send_insufficient_work)
 	nano::publish publish{ nano::dev::network_params.network, block };
 	tcp_channel->send (publish, nano::transport::traffic_type::test);
 
-	ASSERT_TIMELY_EQ (5s, 1, node2.stats.count (nano::stat::type::tcp_server_error, nano::stat::detail::insufficient_work));
+	ASSERT_TIMELY_EQ (5s, 1, node2.stats.count (nano::stat::type::tcp_server_message_error, nano::stat::detail::insufficient_work));
 }
 
 TEST (receivable_processor, confirm_insufficient_pos)
@@ -922,7 +922,7 @@ TEST (network, filter_invalid_network_bytes)
 	const_cast<nano::networks &> (keepalive.header.network) = nano::networks::invalid;
 	channel->send (keepalive, nano::transport::traffic_type::test);
 
-	ASSERT_TIMELY_EQ (5s, 1, node1.stats.count (nano::stat::type::tcp_server_error, nano::stat::detail::invalid_network));
+	ASSERT_TIMELY_EQ (5s, 1, node1.stats.count (nano::stat::type::tcp_server_message_error, nano::stat::detail::invalid_network));
 }
 
 // Ensure the network filters messages with the incorrect minimum version
@@ -941,7 +941,7 @@ TEST (network, filter_invalid_version_using)
 	const_cast<uint8_t &> (keepalive.header.version_using) = nano::dev::network_params.network.protocol_version_min - 1;
 	channel->send (keepalive, nano::transport::traffic_type::test);
 
-	ASSERT_TIMELY_EQ (5s, 1, node1.stats.count (nano::stat::type::tcp_server_error, nano::stat::detail::outdated_version));
+	ASSERT_TIMELY_EQ (5s, 1, node1.stats.count (nano::stat::type::tcp_server_message_error, nano::stat::detail::outdated_version));
 }
 
 TEST (network, fill_keepalive_self)

nano/core_test/tcp_server.cpp

@@ -10,6 +10,7 @@ using namespace std::chrono_literals;
 
 /*
  * Test case for valid handshake to ensure normal operation still works
+ * TODO: This is very non tcp_server specific, should be rewritten
  */
 TEST (tcp_server, handshake_success)
 {
@@ -171,4 +172,197 @@ TEST (tcp_server, handshake_invalid_message_type_aborts)
 
 	// Verify the handshake was aborted
 	ASSERT_TIMELY_EQ (5s, node->stats.count (nano::stat::type::tcp_server, nano::stat::detail::handshake_abort), 1);
+}
+
+/*
+ * Test that a node rejects a connection from itself (same node ID)
+ */
+TEST (tcp_server, handshake_self_connection_rejected)
+{
+	nano::test::system system;
+	auto node = system.add_node ();
+
+	auto client_socket = std::make_shared<nano::transport::tcp_socket> (*node);
+
+	// Connect to the node
+	auto ec = client_socket->blocking_connect (node->network.endpoint ());
+	ASSERT_FALSE (ec);
+
+	ASSERT_TIMELY_EQ (5s, node->tcp_listener.all_sockets ().size (), 1);
+
+	// Create a handshake response claiming to be from the same node
+	nano::node_id_handshake::query_payload query{ nano::random_pool::generate<nano::uint256_union> () };
+
+	nano::node_id_handshake::response_payload response;
+	response.node_id = node->node_id.pub; // Use our own node ID
+	response.v2 = nano::node_id_handshake::response_payload::v2_payload{
+		nano::random_pool::generate<nano::uint256_union> (), // salt
+		node->network_params.ledger.genesis->hash () // genesis
+	};
+	response.sign (query.cookie, node->node_id); // Sign with our own key
+
+	nano::node_id_handshake handshake_msg (node->network_params.network, query, response);
+
+	// Send the handshake with our own node ID
+	auto shared_const_buffer = handshake_msg.to_shared_const_buffer ();
+	auto [write_ec, bytes_written] = client_socket->blocking_write (shared_const_buffer, shared_const_buffer.size ());
+	ASSERT_FALSE (write_ec);
+	ASSERT_EQ (bytes_written, shared_const_buffer.size ());
+
+	// The server should reject the connection due to self-connection
+	ASSERT_TIMELY (5s, node->tcp_listener.all_sockets ().empty ());
+
+	// Verify the handshake was rejected
+	ASSERT_TIMELY_EQ (5s, node->stats.count (nano::stat::type::handshake, nano::stat::detail::invalid_node_id), 1);
+}
+
+/*
+ * Test that multiple handshake queries are rejected (only one query allowed per connection)
+ */
+TEST (tcp_server, handshake_multiple_queries_rejected)
+{
+	nano::test::system system;
+	auto node = system.add_node ();
+
+	auto client_socket = std::make_shared<nano::transport::tcp_socket> (*node);
+
+	// Connect to node1
+	auto ec = client_socket->blocking_connect (node->network.endpoint ());
+	ASSERT_FALSE (ec);
+
+	ASSERT_TIMELY_EQ (5s, node->tcp_listener.all_sockets ().size (), 1);
+
+	// Send first handshake query
+	nano::node_id_handshake::query_payload query1{ nano::random_pool::generate<nano::uint256_union> () };
+	nano::node_id_handshake handshake1 (node->network_params.network, query1);
+
+	auto buffer1 = handshake1.to_shared_const_buffer ();
+	auto [write_ec1, bytes_written1] = client_socket->blocking_write (buffer1, buffer1.size ());
+	ASSERT_FALSE (write_ec1);
+	ASSERT_EQ (bytes_written1, buffer1.size ());
+
+	// Send second handshake query (should be rejected)
+	nano::node_id_handshake::query_payload query2{ nano::random_pool::generate<nano::uint256_union> () };
+	nano::node_id_handshake handshake2 (node->network_params.network, query2);
+
+	auto buffer2 = handshake2.to_shared_const_buffer ();
+	auto [write_ec2, bytes_written2] = client_socket->blocking_write (buffer2, buffer2.size ());
+	ASSERT_FALSE (write_ec2);
+	ASSERT_EQ (bytes_written2, buffer2.size ());
+
+	// The server should abort the connection due to multiple queries
+	ASSERT_TIMELY (5s, node->tcp_listener.all_sockets ().empty ());
+
+	// Verify the handshake was aborted due to multiple queries
+	ASSERT_TIMELY_EQ (5s, node->stats.count (nano::stat::type::tcp_server, nano::stat::detail::handshake_error), 1);
+}
+
+/*
+ * Test that messages with outdated protocol version are rejected
+ */
+TEST (tcp_server, handshake_outdated_protocol_version)
+{
+	nano::test::system system;
+	auto node = system.add_node ();
+
+	auto client_socket = std::make_shared<nano::transport::tcp_socket> (*node);
+
+	// Connect to the node
+	auto ec = client_socket->blocking_connect (node->network.endpoint ());
+	ASSERT_FALSE (ec);
+
+	ASSERT_TIMELY_EQ (5s, node->tcp_listener.all_sockets ().size (), 1);
+
+	// Create a handshake message with outdated protocol version
+	std::vector<uint8_t> handshake_data;
+
+	nano::message_header header (nano::dev::network_params.network, nano::message_type::node_id_handshake);
+	header.version_using = node->network_params.network.protocol_version_min - 1; // Use version below minimum
+	header.version_min = node->network_params.network.protocol_version_min - 1;
+	header.extensions = 0;
+
+	// Serialize the header
+	{
+		nano::vectorstream stream (handshake_data);
+		header.serialize (stream);
+	}
+
+	// Add a basic handshake payload
+	nano::node_id_handshake::query_payload query{ nano::random_pool::generate<nano::uint256_union> () };
+	nano::node_id_handshake handshake_msg (node->network_params.network, query);
+	{
+		nano::vectorstream stream (handshake_data);
+		handshake_msg.serialize (stream);
+	}
+
+	// Send the handshake with outdated protocol version
+	auto buffer = std::make_shared<std::vector<uint8_t>> (handshake_data);
+	auto [write_ec, bytes_written] = client_socket->blocking_write (buffer, handshake_data.size ());
+	ASSERT_FALSE (write_ec);
+	ASSERT_EQ (bytes_written, handshake_data.size ());
+
+	// The server should reject the connection due to outdated protocol version
+	ASSERT_TIMELY (5s, node->tcp_listener.all_sockets ().empty ());
+
+	// Verify the handshake was aborted
+	ASSERT_TIMELY_EQ (5s, node->stats.count (nano::stat::type::tcp_server, nano::stat::detail::handshake_abort), 1);
+	ASSERT_TIMELY_EQ (5s, node->stats.count (nano::stat::type::tcp_server_message_error, nano::stat::detail::outdated_version), 1);
+}
+
+/*
+ * Test that messages with wrong network ID are rejected
+ */
+TEST (tcp_server, handshake_wrong_network_id)
+{
+	nano::test::system system;
+	auto node = system.add_node ();
+
+	auto client_socket = std::make_shared<nano::transport::tcp_socket> (*node);
+
+	// Connect to the node
+	auto ec = client_socket->blocking_connect (node->network.endpoint ());
+	ASSERT_FALSE (ec);
+
+	ASSERT_TIMELY_EQ (5s, node->tcp_listener.all_sockets ().size (), 1);
+
+	// Create a handshake message with wrong network ID
+	std::vector<uint8_t> handshake_data;
+
+	// Use a different network ID - create a different network_constants
+	nano::networks wrong_network = (node->network_params.network.current_network == nano::networks::nano_live_network)
+	? nano::networks::nano_beta_network
+	: nano::networks::nano_live_network;
+	nano::network_constants wrong_network_constants (nano::work_thresholds::publish_dev, wrong_network);
+
+	nano::message_header header (wrong_network_constants, nano::message_type::node_id_handshake);
+	header.version_using = node->network_params.network.protocol_version;
+	header.version_min = node->network_params.network.protocol_version_min;
+	header.extensions = 0;
+
+	// Serialize the header
+	{
+		nano::vectorstream stream (handshake_data);
+		header.serialize (stream);
+	}
+
+	// Add a basic handshake payload
+	nano::node_id_handshake::query_payload query{ nano::random_pool::generate<nano::uint256_union> () };
+	nano::node_id_handshake handshake_msg (node->network_params.network, query);
+	{
+		nano::vectorstream stream (handshake_data);
+		handshake_msg.serialize (stream);
+	}
+
+	// Send the handshake with wrong network ID
+	auto buffer = std::make_shared<std::vector<uint8_t>> (handshake_data);
+	auto [write_ec, bytes_written] = client_socket->blocking_write (buffer, handshake_data.size ());
+	ASSERT_FALSE (write_ec);
+	ASSERT_EQ (bytes_written, handshake_data.size ());
+
+	// The server should reject the connection due to wrong network ID
+	ASSERT_TIMELY (5s, node->tcp_listener.all_sockets ().empty ());
+
+	// Verify the handshake was aborted
+	ASSERT_TIMELY_EQ (5s, node->stats.count (nano::stat::type::tcp_server, nano::stat::detail::handshake_abort), 1);
+	ASSERT_TIMELY_EQ (5s, node->stats.count (nano::stat::type::tcp_server_message_error, nano::stat::detail::invalid_network), 1);
 }

nano/lib/constants.hpp

@@ -76,7 +76,7 @@ class network_constants
 	static constexpr std::chrono::seconds default_cleanup_period = std::chrono::seconds (60);
 
 public:
-	network_constants (nano::work_thresholds & work_, nano::networks network_a) :
+	network_constants (nano::work_thresholds const & work_, nano::networks network_a) :
 		current_network (network_a),
 		work (work_),
 		principal_weight_factor (1000), // 0.1% A representative is classified as principal based on its weight and this factor
@@ -140,7 +140,7 @@ public:
 
 	/** The network this param object represents. This may differ from the global active network; this is needed for certain --debug... commands */
 	nano::networks current_network{ nano::network_constants::active_network };
-	nano::work_thresholds & work;
+	nano::work_thresholds const & work;
 
 	unsigned principal_weight_factor;
 	uint16_t default_node_port;

nano/lib/stats_enums.hpp

@@ -39,6 +39,7 @@ enum class type
 	tcp_socket_timeout,
 	tcp_server,
 	tcp_server_message,
+	tcp_server_message_error,
 	tcp_server_read,
 	tcp_server_error,
 	tcp_channel,

nano/node/transport/tcp_server.cpp

@@ -57,14 +57,16 @@ auto nano::transport::tcp_server::start_impl () -> asio::awaitable<void>
 	try
 	{
 		auto handshake_result = co_await perform_handshake ();
-		if (handshake_result != process_result::progress)
+
+		// Only realtime mode is supported now
+		if (handshake_result == handshake_status::realtime)
 		{
-			node.stats.inc (nano::stat::type::tcp_server, nano::stat::detail::handshake_abort);
+			co_await run_realtime ();
-			node.logger.debug (nano::log::type::tcp_server, "Handshake aborted: {}", get_remote_endpoint ());
 		}
 		else
 		{
-			co_await run_realtime ();
+			node.stats.inc (nano::stat::type::tcp_server, nano::stat::detail::handshake_abort);
+			node.logger.debug (nano::log::type::tcp_server, "Handshake aborted: {}", get_remote_endpoint ());
 		}
 	}
 	catch (boost::system::system_error const & ex)
@@ -87,7 +89,7 @@ bool nano::transport::tcp_server::alive () const
 	return socket->alive ();
 }
 
-auto nano::transport::tcp_server::perform_handshake () -> asio::awaitable<process_result>
+auto nano::transport::tcp_server::perform_handshake () -> asio::awaitable<handshake_status>
 {
 	debug_assert (strand.running_in_this_thread ());
 	debug_assert (get_type () == nano::transport::socket_type::undefined);
@@ -120,7 +122,7 @@ auto nano::transport::tcp_server::perform_handshake () -> asio::awaitable<proces
 			to_string (message_status),
 			get_remote_endpoint ());
 
-			co_return process_result::abort;
+			co_return handshake_status::abort;
 		}
 
 		handshake_message_visitor handshake_visitor{};
@@ -137,11 +139,11 @@ auto nano::transport::tcp_server::perform_handshake () -> asio::awaitable<proces
 			case handshake_status::abort:
 			case handshake_status::bootstrap: // Legacy bootstrap is no longer supported
 			{
-				co_return process_result::abort;
+				co_return handshake_status::abort;
 			}
 			case handshake_status::realtime:
 			{
-				co_return process_result::progress; // Continue receiving new messages
+				co_return handshake_status::realtime; // Switch to realtime mode
 			}
 			case handshake_status::handshake:
 			{
@@ -153,7 +155,8 @@ auto nano::transport::tcp_server::perform_handshake () -> asio::awaitable<proces
 	// Failed to complete handshake, abort
 	node.stats.inc (nano::stat::type::tcp_server, nano::stat::detail::handshake_failed);
 	node.logger.debug (nano::log::type::tcp_server, "Failed to complete handshake ({})", get_remote_endpoint ());
-	co_return process_result::abort;
+
+	co_return handshake_status::abort;
 }
 
 auto nano::transport::tcp_server::run_realtime () -> asio::awaitable<void>
@@ -191,8 +194,6 @@ auto nano::transport::tcp_server::run_realtime () -> asio::awaitable<void>
 		{
 			debug_assert (status != nano::deserialize_message_status::success);
 
-			node.stats.inc (nano::stat::type::tcp_server_error, to_stat_detail (status));
-
 			switch (status)
 			{
 				// Avoid too much noise about `duplicate_publish_message` errors
@@ -221,6 +222,23 @@ auto nano::transport::tcp_server::run_realtime () -> asio::awaitable<void>
 }
 
 auto nano::transport::tcp_server::receive_message () -> asio::awaitable<nano::deserialize_message_result>
+{
+	auto result = co_await receive_message_impl ();
+
+	auto const & [message, status] = result;
+	if (message)
+	{
+		node.stats.inc (nano::stat::type::tcp_server_message, to_stat_detail (message->type ()), nano::stat::dir::in);
+	}
+	else
+	{
+		node.stats.inc (nano::stat::type::tcp_server_message_error, to_stat_detail (status), nano::stat::dir::in);
+	}
+
+	co_return result;
+}
+
+auto nano::transport::tcp_server::receive_message_impl () -> asio::awaitable<nano::deserialize_message_result>
 {
 	debug_assert (strand.running_in_this_thread ());
 
@@ -263,12 +281,6 @@ auto nano::transport::tcp_server::receive_message () -> asio::awaitable<nano::de
 	&node.block_uniquer,
 	&node.vote_uniquer);
 
-	auto const & [message, status] = result;
-	if (message)
-	{
-		node.stats.inc (nano::stat::type::tcp_server_message, to_stat_detail (message->type ()), nano::stat::dir::in);
-	}
-
 	co_return result;
 }
 

nano/node/transport/tcp_server.hpp

@@ -39,20 +39,6 @@ public:
 	}
 
 private:
-	void stop ();
-
-	enum class process_result
-	{
-		abort,
-		progress,
-	};
-
-	asio::awaitable<void> start_impl ();
-	asio::awaitable<process_result> perform_handshake ();
-	asio::awaitable<void> run_realtime ();
-	asio::awaitable<nano::deserialize_message_result> receive_message ();
-	asio::awaitable<nano::buffer_view> read_socket (size_t size) const;
-
 	enum class handshake_status
 	{
 		abort,
@@ -61,6 +47,15 @@ private:
 		bootstrap,
 	};
 
+	void stop ();
+
+	asio::awaitable<void> start_impl ();
+	asio::awaitable<handshake_status> perform_handshake ();
+	asio::awaitable<void> run_realtime ();
+	asio::awaitable<nano::deserialize_message_result> receive_message ();
+	asio::awaitable<nano::deserialize_message_result> receive_message_impl ();
+	asio::awaitable<nano::buffer_view> read_socket (size_t size) const;
+
 	asio::awaitable<handshake_status> process_handshake (nano::node_id_handshake const & message);
 	asio::awaitable<void> send_handshake_response (nano::node_id_handshake::query_payload const & query, bool v2);
 	asio::awaitable<void> send_handshake_request ();