Use saturation arithmetic for incrementing account numbers

nanocurrency · Nov 26, 2024 · afd5949 · afd5949
1 parent bc77ff9
commit afd5949
Show file tree

Hide file tree

Showing 7 changed files with 129 additions and 7 deletions.
diff --git a/nano/core_test/numbers.cpp b/nano/core_test/numbers.cpp
@@ -649,3 +649,98 @@ TEST (uint512_union, hash)
 		}
 	}
 }
+
+TEST (sat_math, add_sat)
+{
+	// Test uint128_t
+	{
+		nano::uint128_t max = std::numeric_limits<nano::uint128_t>::max ();
+		nano::uint128_t one = 1;
+		nano::uint128_t large_val = max - 100;
+
+		// Normal addition
+		ASSERT_EQ (nano::add_sat (one, one), nano::uint128_t (2));
+
+		// Saturation at max
+		ASSERT_EQ (nano::add_sat (max, one), max);
+		ASSERT_EQ (nano::add_sat (large_val, nano::uint128_t (200)), max);
+		ASSERT_EQ (nano::add_sat (max, max), max);
+	}
+	// Test uint256_t
+	{
+		nano::uint256_t max = std::numeric_limits<nano::uint256_t>::max ();
+		nano::uint256_t one = 1;
+		nano::uint256_t large_val = max - 100;
+
+		// Normal addition
+		ASSERT_EQ (nano::add_sat (one, one), nano::uint256_t (2));
+
+		// Saturation at max
+		ASSERT_EQ (nano::add_sat (max, one), max);
+		ASSERT_EQ (nano::add_sat (large_val, nano::uint256_t (200)), max);
+		ASSERT_EQ (nano::add_sat (max, max), max);
+	}
+	// Test uint512_t
+	{
+		nano::uint512_t max = std::numeric_limits<nano::uint512_t>::max ();
+		nano::uint512_t one = 1;
+		nano::uint512_t large_val = max - 100;
+
+		// Normal addition
+		ASSERT_EQ (nano::add_sat (one, one), nano::uint512_t (2));
+
+		// Saturation at max
+		ASSERT_EQ (nano::add_sat (max, one), max);
+		ASSERT_EQ (nano::add_sat (large_val, nano::uint512_t (200)), max);
+		ASSERT_EQ (nano::add_sat (max, max), max);
+	}
+}
+
+TEST (sat_math, sub_sat)
+{
+	// Test uint128_t
+	{
+		nano::uint128_t max = std::numeric_limits<nano::uint128_t>::max ();
+		nano::uint128_t min = std::numeric_limits<nano::uint128_t>::min ();
+		nano::uint128_t one = 1;
+		nano::uint128_t hundred (100);
+
+		// Normal subtraction
+		ASSERT_EQ (nano::sub_sat (hundred, one), nano::uint128_t (99));
+
+		// Saturation at min
+		ASSERT_EQ (nano::sub_sat (min, one), min);
+		ASSERT_EQ (nano::sub_sat (hundred, nano::uint128_t (200)), min);
+		ASSERT_EQ (nano::sub_sat (min, max), min);
+	}
+	// Test uint256_t
+	{
+		nano::uint256_t max = std::numeric_limits<nano::uint256_t>::max ();
+		nano::uint256_t min = std::numeric_limits<nano::uint256_t>::min ();
+		nano::uint256_t one = 1;
+		nano::uint256_t hundred (100);
+
+		// Normal subtraction
+		ASSERT_EQ (nano::sub_sat (hundred, one), nano::uint256_t (99));
+
+		// Saturation at min
+		ASSERT_EQ (nano::sub_sat (min, one), min);
+		ASSERT_EQ (nano::sub_sat (hundred, nano::uint256_t (200)), min);
+		ASSERT_EQ (nano::sub_sat (min, max), min);
+	}
+	// Test uint512_t
+	{
+		nano::uint512_t max = std::numeric_limits<nano::uint512_t>::max ();
+		nano::uint512_t min = std::numeric_limits<nano::uint512_t>::min ();
+		nano::uint512_t one = 1;
+		nano::uint512_t hundred (100);
+
+		// Normal subtraction
+		ASSERT_EQ (nano::sub_sat (hundred, one), nano::uint512_t (99));
+
+		// Saturation at min
+		ASSERT_EQ (nano::sub_sat (min, one), min);
+		ASSERT_EQ (nano::sub_sat (hundred, nano::uint512_t (200)), min);
+		ASSERT_EQ (nano::sub_sat (min, max), min);
+	}
+}
diff --git a/nano/lib/numbers.hpp b/nano/lib/numbers.hpp
@@ -507,6 +507,33 @@ namespace difficulty
 	uint64_t from_multiplier (double const, uint64_t const);
 	double to_multiplier (uint64_t const, uint64_t const);
 }
