From 2b2fd12106b8be409362dc84d28ea9a3db3ab389 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?=
 <3044353+pwojcikdev@users.noreply.github.com>
Date: Fri, 25 Oct 2024 12:41:45 +0200
Subject: [PATCH] Hashing cleanup

---
 nano/core_test/numbers.cpp   |  94 ++++++++++++++++++++
 nano/lib/numbers.hpp         | 160 ++++++++++++++++++++++++++++-------
 nano/secure/common.hpp       |  55 ------------
 nano/secure/pending_info.hpp |   4 +-
 4 files changed, 224 insertions(+), 89 deletions(-)

diff --git a/nano/core_test/numbers.cpp b/nano/core_test/numbers.cpp
index 1ffe78808b..d6b141fa58 100644
--- a/nano/core_test/numbers.cpp
+++ b/nano/core_test/numbers.cpp
@@ -3,7 +3,10 @@
 
 #include <gtest/gtest.h>
 
+#include <boost/container_hash/hash.hpp>
+
 #include <thread>
+#include <unordered_set>
 
 namespace
 {
@@ -577,3 +580,94 @@ void check_operator_greater_than (Num lhs, Num rhs)
 	ASSERT_FALSE (rhs > rhs);
 }
 }
+
+namespace
+{
+template <typename Type, template <typename> class Hash>
+void test_hashing ()
+{
+	Hash<Type> hash;
+	using underlying_t = typename Type::underlying_type;
+
+	// Basic equality tests
+	ASSERT_EQ (hash (Type{}), hash (Type{}));
+	ASSERT_EQ (hash (Type{ 123 }), hash (Type{ 123 }));
+
+	// Basic inequality tests
+	ASSERT_NE (hash (Type{ 123 }), hash (Type{ 124 }));
+	ASSERT_NE (hash (Type{ 0 }), hash (Type{ 1 }));
+
+	// Boundary value tests
+	constexpr auto min_val = std::numeric_limits<underlying_t>::min ();
+	constexpr auto max_val = std::numeric_limits<underlying_t>::max ();
+
+	// Min/Max tests
+	ASSERT_EQ (hash (Type{ min_val }), hash (Type{ min_val }));
+	ASSERT_EQ (hash (Type{ max_val }), hash (Type{ max_val }));
+	ASSERT_NE (hash (Type{ min_val }), hash (Type{ max_val }));
+
+	// Near boundary tests
+	ASSERT_NE (hash (Type{ min_val }), hash (Type{ min_val + 1 }));
+	ASSERT_NE (hash (Type{ max_val }), hash (Type{ max_val - 1 }));
+	ASSERT_NE (hash (Type{ min_val + 1 }), hash (Type{ max_val }));
+	ASSERT_NE (hash (Type{ max_val - 1 }), hash (Type{ min_val }));
+
+	// Common value tests
+	std::vector<underlying_t> common_values = {
+		0, // Zero
+		1, // One
+		42, // Common test value
+		0xFF, // Byte boundary
+		0xFFFF, // Word boundary
+		min_val, // Minimum
+		max_val, // Maximum
+		max_val / 2, // Middle value
+		min_val + (max_val / 2) // Offset middle
+	};
+
+	// Test all common values against each other
+	for (size_t i = 0; i < common_values.size (); ++i)
+	{
+		for (size_t j = i + 1; j < common_values.size (); ++j)
+		{
+			if (common_values[i] != common_values[j])
+			{
+				ASSERT_NE (hash (Type{ common_values[i] }), hash (Type{ common_values[j] }));
+			}
+			else
+			{
+				ASSERT_EQ (hash (Type{ common_values[i] }), hash (Type{ common_values[j] }));
+			}
+		}
+	}
+}
+}
+
+TEST (numbers, hashing)
+{
+	// Using std::hash
+	test_hashing<nano::uint128_union, std::hash> ();
+	test_hashing<nano::uint256_union, std::hash> ();
+	test_hashing<nano::uint512_union, std::hash> ();
+	test_hashing<nano::block_hash, std::hash> ();
+	test_hashing<nano::public_key, std::hash> ();
+	test_hashing<nano::hash_or_account, std::hash> ();
+	test_hashing<nano::link, std::hash> ();
+	test_hashing<nano::root, std::hash> ();
+	test_hashing<nano::raw_key, std::hash> ();
+	test_hashing<nano::wallet_id, std::hash> ();
+	test_hashing<nano::qualified_root, std::hash> ();
+
+	// Using boost::hash
+	test_hashing<nano::uint128_union, boost::hash> ();
+	test_hashing<nano::uint256_union, boost::hash> ();
+	test_hashing<nano::uint512_union, boost::hash> ();
+	test_hashing<nano::block_hash, boost::hash> ();
+	test_hashing<nano::public_key, boost::hash> ();
+	test_hashing<nano::hash_or_account, boost::hash> ();
+	test_hashing<nano::link, boost::hash> ();
+	test_hashing<nano::root, boost::hash> ();
+	test_hashing<nano::raw_key, boost::hash> ();
+	test_hashing<nano::wallet_id, boost::hash> ();
+	test_hashing<nano::qualified_root, boost::hash> ();
+}
diff --git a/nano/lib/numbers.hpp b/nano/lib/numbers.hpp
index 88e4de560d..bf8137c657 100644
--- a/nano/lib/numbers.hpp
+++ b/nano/lib/numbers.hpp
@@ -5,6 +5,7 @@
 
 #include <array>
 #include <compare>
