From 8ee92f15f9367c608db260ac6e0e317f4dc07f12 Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Tue, 12 Apr 2022 22:30:56 -0300 Subject: [PATCH 1/2] Changes the accounts_representatives RPC to return per account results --- nano/node/json_handler.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index 7422032c93..bcc5087228 100644 --- a/nano/node/json_handler.cpp +++ b/nano/node/json_handler.cpp @@ -14,6 +14,7 @@ #include #include +#include namespace { @@ -911,18 +912,21 @@ void nano::json_handler::accounts_balances () void nano::json_handler::accounts_representatives () { boost::property_tree::ptree representatives; - for (auto & accounts : request.get_child ("accounts")) + auto transaction = node.store.tx_begin_read (); + for (auto & account_from_request : request.get_child ("accounts")) { - auto account (account_impl (accounts.second.data ())); - auto transaction (node.store.tx_begin_read ()); - auto info (account_info_impl (transaction, account)); - + auto account = account_impl (account_from_request.second.data ()); if (!ec) { - boost::property_tree::ptree entry; - entry.put ("", info.representative.to_account ()); - representatives.push_back (std::make_pair (accounts.second.data (), entry)); + auto info = account_info_impl (transaction, account); + if (!ec) + { + representatives.put (account_from_request.second.data (), info.representative.to_account ()); + continue; + } } + representatives.put (account_from_request.second.data (), boost::format ("error: %1%") % ec.message ()); + ec = {}; } response_l.add_child ("representatives", representatives); response_errors (); From f86622f7828f2c1b15a1790e988de759835061ac Mon Sep 17 00:00:00 2001 From: Thiago Silva Date: Thu, 14 Apr 2022 15:05:05 -0300 Subject: [PATCH 2/2] Adds an unit test for checking per account results --- nano/rpc_test/rpc.cpp | 55 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp index 89f459a992..02124b7dc0 100644 --- a/nano/rpc_test/rpc.cpp +++ b/nano/rpc_test/rpc.cpp @@ -3157,6 +3157,7 @@ TEST (rpc, accounts_balances) } } +// Tests the happy path of retrieving an account's representative TEST (rpc, accounts_representatives) { nano::system system; @@ -3166,14 +3167,68 @@ TEST (rpc, accounts_representatives) request.put ("action", "accounts_representatives"); boost::property_tree::ptree entry; boost::property_tree::ptree accounts; + // Adds a valid account present in the ledger. entry.put ("", nano::dev::genesis_key.pub.to_account ()); accounts.push_back (std::make_pair ("", entry)); request.add_child ("accounts", accounts); auto response (wait_response (system, rpc_ctx, request)); + // Ensures the response is correct. auto response_representative (response.get_child ("representatives").get (nano::dev::genesis->account ().to_account ())); ASSERT_EQ (response_representative, nano::dev::genesis->account ().to_account ()); } +// Tests the accounts_representatives RPC for expected per account results with +// two possible error conditions: bad account number and account not found. +TEST (rpc, accounts_representatives_per_account_result_with_errors) +{ + nano::system system; + auto node = add_ipc_enabled_node (system); + auto const rpc_ctx = add_rpc (system, node); + boost::property_tree::ptree request; + request.put ("action", "accounts_representatives"); + boost::property_tree::ptree entry1, entry2, entry3; + boost::property_tree::ptree accounts_l; + + // Adds a valid account present in the ledger. + entry1.put ("", nano::dev::genesis_key.pub.to_account ()); + accounts_l.push_back (std::make_pair ("", entry1)); + + // Adds an invalid account, malformed number with a wrong checksum. + // Got with this formula: key1.substr(0, 40) + key2.substr(40, key2.size()). + auto const invalid_key = "nano_36uccgpjzhjsdbj44wm1y5hyz8gefx3wjpp1jircxt84nopxkxti5bzq1rnz"; + entry2.put ("", invalid_key); + accounts_l.push_back (std::make_pair ("", entry2)); + + // Adds a valid key but that isn't on the ledger. It wont'be found. + auto const valid_key = "nano_1hrts7hcoozxccnffoq9hqhngnn9jz783usapejm57ejtqcyz9dpso1bibuy"; + entry3.put ("", valid_key); + accounts_l.push_back (std::make_pair ("", entry3)); + + // Packs all the account entries. + request.add_child ("accounts", accounts_l); + auto response (wait_response (system, rpc_ctx, request)); + + auto get_error_message = [] (nano::error_common error_common) -> std::string { + std::error_code ec = error_common; + return boost::str (boost::format ("error: %1%") % ec.message ()); + }; + + std::map reply_map{ + { nano::dev::genesis_key.pub.to_account (), nano::dev::genesis->account ().to_account () }, + { invalid_key, get_error_message (nano::error_common::bad_account_number) }, + { valid_key, get_error_message (nano::error_common::account_not_found) } + }; + + for (auto & representative : response.get_child ("representatives")) + { + std::string account_text = representative.first; + std::string frontier_text = representative.second.get (""); + ASSERT_EQ (frontier_text, reply_map[account_text]); + reply_map.erase (account_text); + } + ASSERT_EQ (reply_map.size (), 0); +} + TEST (rpc, accounts_frontiers) { nano::system system;