+
+/**
+ * Add to or substract from a value without overflow
+ * TODO: C++26 replace with std::add_sat and std::sub_sat
+ */
+template <typename T>
+T add_sat (T const & value, T const & diff) noexcept
+{
+	static_assert (std::numeric_limits<T>::is_specialized, "std::numeric_limits<T> must be specialized");
+	return (value > std::numeric_limits<T>::max () - diff) ? std::numeric_limits<T>::max () : value + diff;
+}
+template <typename T>
+T sub_sat (T const & value, T const & diff) noexcept
+{
+	static_assert (std::numeric_limits<T>::is_specialized, "std::numeric_limits<T> must be specialized");
+	return (value < std::numeric_limits<T>::min () + diff) ? std::numeric_limits<T>::min () : value - diff;
+}
+template <typename T>
+T inc_sat (T const & value) noexcept
+{
+	return add_sat (value, static_cast<T> (1));
+}
+template <typename T>
+T dec_sat (T const & value) noexcept
+{
+	return sub_sat (value, static_cast<T> (1));
+}
 }
 
 /*

diff --git a/nano/node/backlog_scan.cpp b/nano/node/backlog_scan.cpp
@@ -126,7 +126,7 @@ void nano::backlog_scan::populate_backlog (nano::unique_lock<nano::mutex> & lock
 					activated.push_back (info);
 				}
 
-				next = account.number () + 1; // TODO: Prevent account overflow
+				next = inc_sat (account.number ());
 			}
 
 			done = (it == end);

diff --git a/nano/node/bootstrap/crawlers.hpp b/nano/node/bootstrap/crawlers.hpp
@@ -134,7 +134,7 @@ struct pending_database_crawler
 		if (it != end)
 		{
 			// If that fails, perform a fresh lookup
-			seek (starting_account.number () + 1);
+			seek (inc_sat (starting_account.number ()));
 		}
 
 		update_current ();

diff --git a/nano/node/bootstrap/database_scan.cpp b/nano/node/bootstrap/database_scan.cpp
@@ -78,7 +78,7 @@ std::deque<nano::account> nano::bootstrap::account_database_scanner::next_batch
 	{
 		auto const & [account, info] = crawler.current.value ();
 		result.push_back (account);
-		next = account.number () + 1; // TODO: Handle account number overflow
+		next = inc_sat (account.number ());
 	}
 
 	// Empty current value indicates the end of the table
@@ -106,7 +106,7 @@ std::deque<nano::account> nano::bootstrap::pending_database_scanner::next_batch
 	{
 		auto const & [key, info] = crawler.current.value ();
 		result.push_back (key.account);
-		next = key.account.number () + 1; // TODO: Handle account number overflow
+		next = inc_sat (key.account.number ());
 	}
 
 	// Empty current value indicates the end of the table

diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp
@@ -2173,7 +2173,7 @@ void nano::json_handler::delegators ()
 	{
 		auto transaction (node.ledger.tx_begin_read ());
 		boost::property_tree::ptree delegators;
-		for (auto i (node.store.account.begin (transaction, start_account.number () + 1)), n (node.store.account.end (transaction)); i != n && delegators.size () < count; ++i)
+		for (auto i (node.store.account.begin (transaction, inc_sat (start_account.number ()))), n (node.store.account.end (transaction)); i != n && delegators.size () < count; ++i)
 		{
 			nano::account_info const & info (i->second);
 			if (info.representative == representative)
@@ -4189,7 +4189,7 @@ void nano::json_handler::unopened ()
 					break;
 				}
 				// Skip existing accounts
-				iterator = node.store.pending.begin (transaction, nano::pending_key (account.number () + 1, 0));
+				iterator = node.store.pending.begin (transaction, nano::pending_key (inc_sat (account.number ()), 0));
 			}
 			else
 			{

diff --git a/nano/node/node.cpp b/nano/node/node.cpp
@@ -856,7 +856,7 @@ bool nano::node::collect_ledger_pruning_targets (std::deque<nano::block_hash> &
 		read_operations += depth;
 		if (read_operations >= batch_read_size_a)
 		{
-			last_account_a = account.number () + 1;
+			last_account_a = inc_sat (account.number ());
 			finish_transaction = true;
 		}
 		else