Merge pull request #4496 from clemahieu/receivable_iteration

Adding abstraction of iteration over receivable entries

nano/core_test/bootstrap_ascending.cpp

@@ -4,6 +4,7 @@
 #include <nano/lib/tomlconfig.hpp>
 #include <nano/node/bootstrap_ascending/service.hpp>
 #include <nano/node/make_store.hpp>
+#include <nano/secure/ledger.hpp>
 #include <nano/test_common/system.hpp>
 #include <nano/test_common/testutil.hpp>
 
@@ -251,7 +252,7 @@ TEST (bootstrap_ascending, trace_base)
 	//	std::cerr << "--------------- Start ---------------\n";
 	ASSERT_EQ (nano::block_status::progress, node0.process (send1));
 	ASSERT_EQ (nano::block_status::progress, node0.process (receive1));
-	ASSERT_EQ (node1.store.pending.begin (node1.store.tx_begin_read (), nano::pending_key{ key.pub, 0 }), node1.store.pending.end ());
+	ASSERT_EQ (node1.ledger.receivable_end (), node1.ledger.receivable_upper_bound (node1.store.tx_begin_read (), key.pub, 0));
 	//	std::cerr << "node0: " << node0.network.endpoint () << std::endl;
 	//	std::cerr << "node1: " << node1.network.endpoint () << std::endl;
 	ASSERT_TIMELY (10s, node1.block (receive1->hash ()) != nullptr);

nano/core_test/ledger.cpp

@@ -5559,3 +5559,124 @@ TEST (ledger, head_block)
 	auto tx = store.tx_begin_read ();
 	ASSERT_EQ (*nano::dev::genesis, *ledger.head_block (tx, nano::dev::genesis_key.pub));
 }
