Write improved forAllApiVersions used in NetworkOPs (#4833)

XRPLF · Mar 22, 2024 · 6edf03c · 6edf03c
1 parent 47c8cc2
commit 6edf03c
Show file tree

Hide file tree

Showing 26 changed files with 1,724 additions and 689 deletions.
diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake
@@ -218,7 +218,6 @@ install (
 install (
   FILES
     src/ripple/json/JsonPropertyStream.h
-    src/ripple/json/MultivarJson.h
     src/ripple/json/Object.h
     src/ripple/json/Output.h
     src/ripple/json/Writer.h
@@ -237,6 +236,7 @@ install (
     src/ripple/protocol/AccountID.h
     src/ripple/protocol/AMMCore.h
     src/ripple/protocol/AmountConversions.h
+    src/ripple/protocol/ApiVersion.h
     src/ripple/protocol/Book.h
     src/ripple/protocol/BuildInfo.h
     src/ripple/protocol/ErrorCodes.h
@@ -252,6 +252,8 @@ install (
     src/ripple/protocol/KnownFormats.h
     src/ripple/protocol/LedgerFormats.h
     src/ripple/protocol/LedgerHeader.h
+    src/ripple/protocol/MultiApiJson.h
+    src/ripple/protocol/NFTSyntheticSerializer.h
     src/ripple/protocol/NFTokenID.h
     src/ripple/protocol/NFTokenOfferID.h
     src/ripple/protocol/NFTSyntheticSerializer.h
@@ -944,7 +946,6 @@ if (tests)
     src/test/json/Output_test.cpp
     src/test/json/Writer_test.cpp
     src/test/json/json_value_test.cpp
-    src/test/json/MultivarJson_test.cpp
     #[===============================[
        test sources:
          subdir: jtx
@@ -1042,11 +1043,13 @@ if (tests)
        test sources:
          subdir: protocol
     #]===============================]
+    src/test/protocol/ApiVersion_test.cpp
     src/test/protocol/BuildInfo_test.cpp
     src/test/protocol/InnerObjectFormats_test.cpp
     src/test/protocol/Issue_test.cpp
     src/test/protocol/Hooks_test.cpp
     src/test/protocol/Memo_test.cpp
+    src/test/protocol/MultiApiJson_test.cpp
     src/test/protocol/PublicKey_test.cpp
     src/test/protocol/Quality_test.cpp
     src/test/protocol/STAccount_test.cpp

diff --git a/Builds/levelization/results/ordering.txt b/Builds/levelization/results/ordering.txt
@@ -132,7 +132,6 @@ test.csf > ripple.json
 test.csf > ripple.protocol
 test.json > ripple.beast
 test.json > ripple.json
-test.json > ripple.rpc
 test.json > test.jtx
 test.jtx > ripple.app
 test.jtx > ripple.basics

diff --git a/src/ripple/app/ledger/BookListeners.cpp b/src/ripple/app/ledger/BookListeners.cpp
@@ -54,8 +54,9 @@ BookListeners::publish(
             // Only publish jvObj if this is the first occurence
             if (havePublished.emplace(p->getSeq()).second)
             {
-                p->send(
-                    jvObj.select(apiVersionSelector(p->getApiVersion())), true);
+                jvObj.visit(
+                    p->getApiVersion(),  //
+                    [&](Json::Value const& jv) { p->send(jv, true); });
             }
             ++it;
         }

diff --git a/src/ripple/app/ledger/BookListeners.h b/src/ripple/app/ledger/BookListeners.h
@@ -20,8 +20,8 @@
 #ifndef RIPPLE_APP_LEDGER_BOOKLISTENERS_H_INCLUDED
 #define RIPPLE_APP_LEDGER_BOOKLISTENERS_H_INCLUDED
 
-#include <ripple/json/MultivarJson.h>
 #include <ripple/net/InfoSub.h>
+#include <ripple/protocol/MultiApiJson.h>
 
 #include <memory>
 #include <mutex>

diff --git a/src/ripple/app/ledger/OrderBookDB.h b/src/ripple/app/ledger/OrderBookDB.h
@@ -23,7 +23,7 @@
 #include <ripple/app/ledger/AcceptedLedgerTx.h>
 #include <ripple/app/ledger/BookListeners.h>
 #include <ripple/app/main/Application.h>
-#include <ripple/json/MultivarJson.h>
+#include <ripple/protocol/MultiApiJson.h>
 
 #include <mutex>
 

diff --git a/src/ripple/app/ledger/impl/LedgerToJson.cpp b/src/ripple/app/ledger/impl/LedgerToJson.cpp
@@ -168,9 +168,7 @@ fillJsonTx(
         if (validated)
         {
             auto const seq = fill.ledger.seq();
-            txJson[jss::ledger_index] = (fill.context->apiVersion > 1)
-                ? Json::Value(seq)
-                : Json::Value(std::to_string(seq));
+            txJson[jss::ledger_index] = seq;
             if (fill.closeTime)
                 txJson[jss::close_time_iso] = to_string_iso(*fill.closeTime);
         }

diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp
@@ -52,14 +52,14 @@
 #include <ripple/consensus/ConsensusParms.h>
 #include <ripple/crypto/RFC1751.h>
 #include <ripple/crypto/csprng.h>
-#include <ripple/json/MultivarJson.h>
 #include <ripple/json/to_string.h>
 #include <ripple/nodestore/DatabaseShard.h>
 #include <ripple/overlay/Cluster.h>
 #include <ripple/overlay/Overlay.h>
 #include <ripple/overlay/predicates.h>
 #include <ripple/protocol/BuildInfo.h>
 #include <ripple/protocol/Feature.h>
+#include <ripple/protocol/MultiApiJson.h>
 #include <ripple/protocol/RPCErr.h>
 #include <ripple/protocol/STParsedJSON.h>
 #include <ripple/protocol/jss.h>
@@ -68,7 +68,6 @@
 #include <ripple/rpc/BookChanges.h>
 #include <ripple/rpc/DeliveredAmount.h>
 #include <ripple/rpc/ServerHandler.h>
-#include <ripple/rpc/impl/RPCHelpers.h>
 #include <boost/asio/ip/host_name.hpp>
 #include <boost/asio/steady_timer.hpp>
 
@@ -2213,11 +2212,11 @@ NetworkOPsImp::pubValidation(std::shared_ptr<STValidation> const& val)
         // NOTE Use MultiApiJson to publish two slightly different JSON objects
         // for consumers supporting different API versions
         MultiApiJson multiObj{jvObj};
-        visit<RPC::apiMinimumSupportedVersion, RPC::apiMaximumValidVersion>(
-            multiObj,  //
-            [](Json::Value& jvTx, unsigned int apiVersion) {
+        multiObj.visit(
+            RPC::apiVersion<1>,  //
+            [](Json::Value& jvTx) {
                 // Type conversion for older API versions to string
-                if (jvTx.isMember(jss::ledger_index) && apiVersion < 2)
+                if (jvTx.isMember(jss::ledger_index))
                 {
                     jvTx[jss::ledger_index] =
                         std::to_string(jvTx[jss::ledger_index].asUInt());
@@ -2229,9 +2228,9 @@ NetworkOPsImp::pubValidation(std::shared_ptr<STValidation> const& val)
         {
             if (auto p = i->second.lock())
             {
-                p->send(
-                    multiObj.select(apiVersionSelector(p->getApiVersion())),
-                    true);
+                multiObj.visit(
+                    p->getApiVersion(),  //
+                    [&](Json::Value const& jv) { p->send(jv, true); });
                 ++i;
             }
             else
@@ -2769,8 +2768,9 @@ NetworkOPsImp::pubProposedTransaction(
 
             if (p)
             {
-                p->send(
-                    jvObj.select(apiVersionSelector(p->getApiVersion())), true);
+                jvObj.visit(
+                    p->getApiVersion(),  //
+                    [&](Json::Value const& jv) { p->send(jv, true); });
                 ++it;
             }
             else
@@ -3167,13 +3167,14 @@ NetworkOPsImp::transJson(
 
     std::string const hash = to_string(transaction->getTransactionID());
     MultiApiJson multiObj{jvObj};
-    visit<RPC::apiMinimumSupportedVersion, RPC::apiMaximumValidVersion>(
-        multiObj,  //
-        [&](Json::Value& jvTx, unsigned int apiVersion) {
+    forAllApiVersions(
+        multiObj.visit(),  //
+        [&]<unsigned Version>(
+            Json::Value& jvTx, std::integral_constant<unsigned, Version>) {
             RPC::insertDeliverMax(
-                jvTx[jss::transaction], transaction->getTxnType(), apiVersion);
+                jvTx[jss::transaction], transaction->getTxnType(), Version);
 
-            if (apiVersion > 1)
+            if constexpr (Version > 1)
             {
                 jvTx[jss::tx_json] = jvTx.removeMember(jss::transaction);
                 jvTx[jss::hash] = hash;
@@ -3210,8 +3211,9 @@ NetworkOPsImp::pubValidatedTransaction(
 
             if (p)
             {
-                p->send(
-                    jvObj.select(apiVersionSelector(p->getApiVersion())), true);
+                jvObj.visit(
+                    p->getApiVersion(),  //
+                    [&](Json::Value const& jv) { p->send(jv, true); });
                 ++it;
             }
             else
@@ -3226,8 +3228,9 @@ NetworkOPsImp::pubValidatedTransaction(
 
             if (p)
             {
-                p->send(
-                    jvObj.select(apiVersionSelector(p->getApiVersion())), true);
+                jvObj.visit(
+                    p->getApiVersion(),  //
+                    [&](Json::Value const& jv) { p->send(jv, true); });
                 ++it;
             }
             else
@@ -3347,9 +3350,9 @@ NetworkOPsImp::pubAccountTransaction(
 
         for (InfoSub::ref isrListener : notify)
         {
-            isrListener->send(
-                jvObj.select(apiVersionSelector(isrListener->getApiVersion())),
-                true);
+            jvObj.visit(
+                isrListener->getApiVersion(),  //
+                [&](Json::Value const& jv) { isrListener->send(jv, true); });
         }
 
         if (last)
@@ -3366,9 +3369,9 @@ NetworkOPsImp::pubAccountTransaction(
 
             jvObj.set(jss::account_history_tx_index, index->forwardTxIndex_++);
 
-            info.sink_->send(
-                jvObj.select(apiVersionSelector(info.sink_->getApiVersion())),
-                true);
+            jvObj.visit(
+                info.sink_->getApiVersion(),  //
+                [&](Json::Value const& jv) { info.sink_->send(jv, true); });
         }
     }
 }
@@ -3426,9 +3429,9 @@ NetworkOPsImp::pubProposedAccountTransaction(
         MultiApiJson jvObj = transJson(tx, result, false, ledger, std::nullopt);
 
         for (InfoSub::ref isrListener : notify)
-            isrListener->send(
-                jvObj.select(apiVersionSelector(isrListener->getApiVersion())),
-                true);
+            jvObj.visit(
+                isrListener->getApiVersion(),  //
+                [&](Json::Value const& jv) { isrListener->send(jv, true); });
 
         assert(
             jvObj.isMember(jss::account_history_tx_stream) ==
@@ -3439,9 +3442,9 @@ NetworkOPsImp::pubProposedAccountTransaction(
             if (index->forwardTxIndex_ == 0 && !index->haveHistorical_)
                 jvObj.set(jss::account_history_tx_first, true);
             jvObj.set(jss::account_history_tx_index, index->forwardTxIndex_++);
-            info.sink_->send(
-                jvObj.select(apiVersionSelector(info.sink_->getApiVersion())),
-                true);
+            jvObj.visit(
+                info.sink_->getApiVersion(),  //
+                [&](Json::Value const& jv) { info.sink_->send(jv, true); });
         }
     }
 }
@@ -3647,9 +3650,9 @@ NetworkOPsImp::addAccountHistoryJob(SubAccountHistoryInfoWeak subInfo)
                                         bool unsubscribe) -> bool {
                 if (auto sptr = subInfo.sinkWptr_.lock())
                 {
-                    sptr->send(
-                        jvObj.select(apiVersionSelector(sptr->getApiVersion())),
-                        true);
+                    jvObj.visit(
+                        sptr->getApiVersion(),  //
+                        [&](Json::Value const& jv) { sptr->send(jv, true); });
 
                     if (unsubscribe)
                         unsubAccountHistory(sptr, accountId, false);