+#include <limits>
 #include <ostream>
 
 #include <fmt/ostream.h>
@@ -22,6 +23,10 @@ nano::uint128_t const raw_ratio = nano::uint128_t ("1"); // 10^0
 
 class uint128_union
 {
+public:
+	// Type that is implicitly convertible to this union
+	using underlying_type = nano::uint128_t;
+
 public:
 	uint128_union () = default;
 	uint128_union (uint64_t value) :
@@ -115,11 +120,15 @@ class raw_key;
 
 class uint256_union
 {
+public:
+	// Type that is implicitly convertible to this union
+	using underlying_type = nano::uint256_t;
+
 public:
 	uint256_union () = default;
 	uint256_union (uint64_t value) :
 		uint256_union (nano::uint256_t{ value }){};
-	uint256_union (uint256_t const & value)
+	uint256_union (nano::uint256_t const & value)
 	{
 		bytes.fill (0);
 		boost::multiprecision::export_bits (value, bytes.rbegin (), 8, false);
@@ -256,6 +265,10 @@ using account = public_key;
 
 class hash_or_account
 {
+public:
+	// Type that is implicitly convertible to this union
+	using underlying_type = nano::uint256_t;
+
 public:
 	hash_or_account () :
 		account{} {};
@@ -369,6 +382,10 @@ class raw_key final : public uint256_union
 
 class uint512_union
 {
+public:
+	// Type that is implicitly convertible to this union
+	using underlying_type = nano::uint512_t;
+
 public:
 	uint512_union () = default;
 	uint512_union (nano::uint512_t const & value)
@@ -499,83 +516,91 @@ namespace difficulty
 namespace std
 {
 template <>
+struct hash<::nano::uint128_union>
+{
+	size_t operator() (::nano::uint128_union const & value) const noexcept
+	{
+		return value.qwords[0] + value.qwords[1];
+	}
+};
+template <>
 struct hash<::nano::uint256_union>
 {
-	size_t operator() (::nano::uint256_union const & data_a) const
+	size_t operator() (::nano::uint256_union const & value) const noexcept
 	{
-		return data_a.qwords[0] + data_a.qwords[1] + data_a.qwords[2] + data_a.qwords[3];
+		return value.qwords[0] + value.qwords[1] + value.qwords[2] + value.qwords[3];
 	}
 };
 template <>
-struct hash<::nano::account>
+struct hash<::nano::public_key>
 {
-	size_t operator() (::nano::account const & data_a) const
+	size_t operator() (::nano::public_key const & value) const noexcept
 	{
-		return hash<::nano::uint256_union> () (data_a);
+		return hash<::nano::uint256_union>{}(value);
 	}
 };
 template <>
 struct hash<::nano::block_hash>
 {
-	size_t operator() (::nano::block_hash const & data_a) const
+	size_t operator() (::nano::block_hash const & value) const noexcept
 	{
-		return hash<::nano::uint256_union> () (data_a);
+		return hash<::nano::uint256_union>{}(value);
 	}
 };
 template <>
 struct hash<::nano::hash_or_account>
 {
-	size_t operator() (::nano::hash_or_account const & data_a) const
+	size_t operator() (::nano::hash_or_account const & value) const noexcept
 	{
-		return hash<::nano::block_hash> () (data_a.as_block_hash ());
+		return hash<::nano::block_hash>{}(value.as_block_hash ());
 	}
 };
 template <>
-struct hash<::nano::raw_key>
+struct hash<::nano::root>
 {
-	size_t operator() (::nano::raw_key const & data_a) const
+	size_t operator() (::nano::root const & value) const noexcept
 	{
-		return hash<::nano::uint256_union> () (data_a);
+		return hash<::nano::hash_or_account>{}(value);
 	}
 };
 template <>
-struct hash<::nano::root>
+struct hash<::nano::link>
 {
-	size_t operator() (::nano::root const & data_a) const
+	size_t operator() (::nano::link const & value) const noexcept
 	{
-		return hash<::nano::uint256_union> () (data_a);
+		return hash<::nano::hash_or_account>{}(value);
 	}
 };
 template <>
-struct hash<::nano::wallet_id>
+struct hash<::nano::raw_key>
 {
-	size_t operator() (::nano::wallet_id const & data_a) const
+	size_t operator() (::nano::raw_key const & value) const noexcept
 	{
-		return hash<::nano::uint256_union> () (data_a);
+		return hash<::nano::uint256_union>{}(value);
 	}
 };
 template <>
-struct hash<::nano::uint256_t>
+struct hash<::nano::wallet_id>
 {
-	size_t operator() (::nano::uint256_t const & number_a) const
+	size_t operator() (::nano::wallet_id const & value) const noexcept
 	{
-		return number_a.convert_to<size_t> ();
+		return hash<::nano::uint256_union>{}(value);
 	}
 };
 template <>
 struct hash<::nano::uint512_union>
 {
-	size_t operator() (::nano::uint512_union const & data_a) const
+	size_t operator() (::nano::uint512_union const & value) const noexcept
 	{
-		return hash<::nano::uint256_union> () (data_a.uint256s[0]) + hash<::nano::uint256_union> () (data_a.uint256s[1]);
+		return hash<::nano::uint256_union>{}(value.uint256s[0]) + hash<::nano::uint256_union> () (value.uint256s[1]);
 	}
 };
 template <>
 struct hash<::nano::qualified_root>
 {
-	size_t operator() (::nano::qualified_root const & data_a) const
+	size_t operator() (::nano::qualified_root const & value) const noexcept
 	{
-		return hash<::nano::uint512_union> () (data_a);
+		return hash<::nano::uint512_union>{}(value);
 	}
 };
 }
@@ -583,20 +608,91 @@ struct hash<::nano::qualified_root>
 namespace boost
 {
 template <>
-struct hash<std::reference_wrapper<::nano::block_hash const>>
+struct hash<::nano::uint128_union>
+{
+	size_t operator() (::nano::uint128_union const & value) const noexcept
+	{
+		return std::hash<::nano::uint128_union> () (value);
+	}
+};
+template <>
+struct hash<::nano::uint256_union>
+{
+	size_t operator() (::nano::uint256_union const & value) const noexcept
+	{
+		return std::hash<::nano::uint256_union> () (value);
+	}
+};
+template <>
+struct hash<::nano::public_key>
+{
+	size_t operator() (::nano::public_key const & value) const noexcept
+	{
+		return std::hash<::nano::public_key> () (value);
+	}
+};
+template <>
+struct hash<::nano::block_hash>
 {
-	size_t operator() (std::reference_wrapper<::nano::block_hash const> const & hash_a) const
+	size_t operator() (::nano::block_hash const & value) const noexcept
 	{
-		std::hash<::nano::block_hash> hash;
-		return hash (hash_a);
+		return std::hash<::nano::block_hash> () (value);
+	}
+};
+template <>
+struct hash<::nano::hash_or_account>
+{
+	size_t operator() (::nano::hash_or_account const & value) const noexcept
+	{
+		return std::hash<::nano::hash_or_account> () (value);
 	}
 };
 template <>
 struct hash<::nano::root>
 {
-	size_t operator() (::nano::root const & value_a) const
+	size_t operator() (::nano::root const & value) const noexcept
+	{
+		return std::hash<::nano::root> () (value);
+	}
+};
+template <>
+struct hash<::nano::link>
+{
+	size_t operator() (::nano::link const & value) const noexcept
+	{
+		return std::hash<::nano::link> () (value);
+	}
+};
+template <>
+struct hash<::nano::raw_key>
+{
+	size_t operator() (::nano::raw_key const & value) const noexcept
+	{
+		return std::hash<::nano::raw_key> () (value);
+	}
+};
+template <>
+struct hash<::nano::wallet_id>
+{
+	size_t operator() (::nano::wallet_id const & value) const noexcept
+	{
+		return std::hash<::nano::wallet_id> () (value);
+	}
+};
+template <>
+struct hash<::nano::uint512_union>
+{
+	size_t operator() (::nano::uint512_union const & value) const noexcept
+	{
+		return std::hash<::nano::uint512_union> () (value);
+	}
+};
+template <>
+struct hash<::nano::qualified_root>
+{
+	size_t operator() (::nano::qualified_root const & value) const noexcept
 	{
-		return std::hash<::nano::root> () (value_a);
+		return std::hash<::nano::qualified_root> () (value);
 	}
 };
 }
diff --git a/nano/secure/common.hpp b/nano/secure/common.hpp
index fd28d30b13..e1639bd265 100644
--- a/nano/secure/common.hpp
+++ b/nano/secure/common.hpp
@@ -21,61 +21,6 @@
 #include <array>
 #include <unordered_map>
 
-namespace boost
-{
-template <>
-struct hash<::nano::uint256_union>
-{
-	size_t operator() (::nano::uint256_union const & value_a) const
-	{
-		return std::hash<::nano::uint256_union> () (value_a);
-	}
-};
-
-template <>
-struct hash<::nano::block_hash>
-{
-	size_t operator() (::nano::block_hash const & value_a) const
-	{
-		return std::hash<::nano::block_hash> () (value_a);
-	}
-};
-
-template <>
-struct hash<::nano::hash_or_account>
-{
-	size_t operator() (::nano::hash_or_account const & data_a) const
-	{
-		return std::hash<::nano::hash_or_account> () (data_a);
-	}
-};
-
-template <>
-struct hash<::nano::public_key>
-{
-	size_t operator() (::nano::public_key const & value_a) const
-	{
-		return std::hash<::nano::public_key> () (value_a);
-	}
-};
-template <>
-struct hash<::nano::uint512_union>
-{
-	size_t operator() (::nano::uint512_union const & value_a) const
-	{
-		return std::hash<::nano::uint512_union> () (value_a);
-	}
-};
-template <>
-struct hash<::nano::qualified_root>
-{
-	size_t operator() (::nano::qualified_root const & value_a) const
-	{
-		return std::hash<::nano::qualified_root> () (value_a);
-	}
-};
-}
-
 namespace nano
 {
 /**
diff --git a/nano/secure/pending_info.hpp b/nano/secure/pending_info.hpp
index 2d57df9ba2..c00e1cdb39 100644
--- a/nano/secure/pending_info.hpp
+++ b/nano/secure/pending_info.hpp
@@ -67,9 +67,9 @@ namespace std
 template <>
 struct hash<::nano::pending_key>
 {
-	size_t operator() (::nano::pending_key const & data_a) const
+	size_t operator() (::nano::pending_key const & value) const
 	{
-		return hash<::nano::uint512_union>{}({ ::nano::uint256_union{ data_a.account.number () }, data_a.hash });
+		return hash<::nano::uint512_union>{}({ ::nano::uint256_union{ value.account.number () }, value.hash });
 	}
 };
 }