+
+// Test that nullopt can be returned when there are no receivable entries
+TEST (ledger_receivable, upper_bound_account_none)
+{
+	auto ctx = nano::test::context::ledger_empty ();
+	ASSERT_EQ (ctx.ledger ().receivable_end (), ctx.ledger ().receivable_upper_bound (ctx.store ().tx_begin_read (), 0));
+}
+
+// Test behavior of ledger::receivable_upper_bound when there are receivable entries for multiple accounts
+TEST (ledger_receivable, upper_bound_account_key)
+{
+	auto ctx = nano::test::context::ledger_empty ();
+	nano::block_builder builder;
+	nano::keypair key;
+	auto send1 = builder
+				 .state ()
+				 .account (nano::dev::genesis_key.pub)
+				 .previous (nano::dev::genesis->hash ())
+				 .representative (nano::dev::genesis_key.pub)
+				 .balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio)
+				 .link (key.pub)
+				 .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
+				 .work (*ctx.pool ().generate (nano::dev::genesis->hash ()))
+				 .build ();
+	ASSERT_EQ (nano::block_status::progress, ctx.ledger ().process (ctx.store ().tx_begin_write (), send1));
+	auto send2 = builder
+				 .state ()
+				 .account (nano::dev::genesis_key.pub)
+				 .previous (send1->hash ())
+				 .representative (nano::dev::genesis_key.pub)
+				 .balance (nano::dev::constants.genesis_amount - 2 * nano::Gxrb_ratio)
+				 .link (nano::dev::genesis_key.pub)
+				 .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
+				 .work (*ctx.pool ().generate (send1->hash ()))
+				 .build ();
+	ASSERT_EQ (nano::block_status::progress, ctx.ledger ().process (ctx.store ().tx_begin_write (), send2));
+	auto tx = ctx.store ().tx_begin_read ();
+	auto & ledger = ctx.ledger ();
+	auto next1 = ledger.receivable_upper_bound (tx, nano::dev::genesis_key.pub);
+	auto next2 = ledger.receivable_upper_bound (tx, key.pub);
+	// Depending on which is greater but only one should have a value
+	ASSERT_TRUE (next1 == ledger.receivable_end () xor next2 == ledger.receivable_end ());
+	// The account returned should be after the one we searched for
+	ASSERT_TRUE (next1 == ledger.receivable_end () || next1->first.account == key.pub);
+	ASSERT_TRUE (next2 == ledger.receivable_end () || next2->first.account == nano::dev::genesis_key.pub);
+	auto next3 = ledger.receivable_upper_bound (tx, nano::dev::genesis_key.pub, 0);
+	auto next4 = ledger.receivable_upper_bound (tx, key.pub, 0);
+	// Neither account has more than one receivable
+	ASSERT_TRUE (next3 != ledger.receivable_end () && next4 != ledger.receivable_end ());
+	auto next5 = ledger.receivable_upper_bound (tx, next3->first.account, next3->first.hash);
+	auto next6 = ledger.receivable_upper_bound (tx, next4->first.account, next4->first.hash);
+	ASSERT_TRUE (next5 == ledger.receivable_end () && next6 == ledger.receivable_end ());
+	ASSERT_EQ (ledger.receivable_end (), ++next3);
+	ASSERT_EQ (ledger.receivable_end (), ++next4);
+}
+
+// Test that multiple receivable entries for the same account
+TEST (ledger_receivable, key_two)
+{
+	auto ctx = nano::test::context::ledger_empty ();
+	nano::block_builder builder;
+	nano::keypair key;
+	auto send1 = builder
+				 .state ()
+				 .account (nano::dev::genesis_key.pub)
+				 .previous (nano::dev::genesis->hash ())
+				 .representative (nano::dev::genesis_key.pub)
+				 .balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio)
+				 .link (key.pub)
+				 .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
+				 .work (*ctx.pool ().generate (nano::dev::genesis->hash ()))
+				 .build ();
+	ASSERT_EQ (nano::block_status::progress, ctx.ledger ().process (ctx.store ().tx_begin_write (), send1));
+	auto send2 = builder
+				 .state ()
+				 .account (nano::dev::genesis_key.pub)
+				 .previous (send1->hash ())
+				 .representative (nano::dev::genesis_key.pub)
+				 .balance (nano::dev::constants.genesis_amount - 2 * nano::Gxrb_ratio)
+				 .link (key.pub)
+				 .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
+				 .work (*ctx.pool ().generate (send1->hash ()))
+				 .build ();
+	ASSERT_EQ (nano::block_status::progress, ctx.ledger ().process (ctx.store ().tx_begin_write (), send2));
+	auto tx = ctx.store ().tx_begin_read ();
+	auto & ledger = ctx.ledger ();
+	auto next1 = ledger.receivable_upper_bound (tx, key.pub, 0);
+	ASSERT_TRUE (next1 != ledger.receivable_end () && next1->first.account == key.pub);
+	auto next2 = ledger.receivable_upper_bound (tx, key.pub, next1->first.hash);
+	ASSERT_TRUE (next2 != ledger.receivable_end () && next2->first.account == key.pub);
+	ASSERT_NE (next1->first.hash, next2->first.hash);
+	ASSERT_EQ (next2, ++next1);
+	ASSERT_EQ (ledger.receivable_end (), ++next1);
+	ASSERT_EQ (ledger.receivable_end (), ++next2);
+}
+
+TEST (ledger_receivable, any_none)
+{
+	auto ctx = nano::test::context::ledger_empty ();
+	ASSERT_FALSE (ctx.ledger ().receivable_any (ctx.store ().tx_begin_read (), nano::dev::genesis_key.pub));
+}
+
+TEST (ledger_receivable, any_one)
+{
+	auto ctx = nano::test::context::ledger_empty ();
+	nano::block_builder builder;
+	nano::keypair key;
+	auto send1 = builder
+				 .state ()
+				 .account (nano::dev::genesis_key.pub)
+				 .previous (nano::dev::genesis->hash ())
+				 .representative (nano::dev::genesis_key.pub)
+				 .balance (nano::dev::constants.genesis_amount - nano::Gxrb_ratio)
+				 .link (nano::dev::genesis_key.pub)
+				 .sign (nano::dev::genesis_key.prv, nano::dev::genesis_key.pub)
+				 .work (*ctx.pool ().generate (nano::dev::genesis->hash ()))
+				 .build ();
+	ASSERT_EQ (nano::block_status::progress, ctx.ledger ().process (ctx.store ().tx_begin_write (), send1));
+	ASSERT_TRUE (ctx.ledger ().receivable_any (ctx.store ().tx_begin_read (), nano::dev::genesis_key.pub));
+	ASSERT_FALSE (ctx.ledger ().receivable_any (ctx.store ().tx_begin_read (), key.pub));
+}

