From ee12994f42f207a122dd7f934587171ce98c9921 Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Mon, 19 Sep 2022 18:19:56 -0300 Subject: [PATCH 01/11] Return if !socket_l --- nano/node/transport/tcp.cpp | 196 ++++++++++++++++++------------------ 1 file changed, 99 insertions(+), 97 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index 3d5f18fe..47166415 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -595,141 +595,143 @@ void nano::transport::tcp_channels::start_tcp (nano::endpoint const & endpoint_a void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptr const & channel_a, nano::endpoint const & endpoint_a, std::shared_ptr> const & receive_buffer_a) { std::weak_ptr node_w (node.shared ()); - if (auto socket_l = channel_a->socket.lock ()) + auto socket_l = channel_a->socket.lock (); + if (!socket_l) { - auto cleanup_node_id_handshake_socket = [socket_w = channel_a->socket, node_w] (nano::endpoint const & endpoint_a) { - if (auto node_l = node_w.lock ()) + return; + } + auto cleanup_node_id_handshake_socket = [socket_w = channel_a->socket, node_w] (nano::endpoint const & endpoint_a) { + if (auto node_l = node_w.lock ()) + { + if (auto socket_l = socket_w.lock ()) { - if (auto socket_l = socket_w.lock ()) - { - socket_l->close (); - } + socket_l->close (); } - }; + } + }; - socket_l->async_read (receive_buffer_a, 8 + sizeof (nano::account) + sizeof (nano::account) + sizeof (nano::signature), [node_w, channel_a, endpoint_a, receive_buffer_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { - if (auto node_l = node_w.lock ()) + socket_l->async_read (receive_buffer_a, 8 + sizeof (nano::account) + sizeof (nano::account) + sizeof (nano::signature), [node_w, channel_a, endpoint_a, receive_buffer_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { + if (auto node_l = node_w.lock ()) + { + if (!ec && channel_a) { - if (!ec && channel_a) + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::node_id_handshake, nano::stat::dir::in); + auto error (false); + nano::bufferstream stream (receive_buffer_a->data (), size_a); + nano::message_header header (error, stream); + // the header type should in principle be checked after checking the network bytes and the version numbers, I will not change it here since the benefits do not outweight the difficulties + if (!error && header.type == nano::message_type::node_id_handshake) { - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::node_id_handshake, nano::stat::dir::in); - auto error (false); - nano::bufferstream stream (receive_buffer_a->data (), size_a); - nano::message_header header (error, stream); - // the header type should in principle be checked after checking the network bytes and the version numbers, I will not change it here since the benefits do not outweight the difficulties - if (!error && header.type == nano::message_type::node_id_handshake) + if (header.network == node_l->network_params.network.current_network && header.version_using >= node_l->network_params.network.protocol_version_min) { - if (header.network == node_l->network_params.network.current_network && header.version_using >= node_l->network_params.network.protocol_version_min) + nano::node_id_handshake message (error, stream, header); + if (!error && message.response && message.query) { - nano::node_id_handshake message (error, stream, header); - if (!error && message.response && message.query) + channel_a->set_network_version (header.version_using); + auto node_id (message.response->first); + bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); + if (process) { - channel_a->set_network_version (header.version_using); - auto node_id (message.response->first); - bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); - if (process) + /* If node ID is known, don't establish new connection +Exception: temporary channels from bootstrap_server */ + auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); + if (existing_channel) { - /* If node ID is known, don't establish new connection - Exception: temporary channels from bootstrap_server */ - auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); - if (existing_channel) - { - process = existing_channel->temporary; - } - } - if (process) - { - channel_a->set_node_id (node_id); - channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); - boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); - nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); - } - channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { - if (auto node_l = node_w.lock ()) - { - if (!ec && channel_a) - { - // Insert new node ID connection - if (auto socket_l = channel_a->socket.lock ()) - { - channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); - auto response_server = std::make_shared (socket_l, node_l); - node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); - // Listen for possible responses - response_server->socket->type_set (nano::socket::type_t::realtime_response_server); - response_server->remote_node_id = channel_a->get_node_id (); - response_server->start (); - - if (!node_l->flags.disable_initial_telemetry_requests) - { - node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { - // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers - }); - } - } - } - else - { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); - } - cleanup_node_id_handshake_socket (endpoint_a); - } - } - }); + process = existing_channel->temporary; } } - else + if (process) { + channel_a->set_node_id (node_id); + channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); + boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); + nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); if (node_l->config.logging.network_node_id_handshake_logging ()) { - node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); + node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); } - cleanup_node_id_handshake_socket (endpoint_a); + channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { + if (auto node_l = node_w.lock ()) + { + if (!ec && channel_a) + { + // Insert new node ID connection + if (auto socket_l = channel_a->socket.lock ()) + { + channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); + auto response_server = std::make_shared (socket_l, node_l); + node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); + // Listen for possible responses + response_server->socket->type_set (nano::socket::type_t::realtime_response_server); + response_server->remote_node_id = channel_a->get_node_id (); + response_server->start (); + + if (!node_l->flags.disable_initial_telemetry_requests) + { + node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { + // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers + }); + } + } + } + else + { + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); + } + cleanup_node_id_handshake_socket (endpoint_a); + } + } + }); } } else { - // error handling, either the networks bytes or the version is wrong - if (header.network == node_l->network_params.network.current_network) + if (node_l->config.logging.network_node_id_handshake_logging ()) { - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network); + node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); } - else - { - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::outdated_version); - } - cleanup_node_id_handshake_socket (endpoint_a); - // Cleanup attempt - { - nano::lock_guard lock (node_l->network.tcp_channels.mutex); - node_l->network.tcp_channels.attempts.get ().erase (nano::transport::map_endpoint_to_tcp (endpoint_a)); - } } } else { - if (node_l->config.logging.network_node_id_handshake_logging ()) + // error handling, either the networks bytes or the version is wrong + if (header.network == node_l->network_params.network.current_network) { - node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake message header from %1%") % endpoint_a)); + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network); } + else + { + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::outdated_version); + } + cleanup_node_id_handshake_socket (endpoint_a); + // Cleanup attempt + { + nano::lock_guard lock (node_l->network.tcp_channels.mutex); + node_l->network.tcp_channels.attempts.get ().erase (nano::transport::map_endpoint_to_tcp (endpoint_a)); + } } } else { if (node_l->config.logging.network_node_id_handshake_logging ()) { - node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%: %2%") % endpoint_a % ec.message ())); + node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake message header from %1%") % endpoint_a)); } cleanup_node_id_handshake_socket (endpoint_a); } } - }); - } + else + { + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%: %2%") % endpoint_a % ec.message ())); + } + cleanup_node_id_handshake_socket (endpoint_a); + } + } + }); } From 284ae3a458f5af49e4769252ce54ba0ddf208a35 Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Mon, 19 Sep 2022 18:52:26 -0300 Subject: [PATCH 02/11] Invert condition to (ec || !channel_a) and returns Invert condition (!ec && channel_a) to (ec || !channel_a) using DeMorgan and returns --- nano/node/transport/tcp.cpp | 42 ++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index 47166415..212d03a5 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -653,34 +653,32 @@ Exception: temporary channels from bootstrap_server */ channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { if (auto node_l = node_w.lock ()) { - if (!ec && channel_a) - { - // Insert new node ID connection - if (auto socket_l = channel_a->socket.lock ()) - { - channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); - auto response_server = std::make_shared (socket_l, node_l); - node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); - // Listen for possible responses - response_server->socket->type_set (nano::socket::type_t::realtime_response_server); - response_server->remote_node_id = channel_a->get_node_id (); - response_server->start (); - - if (!node_l->flags.disable_initial_telemetry_requests) - { - node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { - // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers - }); - } - } - } - else + if (ec || !channel_a) { if (node_l->config.logging.network_node_id_handshake_logging ()) { node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); } cleanup_node_id_handshake_socket (endpoint_a); + return; + } + // Insert new node ID connection + if (auto socket_l = channel_a->socket.lock ()) + { + channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); + auto response_server = std::make_shared (socket_l, node_l); + node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); + // Listen for possible responses + response_server->socket->type_set (nano::socket::type_t::realtime_response_server); + response_server->remote_node_id = channel_a->get_node_id (); + response_server->start (); + + if (!node_l->flags.disable_initial_telemetry_requests) + { + node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { + // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers + }); + } } } }); From 521d539e51cfa0781dfdf170869751bce43f1d3e Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Mon, 19 Sep 2022 18:58:39 -0300 Subject: [PATCH 03/11] Inverts condition to (!error && message.response && message.query) Inverts condition (error || !message.response || !message.query) to (!error && message.response && message.query) using DeMorgan and returns --- nano/node/transport/tcp.cpp | 118 ++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 60 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index 212d03a5..28c99d72 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -625,72 +625,70 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptrnetwork_params.network.current_network && header.version_using >= node_l->network_params.network.protocol_version_min) { nano::node_id_handshake message (error, stream, header); - if (!error && message.response && message.query) - { - channel_a->set_network_version (header.version_using); - auto node_id (message.response->first); - bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); - if (process) - { - /* If node ID is known, don't establish new connection -Exception: temporary channels from bootstrap_server */ - auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); - if (existing_channel) - { - process = existing_channel->temporary; - } - } - if (process) - { - channel_a->set_node_id (node_id); - channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); - boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); - nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); - } - channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { - if (auto node_l = node_w.lock ()) - { - if (ec || !channel_a) - { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; - } - // Insert new node ID connection - if (auto socket_l = channel_a->socket.lock ()) - { - channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); - auto response_server = std::make_shared (socket_l, node_l); - node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); - // Listen for possible responses - response_server->socket->type_set (nano::socket::type_t::realtime_response_server); - response_server->remote_node_id = channel_a->get_node_id (); - response_server->start (); - - if (!node_l->flags.disable_initial_telemetry_requests) - { - node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { - // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers - }); - } - } - } - }); - } - } - else + if (error || !message.response || !message.query) { if (node_l->config.logging.network_node_id_handshake_logging ()) { node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); } cleanup_node_id_handshake_socket (endpoint_a); + return; + } + channel_a->set_network_version (header.version_using); + auto node_id (message.response->first); + bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); + if (process) + { + /* If node ID is known, don't establish new connection +Exception: temporary channels from bootstrap_server */ + auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); + if (existing_channel) + { + process = existing_channel->temporary; + } + } + if (process) + { + channel_a->set_node_id (node_id); + channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); + boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); + nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); + } + channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { + if (auto node_l = node_w.lock ()) + { + if (ec || !channel_a) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); + } + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + // Insert new node ID connection + if (auto socket_l = channel_a->socket.lock ()) + { + channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); + auto response_server = std::make_shared (socket_l, node_l); + node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); + // Listen for possible responses + response_server->socket->type_set (nano::socket::type_t::realtime_response_server); + response_server->remote_node_id = channel_a->get_node_id (); + response_server->start (); + + if (!node_l->flags.disable_initial_telemetry_requests) + { + node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { + // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers + }); + } + } + } + }); } } else From 5fbf176ccc1cf7cb0883ed1fd47feae15201b751 Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Mon, 19 Sep 2022 19:04:45 -0300 Subject: [PATCH 04/11] Invert network header check condition Invert (header.network == node_l->network_params.network.current_network && header.version_using >= node_l->network_params.network.protocol_version_min) to (header.network != node_l->network_params.network.current_network || header.version_using < node_l->network_params.network.protocol_version_min) and returns --- nano/node/transport/tcp.cpp | 138 ++++++++++++++++++------------------ 1 file changed, 68 insertions(+), 70 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index 28c99d72..d32090a9 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -622,76 +622,7 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptrnetwork_params.network.current_network && header.version_using >= node_l->network_params.network.protocol_version_min) - { - nano::node_id_handshake message (error, stream, header); - if (error || !message.response || !message.query) - { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; - } - channel_a->set_network_version (header.version_using); - auto node_id (message.response->first); - bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); - if (process) - { - /* If node ID is known, don't establish new connection -Exception: temporary channels from bootstrap_server */ - auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); - if (existing_channel) - { - process = existing_channel->temporary; - } - } - if (process) - { - channel_a->set_node_id (node_id); - channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); - boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); - nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); - } - channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { - if (auto node_l = node_w.lock ()) - { - if (ec || !channel_a) - { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; - } - // Insert new node ID connection - if (auto socket_l = channel_a->socket.lock ()) - { - channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); - auto response_server = std::make_shared (socket_l, node_l); - node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); - // Listen for possible responses - response_server->socket->type_set (nano::socket::type_t::realtime_response_server); - response_server->remote_node_id = channel_a->get_node_id (); - response_server->start (); - - if (!node_l->flags.disable_initial_telemetry_requests) - { - node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { - // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers - }); - } - } - } - }); - } - } - else + if (header.network != node_l->network_params.network.current_network || header.version_using < node_l->network_params.network.protocol_version_min) { // error handling, either the networks bytes or the version is wrong if (header.network == node_l->network_params.network.current_network) @@ -709,6 +640,73 @@ Exception: temporary channels from bootstrap_server */ nano::lock_guard lock (node_l->network.tcp_channels.mutex); node_l->network.tcp_channels.attempts.get ().erase (nano::transport::map_endpoint_to_tcp (endpoint_a)); } + return; + } + nano::node_id_handshake message (error, stream, header); + if (error || !message.response || !message.query) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); + } + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + channel_a->set_network_version (header.version_using); + auto node_id (message.response->first); + bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); + if (process) + { + /* If node ID is known, don't establish new connection +Exception: temporary channels from bootstrap_server */ + auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); + if (existing_channel) + { + process = existing_channel->temporary; + } + } + if (process) + { + channel_a->set_node_id (node_id); + channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); + boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); + nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); + } + channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { + if (auto node_l = node_w.lock ()) + { + if (ec || !channel_a) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); + } + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + // Insert new node ID connection + if (auto socket_l = channel_a->socket.lock ()) + { + channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); + auto response_server = std::make_shared (socket_l, node_l); + node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); + // Listen for possible responses + response_server->socket->type_set (nano::socket::type_t::realtime_response_server); + response_server->remote_node_id = channel_a->get_node_id (); + response_server->start (); + + if (!node_l->flags.disable_initial_telemetry_requests) + { + node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { + // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers + }); + } + } + } + }); } } else From bfb91907b34215a29a8ef95b155ce035a4516f70 Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Mon, 19 Sep 2022 19:09:30 -0300 Subject: [PATCH 05/11] Invert network header desserialization check Inverts (!error && header.type == nano::message_type::node_id_handshake) to (error || header.type != nano::message_type::node_id_handshake) and returns --- nano/node/transport/tcp.cpp | 178 ++++++++++++++++++------------------ 1 file changed, 88 insertions(+), 90 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index d32090a9..fb7fc2f7 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -620,102 +620,100 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptrdata (), size_a); nano::message_header header (error, stream); // the header type should in principle be checked after checking the network bytes and the version numbers, I will not change it here since the benefits do not outweight the difficulties - if (!error && header.type == nano::message_type::node_id_handshake) - { - if (header.network != node_l->network_params.network.current_network || header.version_using < node_l->network_params.network.protocol_version_min) - { - // error handling, either the networks bytes or the version is wrong - if (header.network == node_l->network_params.network.current_network) - { - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network); - } - else - { - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::outdated_version); - } - - cleanup_node_id_handshake_socket (endpoint_a); - // Cleanup attempt - { - nano::lock_guard lock (node_l->network.tcp_channels.mutex); - node_l->network.tcp_channels.attempts.get ().erase (nano::transport::map_endpoint_to_tcp (endpoint_a)); - } - return; - } - nano::node_id_handshake message (error, stream, header); - if (error || !message.response || !message.query) - { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; - } - channel_a->set_network_version (header.version_using); - auto node_id (message.response->first); - bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); - if (process) - { - /* If node ID is known, don't establish new connection -Exception: temporary channels from bootstrap_server */ - auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); - if (existing_channel) - { - process = existing_channel->temporary; - } - } - if (process) - { - channel_a->set_node_id (node_id); - channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); - boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); - nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); - } - channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { - if (auto node_l = node_w.lock ()) - { - if (ec || !channel_a) - { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; - } - // Insert new node ID connection - if (auto socket_l = channel_a->socket.lock ()) - { - channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); - auto response_server = std::make_shared (socket_l, node_l); - node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); - // Listen for possible responses - response_server->socket->type_set (nano::socket::type_t::realtime_response_server); - response_server->remote_node_id = channel_a->get_node_id (); - response_server->start (); - - if (!node_l->flags.disable_initial_telemetry_requests) - { - node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { - // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers - }); - } - } - } - }); - } - } - else + if (error || header.type != nano::message_type::node_id_handshake) { if (node_l->config.logging.network_node_id_handshake_logging ()) { node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake message header from %1%") % endpoint_a)); } cleanup_node_id_handshake_socket (endpoint_a); + return; + } + if (header.network != node_l->network_params.network.current_network || header.version_using < node_l->network_params.network.protocol_version_min) + { + // error handling, either the networks bytes or the version is wrong + if (header.network == node_l->network_params.network.current_network) + { + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network); + } + else + { + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::outdated_version); + } + + cleanup_node_id_handshake_socket (endpoint_a); + // Cleanup attempt + { + nano::lock_guard lock (node_l->network.tcp_channels.mutex); + node_l->network.tcp_channels.attempts.get ().erase (nano::transport::map_endpoint_to_tcp (endpoint_a)); + } + return; + } + nano::node_id_handshake message (error, stream, header); + if (error || !message.response || !message.query) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); + } + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + channel_a->set_network_version (header.version_using); + auto node_id (message.response->first); + bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); + if (process) + { + /* If node ID is known, don't establish new connection +Exception: temporary channels from bootstrap_server */ + auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); + if (existing_channel) + { + process = existing_channel->temporary; + } + } + if (process) + { + channel_a->set_node_id (node_id); + channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); + boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); + nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); + } + channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { + if (auto node_l = node_w.lock ()) + { + if (ec || !channel_a) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); + } + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + // Insert new node ID connection + if (auto socket_l = channel_a->socket.lock ()) + { + channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); + auto response_server = std::make_shared (socket_l, node_l); + node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); + // Listen for possible responses + response_server->socket->type_set (nano::socket::type_t::realtime_response_server); + response_server->remote_node_id = channel_a->get_node_id (); + response_server->start (); + + if (!node_l->flags.disable_initial_telemetry_requests) + { + node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { + // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers + }); + } + } + } + }); } } else From 1e0216a52c280e901d5904145c1bd7d937b25513 Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Mon, 19 Sep 2022 19:13:16 -0300 Subject: [PATCH 06/11] Invert async_read ec and channel check condition Inverts check from (!ec && channel_a) to (ec || !channel_a) and returns --- nano/node/transport/tcp.cpp | 206 ++++++++++++++++++------------------ 1 file changed, 102 insertions(+), 104 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index fb7fc2f7..829648c4 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -613,116 +613,114 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptrasync_read (receive_buffer_a, 8 + sizeof (nano::account) + sizeof (nano::account) + sizeof (nano::signature), [node_w, channel_a, endpoint_a, receive_buffer_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { if (auto node_l = node_w.lock ()) { - if (!ec && channel_a) - { - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::node_id_handshake, nano::stat::dir::in); - auto error (false); - nano::bufferstream stream (receive_buffer_a->data (), size_a); - nano::message_header header (error, stream); - // the header type should in principle be checked after checking the network bytes and the version numbers, I will not change it here since the benefits do not outweight the difficulties - if (error || header.type != nano::message_type::node_id_handshake) - { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake message header from %1%") % endpoint_a)); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; - } - if (header.network != node_l->network_params.network.current_network || header.version_using < node_l->network_params.network.protocol_version_min) - { - // error handling, either the networks bytes or the version is wrong - if (header.network == node_l->network_params.network.current_network) - { - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network); - } - else - { - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::outdated_version); - } - - cleanup_node_id_handshake_socket (endpoint_a); - // Cleanup attempt - { - nano::lock_guard lock (node_l->network.tcp_channels.mutex); - node_l->network.tcp_channels.attempts.get ().erase (nano::transport::map_endpoint_to_tcp (endpoint_a)); - } - return; - } - nano::node_id_handshake message (error, stream, header); - if (error || !message.response || !message.query) - { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; - } - channel_a->set_network_version (header.version_using); - auto node_id (message.response->first); - bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); - if (process) - { - /* If node ID is known, don't establish new connection -Exception: temporary channels from bootstrap_server */ - auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); - if (existing_channel) - { - process = existing_channel->temporary; - } - } - if (process) - { - channel_a->set_node_id (node_id); - channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); - boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); - nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); - } - channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { - if (auto node_l = node_w.lock ()) - { - if (ec || !channel_a) - { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; - } - // Insert new node ID connection - if (auto socket_l = channel_a->socket.lock ()) - { - channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); - auto response_server = std::make_shared (socket_l, node_l); - node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); - // Listen for possible responses - response_server->socket->type_set (nano::socket::type_t::realtime_response_server); - response_server->remote_node_id = channel_a->get_node_id (); - response_server->start (); - - if (!node_l->flags.disable_initial_telemetry_requests) - { - node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { - // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers - }); - } - } - } - }); - } - } - else + if (ec || !channel_a) { if (node_l->config.logging.network_node_id_handshake_logging ()) { node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%: %2%") % endpoint_a % ec.message ())); } cleanup_node_id_handshake_socket (endpoint_a); + return; + } + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::node_id_handshake, nano::stat::dir::in); + auto error (false); + nano::bufferstream stream (receive_buffer_a->data (), size_a); + nano::message_header header (error, stream); + // the header type should in principle be checked after checking the network bytes and the version numbers, I will not change it here since the benefits do not outweight the difficulties + if (error || header.type != nano::message_type::node_id_handshake) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake message header from %1%") % endpoint_a)); + } + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + if (header.network != node_l->network_params.network.current_network || header.version_using < node_l->network_params.network.protocol_version_min) + { + // error handling, either the networks bytes or the version is wrong + if (header.network == node_l->network_params.network.current_network) + { + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network); + } + else + { + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::outdated_version); + } + + cleanup_node_id_handshake_socket (endpoint_a); + // Cleanup attempt + { + nano::lock_guard lock (node_l->network.tcp_channels.mutex); + node_l->network.tcp_channels.attempts.get ().erase (nano::transport::map_endpoint_to_tcp (endpoint_a)); + } + return; + } + nano::node_id_handshake message (error, stream, header); + if (error || !message.response || !message.query) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); + } + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + channel_a->set_network_version (header.version_using); + auto node_id (message.response->first); + bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); + if (process) + { + /* If node ID is known, don't establish new connection +Exception: temporary channels from bootstrap_server */ + auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); + if (existing_channel) + { + process = existing_channel->temporary; + } + } + if (process) + { + channel_a->set_node_id (node_id); + channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); + boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); + nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); + } + channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { + if (auto node_l = node_w.lock ()) + { + if (ec || !channel_a) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); + } + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + // Insert new node ID connection + if (auto socket_l = channel_a->socket.lock ()) + { + channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); + auto response_server = std::make_shared (socket_l, node_l); + node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); + // Listen for possible responses + response_server->socket->type_set (nano::socket::type_t::realtime_response_server); + response_server->remote_node_id = channel_a->get_node_id (); + response_server->start (); + + if (!node_l->flags.disable_initial_telemetry_requests) + { + node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { + // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers + }); + } + } + } + }); } } }); From 891d6385afa2609854c44bf5b8023ef50089f82f Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Mon, 19 Sep 2022 19:16:28 -0300 Subject: [PATCH 07/11] Return if !node_l in async_read --- nano/node/transport/tcp.cpp | 186 ++++++++++++++++++------------------ 1 file changed, 94 insertions(+), 92 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index 829648c4..0c545341 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -611,117 +611,119 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptrasync_read (receive_buffer_a, 8 + sizeof (nano::account) + sizeof (nano::account) + sizeof (nano::signature), [node_w, channel_a, endpoint_a, receive_buffer_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { - if (auto node_l = node_w.lock ()) + auto node_l = node_w.lock (); + if (!node_l) { - if (ec || !channel_a) + return; + } + if (ec || !channel_a) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%: %2%") % endpoint_a % ec.message ())); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; + node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%: %2%") % endpoint_a % ec.message ())); } - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::node_id_handshake, nano::stat::dir::in); - auto error (false); - nano::bufferstream stream (receive_buffer_a->data (), size_a); - nano::message_header header (error, stream); - // the header type should in principle be checked after checking the network bytes and the version numbers, I will not change it here since the benefits do not outweight the difficulties - if (error || header.type != nano::message_type::node_id_handshake) + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::node_id_handshake, nano::stat::dir::in); + auto error (false); + nano::bufferstream stream (receive_buffer_a->data (), size_a); + nano::message_header header (error, stream); + // the header type should in principle be checked after checking the network bytes and the version numbers, I will not change it here since the benefits do not outweight the difficulties + if (error || header.type != nano::message_type::node_id_handshake) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake message header from %1%") % endpoint_a)); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; + node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake message header from %1%") % endpoint_a)); } - if (header.network != node_l->network_params.network.current_network || header.version_using < node_l->network_params.network.protocol_version_min) + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + if (header.network != node_l->network_params.network.current_network || header.version_using < node_l->network_params.network.protocol_version_min) + { + // error handling, either the networks bytes or the version is wrong + if (header.network == node_l->network_params.network.current_network) { - // error handling, either the networks bytes or the version is wrong - if (header.network == node_l->network_params.network.current_network) - { - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network); - } - else - { - node_l->stats.inc (nano::stat::type::message, nano::stat::detail::outdated_version); - } + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::invalid_network); + } + else + { + node_l->stats.inc (nano::stat::type::message, nano::stat::detail::outdated_version); + } - cleanup_node_id_handshake_socket (endpoint_a); - // Cleanup attempt - { - nano::lock_guard lock (node_l->network.tcp_channels.mutex); - node_l->network.tcp_channels.attempts.get ().erase (nano::transport::map_endpoint_to_tcp (endpoint_a)); - } - return; - } - nano::node_id_handshake message (error, stream, header); - if (error || !message.response || !message.query) + cleanup_node_id_handshake_socket (endpoint_a); + // Cleanup attempt { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; + nano::lock_guard lock (node_l->network.tcp_channels.mutex); + node_l->network.tcp_channels.attempts.get ().erase (nano::transport::map_endpoint_to_tcp (endpoint_a)); } - channel_a->set_network_version (header.version_using); - auto node_id (message.response->first); - bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); - if (process) + return; + } + nano::node_id_handshake message (error, stream, header); + if (error || !message.response || !message.query) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) { - /* If node ID is known, don't establish new connection + node_l->logger.try_log (boost::str (boost::format ("Error reading node_id_handshake from %1%") % endpoint_a)); + } + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + channel_a->set_network_version (header.version_using); + auto node_id (message.response->first); + bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); + if (process) + { + /* If node ID is known, don't establish new connection Exception: temporary channels from bootstrap_server */ - auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); - if (existing_channel) - { - process = existing_channel->temporary; - } - } - if (process) + auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); + if (existing_channel) { - channel_a->set_node_id (node_id); - channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); - boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); - nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); - if (node_l->config.logging.network_node_id_handshake_logging ()) + process = existing_channel->temporary; + } + } + if (process) + { + channel_a->set_node_id (node_id); + channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); + boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); + nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); + if (node_l->config.logging.network_node_id_handshake_logging ()) + { + node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); + } + channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { + if (auto node_l = node_w.lock ()) { - node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); - } - channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { - if (auto node_l = node_w.lock ()) + if (ec || !channel_a) { - if (ec || !channel_a) + if (node_l->config.logging.network_node_id_handshake_logging ()) { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; + node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); } - // Insert new node ID connection - if (auto socket_l = channel_a->socket.lock ()) - { - channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); - auto response_server = std::make_shared (socket_l, node_l); - node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); - // Listen for possible responses - response_server->socket->type_set (nano::socket::type_t::realtime_response_server); - response_server->remote_node_id = channel_a->get_node_id (); - response_server->start (); + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + // Insert new node ID connection + if (auto socket_l = channel_a->socket.lock ()) + { + channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); + auto response_server = std::make_shared (socket_l, node_l); + node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); + // Listen for possible responses + response_server->socket->type_set (nano::socket::type_t::realtime_response_server); + response_server->remote_node_id = channel_a->get_node_id (); + response_server->start (); - if (!node_l->flags.disable_initial_telemetry_requests) - { - node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { - // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers - }); - } + if (!node_l->flags.disable_initial_telemetry_requests) + { + node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { + // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers + }); } } - }); - } + } + }); } }); } From faa1066937a5356e1b642c12d9ca8b904387cabf Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Mon, 19 Sep 2022 19:19:11 -0300 Subject: [PATCH 08/11] If !process it won't hold true anymore --- nano/node/transport/tcp.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index 0c545341..80d91227 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -672,15 +672,16 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptrset_network_version (header.version_using); auto node_id (message.response->first); bool process (!node_l->network.syn_cookies.validate (endpoint_a, node_id, message.response->second) && node_id != node_l->node_id.pub); - if (process) + if (!process) { - /* If node ID is known, don't establish new connection + return; + } + /* If node ID is known, don't establish new connection Exception: temporary channels from bootstrap_server */ - auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); - if (existing_channel) - { - process = existing_channel->temporary; - } + auto existing_channel (node_l->network.tcp_channels.find_node_id (node_id)); + if (existing_channel) + { + process = existing_channel->temporary; } if (process) { From 21f8be2b0dd58caa132db825e779c934af7d040b Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Mon, 19 Sep 2022 19:25:20 -0300 Subject: [PATCH 09/11] Preemptively return if existing_channel && !existing_channel->temporary process may only be set to true iff existing_channel == true, so we can exit if (existing_channel && !existing_channel->temporary) --- nano/node/transport/tcp.cpp | 71 ++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index 80d91227..0c36be9e 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -679,52 +679,49 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptrnetwork.tcp_channels.find_node_id (node_id)); - if (existing_channel) + if (existing_channel && !existing_channel->temporary) { - process = existing_channel->temporary; + return; } - if (process) + channel_a->set_node_id (node_id); + channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); + boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); + nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); + if (node_l->config.logging.network_node_id_handshake_logging ()) { - channel_a->set_node_id (node_id); - channel_a->set_last_packet_received (std::chrono::steady_clock::now ()); - boost::optional> response (std::make_pair (node_l->node_id.pub, nano::sign_message (node_l->node_id.prv, node_l->node_id.pub, *message.query))); - nano::node_id_handshake response_message (node_l->network_params.network, boost::none, response); - if (node_l->config.logging.network_node_id_handshake_logging ()) + node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); + } + channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { + if (auto node_l = node_w.lock ()) { - node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); - } - channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { - if (auto node_l = node_w.lock ()) + if (ec || !channel_a) { - if (ec || !channel_a) + if (node_l->config.logging.network_node_id_handshake_logging ()) { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; + node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); } - // Insert new node ID connection - if (auto socket_l = channel_a->socket.lock ()) - { - channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); - auto response_server = std::make_shared (socket_l, node_l); - node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); - // Listen for possible responses - response_server->socket->type_set (nano::socket::type_t::realtime_response_server); - response_server->remote_node_id = channel_a->get_node_id (); - response_server->start (); + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + // Insert new node ID connection + if (auto socket_l = channel_a->socket.lock ()) + { + channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); + auto response_server = std::make_shared (socket_l, node_l); + node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); + // Listen for possible responses + response_server->socket->type_set (nano::socket::type_t::realtime_response_server); + response_server->remote_node_id = channel_a->get_node_id (); + response_server->start (); - if (!node_l->flags.disable_initial_telemetry_requests) - { - node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { - // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers - }); - } + if (!node_l->flags.disable_initial_telemetry_requests) + { + node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { + // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers + }); } } - }); - } + } + }); }); } From fdc8b3d12108281defdac3b8db502e18a4a196db Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Tue, 20 Sep 2022 13:35:57 -0300 Subject: [PATCH 10/11] Return if !node_l in channel_a->send() --- nano/node/transport/tcp.cpp | 52 +++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index 0c36be9e..dc303848 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -677,7 +677,7 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptrnetwork.tcp_channels.find_node_id (node_id)); if (existing_channel && !existing_channel->temporary) { @@ -692,34 +692,36 @@ Exception: temporary channels from bootstrap_server */ node_l->logger.try_log (boost::str (boost::format ("Node ID handshake response sent with node ID %1% to %2%: query %3%") % node_l->node_id.pub.to_node_id () % endpoint_a % (*message.query).to_string ())); } channel_a->send (response_message, [node_w, channel_a, endpoint_a, cleanup_node_id_handshake_socket] (boost::system::error_code const & ec, std::size_t size_a) { - if (auto node_l = node_w.lock ()) + auto node_l = node_w.lock (); + if (!node_l) { - if (ec || !channel_a) + return; + } + if (ec || !channel_a) + { + if (node_l->config.logging.network_node_id_handshake_logging ()) { - if (node_l->config.logging.network_node_id_handshake_logging ()) - { - node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); - } - cleanup_node_id_handshake_socket (endpoint_a); - return; + node_l->logger.try_log (boost::str (boost::format ("Error sending node_id_handshake to %1%: %2%") % endpoint_a % ec.message ())); } - // Insert new node ID connection - if (auto socket_l = channel_a->socket.lock ()) - { - channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); - auto response_server = std::make_shared (socket_l, node_l); - node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); - // Listen for possible responses - response_server->socket->type_set (nano::socket::type_t::realtime_response_server); - response_server->remote_node_id = channel_a->get_node_id (); - response_server->start (); + cleanup_node_id_handshake_socket (endpoint_a); + return; + } + // Insert new node ID connection + if (auto socket_l = channel_a->socket.lock ()) + { + channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); + auto response_server = std::make_shared (socket_l, node_l); + node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); + // Listen for possible responses + response_server->socket->type_set (nano::socket::type_t::realtime_response_server); + response_server->remote_node_id = channel_a->get_node_id (); + response_server->start (); - if (!node_l->flags.disable_initial_telemetry_requests) - { - node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { - // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers - }); - } + if (!node_l->flags.disable_initial_telemetry_requests) + { + node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { + // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers + }); } } }); From f49807dea5df26ea3da55c3f67335b7fb2789379 Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Tue, 20 Sep 2022 13:38:27 -0300 Subject: [PATCH 11/11] Return if !socket_l in channel_a->send() --- nano/node/transport/tcp.cpp | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/nano/node/transport/tcp.cpp b/nano/node/transport/tcp.cpp index dc303848..351e5ba7 100644 --- a/nano/node/transport/tcp.cpp +++ b/nano/node/transport/tcp.cpp @@ -707,22 +707,24 @@ void nano::transport::tcp_channels::start_tcp_receive_node_id (std::shared_ptrsocket.lock ()) + auto socket_l = channel_a->socket.lock (); + if (!socket_l) { - channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); - auto response_server = std::make_shared (socket_l, node_l); - node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); - // Listen for possible responses - response_server->socket->type_set (nano::socket::type_t::realtime_response_server); - response_server->remote_node_id = channel_a->get_node_id (); - response_server->start (); + return; + } + channel_a->set_last_packet_sent (std::chrono::steady_clock::now ()); + auto response_server = std::make_shared (socket_l, node_l); + node_l->network.tcp_channels.insert (channel_a, socket_l, response_server); + // Listen for possible responses + response_server->socket->type_set (nano::socket::type_t::realtime_response_server); + response_server->remote_node_id = channel_a->get_node_id (); + response_server->start (); - if (!node_l->flags.disable_initial_telemetry_requests) - { - node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { - // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers - }); - } + if (!node_l->flags.disable_initial_telemetry_requests) + { + node_l->telemetry->get_metrics_single_peer_async (channel_a, [] (nano::telemetry_data_response const &) { + // Intentionally empty, starts the telemetry request cycle to more quickly disconnect from invalid peers + }); } }); });