Write improved forAllApiVersions used in NetworkOPs (#4833)

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

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

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.visit(
-                    jvObj.select(apiVersionSelector(p->getApiVersion())), true);
+                    p->getApiVersion(),  //
+                    [&](Json::Value const& jv) { p->send(jv, true); });
             }
             ++it;
         }

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>

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>
 

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)
+            txJson[jss::ledger_index] = seq;
-                ? Json::Value(seq)
-                : Json::Value(std::to_string(seq));
             if (fill.closeTime)
                 txJson[jss::close_time_iso] = to_string_iso(*fill.closeTime);
         }

src/ripple/app/misc/NetworkOPs.cpp

@@ -52,7 +52,6 @@
 #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>
@@ -60,6 +59,7 @@
 #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.visit(
-            multiObj,  //
+            RPC::apiVersion<1>,  //
-            [](Json::Value& jvTx, unsigned int apiVersion) {
+            [](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.visit(
-                    multiObj.select(apiVersionSelector(p->getApiVersion())),
+                    p->getApiVersion(),  //
-                    true);
+                    [&](Json::Value const& jv) { p->send(jv, true); });
                 ++i;
             }
             else
@@ -2769,8 +2768,9 @@ NetworkOPsImp::pubProposedTransaction(
 
             if (p)
             {
-                p->send(
+                jvObj.visit(
-                    jvObj.select(apiVersionSelector(p->getApiVersion())), true);
+                    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>(
+    forAllApiVersions(
-        multiObj,  //
+        multiObj.visit(),  //
-        [&](Json::Value& jvTx, unsigned int apiVersion) {
+        [&]<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.visit(
-                    jvObj.select(apiVersionSelector(p->getApiVersion())), true);
+                    p->getApiVersion(),  //
+                    [&](Json::Value const& jv) { p->send(jv, true); });
                 ++it;
             }
             else
@@ -3226,8 +3228,9 @@ NetworkOPsImp::pubValidatedTransaction(
 
             if (p)
             {
-                p->send(
+                jvObj.visit(
-                    jvObj.select(apiVersionSelector(p->getApiVersion())), true);
+                    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.visit(
-                jvObj.select(apiVersionSelector(isrListener->getApiVersion())),
+                isrListener->getApiVersion(),  //
-                true);
+                [&](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.visit(
-                jvObj.select(apiVersionSelector(info.sink_->getApiVersion())),
+                info.sink_->getApiVersion(),  //
-                true);
+                [&](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.visit(
-                jvObj.select(apiVersionSelector(isrListener->getApiVersion())),
+                isrListener->getApiVersion(),  //
-                true);
+                [&](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.visit(
-                jvObj.select(apiVersionSelector(info.sink_->getApiVersion())),
+                info.sink_->getApiVersion(),  //
-                true);
+                [&](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.visit(
-                        jvObj.select(apiVersionSelector(sptr->getApiVersion())),
+                        sptr->getApiVersion(),  //
-                        true);
+                        [&](Json::Value const& jv) { sptr->send(jv, true); });
 
                     if (unsubscribe)
                         unsubAccountHistory(sptr, accountId, false);

src/ripple/json/MultivarJson.h

@@ -1,150 +0,0 @@
-//------------------------------------------------------------------------------
-/*
-    This file is part of rippled: https://github.com/ripple/rippled
-    Copyright (c) 2023 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_JSON_MULTIVARJSON_H_INCLUDED
-#define RIPPLE_JSON_MULTIVARJSON_H_INCLUDED
-
-#include <ripple/json/json_value.h>
-
-#include <array>
-#include <cassert>
-#include <concepts>
-#include <cstdlib>
-#include <type_traits>
-#include <utility>
-
-namespace ripple {
-template <std::size_t Size>
-struct MultivarJson
-{
-    std::array<Json::Value, Size> val = {};
-    constexpr static std::size_t size = Size;
-
-    explicit MultivarJson(Json::Value const& init = {})
-    {
-        if (init == Json::Value{})
-            return;  // All elements are already default-initialized
-        for (auto& v : val)
-            v = init;
-    }
-
-    Json::Value const&
-    select(auto&& selector) const
-        requires std::same_as<std::size_t, decltype(selector())>
-    {
-        auto const index = selector();
-        assert(index < size);
-        return val[index];
-    }
-
-    void
-    set(const char* key,
-        auto const&
-            v) requires std::constructible_from<Json::Value, decltype(v)>
-    {
-        for (auto& a : this->val)
-            a[key] = v;
-    }
-
-    // Intentionally not using class enum here, MultivarJson is scope enough
-    enum IsMemberResult : int { none = 0, some, all };
-
-    [[nodiscard]] IsMemberResult
-    isMember(const char* key) const
-    {
-        int count = 0;
-        for (auto& a : this->val)
-            if (a.isMember(key))
-                count += 1;
-
-        return (count == 0 ? none : (count < size ? some : all));
-    }
-};
-
-// Wrapper for Json for all supported API versions.
-using MultiApiJson = MultivarJson<3>;
-
-/*
-
-NOTE:
-
-If a future API version change adds another possible format, change the size of
-`MultiApiJson`, and update `apiVersionSelector()` to return the appropriate
-selection value for the new `apiVersion` and higher.
-
-The more different JSON formats we support, the more CPU cycles we need to
-prepare JSON for different API versions e.g. when publishing streams to
-`subscribe` clients. Hence it is desirable to keep MultiApiJson small and
-instead fully deprecate and remove support for old API versions. For example, if
-we removed support for API version 1 and added a different format for API
-version 3, the `apiVersionSelector` would change to
-`static_cast<std::size_t>(apiVersion > 2)`
-
-Such hypothetical change should correspond with change in RPCHelpers.h
-`apiMinimumSupportedVersion = 2;`
-
-*/
-
-// Helper to create appropriate selector for indexing MultiApiJson by apiVersion
-constexpr auto
-apiVersionSelector(unsigned int apiVersion) noexcept
-{
-    return [apiVersion]() constexpr
-    {
-        return static_cast<std::size_t>(
-            apiVersion <= 1         //
-                ? 0                 //
-                : (apiVersion <= 2  //
-                       ? 1          //
-                       : 2));
-    };
-}
-
-// Helper to execute a callback for every version. Want both min and max version
-// provided explicitly, so user will know to do update `size` when they change
-template <
-    unsigned int minVer,
-    unsigned int maxVer,
-    std::size_t size,
-    typename Fn>
-    requires                                       //
-    (maxVer >= minVer) &&                          //
-    (size == maxVer + 1 - minVer) &&               //
-    (apiVersionSelector(minVer)() == 0) &&         //
-    (apiVersionSelector(maxVer)() + 1 == size) &&  //
-    requires(Json::Value& json, Fn fn)
-{
-    fn(json, static_cast<unsigned int>(1));
-}
-void
-visit(MultivarJson<size>& json, Fn fn)
-{
-    [&]<std::size_t... offset>(std::index_sequence<offset...>)
-    {
-        static_assert(((apiVersionSelector(minVer + offset)() >= 0) && ...));
-        static_assert(((apiVersionSelector(minVer + offset)() < size) && ...));
-        (fn(json.val[apiVersionSelector(minVer + offset)()], minVer + offset),
-         ...);
-    }
-    (std::make_index_sequence<size>{});
-}
-
-}  // namespace ripple
-
-#endif

src/ripple/protocol/ApiVersion.h

@@ -0,0 +1,119 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/ripple/rippled
+    Copyright (c) 2023 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_PROTOCOL_APIVERSION_H_INCLUDED
+#define RIPPLE_PROTOCOL_APIVERSION_H_INCLUDED
+
+#include <functional>
+#include <type_traits>
+#include <utility>
+
+namespace ripple {
+
+/**
+ * API version numbers used in later API versions
+ *
+ * Requests with a version number in the range
+ * [apiMinimumSupportedVersion, apiMaximumSupportedVersion]
+ * are supported.
+ *
+ * If [beta_rpc_api] is enabled in config, the version numbers
+ * in the range [apiMinimumSupportedVersion, apiBetaVersion]
+ * are supported.
+ *
+ * Network Requests without explicit version numbers use
+ * apiVersionIfUnspecified. apiVersionIfUnspecified is 1,
+ * because all the RPC requests with a version >= 2 must
+ * explicitly specify the version in the requests.
+ * Note that apiVersionIfUnspecified will be lower than
+ * apiMinimumSupportedVersion when we stop supporting API
+ * version 1.
+ *
+ * Command line Requests use apiCommandLineVersion.
+ */
+
+namespace RPC {
+
+template <unsigned int Version>
+constexpr static std::integral_constant<unsigned, Version> apiVersion = {};
+
+constexpr static auto apiInvalidVersion = apiVersion<0>;
+constexpr static auto apiMinimumSupportedVersion = apiVersion<1>;
+constexpr static auto apiMaximumSupportedVersion = apiVersion<2>;
+constexpr static auto apiVersionIfUnspecified = apiVersion<1>;
+constexpr static auto apiCommandLineVersion =
+    apiVersion<1>;  // TODO Bump to 2 later
+constexpr static auto apiBetaVersion = apiVersion<3>;
+constexpr static auto apiMaximumValidVersion = apiBetaVersion;
+
+static_assert(apiInvalidVersion < apiMinimumSupportedVersion);
+static_assert(
+    apiVersionIfUnspecified >= apiMinimumSupportedVersion &&
+    apiVersionIfUnspecified <= apiMaximumSupportedVersion);
+static_assert(
+    apiCommandLineVersion >= apiMinimumSupportedVersion &&
+    apiCommandLineVersion <= apiMaximumSupportedVersion);
+static_assert(apiMaximumSupportedVersion >= apiMinimumSupportedVersion);
+static_assert(apiBetaVersion >= apiMaximumSupportedVersion);
+static_assert(apiMaximumValidVersion >= apiMaximumSupportedVersion);
+
+}  // namespace RPC
+
+template <unsigned minVer, unsigned maxVer, typename Fn, typename... Args>
+    void
+    forApiVersions(Fn const& fn, Args&&... args) requires  //
+    (maxVer >= minVer) &&                                  //
+    (minVer >= RPC::apiMinimumSupportedVersion) &&         //
+    (RPC::apiMaximumValidVersion >= maxVer) &&
+    requires
+{
+    fn(std::integral_constant<unsigned int, minVer>{},
+       std::forward<Args>(args)...);
+    fn(std::integral_constant<unsigned int, maxVer>{},
+       std::forward<Args>(args)...);
+}
+{
+    constexpr auto size = maxVer + 1 - minVer;
+    [&]<std::size_t... offset>(std::index_sequence<offset...>)
+    {
+        (((void)fn(
+             std::integral_constant<unsigned int, minVer + offset>{},
+             std::forward<Args>(args)...)),
+         ...);
+    }
+    (std::make_index_sequence<size>{});
+}
+
+template <typename Fn, typename... Args>
+void
+forAllApiVersions(Fn const& fn, Args&&... args) requires requires
+{
+    forApiVersions<
+        RPC::apiMinimumSupportedVersion,
+        RPC::apiMaximumValidVersion>(fn, std::forward<Args>(args)...);
+}
+{
+    forApiVersions<
+        RPC::apiMinimumSupportedVersion,
+        RPC::apiMaximumValidVersion>(fn, std::forward<Args>(args)...);
+}
+
+}  // namespace ripple
+
+#endif

src/ripple/protocol/MultiApiJson.h

@@ -0,0 +1,247 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/ripple/rippled
+    Copyright (c) 2023 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_JSON_MULTIAPIJSON_H_INCLUDED
+#define RIPPLE_JSON_MULTIAPIJSON_H_INCLUDED
+
+#include <ripple/json/json_value.h>
+#include <ripple/protocol/ApiVersion.h>
+
+#include <array>
+#include <cassert>
+#include <concepts>
+#include <cstdlib>
+#include <functional>
+#include <limits>
+#include <type_traits>
+#include <utility>
+
+namespace ripple {
+
+namespace detail {
+template <typename T>
+constexpr bool is_integral_constant = false;
+template <typename I, auto A>
+constexpr bool is_integral_constant<std::integral_constant<I, A>&> = true;
+template <typename I, auto A>
+constexpr bool is_integral_constant<std::integral_constant<I, A> const&> = true;
+
+template <typename T>
+concept some_integral_constant = detail::is_integral_constant<T&>;
+
+// This class is designed to wrap a collection of _almost_ identical Json::Value
+// objects, indexed by version (i.e. there is some mapping of version to object
+// index). It is used e.g. when we need to publish JSON data to users supporting
+// different API versions. We allow manipulation and inspection of all objects
+// at once with `isMember` and `set`, and also individual inspection and updates
+// of an object selected by the user by version, using `visitor_t` nested type.
+template <unsigned MinVer, unsigned MaxVer>
+struct MultiApiJson
+{
+    static_assert(MinVer <= MaxVer);
+
+    static constexpr auto
+    valid(unsigned int v) noexcept -> bool
+    {
+        return v >= MinVer && v <= MaxVer;
+    }
+
+    static constexpr auto
+    index(unsigned int v) noexcept -> std::size_t
+    {
+        return (v < MinVer) ? 0 : static_cast<std::size_t>(v - MinVer);
+    }
+
+    constexpr static std::size_t size = MaxVer + 1 - MinVer;
+    std::array<Json::Value, size> val = {};
+
+    explicit MultiApiJson(Json::Value const& init = {})
+    {
+        if (init == Json::Value{})
+            return;  // All elements are already default-initialized
+        for (auto& v : val)
+            v = init;
+    }
+
+    void
+    set(const char* key,
+        auto const&
+            v) requires std::constructible_from<Json::Value, decltype(v)>
+    {
+        for (auto& a : this->val)
+            a[key] = v;
+    }
+
+    // Intentionally not using class enum here, MultivarJson is scope enough
+    enum IsMemberResult : int { none = 0, some, all };
+
+    [[nodiscard]] IsMemberResult
+    isMember(const char* key) const
+    {
+        int count = 0;
+        for (auto& a : this->val)
+            if (a.isMember(key))
+                count += 1;
+
+        return (count == 0 ? none : (count < size ? some : all));
+    }
+
+    static constexpr struct visitor_t final
+    {
+        // integral_constant version, extra arguments
+        template <
+            typename Json,
+            unsigned int Version,
+            typename... Args,
+            typename Fn>
+        requires std::same_as<std::remove_cvref_t<Json>, MultiApiJson> auto
+        operator()(
+            Json& json,
+            std::integral_constant<unsigned int, Version> const version,
+            Fn fn,
+            Args&&... args) const
+            -> std::invoke_result_t<
+                Fn,
+                decltype(json.val[0]),
+                std::integral_constant<unsigned int, Version>,
+                Args&&...>
+        {
+            static_assert(
+                valid(Version) && index(Version) >= 0 && index(Version) < size);
+            return std::invoke(
+                fn,
+                json.val[index(Version)],
+                version,
+                std::forward<Args>(args)...);
+        }
+
+        // integral_constant version, Json only
+        template <typename Json, unsigned int Version, typename Fn>
+        requires std::same_as<std::remove_cvref_t<Json>, MultiApiJson> auto
+        operator()(
+            Json& json,
+            std::integral_constant<unsigned int, Version> const,
+            Fn fn) const -> std::invoke_result_t<Fn, decltype(json.val[0])>
+        {
+            static_assert(
+                valid(Version) && index(Version) >= 0 && index(Version) < size);
+            return std::invoke(fn, json.val[index(Version)]);
+        }
+
+        // unsigned int version, extra arguments
+        template <
+            typename Json,
+            typename Version,
+            typename... Args,
+            typename Fn>
+            requires(!some_integral_constant<Version>) &&
+            std::convertible_to<Version, unsigned>&& std::same_as<
+                std::remove_cvref_t<Json>,
+                MultiApiJson> auto
+            operator()(Json& json, Version version, Fn fn, Args&&... args) const
+            -> std::
+                invoke_result_t<Fn, decltype(json.val[0]), Version, Args&&...>
+        {
+            assert(
+                valid(version) && index(version) >= 0 && index(version) < size);
+            return std::invoke(
+                fn,
+                json.val[index(version)],
+                version,
+                std::forward<Args>(args)...);
+        }
+
+        // unsigned int version, Json only
+        template <typename Json, typename Version, typename Fn>
+            requires(!some_integral_constant<Version>) &&
+            std::convertible_to<Version, unsigned>&& std::
+                same_as<std::remove_cvref_t<Json>, MultiApiJson> auto
+                operator()(Json& json, Version version, Fn fn) const
+            -> std::invoke_result_t<Fn, decltype(json.val[0])>
+        {
+            assert(
+                valid(version) && index(version) >= 0 && index(version) < size);
+            return std::invoke(fn, json.val[index(version)]);
+        }
+    } visitor = {};
+
+    auto
+    visit()
+    {
+        return [self = this](auto... args) requires requires
+        {
+            visitor(
+                std::declval<MultiApiJson&>(),
+                std::declval<decltype(args)>()...);
+        }
+        {
+            return visitor(*self, std::forward<decltype(args)>(args)...);
+        };
+    }
+
+    auto
+    visit() const
+    {
+        return [self = this](auto... args) requires requires
+        {
+            visitor(
+                std::declval<MultiApiJson const&>(),
+                std::declval<decltype(args)>()...);
+        }
+        {
+            return visitor(*self, std::forward<decltype(args)>(args)...);
+        };
+    }
+
+    template <typename... Args>
+        auto
+        visit(Args... args)
+            -> std::invoke_result_t<visitor_t, MultiApiJson&, Args...> requires(
+                sizeof...(args) > 0) &&
+        requires
+    {
+        visitor(*this, std::forward<decltype(args)>(args)...);
+    }
+    {
+        return visitor(*this, std::forward<decltype(args)>(args)...);
+    }
+
+    template <typename... Args>
+        auto
+        visit(Args... args) const -> std::
+            invoke_result_t<visitor_t, MultiApiJson const&, Args...> requires(
+                sizeof...(args) > 0) &&
+        requires
+    {
+        visitor(*this, std::forward<decltype(args)>(args)...);
+    }
+    {
+        return visitor(*this, std::forward<decltype(args)>(args)...);
+    }
+};
+
+}  // namespace detail
+
+// Wrapper for Json for all supported API versions.
+using MultiApiJson = detail::
+    MultiApiJson<RPC::apiMinimumSupportedVersion, RPC::apiMaximumValidVersion>;
+
+}  // namespace ripple
+
+#endif

src/ripple/rpc/impl/RPCHelpers.h

@@ -22,6 +22,7 @@
 
 #include <ripple/beast/core/SemanticVersion.h>
 #include <ripple/proto/org/xrpl/rpc/v1/xrp_ledger.pb.h>
+#include <ripple/protocol/ApiVersion.h>
 #include <ripple/protocol/TxMeta.h>
 
 #include <ripple/app/misc/NetworkOPs.h>
@@ -30,6 +31,7 @@
 #include <ripple/rpc/Context.h>
 #include <ripple/rpc/Status.h>
 #include <ripple/rpc/impl/Tuning.h>
+
 #include <optional>
 #include <variant>
 
@@ -207,41 +209,6 @@ extern beast::SemanticVersion const firstVersion;
 extern beast::SemanticVersion const goodVersion;
 extern beast::SemanticVersion const lastVersion;
 
-/**
- * API version numbers used in later API versions
- *
- * Requests with a version number in the range
- * [apiMinimumSupportedVersion, apiMaximumSupportedVersion]
- * are supported.
- *
- * If [beta_rpc_api] is enabled in config, the version numbers
- * in the range [apiMinimumSupportedVersion, apiBetaVersion]
- * are supported.
- *
- * Network Requests without explicit version numbers use
- * apiVersionIfUnspecified. apiVersionIfUnspecified is 1,
- * because all the RPC requests with a version >= 2 must
- * explicitly specify the version in the requests.
- * Note that apiVersionIfUnspecified will be lower than
- * apiMinimumSupportedVersion when we stop supporting API
- * version 1.
- *
- * Command line Requests use apiMaximumSupportedVersion.
- */
-
-constexpr unsigned int apiInvalidVersion = 0;
-constexpr unsigned int apiVersionIfUnspecified = 1;
-constexpr unsigned int apiMinimumSupportedVersion = 1;
-constexpr unsigned int apiMaximumSupportedVersion = 2;
-constexpr unsigned int apiCommandLineVersion = 1;  // TODO Bump to 2 later
-constexpr unsigned int apiBetaVersion = 3;
-constexpr unsigned int apiMaximumValidVersion = apiBetaVersion;
-
-static_assert(apiMinimumSupportedVersion >= apiVersionIfUnspecified);
-static_assert(apiMaximumSupportedVersion >= apiMinimumSupportedVersion);
-static_assert(apiBetaVersion >= apiMaximumSupportedVersion);
-static_assert(apiMaximumValidVersion >= apiMaximumSupportedVersion);
-
 template <class Object>
 void
 setVersion(Object& parent, unsigned int apiVersion, bool betaEnabled)
@@ -256,7 +223,7 @@ setVersion(Object& parent, unsigned int apiVersion, bool betaEnabled)
     }
     else
     {
-        object[jss::first] = apiMinimumSupportedVersion;
+        object[jss::first] = apiMinimumSupportedVersion.value;
         object[jss::last] =
             betaEnabled ? apiBetaVersion : apiMaximumSupportedVersion;
     }

src/ripple/rpc/impl/ServerHandler.cpp

@@ -643,7 +643,7 @@ ServerHandler::processRequest(
             continue;
         }
 
-        auto apiVersion = RPC::apiVersionIfUnspecified;
+        unsigned apiVersion = RPC::apiVersionIfUnspecified;
         if (jsonRPC.isMember(jss::params) && jsonRPC[jss::params].isArray() &&
             jsonRPC[jss::params].size() > 0 &&
             jsonRPC[jss::params][0u].isObject())

src/test/app/PayChan_test.cpp

@@ -1088,10 +1088,7 @@ struct PayChan_test : public beast::unit_test::suite
         args[jss::amount] = 51110000;
 
         // test for all api versions
-        for (auto apiVersion = RPC::apiMinimumSupportedVersion;
+        forAllApiVersions([&, this](unsigned apiVersion) {
-             apiVersion <= RPC::apiBetaVersion;
-             ++apiVersion)
-        {
             testcase(
                 "PayChan Channel_Auth RPC Api " + std::to_string(apiVersion));
             args[jss::api_version] = apiVersion;
@@ -1101,7 +1098,7 @@ struct PayChan_test : public beast::unit_test::suite
                 args.toStyledString())[jss::result];
             auto const error = apiVersion < 2u ? "invalidParams" : "badKeyType";
             BEAST_EXPECT(rs[jss::error] == error);
-        }
+        });
     }
 
     void

src/test/json/MultivarJson_test.cpp

@@ -1,387 +0,0 @@
-//------------------------------------------------------------------------------
-/*
-    This file is part of rippled: https://github.com/XRPLF/rippled/
-    Copyright (c) 2023 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.
-*/
-//==============================================================================
-
-#include <ripple/json/MultivarJson.h>
-#include <ripple/rpc/impl/RPCHelpers.h>
-
-#include <ripple/beast/unit_test.h>
-#include "ripple/beast/unit_test/suite.hpp"
-#include "ripple/json/json_value.h"
-#include <cstdint>
-#include <limits>
-#include <optional>
-#include <type_traits>
-
-namespace ripple {
-namespace test {
-
-struct MultivarJson_test : beast::unit_test::suite
-{
-    static auto
-    makeJson(const char* key, int val)
-    {
-        Json::Value obj1(Json::objectValue);
-        obj1[key] = val;
-        return obj1;
-    };
-
-    void
-    run() override
-    {
-        Json::Value const obj1 = makeJson("value", 1);
-        Json::Value const obj2 = makeJson("value", 2);
-        Json::Value const obj3 = makeJson("value", 3);
-        Json::Value const jsonNull{};
-
-        MultivarJson<3> subject{};
-        static_assert(sizeof(subject) == sizeof(subject.val));
-        static_assert(subject.size == subject.val.size());
-        static_assert(
-            std::is_same_v<decltype(subject.val), std::array<Json::Value, 3>>);
-
-        BEAST_EXPECT(subject.val.size() == 3);
-        BEAST_EXPECT(
-            (subject.val ==
-             std::array<Json::Value, 3>{jsonNull, jsonNull, jsonNull}));
-
-        subject.val[0] = obj1;
-        subject.val[1] = obj2;
-
-        {
-            testcase("default copy construction / assignment");
-
-            MultivarJson<3> x{subject};
-
-            BEAST_EXPECT(x.val.size() == subject.val.size());
-            BEAST_EXPECT(x.val[0] == subject.val[0]);
-            BEAST_EXPECT(x.val[1] == subject.val[1]);
-            BEAST_EXPECT(x.val[2] == subject.val[2]);
-            BEAST_EXPECT(x.val == subject.val);
-            BEAST_EXPECT(&x.val[0] != &subject.val[0]);
-            BEAST_EXPECT(&x.val[1] != &subject.val[1]);
-            BEAST_EXPECT(&x.val[2] != &subject.val[2]);
-
-            MultivarJson<3> y;
-            BEAST_EXPECT((y.val == std::array<Json::Value, 3>{}));
-            y = subject;
-            BEAST_EXPECT(y.val == subject.val);
-            BEAST_EXPECT(&y.val[0] != &subject.val[0]);
-            BEAST_EXPECT(&y.val[1] != &subject.val[1]);
-            BEAST_EXPECT(&y.val[2] != &subject.val[2]);
-
-            y = std::move(x);
-            BEAST_EXPECT(y.val == subject.val);
-            BEAST_EXPECT(&y.val[0] != &subject.val[0]);
-            BEAST_EXPECT(&y.val[1] != &subject.val[1]);
-            BEAST_EXPECT(&y.val[2] != &subject.val[2]);
-        }
-
-        {
-            testcase("select");
-
-            BEAST_EXPECT(
-                subject.select([]() -> std::size_t { return 0; }) == obj1);
-            BEAST_EXPECT(
-                subject.select([]() -> std::size_t { return 1; }) == obj2);
-            BEAST_EXPECT(
-                subject.select([]() -> std::size_t { return 2; }) == jsonNull);
-
-            // Tests of requires clause - these are expected to match
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    v.select([]() -> std::size_t { return 0; });
-                };
-            }(subject));
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    v.select([]() constexpr->std::size_t { return 0; });
-                };
-            }(subject));
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    v.select([]() mutable -> std::size_t { return 0; });
-                };
-            }(subject));
-
-            // Tests of requires clause - these are expected NOT to match
-            static_assert([](auto&& a) {
-                return !requires
-                {
-                    subject.select([]() -> int { return 0; });
-                };
-            }(subject));
-            static_assert([](auto&& v) {
-                return !requires
-                {
-                    v.select([]() -> void {});
-                };
-            }(subject));
-            static_assert([](auto&& v) {
-                return !requires
-                {
-                    v.select([]() -> bool { return false; });
-                };
-            }(subject));
-        }
-
-        {
-            testcase("set");
-
-            auto x = MultivarJson<2>{Json::objectValue};
-            x.set("name1", 42);
-            BEAST_EXPECT(x.val[0].isMember("name1"));
-            BEAST_EXPECT(x.val[1].isMember("name1"));
-            BEAST_EXPECT(x.val[0]["name1"].isInt());
-            BEAST_EXPECT(x.val[1]["name1"].isInt());
-            BEAST_EXPECT(x.val[0]["name1"].asInt() == 42);
-            BEAST_EXPECT(x.val[1]["name1"].asInt() == 42);
-
-            x.set("name2", "bar");
-            BEAST_EXPECT(x.val[0].isMember("name2"));
-            BEAST_EXPECT(x.val[1].isMember("name2"));
-            BEAST_EXPECT(x.val[0]["name2"].isString());
-            BEAST_EXPECT(x.val[1]["name2"].isString());
-            BEAST_EXPECT(x.val[0]["name2"].asString() == "bar");
-            BEAST_EXPECT(x.val[1]["name2"].asString() == "bar");
-
-            // Tests of requires clause - these are expected to match
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    v.set("name", Json::nullValue);
-                };
-            }(x));
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    v.set("name", "value");
-                };
-            }(x));
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    v.set("name", true);
-                };
-            }(x));
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    v.set("name", 42);
-                };
-            }(x));
-
-            // Tests of requires clause - these are expected NOT to match
-            struct foo_t final
-            {
-            };
-            static_assert([](auto&& v) {
-                return !requires
-                {
-                    v.set("name", foo_t{});
-                };
-            }(x));
-            static_assert([](auto&& v) {
-                return !requires
-                {
-                    v.set("name", std::nullopt);
-                };
-            }(x));
-        }
-
-        {
-            testcase("isMember");
-
-            // Well defined behaviour even if we have different types of members
-            BEAST_EXPECT(subject.isMember("foo") == decltype(subject)::none);
-
-            {
-                // All variants have element "One", none have element "Two"
-                MultivarJson<2> s1{};
-                s1.val[0] = makeJson("One", 12);
-                s1.val[1] = makeJson("One", 42);
-                BEAST_EXPECT(s1.isMember("One") == decltype(s1)::all);
-                BEAST_EXPECT(s1.isMember("Two") == decltype(s1)::none);
-            }
-
-            {
-                // Some variants have element "One" and some have "Two"
-                MultivarJson<2> s2{};
-                s2.val[0] = makeJson("One", 12);
-                s2.val[1] = makeJson("Two", 42);
-                BEAST_EXPECT(s2.isMember("One") == decltype(s2)::some);
-                BEAST_EXPECT(s2.isMember("Two") == decltype(s2)::some);
-            }
-
-            {
-                // Not all variants have element "One", because last one is null
-                MultivarJson<3> s3{};
-                s3.val[0] = makeJson("One", 12);
-                s3.val[1] = makeJson("One", 42);
-                BEAST_EXPECT(s3.isMember("One") == decltype(s3)::some);
-                BEAST_EXPECT(s3.isMember("Two") == decltype(s3)::none);
-            }
-        }
-
-        {
-            // NOTE It's fine to change this test when we change API versions
-            testcase("apiVersionSelector");
-
-            static_assert(MultiApiJson::size == 3);
-            static MultiApiJson x{obj1};
-            x.val[1] = obj2;
-            x.val[2] = obj3;
-
-            static_assert(
-                std::is_same_v<decltype(apiVersionSelector(1)()), std::size_t>);
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    v.select(apiVersionSelector(1));
-                };
-            }(x));
-
-            BEAST_EXPECT(x.select(apiVersionSelector(0)) == obj1);
-            BEAST_EXPECT(x.select(apiVersionSelector(2)) == obj2);
-
-            static_assert(apiVersionSelector(0)() == 0);
-            static_assert(apiVersionSelector(1)() == 0);
-            static_assert(apiVersionSelector(2)() == 1);
-            static_assert(apiVersionSelector(3)() == 2);
-            static_assert(apiVersionSelector(4)() == 2);
-            static_assert(
-                apiVersionSelector(
-                    std::numeric_limits<unsigned int>::max())() == 2);
-        }
-
-        {
-            // There should be no reson to change this test
-            testcase("apiVersionSelector invariants");
-
-            static_assert(
-                apiVersionSelector(RPC::apiMinimumSupportedVersion)() == 0);
-            static_assert(
-                apiVersionSelector(RPC::apiBetaVersion)() + 1  //
-                == MultiApiJson::size);
-
-            BEAST_EXPECT(MultiApiJson::size >= 1);
-        }
-
-        {
-            testcase("visit");
-
-            MultivarJson<3> s1{};
-            s1.val[0] = makeJson("value", 2);
-            s1.val[1] = makeJson("value", 3);
-            s1.val[2] = makeJson("value", 5);
-
-            int result = 1;
-            ripple::visit<1, 3>(
-                s1, [&](Json::Value& json, unsigned int i) -> void {
-                    if (BEAST_EXPECT(json.isObject() && json.isMember("value")))
-                    {
-                        auto const value = json["value"].asInt();
-                        BEAST_EXPECT(
-                            (value == 2 && i == 1) ||  //
-                            (value == 3 && i == 2) ||  //
-                            (value == 5 && i == 3));
-                        result *= value;
-                    }
-                });
-            BEAST_EXPECT(result == 30);
-
-            // Can use fn with constexpr functor
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    ripple::visit<1, 3>(
-                        v, [](Json::Value&, unsigned int) constexpr {});
-                };
-            }(s1));
-
-            // Can use fn with deduction over all parameters
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    ripple::visit<1, 3>(v, [](auto&, auto) constexpr {});
-                };
-            }(s1));
-
-            // Can use fn with conversion of version parameter
-            static_assert([](auto&& v) {
-                return requires
-                {
-                    ripple::visit<1, 3>(v, [](auto&, std::size_t) constexpr {});
-                };
-            }(s1));
-
-            // Cannot use fn with const parameter
-            static_assert([](auto&& v) {
-                return !requires
-                {
-                    ripple::visit<1, 3>(
-                        v, [](Json::Value const&, auto) constexpr {});
-                };
-            }(const_cast<MultivarJson<3> const&>(s1)));
-
-            // Cannot call visit with size mismatch
-            static_assert([](auto&& v) {
-                return !requires
-                {
-                    ripple::visit<1, 2>(
-                        v, [](Json::Value&, unsigned int) constexpr {});
-                };
-            }(s1));
-
-            // Cannot call visit with version offset
-            static_assert([](auto&& v) {
-                return !requires
-                {
-                    ripple::visit<0, 2>(
-                        v, [](Json::Value&, unsigned int) constexpr {});
-                };
-            }(s1));
-
-            // Cannot call visit with size mismatch
-            static_assert([](auto&& v) {
-                return !requires
-                {
-                    ripple::visit<1, 4>(
-                        v, [](Json::Value&, unsigned int) constexpr {});
-                };
-            }(s1));
-
-            // Cannot call visit with wrong order of versions
-            static_assert([](auto&& v) {
-                return !requires
-                {
-                    ripple::visit<3, 1>(
-                        v, [](Json::Value&, unsigned int) constexpr {});
-                };
-            }(s1));
-        }
-    }
-};
-
-BEAST_DEFINE_TESTSUITE(MultivarJson, ripple_basics, ripple);
-
-}  // namespace test
-}  // namespace ripple