nano/node/bootstrap/bootstrap_bulk_pull.cpp

@@ -753,30 +753,16 @@ std::pair<std::unique_ptr<nano::pending_key>, std::unique_ptr<nano::pending_info
 		 * destroy a database transaction, to avoid locking the
 		 * database for a prolonged period.
 		 */
-		auto stream_transaction (node->store.tx_begin_read ());
-		auto stream (node->store.pending.begin (stream_transaction, current_key));
-
-		if (stream == store::iterator<nano::pending_key, nano::pending_info> (nullptr))
-		{
-			break;
-		}
-
-		nano::pending_key key (stream->first);
-		nano::pending_info info (stream->second);
+		auto tx = node->store.tx_begin_read ();
+		auto & ledger = node->ledger;
+		auto stream = ledger.receivable_upper_bound (tx, current_key.account, current_key.hash);
 
-		/*
-		 * Get the key for the next value, to use in the next call or iteration
-		 */
-		current_key.account = key.account;
-		current_key.hash = key.hash.number () + 1;
-
-		/*
-		 * Finish up if the response is for a different account
-		 */
-		if (key.account != request->account)
+		if (stream == ledger.receivable_end ())
 		{
 			break;
 		}
+		auto const & [key, info] = *stream;
+		current_key = key;
 
 		/*
 		 * Skip entries where the amount is less than the requested

nano/node/bootstrap_ascending/iterators.cpp

@@ -1,6 +1,7 @@
 #include <nano/lib/utility.hpp>
 #include <nano/node/bootstrap_ascending/iterators.hpp>
 #include <nano/secure/common.hpp>
+#include <nano/secure/ledger.hpp>
 #include <nano/store/account.hpp>
 #include <nano/store/component.hpp>
 #include <nano/store/pending.hpp>
@@ -9,8 +10,8 @@
  * database_iterator
  */
 
-nano::bootstrap_ascending::database_iterator::database_iterator (nano::store::component & store_a, table_type table_a) :
-	store{ store_a },
+nano::bootstrap_ascending::database_iterator::database_iterator (nano::ledger & ledger, table_type table_a) :
+	ledger{ ledger },
 	table{ table_a }
 {
 }
@@ -27,8 +28,8 @@ void nano::bootstrap_ascending::database_iterator::next (store::transaction & tx
 		case table_type::account:
 		{
 			auto i = current.number () + 1;
-			auto item = store.account.begin (tx, i);
-			if (item != store.account.end ())
+			auto item = ledger.store.account.begin (tx, i);
+			if (item != ledger.store.account.end ())
 			{
 				current = item->first;
 			}
@@ -40,9 +41,8 @@ void nano::bootstrap_ascending::database_iterator::next (store::transaction & tx
 		}
 		case table_type::pending:
 		{
-			auto i = current.number () + 1;
-			auto item = store.pending.begin (tx, nano::pending_key{ i, 0 });
-			if (item != store.pending.end ())
+			auto item = ledger.receivable_upper_bound (tx, current);
+			if (item != ledger.receivable_end ())
 			{
 				current = item->first.account;
 			}
@@ -59,10 +59,10 @@ void nano::bootstrap_ascending::database_iterator::next (store::transaction & tx
  * buffered_iterator
  */
 
-nano::bootstrap_ascending::buffered_iterator::buffered_iterator (nano::store::component & store_a) :
-	store{ store_a },
-	accounts_iterator{ store, database_iterator::table_type::account },
-	pending_iterator{ store, database_iterator::table_type::pending }
+nano::bootstrap_ascending::buffered_iterator::buffered_iterator (nano::ledger & ledger) :
+	ledger{ ledger },
+	accounts_iterator{ ledger, database_iterator::table_type::account },
+	pending_iterator{ ledger, database_iterator::table_type::pending }
 {
 }
 
@@ -95,7 +95,7 @@ void nano::bootstrap_ascending::buffered_iterator::fill ()
 	debug_assert (buffer.empty ());
 
 	// Fill half from accounts table and half from pending table
-	auto transaction = store.tx_begin_read ();
+	auto transaction = ledger.store.tx_begin_read ();
 
 	for (int n = 0; n < size / 2; ++n)
 	{

nano/node/bootstrap_ascending/iterators.hpp

@@ -4,6 +4,11 @@
 
 #include <deque>
 
+namespace nano
+{
+class ledger;
+}
+
 namespace nano::store
 {
 class component;
@@ -21,20 +26,20 @@ class database_iterator
 		pending
 	};
 
-	explicit database_iterator (nano::store::component & store, table_type);
+	explicit database_iterator (nano::ledger & ledger, table_type);
 	nano::account operator* () const;
 	void next (nano::store::transaction & tx);
 
 private:
-	nano::store::component & store;
+	nano::ledger & ledger;
 	nano::account current{ 0 };
 	const table_type table;
 };
 
 class buffered_iterator
 {
 public:
-	explicit buffered_iterator (nano::store::component & store);
+	explicit buffered_iterator (nano::ledger & ledger);
 	nano::account operator* () const;
 	nano::account next ();
 	// Indicates if a full ledger iteration has taken place e.g. warmed up
@@ -44,7 +49,7 @@ class buffered_iterator
 	void fill ();
 
 private:
-	nano::store::component & store;
+	nano::ledger & ledger;
 	std::deque<nano::account> buffer;
 	bool warmup_m{ true };
 

nano/node/bootstrap_ascending/service.cpp

@@ -24,7 +24,7 @@ nano::bootstrap_ascending::service::service (nano::node_config & config_a, nano:
 	network{ network_a },
 	stats{ stat_a },
 	accounts{ stats },
-	iterator{ ledger.store },
+	iterator{ ledger },
 	throttle{ compute_throttle_size () },
 	scoring{ config.bootstrap_ascending, config.network_params.network },
 	database_limiter{ config.bootstrap_ascending.database_requests_limit, 1.0 }

nano/node/epoch_upgrader.cpp

@@ -210,14 +210,12 @@ void nano::epoch_upgrader::upgrade_impl (nano::raw_key const & prv_a, nano::epoc
 			std::atomic<uint64_t> upgraded_pending (0);
 			uint64_t workers (0);
 			uint64_t attempts (0);
-			auto transaction (store.tx_begin_read ());
-			for (auto i (store.pending.begin (transaction, nano::pending_key (1, 0))), n (store.pending.end ()); i != n && attempts < upgrade_batch_size && attempts < count_limit && !stopped;)
+			auto transaction = store.tx_begin_read ();
+			for (auto current = ledger.receivable_upper_bound (transaction, 0), end = ledger.receivable_end (); current != end && attempts < upgrade_batch_size && attempts < count_limit && !stopped;)
 			{
-				bool to_next_account (false);
-				nano::pending_key const & key (i->first);
+				auto const & [key, info] = *current;
 				if (!store.account.exists (transaction, key.account))
 				{
-					nano::pending_info const & info (i->second);
 					if (info.epoch < epoch_a)
 					{
 						++attempts;
@@ -258,12 +256,10 @@ void nano::epoch_upgrader::upgrade_impl (nano::raw_key const & prv_a, nano::epoc
 							upgrader_process (upgraded_pending, epoch, difficulty, signer, root, account);
 						}
 					}
+					// Move to next pending item
+					current = ledger.receivable_upper_bound (transaction, key.account, key.hash);
 				}
 				else
-				{
-					to_next_account = true;
-				}
-				if (to_next_account)
 				{
 					// Move to next account if pending account exists or was upgraded
 					if (key.account.number () == std::numeric_limits<nano::uint256_t>::max ())
@@ -272,14 +268,9 @@ void nano::epoch_upgrader::upgrade_impl (nano::raw_key const & prv_a, nano::epoc
 					}
 					else
 					{
-						i = store.pending.begin (transaction, nano::pending_key (key.account.number () + 1, 0));
+						current = ledger.receivable_upper_bound (transaction, key.account);
 					}
 				}
-				else
-				{
-					// Move to next pending item
-					++i;
-				}
 			}
 			{
 				nano::unique_lock<nano::mutex> lock{ upgrader_mutex };