Skip to content

Commit

Permalink
test: Env unit test RPC errors return a unique result: (#4877)
Browse files Browse the repository at this point in the history
* telENV_RPC_FAILED is a new code, reserved exclusively
  for unit tests when RPC fails. This will
  make those types of errors distinct and easier to test
  for when expected and/or diagnose when not.
* Output RPC command result when result is not expected.
  • Loading branch information
ximinez authored Mar 19, 2024
1 parent af9cabe commit 0c32fc5
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 32 deletions.
3 changes: 2 additions & 1 deletion src/ripple/protocol/TER.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ enum TELcodes : TERUnderlyingType {
telCAN_NOT_QUEUE_FULL,
telWRONG_NETWORK,
telREQUIRES_NETWORK_ID,
telNETWORK_ID_MAKES_TX_NON_CANONICAL
telNETWORK_ID_MAKES_TX_NON_CANONICAL,
telENV_RPC_FAILED
};

//------------------------------------------------------------------------------
Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/TER.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ transResults()
MAKE_ERROR(telWRONG_NETWORK, "Transaction specifies a Network ID that differs from that of the local node."),
MAKE_ERROR(telREQUIRES_NETWORK_ID, "Transactions submitted to this node/network must include a correct NetworkID field."),
MAKE_ERROR(telNETWORK_ID_MAKES_TX_NON_CANONICAL, "Transactions submitted to this node/network must NOT include a NetworkID field."),
MAKE_ERROR(telENV_RPC_FAILED, "Unit test RPC failure."),

MAKE_ERROR(temMALFORMED, "Malformed transaction."),
MAKE_ERROR(temBAD_AMM_TOKENS, "Malformed: Invalid LPTokens."),
Expand Down
12 changes: 9 additions & 3 deletions src/test/app/MultiSign_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,10 @@ class MultiSign_test : public beast::unit_test::suite

// Duplicate signers should fail.
aliceSeq = env.seq(alice);
env(noop(alice), msig(demon, demon), fee(3 * baseFee), ter(temINVALID));
env(noop(alice),
msig(demon, demon),
fee(3 * baseFee),
ter(telENV_RPC_FAILED));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);

Expand Down Expand Up @@ -358,7 +361,7 @@ class MultiSign_test : public beast::unit_test::suite
msig phantoms{bogie, demon};
std::reverse(phantoms.signers.begin(), phantoms.signers.end());
std::uint32_t const aliceSeq = env.seq(alice);
env(noop(alice), phantoms, ter(temINVALID));
env(noop(alice), phantoms, ter(telENV_RPC_FAILED));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);
}
Expand Down Expand Up @@ -1634,7 +1637,10 @@ class MultiSign_test : public beast::unit_test::suite

// Duplicate signers should fail.
aliceSeq = env.seq(alice);
env(noop(alice), msig(demon, demon), fee(3 * baseFee), ter(temINVALID));
env(noop(alice),
msig(demon, demon),
fee(3 * baseFee),
ter(telENV_RPC_FAILED));
env.close();
BEAST_EXPECT(env.seq(alice) == aliceSeq);

Expand Down
2 changes: 1 addition & 1 deletion src/test/app/Regression_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ struct Regression_test : public beast::unit_test::suite
secp256r1Sig->setFieldVL(sfSigningPubKey, *pubKeyBlob);
jt.stx.reset(secp256r1Sig.release());

env(jt, ter(temINVALID));
env(jt, ter(telENV_RPC_FAILED));
};

Account const alice{"alice", KeyType::secp256k1};
Expand Down
3 changes: 3 additions & 0 deletions src/test/app/ValidatorSite_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,10 @@ class ValidatorSite_test : public beast::unit_test::suite

std::vector<std::string> uris;
for (auto const& u : servers)
{
log << "Testing " << u.uri << std::endl;
uris.push_back(u.uri);
}
sites->load(uris);
sites->start();
sites->join();
Expand Down
6 changes: 5 additions & 1 deletion src/test/jtx/Env.h
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,11 @@ class Env
of JTx submission.
*/
void
postconditions(JTx const& jt, TER ter, bool didApply);
postconditions(
JTx const& jt,
TER ter,
bool didApply,
Json::Value const& jr = Json::Value());

/** Apply funclets and submit. */
/** @{ */
Expand Down
5 changes: 3 additions & 2 deletions src/test/jtx/Env_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -747,8 +747,9 @@ class Env_test : public beast::unit_test::suite
// Force the factor low enough to fail
params[jss::fee_mult_max] = 1;
params[jss::fee_div_max] = 2;
// RPC errors result in temINVALID
envs(noop(alice), fee(none), seq(none), ter(temINVALID))(params);
// RPC errors result in telENV_RPC_FAILED
envs(noop(alice), fee(none), seq(none), ter(telENV_RPC_FAILED))(
params);

auto tx = env.tx();
BEAST_EXPECT(!tx);
Expand Down
52 changes: 33 additions & 19 deletions src/test/jtx/impl/Env.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,31 +280,39 @@ Env::parseResult(Json::Value const& jr)
jr[jss::result].isMember(jss::engine_result_code))
ter = TER::fromInt(jr[jss::result][jss::engine_result_code].asInt());
else
ter = temINVALID;
// Use an error code that is not used anywhere in the transaction engine
// to distinguish this case.
ter = telENV_RPC_FAILED;
return std::make_pair(ter, isTesSuccess(ter) || isTecClaim(ter));
}

