Minor improvements

XRPLF · Nov 3, 2023 · 53e384f · 53e384f
1 parent c1f2ea4
commit 53e384f
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 75 deletions.
diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp
@@ -621,7 +621,7 @@ LedgerMaster::isValidated(ReadView const& ledger)
         // Use the skip list in the last validated ledger to see if ledger
         // comes before the last validated ledger (and thus has been
         // validated).
-        auto hash = walkHashBySeq(seq, InboundLedger::Reason::GENERIC);
+        auto const hash = walkHashBySeq(seq, InboundLedger::Reason::GENERIC);
 
         if (!hash || ledger.info().hash != *hash)
         {
@@ -642,8 +642,7 @@ LedgerMaster::isValidated(ReadView const& ledger)
     }
     catch (SHAMapMissingNode const& mn)
     {
-        auto stream = app_.journal("IsValidated").warn();  // TODO Better name ?
-        JLOG(stream) << "Ledger #" << seq << ": " << mn.what();
+        JLOG(m_journal.warn()) << "Ledger #" << seq << ": " << mn.what();
         return false;
     }
 

diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp
@@ -129,60 +129,56 @@ fillJsonTx(
         if (stMeta)
             txJson[json_meta] = serializeHex(*stMeta);
     }
-    else
+    else if (fill.context->apiVersion > 1)
     {
-        if (fill.context->apiVersion > 1)
-        {
-            copyFrom(
-                txJson[jss::tx_json],
-                txn->getJson(JsonOptions::disable_API_prior_V2, false));
-            txJson[jss::hash] = to_string(txn->getTransactionID());
-            RPC::insertDeliverMax(
-                txJson[jss::tx_json], txnType, fill.context->apiVersion);
+        copyFrom(
+            txJson[jss::tx_json],
+            txn->getJson(JsonOptions::disable_API_prior_V2, false));
+        txJson[jss::hash] = to_string(txn->getTransactionID());
+        RPC::insertDeliverMax(
+            txJson[jss::tx_json], txnType, fill.context->apiVersion);
 
-            if (stMeta)
-            {
-                txJson[jss::meta] = stMeta->getJson(JsonOptions::none);
-
-                // If applicable, insert delivered amount
-                if (txnType == ttPAYMENT || txnType == ttCHECK_CASH)
-                    RPC::insertDeliveredAmount(
-                        txJson[jss::meta],
-                        fill.ledger,
-                        txn,
-                        {txn->getTransactionID(), fill.ledger.seq(), *stMeta});
-            }
+        if (stMeta)
+        {
+            txJson[jss::meta] = stMeta->getJson(JsonOptions::none);
+
+            // If applicable, insert delivered amount
+            if (txnType == ttPAYMENT || txnType == ttCHECK_CASH)
+                RPC::insertDeliveredAmount(
+                    txJson[jss::meta],
+                    fill.ledger,
+                    txn,
+                    {txn->getTransactionID(), fill.ledger.seq(), *stMeta});
+        }
 
-            if (!fill.ledger.open())
-                txJson[jss::ledger_hash] = to_string(fill.ledger.info().hash);
+        if (!fill.ledger.open())
+            txJson[jss::ledger_hash] = to_string(fill.ledger.info().hash);
 
-            const bool validated =
-                fill.context->ledgerMaster.isValidated(fill.ledger);
-            txJson[jss::validated] = validated;
-            if (validated)
-            {
-                txJson[jss::ledger_index] = to_string(fill.ledger.seq());
-                if (fill.closeTime)
-                    txJson[jss::close_time_iso] =
-                        to_string_iso(*fill.closeTime);
-            }
+        const bool validated =
+            fill.context->ledgerMaster.isValidated(fill.ledger);
+        txJson[jss::validated] = validated;
+        if (validated)
+        {
+            txJson[jss::ledger_index] = to_string(fill.ledger.seq());
+            if (fill.closeTime)
+                txJson[jss::close_time_iso] = to_string_iso(*fill.closeTime);
         }
-        else
+    }
+    else
+    {
+        copyFrom(txJson, txn->getJson(JsonOptions::none));
+        RPC::insertDeliverMax(txJson, txnType, fill.context->apiVersion);
+        if (stMeta)
         {
-            copyFrom(txJson, txn->getJson(JsonOptions::none));
-            RPC::insertDeliverMax(txJson, txnType, fill.context->apiVersion);
-            if (stMeta)
-            {
-                txJson[jss::metaData] = stMeta->getJson(JsonOptions::none);
-
-                // If applicable, insert delivered amount
-                if (txnType == ttPAYMENT || txnType == ttCHECK_CASH)
-                    RPC::insertDeliveredAmount(
-                        txJson[jss::metaData],
-                        fill.ledger,
-                        txn,
-                        {txn->getTransactionID(), fill.ledger.seq(), *stMeta});
-            }
+            txJson[jss::metaData] = stMeta->getJson(JsonOptions::none);
+
+            // If applicable, insert delivered amount
+            if (txnType == ttPAYMENT || txnType == ttCHECK_CASH)
+                RPC::insertDeliveredAmount(
+                    txJson[jss::metaData],
+                    fill.ledger,
+                    txn,
+                    {txn->getTransactionID(), fill.ledger.seq(), *stMeta});
         }
     }
 

diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp
@@ -3106,7 +3106,6 @@ NetworkOPsImp::transJson(
     // NOTE jvObj which is not a finished object for either API version. After
     // it's populated, we need to finish it for a specific API version. This is
     // done in a loop, near the end of this function.
-    std::string const hash = to_string(transaction->getTransactionID());
     jvObj[jss::transaction] =
         transaction->getJson(JsonOptions::disable_API_prior_V2, false);
 
@@ -3159,6 +3158,7 @@ NetworkOPsImp::transJson(
         }
     }
 
+    std::string const hash = to_string(transaction->getTransactionID());
     MultiApiJson multiObj({jvObj, jvObj});
     // Minimum supported API version must match index 0 in MultiApiJson
     static_assert(apiVersionSelector(RPC::apiMinimumSupportedVersion)() == 0);

diff --git a/src/ripple/protocol/STBase.h b/src/ripple/protocol/STBase.h
@@ -38,10 +38,14 @@ struct JsonOptions
     underlying_t value;
 
     enum values : underlying_t {
-        none = 0u,
-        include_date = 1u,
-        disable_API_prior_V2 = 2u,
-        _all = 3u  // NOTE see `operator~` and bump as needed when adding enums
+        // clang-format off
+        none                        = 0b0000'0000,
+        include_date                = 0b0000'0001,
+        disable_API_prior_V2        = 0b0000'0010,
+
+        // IMPORTANT `_all` must be union of all of the above; see also operator~
+        _all                        = 0b0000'0011
+        // clang-format on
     };
 
     constexpr JsonOptions(underlying_t v) noexcept : value(v)

diff --git a/src/ripple/protocol/STTx.h b/src/ripple/protocol/STTx.h
@@ -24,7 +24,6 @@
 #include <ripple/protocol/Feature.h>
 #include <ripple/protocol/PublicKey.h>
 #include <ripple/protocol/Rules.h>
-#include <ripple/protocol/STBase.h>
 #include <ripple/protocol/STObject.h>
 #include <ripple/protocol/SecretKey.h>
 #include <ripple/protocol/SeqProxy.h>

diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp
@@ -237,31 +237,29 @@ Json::Value STTx::getJson(JsonOptions) const
 Json::Value
 STTx::getJson(JsonOptions options, bool binary) const
 {
-    if (!(options & JsonOptions::disable_API_prior_V2))
+    bool const V1 = !(options & JsonOptions::disable_API_prior_V2);
+
+    if (binary)
     {
-        // Behaviour before API version 2
-        if (binary)
+        Serializer s = STObject::getSerializer();
+        std::string const dataBin = strHex(s.peekData());
+
+        if (V1)
         {
-            Json::Value ret;
-            Serializer s = STObject::getSerializer();
-            ret[jss::tx] = strHex(s.peekData());
+            Json::Value ret(Json::objectValue);
+            ret[jss::tx] = dataBin;
             ret[jss::hash] = to_string(getTransactionID());
             return ret;
         }
-        return getJson(options);
+        else
+            return Json::Value{dataBin};
     }
 
-    // API version 2 behaviour
-    if (binary)
-    {
-        Serializer s = STObject::getSerializer();
-        Json::Value ret = strHex(s.peekData());
-        return ret;
-    }
-    // We want virtualy the same output as `getJson(JsonOptions)` overload
-    // above, but without the hash element. Since `getJson(JsonOptions)`
-    // is using STObject::getJson(JsonOptions::none), we use it here as well
-    return STObject::getJson(JsonOptions::none);
+    Json::Value ret = STObject::getJson(JsonOptions::none);
+    if (V1)
+        ret[jss::hash] = to_string(getTransactionID());
+
+    return ret;
 }
 
 std::string const&