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] 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 039dbb1a..f388b406 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 76c9dce4..65cb8ac8 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 bd05981f..d73310d1 100644 --- a/nano/lib/numbers.hpp +++ b/nano/lib/numbers.hpp @@ -93,8 +93,6 @@ public: 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 435bda61..521b06c9 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 4bf38e12..cb2385f3 100644 --- a/nano/node/messages.hpp +++ b/nano/node/messages.hpp @@ -449,9 +449,10 @@ public: // Payload definitions 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 @@ public: // Payload definitions 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 @@ public: // Payload definitions void serialize (nano::stream &) const; void deserialize (nano::stream &); + // Payload nano::account start{ 0 }; uint16_t count{ 0 }; }; @@ -520,13 +523,14 @@ private: // Debug 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 @@ public: // Payload definitions 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 @@ public: // Payload definitions 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