Rewrite ledger random block sampling

nano/core_test/ledger.cpp

@@ -5351,7 +5351,7 @@ TEST (ledger, pruning_safe_functions)
 	ASSERT_EQ (nano::dev::genesis_key.pub, ledger.any.block_account (transaction, send2->hash ()).value ());
 }
 
-TEST (ledger, hash_root_random)
+TEST (ledger, random_blocks)
 {
 	nano::logger logger;
 	auto store = nano::make_store (logger, nano::unique_path (), nano::dev::constants);
@@ -5394,23 +5394,46 @@ TEST (ledger, hash_root_random)
 	ASSERT_TRUE (store->pruned.exists (transaction, send1->hash ()));
 	ASSERT_TRUE (ledger.any.block_exists (transaction, nano::dev::genesis->hash ()));
 	ASSERT_TRUE (ledger.any.block_exists (transaction, send2->hash ()));
-	// Test random block including pruned
-	bool done (false);
-	auto iteration (0);
-	while (!done)
+	// Prunned block will not be included in the random selection because it's not in the blocks set
 	{
-		++iteration;
-		auto root_hash (ledger.hash_root_random (transaction));
-		done = (root_hash.first == send1->hash ()) && (root_hash.second.is_zero ());
-		ASSERT_LE (iteration, 1000);
+		bool done = false;
+		size_t iteration = 0;
+		while (!done && iteration < 42)
+		{
+			++iteration;
+			auto blocks = ledger.random_blocks (transaction, 10);
+			ASSERT_EQ (blocks.size (), 10); // Random blocks should repeat if the ledger is smaller than the requested count
+			auto first = blocks.front ();
+			done = (first->hash () == send1->hash ());
+		}
+		ASSERT_FALSE (done);
 	}
-	done = false;
-	while (!done)
+	// Genesis and send2 should be included in the random selection
 	{
-		++iteration;
-		auto root_hash (ledger.hash_root_random (transaction));
-		done = (root_hash.first == send2->hash ()) && (root_hash.second == send2->root ().as_block_hash ());
-		ASSERT_LE (iteration, 1000);
+		bool done = false;
+		size_t iteration = 0;
+		while (!done)
+		{
+			++iteration;
+			auto blocks = ledger.random_blocks (transaction, 1);
+			ASSERT_EQ (blocks.size (), 1);
+			auto first = blocks.front ();
+			done = (first->hash () == send2->hash ());
+			ASSERT_LE (iteration, 1000);
+		}
+	}
+	{
+		bool done = false;
+		size_t iteration = 0;
+		while (!done)
+		{
+			++iteration;
+			auto blocks = ledger.random_blocks (transaction, 1);
+			ASSERT_EQ (blocks.size (), 1);
+			auto first = blocks.front ();
+			done = (first->hash () == nano::dev::genesis->hash ());
+			ASSERT_LE (iteration, 1000);
+		}
 	}
 }
 

nano/node/repcrawler.cpp

@@ -271,31 +271,20 @@ std::vector<std::shared_ptr<nano::transport::channel>> nano::rep_crawler::prepar
 
 auto nano::rep_crawler::prepare_query_target () const -> std::optional<hash_root_t>
 {
-	constexpr int max_attempts = 4;
+	constexpr int max_attempts = 10;
 
 	auto transaction = node.ledger.tx_begin_read ();
 
-	std::optional<std::pair<nano::block_hash, nano::root>> hash_root;
-
-	// Randomly select a block from ledger to request votes for
-	for (auto i = 0; i < max_attempts && !hash_root; ++i)
+	auto random_blocks = node.ledger.random_blocks (transaction, max_attempts);
+	for (auto const & block : random_blocks)
 	{
-		hash_root = node.ledger.hash_root_random (transaction);
-
-		// Rebroadcasted votes for recently confirmed blocks might confuse the rep crawler
-		if (active.recently_confirmed.exists (hash_root->first))
+		if (!active.recently_confirmed.exists (block->hash ()))
 		{
-			hash_root = std::nullopt;
+			return std::make_pair (block->hash (), block->root ());
 		}
 	}
 
-	// Special case for dev network where number of blocks might be very low: if we can't find a block to query, just pick genesis
-	if (node.network_params.network.is_dev_network () && !hash_root)
-	{
-		hash_root = std::make_pair (node.network_params.ledger.genesis->hash (), node.network_params.ledger.genesis->root ());
-	}
-
-	return hash_root;
+	return std::nullopt;
 }
 
 bool nano::rep_crawler::track_rep_request (hash_root_t hash_root, std::shared_ptr<nano::transport::channel> const & channel)

nano/secure/ledger.cpp

@@ -936,33 +936,25 @@ std::string nano::ledger::block_text (nano::block_hash const & hash_a)
 	return result;
 }
 
-std::pair<nano::block_hash, nano::root> nano::ledger::hash_root_random (secure::transaction const & transaction_a) const
+std::deque<std::shared_ptr<nano::block>> nano::ledger::random_blocks (secure::transaction const & transaction, size_t count) const
 {
-	nano::block_hash hash (0);
-	nano::root root (0);
-	if (!pruning)
+	std::deque<std::shared_ptr<nano::block>> result;
+
+	auto const starting_hash = nano::random_pool::generate<nano::block_hash> ();
+
+	// It is more efficient to choose a random starting point and pick a few sequential blocks from there
+	auto it = store.block.begin (transaction, starting_hash);
+	auto const end = store.block.end (transaction);
+	while (result.size () < count)
 	{
-		auto block (store.block.random (transaction_a));
-		hash = block->hash ();
-		root = block->root ();
-	}
-	else
-	{
-		uint64_t count (cache.block_count);
-		auto region = nano::random_pool::generate_word64 (0, count - 1);
-		// Pruned cache cannot guarantee that pruned blocks are already commited
-		if (region < cache.pruned_count)
+		if (it != end)
 		{
-			hash = store.pruned.random (transaction_a);
-		}
-		if (hash.is_zero ())
-		{
-			auto block (store.block.random (transaction_a));
-			hash = block->hash ();
-			root = block->root ();
+			result.push_back (it->second.block);
 		}
+		++it; // Store iterators wrap around when reaching the end
 	}
-	return std::make_pair (hash, root);
+
+	return result;
 }
 
 // Vote weight of an account

nano/secure/ledger.hpp

@@ -58,7 +58,7 @@ public:
 	nano::block_hash representative_calculated (secure::transaction const &, nano::block_hash const &);
 	std::string block_text (char const *);
 	std::string block_text (nano::block_hash const &);
-	std::pair<nano::block_hash, nano::root> hash_root_random (secure::transaction const &) const;
+	std::deque<std::shared_ptr<nano::block>> random_blocks (secure::transaction const &, size_t count) const;
 	std::optional<nano::pending_info> pending_info (secure::transaction const &, nano::pending_key const & key) const;
 	std::deque<std::shared_ptr<nano::block>> confirm (secure::write_transaction &, nano::block_hash const & hash, size_t max_blocks = 1024 * 128);
 	nano::block_status process (secure::write_transaction const &, std::shared_ptr<nano::block> block);