From adba32ea833aa8ce76bbbcedae564680e3983f51 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 |  4 +---
 nano/lib/numbers.cpp       |  2 --
 nano/lib/numbers.hpp       |  2 --
 nano/node/messages.cpp     | 49 +++++++++++++++++++-------------------
 nano/node/messages.hpp     |  7 +++++-
 5 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/nano/core_test/message.cpp b/nano/core_test/message.cpp
index 039dbb1afa..84d4a49983 100644
--- a/nano/core_test/message.cpp
+++ b/nano/core_test/message.cpp
@@ -415,9 +415,7 @@ TEST (message, asc_pull_ack_serialization_blocks)
 	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)
+	for (int n = 0; n < nano::asc_pull_ack::blocks_payload::max_blocks; ++n)
 	{
 		original_payload.blocks.push_back (random_block ());
 	}
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<uint64_t, 4> qwords;
 		std::array<uint128_union, 2> 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..2eb6a5ae79 100644
--- a/nano/node/messages.cpp
+++ b/nano/node/messages.cpp
@@ -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 () && 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..dc6896e5c9 100644
--- a/nano/node/messages.hpp
+++ b/nano/node/messages.hpp
@@ -544,10 +544,15 @@ class asc_pull_ack final : public message
 
 	struct frontiers_payload
 	{
+		using frontier = std::pair<nano::account, nano::block_hash>;
+
 		void serialize (nano::stream &) const;
 		void deserialize (nano::stream &);
 
-		std::vector<std::pair<nano::account, nano::block_hash>> frontiers;
+		static void serialize_frontier (nano::stream &, frontier const &);
+		static frontier deserialize_frontier (nano::stream &);
+
+		std::vector<frontier> 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;