From 9d7782ca0b84310d819a1799dabdba9658517cd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 6 Nov 2023 18:51:44 +0100 Subject: [PATCH 1/7] Introduce frontier info payload message --- nano/lib/numbers.cpp | 2 + nano/lib/numbers.hpp | 2 + nano/lib/stats_enums.hpp | 2 + nano/node/bootstrap/bootstrap_server.cpp | 27 +++++++- nano/node/bootstrap/bootstrap_server.hpp | 12 +++- nano/node/bootstrap_ascending/service.cpp | 6 ++ nano/node/bootstrap_ascending/service.hpp | 1 + nano/node/messages.cpp | 77 ++++++++++++++++++++++- nano/node/messages.hpp | 55 +++++++++------- 9 files changed, 152 insertions(+), 32 deletions(-) diff --git a/nano/lib/numbers.cpp b/nano/lib/numbers.cpp index 65cb8ac86d..76c9dce4ea 100644 --- a/nano/lib/numbers.cpp +++ b/nano/lib/numbers.cpp @@ -143,6 +143,8 @@ bool nano::public_key::decode_account (std::string const & source_a) return error; } +nano::uint256_union const nano::uint256_union::zero{ 0 }; + nano::uint256_union::uint256_union (nano::uint256_t const & number_a) { bytes.fill (0); diff --git a/nano/lib/numbers.hpp b/nano/lib/numbers.hpp index d73310d1b4..bd05981f3e 100644 --- a/nano/lib/numbers.hpp +++ b/nano/lib/numbers.hpp @@ -93,6 +93,8 @@ class uint256_union std::array qwords; std::array owords; }; + + static const uint256_union zero; }; inline bool operator== (nano::uint256_union const & lhs, nano::uint256_union const & rhs) { diff --git a/nano/lib/stats_enums.hpp b/nano/lib/stats_enums.hpp index 00bc95872f..7473d85c66 100644 --- a/nano/lib/stats_enums.hpp +++ b/nano/lib/stats_enums.hpp @@ -242,6 +242,8 @@ enum class detail : uint8_t response_blocks, response_account_info, channel_full, + response_frontiers, + frontiers, // backlog activated, diff --git a/nano/node/bootstrap/bootstrap_server.cpp b/nano/node/bootstrap/bootstrap_server.cpp index 5bfc959220..a11c5657db 100644 --- a/nano/node/bootstrap/bootstrap_server.cpp +++ b/nano/node/bootstrap/bootstrap_server.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -21,7 +22,6 @@ nano::bootstrap_server::bootstrap_server (nano::store::component & store_a, nano nano::bootstrap_server::~bootstrap_server () { - stop (); } void nano::bootstrap_server::start () @@ -42,6 +42,7 @@ bool nano::bootstrap_server::verify_request_type (nano::asc_pull_type type) cons return false; case asc_pull_type::blocks: case asc_pull_type::account_info: + case asc_pull_type::frontiers: return true; } return false; @@ -68,6 +69,10 @@ bool nano::bootstrap_server::verify (const nano::asc_pull_req & message) const { return !pld.target.is_zero (); } + bool operator() (nano::asc_pull_req::frontiers_payload const & pld) const + { + return pld.count > 0 && pld.count <= max_frontiers && !pld.start.is_zero (); + } }; return std::visit (verify_visitor{}, message.payload); @@ -115,6 +120,11 @@ void nano::bootstrap_server::respond (nano::asc_pull_ack & response, std::shared { stats.inc (nano::stat::type::bootstrap_server, nano::stat::detail::response_account_info, nano::stat::dir::out); } + void operator() (nano::asc_pull_ack::frontiers_payload const & pld) + { + stats.inc (nano::stat::type::bootstrap_server, nano::stat::detail::response_frontiers, nano::stat::dir::out); + stats.add (nano::stat::type::bootstrap_server, nano::stat::detail::frontiers, nano::stat::dir::out, pld.frontiers.size ()); + } }; std::visit (stat_visitor{ stats }, response.payload); @@ -168,7 +178,7 @@ nano::asc_pull_ack nano::bootstrap_server::process (const store::transaction &, } /* - * Blocks response + * Blocks request */ nano::asc_pull_ack nano::bootstrap_server::process (store::transaction const & transaction, nano::asc_pull_req::id_t id, nano::asc_pull_req::blocks_payload const & request) @@ -253,7 +263,7 @@ std::vector> nano::bootstrap_server::prepare_blocks } /* - * Account info response + * Account info request */ nano::asc_pull_ack nano::bootstrap_server::process (const store::transaction & transaction, nano::asc_pull_req::id_t id, const nano::asc_pull_req::account_info_payload & request) @@ -301,3 +311,14 @@ nano::asc_pull_ack nano::bootstrap_server::process (const store::transaction & t response.update_header (); return response; } + +/* + * Frontiers request + */ + +nano::asc_pull_ack nano::bootstrap_server::process (const store::transaction & transaction, nano::asc_pull_req::id_t id, const nano::asc_pull_req::frontiers_payload & request) +{ + debug_assert (false, "not implemented"); + nano::asc_pull_ack ack{ network_constants }; + return ack; +} \ No newline at end of file diff --git a/nano/node/bootstrap/bootstrap_server.hpp b/nano/node/bootstrap/bootstrap_server.hpp index 98fd3823f9..e1358b4c3c 100644 --- a/nano/node/bootstrap/bootstrap_server.hpp +++ b/nano/node/bootstrap/bootstrap_server.hpp @@ -57,7 +57,7 @@ class bootstrap_server final nano::asc_pull_ack process (store::transaction const &, nano::asc_pull_req::id_t id, nano::empty_payload const & request); /* - * Blocks response + * Blocks request */ nano::asc_pull_ack process (store::transaction const &, nano::asc_pull_req::id_t id, nano::asc_pull_req::blocks_payload const & request); nano::asc_pull_ack prepare_response (store::transaction const &, nano::asc_pull_req::id_t id, nano::block_hash start_block, std::size_t count); @@ -65,10 +65,15 @@ class bootstrap_server final std::vector> prepare_blocks (store::transaction const &, nano::block_hash start_block, std::size_t count) const; /* - * Account info response + * Account info request */ nano::asc_pull_ack process (store::transaction const &, nano::asc_pull_req::id_t id, nano::asc_pull_req::account_info_payload const & request); + /* + * Frontiers request + */ + nano::asc_pull_ack process (store::transaction const &, nano::asc_pull_req::id_t id, nano::asc_pull_req::frontiers_payload const & request); + /* * Checks if the request should be dropped early on */ @@ -82,10 +87,11 @@ class bootstrap_server final nano::stats & stats; private: - processing_queue request_queue; + nano::processing_queue request_queue; public: // Config /** Maximum number of blocks to send in a single response, cannot be higher than capacity of a single `asc_pull_ack` message */ constexpr static std::size_t max_blocks = nano::asc_pull_ack::blocks_payload::max_blocks; + constexpr static std::size_t max_frontiers = nano::asc_pull_ack::frontiers_payload::max_frontiers; }; } diff --git a/nano/node/bootstrap_ascending/service.cpp b/nano/node/bootstrap_ascending/service.cpp index 52b9b317b9..facea3f9d3 100644 --- a/nano/node/bootstrap_ascending/service.cpp +++ b/nano/node/bootstrap_ascending/service.cpp @@ -366,6 +366,7 @@ void nano::bootstrap_ascending::service::process (nano::asc_pull_ack const & mes on_reply.notify (tag); condition.notify_all (); + std::visit ([this, &tag] (auto && request) { return process (request, tag); }, message.payload); } else @@ -416,6 +417,11 @@ void nano::bootstrap_ascending::service::process (const nano::asc_pull_ack::acco // TODO: Make use of account info } +void nano::bootstrap_ascending::service::process (const nano::asc_pull_ack::frontiers_payload & response, const nano::bootstrap_ascending::service::async_tag & tag) +{ + // TODO: Make use of frontiers info +} + void nano::bootstrap_ascending::service::process (const nano::empty_payload & response, const nano::bootstrap_ascending::service::async_tag & tag) { // Should not happen diff --git a/nano/node/bootstrap_ascending/service.hpp b/nano/node/bootstrap_ascending/service.hpp index a518b3b3da..3c7a436f7c 100644 --- a/nano/node/bootstrap_ascending/service.hpp +++ b/nano/node/bootstrap_ascending/service.hpp @@ -109,6 +109,7 @@ namespace bootstrap_ascending void process (nano::asc_pull_ack::blocks_payload const & response, async_tag const & tag); void process (nano::asc_pull_ack::account_info_payload const & response, async_tag const & tag); + void process (nano::asc_pull_ack::frontiers_payload const & response, async_tag const & tag); void process (nano::empty_payload const & response, async_tag const & tag); enum class verify_result diff --git a/nano/node/messages.cpp b/nano/node/messages.cpp index 0f4699c009..0837b756fa 100644 --- a/nano/node/messages.cpp +++ b/nano/node/messages.cpp @@ -1611,6 +1611,13 @@ void nano::asc_pull_req::deserialize_payload (nano::stream & stream) payload = pld; break; } + case asc_pull_type::frontiers: + { + frontiers_payload pld; + pld.deserialize (stream); + payload = pld; + break; + } default: throw std::runtime_error ("Unknown asc_pull_type"); } @@ -1618,12 +1625,13 @@ void nano::asc_pull_req::deserialize_payload (nano::stream & stream) void nano::asc_pull_req::update_header () { + // TODO: Avoid serializing the payload twice std::vector bytes; { nano::vectorstream payload_stream (bytes); serialize_payload (payload_stream); } - debug_assert (bytes.size () <= 65535u); // Max int16 for storing size + debug_assert (bytes.size () <= std::numeric_limits::max ()); // Max uint16 for storing size debug_assert (bytes.size () >= 1); header.extensions = std::bitset<16> (bytes.size ()); } @@ -1652,6 +1660,10 @@ bool nano::asc_pull_req::verify_consistency () const { debug_assert (type == asc_pull_type::account_info); } + void operator() (frontiers_payload) const + { + debug_assert (type == asc_pull_type::frontiers); + } }; std::visit (consistency_visitor{ type }, payload); return true; // Just for convenience of calling from asserts @@ -1720,6 +1732,22 @@ void nano::asc_pull_req::account_info_payload::deserialize (stream & stream) nano::read (stream, target_type); } +/* + * asc_pull_req::frontiers_payload + */ + +void nano::asc_pull_req::frontiers_payload::serialize (nano::stream & stream) const +{ + nano::write (stream, start); + nano::write_big_endian (stream, count); +} + +void nano::asc_pull_req::frontiers_payload::deserialize (nano::stream & stream) +{ + nano::read (stream, start); + nano::read_big_endian (stream, count); +} + /* * asc_pull_ack */ @@ -1742,7 +1770,7 @@ void nano::asc_pull_ack::visit (nano::message_visitor & visitor) const void nano::asc_pull_ack::serialize (nano::stream & stream) const { - debug_assert (header.extensions.to_ulong () > 0); // Block payload must have least `not_a_block` terminator + debug_assert (header.extensions.to_ulong () > 0); // Block payload must have at least `not_a_block` terminator header.serialize (stream); nano::write (stream, type); nano::write_big_endian (stream, id); @@ -1793,6 +1821,13 @@ void nano::asc_pull_ack::deserialize_payload (nano::stream & stream) payload = pld; break; } + case asc_pull_type::frontiers: + { + frontiers_payload pld; + pld.deserialize (stream); + payload = pld; + break; + } default: throw std::runtime_error ("Unknown asc_pull_type"); } @@ -1800,12 +1835,13 @@ void nano::asc_pull_ack::deserialize_payload (nano::stream & stream) void nano::asc_pull_ack::update_header () { + // TODO: Avoid serializing the payload twice std::vector bytes; { nano::vectorstream payload_stream (bytes); serialize_payload (payload_stream); } - debug_assert (bytes.size () <= 65535u); // Max int16 for storing size + debug_assert (bytes.size () <= std::numeric_limits::max ()); // Max uint16 for storing size debug_assert (bytes.size () >= 1); header.extensions = std::bitset<16> (bytes.size ()); } @@ -1834,6 +1870,10 @@ bool nano::asc_pull_ack::verify_consistency () const { debug_assert (type == asc_pull_type::account_info); } + void operator() (frontiers_payload) const + { + debug_assert (type == asc_pull_type::frontiers); + } }; std::visit (consistency_visitor{ type }, payload); return true; // Just for convenience of calling from asserts @@ -1927,3 +1967,34 @@ void nano::asc_pull_ack::account_info_payload::deserialize (nano::stream & strea nano::read (stream, account_conf_frontier); nano::read_big_endian (stream, account_conf_height); } + +/* + * asc_pull_ack::frontiers_payload + */ + +void nano::asc_pull_ack::frontiers_payload::serialize (nano::stream & stream) const +{ + for (auto const & [account, frontier] : frontiers) + { + nano::write (stream, account); + nano::write (stream, frontier); + } + nano::write (stream, nano::account::zero); + nano::write (stream, nano::block_hash::zero); +} + +void nano::asc_pull_ack::frontiers_payload::deserialize (nano::stream & stream) +{ + nano::account account; + nano::block_hash frontier; + while (frontiers.size () < max_frontiers) + { + nano::read (stream, account); + nano::read (stream, frontier); + if (account.is_zero () || frontier.is_zero ()) + { + break; + } + frontiers.emplace_back (account, frontier); + } +} \ No newline at end of file diff --git a/nano/node/messages.hpp b/nano/node/messages.hpp index bf4ab2b2ab..4bf38e120f 100644 --- a/nano/node/messages.hpp +++ b/nano/node/messages.hpp @@ -389,11 +389,11 @@ enum class asc_pull_type : uint8_t invalid = 0x0, blocks = 0x1, account_info = 0x2, + frontiers = 0x3, }; -class empty_payload +struct empty_payload { -public: void serialize (nano::stream &) const { debug_assert (false); @@ -444,36 +444,40 @@ class asc_pull_req final : public message block = 1, }; - class blocks_payload + struct blocks_payload { - public: void serialize (nano::stream &) const; void deserialize (nano::stream &); - public: nano::hash_or_account start{ 0 }; uint8_t count{ 0 }; - asc_pull_req::hash_type start_type{ 0 }; + hash_type start_type{ 0 }; }; - class account_info_payload + struct account_info_payload { - public: void serialize (nano::stream &) const; void deserialize (nano::stream &); - public: nano::hash_or_account target{ 0 }; - asc_pull_req::hash_type target_type{ 0 }; + hash_type target_type{ 0 }; + }; + + struct frontiers_payload + { + void serialize (nano::stream &) const; + void deserialize (nano::stream &); + + nano::account start{ 0 }; + uint16_t count{ 0 }; }; public: // Payload - /** Currently unused, allows extensions in the future */ asc_pull_type type{ asc_pull_type::invalid }; id_t id{ 0 }; /** Payload depends on `asc_pull_type` */ - std::variant payload; + std::variant payload; public: /** Size of message without payload */ @@ -514,27 +518,22 @@ class asc_pull_ack final : public message bool verify_consistency () const; public: // Payload definitions - class blocks_payload + struct blocks_payload { - public: void serialize (nano::stream &) const; void deserialize (nano::stream &); - public: - std::vector> blocks{}; + std::vector> blocks; - public: - /* Header allows for 16 bit extensions; 65535 bytes / 500 bytes (block size with some future margin) ~ 131 */ + /* Header allows for 16 bit extensions; 65536 bytes / 500 bytes (block size with some future margin) ~ 131 */ constexpr static std::size_t max_blocks = 128; }; - class account_info_payload + struct account_info_payload { - public: void serialize (nano::stream &) const; void deserialize (nano::stream &); - public: nano::account account{ 0 }; nano::block_hash account_open{ 0 }; nano::block_hash account_head{ 0 }; @@ -543,13 +542,23 @@ class asc_pull_ack final : public message uint64_t account_conf_height{ 0 }; }; + struct frontiers_payload + { + void serialize (nano::stream &) const; + void deserialize (nano::stream &); + + std::vector> frontiers; + + /* Header allows for 16 bit extensions; 65536 bytes / 64 bytes (account + frontier) ~ 1024, but we need some space for null frontier terminator*/ + constexpr static std::size_t max_frontiers = 1000; + }; + public: // Payload - /** Currently unused, allows extensions in the future */ asc_pull_type type{ asc_pull_type::invalid }; id_t id{ 0 }; /** Payload depends on `asc_pull_type` */ - std::variant payload; + std::variant payload; public: /** Size of message without payload */ From db2b98f5a4aedc098b8bae212be81d97c042200c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 6 Nov 2023 18:53:36 +0100 Subject: [PATCH 2/7] Periodically refresh read transaction in `bootstrap_server` --- nano/node/bootstrap/bootstrap_server.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nano/node/bootstrap/bootstrap_server.cpp b/nano/node/bootstrap/bootstrap_server.cpp index a11c5657db..6f24c975b5 100644 --- a/nano/node/bootstrap/bootstrap_server.cpp +++ b/nano/node/bootstrap/bootstrap_server.cpp @@ -150,6 +150,8 @@ void nano::bootstrap_server::process_batch (std::deque & batch) for (auto & [request, channel] : batch) { + transaction.refresh_if_needed (); + if (!channel->max (nano::transport::traffic_type::bootstrap)) { auto response = process (transaction, request); From da5b6a29c8e6818c992d980e711f514c5efcfb5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 6 Nov 2023 19:18:17 +0100 Subject: [PATCH 3/7] Add frontiers message serialization/deserialization tests --- nano/core_test/message.cpp | 84 ++++++++++++++++++++++++++++++++++++++ nano/node/messages.cpp | 14 ++++++- 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/nano/core_test/message.cpp b/nano/core_test/message.cpp index 082a600978..039dbb1afa 100644 --- a/nano/core_test/message.cpp +++ b/nano/core_test/message.cpp @@ -367,6 +367,47 @@ TEST (message, asc_pull_req_serialization_account_info) ASSERT_TRUE (nano::at_end (stream)); } +TEST (message, asc_pull_req_serialization_frontiers) +{ + nano::asc_pull_req original{ nano::dev::network_params.network }; + original.id = 7; + original.type = nano::asc_pull_type::frontiers; + + nano::asc_pull_req::frontiers_payload original_payload; + original_payload.start = nano::test::random_account (); + original_payload.count = 123; + + original.payload = original_payload; + original.update_header (); + + // Serialize + std::vector bytes; + { + nano::vectorstream stream{ bytes }; + original.serialize (stream); + } + nano::bufferstream stream{ bytes.data (), bytes.size () }; + + // Header + bool error = false; + nano::message_header header (error, stream); + ASSERT_FALSE (error); + ASSERT_EQ (nano::message_type::asc_pull_req, header.type); + + // Message + nano::asc_pull_req message (error, stream, header); + ASSERT_FALSE (error); + ASSERT_EQ (original.id, message.id); + ASSERT_EQ (original.type, message.type); + + nano::asc_pull_req::frontiers_payload message_payload; + ASSERT_NO_THROW (message_payload = std::get (message.payload)); + ASSERT_EQ (original_payload.start, message_payload.start); + ASSERT_EQ (original_payload.count, message_payload.count); + + ASSERT_TRUE (nano::at_end (stream)); +} + TEST (message, asc_pull_ack_serialization_blocks) { nano::asc_pull_ack original{ nano::dev::network_params.network }; @@ -466,6 +507,49 @@ TEST (message, asc_pull_ack_serialization_account_info) ASSERT_TRUE (nano::at_end (stream)); } +TEST (message, asc_pull_ack_serialization_frontiers) +{ + nano::asc_pull_ack original{ nano::dev::network_params.network }; + original.id = 11; + original.type = nano::asc_pull_type::frontiers; + + nano::asc_pull_ack::frontiers_payload original_payload; + for (int n = 0; n < nano::asc_pull_ack::frontiers_payload::max_frontiers; ++n) + { + original_payload.frontiers.push_back ({ nano::test::random_account (), nano::test::random_hash () }); + } + + original.payload = original_payload; + original.update_header (); + + // Serialize + std::vector bytes; + { + nano::vectorstream stream{ bytes }; + original.serialize (stream); + } + nano::bufferstream stream{ bytes.data (), bytes.size () }; + + // Header + bool error = false; + nano::message_header header (error, stream); + ASSERT_FALSE (error); + ASSERT_EQ (nano::message_type::asc_pull_ack, header.type); + + // Message + nano::asc_pull_ack message (error, stream, header); + ASSERT_FALSE (error); + ASSERT_EQ (original.id, message.id); + ASSERT_EQ (original.type, message.type); + + nano::asc_pull_ack::frontiers_payload message_payload; + ASSERT_NO_THROW (message_payload = std::get (message.payload)); + + ASSERT_EQ (original_payload.frontiers, message_payload.frontiers); + + ASSERT_TRUE (nano::at_end (stream)); +} + TEST (message, node_id_handshake_query_serialization) { nano::node_id_handshake::query_payload query{}; diff --git a/nano/node/messages.cpp b/nano/node/messages.cpp index 0837b756fa..435bda6191 100644 --- a/nano/node/messages.cpp +++ b/nano/node/messages.cpp @@ -1974,6 +1974,8 @@ void nano::asc_pull_ack::account_info_payload::deserialize (nano::stream & strea void nano::asc_pull_ack::frontiers_payload::serialize (nano::stream & stream) const { + debug_assert (frontiers.size () <= max_frontiers); + for (auto const & [account, frontier] : frontiers) { nano::write (stream, account); @@ -1987,7 +1989,7 @@ void nano::asc_pull_ack::frontiers_payload::deserialize (nano::stream & stream) { nano::account account; nano::block_hash frontier; - while (frontiers.size () < max_frontiers) + while (true) { nano::read (stream, account); nano::read (stream, frontier); @@ -1995,6 +1997,14 @@ void nano::asc_pull_ack::frontiers_payload::deserialize (nano::stream & stream) { break; } - frontiers.emplace_back (account, frontier); + debug_assert (frontiers.size () < max_frontiers); + if (frontiers.size () < max_frontiers) + { + frontiers.emplace_back (account, frontier); + } + else + { + break; + } } } \ No newline at end of file From fb7d8d2d660a845b12271faaa626ea3903e6cdbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 6 Nov 2023 19:34:00 +0100 Subject: [PATCH 4/7] Cleanup serialization/deserialization logic --- nano/core_test/message.cpp | 16 +++++----- nano/lib/numbers.cpp | 2 -- nano/lib/numbers.hpp | 2 -- nano/node/messages.cpp | 61 +++++++++++++++++++------------------- nano/node/messages.hpp | 27 ++++++++++++----- 5 files changed, 57 insertions(+), 51 deletions(-) diff --git a/nano/core_test/message.cpp b/nano/core_test/message.cpp index 039dbb1afa..f388b406e9 100644 --- a/nano/core_test/message.cpp +++ b/nano/core_test/message.cpp @@ -293,7 +293,7 @@ TEST (message, asc_pull_req_serialization_blocks) original.id = 7; original.type = nano::asc_pull_type::blocks; - nano::asc_pull_req::blocks_payload original_payload; + nano::asc_pull_req::blocks_payload original_payload{}; original_payload.start = nano::test::random_hash (); original_payload.count = 111; @@ -334,7 +334,7 @@ TEST (message, asc_pull_req_serialization_account_info) original.id = 7; original.type = nano::asc_pull_type::account_info; - nano::asc_pull_req::account_info_payload original_payload; + nano::asc_pull_req::account_info_payload original_payload{}; original_payload.target = nano::test::random_hash (); original.payload = original_payload; @@ -373,7 +373,7 @@ TEST (message, asc_pull_req_serialization_frontiers) original.id = 7; original.type = nano::asc_pull_type::frontiers; - nano::asc_pull_req::frontiers_payload original_payload; + nano::asc_pull_req::frontiers_payload original_payload{}; original_payload.start = nano::test::random_account (); original_payload.count = 123; @@ -414,10 +414,8 @@ TEST (message, asc_pull_ack_serialization_blocks) original.id = 11; original.type = nano::asc_pull_type::blocks; - nano::asc_pull_ack::blocks_payload original_payload; - // Generate blocks - const int num_blocks = 128; // Maximum allowed - for (int n = 0; n < num_blocks; ++n) + nano::asc_pull_ack::blocks_payload original_payload{}; + for (int n = 0; n < nano::asc_pull_ack::blocks_payload::max_blocks; ++n) { original_payload.blocks.push_back (random_block ()); } @@ -463,7 +461,7 @@ TEST (message, asc_pull_ack_serialization_account_info) original.id = 11; original.type = nano::asc_pull_type::account_info; - nano::asc_pull_ack::account_info_payload original_payload; + nano::asc_pull_ack::account_info_payload original_payload{}; original_payload.account = nano::test::random_account (); original_payload.account_open = nano::test::random_hash (); original_payload.account_head = nano::test::random_hash (); @@ -513,7 +511,7 @@ TEST (message, asc_pull_ack_serialization_frontiers) original.id = 11; original.type = nano::asc_pull_type::frontiers; - nano::asc_pull_ack::frontiers_payload original_payload; + nano::asc_pull_ack::frontiers_payload original_payload{}; for (int n = 0; n < nano::asc_pull_ack::frontiers_payload::max_frontiers; ++n) { original_payload.frontiers.push_back ({ nano::test::random_account (), nano::test::random_hash () }); diff --git a/nano/lib/numbers.cpp b/nano/lib/numbers.cpp index 76c9dce4ea..65cb8ac86d 100644 --- a/nano/lib/numbers.cpp +++ b/nano/lib/numbers.cpp @@ -143,8 +143,6 @@ bool nano::public_key::decode_account (std::string const & source_a) return error; } -nano::uint256_union const nano::uint256_union::zero{ 0 }; - nano::uint256_union::uint256_union (nano::uint256_t const & number_a) { bytes.fill (0); diff --git a/nano/lib/numbers.hpp b/nano/lib/numbers.hpp index bd05981f3e..d73310d1b4 100644 --- a/nano/lib/numbers.hpp +++ b/nano/lib/numbers.hpp @@ -93,8 +93,6 @@ class uint256_union std::array qwords; std::array owords; }; - - static const uint256_union zero; }; inline bool operator== (nano::uint256_union const & lhs, nano::uint256_union const & rhs) { diff --git a/nano/node/messages.cpp b/nano/node/messages.cpp index 435bda6191..521b06c967 100644 --- a/nano/node/messages.cpp +++ b/nano/node/messages.cpp @@ -1599,21 +1599,21 @@ void nano::asc_pull_req::deserialize_payload (nano::stream & stream) { case asc_pull_type::blocks: { - blocks_payload pld; + blocks_payload pld{}; pld.deserialize (stream); payload = pld; break; } case asc_pull_type::account_info: { - account_info_payload pld; + account_info_payload pld{}; pld.deserialize (stream); payload = pld; break; } case asc_pull_type::frontiers: { - frontiers_payload pld; + frontiers_payload pld{}; pld.deserialize (stream); payload = pld; break; @@ -1809,21 +1809,21 @@ void nano::asc_pull_ack::deserialize_payload (nano::stream & stream) { case asc_pull_type::blocks: { - blocks_payload pld; + blocks_payload pld{}; pld.deserialize (stream); payload = pld; break; } case asc_pull_type::account_info: { - account_info_payload pld; + account_info_payload pld{}; pld.deserialize (stream); payload = pld; break; } case asc_pull_type::frontiers: { - frontiers_payload pld; + frontiers_payload pld{}; pld.deserialize (stream); payload = pld; break; @@ -1925,6 +1925,7 @@ std::string nano::asc_pull_ack::to_string () const void nano::asc_pull_ack::blocks_payload::serialize (nano::stream & stream) const { debug_assert (blocks.size () <= max_blocks); + for (auto & block : blocks) { debug_assert (block != nullptr); @@ -1972,39 +1973,39 @@ void nano::asc_pull_ack::account_info_payload::deserialize (nano::stream & strea * asc_pull_ack::frontiers_payload */ +void nano::asc_pull_ack::frontiers_payload::serialize_frontier (nano::stream & stream, nano::asc_pull_ack::frontiers_payload::frontier const & frontier) +{ + auto const & [account, hash] = frontier; + nano::write (stream, account); + nano::write (stream, hash); +} + +nano::asc_pull_ack::frontiers_payload::frontier nano::asc_pull_ack::frontiers_payload::deserialize_frontier (nano::stream & stream) +{ + nano::account account; + nano::block_hash hash; + nano::read (stream, account); + nano::read (stream, hash); + return { account, hash }; +} + void nano::asc_pull_ack::frontiers_payload::serialize (nano::stream & stream) const { debug_assert (frontiers.size () <= max_frontiers); - for (auto const & [account, frontier] : frontiers) + for (auto const & frontier : frontiers) { - nano::write (stream, account); - nano::write (stream, frontier); + serialize_frontier (stream, frontier); } - nano::write (stream, nano::account::zero); - nano::write (stream, nano::block_hash::zero); + serialize_frontier (stream, { nano::account{ 0 }, nano::block_hash{ 0 } }); } void nano::asc_pull_ack::frontiers_payload::deserialize (nano::stream & stream) { - nano::account account; - nano::block_hash frontier; - while (true) + auto current = deserialize_frontier (stream); + while ((!current.first.is_zero () && !current.second.is_zero ()) && frontiers.size () < max_frontiers) { - nano::read (stream, account); - nano::read (stream, frontier); - if (account.is_zero () || frontier.is_zero ()) - { - break; - } - debug_assert (frontiers.size () < max_frontiers); - if (frontiers.size () < max_frontiers) - { - frontiers.emplace_back (account, frontier); - } - else - { - break; - } + frontiers.push_back (current); + current = deserialize_frontier (stream); } -} \ No newline at end of file +} diff --git a/nano/node/messages.hpp b/nano/node/messages.hpp index 4bf38e120f..cb2385f3ca 100644 --- a/nano/node/messages.hpp +++ b/nano/node/messages.hpp @@ -449,9 +449,10 @@ class asc_pull_req final : public message void serialize (nano::stream &) const; void deserialize (nano::stream &); + // Payload nano::hash_or_account start{ 0 }; uint8_t count{ 0 }; - hash_type start_type{ 0 }; + hash_type start_type{}; }; struct account_info_payload @@ -459,8 +460,9 @@ class asc_pull_req final : public message void serialize (nano::stream &) const; void deserialize (nano::stream &); + // Payload nano::hash_or_account target{ 0 }; - hash_type target_type{ 0 }; + hash_type target_type{}; }; struct frontiers_payload @@ -468,6 +470,7 @@ class asc_pull_req final : public message void serialize (nano::stream &) const; void deserialize (nano::stream &); + // Payload nano::account start{ 0 }; uint16_t count{ 0 }; }; @@ -520,13 +523,14 @@ class asc_pull_ack final : public message public: // Payload definitions struct blocks_payload { + /* Header allows for 16 bit extensions; 65536 bytes / 500 bytes (block size with some future margin) ~ 131 */ + constexpr static std::size_t max_blocks = 128; + void serialize (nano::stream &) const; void deserialize (nano::stream &); + // Payload std::vector> blocks; - - /* Header allows for 16 bit extensions; 65536 bytes / 500 bytes (block size with some future margin) ~ 131 */ - constexpr static std::size_t max_blocks = 128; }; struct account_info_payload @@ -534,6 +538,7 @@ class asc_pull_ack final : public message void serialize (nano::stream &) const; void deserialize (nano::stream &); + // Payload nano::account account{ 0 }; nano::block_hash account_open{ 0 }; nano::block_hash account_head{ 0 }; @@ -544,13 +549,19 @@ class asc_pull_ack final : public message struct frontiers_payload { + /* Header allows for 16 bit extensions; 65536 bytes / 64 bytes (account + frontier) ~ 1024, but we need some space for null frontier terminator */ + constexpr static std::size_t max_frontiers = 1000; + + using frontier = std::pair; + void serialize (nano::stream &) const; void deserialize (nano::stream &); - std::vector> frontiers; + static void serialize_frontier (nano::stream &, frontier const &); + static frontier deserialize_frontier (nano::stream &); - /* Header allows for 16 bit extensions; 65536 bytes / 64 bytes (account + frontier) ~ 1024, but we need some space for null frontier terminator*/ - constexpr static std::size_t max_frontiers = 1000; + // Payload + std::vector frontiers; }; public: // Payload From 7c2875877b8a6a4d9c45ca982a636649ceaf777e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 6 Nov 2023 20:50:49 +0100 Subject: [PATCH 5/7] Logic to handle requests for frontiers in bootstrap server --- nano/node/bootstrap/bootstrap_server.cpp | 28 ++++++++++++++++++------ 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/nano/node/bootstrap/bootstrap_server.cpp b/nano/node/bootstrap/bootstrap_server.cpp index 6f24c975b5..8fda11587c 100644 --- a/nano/node/bootstrap/bootstrap_server.cpp +++ b/nano/node/bootstrap/bootstrap_server.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -71,7 +72,7 @@ bool nano::bootstrap_server::verify (const nano::asc_pull_req & message) const } bool operator() (nano::asc_pull_req::frontiers_payload const & pld) const { - return pld.count > 0 && pld.count <= max_frontiers && !pld.start.is_zero (); + return pld.count > 0 && pld.count <= max_frontiers; } }; @@ -215,7 +216,7 @@ nano::asc_pull_ack nano::bootstrap_server::process (store::transaction const & t nano::asc_pull_ack nano::bootstrap_server::prepare_response (store::transaction const & transaction, nano::asc_pull_req::id_t id, nano::block_hash start_block, std::size_t count) { - debug_assert (count <= max_blocks); + debug_assert (count <= max_blocks); // Should be filtered out earlier auto blocks = prepare_blocks (transaction, start_block, count); debug_assert (blocks.size () <= count); @@ -224,7 +225,7 @@ nano::asc_pull_ack nano::bootstrap_server::prepare_response (store::transaction response.id = id; response.type = nano::asc_pull_type::blocks; - nano::asc_pull_ack::blocks_payload response_payload; + nano::asc_pull_ack::blocks_payload response_payload{}; response_payload.blocks = blocks; response.payload = response_payload; @@ -247,7 +248,7 @@ nano::asc_pull_ack nano::bootstrap_server::prepare_empty_blocks_response (nano:: std::vector> nano::bootstrap_server::prepare_blocks (store::transaction const & transaction, nano::block_hash start_block, std::size_t count) const { - debug_assert (count <= max_blocks); + debug_assert (count <= max_blocks); // Should be filtered out earlier std::vector> result; if (!start_block.is_zero ()) @@ -320,7 +321,20 @@ nano::asc_pull_ack nano::bootstrap_server::process (const store::transaction & t nano::asc_pull_ack nano::bootstrap_server::process (const store::transaction & transaction, nano::asc_pull_req::id_t id, const nano::asc_pull_req::frontiers_payload & request) { - debug_assert (false, "not implemented"); - nano::asc_pull_ack ack{ network_constants }; - return ack; + debug_assert (request.count <= max_frontiers); // Should be filtered out earlier + + nano::asc_pull_ack response{ network_constants }; + response.id = id; + response.type = nano::asc_pull_type::frontiers; + + nano::asc_pull_ack::frontiers_payload response_payload{}; + + for (auto it = store.account.begin (transaction, request.start), end = store.account.end (); it != end && response_payload.frontiers.size () < request.count; ++it) + { + response_payload.frontiers.emplace_back (it->first, it->second.head); + } + + response.payload = response_payload; + response.update_header (); + return response; } \ No newline at end of file From 12bc50c7733df24906cf5807d30bf1696d53bcb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Mon, 6 Nov 2023 23:33:54 +0100 Subject: [PATCH 6/7] Add bootstrap server tests --- nano/core_test/bootstrap_server.cpp | 176 +++++++++++++++++++++++----- 1 file changed, 144 insertions(+), 32 deletions(-) diff --git a/nano/core_test/bootstrap_server.cpp b/nano/core_test/bootstrap_server.cpp index 5ca4a460cb..984cb58567 100644 --- a/nano/core_test/bootstrap_server.cpp +++ b/nano/core_test/bootstrap_server.cpp @@ -5,6 +5,7 @@ #include #include +#include using namespace std::chrono_literals; @@ -31,6 +32,13 @@ class responses_helper final return responses.size (); } + void connect (nano::bootstrap_server & server) + { + server.on_response.add ([&] (auto & response, auto & channel) { + add (response); + }); + } + private: nano::mutex mutex; std::vector responses; @@ -65,9 +73,7 @@ TEST (bootstrap_server, serve_account_blocks) auto & node = *system.add_node (); responses_helper responses; - node.bootstrap_server.on_response.add ([&] (auto & response, auto & channel) { - responses.add (response); - }); + responses.connect (node.bootstrap_server); auto chains = nano::test::setup_chains (system, node, 1, 128); auto [first_account, first_blocks] = chains.front (); @@ -77,7 +83,7 @@ TEST (bootstrap_server, serve_account_blocks) request.id = 7; request.type = nano::asc_pull_type::blocks; - nano::asc_pull_req::blocks_payload request_payload; + nano::asc_pull_req::blocks_payload request_payload{}; request_payload.start = first_account; request_payload.count = nano::bootstrap_server::max_blocks; request_payload.start_type = nano::asc_pull_req::hash_type::account; @@ -109,9 +115,7 @@ TEST (bootstrap_server, serve_hash) auto & node = *system.add_node (); responses_helper responses; - node.bootstrap_server.on_response.add ([&] (auto & response, auto & channel) { - responses.add (response); - }); + responses.connect (node.bootstrap_server); auto chains = nano::test::setup_chains (system, node, 1, 256); auto [account, blocks] = chains.front (); @@ -124,7 +128,7 @@ TEST (bootstrap_server, serve_hash) request.id = 7; request.type = nano::asc_pull_type::blocks; - nano::asc_pull_req::blocks_payload request_payload; + nano::asc_pull_req::blocks_payload request_payload{}; request_payload.start = blocks.front ()->hash (); request_payload.count = nano::bootstrap_server::max_blocks; request_payload.start_type = nano::asc_pull_req::hash_type::block; @@ -156,9 +160,7 @@ TEST (bootstrap_server, serve_hash_one) auto & node = *system.add_node (); responses_helper responses; - node.bootstrap_server.on_response.add ([&] (auto & response, auto & channel) { - responses.add (response); - }); + responses.connect (node.bootstrap_server); auto chains = nano::test::setup_chains (system, node, 1, 256); auto [account, blocks] = chains.front (); @@ -171,7 +173,7 @@ TEST (bootstrap_server, serve_hash_one) request.id = 7; request.type = nano::asc_pull_type::blocks; - nano::asc_pull_req::blocks_payload request_payload; + nano::asc_pull_req::blocks_payload request_payload{}; request_payload.start = blocks.front ()->hash (); request_payload.count = 1; request_payload.start_type = nano::asc_pull_req::hash_type::block; @@ -200,9 +202,7 @@ TEST (bootstrap_server, serve_end_of_chain) auto & node = *system.add_node (); responses_helper responses; - node.bootstrap_server.on_response.add ([&] (auto & response, auto & channel) { - responses.add (response); - }); + responses.connect (node.bootstrap_server); auto chains = nano::test::setup_chains (system, node, 1, 128); auto [account, blocks] = chains.front (); @@ -212,7 +212,7 @@ TEST (bootstrap_server, serve_end_of_chain) request.id = 7; request.type = nano::asc_pull_type::blocks; - nano::asc_pull_req::blocks_payload request_payload; + nano::asc_pull_req::blocks_payload request_payload{}; request_payload.start = blocks.back ()->hash (); request_payload.count = nano::bootstrap_server::max_blocks; request_payload.start_type = nano::asc_pull_req::hash_type::block; @@ -242,9 +242,7 @@ TEST (bootstrap_server, serve_missing) auto & node = *system.add_node (); responses_helper responses; - node.bootstrap_server.on_response.add ([&] (auto & response, auto & channel) { - responses.add (response); - }); + responses.connect (node.bootstrap_server); auto chains = nano::test::setup_chains (system, node, 1, 128); @@ -253,7 +251,7 @@ TEST (bootstrap_server, serve_missing) request.id = 7; request.type = nano::asc_pull_type::blocks; - nano::asc_pull_req::blocks_payload request_payload; + nano::asc_pull_req::blocks_payload request_payload{}; request_payload.start = nano::test::random_hash (); request_payload.count = nano::bootstrap_server::max_blocks; request_payload.start_type = nano::asc_pull_req::hash_type::block; @@ -282,9 +280,7 @@ TEST (bootstrap_server, serve_multiple) auto & node = *system.add_node (); responses_helper responses; - node.bootstrap_server.on_response.add ([&] (auto & response, auto & channel) { - responses.add (response); - }); + responses.connect (node.bootstrap_server); auto chains = nano::test::setup_chains (system, node, 32, 16); @@ -298,7 +294,7 @@ TEST (bootstrap_server, serve_multiple) request.id = next_id++; request.type = nano::asc_pull_type::blocks; - nano::asc_pull_req::blocks_payload request_payload; + nano::asc_pull_req::blocks_payload request_payload{}; request_payload.start = account; request_payload.count = nano::bootstrap_server::max_blocks; request_payload.start_type = nano::asc_pull_req::hash_type::account; @@ -345,9 +341,7 @@ TEST (bootstrap_server, serve_account_info) auto & node = *system.add_node (); responses_helper responses; - node.bootstrap_server.on_response.add ([&] (auto & response, auto & channel) { - responses.add (response); - }); + responses.connect (node.bootstrap_server); auto chains = nano::test::setup_chains (system, node, 1, 128); auto [account, blocks] = chains.front (); @@ -357,7 +351,7 @@ TEST (bootstrap_server, serve_account_info) request.id = 7; request.type = nano::asc_pull_type::account_info; - nano::asc_pull_req::account_info_payload request_payload; + nano::asc_pull_req::account_info_payload request_payload{}; request_payload.target = account; request_payload.target_type = nano::asc_pull_req::hash_type::account; @@ -393,9 +387,7 @@ TEST (bootstrap_server, serve_account_info_missing) auto & node = *system.add_node (); responses_helper responses; - node.bootstrap_server.on_response.add ([&] (auto & response, auto & channel) { - responses.add (response); - }); + responses.connect (node.bootstrap_server); auto chains = nano::test::setup_chains (system, node, 1, 128); auto [account, blocks] = chains.front (); @@ -405,7 +397,7 @@ TEST (bootstrap_server, serve_account_info_missing) request.id = 7; request.type = nano::asc_pull_type::account_info; - nano::asc_pull_req::account_info_payload request_payload; + nano::asc_pull_req::account_info_payload request_payload{}; request_payload.target = nano::test::random_account (); request_payload.target_type = nano::asc_pull_req::hash_type::account; @@ -434,3 +426,123 @@ TEST (bootstrap_server, serve_account_info_missing) // Ensure we don't get any unexpected responses ASSERT_ALWAYS (1s, responses.size () == 1); } + +TEST (bootstrap_server, serve_frontiers) +{ + nano::test::system system{}; + auto & node = *system.add_node (); + + responses_helper responses; + responses.connect (node.bootstrap_server); + + auto chains = nano::test::setup_chains (system, node, /* chain count */ 32, /* block count */ 4); + + // Request all frontiers + nano::asc_pull_req request{ node.network_params.network }; + request.id = 7; + request.type = nano::asc_pull_type::frontiers; + + nano::asc_pull_req::frontiers_payload request_payload{}; + request_payload.count = nano::bootstrap_server::max_frontiers; + request_payload.start = 0; + + request.payload = request_payload; + request.update_header (); + + node.network.inbound (request, nano::test::fake_channel (node)); + + ASSERT_TIMELY (5s, responses.size () == 1); + + auto response = responses.get ().front (); + // Ensure we got response exactly for what we asked for + ASSERT_EQ (response.id, 7); + ASSERT_EQ (response.type, nano::asc_pull_type::frontiers); + + nano::asc_pull_ack::frontiers_payload response_payload; + ASSERT_NO_THROW (response_payload = std::get (response.payload)); + + ASSERT_EQ (response_payload.frontiers.size (), chains.size () + 1); // +1 for genesis + + // Ensure frontiers match what we expect + std::map expected_frontiers; + for (auto & [account, blocks] : chains) + { + expected_frontiers[account] = blocks.back ()->hash (); + } + expected_frontiers[nano::dev::genesis_key.pub] = node.latest (nano::dev::genesis_key.pub); + + for (auto & [account, frontier] : response_payload.frontiers) + { + ASSERT_EQ (frontier, expected_frontiers[account]); + expected_frontiers.erase (account); + } + ASSERT_TRUE (expected_frontiers.empty ()); +} + +TEST (bootstrap_server, serve_frontiers_invalid_count) +{ + nano::test::system system{}; + auto & node = *system.add_node (); + + responses_helper responses; + responses.connect (node.bootstrap_server); + + auto chains = nano::test::setup_chains (system, node, /* chain count */ 4, /* block count */ 4); + + // Zero count + { + nano::asc_pull_req request{ node.network_params.network }; + request.id = 7; + request.type = nano::asc_pull_type::frontiers; + + nano::asc_pull_req::frontiers_payload request_payload{}; + request_payload.count = 0; + request_payload.start = 0; + + request.payload = request_payload; + request.update_header (); + + node.network.inbound (request, nano::test::fake_channel (node)); + } + + ASSERT_TIMELY_EQ (5s, node.stats.count (nano::stat::type::bootstrap_server, nano::stat::detail::invalid), 1); + + // Count larger than allowed + { + nano::asc_pull_req request{ node.network_params.network }; + request.id = 7; + request.type = nano::asc_pull_type::frontiers; + + nano::asc_pull_req::frontiers_payload request_payload{}; + request_payload.count = nano::bootstrap_server::max_frontiers + 1; + request_payload.start = 0; + + request.payload = request_payload; + request.update_header (); + + node.network.inbound (request, nano::test::fake_channel (node)); + } + + ASSERT_TIMELY_EQ (5s, node.stats.count (nano::stat::type::bootstrap_server, nano::stat::detail::invalid), 2); + + // Max numeric value + { + nano::asc_pull_req request{ node.network_params.network }; + request.id = 7; + request.type = nano::asc_pull_type::frontiers; + + nano::asc_pull_req::frontiers_payload request_payload{}; + request_payload.count = std::numeric_limits::max (); + request_payload.start = 0; + + request.payload = request_payload; + request.update_header (); + + node.network.inbound (request, nano::test::fake_channel (node)); + } + + ASSERT_TIMELY_EQ (5s, node.stats.count (nano::stat::type::bootstrap_server, nano::stat::detail::invalid), 3); + + // Ensure we don't get any unexpected responses + ASSERT_ALWAYS (1s, responses.size () == 0); +} \ No newline at end of file From 165cc91fbb7dc78c27b634c216a108b64205558b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Wed, 8 Nov 2023 17:57:20 +0100 Subject: [PATCH 7/7] Bump protocol version --- nano/lib/config.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nano/lib/config.hpp b/nano/lib/config.hpp index fd7eff2603..db8ef89752 100644 --- a/nano/lib/config.hpp +++ b/nano/lib/config.hpp @@ -372,10 +372,12 @@ class network_constants /** Initial value is ACTIVE_NETWORK compile flag, but can be overridden by a CLI flag */ static nano::networks active_network; + /** Current protocol version */ - uint8_t const protocol_version = 0x13; + uint8_t const protocol_version = 0x14; /** Minimum accepted protocol version */ uint8_t const protocol_version_min = 0x12; + /** Minimum accepted protocol version used when bootstrapping */ uint8_t const bootstrap_protocol_version_min = 0x13; };