Skip to content

Commit d9196d6

Browse files
ximinezsophiax851
ximinez
authored and
sophiax851
committed
test: Add RPC error checking support to unit tests (XRPLF#4987)
1 parent 6b95cf8 commit d9196d6

File tree

10 files changed

+230
-45
lines changed

10 files changed

+230
-45
lines changed

‎src/test/app/MultiSign_test.cpp

+8-3
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ class MultiSign_test : public beast::unit_test::suite
250250
env(noop(alice),
251251
msig(demon, demon),
252252
fee(3 * baseFee),
253-
ter(telENV_RPC_FAILED));
253+
rpc("invalidTransaction",
254+
"fails local checks: Duplicate Signers not allowed."));
254255
env.close();
255256
BEAST_EXPECT(env.seq(alice) == aliceSeq);
256257

@@ -361,7 +362,10 @@ class MultiSign_test : public beast::unit_test::suite
361362
msig phantoms{bogie, demon};
362363
std::reverse(phantoms.signers.begin(), phantoms.signers.end());
363364
std::uint32_t const aliceSeq = env.seq(alice);
364-
env(noop(alice), phantoms, ter(telENV_RPC_FAILED));
365+
env(noop(alice),
366+
phantoms,
367+
rpc("invalidTransaction",
368+
"fails local checks: Unsorted Signers array."));
365369
env.close();
366370
BEAST_EXPECT(env.seq(alice) == aliceSeq);
367371
}
@@ -1640,7 +1644,8 @@ class MultiSign_test : public beast::unit_test::suite
16401644
env(noop(alice),
16411645
msig(demon, demon),
16421646
fee(3 * baseFee),
1643-
ter(telENV_RPC_FAILED));
1647+
rpc("invalidTransaction",
1648+
"fails local checks: Duplicate Signers not allowed."));
16441649
env.close();
16451650
BEAST_EXPECT(env.seq(alice) == aliceSeq);
16461651

‎src/test/app/Regression_test.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,9 @@ struct Regression_test : public beast::unit_test::suite
149149
secp256r1Sig->setFieldVL(sfSigningPubKey, *pubKeyBlob);
150150
jt.stx.reset(secp256r1Sig.release());
151151

152-
env(jt, ter(telENV_RPC_FAILED));
152+
env(jt,
153+
rpc("invalidTransaction",
154+
"fails local checks: Invalid signature."));
153155
};
154156

155157
Account const alice{"alice", KeyType::secp256k1};

‎src/test/app/TxQ_test.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -1058,16 +1058,17 @@ class TxQPosNegFlows_test : public beast::unit_test::suite
10581058
auto const& jt = env.jt(noop(alice));
10591059
BEAST_EXPECT(jt.stx);
10601060

1061-
bool didApply;
1062-
TER ter;
1061+
Env::ParsedResult parsed;
10631062

10641063
env.app().openLedger().modify(
10651064
[&](OpenView& view, beast::Journal j) {
1066-
std::tie(ter, didApply) = ripple::apply(
1065+
// No need to initialize, since it's about to get set
1066+
bool didApply;
1067+
std::tie(parsed.ter, didApply) = ripple::apply(
10671068
env.app(), view, *jt.stx, tapNONE, env.journal);
10681069
return didApply;
10691070
});
1070-
env.postconditions(jt, ter, didApply);
1071+
env.postconditions(jt, parsed);
10711072
}
10721073
checkMetrics(__LINE__, env, 1, std::nullopt, 4, 2, 256);
10731074

‎src/test/jtx.h

+1
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
#include <test/jtx/regkey.h>
5555
#include <test/jtx/require.h>
5656
#include <test/jtx/requires.h>
57+
#include <test/jtx/rpc.h>
5758
#include <test/jtx/sendmax.h>
5859
#include <test/jtx/seq.h>
5960
#include <test/jtx/sig.h>

‎src/test/jtx/Env.h

+16-3
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,20 @@ class Env
120120

121121
Account const& master = Account::master;
122122

