Skip to content

Commit

Permalink
Changes the accounts_frontiers RPC to return per account results (#3791)
Browse files Browse the repository at this point in the history
* Changes the accounts_frontiers RPC to return per account results
* Adds an unit test for checking per account responses

Co-authored-by: Dimitrios Siganos <[email protected]>
  • Loading branch information
thsfs and dsiganos authored Apr 14, 2022
1 parent ba83c03 commit c647b6b
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 16 deletions.
15 changes: 11 additions & 4 deletions nano/node/json_handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 ();
Expand Down
54 changes: 42 additions & 12 deletions nano/rpc_test/rpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<std::string, std::string> 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<std::string> (""));
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<std::string> ("");
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)
Expand Down

0 comments on commit c647b6b

Please sign in to comment.