void
Env::submit(JTx const& jt)
{
bool didApply;
if (jt.stx)
{
txid_ = jt.stx->getTransactionID();
Serializer s;
jt.stx->add(s);
auto const jr = rpc("submit", strHex(s.slice()));
auto const jr = [&]() {
if (jt.stx)
{
txid_ = jt.stx->getTransactionID();
Serializer s;
jt.stx->add(s);
auto const jr = rpc("submit", strHex(s.slice()));

std::tie(ter_, didApply) = parseResult(jr);
}
else
{
// Parsing failed or the JTx is
// otherwise missing the stx field.
ter_ = temMALFORMED;
didApply = false;
}
return postconditions(jt, ter_, didApply);
std::tie(ter_, didApply) = parseResult(jr);

return jr;
}
else
{
// Parsing failed or the JTx is
// otherwise missing the stx field.
ter_ = temMALFORMED;
didApply = false;

return Json::Value();
}
}();
return postconditions(jt, ter_, didApply, jr);
}

void
Expand Down Expand Up @@ -342,11 +350,15 @@ Env::sign_and_submit(JTx const& jt, Json::Value params)

std::tie(ter_, didApply) = parseResult(jr);

return postconditions(jt, ter_, didApply);
return postconditions(jt, ter_, didApply, jr);
}

void
Env::postconditions(JTx const& jt, TER ter, bool didApply)
Env::postconditions(
JTx const& jt,
TER ter,
bool didApply,
Json::Value const& jr)
{
if (jt.ter &&
!test.expect(
Expand All @@ -356,6 +368,8 @@ Env::postconditions(JTx const& jt, TER ter, bool didApply)
transHuman(*jt.ter) + ")"))
{
test.log << pretty(jt.jv) << std::endl;
if (jr)
test.log << pretty(jr) << std::endl;
// Don't check postconditions if
// we didn't get the expected result.
return;
Expand Down
7 changes: 7 additions & 0 deletions src/test/net/DatabaseDownloader_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ class DatabaseDownloader_test : public beast::unit_test::suite
// server to request from. Use the /textfile endpoint
// to get a simple text file sent as response.
auto server = createServer(env);
log << "Downloading DB from " << server->local_endpoint() << std::endl;

ripple::test::detail::FileDirGuard const data{
*this, "downloads", "data", "", false, false};
Expand Down Expand Up @@ -225,6 +226,8 @@ class DatabaseDownloader_test : public beast::unit_test::suite
auto server = createServer(env);
auto host = server->local_endpoint().address().to_string();
auto port = std::to_string(server->local_endpoint().port());
log << "Downloading DB from " << server->local_endpoint()
<< std::endl;
server->stop();
BEAST_EXPECT(dl->download(
host,
Expand All @@ -249,6 +252,8 @@ class DatabaseDownloader_test : public beast::unit_test::suite
ripple::test::detail::FileDirGuard const datafile{
*this, "downloads", "data", "", false, false};
auto server = createServer(env, false);
log << "Downloading DB from " << server->local_endpoint()
<< std::endl;
BEAST_EXPECT(dl->download(
server->local_endpoint().address().to_string(),
std::to_string(server->local_endpoint().port()),
Expand All @@ -272,6 +277,8 @@ class DatabaseDownloader_test : public beast::unit_test::suite
ripple::test::detail::FileDirGuard const datafile{
*this, "downloads", "data", "", false, false};
auto server = createServer(env);
log << "Downloading DB from " << server->local_endpoint()
<< std::endl;
BEAST_EXPECT(dl->download(
server->local_endpoint().address().to_string(),
std::to_string(server->local_endpoint().port()),
Expand Down
10 changes: 5 additions & 5 deletions src/test/protocol/Memo_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class Memo_test : public beast::unit_test::suite
JTx memoSize = makeJtxWithMemo();
memoSize.jv[sfMemos.jsonName][0u][sfMemo.jsonName]
[sfMemoData.jsonName] = std::string(2020, '0');
env(memoSize, ter(temINVALID));
env(memoSize, ter(telENV_RPC_FAILED));

// This memo is just barely small enough.
memoSize.jv[sfMemos.jsonName][0u][sfMemo.jsonName]
Expand All @@ -72,23 +72,23 @@ class Memo_test : public beast::unit_test::suite
auto& m = mi[sfCreatedNode.jsonName]; // CreatedNode in Memos
m[sfMemoData.jsonName] = "3030303030";

env(memoNonMemo, ter(temINVALID));
env(memoNonMemo, ter(telENV_RPC_FAILED));
}
{
// Put an invalid field in a Memo object.
JTx memoExtra = makeJtxWithMemo();
memoExtra
.jv[sfMemos.jsonName][0u][sfMemo.jsonName][sfFlags.jsonName] =
13;
env(memoExtra, ter(temINVALID));
env(memoExtra, ter(telENV_RPC_FAILED));
}
{
// Put a character that is not allowed in a URL in a MemoType field.
JTx memoBadChar = makeJtxWithMemo();
memoBadChar.jv[sfMemos.jsonName][0u][sfMemo.jsonName]
[sfMemoType.jsonName] =
strHex(std::string_view("ONE<INFINITY"));
env(memoBadChar, ter(temINVALID));
env(memoBadChar, ter(telENV_RPC_FAILED));
}
{
// Put a character that is not allowed in a URL in a MemoData field.
Expand All @@ -105,7 +105,7 @@ class Memo_test : public beast::unit_test::suite
memoBadChar.jv[sfMemos.jsonName][0u][sfMemo.jsonName]
[sfMemoFormat.jsonName] =
strHex(std::string_view("NoBraces{}InURL"));
env(memoBadChar, ter(temINVALID));
env(memoBadChar, ter(telENV_RPC_FAILED));
}
}

Expand Down

0 comments on commit 0c32fc5

Please sign in to comment.