123+
/// Used by parseResult() and postConditions()
124+
struct ParsedResult
125+
{
126+
std::optional<TER> ter{};
127+
// RPC errors tend to return either a "code" and a "message" (sometimes
128+
// with an "error" that corresponds to the "code"), or with an "error"
129+
// and an "exception". However, this structure allows all possible
130+
// combinations.
131+
std::optional<error_code_i> rpcCode{};
132+
std::string rpcMessage;
133+
std::string rpcError;
134+
std::string rpcException;
135+
};
136+
123137
private:
124138
struct AppBundle
125139
{
@@ -493,7 +507,7 @@ class Env
493507

494508
/** Gets the TER result and `didApply` flag from a RPC Json result object.
495509
*/
496-
static std::pair<TER, bool>
510+
static ParsedResult
497511
parseResult(Json::Value const& jr);
498512

499513
/** Submit an existing JTx.
@@ -514,8 +528,7 @@ class Env
514528
void
515529
postconditions(
516530
JTx const& jt,
517-
TER ter,
518-
bool didApply,
531+
ParsedResult const& parsed,
519532
Json::Value const& jr = Json::Value());
520533

521534
/** Apply funclets and submit. */

‎src/test/jtx/Env_test.cpp

+6-3
Original file line numberDiff line numberDiff line change
@@ -747,9 +747,12 @@ class Env_test : public beast::unit_test::suite
747747
// Force the factor low enough to fail
748748
params[jss::fee_mult_max] = 1;
749749
params[jss::fee_div_max] = 2;
750-
// RPC errors result in telENV_RPC_FAILED
751-
envs(noop(alice), fee(none), seq(none), ter(telENV_RPC_FAILED))(
752-
params);
750+
envs(
751+
noop(alice),
752+
fee(none),
753+
seq(none),
754+
rpc(rpcHIGH_FEE,
755+
"Fee of 10 exceeds the requested tx limit of 5"))(params);
753756

754757
auto tx = env.tx();
755758
BEAST_EXPECT(!tx);

‎src/test/jtx/JTx.h

+3
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ struct JTx
4444
Json::Value jv;
4545
requires_t require;
4646
std::optional<TER> ter = TER{tesSUCCESS};
47+
std::optional<std::pair<error_code_i, std::string>> rpcCode = std::nullopt;
48+
std::optional<std::pair<std::string, std::optional<std::string>>>
49+
rpcException = std::nullopt;
4750
bool fill_fee = true;
4851
bool fill_seq = true;
4952
bool fill_sig = true;

‎src/test/jtx/impl/Env.cpp

+80-26
Original file line numberDiff line numberDiff line change
@@ -272,24 +272,48 @@ Env::trust(STAmount const& amount, Account const& account)
272272
test.expect(balance(account) == start);
273273
}
274274

275-
std::pair<TER, bool>
275+
Env::ParsedResult
276276
Env::parseResult(Json::Value const& jr)
277277
{
278-
TER ter;
279-
if (jr.isObject() && jr.isMember(jss::result) &&
280-
jr[jss::result].isMember(jss::engine_result_code))
281-
ter = TER::fromInt(jr[jss::result][jss::engine_result_code].asInt());
278+
auto error = [](ParsedResult& parsed, Json::Value const& object) {
279+
// Use an error code that is not used anywhere in the transaction
280+
// engine to distinguish this case.
281+
parsed.ter = telENV_RPC_FAILED;
282+
// Extract information about the error
283+
if (!object.isObject())
284+
return;
285+
if (object.isMember(jss::error_code))
286+
parsed.rpcCode =
287+
safe_cast<error_code_i>(object[jss::error_code].asInt());
288+
if (object.isMember(jss::error_message))
289+
parsed.rpcMessage = object[jss::error_message].asString();
290+
if (object.isMember(jss::error))
291+
parsed.rpcError = object[jss::error].asString();
292+
if (object.isMember(jss::error_exception))
293+
parsed.rpcException = object[jss::error_exception].asString();
294+
};
295+
ParsedResult parsed;
296+
if (jr.isObject() && jr.isMember(jss::result))
297+
{
298+
auto const& result = jr[jss::result];
299+
if (result.isMember(jss::engine_result_code))
300+
{
301+
parsed.ter = TER::fromInt(result[jss::engine_result_code].asInt());
302+
parsed.rpcCode.emplace(rpcSUCCESS);
303+
}
304+
else
305+
error(parsed, result);
306+
}
282307
else
283-
// Use an error code that is not used anywhere in the transaction engine
284-
// to distinguish this case.
285-
ter = telENV_RPC_FAILED;
286-
return std::make_pair(ter, isTesSuccess(ter) || isTecClaim(ter));
308+
error(parsed, jr);
309+
310+
return parsed;
287311
}
288312