src/test/jtx/Env.h

@@ -758,40 +758,6 @@ Env::rpc(std::string const& cmd, Args&&... args)
         std::forward<Args>(args)...);
 }
 
-/**
- * The SingleVersionedTestCallable concept checks for a callable that takes
- * an unsigned integer as its argument and returns void.
- */
-template <class T>
-concept SingleVersionedTestCallable = requires(T callable, unsigned int version)
-{
-    {
-        callable(version)
-    }
-    ->std::same_as<void>;
-};
-
-/**
- * The VersionedTestCallable concept checks if a set of callables all satisfy
- * the SingleVersionedTestCallable concept. This allows forAllApiVersions to
- * accept any number of functions. It executes a set of provided functions over
- * a range of versions from RPC::apiMinimumSupportedVersion to
- * RPC::apiBetaVersion. This is useful for running a series of tests or
- * operations that need to be performed on multiple versions of an API.
- */
-template <class... T>
-concept VersionedTestCallable = (... && SingleVersionedTestCallable<T>);
-void
-forAllApiVersions(VersionedTestCallable auto... testCallable)
-{
-    for (auto testVersion = RPC::apiMinimumSupportedVersion;
-         testVersion <= RPC::apiMaximumValidVersion;
-         ++testVersion)
-    {
-        (..., testCallable(testVersion));
-    }
-}
-
 }  // namespace jtx
 }  // namespace test
 }  // namespace ripple

