Set version to 2.0.0-rc4

Show `DeliverMax` instead of `Amount` in output from `submit`,
`submit_multisigned`, `sign`, and `sign_for`.

Fix #4829

src/ripple/app/ledger/impl/LedgerToJson.cpp

@@ -27,6 +27,7 @@
 #include <ripple/protocol/jss.h>
 #include <ripple/rpc/Context.h>
 #include <ripple/rpc/DeliveredAmount.h>
+#include <ripple/rpc/impl/RPCHelpers.h>
 
 namespace ripple {
 
@@ -52,10 +53,17 @@ isBinary(LedgerFill const& fill)
 
 template <class Object>
 void
-fillJson(Object& json, bool closed, LedgerInfo const& info, bool bFull)
+fillJson(
+    Object& json,
+    bool closed,
+    LedgerInfo const& info,
+    bool bFull,
+    unsigned apiVersion)
 {
     json[jss::parent_hash] = to_string(info.parentHash);
-    json[jss::ledger_index] = to_string(info.seq);
+    json[jss::ledger_index] = (apiVersion > 1)
+        ? Json::Value(info.seq)
+        : Json::Value(std::to_string(info.seq));
 
     if (closed)
     {
@@ -159,7 +167,10 @@ fillJsonTx(
         txJson[jss::validated] = validated;
         if (validated)
         {
-            txJson[jss::ledger_index] = to_string(fill.ledger.seq());
+            auto const seq = fill.ledger.seq();
+            txJson[jss::ledger_index] = (fill.context->apiVersion > 1)
+                ? Json::Value(seq)
+                : Json::Value(std::to_string(seq));
             if (fill.closeTime)
                 txJson[jss::close_time_iso] = to_string_iso(*fill.closeTime);
         }
@@ -315,7 +326,13 @@ fillJson(Object& json, LedgerFill const& fill)
     if (isBinary(fill))
         fillJsonBinary(json, !fill.ledger.open(), fill.ledger.info());
     else
-        fillJson(json, !fill.ledger.open(), fill.ledger.info(), bFull);
+        fillJson(
+            json,
+            !fill.ledger.open(),
+            fill.ledger.info(),
+            bFull,
+            (fill.context ? fill.context->apiVersion
+                          : RPC::apiMaximumSupportedVersion));
 
     if (bFull || fill.options & LedgerFill::dumpTxrp)
         fillJsonTx(json, fill);

src/ripple/app/misc/NetworkOPs.cpp

@@ -2181,8 +2181,10 @@ NetworkOPsImp::pubValidation(std::shared_ptr<STValidation> const& val)
         if (masterKey != signerPublic)
             jvObj[jss::master_key] = toBase58(TokenType::NodePublic, masterKey);
 
+        // NOTE *seq is a number, but old API versions used string. We replace
+        // number with a string using MultiApiJson near end of this function
         if (auto const seq = (*val)[~sfLedgerSequence])
-            jvObj[jss::ledger_index] = to_string(*seq);
+            jvObj[jss::ledger_index] = *seq;
 
         if (val->isFieldPresent(sfAmendments))
         {
@@ -2220,12 +2222,28 @@ NetworkOPsImp::pubValidation(std::shared_ptr<STValidation> const& val)
             reserveIncXRP && reserveIncXRP->native())
             jvObj[jss::reserve_inc] = reserveIncXRP->xrp().jsonClipped();
 
+        // 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) {
+                // Type conversion for older API versions to string
+                if (jvTx.isMember(jss::ledger_index) && apiVersion < 2)
+                {
+                    jvTx[jss::ledger_index] =
+                        std::to_string(jvTx[jss::ledger_index].asUInt());
+                }
+            });
+
         for (auto i = mStreamMaps[sValidations].begin();
              i != mStreamMaps[sValidations].end();)
         {
             if (auto p = i->second.lock())
             {
-                p->send(jvObj, true);
+                p->send(
+                    multiObj.select(apiVersionSelector(p->getApiVersion())),
+                    true);
                 ++i;
             }
             else
@@ -3159,25 +3177,10 @@ 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);
-    // Last valid (possibly beta) API ver. must match last index in MultiApiJson
-    static_assert(
-        apiVersionSelector(RPC::apiMaximumValidVersion)() + 1  //
-        == MultiApiJson::size);
-    for (unsigned apiVersion = RPC::apiMinimumSupportedVersion,
-                  lastIndex = MultiApiJson::size;
-         apiVersion <= RPC::apiMaximumValidVersion;
-         ++apiVersion)
-    {
-        unsigned const index = apiVersionSelector(apiVersion)();
-        assert(index < MultiApiJson::size);
-        if (index != lastIndex)
-        {
-            lastIndex = index;
-
-            Json::Value& jvTx = multiObj.val[index];
+    MultiApiJson multiObj{jvObj};
+    visit<RPC::apiMinimumSupportedVersion, RPC::apiMaximumValidVersion>(
+        multiObj,  //
+        [&](Json::Value& jvTx, unsigned int apiVersion) {
             RPC::insertDeliverMax(
                 jvTx[jss::transaction], transaction->getTxnType(), apiVersion);
 
@@ -3190,8 +3193,7 @@ NetworkOPsImp::transJson(
             {
                 jvTx[jss::transaction][jss::hash] = hash;
             }
-        }
-    }
+        });
 
     return multiObj;
 }

src/ripple/json/MultivarJson.h

@@ -26,14 +26,24 @@
 #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;
+    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())>
@@ -68,7 +78,7 @@ struct MultivarJson
 };
 
 // Wrapper for Json for all supported API versions.