289313
void
290314
Env::submit(JTx const& jt)
291315
{
292-
bool didApply;
316+
ParsedResult parsedResult;
293317
auto const jr = [&]() {
294318
if (jt.stx)
295319
{
@@ -298,28 +322,27 @@ Env::submit(JTx const& jt)
298322
jt.stx->add(s);
299323
auto const jr = rpc("submit", strHex(s.slice()));
300324

301-
std::tie(ter_, didApply) = parseResult(jr);
325+
parsedResult = parseResult(jr);
326+
test.expect(parsedResult.ter, "ter uninitialized!");
327+
ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED);
302328

303329
return jr;
304330
}
305331
else
306332
{
307333
// Parsing failed or the JTx is
308334
// otherwise missing the stx field.
309-
ter_ = temMALFORMED;
310-
didApply = false;
335+
parsedResult.ter = ter_ = temMALFORMED;
311336

312337
return Json::Value();
313338
}
314339
}();
315-
return postconditions(jt, ter_, didApply, jr);
340+
return postconditions(jt, parsedResult, jr);
316341
}
317342

318343
void
319344
Env::sign_and_submit(JTx const& jt, Json::Value params)
320345
{
321-
bool didApply;
322-
323346
auto const account = lookup(jt.jv[jss::Account].asString());
324347
auto const& passphrase = account.name();
325348

@@ -348,24 +371,55 @@ Env::sign_and_submit(JTx const& jt, Json::Value params)
348371
if (!txid_.parseHex(jr[jss::result][jss::tx_json][jss::hash].asString()))
349372
txid_.zero();
350373

351-
std::tie(ter_, didApply) = parseResult(jr);
374+
ParsedResult const parsedResult = parseResult(jr);
375+
test.expect(parsedResult.ter, "ter uninitialized!");
376+
ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED);
352377

353-
return postconditions(jt, ter_, didApply, jr);
378+
return postconditions(jt, parsedResult, jr);
354379
}
355380

