From d30dcea56b29301164f3a096007de2d1395c40ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Fri, 3 May 2024 11:44:28 +0200 Subject: [PATCH 1/4] Dead code `tcp_channels::modify` --- nano/core_test/peer_container.cpp | 8 ++------ nano/node/transport/tcp_channels.cpp | 12 ------------ nano/node/transport/tcp_channels.hpp | 1 - 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/nano/core_test/peer_container.cpp b/nano/core_test/peer_container.cpp index 5418ba98c8..44f7993b7a 100644 --- a/nano/core_test/peer_container.cpp +++ b/nano/core_test/peer_container.cpp @@ -79,16 +79,12 @@ TEST (peer_container, tcp_channel_cleanup_works) ASSERT_NE (nullptr, channel1); // set the last packet sent for channel1 only to guarantee it contains a value. // it won't be necessarily the same use by the cleanup cutoff time - node1.network.tcp_channels.modify (channel1, [&now] (auto channel) { - channel->set_last_packet_sent (now - std::chrono::seconds (5)); - }); + channel1->set_last_packet_sent (now - std::chrono::seconds (5)); auto channel2 = nano::test::establish_tcp (system, node1, outer_node2->network.endpoint ()); ASSERT_NE (nullptr, channel2); // set the last packet sent for channel2 only to guarantee it contains a value. // it won't be necessarily the same use by the cleanup cutoff time - node1.network.tcp_channels.modify (channel2, [&now] (auto channel) { - channel->set_last_packet_sent (now + std::chrono::seconds (1)); - }); + channel2->set_last_packet_sent (now + std::chrono::seconds (1)); ASSERT_EQ (2, node1.network.size ()); ASSERT_EQ (2, node1.network.tcp_channels.size ()); diff --git a/nano/node/transport/tcp_channels.cpp b/nano/node/transport/tcp_channels.cpp index 7a41280563..2d1f147f03 100644 --- a/nano/node/transport/tcp_channels.cpp +++ b/nano/node/transport/tcp_channels.cpp @@ -417,18 +417,6 @@ void nano::transport::tcp_channels::list (std::deque const & channel_a, std::function const &)> modify_callback_a) -{ - nano::lock_guard lock{ mutex }; - auto existing (channels.get ().find (channel_a->get_tcp_endpoint ())); - if (existing != channels.get ().end ()) - { - channels.get ().modify (existing, [modify_callback = std::move (modify_callback_a)] (channel_entry & wrapper_a) { - modify_callback (wrapper_a.channel); - }); - } -} - void nano::transport::tcp_channels::start_tcp (nano::endpoint const & endpoint) { node.tcp_listener.connect (endpoint.address (), endpoint.port ()); diff --git a/nano/node/transport/tcp_channels.hpp b/nano/node/transport/tcp_channels.hpp index 7afd89fc79..0e18d75e82 100644 --- a/nano/node/transport/tcp_channels.hpp +++ b/nano/node/transport/tcp_channels.hpp @@ -50,7 +50,6 @@ class tcp_channels final bool track_reachout (nano::endpoint const &); void purge (std::chrono::steady_clock::time_point cutoff_deadline); void list (std::deque> &, uint8_t = 0, bool = true); - void modify (std::shared_ptr const &, std::function const &)>); void keepalive (); std::optional sample_keepalive (); From 28bc44d95019b7efc9c812cb20b4b269ad47f53d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Fri, 3 May 2024 11:41:29 +0200 Subject: [PATCH 2/4] Cleanup channel endpoints --- nano/core_test/network.cpp | 5 ++--- nano/core_test/node.cpp | 8 ++++---- nano/core_test/request_aggregator.cpp | 2 +- nano/core_test/telemetry.cpp | 18 +++++++++--------- nano/core_test/websocket.cpp | 2 +- nano/node/bootstrap/bootstrap_connections.cpp | 4 ++-- nano/node/bootstrap/bootstrap_legacy.cpp | 2 +- nano/node/json_handler.cpp | 2 +- nano/node/message_processor.cpp | 2 +- nano/node/network.cpp | 4 ++-- nano/node/repcrawler.cpp | 2 +- nano/node/telemetry.cpp | 2 +- nano/node/transport/channel.cpp | 17 ++++++++--------- nano/node/transport/channel.hpp | 6 +++--- nano/node/transport/fake.hpp | 7 +------ nano/node/transport/inproc.hpp | 7 +------ nano/node/transport/tcp_channel.cpp | 6 +++--- nano/node/transport/tcp_channel.hpp | 11 +++-------- nano/node/transport/tcp_channels.cpp | 4 ++-- nano/node/transport/tcp_channels.hpp | 2 +- nano/node/websocket.cpp | 2 +- nano/qt/qt.cpp | 2 +- nano/rpc_test/rpc.cpp | 2 +- nano/slow_test/node.cpp | 8 ++++---- 24 files changed, 55 insertions(+), 72 deletions(-) diff --git a/nano/core_test/network.cpp b/nano/core_test/network.cpp index eb4713f946..9254e8cd10 100644 --- a/nano/core_test/network.cpp +++ b/nano/core_test/network.cpp @@ -1024,14 +1024,13 @@ TEST (network, loopback_channel) auto & node2 = *system.nodes[1]; nano::transport::inproc::channel channel1 (node1, node1); ASSERT_EQ (channel1.get_type (), nano::transport::transport_type::loopback); - ASSERT_EQ (channel1.get_endpoint (), node1.network.endpoint ()); - ASSERT_EQ (channel1.get_tcp_endpoint (), nano::transport::map_endpoint_to_tcp (node1.network.endpoint ())); + ASSERT_EQ (channel1.get_remote_endpoint (), node1.network.endpoint ()); ASSERT_EQ (channel1.get_network_version (), node1.network_params.network.protocol_version); ASSERT_EQ (channel1.get_node_id (), node1.node_id.pub); ASSERT_EQ (channel1.get_node_id_optional ().value_or (0), node1.node_id.pub); nano::transport::inproc::channel channel2 (node2, node2); ++node1.network.port; - ASSERT_NE (channel1.get_endpoint (), node1.network.endpoint ()); + ASSERT_NE (channel1.get_remote_endpoint (), node1.network.endpoint ()); } // Ensure the network filters messages with the incorrect magic number diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index 54d1ef29f6..c7654c895e 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -2704,7 +2704,7 @@ TEST (node, peer_history_restart) ASSERT_TIMELY (10s, !node2->network.empty ()); // Confirm that the peers match with the endpoints we are expecting auto list (node2->network.list (2)); - ASSERT_EQ (node1->network.endpoint (), list[0]->get_endpoint ()); + ASSERT_EQ (node1->network.endpoint (), list[0]->get_remote_endpoint ()); ASSERT_EQ (1, node2->network.size ()); system.stop_node (*node2); } @@ -2727,7 +2727,7 @@ TEST (node, peer_history_restart) ASSERT_TIMELY (10s, !node3->network.empty ()); // Confirm that the peers match with the endpoints we are expecting auto list (node3->network.list (2)); - ASSERT_EQ (node1->network.endpoint (), list[0]->get_endpoint ()); + ASSERT_EQ (node1->network.endpoint (), list[0]->get_remote_endpoint ()); ASSERT_EQ (1, node3->network.size ()); system.stop_node (*node3); } @@ -2789,11 +2789,11 @@ TEST (node, bidirectional_tcp) ASSERT_EQ (1, node2->network.size ()); auto list1 (node1->network.list (1)); ASSERT_EQ (nano::transport::transport_type::tcp, list1[0]->get_type ()); - ASSERT_NE (node2->network.endpoint (), list1[0]->get_endpoint ()); // Ephemeral port + ASSERT_NE (node2->network.endpoint (), list1[0]->get_remote_endpoint ()); // Ephemeral port ASSERT_EQ (node2->node_id.pub, list1[0]->get_node_id ()); auto list2 (node2->network.list (1)); ASSERT_EQ (nano::transport::transport_type::tcp, list2[0]->get_type ()); - ASSERT_EQ (node1->network.endpoint (), list2[0]->get_endpoint ()); + ASSERT_EQ (node1->network.endpoint (), list2[0]->get_remote_endpoint ()); ASSERT_EQ (node1->node_id.pub, list2[0]->get_node_id ()); // Test block propagation from node 1 nano::keypair key; diff --git a/nano/core_test/request_aggregator.cpp b/nano/core_test/request_aggregator.cpp index b0da7dd3b6..2f85074e40 100644 --- a/nano/core_test/request_aggregator.cpp +++ b/nano/core_test/request_aggregator.cpp @@ -230,7 +230,7 @@ TEST (request_aggregator, two_endpoints) auto dummy_channel1 = std::make_shared (node1, node1); auto dummy_channel2 = std::make_shared (node2, node2); - ASSERT_NE (nano::transport::map_endpoint_to_v6 (dummy_channel1->get_endpoint ()), nano::transport::map_endpoint_to_v6 (dummy_channel2->get_endpoint ())); + ASSERT_NE (nano::transport::map_endpoint_to_v6 (dummy_channel1->get_remote_endpoint ()), nano::transport::map_endpoint_to_v6 (dummy_channel2->get_remote_endpoint ())); std::vector> request{ { send1->hash (), send1->root () } }; diff --git a/nano/core_test/telemetry.cpp b/nano/core_test/telemetry.cpp index 4ebe65c5c9..d5b898b5b9 100644 --- a/nano/core_test/telemetry.cpp +++ b/nano/core_test/telemetry.cpp @@ -68,18 +68,18 @@ TEST (telemetry, basic) ASSERT_NE (nullptr, channel); std::optional telemetry_data; - ASSERT_TIMELY (5s, telemetry_data = node_client->telemetry.get_telemetry (channel->get_endpoint ())); + ASSERT_TIMELY (5s, telemetry_data = node_client->telemetry.get_telemetry (channel->get_remote_endpoint ())); ASSERT_EQ (node_server->get_node_id (), telemetry_data->node_id); // Check the metrics are correct ASSERT_TRUE (nano::test::compare_telemetry (*telemetry_data, *node_server)); // Call again straight away - auto telemetry_data_2 = node_client->telemetry.get_telemetry (channel->get_endpoint ()); + auto telemetry_data_2 = node_client->telemetry.get_telemetry (channel->get_remote_endpoint ()); ASSERT_TRUE (telemetry_data_2); // Call again straight away - auto telemetry_data_3 = node_client->telemetry.get_telemetry (channel->get_endpoint ()); + auto telemetry_data_3 = node_client->telemetry.get_telemetry (channel->get_remote_endpoint ()); ASSERT_TRUE (telemetry_data_3); // we expect at least one consecutive repeat of telemetry @@ -89,7 +89,7 @@ TEST (telemetry, basic) WAIT (3s); std::optional telemetry_data_4; - ASSERT_TIMELY (5s, telemetry_data_4 = node_client->telemetry.get_telemetry (channel->get_endpoint ())); + ASSERT_TIMELY (5s, telemetry_data_4 = node_client->telemetry.get_telemetry (channel->get_remote_endpoint ())); ASSERT_NE (*telemetry_data, *telemetry_data_4); } @@ -120,13 +120,13 @@ TEST (telemetry, disconnected) ASSERT_NE (nullptr, channel); // Ensure telemetry is available before disconnecting - ASSERT_TIMELY (5s, node_client->telemetry.get_telemetry (channel->get_endpoint ())); + ASSERT_TIMELY (5s, node_client->telemetry.get_telemetry (channel->get_remote_endpoint ())); system.stop_node (*node_server); ASSERT_TRUE (channel); // Ensure telemetry from disconnected peer is removed - ASSERT_TIMELY (5s, !node_client->telemetry.get_telemetry (channel->get_endpoint ())); + ASSERT_TIMELY (5s, !node_client->telemetry.get_telemetry (channel->get_remote_endpoint ())); } TEST (telemetry, dos_tcp) @@ -185,14 +185,14 @@ TEST (telemetry, disable_metrics) node_client->telemetry.trigger (); - ASSERT_NEVER (1s, node_client->telemetry.get_telemetry (channel->get_endpoint ())); + ASSERT_NEVER (1s, node_client->telemetry.get_telemetry (channel->get_remote_endpoint ())); // It should still be able to receive metrics though auto channel1 = node_server->network.find_node_id (node_client->get_node_id ()); ASSERT_NE (nullptr, channel1); std::optional telemetry_data; - ASSERT_TIMELY (5s, telemetry_data = node_server->telemetry.get_telemetry (channel1->get_endpoint ())); + ASSERT_TIMELY (5s, telemetry_data = node_server->telemetry.get_telemetry (channel1->get_remote_endpoint ())); ASSERT_TRUE (nano::test::compare_telemetry (*telemetry_data, *node_client)); } @@ -237,7 +237,7 @@ TEST (telemetry, maker_pruning) ASSERT_NE (nullptr, channel); std::optional telemetry_data; - ASSERT_TIMELY (5s, telemetry_data = node_client->telemetry.get_telemetry (channel->get_endpoint ())); + ASSERT_TIMELY (5s, telemetry_data = node_client->telemetry.get_telemetry (channel->get_remote_endpoint ())); ASSERT_EQ (node_server->get_node_id (), telemetry_data->node_id); // Ensure telemetry response indicates pruned node diff --git a/nano/core_test/websocket.cpp b/nano/core_test/websocket.cpp index dacfd7b10e..50d30d9534 100644 --- a/nano/core_test/websocket.cpp +++ b/nano/core_test/websocket.cpp @@ -1005,7 +1005,7 @@ TEST (websocket, telemetry) auto channel = node1->network.find_node_id (node2->get_node_id ()); ASSERT_NE (channel, nullptr); - ASSERT_TIMELY (5s, node1->telemetry.get_telemetry (channel->get_endpoint ())); + ASSERT_TIMELY (5s, node1->telemetry.get_telemetry (channel->get_remote_endpoint ())); ASSERT_TIMELY_EQ (10s, future.wait_for (0s), std::future_status::ready); diff --git a/nano/node/bootstrap/bootstrap_connections.cpp b/nano/node/bootstrap/bootstrap_connections.cpp index dfbdff12f9..358538a651 100644 --- a/nano/node/bootstrap/bootstrap_connections.cpp +++ b/nano/node/bootstrap/bootstrap_connections.cpp @@ -102,7 +102,7 @@ void nano::bootstrap_connections::pool_connection (std::shared_ptr lock{ mutex }; auto const & socket_l = client_a->socket; - if (!stopped && !client_a->pending_stop && !node.network.excluded_peers.check (client_a->channel->get_tcp_endpoint ())) + if (!stopped && !client_a->pending_stop && !node.network.excluded_peers.check (client_a->channel->get_remote_endpoint ())) { socket_l->set_timeout (node.network_params.network.idle_timeout); // Push into idle deque @@ -138,7 +138,7 @@ std::shared_ptr nano::bootstrap_connections::find_connec std::shared_ptr result; for (auto i (idle.begin ()), end (idle.end ()); i != end && !stopped; ++i) { - if ((*i)->channel->get_tcp_endpoint () == endpoint_a) + if ((*i)->channel->get_remote_endpoint () == endpoint_a) { result = *i; idle.erase (i); diff --git a/nano/node/bootstrap/bootstrap_legacy.cpp b/nano/node/bootstrap/bootstrap_legacy.cpp index 6ceb30394b..47368eccf2 100644 --- a/nano/node/bootstrap/bootstrap_legacy.cpp +++ b/nano/node/bootstrap/bootstrap_legacy.cpp @@ -138,7 +138,7 @@ bool nano::bootstrap_attempt_legacy::request_frontier (nano::unique_lockchannel->get_tcp_endpoint (); + endpoint_frontier_request = connection_l->channel->get_remote_endpoint (); std::future future; { auto this_l = std::dynamic_pointer_cast (shared_from_this ()); diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index 198bdeac13..d8a253a7ab 100644 --- a/nano/node/json_handler.cpp +++ b/nano/node/json_handler.cpp @@ -2991,7 +2991,7 @@ void nano::json_handler::peers () bool const peer_details = request.get ("peer_details", false); auto peers_list (node.network.list (std::numeric_limits::max ())); std::sort (peers_list.begin (), peers_list.end (), [] (auto const & lhs, auto const & rhs) { - return lhs->get_endpoint () < rhs->get_endpoint (); + return lhs->get_remote_endpoint () < rhs->get_remote_endpoint (); }); for (auto i (peers_list.begin ()), n (peers_list.end ()); i != n; ++i) { diff --git a/nano/node/message_processor.cpp b/nano/node/message_processor.cpp index bf3dce51a0..469c55bd18 100644 --- a/nano/node/message_processor.cpp +++ b/nano/node/message_processor.cpp @@ -175,7 +175,7 @@ class process_visitor : public nano::message_visitor if (peer0.address () == boost::asio::ip::address_v6{} && peer0.port () != 0) { // TODO: Remove this as we do not need to establish a second connection to the same peer - nano::endpoint new_endpoint (channel->get_tcp_endpoint ().address (), peer0.port ()); + nano::endpoint new_endpoint (channel->get_remote_endpoint ().address (), peer0.port ()); node.network.merge_peer (new_endpoint); // Remember this for future forwarding to other peers diff --git a/nano/node/network.cpp b/nano/node/network.cpp index d780f634e8..7b0435bd2c 100644 --- a/nano/node/network.cpp +++ b/nano/node/network.cpp @@ -504,14 +504,14 @@ void nano::network::erase (nano::transport::channel const & channel_a) auto const channel_type = channel_a.get_type (); if (channel_type == nano::transport::transport_type::tcp) { - tcp_channels.erase (channel_a.get_tcp_endpoint ()); + tcp_channels.erase (channel_a.get_remote_endpoint ()); } } void nano::network::exclude (std::shared_ptr const & channel) { // Add to peer exclusion list - excluded_peers.add (channel->get_tcp_endpoint ()); + excluded_peers.add (channel->get_remote_endpoint ()); // Disconnect erase (*channel); diff --git a/nano/node/repcrawler.cpp b/nano/node/repcrawler.cpp index 16d72b17a1..b3dc58c405 100644 --- a/nano/node/repcrawler.cpp +++ b/nano/node/repcrawler.cpp @@ -102,7 +102,7 @@ void nano::rep_crawler::validate_and_process (nano::unique_lock & l rep.last_response = std::chrono::steady_clock::now (); // Update if representative channel was changed - if (rep.channel->get_endpoint () != channel->get_endpoint ()) + if (rep.channel->get_remote_endpoint () != channel->get_remote_endpoint ()) { debug_assert (rep.account == vote->account); updated = true; diff --git a/nano/node/telemetry.cpp b/nano/node/telemetry.cpp index 1ae2a59be0..593454241f 100644 --- a/nano/node/telemetry.cpp +++ b/nano/node/telemetry.cpp @@ -96,7 +96,7 @@ void nano::telemetry::process (const nano::telemetry_ack & telemetry, const std: nano::unique_lock lock{ mutex }; - const auto endpoint = channel->get_endpoint (); + const auto endpoint = channel->get_remote_endpoint (); if (auto it = telemetries.get ().find (endpoint); it != telemetries.get ().end ()) { diff --git a/nano/node/transport/channel.cpp b/nano/node/transport/channel.cpp index 1837044490..60c758ba54 100644 --- a/nano/node/transport/channel.cpp +++ b/nano/node/transport/channel.cpp @@ -51,21 +51,20 @@ void nano::transport::channel::set_peering_endpoint (nano::endpoint endpoint) nano::endpoint nano::transport::channel::get_peering_endpoint () const { - nano::unique_lock lock{ channel_mutex }; - if (peering_endpoint) { - return *peering_endpoint; - } - else - { - lock.unlock (); - return get_endpoint (); + nano::lock_guard lock{ channel_mutex }; + if (peering_endpoint) + { + return *peering_endpoint; + } } + return get_remote_endpoint (); } void nano::transport::channel::operator() (nano::object_stream & obs) const { - obs.write ("endpoint", get_endpoint ()); + obs.write ("remote_endpoint", get_remote_endpoint ()); + obs.write ("local_endpoint", get_local_endpoint ()); obs.write ("peering_endpoint", get_peering_endpoint ()); obs.write ("node_id", get_node_id ().to_node_id ()); } diff --git a/nano/node/transport/channel.hpp b/nano/node/transport/channel.hpp index e01cc8c579..cb04586660 100644 --- a/nano/node/transport/channel.hpp +++ b/nano/node/transport/channel.hpp @@ -40,10 +40,10 @@ class channel virtual void close () = 0; - virtual std::string to_string () const = 0; - virtual nano::endpoint get_endpoint () const = 0; - virtual nano::tcp_endpoint get_tcp_endpoint () const = 0; + virtual nano::endpoint get_remote_endpoint () const = 0; virtual nano::endpoint get_local_endpoint () const = 0; + + virtual std::string to_string () const = 0; virtual nano::transport::transport_type get_type () const = 0; virtual bool max (nano::transport::traffic_type = nano::transport::traffic_type::generic) diff --git a/nano/node/transport/fake.hpp b/nano/node/transport/fake.hpp index a1c95b55f5..d9ce585cbb 100644 --- a/nano/node/transport/fake.hpp +++ b/nano/node/transport/fake.hpp @@ -30,16 +30,11 @@ namespace transport endpoint = endpoint_a; } - nano::endpoint get_endpoint () const override + nano::endpoint get_remote_endpoint () const override { return endpoint; } - nano::tcp_endpoint get_tcp_endpoint () const override - { - return nano::transport::map_endpoint_to_tcp (endpoint); - } - nano::endpoint get_local_endpoint () const override { return endpoint; diff --git a/nano/node/transport/inproc.hpp b/nano/node/transport/inproc.hpp index 9ae5670293..d93fbed2d5 100644 --- a/nano/node/transport/inproc.hpp +++ b/nano/node/transport/inproc.hpp @@ -22,16 +22,11 @@ namespace transport std::string to_string () const override; - nano::endpoint get_endpoint () const override + nano::endpoint get_remote_endpoint () const override { return endpoint; } - nano::tcp_endpoint get_tcp_endpoint () const override - { - return nano::transport::map_endpoint_to_tcp (endpoint); - } - nano::endpoint get_local_endpoint () const override { return endpoint; diff --git a/nano/node/transport/tcp_channel.cpp b/nano/node/transport/tcp_channel.cpp index 57f8811ba4..3d4d063838 100644 --- a/nano/node/transport/tcp_channel.cpp +++ b/nano/node/transport/tcp_channel.cpp @@ -28,12 +28,12 @@ void nano::transport::tcp_channel::update_endpoints () { nano::lock_guard lk (channel_mutex); - debug_assert (endpoint == nano::endpoint{}); // Not initialized endpoint value + debug_assert (remote_endpoint == nano::endpoint{}); // Not initialized endpoint value debug_assert (local_endpoint == nano::endpoint{}); // Not initialized endpoint value if (auto socket_l = socket.lock ()) { - endpoint = socket_l->remote_endpoint (); + remote_endpoint = socket_l->remote_endpoint (); local_endpoint = socket_l->local_endpoint (); } } @@ -90,7 +90,7 @@ void nano::transport::tcp_channel::send_buffer (nano::shared_const_buffer const std::string nano::transport::tcp_channel::to_string () const { - return nano::util::to_str (get_tcp_endpoint ()); + return nano::util::to_str (get_remote_endpoint ()); } void nano::transport::tcp_channel::operator() (nano::object_stream & obs) const diff --git a/nano/node/transport/tcp_channel.hpp b/nano/node/transport/tcp_channel.hpp index b72db56546..ea77f48115 100644 --- a/nano/node/transport/tcp_channel.hpp +++ b/nano/node/transport/tcp_channel.hpp @@ -24,15 +24,10 @@ class tcp_channel : public nano::transport::channel, public std::enable_shared_f std::string to_string () const override; - nano::endpoint get_endpoint () const override - { - return nano::transport::map_tcp_to_endpoint (get_tcp_endpoint ()); - } - - nano::tcp_endpoint get_tcp_endpoint () const override + nano::endpoint get_remote_endpoint () const override { nano::lock_guard lk (channel_mutex); - return endpoint; + return remote_endpoint; } nano::endpoint get_local_endpoint () const override @@ -77,7 +72,7 @@ class tcp_channel : public nano::transport::channel, public std::enable_shared_f std::weak_ptr socket; private: - nano::endpoint endpoint; + nano::endpoint remote_endpoint; nano::endpoint local_endpoint; public: // Logging diff --git a/nano/node/transport/tcp_channels.cpp b/nano/node/transport/tcp_channels.cpp index 2d1f147f03..4b983cb7bb 100644 --- a/nano/node/transport/tcp_channels.cpp +++ b/nano/node/transport/tcp_channels.cpp @@ -196,9 +196,9 @@ void nano::transport::tcp_channels::random_fill (std::array & auto j (target_a.begin ()); for (auto i (peers.begin ()), n (peers.end ()); i != n; ++i, ++j) { - debug_assert ((*i)->get_endpoint ().address ().is_v6 ()); + debug_assert ((*i)->get_remote_endpoint ().address ().is_v6 ()); debug_assert (j < target_a.end ()); - *j = (*i)->get_endpoint (); + *j = (*i)->get_remote_endpoint (); } } diff --git a/nano/node/transport/tcp_channels.hpp b/nano/node/transport/tcp_channels.hpp index 0e18d75e82..1b6f1e363d 100644 --- a/nano/node/transport/tcp_channels.hpp +++ b/nano/node/transport/tcp_channels.hpp @@ -80,7 +80,7 @@ class tcp_channels final } nano::tcp_endpoint endpoint () const { - return channel->get_tcp_endpoint (); + return channel->get_remote_endpoint (); } std::chrono::steady_clock::time_point last_bootstrap_attempt () const { diff --git a/nano/node/websocket.cpp b/nano/node/websocket.cpp index 76d19aa80d..d2b8eab72d 100644 --- a/nano/node/websocket.cpp +++ b/nano/node/websocket.cpp @@ -1050,7 +1050,7 @@ nano::websocket_server::websocket_server (nano::websocket::config & config_a, na if (server->any_subscriber (nano::websocket::topic::telemetry)) { nano::websocket::message_builder builder; - server->broadcast (builder.telemetry_received (telemetry_data, channel->get_endpoint ())); + server->broadcast (builder.telemetry_received (telemetry_data, channel->get_remote_endpoint ())); } }); diff --git a/nano/qt/qt.cpp b/nano/qt/qt.cpp index b7e1e13be1..763a6e5cf0 100644 --- a/nano/qt/qt.cpp +++ b/nano/qt/qt.cpp @@ -1944,7 +1944,7 @@ void nano_qt::advanced_actions::refresh_peers () peers_model->removeRows (0, peers_model->rowCount ()); auto list (wallet.node.network.list (std::numeric_limits::max ())); std::sort (list.begin (), list.end (), [] (auto const & lhs, auto const & rhs) { - return lhs->get_endpoint () < rhs->get_endpoint (); + return lhs->get_remote_endpoint () < rhs->get_remote_endpoint (); }); for (auto i (list.begin ()), n (list.end ()); i != n; ++i) { diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp index 5539efc96b..1efdeacaff 100644 --- a/nano/rpc_test/rpc.cpp +++ b/nano/rpc_test/rpc.cpp @@ -6786,7 +6786,7 @@ TEST (rpc, telemetry_all) auto channel = node1->network.find_node_id (node->get_node_id ()); ASSERT_TRUE (channel); - ASSERT_TIMELY (10s, node1->telemetry.get_telemetry (channel->get_endpoint ())); + ASSERT_TIMELY (10s, node1->telemetry.get_telemetry (channel->get_remote_endpoint ())); boost::property_tree::ptree request; request.put ("action", "telemetry"); diff --git a/nano/slow_test/node.cpp b/nano/slow_test/node.cpp index 4e4781d412..945b661cf7 100644 --- a/nano/slow_test/node.cpp +++ b/nano/slow_test/node.cpp @@ -1383,7 +1383,7 @@ namespace transport // Pick first peer to be consistent auto peer = data.node->network.tcp_channels.channels[0].channel; - auto maybe_telemetry = data.node->telemetry.get_telemetry (peer->get_endpoint ()); + auto maybe_telemetry = data.node->telemetry.get_telemetry (peer->get_remote_endpoint ()); if (maybe_telemetry) { callback_process (shared_data, data, node_data, maybe_telemetry->timestamp); @@ -1513,7 +1513,7 @@ TEST (telemetry, cache_read_and_timeout) ASSERT_NE (channel, nullptr); node_client->telemetry.trigger (); - ASSERT_TIMELY (5s, telemetry_data = node_client->telemetry.get_telemetry (channel->get_endpoint ())); + ASSERT_TIMELY (5s, telemetry_data = node_client->telemetry.get_telemetry (channel->get_remote_endpoint ())); auto responses = node_client->telemetry.get_all_telemetries (); ASSERT_TRUE (!responses.empty ()); @@ -1536,7 +1536,7 @@ TEST (telemetry, cache_read_and_timeout) // Request telemetry metrics again node_client->telemetry.trigger (); - ASSERT_TIMELY (5s, telemetry_data = node_client->telemetry.get_telemetry (channel->get_endpoint ())); + ASSERT_TIMELY (5s, telemetry_data = node_client->telemetry.get_telemetry (channel->get_remote_endpoint ())); responses = node_client->telemetry.get_all_telemetries (); ASSERT_TRUE (!responses.empty ()); @@ -1611,7 +1611,7 @@ TEST (telemetry, many_nodes) for (auto const & peer : peers) { std::optional telemetry_data; - ASSERT_TIMELY (5s, telemetry_data = node_client->telemetry.get_telemetry (peer->get_endpoint ())); + ASSERT_TIMELY (5s, telemetry_data = node_client->telemetry.get_telemetry (peer->get_remote_endpoint ())); telemetry_datas.push_back (*telemetry_data); } From 0990dc73064ea358c45df2df976cdf211279c5e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Fri, 3 May 2024 11:51:31 +0200 Subject: [PATCH 3/4] Encapsulate channel mutex --- nano/node/transport/channel.cpp | 4 ++-- nano/node/transport/channel.hpp | 25 ++++++++++++------------- nano/node/transport/tcp_channel.cpp | 4 +--- nano/node/transport/tcp_channel.hpp | 4 ++-- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/nano/node/transport/channel.cpp b/nano/node/transport/channel.cpp index 60c758ba54..7a3328100f 100644 --- a/nano/node/transport/channel.cpp +++ b/nano/node/transport/channel.cpp @@ -45,14 +45,14 @@ void nano::transport::channel::send (nano::message & message_a, std::function lock{ channel_mutex }; + nano::lock_guard lock{ mutex }; peering_endpoint = endpoint; } nano::endpoint nano::transport::channel::get_peering_endpoint () const { { - nano::lock_guard lock{ channel_mutex }; + nano::lock_guard lock{ mutex }; if (peering_endpoint) { return *peering_endpoint; diff --git a/nano/node/transport/channel.hpp b/nano/node/transport/channel.hpp index cb04586660..157b240cfa 100644 --- a/nano/node/transport/channel.hpp +++ b/nano/node/transport/channel.hpp @@ -58,49 +58,49 @@ class channel std::chrono::steady_clock::time_point get_last_bootstrap_attempt () const { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; return last_bootstrap_attempt; } void set_last_bootstrap_attempt (std::chrono::steady_clock::time_point const time_a) { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; last_bootstrap_attempt = time_a; } std::chrono::steady_clock::time_point get_last_packet_received () const { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; return last_packet_received; } void set_last_packet_received (std::chrono::steady_clock::time_point const time_a) { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; last_packet_received = time_a; } std::chrono::steady_clock::time_point get_last_packet_sent () const { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; return last_packet_sent; } void set_last_packet_sent (std::chrono::steady_clock::time_point const time_a) { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; last_packet_sent = time_a; } boost::optional get_node_id_optional () const { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; return node_id; } nano::account get_node_id () const { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; if (node_id.is_initialized ()) { return node_id.get (); @@ -113,7 +113,7 @@ class channel void set_node_id (nano::account node_id_a) { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; node_id = node_id_a; } @@ -130,7 +130,9 @@ class channel nano::endpoint get_peering_endpoint () const; void set_peering_endpoint (nano::endpoint endpoint); - mutable nano::mutex channel_mutex; +protected: + nano::node & node; + mutable nano::mutex mutex; private: std::chrono::steady_clock::time_point last_bootstrap_attempt{ std::chrono::steady_clock::time_point () }; @@ -140,9 +142,6 @@ class channel std::atomic network_version{ 0 }; std::optional peering_endpoint{}; -protected: - nano::node & node; - public: // Logging virtual void operator() (nano::object_stream &) const; }; diff --git a/nano/node/transport/tcp_channel.cpp b/nano/node/transport/tcp_channel.cpp index 3d4d063838..cd0f3645bc 100644 --- a/nano/node/transport/tcp_channel.cpp +++ b/nano/node/transport/tcp_channel.cpp @@ -16,8 +16,6 @@ nano::transport::tcp_channel::tcp_channel (nano::node & node_a, std::weak_ptr lk{ channel_mutex }; - // Close socket. Exception: socket is used by tcp_server if (auto socket_l = socket.lock ()) { socket_l->close (); @@ -26,7 +24,7 @@ nano::transport::tcp_channel::~tcp_channel () void nano::transport::tcp_channel::update_endpoints () { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; debug_assert (remote_endpoint == nano::endpoint{}); // Not initialized endpoint value debug_assert (local_endpoint == nano::endpoint{}); // Not initialized endpoint value diff --git a/nano/node/transport/tcp_channel.hpp b/nano/node/transport/tcp_channel.hpp index ea77f48115..3936e3e9ac 100644 --- a/nano/node/transport/tcp_channel.hpp +++ b/nano/node/transport/tcp_channel.hpp @@ -26,13 +26,13 @@ class tcp_channel : public nano::transport::channel, public std::enable_shared_f nano::endpoint get_remote_endpoint () const override { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; return remote_endpoint; } nano::endpoint get_local_endpoint () const override { - nano::lock_guard lk (channel_mutex); + nano::lock_guard lock{ mutex }; return local_endpoint; } From 934cd6eba57e2a66457d96fdfd1dab5e412fca3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 7 Oct 2024 22:55:16 +0200 Subject: [PATCH 4/4] Use std:::optional --- nano/node/json_handler.cpp | 4 ++-- nano/node/transport/channel.hpp | 13 +++---------- nano/qt/qt.cpp | 4 ++-- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index d8a253a7ab..a0232c3428 100644 --- a/nano/node/json_handler.cpp +++ b/nano/node/json_handler.cpp @@ -3003,9 +3003,9 @@ void nano::json_handler::peers () boost::property_tree::ptree pending_tree; pending_tree.put ("protocol_version", std::to_string (channel->get_network_version ())); auto node_id_l (channel->get_node_id_optional ()); - if (node_id_l.is_initialized ()) + if (node_id_l.has_value ()) { - pending_tree.put ("node_id", node_id_l.get ().to_node_id ()); + pending_tree.put ("node_id", node_id_l.value ().to_node_id ()); } else { diff --git a/nano/node/transport/channel.hpp b/nano/node/transport/channel.hpp index 157b240cfa..2dbe3c8487 100644 --- a/nano/node/transport/channel.hpp +++ b/nano/node/transport/channel.hpp @@ -92,7 +92,7 @@ class channel last_packet_sent = time_a; } - boost::optional get_node_id_optional () const + std::optional get_node_id_optional () const { nano::lock_guard lock{ mutex }; return node_id; @@ -101,14 +101,7 @@ class channel nano::account get_node_id () const { nano::lock_guard lock{ mutex }; - if (node_id.is_initialized ()) - { - return node_id.get (); - } - else - { - return 0; - } + return node_id.value_or (0); } void set_node_id (nano::account node_id_a) @@ -138,7 +131,7 @@ class channel std::chrono::steady_clock::time_point last_bootstrap_attempt{ std::chrono::steady_clock::time_point () }; std::chrono::steady_clock::time_point last_packet_received{ std::chrono::steady_clock::now () }; std::chrono::steady_clock::time_point last_packet_sent{ std::chrono::steady_clock::now () }; - boost::optional node_id{ boost::none }; + std::optional node_id{}; std::atomic network_version{ 0 }; std::optional peering_endpoint{}; diff --git a/nano/qt/qt.cpp b/nano/qt/qt.cpp index 763a6e5cf0..aefd693bd0 100644 --- a/nano/qt/qt.cpp +++ b/nano/qt/qt.cpp @@ -1959,9 +1959,9 @@ void nano_qt::advanced_actions::refresh_peers () items.push_back (version); QString node_id (""); auto node_id_l (channel->get_node_id_optional ()); - if (node_id_l.is_initialized ()) + if (node_id_l.has_value ()) { - node_id = node_id_l.get ().to_account ().c_str (); + node_id = node_id_l.value ().to_account ().c_str (); } items.push_back (new QStandardItem (node_id)); peers_model->appendRow (items);