Skip to content

Commit

Permalink
Improve handling of RPC ledger_index argument:
Browse files Browse the repository at this point in the history
Some RPC commands return `ledger_index` as a quoted numeric
string. This change allows the returned value to be directly
copied and used for follow-on RPC commands.

This commit fixes #3533
  • Loading branch information
gregtatcam authored and nbougalis committed Sep 1, 2020
1 parent 707868b commit 801b158
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 30 deletions.
2 changes: 1 addition & 1 deletion src/ripple/json/json_value.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ operator!=(StaticString x, std::string const& y)
* The sequence of an #arrayValue will be automatically resize and initialized
* with #nullValue. resize() can be used to enlarge or truncate an #arrayValue.
*
* The get() methods can be used to obtanis default value in the case the
* The get() methods can be used to obtain a default value in the case the
* required element does not exist.
*
* It is possible to iterate over the list of a #objectValue values using
Expand Down
40 changes: 16 additions & 24 deletions src/ripple/rpc/impl/RPCHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,31 +228,23 @@ ledgerFromRequest(T& ledger, JsonContext& context)
return {rpcINVALID_PARAMS, "ledgerHashMalformed"};
return getLedger(ledger, ledgerHash, context);
}
else if (indexValue.isNumeric())
{
return getLedger(ledger, indexValue.asInt(), context);
}
else
{
auto const index = indexValue.asString();
if (index == "validated")
{
return getLedger(ledger, LedgerShortcut::VALIDATED, context);
}
else
{
if (index.empty() || index == "current")
return getLedger(ledger, LedgerShortcut::CURRENT, context);
else if (index == "closed")
return getLedger(ledger, LedgerShortcut::CLOSED, context);
else
{
return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
}
}
}

return Status::OK;
auto const index = indexValue.asString();

if (index.empty() || index == "current")
return getLedger(ledger, LedgerShortcut::CURRENT, context);

if (index == "validated")
return getLedger(ledger, LedgerShortcut::VALIDATED, context);

if (index == "closed")
return getLedger(ledger, LedgerShortcut::CLOSED, context);

std::uint32_t iVal;
if (beast::lexicalCastChecked(iVal, index))
return getLedger(ledger, iVal, context);

return {rpcINVALID_PARAMS, "ledgerIndexMalformed"};
}
} // namespace

Expand Down
9 changes: 8 additions & 1 deletion src/test/rpc/DepositAuthorized_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,11 +228,18 @@ class DepositAuthorized_test : public beast::unit_test::suite
}
{
// Request an invalid ledger.
Json::Value args{depositAuthArgs(alice, becky, "17")};
Json::Value args{depositAuthArgs(alice, becky, "-1")};
Json::Value const result{
env.rpc("json", "deposit_authorized", args.toStyledString())};
verifyErr(result, "invalidParams", "ledgerIndexMalformed");
}
{
// Request a ledger that doesn't exist yet as a string.
Json::Value args{depositAuthArgs(alice, becky, "17")};
Json::Value const result{
env.rpc("json", "deposit_authorized", args.toStyledString())};
verifyErr(result, "lgrNotFound", "ledgerNotFound");
}
{
// Request a ledger that doesn't exist yet.
Json::Value args{depositAuthArgs(alice, becky)};
Expand Down
36 changes: 32 additions & 4 deletions src/test/rpc/LedgerRPC_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,24 @@ class LedgerRPC_test : public beast::unit_test::suite
BEAST_EXPECT(env.current()->info().seq == 4);

{
// in this case, numeric string converted to number
auto const jrr = env.rpc("ledger", "1")[jss::result];
Json::Value jvParams;
// can be either numeric or quoted numeric
jvParams[jss::ledger_index] = 1;
auto const jrr =
env.rpc("json", "ledger", to_string(jvParams))[jss::result];
BEAST_EXPECT(jrr[jss::ledger][jss::closed] == true);
BEAST_EXPECT(jrr[jss::ledger][jss::ledger_index] == "1");
BEAST_EXPECT(jrr[jss::ledger][jss::accepted] == true);
BEAST_EXPECT(
jrr[jss::ledger][jss::totalCoins] ==
env.balance(env.master).value().getText());
}

{
Json::Value jvParams;
jvParams[jss::ledger_index] = "1";
auto const jrr =
env.rpc("json", "ledger", to_string(jvParams))[jss::result];
BEAST_EXPECT(jrr[jss::ledger][jss::closed] == true);
BEAST_EXPECT(jrr[jss::ledger][jss::ledger_index] == "1");
BEAST_EXPECT(jrr[jss::ledger][jss::accepted] == true);
Expand Down Expand Up @@ -110,8 +126,18 @@ class LedgerRPC_test : public beast::unit_test::suite
env.close();

{
// ask for an arbitrary string - index
Json::Value jvParams;
jvParams[jss::ledger_index] = "potato";
auto const jrr =
env.rpc("json", "ledger", to_string(jvParams))[jss::result];
checkErrorValue(jrr, "invalidParams", "ledgerIndexMalformed");
}

{
// ask for a negative index
Json::Value jvParams;
jvParams[jss::ledger_index] = "0"; // NOT an integer
jvParams[jss::ledger_index] = -1;
auto const jrr =
env.rpc("json", "ledger", to_string(jvParams))[jss::result];
checkErrorValue(jrr, "invalidParams", "ledgerIndexMalformed");
Expand Down Expand Up @@ -393,8 +419,10 @@ class LedgerRPC_test : public beast::unit_test::suite
void
testLedgerEntryDepositPreauth()
{
testcase("ledger_entry Request DepositPreauth");
testcase("ledger_entry Deposit Preauth");

using namespace test::jtx;

Env env{*this};
Account const alice{"alice"};
Account const becky{"becky"};
Expand Down

0 comments on commit 801b158

Please sign in to comment.