Skip to content

Commit

Permalink
APIv2(ledger_entry): return "invalidParams" when fields missing (XRPL…
Browse files Browse the repository at this point in the history
…F#4552)

Improve error handling for ledger_entry by returning an "invalidParams"
error when one or more request fields are specified incorrectly, or one
or more required fields are missing.

For example, if none of of the following fields is provided, then the
API should return an invalidParams error:
* index, account_root, directory, offer, ripple_state, check, escrow,
  payment_channel, deposit_preauth, ticket

Prior to this commit, the API returned an "unknownOption" error instead.
Since the error was actually due to invalid parameters, rather than
unknown options, this error was misleading.

Since this is an API breaking change, the "invalidParams" error is only
returned for requests using api_version: 2 and above. To maintain
backward compatibility, the "unknownOption" error is still returned for
api_version: 1.

Related: XRPLF#4573

Fix XRPLF#4303
  • Loading branch information
arihantkothari authored and ckeshava committed Sep 22, 2023
1 parent c013763 commit 0fa1b97
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
7 changes: 6 additions & 1 deletion src/ripple/rpc/handlers/LedgerEntry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,12 @@ doLedgerEntry(RPC::JsonContext& context)
}
}
else
jvResult[jss::error] = "unknownOption";
{
if (context.apiVersion < 2u)
jvResult[jss::error] = "unknownOption";
else
jvResult[jss::error] = "invalidParams";
}
}

if (uNodeIndex.isNonZero())
Expand Down
23 changes: 19 additions & 4 deletions src/test/rpc/LedgerRPC_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <ripple/beast/unit_test.h>
#include <ripple/protocol/ErrorCodes.h>
#include <ripple/protocol/jss.h>
#include <ripple/rpc/impl/RPCHelpers.h>
#include <test/jtx.h>

namespace ripple {
Expand Down Expand Up @@ -1212,21 +1213,28 @@ class LedgerRPC_test : public beast::unit_test::suite
}

void
testLedgerEntryUnknownOption()
testLedgerEntryInvalidParams(unsigned int apiVersion)
{
testcase("ledger_entry Request Unknown Option");
testcase(
"ledger_entry Request With Invalid Parameters v" +
std::to_string(apiVersion));
using namespace test::jtx;
Env env{*this};

std::string const ledgerHash{to_string(env.closed()->info().hash)};

// "features" is not an option supported by ledger_entry.
Json::Value jvParams;
jvParams[jss::api_version] = apiVersion;
jvParams[jss::features] = ledgerHash;
jvParams[jss::ledger_hash] = ledgerHash;
Json::Value const jrr =
env.rpc("json", "ledger_entry", to_string(jvParams))[jss::result];
checkErrorValue(jrr, "unknownOption", "");

if (apiVersion < 2u)
checkErrorValue(jrr, "unknownOption", "");
else
checkErrorValue(jrr, "invalidParams", "");
}

/// @brief ledger RPC requests as a way to drive
Expand Down Expand Up @@ -1724,11 +1732,18 @@ class LedgerRPC_test : public beast::unit_test::suite
testLedgerEntryPayChan();
testLedgerEntryRippleState();
testLedgerEntryTicket();
testLedgerEntryUnknownOption();
testLookupLedger();
testNoQueue();
testQueue();
testLedgerAccountsOption();

// version specific tests
for (auto testVersion = RPC::apiMinimumSupportedVersion;
testVersion <= RPC::apiBetaVersion;
++testVersion)
{
testLedgerEntryInvalidParams(testVersion);
}
}
};

Expand Down

0 comments on commit 0fa1b97

Please sign in to comment.