diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index 6e918e36c79..0e151b38d0a 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/src/test/app/MultiSign_test.cpp @@ -250,7 +250,8 @@ class MultiSign_test : public beast::unit_test::suite env(noop(alice), msig(demon, demon), fee(3 * baseFee), - ter(telENV_RPC_FAILED)); + rpc("invalidTransaction", + "fails local checks: Duplicate Signers not allowed.")); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq); @@ -361,7 +362,10 @@ 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(telENV_RPC_FAILED)); + env(noop(alice), + phantoms, + rpc("invalidTransaction", + "fails local checks: Unsorted Signers array.")); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq); } @@ -1640,7 +1644,8 @@ class MultiSign_test : public beast::unit_test::suite env(noop(alice), msig(demon, demon), fee(3 * baseFee), - ter(telENV_RPC_FAILED)); + rpc("invalidTransaction", + "fails local checks: Duplicate Signers not allowed.")); env.close(); BEAST_EXPECT(env.seq(alice) == aliceSeq); diff --git a/src/test/app/Regression_test.cpp b/src/test/app/Regression_test.cpp index e2c4b355a9d..f743a30f079 100644 --- a/src/test/app/Regression_test.cpp +++ b/src/test/app/Regression_test.cpp @@ -149,7 +149,9 @@ struct Regression_test : public beast::unit_test::suite secp256r1Sig->setFieldVL(sfSigningPubKey, *pubKeyBlob); jt.stx.reset(secp256r1Sig.release()); - env(jt, ter(telENV_RPC_FAILED)); + env(jt, + rpc("invalidTransaction", + "fails local checks: Invalid signature.")); }; Account const alice{"alice", KeyType::secp256k1}; diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 4ddb15a1454..086bb787d68 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -1058,16 +1058,17 @@ class TxQPosNegFlows_test : public beast::unit_test::suite auto const& jt = env.jt(noop(alice)); BEAST_EXPECT(jt.stx); - bool didApply; - TER ter; + Env::ParsedResult parsed; env.app().openLedger().modify( [&](OpenView& view, beast::Journal j) { - std::tie(ter, didApply) = ripple::apply( + // No need to initialize, since it's about to get set + bool didApply; + std::tie(parsed.ter, didApply) = ripple::apply( env.app(), view, *jt.stx, tapNONE, env.journal); return didApply; }); - env.postconditions(jt, ter, didApply); + env.postconditions(jt, parsed); } checkMetrics(__LINE__, env, 1, std::nullopt, 4, 2, 256); diff --git a/src/test/jtx.h b/src/test/jtx.h index 03bbf154e63..e6651fc1f0d 100644 --- a/src/test/jtx.h +++ b/src/test/jtx.h @@ -54,6 +54,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h index 69640179b11..0f21bff9fb6 100644 --- a/src/test/jtx/Env.h +++ b/src/test/jtx/Env.h @@ -120,6 +120,20 @@ class Env Account const& master = Account::master; + /// Used by parseResult() and postConditions() + struct ParsedResult + { + std::optional ter{}; + // RPC errors tend to return either a "code" and a "message" (sometimes + // with an "error" that corresponds to the "code"), or with an "error" + // and an "exception". However, this structure allows all possible + // combinations. + std::optional rpcCode{}; + std::string rpcMessage; + std::string rpcError; + std::string rpcException; + }; + private: struct AppBundle { @@ -493,7 +507,7 @@ class Env /** Gets the TER result and `didApply` flag from a RPC Json result object. */ - static std::pair + static ParsedResult parseResult(Json::Value const& jr); /** Submit an existing JTx. @@ -514,8 +528,7 @@ class Env void postconditions( JTx const& jt, - TER ter, - bool didApply, + ParsedResult const& parsed, Json::Value const& jr = Json::Value()); /** Apply funclets and submit. */ diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 8a42b554b8e..5c08469e0b8 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -747,9 +747,12 @@ 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 telENV_RPC_FAILED - envs(noop(alice), fee(none), seq(none), ter(telENV_RPC_FAILED))( - params); + envs( + noop(alice), + fee(none), + seq(none), + rpc(rpcHIGH_FEE, + "Fee of 10 exceeds the requested tx limit of 5"))(params); auto tx = env.tx(); BEAST_EXPECT(!tx); diff --git a/src/test/jtx/JTx.h b/src/test/jtx/JTx.h index 5f73c25f4e7..06b68dda336 100644 --- a/src/test/jtx/JTx.h +++ b/src/test/jtx/JTx.h @@ -44,6 +44,9 @@ struct JTx Json::Value jv; requires_t require; std::optional ter = TER{tesSUCCESS}; + std::optional> rpcCode = std::nullopt; + std::optional>> + rpcException = std::nullopt; bool fill_fee = true; bool fill_seq = true; bool fill_sig = true; diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 17d2d511846..884caf9162e 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -272,24 +272,48 @@ Env::trust(STAmount const& amount, Account const& account) test.expect(balance(account) == start); } -std::pair +Env::ParsedResult Env::parseResult(Json::Value const& jr) { - TER ter; - if (jr.isObject() && jr.isMember(jss::result) && - jr[jss::result].isMember(jss::engine_result_code)) - ter = TER::fromInt(jr[jss::result][jss::engine_result_code].asInt()); + auto error = [](ParsedResult& parsed, Json::Value const& object) { + // Use an error code that is not used anywhere in the transaction + // engine to distinguish this case. + parsed.ter = telENV_RPC_FAILED; + // Extract information about the error + if (!object.isObject()) + return; + if (object.isMember(jss::error_code)) + parsed.rpcCode = + safe_cast(object[jss::error_code].asInt()); + if (object.isMember(jss::error_message)) + parsed.rpcMessage = object[jss::error_message].asString(); + if (object.isMember(jss::error)) + parsed.rpcError = object[jss::error].asString(); + if (object.isMember(jss::error_exception)) + parsed.rpcException = object[jss::error_exception].asString(); + }; + ParsedResult parsed; + if (jr.isObject() && jr.isMember(jss::result)) + { + auto const& result = jr[jss::result]; + if (result.isMember(jss::engine_result_code)) + { + parsed.ter = TER::fromInt(result[jss::engine_result_code].asInt()); + parsed.rpcCode.emplace(rpcSUCCESS); + } + else + error(parsed, result); + } else - // 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)); + error(parsed, jr); + + return parsed; } void Env::submit(JTx const& jt) { - bool didApply; + ParsedResult parsedResult; auto const jr = [&]() { if (jt.stx) { @@ -298,7 +322,9 @@ Env::submit(JTx const& jt) jt.stx->add(s); auto const jr = rpc("submit", strHex(s.slice())); - std::tie(ter_, didApply) = parseResult(jr); + parsedResult = parseResult(jr); + test.expect(parsedResult.ter, "ter uninitialized!"); + ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED); return jr; } @@ -306,20 +332,17 @@ Env::submit(JTx const& jt) { // Parsing failed or the JTx is // otherwise missing the stx field. - ter_ = temMALFORMED; - didApply = false; + parsedResult.ter = ter_ = temMALFORMED; return Json::Value(); } }(); - return postconditions(jt, ter_, didApply, jr); + return postconditions(jt, parsedResult, jr); } void Env::sign_and_submit(JTx const& jt, Json::Value params) { - bool didApply; - auto const account = lookup(jt.jv[jss::Account].asString()); auto const& passphrase = account.name(); @@ -348,24 +371,55 @@ Env::sign_and_submit(JTx const& jt, Json::Value params) if (!txid_.parseHex(jr[jss::result][jss::tx_json][jss::hash].asString())) txid_.zero(); - std::tie(ter_, didApply) = parseResult(jr); + ParsedResult const parsedResult = parseResult(jr); + test.expect(parsedResult.ter, "ter uninitialized!"); + ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED); - return postconditions(jt, ter_, didApply, jr); + return postconditions(jt, parsedResult, jr); } void Env::postconditions( JTx const& jt, - TER ter, - bool didApply, + ParsedResult const& parsed, Json::Value const& jr) { - if (jt.ter && - !test.expect( - ter == *jt.ter, - "apply: Got " + transToken(ter) + " (" + transHuman(ter) + - "); Expected " + transToken(*jt.ter) + " (" + - transHuman(*jt.ter) + ")")) + bool bad = !test.expect(parsed.ter, "apply: No ter result!"); + bad = + (jt.ter && parsed.ter && + !test.expect( + *parsed.ter == *jt.ter, + "apply: Got " + transToken(*parsed.ter) + " (" + + transHuman(*parsed.ter) + "); Expected " + + transToken(*jt.ter) + " (" + transHuman(*jt.ter) + ")")); + using namespace std::string_literals; + bad = (jt.rpcCode && + !test.expect( + parsed.rpcCode == jt.rpcCode->first && + parsed.rpcMessage == jt.rpcCode->second, + "apply: Got RPC result "s + + (parsed.rpcCode + ? RPC::get_error_info(*parsed.rpcCode).token.c_str() + : "NO RESULT") + + " (" + parsed.rpcMessage + "); Expected " + + RPC::get_error_info(jt.rpcCode->first).token.c_str() + " (" + + jt.rpcCode->second + ")")) || + bad; + // If we have an rpcCode (just checked), then the rpcException check is + // optional - the 'error' field may not be defined, but if it is, it must + // match rpcError. + bad = + (jt.rpcException && + !test.expect( + (jt.rpcCode && parsed.rpcError.empty()) || + (parsed.rpcError == jt.rpcException->first && + (!jt.rpcException->second || + parsed.rpcException == *jt.rpcException->second)), + "apply: Got RPC result "s + parsed.rpcError + " (" + + parsed.rpcException + "); Expected " + jt.rpcException->first + + " (" + jt.rpcException->second.value_or("n/a") + ")")) || + bad; + if (bad) { test.log << pretty(jt.jv) << std::endl; if (jr) diff --git a/src/test/jtx/rpc.h b/src/test/jtx/rpc.h new file mode 100644 index 00000000000..f9962849698 --- /dev/null +++ b/src/test/jtx/rpc.h @@ -0,0 +1,86 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#ifndef RIPPLE_TEST_JTX_RPC_H_INCLUDED +#define RIPPLE_TEST_JTX_RPC_H_INCLUDED + +#include +#include + +namespace ripple { +namespace test { +namespace jtx { + +/** Set the expected result code for a JTx + The test will fail if the code doesn't match. +*/ +class rpc +{ +private: + std::optional code_; + std::optional errorMessage_; + std::optional error_; + std::optional errorException_; + +public: + /// If there's an error code, we expect an error message + explicit rpc(error_code_i code, std::optional m = {}) + : code_(code), errorMessage_(m) + { + } + + /// If there is not a code, we expect an exception message + explicit rpc( + std::string error, + std::optional exceptionMessage = {}) + : error_(error), errorException_(exceptionMessage) + { + } + + void + operator()(Env&, JTx& jt) const + { + // The RPC request should fail. RPC errors result in telENV_RPC_FAILED. + jt.ter = telENV_RPC_FAILED; + if (code_) + { + auto const& errorInfo = RPC::get_error_info(*code_); + // When an RPC request returns an error code ('error_code'), it + // always includes an error message ('error_message'), and sometimes + // includes an error token ('error'). If it does, the error token is + // always obtained from the lookup into the ErrorInfo lookup table. + // + // Take advantage of that fact to populate jt.rpcException. The + // check will be aware of whether the rpcExcpetion can be safely + // ignored. + jt.rpcCode = { + *code_, + errorMessage_ ? *errorMessage_ : errorInfo.message.c_str()}; + jt.rpcException = {errorInfo.token.c_str(), std::nullopt}; + } + if (error_) + jt.rpcException = {*error_, errorException_}; + } +}; + +} // namespace jtx +} // namespace test +} // namespace ripple + +#endif diff --git a/src/test/protocol/Memo_test.cpp b/src/test/protocol/Memo_test.cpp index 89ae6dfe18a..e119ee76eec 100644 --- a/src/test/protocol/Memo_test.cpp +++ b/src/test/protocol/Memo_test.cpp @@ -56,7 +56,10 @@ 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(telENV_RPC_FAILED)); + env(memoSize, + rpc("invalidTransaction", + "fails local checks: The memo exceeds the maximum allowed " + "size.")); // This memo is just barely small enough. memoSize.jv[sfMemos.jsonName][0u][sfMemo.jsonName] @@ -72,7 +75,10 @@ class Memo_test : public beast::unit_test::suite auto& m = mi[sfCreatedNode.jsonName]; // CreatedNode in Memos m[sfMemoData.jsonName] = "3030303030"; - env(memoNonMemo, ter(telENV_RPC_FAILED)); + env(memoNonMemo, + rpc("invalidTransaction", + "fails local checks: A memo array may contain only Memo " + "objects.")); } { // Put an invalid field in a Memo object. @@ -80,7 +86,10 @@ class Memo_test : public beast::unit_test::suite memoExtra .jv[sfMemos.jsonName][0u][sfMemo.jsonName][sfFlags.jsonName] = 13; - env(memoExtra, ter(telENV_RPC_FAILED)); + env(memoExtra, + rpc("invalidTransaction", + "fails local checks: A memo may contain only MemoType, " + "MemoData or MemoFormat fields.")); } { // Put a character that is not allowed in a URL in a MemoType field. @@ -88,7 +97,11 @@ class Memo_test : public beast::unit_test::suite memoBadChar.jv[sfMemos.jsonName][0u][sfMemo.jsonName] [sfMemoType.jsonName] = strHex(std::string_view("ONE