From 84fef77255fd7101067a86bec8a1ba26e00daabd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?=
 <3044353+pwojcikdev@users.noreply.github.com>
Date: Wed, 25 Sep 2024 21:54:56 +0200
Subject: [PATCH] Improved database iteration

---
 .../bootstrap_ascending/database_scan.hpp     |   2 +
 nano/node/bootstrap_ascending/iterators.hpp   | 187 ++++++++++++++++++
 nano/node/bootstrap_ascending/service.cpp     |  55 ++++--
 nano/store/account.hpp                        |  13 +-
 nano/store/iterator.hpp                       |   5 +-
 5 files changed, 239 insertions(+), 23 deletions(-)
 create mode 100644 nano/node/bootstrap_ascending/iterators.hpp

diff --git a/nano/node/bootstrap_ascending/database_scan.hpp b/nano/node/bootstrap_ascending/database_scan.hpp
index 3c61be9a6..77bb011c1 100644
--- a/nano/node/bootstrap_ascending/database_scan.hpp
+++ b/nano/node/bootstrap_ascending/database_scan.hpp
@@ -8,6 +8,7 @@
 
 namespace nano::bootstrap_ascending
 {
+// TODO: Rename to *_scanner
 struct account_database_iterator
 {
 	explicit account_database_iterator (nano::ledger &);
@@ -20,6 +21,7 @@ struct account_database_iterator
 	size_t completed{ 0 };
 };
 
+// TODO: Rename to *_scanner
 struct pending_database_iterator
 {
 	explicit pending_database_iterator (nano::ledger &);
diff --git a/nano/node/bootstrap_ascending/iterators.hpp b/nano/node/bootstrap_ascending/iterators.hpp
new file mode 100644
index 000000000..48efd1fa8
--- /dev/null
+++ b/nano/node/bootstrap_ascending/iterators.hpp
@@ -0,0 +1,187 @@
+#pragma once
+
+#include <nano/secure/account_info.hpp>
+#include <nano/secure/pending_info.hpp>
+#include <nano/store/account.hpp>
+#include <nano/store/component.hpp>
+#include <nano/store/pending.hpp>
+
+#include <optional>
+
+namespace nano::bootstrap_ascending
+{
+struct account_database_crawler
+{
+	using value_type = std::pair<nano::account, nano::account_info>;
+
+	static constexpr size_t sequential_attempts = 10;
+
+	account_database_crawler (nano::store::component & store, nano::store::transaction const & transaction, nano::account const & start) :
+		store{ store },
+		transaction{ transaction },
+		it{ store.account.end () },
+		end{ store.account.end () }
+	{
+		seek (start);
+	}
+
+	void seek (nano::account const & account)
+	{
+		it = store.account.begin (transaction, account);
+		update_current ();
+	}
+
+	void advance ()
+	{
+		if (it == end)
+		{
+			debug_assert (!current);
+			return;
+		}
+
+		++it;
+		update_current ();
+	}
+
+	void advance_to (nano::account const & account)
+	{
+		if (it == end)
+		{
+			debug_assert (!current);
+			return;
+		}
+
+		// First try advancing sequentially
+		for (size_t count = 0; count < sequential_attempts && it != end; ++count, ++it)
+		{
+			// Break if we've reached or overshoot the target account
+			if (it->first.number () >= account.number ())
+			{
+				update_current ();
+				return;
+			}
+		}
+
+		// If that fails, perform a fresh lookup
+		seek (account);
+	}
+
+	std::optional<value_type> current{};
+
+private:
+	void update_current ()
+	{
+		if (it != end)
+		{
+			current = *it;
+		}
+		else
+		{
+			current = std::nullopt;
+		}
+	}
+
+	nano::store::component & store;
+	nano::store::transaction const & transaction;
+
+	nano::store::account::iterator it;
+	nano::store::account::iterator const end;
+};
+
+struct pending_database_crawler
+{
+	using value_type = std::pair<nano::pending_key, nano::pending_info>;
+
+	static constexpr size_t sequential_attempts = 10;
+
+	pending_database_crawler (nano::store::component & store, nano::store::transaction const & transaction, nano::account const & start) :
+		store{ store },
+		transaction{ transaction },
+		it{ store.pending.end () },
+		end{ store.pending.end () }
+	{
+		seek (start);
+	}
+
+	void seek (nano::account const & account)
+	{
+		it = store.pending.begin (transaction, { account, 0 });
+		update_current ();
+	}
+
+	// Advance to the next account
+	void advance ()
+	{
+		if (it == end)
+		{
+			debug_assert (!current);
+			return;
+		}
+
+		auto const starting_account = it->first.account;
+
+		// First try advancing sequentially
+		for (size_t count = 0; count < sequential_attempts && it != end; ++count, ++it)
+		{
+			// Break if we've reached the next account
+			if (it->first.account != starting_account)
+			{
+				update_current ();
+				return;
+			}
+		}
+
+		if (it != end)
+		{
+			// If that fails, perform a fresh lookup
+			seek (starting_account.number () + 1);
+		}
+
+		update_current ();
+	}
+
+	void advance_to (nano::account const & account)
+	{
+		if (it == end)
+		{
+			debug_assert (!current);
+			return;
+		}
+
+		// First try advancing sequentially
+		for (size_t count = 0; count < sequential_attempts && it != end; ++count, ++it)
+		{
+			// Break if we've reached or overshoot the target account
+			if (it->first.account.number () >= account.number ())
+			{
+				update_current ();
+				return;
+			}
+		}
+
+		// If that fails, perform a fresh lookup
+		seek (account);
+	}
+
+	std::optional<value_type> current{};
+
+private:
+	void update_current ()
+	{
+		if (it != end)
+		{
+			current = *it;
+		}
+		else
+		{
+			current = std::nullopt;
+		}
+	}
+
+	nano::store::component & store;
+	nano::store::transaction const & transaction;
+
+	nano::store::pending::iterator it;
+	nano::store::pending::iterator const end;
+};
+}
\ No newline at end of file
diff --git a/nano/node/bootstrap_ascending/service.cpp b/nano/node/bootstrap_ascending/service.cpp
index 1de0f696c..0739040ca 100644
--- a/nano/node/bootstrap_ascending/service.cpp
+++ b/nano/node/bootstrap_ascending/service.cpp
@@ -3,6 +3,7 @@
 #include <nano/lib/stats_enums.hpp>
 #include <nano/lib/thread_roles.hpp>
 #include <nano/node/blockprocessor.hpp>
+#include <nano/node/bootstrap_ascending/iterators.hpp>
 #include <nano/node/bootstrap_ascending/service.hpp>
 #include <nano/node/network.hpp>
 #include <nano/node/nodeconfig.hpp>
@@ -636,6 +637,14 @@ void nano::bootstrap_ascending::service::run_frontiers ()
 
 void nano::bootstrap_ascending::service::process_frontiers (std::deque<std::pair<nano::account, nano::block_hash>> const & frontiers)
 {
+	release_assert (!frontiers.empty ());
+
+	// Accounts must be passed in ascending order
+	debug_assert (std::adjacent_find (frontiers.begin (), frontiers.end (), [] (auto const & lhs, auto const & rhs) {
+		return lhs.first.number () >= rhs.first.number ();
+	})
+	== frontiers.end ());
+
 	stats.inc (nano::stat::type::bootstrap_ascending, nano::stat::detail::process_frontiers);
 
 	size_t outdated = 0;
@@ -646,30 +655,42 @@ void nano::bootstrap_ascending::service::process_frontiers (std::deque<std::pair
 	{
 		auto transaction = ledger.tx_begin_read ();
 
+		auto const start = frontiers.front ().first;
+		account_database_crawler account_crawler{ ledger.store, transaction, start };
+		pending_database_crawler pending_crawler{ ledger.store, transaction, start };
+
+		auto block_exists = [&] (nano::block_hash const & hash) {
+			return ledger.any.block_exists_or_pruned (transaction, hash);
+		};
+
 		auto should_prioritize = [&] (nano::account const & account, nano::block_hash const & frontier) {
-			if (ledger.any.block_exists_or_pruned (transaction, frontier))
+			account_crawler.advance_to (account);
+			pending_crawler.advance_to (account);
+
+			// Check if account exists in our ledger
+			if (account_crawler.current && account_crawler.current->first == account)
 			{
-				return false;
-			}
-			if (auto info = ledger.any.account_get (transaction, account))
-			{
-				if (info->head != frontier)
+				// Check for frontier mismatch
+				if (account_crawler.current->second.head != frontier)
 				{
-					outdated++;
-					return true; // Frontier is outdated
+					// Check if frontier block exists in our ledger
+					if (!block_exists (frontier))
+					{
+						outdated++;
+						return true; // Frontier is outdated
+					}
 				}
-				return false;
+				return false; // Account exists and frontier is up-to-date
 			}
-			if (auto receivable = ledger.any.receivable_lower_bound (transaction, account, { 0 }))
+
+			// Check if account has pending blocks in our ledger
+			if (pending_crawler.current && pending_crawler.current->first.account == account)
 			{
-				if (receivable->first.account == account)
-				{
-					pending++;
-					return true; // Account doesn't exist but has pending blocks in the ledger
-				}
-				return false;
+				pending++;
+				return true; // Account doesn't exist but has pending blocks in the ledger
 			}
-			return false;
+
+			return false; // Account doesn't exist in the ledger and has no pending blocks, can't be prioritized right now
 		};
 
 		for (auto const & [account, frontier] : frontiers)
diff --git a/nano/store/account.hpp b/nano/store/account.hpp
index 749cd9c84..41ee47417 100644
--- a/nano/store/account.hpp
+++ b/nano/store/account.hpp
@@ -19,6 +19,9 @@ namespace nano::store
  */
 class account
 {
+public:
+	using iterator = store::iterator<nano::account, nano::account_info>;
+
 public:
 	virtual void put (store::write_transaction const &, nano::account const &, nano::account_info const &) = 0;
 	virtual bool get (store::transaction const &, nano::account const &, nano::account_info &) = 0;
@@ -26,10 +29,10 @@ public:
 	virtual void del (store::write_transaction const &, nano::account const &) = 0;
 	virtual bool exists (store::transaction const &, nano::account const &) = 0;
 	virtual size_t count (store::transaction const &) = 0;
-	virtual iterator<nano::account, nano::account_info> begin (store::transaction const &, nano::account const &) const = 0;
-	virtual iterator<nano::account, nano::account_info> begin (store::transaction const &) const = 0;
-	virtual iterator<nano::account, nano::account_info> rbegin (store::transaction const &) const = 0;
-	virtual iterator<nano::account, nano::account_info> end () const = 0;
-	virtual void for_each_par (std::function<void (store::read_transaction const &, iterator<nano::account, nano::account_info>, iterator<nano::account, nano::account_info>)> const &) const = 0;
+	virtual store::iterator<nano::account, nano::account_info> begin (store::transaction const &, nano::account const &) const = 0;
+	virtual store::iterator<nano::account, nano::account_info> begin (store::transaction const &) const = 0;
+	virtual store::iterator<nano::account, nano::account_info> rbegin (store::transaction const &) const = 0;
+	virtual store::iterator<nano::account, nano::account_info> end () const = 0;
+	virtual void for_each_par (std::function<void (store::read_transaction const &, store::iterator<nano::account, nano::account_info>, store::iterator<nano::account, nano::account_info>)> const &) const = 0;
 };
 } // namespace nano::store
diff --git a/nano/store/iterator.hpp b/nano/store/iterator.hpp
index 7fe957d72..47d57d579 100644
--- a/nano/store/iterator.hpp
+++ b/nano/store/iterator.hpp
@@ -12,6 +12,9 @@ namespace nano::store
 template <typename T, typename U>
 class iterator final
 {
+public:
+	using value_type = std::pair<T, U>;
+
 public:
 	iterator (std::nullptr_t)
 	{
@@ -63,7 +66,7 @@ public:
 	}
 
 private:
-	std::pair<T, U> current;
+	value_type current;
 	std::unique_ptr<iterator_impl<T, U>> impl;
 };
 } // namespace nano::store