From 240b1f894df4ab712831b6cdbf69db8bce316476 Mon Sep 17 00:00:00 2001 From: clemahieu Date: Wed, 4 Aug 2021 00:05:10 +0100 Subject: [PATCH] Moving message header version assignment to nano::channel::send rather than on message header construction. This separates the assignment of protocol-specific information, specifically the version numbers, from the logic of creating messages. --- nano/core_test/message.cpp | 15 +++++++++------ nano/core_test/network.cpp | 7 +++---- nano/node/bootstrap/bootstrap_server.cpp | 3 +++ nano/node/common.cpp | 12 ------------ nano/node/network.cpp | 2 +- nano/node/network.hpp | 2 +- nano/node/transport/transport.cpp | 5 ++++- nano/node/transport/transport.hpp | 2 +- 8 files changed, 22 insertions(+), 26 deletions(-) diff --git a/nano/core_test/message.cpp b/nano/core_test/message.cpp index 579df6ce8..266f0f141 100644 --- a/nano/core_test/message.cpp +++ b/nano/core_test/message.cpp @@ -46,6 +46,9 @@ TEST (message, publish_serialization) { nano::publish publish (std::make_shared (0, 1, 2, nano::keypair ().prv, 4, 5)); publish.header.network = nano::networks::nano_dev_network; + publish.header.version_max = 6; + publish.header.version_using = 5; + publish.header.version_min = 4; ASSERT_EQ (nano::block_type::send, publish.header.block_type ()); std::vector bytes; { @@ -55,9 +58,9 @@ TEST (message, publish_serialization) ASSERT_EQ (8, bytes.size ()); ASSERT_EQ (0x52, bytes[0]); ASSERT_EQ (0x41, bytes[1]); - ASSERT_EQ (nano::dev::network_params.protocol.protocol_version, bytes[2]); - ASSERT_EQ (nano::dev::network_params.protocol.protocol_version, bytes[3]); - ASSERT_EQ (nano::dev::network_params.protocol.protocol_version_min (), bytes[4]); + ASSERT_EQ (6, bytes[2]); + ASSERT_EQ (5, bytes[3]); + ASSERT_EQ (4, bytes[4]); ASSERT_EQ (static_cast (nano::message_type::publish), bytes[5]); ASSERT_EQ (0x00, bytes[6]); // extensions ASSERT_EQ (static_cast (nano::block_type::send), bytes[7]); @@ -65,9 +68,9 @@ TEST (message, publish_serialization) auto error (false); nano::message_header header (error, stream); ASSERT_FALSE (error); - ASSERT_EQ (nano::dev::network_params.protocol.protocol_version_min (), header.version_min); - ASSERT_EQ (nano::dev::network_params.protocol.protocol_version, header.version_using); - ASSERT_EQ (nano::dev::network_params.protocol.protocol_version, header.version_max); + ASSERT_EQ (4, header.version_min); + ASSERT_EQ (5, header.version_using); + ASSERT_EQ (6, header.version_max); ASSERT_EQ (nano::message_type::publish, header.type); } diff --git a/nano/core_test/network.cpp b/nano/core_test/network.cpp index f1e17ac65..53e81d257 100644 --- a/nano/core_test/network.cpp +++ b/nano/core_test/network.cpp @@ -839,12 +839,11 @@ TEST (tcp_listener, tcp_listener_timeout_node_id_handshake) auto socket (std::make_shared (*node0)); auto cookie (node0->network.syn_cookies.assign (nano::transport::map_tcp_to_endpoint (node0->bootstrap.endpoint ()))); nano::node_id_handshake node_id_handshake (cookie, boost::none); - auto input (node_id_handshake.to_shared_const_buffer ()); - socket->async_connect (node0->bootstrap.endpoint (), [&input, socket] (boost::system::error_code const & ec) { + auto channel = std::make_shared (*node0, socket); + socket->async_connect (node0->bootstrap.endpoint (), [&node_id_handshake, channel] (boost::system::error_code const & ec) { ASSERT_FALSE (ec); - socket->async_write (input, [&input] (boost::system::error_code const & ec, size_t size_a) { + channel->send (node_id_handshake, [] (boost::system::error_code const & ec, size_t size_a) { ASSERT_FALSE (ec); - ASSERT_EQ (input.size (), size_a); }); }); ASSERT_TIMELY (5s, node0->stats.count (nano::stat::type::message, nano::stat::detail::node_id_handshake) != 0); diff --git a/nano/node/bootstrap/bootstrap_server.cpp b/nano/node/bootstrap/bootstrap_server.cpp index 8f76aa558..ea519aa47 100644 --- a/nano/node/bootstrap/bootstrap_server.cpp +++ b/nano/node/bootstrap/bootstrap_server.cpp @@ -670,6 +670,9 @@ public: debug_assert (!nano::validate_message (response->first, *message_a.query, response->second)); auto cookie (connection->node->network.syn_cookies.assign (nano::transport::map_tcp_to_endpoint (connection->remote_endpoint))); nano::node_id_handshake response_message (cookie, response); + response_message.header.version_max = connection->node->network_params.protocol.protocol_version; + response_message.header.version_using = connection->node->network_params.protocol.protocol_version; + response_message.header.version_min = connection->node->network_params.protocol.protocol_version_min (); auto shared_const_buffer = response_message.to_shared_const_buffer (); connection->socket->async_write (shared_const_buffer, [connection = std::weak_ptr (connection)] (boost::system::error_code const & ec, size_t size_a) { if (auto connection_l = connection.lock ()) diff --git a/nano/node/common.cpp b/nano/node/common.cpp index c7a929fca..4807b19bf 100644 --- a/nano/node/common.cpp +++ b/nano/node/common.cpp @@ -21,15 +21,6 @@ std::chrono::seconds constexpr nano::telemetry_cache_cutoffs::dev; std::chrono::seconds constexpr nano::telemetry_cache_cutoffs::beta; std::chrono::seconds constexpr nano::telemetry_cache_cutoffs::live; -namespace -{ -nano::protocol_constants const & get_protocol_constants () -{ - static nano::network_params params; - return params.protocol; -} -} - uint64_t nano::ip_address_hash_raw (boost::asio::ip::address const & ip_a, uint16_t port) { static nano::random_constants constants; @@ -51,9 +42,6 @@ uint64_t nano::ip_address_hash_raw (boost::asio::ip::address const & ip_a, uint1 nano::message_header::message_header (nano::message_type type_a) : network (nano::network_constants::active_network), - version_max (get_protocol_constants ().protocol_version), - version_using (get_protocol_constants ().protocol_version), - version_min (get_protocol_constants ().protocol_version_min ()), type (type_a) { } diff --git a/nano/node/network.cpp b/nano/node/network.cpp index aab55793c..22c92e4ab 100644 --- a/nano/node/network.cpp +++ b/nano/node/network.cpp @@ -180,7 +180,7 @@ void nano::network::send_node_id_handshake (std::shared_ptrsend (message); } -void nano::network::flood_message (nano::message const & message_a, nano::buffer_drop_policy const drop_policy_a, float const scale_a) +void nano::network::flood_message (nano::message & message_a, nano::buffer_drop_policy const drop_policy_a, float const scale_a) { for (auto & i : list (fanout (scale_a))) { diff --git a/nano/node/network.hpp b/nano/node/network.hpp index 60223c4f4..598f62e5a 100644 --- a/nano/node/network.hpp +++ b/nano/node/network.hpp @@ -123,7 +123,7 @@ public: nano::networks id; void start (); void stop (); - void flood_message (nano::message const &, nano::buffer_drop_policy const = nano::buffer_drop_policy::limiter, float const = 1.0f); + void flood_message (nano::message &, nano::buffer_drop_policy const = nano::buffer_drop_policy::limiter, float const = 1.0f); void flood_keepalive (float const scale_a = 1.0f) { nano::keepalive message; diff --git a/nano/node/transport/transport.cpp b/nano/node/transport/transport.cpp index 3f8fd72e9..e40c1c2ca 100644 --- a/nano/node/transport/transport.cpp +++ b/nano/node/transport/transport.cpp @@ -100,8 +100,11 @@ nano::transport::channel::channel (nano::node & node_a) : set_network_version (node_a.network_params.protocol.protocol_version); } -void nano::transport::channel::send (nano::message const & message_a, std::function const & callback_a, nano::buffer_drop_policy drop_policy_a) +void nano::transport::channel::send (nano::message & message_a, std::function const & callback_a, nano::buffer_drop_policy drop_policy_a) { + message_a.header.version_max = node.network_params.protocol.protocol_version; + message_a.header.version_using = node.network_params.protocol.protocol_version; + message_a.header.version_min = node.network_params.protocol.protocol_version_min (); callback_visitor visitor; message_a.visit (visitor); auto buffer (message_a.to_shared_const_buffer ()); diff --git a/nano/node/transport/transport.hpp b/nano/node/transport/transport.hpp index b38672ae2..bcee287ee 100644 --- a/nano/node/transport/transport.hpp +++ b/nano/node/transport/transport.hpp @@ -47,7 +47,7 @@ namespace transport virtual ~channel () = default; virtual size_t hash_code () const = 0; virtual bool operator== (nano::transport::channel const &) const = 0; - void send (nano::message const & message_a, std::function const & callback_a = nullptr, nano::buffer_drop_policy policy_a = nano::buffer_drop_policy::limiter); + void send (nano::message & message_a, std::function const & callback_a = nullptr, nano::buffer_drop_policy policy_a = nano::buffer_drop_policy::limiter); virtual void send_buffer (nano::shared_const_buffer const &, std::function const & = nullptr, nano::buffer_drop_policy = nano::buffer_drop_policy::limiter) = 0; virtual std::string to_string () const = 0; virtual nano::endpoint get_endpoint () const = 0;