APIv2: add error messages for account_tx (#4571)

Certain inputs for the AccountTx method should return an error. In other
words, an invalid request from a user or client now results in an error
message.

Since this can change the response from the API, it is an API breaking
change. This commit maintains backward compatibility by keeping the
existing behavior for existing requests. When clients specify
"api_version": 2, they will be able to get the updated error messages.

Update unit tests to check the error based on the API version.

* Fix #4288
* Fix #4545
This commit is contained in:
Peter Chen
2023-06-29 11:41:13 -04:00
committed by GitHub
parent 4111382a31
commit d34e8be316
2 changed files with 192 additions and 88 deletions

View File

@@ -53,9 +53,23 @@ using LedgerSpecifier = RelationalDatabase::LedgerSpecifier;
// parses args into a ledger specifier, or returns a Json object on error
std::variant<std::optional<LedgerSpecifier>, Json::Value>
parseLedgerArgs(Json::Value const& params)
parseLedgerArgs(RPC::Context& context, Json::Value const& params)
{
Json::Value response;
// if ledger_index_min or max is specified, then ledger_hash or ledger_index
// should not be specified. Error out if it is
if (context.apiVersion > 1)
{
if ((params.isMember(jss::ledger_index_min) ||
params.isMember(jss::ledger_index_max)) &&
(params.isMember(jss::ledger_hash) ||
params.isMember(jss::ledger_index)))
{
RPC::Status status{rpcINVALID_PARAMS, "invalidParams"};
status.inject(response);
return response;
}
}
if (params.isMember(jss::ledger_index_min) ||
params.isMember(jss::ledger_index_max))
{
@@ -145,6 +159,17 @@ getLedgerRange(
using T = std::decay_t<decltype(ls)>;
if constexpr (std::is_same_v<T, LedgerRange>)
{
// if ledger_index_min or ledger_index_max is out of
// valid ledger range, error out. exclude -1 as
// it is a valid input
if (context.apiVersion > 1)
{
if ((ls.max > uValidatedMax && ls.max != -1) ||
(ls.min < uValidatedMin && ls.min != 0))
{
return rpcLGR_IDX_MALFORMED;
}
}
if (ls.min > uValidatedMin)
{
uLedgerMin = ls.min;
@@ -379,7 +404,7 @@ doAccountTxJson(RPC::JsonContext& context)
args.account = *account;
auto parseRes = parseLedgerArgs(params);
auto parseRes = parseLedgerArgs(context, params);
if (auto jv = std::get_if<Json::Value>(&parseRes))
{
return *jv;

View File

@@ -108,7 +108,7 @@ class AccountTx_test : public beast::unit_test::suite
};
void
testParameters()
testParameters(unsigned int apiVersion)
{
using namespace test::jtx;
@@ -143,104 +143,178 @@ class AccountTx_test : public beast::unit_test::suite
};
Json::Value jParms;
jParms[jss::api_version] = apiVersion;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(jParms)),
rpcINVALID_PARAMS));
jParms[jss::account] = "0xDEADBEEF";
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(jParms)),
rpcACT_MALFORMED));
jParms[jss::account] = A1.human();
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(jParms))));
// Ledger min/max index
if (apiVersion < 2)
{
Json::Value p{jParms};
p[jss::ledger_index_min] = -1;
p[jss::ledger_index_max] = -1;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = 0;
p[jss::ledger_index_max] = 100;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = 1;
p[jss::ledger_index_max] = 2;
BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = 2;
p[jss::ledger_index_max] = 1;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
(RPC::apiMaximumSupportedVersion == 1 ? rpcLGR_IDXS_INVALID
: rpcINVALID_LGR_RANGE)));
}
env.rpc("json", "account_tx", to_string(jParms)),
rpcINVALID_PARAMS));
// Ledger index min only
{
Json::Value p{jParms};
p[jss::ledger_index_min] = -1;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
jParms[jss::account] = "0xDEADBEEF";
p[jss::ledger_index_min] = 1;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = env.current()->info().seq;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
(RPC::apiMaximumSupportedVersion == 1 ? rpcLGR_IDXS_INVALID
: rpcINVALID_LGR_RANGE)));
env.rpc("json", "account_tx", to_string(jParms)),
rpcACT_MALFORMED));
jParms[jss::account] = A1.human();
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(jParms))));
// Ledger min/max index
{
Json::Value p{jParms};
p[jss::ledger_index_min] = -1;
p[jss::ledger_index_max] = -1;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = 0;
p[jss::ledger_index_max] = 100;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = 1;
p[jss::ledger_index_max] = 2;
BEAST_EXPECT(
noTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = 2;
p[jss::ledger_index_max] = 1;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
(RPC::apiMaximumSupportedVersion == 1
? rpcLGR_IDXS_INVALID
: rpcINVALID_LGR_RANGE)));
}
// Ledger index min only
{
Json::Value p{jParms};
p[jss::ledger_index_min] = -1;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = 1;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = env.current()->info().seq;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
(RPC::apiMaximumSupportedVersion == 1
? rpcLGR_IDXS_INVALID
: rpcINVALID_LGR_RANGE)));
}
// Ledger index max only
{
Json::Value p{jParms};
p[jss::ledger_index_max] = -1;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_max] = env.current()->info().seq;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_max] = 3;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_max] = env.closed()->info().seq;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_max] = env.closed()->info().seq - 1;
BEAST_EXPECT(
noTxs(env.rpc("json", "account_tx", to_string(p))));
}
// Ledger Sequence
{
Json::Value p{jParms};
p[jss::ledger_index] = env.closed()->info().seq;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index] = env.closed()->info().seq - 1;
BEAST_EXPECT(
noTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index] = env.current()->info().seq;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_NOT_VALIDATED));
p[jss::ledger_index] = env.current()->info().seq + 1;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_NOT_FOUND));
}
// Ledger Hash
{
Json::Value p{jParms};
p[jss::ledger_hash] = to_string(env.closed()->info().hash);
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_hash] =
to_string(env.closed()->info().parentHash);
BEAST_EXPECT(
noTxs(env.rpc("json", "account_tx", to_string(p))));
}
}
// Ledger index max only
else
{
Json::Value p{jParms};
p[jss::ledger_index_max] = -1;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
// Ledger index max/min/index all specified
// ERRORS out with invalid Parenthesis
{
jParms[jss::account] = "0xDEADBEEF";
jParms[jss::account] = A1.human();
Json::Value p{jParms};
p[jss::ledger_index_max] = env.current()->info().seq;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_max] = -1;
p[jss::ledger_index_min] = -1;
p[jss::ledger_index] = -1;
p[jss::ledger_index_max] = env.closed()->info().seq;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcINVALID_PARAMS));
}
p[jss::ledger_index_max] = env.closed()->info().seq - 1;
BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));
}
// Ledger index min/max only
{
Json::Value p{jParms};
p[jss::ledger_index_max] = 100;
p[jss::ledger_index_min] = 0;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_IDX_MALFORMED));
// Ledger Sequence
{
Json::Value p{jParms};
p[jss::ledger_index_max] = -1;
p[jss::ledger_index_min] = -1;
BEAST_EXPECT(
hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index] = env.closed()->info().seq;
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index_min] = 2;
p[jss::ledger_index_max] = 1;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcINVALID_LGR_RANGE));
}
p[jss::ledger_index] = env.closed()->info().seq - 1;
BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_index] = env.current()->info().seq;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_NOT_VALIDATED));
p[jss::ledger_index] = env.current()->info().seq + 1;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)), rpcLGR_NOT_FOUND));
}
// Ledger Hash
{
Json::Value p{jParms};
p[jss::ledger_hash] = to_string(env.closed()->info().hash);
BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
p[jss::ledger_hash] = to_string(env.closed()->info().parentHash);
BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));
// Ledger index max only
{
Json::Value p{jParms};
p[jss::ledger_index_max] = env.current()->info().seq;
BEAST_EXPECT(isErr(
env.rpc("json", "account_tx", to_string(p)),
rpcLGR_IDX_MALFORMED));
}
}
}
@@ -593,7 +667,12 @@ public:
void
run() override
{
testParameters();
for (auto testVersion = RPC::apiMinimumSupportedVersion;
testVersion <= RPC::apiBetaVersion;
++testVersion)
{
testParameters(testVersion);
}
testContents();
testAccountDelete();
}