src/test/protocol/ApiVersion_test.cpp

@@ -0,0 +1,75 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/XRPLF/rippled/
+    Copyright (c) 2023 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.
+*/
+//==============================================================================
+
+#include <ripple/beast/unit_test.h>
+#include <ripple/beast/unit_test/suite.hpp>
+#include <ripple/json/json_value.h>
+#include <ripple/protocol/ApiVersion.h>
+
+#include <array>
+#include <cstdint>
+#include <limits>
+#include <optional>
+#include <type_traits>
+#include <utility>
+
+namespace ripple {
+namespace test {
+struct ApiVersion_test : beast::unit_test::suite
+{
+    void
+    run() override
+    {
+        {
+            testcase("API versions invariants");
+
+            static_assert(
+                RPC::apiMinimumSupportedVersion <=
+                RPC::apiMaximumSupportedVersion);
+            static_assert(
+                RPC::apiMinimumSupportedVersion <= RPC::apiMaximumValidVersion);
+            static_assert(
+                RPC::apiMaximumSupportedVersion <= RPC::apiMaximumValidVersion);
+            static_assert(RPC::apiBetaVersion <= RPC::apiMaximumValidVersion);
+
+            BEAST_EXPECT(true);
+        }
+
+        {
+            // Update when we change versions
+            testcase("API versions");
+
+            static_assert(RPC::apiMinimumSupportedVersion >= 1);
+            static_assert(RPC::apiMinimumSupportedVersion < 2);
+            static_assert(RPC::apiMaximumSupportedVersion >= 2);
+            static_assert(RPC::apiMaximumSupportedVersion < 3);
+            static_assert(RPC::apiMaximumValidVersion >= 3);
+            static_assert(RPC::apiMaximumValidVersion < 4);
+            static_assert(RPC::apiBetaVersion >= 3);
+            static_assert(RPC::apiBetaVersion < 4);
+
+            BEAST_EXPECT(true);
+        }
+    }
+};
+
+BEAST_DEFINE_TESTSUITE(ApiVersion, protocol, ripple);
+
+}  // namespace test
+}  // namespace ripple

