Merge pull request #4707 from clemahieu/rpc_unopened_seek

Revert "Rewrite json_handler::unopened in terms of receivable iterators"
nanocurrency · Aug 20, 2024 · 67c88aa · 67c88aa
2 parents 3c67130 + ea79e5b
commit 67c88aa
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 16 deletions.
diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp
@@ -4279,8 +4279,8 @@ void nano::json_handler::unchecked_keys ()
 
 void nano::json_handler::unopened ()
 {
-	auto count (count_optional_impl ());
-	auto threshold (threshold_optional_impl ());
+	auto count{ count_optional_impl () };
+	auto threshold{ threshold_optional_impl () };
 	nano::account start{ 1 }; // exclude burn account by default
 	boost::optional<std::string> account_text (request.get_optional<std::string> ("account"));
 	if (account_text.is_initialized ())
@@ -4289,28 +4289,48 @@ void nano::json_handler::unopened ()
 	}
 	if (!ec)
 	{
-		auto transaction = node.ledger.tx_begin_read ();
-		auto & ledger = node.ledger;
+		auto transaction = node.store.tx_begin_read ();
+		auto iterator = node.store.pending.begin (transaction, nano::pending_key (start, 0));
+		auto end = node.store.pending.end ();
+		nano::account current_account = start;
+		nano::uint128_t current_account_sum{ 0 };
 		boost::property_tree::ptree accounts;
-		for (auto iterator = ledger.any.receivable_upper_bound (transaction, start, 0), end = ledger.any.receivable_end (); iterator != end && accounts.size () < count;)
+		while (iterator != end && accounts.size () < count)
 		{
-			auto const & [key, info] = *iterator;
-			nano::account account = key.account;
-			if (!node.store.account.exists (transaction, account))
+			nano::pending_key key{ iterator->first };
+			nano::account account{ key.account };
+			nano::pending_info info{ iterator->second };
+			if (node.store.account.exists (transaction, account))
 			{
-				nano::uint128_t current_account_sum{ 0 };
-				while (iterator != end)
+				if (account.number () == std::numeric_limits<nano::uint256_t>::max ())
 				{
-					auto const & [key, info] = *iterator;
-					current_account_sum += info.amount.number ();
-					++iterator;
+					break;
 				}
-				if (current_account_sum >= threshold.number ())
+				// Skip existing accounts
+				iterator = node.store.pending.begin (transaction, nano::pending_key (account.number () + 1, 0));
+			}
+			else
+			{
+				if (account != current_account)
 				{
-					accounts.put (account.to_account (), current_account_sum.convert_to<std::string> ());
+					if (current_account_sum > 0)
+					{
+						if (current_account_sum >= threshold.number ())
+						{
+							accounts.put (current_account.to_account (), current_account_sum.convert_to<std::string> ());
+						}
+						current_account_sum = 0;
+					}
+					current_account = account;
 				}
+				current_account_sum += info.amount.number ();
+				++iterator;
 			}
-			iterator = ledger.any.receivable_upper_bound (transaction, account);
+		}
+		// last one after iterator reaches end
+		if (accounts.size () < count && current_account_sum > 0 && current_account_sum >= threshold.number ())
+		{
+			accounts.put (current_account.to_account (), current_account_sum.convert_to<std::string> ());
 		}
 		response_l.add_child ("accounts", accounts);
 	}

diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp
@@ -5554,6 +5554,28 @@ TEST (rpc, unopened)
 		ASSERT_EQ (1, accounts.size ());
 		ASSERT_EQ ("1", accounts.get<std::string> (account1.to_account ()));
 	}
+	{
+		// using count=1 and a known unopened account1 number should get a single result
+		boost::property_tree::ptree request;
+		request.put ("action", "unopened");
+		request.put ("count", "1");
+		request.put ("account", account1.to_account());
+		auto response (wait_response (system, rpc_ctx, request));
+		auto & accounts (response.get_child ("accounts"));
+		ASSERT_EQ (1, accounts.size ());
+		ASSERT_EQ ("1", accounts.get<std::string> (account1.to_account ()));
+	}
+	{
+		// using count=1 and a known unopened account2 number should get a single result
+		boost::property_tree::ptree request;
+		request.put ("action", "unopened");
+		request.put ("count", "1");
+		request.put ("account", account2.to_account());
+		auto response (wait_response (system, rpc_ctx, request));
+		auto & accounts (response.get_child ("accounts"));
+		ASSERT_EQ (1, accounts.size ());
+		ASSERT_EQ ("10", accounts.get<std::string> (account2.to_account ()));
+	}
 	{
 		// using threshold at 5 should get a single result
 		boost::property_tree::ptree request;
@@ -5566,6 +5588,31 @@ TEST (rpc, unopened)
 	}
 }
 
+// Check that the "unopened" RPC can seek
+// Request unopened for the genesis account while there in an unopened account with the max account number
+TEST (rpc, unopened_seek)
+{
+	nano::test::system system;
+	auto node = add_ipc_enabled_node (system);
+	system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv);
+	nano::account last_account{ std::numeric_limits<nano::uint256_t>::max () };
+	auto genesis (node->latest (nano::dev::genesis_key.pub));
+	ASSERT_FALSE (genesis.is_zero ());
+	auto send (system.wallet (0)->send_action (nano::dev::genesis_key.pub, last_account, 1));
+	ASSERT_NE (nullptr, send);
+	auto const rpc_ctx = add_rpc (system, node);
+	{
+		boost::property_tree::ptree request;
+		request.put ("action", "unopened");
+		request.put ("count", "1");
+		request.put ("account", nano::dev::genesis_key.pub.to_account());
+		auto response (wait_response (system, rpc_ctx, request));
+		auto & accounts (response.get_child ("accounts"));
+		ASSERT_EQ (1, accounts.size ());
+		ASSERT_EQ ("1", accounts.get<std::string> (last_account.to_account ()));
+	}
+}
+
 TEST (rpc, unopened_burn)
 {
 	nano::test::system system;