From c647b6b7008ccc69e3c3ab7016bc6eea4149c37f Mon Sep 17 00:00:00 2001 From: Thiago Silva <82097354+thsfs@users.noreply.github.com> Date: Thu, 14 Apr 2022 16:25:04 -0300 Subject: [PATCH] Changes the accounts_frontiers RPC to return per account results (#3791) * Changes the accounts_frontiers RPC to return per account results * Adds an unit test for checking per account responses Co-authored-by: Dimitrios Siganos --- nano/node/json_handler.cpp | 15 ++++++++--- nano/rpc_test/rpc.cpp | 54 +++++++++++++++++++++++++++++--------- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/nano/node/json_handler.cpp b/nano/node/json_handler.cpp index bcc50872..46718977 100644 --- a/nano/node/json_handler.cpp +++ b/nano/node/json_handler.cpp @@ -960,18 +960,25 @@ void nano::json_handler::accounts_create () void nano::json_handler::accounts_frontiers () { boost::property_tree::ptree frontiers; - auto transaction (node.store.tx_begin_read ()); - 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 account = account_impl (account_from_request.second.data ()); if (!ec) { - auto latest (node.ledger.latest (transaction, account)); + auto latest = node.ledger.latest (transaction, account); if (!latest.is_zero ()) { frontiers.put (account.to_account (), latest.to_string ()); + continue; + } + else + { + ec = nano::error_common::account_not_found; } } + frontiers.put (account_from_request.second.data (), boost::str (boost::format ("error: %1%") % ec.message ())); + ec = {}; } response_l.add_child ("frontiers", frontiers); response_errors (); diff --git a/nano/rpc_test/rpc.cpp b/nano/rpc_test/rpc.cpp index 02124b7d..0d745654 100644 --- a/nano/rpc_test/rpc.cpp +++ b/nano/rpc_test/rpc.cpp @@ -3177,8 +3177,9 @@ TEST (rpc, accounts_representatives) 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 the RPC accounts_frontiers with 3 accounts, one good one, one with an invalid account ID and one with an account that does not exist. + */ TEST (rpc, accounts_representatives_per_account_result_with_errors) { nano::system system; @@ -3235,21 +3236,50 @@ TEST (rpc, accounts_frontiers) auto node = add_ipc_enabled_node (system); system.wallet (0)->insert_adhoc (nano::dev::genesis_key.prv); auto const rpc_ctx = add_rpc (system, node); + boost::property_tree::ptree request; request.put ("action", "accounts_frontiers"); - boost::property_tree::ptree entry; - boost::property_tree::ptree peers_l; - entry.put ("", nano::dev::genesis_key.pub.to_account ()); - peers_l.push_back (std::make_pair ("", entry)); - request.add_child ("accounts", peers_l); + boost::property_tree::ptree accounts_l; + // Adds a valid account that will be found in the ledger. + boost::property_tree::ptree entry1; + entry1.put ("", nano::dev::genesis_key.pub.to_account ()); + accounts_l.push_back (std::make_pair ("", entry1)); + // Adds a bad account number for getting an error response. + boost::property_tree::ptree entry2; + auto const bad_account_number = "nano_3e3j5tkog48pnny9dmfzj1r16pg8t1e76dz5tmac6iq689wyjfpiij4txtd1"; + entry2.put ("", bad_account_number); + accounts_l.push_back (std::make_pair ("", entry2)); + // Adds a valid account that isn't on the ledger for getting an error response. + boost::property_tree::ptree entry3; + auto const account_not_found = "nano_1os6txqxyuesnxrtshnfb5or1hesc1647wpk9rsr84pmki6eairwha79hk3j"; + entry3.put ("", account_not_found); + accounts_l.push_back (std::make_pair ("", entry3)); + request.add_child ("accounts", accounts_l); auto response (wait_response (system, rpc_ctx, request)); - for (auto & frontiers : response.get_child ("frontiers")) + + 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 ()); + }; + + // create a map of expected replies, everytime we receive and echeck a reply, we remove it from this map + // in the end, this container should be empty, which would signify that all 3 replies were received correctly + std::map reply_map{ + { nano::dev::genesis_key.pub.to_account (), nano::dev::genesis->hash ().to_string () }, + { bad_account_number, get_error_message (nano::error_common::bad_account_number) }, + { account_not_found, get_error_message (nano::error_common::account_not_found) }, + }; + + for (auto & frontier : response.get_child ("frontiers")) { - std::string account_text (frontiers.first); - ASSERT_EQ (nano::dev::genesis_key.pub.to_account (), account_text); - std::string frontier_text (frontiers.second.get ("")); - ASSERT_EQ (node->latest (nano::dev::genesis->account ()), nano::block_hash{ frontier_text }); + std::string account_text = frontier.first; + std::string frontier_text = frontier.second.get (""); + ASSERT_EQ (frontier_text, reply_map[account_text]); + reply_map.erase (account_text); } + + // we expect all replies to have been received and this container to be empty + ASSERT_EQ (reply_map.size (), 0); } TEST (rpc, accounts_pending)