356381
void
357382
Env::postconditions(
358383
JTx const& jt,
359-
TER ter,
360-
bool didApply,
384+
ParsedResult const& parsed,
361385
Json::Value const& jr)
362386
{
363-
if (jt.ter &&
364-
!test.expect(
365-
ter == *jt.ter,
366-
"apply: Got " + transToken(ter) + " (" + transHuman(ter) +
367-
"); Expected " + transToken(*jt.ter) + " (" +
368-
transHuman(*jt.ter) + ")"))
387+
bool bad = !test.expect(parsed.ter, "apply: No ter result!");
388+
bad =
389+
(jt.ter && parsed.ter &&
390+
!test.expect(
391+
*parsed.ter == *jt.ter,
392+
"apply: Got " + transToken(*parsed.ter) + " (" +
393+
transHuman(*parsed.ter) + "); Expected " +
394+
transToken(*jt.ter) + " (" + transHuman(*jt.ter) + ")"));
395+
using namespace std::string_literals;
396+
bad = (jt.rpcCode &&
397+
!test.expect(
398+
parsed.rpcCode == jt.rpcCode->first &&
399+
parsed.rpcMessage == jt.rpcCode->second,
400+
"apply: Got RPC result "s +
401+
(parsed.rpcCode
402+
? RPC::get_error_info(*parsed.rpcCode).token.c_str()
403+
: "NO RESULT") +
404+
" (" + parsed.rpcMessage + "); Expected " +
405+
RPC::get_error_info(jt.rpcCode->first).token.c_str() + " (" +
406+
jt.rpcCode->second + ")")) ||
407+
bad;
408+
// If we have an rpcCode (just checked), then the rpcException check is
409+
// optional - the 'error' field may not be defined, but if it is, it must
410+
// match rpcError.
411+
bad =
412+
(jt.rpcException &&
413+
!test.expect(
414+
(jt.rpcCode && parsed.rpcError.empty()) ||
415+
(parsed.rpcError == jt.rpcException->first &&
416+
(!jt.rpcException->second ||
417+
parsed.rpcException == *jt.rpcException->second)),
418+
"apply: Got RPC result "s + parsed.rpcError + " (" +
419+
parsed.rpcException + "); Expected " + jt.rpcException->first +
420+
" (" + jt.rpcException->second.value_or("n/a") + ")")) ||
421+
bad;
422+
if (bad)
369423
{
370424
test.log << pretty(jt.jv) << std::endl;
371425
if (jr)

‎src/test/jtx/rpc.h

+86
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
//------------------------------------------------------------------------------
2+
/*
3+
This file is part of rippled: https://github.com/ripple/rippled
4+
Copyright (c) 2024 Ripple Labs Inc.
5+
6+
Permission to use, copy, modify, and/or distribute this software for any
7+
purpose with or without fee is hereby granted, provided that the above
8+
copyright notice and this permission notice appear in all copies.
9+
10+
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
11+
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
12+
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
13+
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
14+
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
15+
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
16+
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
17+
*/
18+
//==============================================================================
19+
20+
#ifndef RIPPLE_TEST_JTX_RPC_H_INCLUDED
21+
#define RIPPLE_TEST_JTX_RPC_H_INCLUDED
22+
23+
#include <test/jtx/Env.h>
24+
#include <tuple>
25+
26+
namespace ripple {
27+
namespace test {
28+
namespace jtx {
29+
30+
/** Set the expected result code for a JTx
31+
The test will fail if the code doesn't match.
32+
*/
33+
class rpc
34+
{
35+
private:
36+
std::optional<error_code_i> code_;
37+
std::optional<std::string> errorMessage_;
38+
std::optional<std::string> error_;
39+
std::optional<std::string> errorException_;
40+
41+
public:
42+
/// If there's an error code, we expect an error message
43+
explicit rpc(error_code_i code, std::optional<std::string> m = {})
44+
: code_(code), errorMessage_(m)
45+
{
46+
}
47+
48+
/// If there is not a code, we expect an exception message
49+
explicit rpc(
50+
std::string error,
51+
std::optional<std::string> exceptionMessage = {})
52+
: error_(error), errorException_(exceptionMessage)
53+
{
54+
}
55+
56+
void
57+
operator()(Env&, JTx& jt) const
58+
{
59+
// The RPC request should fail. RPC errors result in telENV_RPC_FAILED.
60+
jt.ter = telENV_RPC_FAILED;
61+
if (code_)
62+
{
63+
auto const& errorInfo = RPC::get_error_info(*code_);
64+
// When an RPC request returns an error code ('error_code'), it
65+
// always includes an error message ('error_message'), and sometimes
66+
// includes an error token ('error'). If it does, the error token is
67+
// always obtained from the lookup into the ErrorInfo lookup table.
68+
//
69+
// Take advantage of that fact to populate jt.rpcException. The
70+
// check will be aware of whether the rpcExcpetion can be safely
71+
// ignored.
72+
jt.rpcCode = {
73+
*code_,
74+
errorMessage_ ? *errorMessage_ : errorInfo.message.c_str()};
75+
jt.rpcException = {errorInfo.token.c_str(), std::nullopt};
76+
}
77+
if (error_)
78+
jt.rpcException = {*error_, errorException_};
79+
}
80+
};
81+
82+
} // namespace jtx
83+
} // namespace test
84+
} // namespace ripple
85+
86+
#endif

0 commit comments

Comments
 (0)