-using MultiApiJson = MultivarJson<2>;
+using MultiApiJson = MultivarJson<3>;
 
 /*
 
@@ -78,21 +88,17 @@ 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.
 
-e.g. There are 2 formats now, the first, for version one, the second for
-versions > 1. Hypothetically, if API version 4 adds a new format, `MultiApiJson`
-would be MultivarJson<3>, and `apiVersionSelector` would return
-`static_cast<std::size_t>(apiVersion < 2 ? 0u : (apiVersion < 4 ? 1u : 2u))`
-
-NOTE:
-
 The more different JSON formats we support, the more CPU cycles we need to
-pre-build JSON for different API versions e.g. when publishing streams 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
@@ -101,12 +107,44 @@ apiVersionSelector(unsigned int apiVersion) noexcept
 {
     return [apiVersion]() constexpr
     {
-        // apiVersion <= 1 returns 0
-        // apiVersion > 1  returns 1
-        return static_cast<std::size_t>(apiVersion > 1);
+        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/impl/BuildInfo.cpp

@@ -33,7 +33,7 @@ namespace BuildInfo {
 //  and follow the format described at http://semver.org/
 //------------------------------------------------------------------------------
 // clang-format off
-char const* const versionString = "2.0.0-rc3"
+char const* const versionString = "2.0.0-rc4"
 // clang-format on
 
 #if defined(DEBUG) || defined(SANITIZER)

src/ripple/rpc/impl/TransactionSign.cpp

@@ -20,6 +20,7 @@
 #include <ripple/app/ledger/LedgerMaster.h>
 #include <ripple/app/ledger/OpenLedger.h>
 #include <ripple/app/main/Application.h>
+#include <ripple/app/misc/DeliverMax.h>
 #include <ripple/app/misc/LoadFeeTrack.h>
 #include <ripple/app/misc/Transaction.h>
 #include <ripple/app/misc/TxQ.h>
@@ -659,6 +660,11 @@ transactionFormatResultImpl(Transaction::pointer tpTrans, unsigned apiVersion)
         else
             jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none);
 
+        RPC::insertDeliverMax(
+            jvResult[jss::tx_json],
+            tpTrans->getSTransaction()->getTxnType(),
+            apiVersion);
+
         jvResult[jss::tx_blob] =
             strHex(tpTrans->getSTransaction()->getSerializer().peekData());
 

src/test/json/MultivarJson_test.cpp

@@ -33,21 +33,23 @@ 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
     {
-        constexpr static Json::StaticString string1("string1");
-        static Json::Value const str1{string1};
+        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{};
 
-        static Json::Value const obj1{[]() {
-            Json::Value obj1(Json::objectValue);
-            obj1["one"] = 1;
-            return obj1;
-        }()};
-
-        static Json::Value const jsonNull{};
-
-        MultivarJson<3> const subject({str1, obj1});
+        MultivarJson<3> subject{};
         static_assert(sizeof(subject) == sizeof(subject.val));
         static_assert(subject.size == subject.val.size());
         static_assert(
@@ -55,13 +57,11 @@ struct MultivarJson_test : beast::unit_test::suite
 
         BEAST_EXPECT(subject.val.size() == 3);
         BEAST_EXPECT(
-            (subject.val == std::array<Json::Value, 3>{str1, obj1, jsonNull}));
-        BEAST_EXPECT(
-            (MultivarJson<3>({obj1, str1}).val ==
-             std::array<Json::Value, 3>{obj1, str1, jsonNull}));
-        BEAST_EXPECT(
-            (MultivarJson<3>({jsonNull, obj1, str1}).val ==
-             std::array<Json::Value, 3>{jsonNull, obj1, str1}));
+            (subject.val ==
+             std::array<Json::Value, 3>{jsonNull, jsonNull, jsonNull}));
+
+        subject.val[0] = obj1;
+        subject.val[1] = obj2;
 
         {
             testcase("default copy construction / assignment");
@@ -96,9 +96,9 @@ struct MultivarJson_test : beast::unit_test::suite
             testcase("select");
 
             BEAST_EXPECT(
-                subject.select([]() -> std::size_t { return 0; }) == str1);
+                subject.select([]() -> std::size_t { return 0; }) == obj1);
             BEAST_EXPECT(
-                subject.select([]() -> std::size_t { return 1; }) == obj1);
+                subject.select([]() -> std::size_t { return 1; }) == obj2);
             BEAST_EXPECT(
                 subject.select([]() -> std::size_t { return 2; }) == jsonNull);
 
@@ -144,12 +144,9 @@ struct MultivarJson_test : beast::unit_test::suite
         }
 
         {
-            struct foo_t final
-            {
-            };
             testcase("set");
 
-            auto x = MultivarJson<2>{{Json::objectValue, Json::objectValue}};
+            auto x = MultivarJson<2>{Json::objectValue};
             x.set("name1", 42);
             BEAST_EXPECT(x.val[0].isMember("name1"));
             BEAST_EXPECT(x.val[1].isMember("name1"));
@@ -193,6 +190,9 @@ struct MultivarJson_test : beast::unit_test::suite
             }(x));
 
             // Tests of requires clause - these are expected NOT to match
+            struct foo_t final
+            {
+            };
             static_assert([](auto&& v) {
                 return !requires
                 {
@@ -213,32 +213,29 @@ struct MultivarJson_test : beast::unit_test::suite
             // Well defined behaviour even if we have different types of members
             BEAST_EXPECT(subject.isMember("foo") == decltype(subject)::none);
 
-            auto const makeJson = [](const char* key, int val) {
-                Json::Value obj1(Json::objectValue);
-                obj1[key] = val;
-                return obj1;
-            };
-
             {
                 // All variants have element "One", none have element "Two"
-                MultivarJson<2> const s1{
-                    {makeJson("One", 12), makeJson("One", 42)}};
+                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> const s2{
-                    {makeJson("One", 12), makeJson("Two", 42)}};
+                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> const s3{
-                    {makeJson("One", 12), makeJson("One", 42), {}}};
+                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);
             }
@@ -248,8 +245,10 @@ struct MultivarJson_test : beast::unit_test::suite
             // NOTE It's fine to change this test when we change API versions
             testcase("apiVersionSelector");
 
-            static_assert(MultiApiJson::size == 2);
-            static MultiApiJson x{{obj1, str1}};
+            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>);
@@ -261,15 +260,16 @@ struct MultivarJson_test : beast::unit_test::suite
             }(x));
 
             BEAST_EXPECT(x.select(apiVersionSelector(0)) == obj1);
-            BEAST_EXPECT(x.select(apiVersionSelector(2)) == str1);
+            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)() == 1);
+            static_assert(apiVersionSelector(3)() == 2);
+            static_assert(apiVersionSelector(4)() == 2);
             static_assert(
                 apiVersionSelector(
-                    std::numeric_limits<unsigned int>::max())() == 1);
+                    std::numeric_limits<unsigned int>::max())() == 2);
         }
 
         {
@@ -284,6 +284,100 @@ struct MultivarJson_test : beast::unit_test::suite
 
             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));
+        }
     }
 };