src/test/rpc/AccountTx_test.cpp

@@ -734,7 +734,7 @@ public:
     void
     run() override
     {
-        test::jtx::forAllApiVersions(
+        forAllApiVersions(
             std::bind_front(&AccountTx_test::testParameters, this));
         testContents();
         testAccountDelete();

src/test/rpc/GatewayBalances_test.cpp

@@ -172,10 +172,7 @@ public:
         qry2[jss::account] = alice.human();
         qry2[jss::hotwallet] = "asdf";
 
-        for (auto apiVersion = RPC::apiMinimumSupportedVersion;
+        forAllApiVersions([&, this](unsigned apiVersion) {
-             apiVersion <= RPC::apiBetaVersion;
-             ++apiVersion)
-        {
             qry2[jss::api_version] = apiVersion;
             auto jv = wsc->invoke("gateway_balances", qry2);
             expect(jv[jss::status] == "error");
@@ -184,7 +181,7 @@ public:
             auto const error =
                 apiVersion < 2u ? "invalidHotWallet" : "invalidParams";
             BEAST_EXPECT(response[jss::error] == error);
-        }
+        });
     }
 
     void

src/test/rpc/LedgerRPC_test.cpp

@@ -2305,7 +2305,7 @@ public:
         testLedgerAccountsOption();
         testLedgerEntryDID();
 
-        test::jtx::forAllApiVersions(std::bind_front(
+        forAllApiVersions(std::bind_front(
             &LedgerRPC_test::testLedgerEntryInvalidParams, this));
     }
 };

src/test/rpc/LedgerRequestRPC_test.cpp

@@ -359,7 +359,7 @@ public:
     {
         testLedgerRequest();
         testEvolution();
-        test::jtx::forAllApiVersions(
+        forAllApiVersions(
             std::bind_front(&LedgerRequestRPC_test::testBadInput, this));
         testMoreThan256Closed();
         testNonAdmin();

src/test/rpc/NoRipple_test.cpp

@@ -284,12 +284,9 @@ public:
         testSetAndClear();
 
         auto withFeatsTests = [this](FeatureBitset features) {
-            for (auto testVersion = RPC::apiMinimumSupportedVersion;
+            forAllApiVersions([&, this](unsigned testVersion) {
-                 testVersion <= RPC::apiBetaVersion;
-                 ++testVersion)
-            {
                 testDefaultRipple(features, testVersion);
-            }
+            });
             testNegativeBalance(features);
             testPairwise(features);
         };

src/test/rpc/RPCCall_test.cpp

@@ -6219,8 +6219,7 @@ public:
     void
     run() override
     {
-        test::jtx::forAllApiVersions(
+        forAllApiVersions(std::bind_front(&RPCCall_test::testRPCCall, this));
-            std::bind_front(&RPCCall_test::testRPCCall, this));
     }
 };
 

src/test/rpc/TransactionEntry_test.cpp

@@ -388,7 +388,7 @@ public:
     run() override
     {
         testBadInput();
-        test::jtx::forAllApiVersions(
+        forAllApiVersions(
             std::bind_front(&TransactionEntry_test::testRequest, this));
     }
 };

src/test/rpc/Transaction_test.cpp

@@ -849,7 +849,7 @@ public:
     run() override
     {
         using namespace test::jtx;
-        test::jtx::forAllApiVersions(
+        forAllApiVersions(
             std::bind_front(&Transaction_test::testBinaryRequest, this));
 
         FeatureBitset const all{supported_amendments()};
@@ -863,7 +863,7 @@ public:
         testRangeCTIDRequest(features);
         testCTIDValidation(features);
         testCTIDRPC(features);
-        test::jtx::forAllApiVersions(
+        forAllApiVersions(
             std::bind_front(&Transaction_test::testRequest, this, features));
     }
 };

src/test/rpc/Version_test.cpp

@@ -83,7 +83,8 @@ class Version_test : public beast::unit_test::suite
             "{\"api_version\": " +
                 std::to_string(
                     std::max(
-                        RPC::apiMaximumSupportedVersion, RPC::apiBetaVersion) +
+                        RPC::apiMaximumSupportedVersion.value,
+                        RPC::apiBetaVersion.value) +
                     1) +
                 "}");
         BEAST_EXPECT(badVersion(re));
@@ -112,15 +113,15 @@ class Version_test : public beast::unit_test::suite
         Json::Value j_object = Json::Value(Json::objectValue);
         BEAST_EXPECT(
             RPC::getAPIVersionNumber(j_object, false) == versionIfUnspecified);
-        j_object[jss::api_version] = RPC::apiVersionIfUnspecified;
+        j_object[jss::api_version] = RPC::apiVersionIfUnspecified.value;
         BEAST_EXPECT(
             RPC::getAPIVersionNumber(j_object, false) == versionIfUnspecified);
 
-        j_object[jss::api_version] = RPC::apiMinimumSupportedVersion;
+        j_object[jss::api_version] = RPC::apiMinimumSupportedVersion.value;
         BEAST_EXPECT(
             RPC::getAPIVersionNumber(j_object, false) ==
             RPC::apiMinimumSupportedVersion);
-        j_object[jss::api_version] = RPC::apiMaximumSupportedVersion;
+        j_object[jss::api_version] = RPC::apiMaximumSupportedVersion.value;
         BEAST_EXPECT(
             RPC::getAPIVersionNumber(j_object, false) ==
             RPC::apiMaximumSupportedVersion);
@@ -133,14 +134,14 @@ class Version_test : public beast::unit_test::suite
         BEAST_EXPECT(
             RPC::getAPIVersionNumber(j_object, false) ==
             RPC::apiInvalidVersion);
-        j_object[jss::api_version] = RPC::apiBetaVersion;
+        j_object[jss::api_version] = RPC::apiBetaVersion.value;
         BEAST_EXPECT(
             RPC::getAPIVersionNumber(j_object, true) == RPC::apiBetaVersion);
         j_object[jss::api_version] = RPC::apiBetaVersion + 1;
         BEAST_EXPECT(
             RPC::getAPIVersionNumber(j_object, true) == RPC::apiInvalidVersion);
 
-        j_object[jss::api_version] = RPC::apiInvalidVersion;
+        j_object[jss::api_version] = RPC::apiInvalidVersion.value;
         BEAST_EXPECT(
             RPC::getAPIVersionNumber(j_object, false) ==
             RPC::apiInvalidVersion);
@@ -202,17 +203,17 @@ class Version_test : public beast::unit_test::suite
             "\"id\": 5, "
             "\"method\": \"version\", "
             "\"params\": {}}";
-        auto const with_wrong_api_verion =
+        auto const with_wrong_api_verion = std::string("{ ") +
-            std::string("{ ") +
             "\"jsonrpc\": \"2.0\", "
             "\"ripplerpc\": \"2.0\", "
             "\"id\": 6, "
             "\"method\": \"version\", "
             "\"params\": { "
             "\"api_version\": " +
-            std::to_string(
+            std::to_string(std::max(
-                std::max(RPC::apiMaximumSupportedVersion, RPC::apiBetaVersion) +
+                               RPC::apiMaximumSupportedVersion.value,
-                1) +
+                               RPC::apiBetaVersion.value) +
+                           1) +
             "}}";
         auto re = env.rpc(
             "json2",
@@ -275,8 +276,9 @@ class Version_test : public beast::unit_test::suite
             jrr[jss::version].isMember(jss::last))
             return;
         BEAST_EXPECT(
-            jrr[jss::version][jss::first] == RPC::apiMinimumSupportedVersion);
+            jrr[jss::version][jss::first] ==
-        BEAST_EXPECT(jrr[jss::version][jss::last] == RPC::apiBetaVersion);
+            RPC::apiMinimumSupportedVersion.value);
+        BEAST_EXPECT(jrr[jss::version][jss::last] == RPC::apiBetaVersion.value);
     }
 
 public: