test: Env unit test RPC errors return a unique result: (XRPLF#4877)

* 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.
ximinez · Mar 19, 2024 · 60d046e · 60d046e
1 parent af9cabe
commit 60d046e
Show file tree

Hide file tree

Showing 10 changed files with 69 additions and 32 deletions.
diff --git a/src/ripple/protocol/TER.h b/src/ripple/protocol/TER.h
@@ -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
 };
 
 //------------------------------------------------------------------------------

diff --git a/src/ripple/protocol/impl/TER.cpp b/src/ripple/protocol/impl/TER.cpp
@@ -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."),

diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp
@@ -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);
 
@@ -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);
     }
@@ -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);
 

diff --git a/src/test/app/Regression_test.cpp b/src/test/app/Regression_test.cpp
@@ -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};

diff --git a/src/test/app/ValidatorSite_test.cpp b/src/test/app/ValidatorSite_test.cpp
@@ -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();

diff --git a/src/test/jtx/Env.h b/src/test/jtx/Env.h
@@ -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. */
     /** @{ */

diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp
@@ -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);

diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp
@@ -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
@@ -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(
@@ -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;

diff --git a/src/test/net/DatabaseDownloader_test.cpp b/src/test/net/DatabaseDownloader_test.cpp
@@ -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};
@@ -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,
@@ -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()),
@@ -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()),

diff --git a/src/test/protocol/Memo_test.cpp b/src/test/protocol/Memo_test.cpp
@@ -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]
@@ -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.
@@ -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));
         }
     }