refactor: Let json::Value be compared and constructed from a string view

`json::Value` accepts `char const*`, `std::string` and
`json::StaticString`, so a caller holding a `std::string_view` has to
materialize a `std::string` whose characters are then copied a second
time into the value's own storage. Add the missing constructor, which
the `std::string` one delegates to, and an `operator==` that reads the
value's characters in place.

The comparison is constrained to `std::string_view` exactly rather than
taking a view by plain overload. A string literal converts equally well
to a view and to a Value, so a plain overload makes every
`value == "literal"` ambiguous, and a literal `0`, which converts to a
view through `char const*` as well as to a Value, with it.

No reply changes. The two spellings differ only for a view holding an
embedded NUL, which the Value comparison stops at. The `method: "batch"`
check in `ServerHandler.cpp` is the operator's first production user, so
that comparison stops building a `Value` from the literal on every
request.

.github/actions/cargo-cache/action.yml

@@ -30,7 +30,7 @@ runs:
 
   steps:
     - name: Use cargo artifacts cache
-      uses: Swatinem/rust-cache@d1527c8f67ef9af357290475d4af4b0fab8f12e0 # v2.9.3
+      uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2
       with:
         cache-bin: "false"
         cache-directories: ${{ inputs.cache-directories }}

.github/workflows/reusable-check-autogen.yml

@@ -25,7 +25,7 @@ jobs:
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
 
       - name: Set up Python
-        uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0
+        uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
         with:
           python-version: "3.13"
 

.github/workflows/reusable-package.yml

@@ -73,7 +73,7 @@ jobs:
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
 
       - name: Set up Python
-        uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0
+        uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
         with:
           python-version: "3.13"
 
@@ -326,14 +326,14 @@ jobs:
 
       - name: Log in to Docker Hub
         if: ${{ env.PUSH == 'true' && matrix.target == 'xrpld' }}
-        uses: docker/login-action@81c6f93d4bc991e510b342a00b419b3bb4efc5c6 # v4.7.0
+        uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0
         with:
           username: xrplf
           password: ${{ secrets.dockerhub_token }}
 
       - name: Log in to the Antithesis registry
         if: ${{ env.PUSH == 'true' && matrix.target == 'voidstar' }}
-        uses: docker/login-action@81c6f93d4bc991e510b342a00b419b3bb4efc5c6 # v4.7.0
+        uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0
         with:
           registry: ${{ secrets.antithesis_docker_host }}
           username: _json_key

.github/workflows/reusable-strategy-matrix.yml

@@ -26,7 +26,7 @@ jobs:
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
 
       - name: Set up Python
-        uses: actions/setup-python@6d4fd22b20465ccc0f7354476588e8df7c9ca0e0 # v7.1.0
+        uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
         with:
           python-version: "3.13"
 

API-CHANGELOG.md

@@ -38,10 +38,6 @@ Version 3.5.0 is not yet released.
 
 ### Bugfixes in 3.5.0
 
-- `account_tx`, `ledger`, `sign`, `sign_for`, `submit`, `submit_multisigned`, `subscribe` (transaction streams), `transaction_entry`, `tx`, `tx_history`: A Payment inside the `RawTransactions` of a Batch now follows the same `DeliverMax` rule as any other Payment: `Amount` and `DeliverMax` in API version 1, and only `DeliverMax` in API version 2 and later. Previously an inner Payment showed only `Amount` in every API version. [#8558](https://github.com/XRPLF/rippled/pull/8558)
-- `sign`, `sign_for`, `submit`, `submit_multisigned`: An inner Payment of a Batch now accepts `DeliverMax` in place of `Amount`, as a top-level Payment already did; differing `Amount` and `DeliverMax` return `invalidParams`. Previously `DeliverMax` in an inner Payment was rejected as an unknown field. [#8558](https://github.com/XRPLF/rippled/pull/8558)
-- `simulate`: A Payment now accepts `DeliverMax` in place of `Amount`, and the returned `tx_json` follows the `DeliverMax` rule. Previously `DeliverMax` was rejected as an unknown field and the returned `tx_json` showed only `Amount`. [#8558](https://github.com/XRPLF/rippled/pull/8558)
-- `submit`: With `tx_blob`, the returned `tx_json` now follows the `DeliverMax` rule. Previously it showed only `Amount`. [#8558](https://github.com/XRPLF/rippled/pull/8558)
 - `channel_authorize`: The `channel_id` field now returns an `invalidParams` error if the value is not a string. [#7582](https://github.com/XRPLF/rippled/pull/7582)
 - `channel_verify`: The `channel_id` and `signature` fields now return an `invalidParams` error if the value is not a string. [#7582](https://github.com/XRPLF/rippled/pull/7582)
 - `account_currencies`, `account_nfts`, `account_offers`: A request with more than one problem may now report a different one of them. An invalid `ledger_hash` or `ledger_index` is reported before an invalid or missing `account`. An invalid `limit` or `marker` is reported before `lgrNotFound` or `actNotFound`, and for `account_offers` also before `actMalformed`. A request with a single problem returns the same error as before. [#8561](https://github.com/XRPLF/rippled/pull/8561)

include/xrpl/json/json_value.h

@@ -3,11 +3,13 @@
 #include <xrpl/basics/Number.h>
 #include <xrpl/json/json_forwards.h>
 
+#include <concepts>
 #include <cstring>
 #include <iterator>
 #include <limits>
 #include <map>
 #include <string>
+#include <string_view>
 #include <vector>
 
 /**
@@ -198,6 +200,15 @@ public:
      */
     Value(StaticString const& value);
     Value(std::string const& value);
+    /**
+     * @brief Constructs a value from a string view.
+     *
+     * The characters are copied, so the view need not outlive the call and need
+     * not be NUL-terminated.
+     *
+     * @param value The characters to copy.
+     */
+    Value(std::string_view value);
     Value(bool value);
     Value(Value const& other);
     ~Value();
@@ -472,6 +483,32 @@ toJson(xrpl::Number const& number)
 bool
 operator==(Value const&, Value const&);
 
+/**
+ * Compares a value with a string view, reading the value's characters in place
+ * rather than building a Value from the view.
+ *
+ * Constrained to the exact type: a string literal converts equally well to a
+ * view and to a Value, so a plain overload makes `value == "literal"`
+ * ambiguous.
+ *
+ * @param x The value to compare.
+ * @param y The characters to compare it against.
+ * @return Whether `x` is a string whose characters up to its first NUL are
+ *         exactly the characters of `y`.
+ */
+template <class T>
+    requires std::same_as<T, std::string_view>
+bool
+operator==(Value const& x, T y)
+{
+    if (!x.isString())
+        return false;
+
+    // A string `Value` can hold a null pointer, which names no characters, so it equals no view.
+    char const* const s = x.asCString();
+    return s != nullptr && std::string_view{s} == y;
+}
+
 bool
 operator<(Value const&, Value const&);
 

src/libxrpl/json/json_value.cpp

@@ -15,6 +15,7 @@
 #include <cstring>
 #include <limits>
 #include <string>
+#include <string_view>
 #include <utility>
 
 namespace json {
@@ -241,10 +242,14 @@ Value::Value(xrpl::Number const& value) : type_(ValueType::String), allocated_(t
     value_.stringVal = valueAllocator()->duplicateStringValue(tmp.c_str(), tmp.length());
 }
 
-Value::Value(std::string const& value) : type_(ValueType::String), allocated_(true)
+Value::Value(std::string const& value) : Value(std::string_view{value})
 {
-    value_.stringVal =
-        valueAllocator()->duplicateStringValue(value.c_str(), (unsigned int)value.length());
+}
+
+Value::Value(std::string_view value) : type_(ValueType::String), allocated_(true)
+{
+    value_.stringVal = valueAllocator()->duplicateStringValue(
+        value.data(), static_cast<unsigned int>(value.length()));
 }
 
 Value::Value(StaticString const& value) : type_(ValueType::String)

src/test/app/Batch_test.cpp

@@ -37,13 +37,11 @@
 #include <xrpl/config/BasicConfig.h>
 #include <xrpl/config/Constants.h>
 #include <xrpl/core/HashRouter.h>
-#include <xrpl/json/json_forwards.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/json/to_string.h>
 #include <xrpl/ledger/ApplyView.h>
 #include <xrpl/ledger/OpenView.h>
 #include <xrpl/protocol/AccountID.h>
-#include <xrpl/protocol/ApiVersion.h>
 #include <xrpl/protocol/Batch.h>
 #include <xrpl/protocol/ErrorCodes.h>
 #include <xrpl/protocol/Feature.h>
@@ -6361,179 +6359,6 @@ class Batch_test : public beast::unit_test::Suite
         }
     }
 
-    void
-    testDeliverMax(FeatureBitset features, SField const& wrapper)
-    {
-        testcase("DeliverMax in inner Payments wrapped in " + wrapper.getName());
-
-        using namespace test::jtx;
-        Env env(*this, features);
-        Account const alice("alice");
-        Account const bob("bob");
-        env.fund(XRP(10000), alice, bob);
-        env.close();
-
-        auto const seq = env.seq(alice);
-        auto innerPay = [&](STAmount const& amount, std::uint32_t innerSeq) {
-            json::Value inner = pay(alice, bob, amount);
-            inner[jss::SigningPubKey] = "";
-            inner[jss::Sequence] = innerSeq;
-            inner[jss::Fee] = "0";
-            inner[jss::Flags] = tfInnerBatchTxn;
-            return inner;
-        };
-
-        // Signs a Batch of the given inner Payment and a second one paying 2 XRP.
-        auto const signBatch = [&](json::Value const& first,
-                                   json::Value const& outerType = json::Value(jss::Batch)) {
-            json::Value txJson = batch::outer(alice, seq, XRPAmount{}, tfAllOrNothing);
-            txJson[jss::TransactionType] = outerType;
-            txJson.removeMember(jss::Fee);
-            txJson[jss::RawTransactions][0u][wrapper.jsonName] = first;
-            txJson[jss::RawTransactions][1u][wrapper.jsonName] = innerPay(XRP(2), seq + 2);
-            json::Value params;
-            params[jss::secret] = alice.name();
-            params[jss::tx_json] = txJson;
-            return env.rpc("json", "sign", to_string(params))[jss::result];
-        };
-
-        // Both inner Payments follow the DeliverMax rule for apiVersion.
-        auto expectInner = [&](json::Value const& txJson, unsigned apiVersion) {
-            auto const& raw = txJson[jss::RawTransactions];
-            if (!BEAST_EXPECT(raw.isArray() && raw.size() == 2))
-                return;
-            for (auto const& [index, drops] : {std::pair{0u, "1000000"}, {1u, "2000000"}})
-            {
-                auto const& inner = raw[index][wrapper.jsonName];
-                BEAST_EXPECT(inner[jss::DeliverMax] == drops);
-                BEAST_EXPECT(
-                    apiVersion > 1 ? !inner.isMember(jss::Amount) : inner[jss::Amount] == drops);
-            }
-        };
-
-        auto isBatch = [](json::Value const& txJson) {
-            return txJson[jss::TransactionType].asString() == jss::Batch.cStr();
-        };
-
-        // An inner Payment with differing Amount and DeliverMax is rejected.
-        {
-            auto inner = innerPay(XRP(1), seq + 1);
-            inner[jss::DeliverMax] = "3000000";
-            auto const jr = signBatch(inner);
-            BEAST_EXPECT(jr[jss::error] == "invalidParams");
-            BEAST_EXPECT(
-                jr[jss::error_message] == "Cannot specify differing 'Amount' and 'DeliverMax'");
-        }
-
-        // An inner Payment with DeliverMax in place of Amount is signed, and the
-        // autofilled fee covers both inner transactions.
-        auto inner = innerPay(XRP(1), seq + 1);
-        inner[jss::DeliverMax] = inner[jss::Amount];
-        inner.removeMember(jss::Amount);
-        auto const signedBatch = signBatch(inner);
-        if (!BEAST_EXPECT(signedBatch[jss::status] == jss::success))
-            return;
-        BEAST_EXPECT(
-            signedBatch[jss::tx_json][jss::Fee] == to_string(batch::calcBatchFee(env, 0, 2)));
-        expectInner(signedBatch[jss::tx_json], 1);
-
-        // TransactionType as a number or a numeric string, on the Batch and on the
-        // inner Payment, is read as STParsedJSON reads it.
-        {
-            auto const batchType = static_cast<json::UInt>(ttBATCH);
-            auto const paymentType = static_cast<json::UInt>(ttPAYMENT);
-            using Form = std::pair<json::Value, json::Value>;
-            for (auto const& [outerType, innerType] : {
-                     Form{batchType, jss::Payment},
-                     Form{jss::Batch, paymentType},
-                     Form{batchType, paymentType},
-                     Form{std::to_string(batchType), std::to_string(paymentType)},
-                 })
-            {
-                auto numeric = inner;
-                numeric[jss::TransactionType] = innerType;
-                auto const jr = signBatch(numeric, outerType);
-                if (BEAST_EXPECT(jr[jss::status] == jss::success))
-                    expectInner(jr[jss::tx_json], 1);
-            }
-
-            // A numeric type that is not a Payment keeps DeliverMax unknown.
-            auto notPayment = inner;
-            notPayment[jss::TransactionType] = static_cast<json::UInt>(ttESCROW_CREATE);
-            auto const jr = signBatch(notPayment, batchType);
-            BEAST_EXPECT(jr[jss::error] == "invalidParams");
-            BEAST_EXPECT(jr[jss::error_message].asString().ends_with(".DeliverMax' is unknown."));
-        }
-
-        auto const preBob = env.balance(bob);
-        {
-            json::Value params;
-            params[jss::tx_blob] = signedBatch[jss::tx_blob];
-            params[jss::api_version] = 2;
-            auto const jr = env.rpc("json", "submit", to_string(params))[jss::result];
-            BEAST_EXPECT(jr[jss::engine_result] == "tesSUCCESS");
-            expectInner(jr[jss::tx_json], 2);
-        }
-        env.close();
-        BEAST_EXPECT(env.balance(bob) == preBob + XRP(3));
-
-        auto const hash = signedBatch[jss::tx_json][jss::hash].asString();
-        forAllApiVersions([&](unsigned apiVersion) {
-            {
-                json::Value params;
-                params[jss::transaction] = hash;
-                params[jss::api_version] = apiVersion;
-                auto const jr = env.rpc("json", "tx", to_string(params))[jss::result];
-                expectInner(apiVersion > 1 ? jr[jss::tx_json] : jr, apiVersion);
-            }
-            {
-                json::Value params;
-                params[jss::tx_hash] = hash;
-                params[jss::ledger_index] = env.closed()->seq();
-                params[jss::api_version] = apiVersion;
-                auto const jr =
-                    env.rpc("json", "transaction_entry", to_string(params))[jss::result];
-                expectInner(jr[jss::tx_json], apiVersion);
-            }
-            {
-                json::Value params;
-                params[jss::ledger_index] = env.closed()->seq();
-                params[jss::transactions] = true;
-                params[jss::expand] = true;
-                params[jss::api_version] = apiVersion;
-                auto const jr = env.rpc("json", "ledger", to_string(params))[jss::result];
-                int batches = 0;
-                for (auto const& txn : jr[jss::ledger][jss::transactions])
-                {
-                    auto const& txJson = apiVersion > 1 ? txn[jss::tx_json] : txn;
-                    if (isBatch(txJson))
-                    {
-                        ++batches;
-                        expectInner(txJson, apiVersion);
-                    }
-                }
-                BEAST_EXPECT(batches == 1);
-            }
-            {
-                json::Value params;
-                params[jss::account] = alice.human();
-                params[jss::api_version] = apiVersion;
-                auto const jr = env.rpc("json", "account_tx", to_string(params))[jss::result];
-                int batches = 0;
-                for (auto const& txn : jr[jss::transactions])
-                {
-                    auto const& txJson = txn[apiVersion > 1 ? jss::tx_json : jss::tx];
-                    if (isBatch(txJson))
-                    {
-                        ++batches;
-                        expectInner(txJson, apiVersion);
-                    }
-                }
-                BEAST_EXPECT(batches == 1);
-            }
-        });
-    }
-
     void
     testWithFeats(FeatureBitset features)
     {
@@ -6575,10 +6400,6 @@ class Batch_test : public beast::unit_test::Suite
         testUnsortedBatchSigners(features);
         testBatchSigCache(features);
         testWrappedInnerSubmission(features);
-        testDeliverMax(features, sfRawTransaction);
-        // Without fixBatchV1_2 any object field may wrap an inner transaction.
-        testDeliverMax(features - fixBatchV1_2, sfMemo);
-        testDeliverMax(features - fixBatchV1_2, sfCreatedNode);
     }
 
 public:

src/test/app/ValidatorSite_test.cpp

@@ -132,21 +132,8 @@ private:
         std::chrono::seconds effectiveOverlap = detail::kDefaultEffectiveOverlap;
         int expectedRefreshMin = 0;
     };
-
-    /**
-     * Serve each configured list from its own publisher server, fetch them all
-     * through one ValidatorSite and check what it reports for each site.
-     *
-     * @param good Directory the cache files are written to.
-     * @param paths One server per entry, with what to serve and what to expect.
-     * @param requestTimeout How long the ValidatorSite waits for a response
-     * before it reports that the request took too long.
-     */
     void
-    testFetchList(
-        detail::DirGuard const& good,
-        std::vector<FetchListConfig> const& paths,
-        std::chrono::seconds requestTimeout = std::chrono::seconds{12})
+    testFetchList(detail::DirGuard const& good, std::vector<FetchListConfig> const& paths)
     {
         testcase << "Fetch list - "
                  << boost::algorithm::join(
@@ -224,10 +211,10 @@ private:
 
         BEAST_EXPECT(trustedKeys.load({}, emptyCfgKeys, cfgPublishers));
 
-        // Normally, tests will only need a fraction of the default timeout,
-        // but sometimes DNS resolution takes an inordinate amount of time, so
-        // the test will just wait.
-        auto sites = std::make_unique<ValidatorSite>(env.app(), journal, requestTimeout);
+        // Normally, tests will only need a fraction of this time,
+        // but sometimes DNS resolution takes an inordinate amount
+        // of time, so the test will just wait.
+        auto sites = std::make_unique<ValidatorSite>(env.app(), journal, 12s);
 
         std::vector<std::string> uris;
         for (auto const& u : servers)
@@ -599,19 +586,18 @@ public:
                         .failApply = true,
                     },
                 });
-            // timeout: the server answers 1s after the request times out
+            // timeout
             testFetchList(
                 good,
                 {
                     {
-                        .path = "/sleep/2",
+                        .path = "/sleep/13",
                         .msg = "took too long",
                         .ssl = ssl,
                         .failFetch = true,
                         .failApply = true,
                     },
-                },
-                std::chrono::seconds{1});
+                });
             // bad manifest format using known versions
             // * Retrieves a v1 formatted list claiming version 2
             testFetchList(

src/test/rpc/Simulate_test.cpp

@@ -24,7 +24,6 @@
 #include <xrpl/config/Constants.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/json/to_string.h>
-#include <xrpl/protocol/ApiVersion.h>
 #include <xrpl/protocol/ErrorCodes.h>
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/SField.h>
@@ -1476,65 +1475,6 @@ class Simulate_test : public beast::unit_test::Suite
         }
     }
 
-    void
-    testDeliverMax()
-    {
-        testcase("DeliverMax");
-
-        using namespace jtx;
-        Env env(*this);
-        Account const alice("alice");
-        Account const becky("becky");
-        env.fund(XRP(10000), alice);
-        env.close();
-        env(signers(alice, 1, {{becky, 1}}));
-        env.close();
-
-        auto simulate = [&](json::Value const& tx, unsigned apiVersion) {
-            json::Value params;
-            params[jss::tx_json] = tx;
-            params[jss::api_version] = apiVersion;
-            return env.rpc("json", "simulate", to_string(params))[jss::result];
-        };
-
-        json::Value tx;
-        tx[jss::TransactionType] = jss::Payment;
-        tx[jss::Account] = alice.human();
-        tx[jss::Destination] = env.master.human();
-        tx[jss::DeliverMax] = "1000000";
-
-        forAllApiVersions([&](unsigned apiVersion) {
-            auto const jr = simulate(tx, apiVersion);
-            BEAST_EXPECT(jr[jss::engine_result] == "tesSUCCESS");
-            BEAST_EXPECT(jr[jss::tx_json][jss::DeliverMax] == "1000000");
-            BEAST_EXPECT(
-                apiVersion > 1 ? !jr[jss::tx_json].isMember(jss::Amount)
-                               : jr[jss::tx_json][jss::Amount] == "1000000");
-        });
-
-        {
-            auto differing = tx;
-            differing[jss::Amount] = "2000000";
-            auto const jr = simulate(differing, 2);
-            BEAST_EXPECT(jr[jss::error] == "invalidParams");
-            BEAST_EXPECT(
-                jr[jss::error_message] == "Cannot specify differing 'Amount' and 'DeliverMax'");
-        }
-
-        // The autofilled fee of a multi-signed Payment counts its signer when
-        // the amount is given as DeliverMax.
-        json::Value signer;
-        signer[jss::Account] = becky.human();
-        tx[sfSigners] = json::ValueType::Array;
-        tx[sfSigners].append(json::Value{});
-        tx[sfSigners][0u][sfSigner] = signer;
-        auto const jr = simulate(tx, 2);
-        BEAST_EXPECT(jr[jss::engine_result] == "tesSUCCESS");
-        BEAST_EXPECT(
-            jr[jss::tx_json][jss::Fee] ==
-            (env.current()->fees().base * 2).jsonClipped().asString());
-    }
-
 public:
     void
     run() override
@@ -1554,7 +1494,6 @@ public:
         testDeleteExpiredCredentials();
         testSuccessfulTransactionNetworkID();
         testSuccessfulTransactionAdditionalMetadata();
-        testDeliverMax();
     }
 };
 

src/test/rpc/Submit_test.cpp

@@ -8,7 +8,6 @@
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/json/to_string.h>
-#include <xrpl/protocol/ApiVersion.h>
 #include <xrpl/protocol/ErrorCodes.h>
 #include <xrpl/protocol/Seed.h>
 #include <xrpl/protocol/jss.h>
@@ -90,37 +89,10 @@ public:
         }
     }
 
-    void
-    testDeliverMax()
-    {
-        testcase("DeliverMax in tx_blob response");
-        using namespace jtx;
-        Env env(*this);
-        Account const alice{"alice"};
-        Account const bob{"bob"};
-        env.fund(XRP(10000), alice, bob);
-        env.close();
-
-        forAllApiVersions([&](unsigned apiVersion) {
-            auto const jt = env.jt(pay(alice, bob, XRP(1)));
-
-            json::Value params;
-            params[jss::tx_blob] = strHex(jt.stx->getSerializer().slice());
-            params[jss::api_version] = apiVersion;
-            auto const jrr = env.rpc("json", "submit", to_string(params))[jss::result];
-            BEAST_EXPECT(jrr[jss::engine_result] == "tesSUCCESS");
-            BEAST_EXPECT(jrr[jss::tx_json][jss::DeliverMax] == "1000000");
-            BEAST_EXPECT(
-                apiVersion > 1 ? !jrr[jss::tx_json].isMember(jss::Amount)
-                               : jrr[jss::tx_json][jss::Amount] == "1000000");
-        });
-    }
-
     void
     run() override
     {
         testFailHardValidation();
-        testDeliverMax();
     }
 };
 

src/tests/libxrpl/json/Value.cpp

@@ -598,6 +598,91 @@ TEST(JsonValue, compare_strings)
 #pragma pop_macro("DO_COMPARE")
 }
 
+TEST(JsonValue, construct_from_string_view)
+{
+    using namespace std::string_view_literals;
+
+    // Characters are copied with an explicit length, so a view need not be NUL-terminated.
+    EXPECT_EQ(json::Value{"2.0"sv}.asString(), "2.0");
+    EXPECT_EQ(json::Value{"2.0 and more"sv.substr(0, 3)}.asString(), "2.0");
+    EXPECT_EQ(json::Value{""sv}.asString(), "");
+
+    // A default-constructed view has a null `data()` and yields an empty string value.
+    std::string_view const empty;
+    json::Value const fromEmpty{empty};
+    EXPECT_TRUE(fromEmpty.isString());
+    EXPECT_EQ(fromEmpty.asString(), "");
+    EXPECT_TRUE(fromEmpty == ""sv);
+
+    // The view need not outlive the call.
+    json::Value copied;
+    {
+        std::string const owner{"borrowed"};
+        copied = json::Value{std::string_view{owner}};
+    }
+    EXPECT_EQ(copied.asString(), "borrowed");
+
+    // A reader of a string value stops at the first NUL, so a view carrying one reads back short.
+    json::Value const embedded{"a\0b"sv};
+    EXPECT_EQ(embedded.asString(), "a");
+    EXPECT_TRUE(embedded != "a\0b"sv);
+    EXPECT_TRUE(embedded == "a"sv);
+    EXPECT_TRUE(embedded == json::Value{"a\0b"sv});
+}
+
+TEST(JsonValue, compare_string_view)
+{
+    // Comparing against a view builds no temporary Value.
+    using namespace std::string_view_literals;
+
+    EXPECT_TRUE(json::Value{"2.0"} == "2.0"sv);
+    EXPECT_FALSE(json::Value{"2.0"} != "2.0"sv);
+    EXPECT_TRUE(json::Value{"2.0"} != "2.00"sv);
+    EXPECT_TRUE(json::Value{"2.0"} != ""sv);
+
+    // A view need not be NUL-terminated, and the comparison must not read past its end.
+    EXPECT_TRUE(json::Value{"2.0"} == "2.0 and more"sv.substr(0, 3));
+
+    EXPECT_TRUE(json::Value{""} == ""sv);
+
+    // A string value holding a null pointer equals nothing, not even the empty view.
+    EXPECT_TRUE(json::Value{json::ValueType::String} != ""sv);
+    EXPECT_TRUE(json::Value{json::ValueType::String} != "2.0"sv);
+    EXPECT_TRUE(json::Value{json::ValueType::String} != json::Value{""});
+
+    // Nothing that is not a string equals one. Null is what an absent member reads as.
+    EXPECT_TRUE(json::Value{json::ValueType::Null} != ""sv);
+    EXPECT_TRUE(json::Value{json::ValueType::Null} != "2.0"sv);
+    EXPECT_TRUE(json::Value{2} != "2"sv);
+    EXPECT_TRUE(json::Value{true} != "true"sv);
+    EXPECT_TRUE(json::Value{json::ValueType::Object} != ""sv);
+
+    // Only a view reaches this overload; every other spelling compares through Value.
+    static constexpr json::StaticString kVersion{"2.0"};
+    EXPECT_TRUE(json::Value{"2.0"} == kVersion);
+    EXPECT_TRUE(json::Value{"2.1"} != kVersion);
+    EXPECT_TRUE(json::Value{} != kVersion);
+
+    EXPECT_TRUE(json::Value{"2.0"} == "2.0");
+    EXPECT_TRUE(json::Value{"2.0"} != "2.1");
+    EXPECT_TRUE(json::Value{} != "2.0");
+
+    EXPECT_TRUE(json::Value{"2.0"} == std::string{"2.0"});
+    EXPECT_TRUE(json::Value{"2.0"} != std::string{"2.1"});
+    EXPECT_TRUE(json::Value{json::ValueType::Null} != std::string{"2.0"});
+
+    // A literal `0` also converts to a view through `char const*`, so a plain overload would make
+    // this comparison ambiguous. It must compare as a number: RPCCall.cpp rejects a
+    // `transaction_entry` ledger index this way.
+    EXPECT_TRUE(json::Value{0} == 0);
+    EXPECT_TRUE(json::Value{4294967295u} != 0);
+    EXPECT_TRUE(json::Value{""} != 0);
+    EXPECT_TRUE(json::Value{2} != 0);
+
+    // A null and an Int are different types, so the comparison is false.
+    EXPECT_TRUE(json::Value{} != 0);
+}
+
 TEST(JsonValue, bool)
 {
     EXPECT_FALSE(json::Value());

src/xrpld/app/misc/DeliverMax.h

@@ -10,8 +10,7 @@ namespace xrpl::rpc {
 
 /**
  * Copy `Amount` field to `DeliverMax` field in transaction output JSON.
- * This applies to a Payment and to each inner Payment in the
- * `RawTransactions` of a Batch; all other transaction types are ignored.
+ * This only applies to Payment transaction type, all others are ignored.
  *
  * When apiVersion > 1 will also remove `Amount` field, forcing users
  * to access this value using new `DeliverMax` field only.
@@ -23,17 +22,4 @@ insertDeliverMax(json::Value& txJson, TxType txnType, unsigned int apiVersion);
 
 /** @} */
 
-/**
- * Replace the `DeliverMax` alias with `Amount` in transaction input JSON.
- * This applies to a Payment and to each inner Payment in the
- * `RawTransactions` of a Batch; all other transaction types are ignored.
- * Malformed `RawTransactions` entries are left for the JSON parser to report.
- *
- * @param txJson The `tx_json` object of a request.
- * @return An `invalidParams` error if a Payment specifies differing `Amount`
- *         and `DeliverMax`, otherwise a null value.
- */
-json::Value
-replaceDeliverMaxWithAmount(json::Value& txJson);
-
 }  // namespace xrpl::rpc

src/xrpld/app/misc/NetworkOPs.cpp

@@ -3902,37 +3902,32 @@ NetworkOPsImp::pubProposedAccountTransaction(
     HashSet<InfoSub::pointer> notify;
     int iProposed = 0;
 
-    std::vector<SubAccountHistoryInfo> accountHistoryNotify;
-
     {
         std::scoped_lock const sl(accountLock_);
 
         if (subRTAccount_.empty())
             return;
 
-        if (!subAccount_.empty() || !subRTAccount_.empty() || !subAccountHistory_.empty())
+        for (auto const& affectedAccount : tx->getMentionedAccounts())
         {
-            for (auto const& affectedAccount : tx->getMentionedAccounts())
+            if (auto const simiIt = subRTAccount_.find(affectedAccount);
+                simiIt != subRTAccount_.end())
             {
-                if (auto const simiIt = subRTAccount_.find(affectedAccount);
-                    simiIt != subRTAccount_.end())
+                auto it = simiIt->second.begin();
+
+                while (it != simiIt->second.end())
                 {
-                    auto it = simiIt->second.begin();
+                    InfoSub::pointer const p = it->second.lock();
 
-                    while (it != simiIt->second.end())
+                    if (p)
                     {
-                        InfoSub::pointer const p = it->second.lock();
-
-                        if (p)
-                        {
-                            notify.insert(p);
-                            ++it;
-                            ++iProposed;
-                        }
-                        else
-                        {
-                            it = simiIt->second.erase(it);
-                        }
+                        notify.insert(p);
+                        ++it;
+                        ++iProposed;
+                    }
+                    else
+                    {
+                        it = simiIt->second.erase(it);
                     }
                 }
             }
@@ -3941,10 +3936,10 @@ NetworkOPsImp::pubProposedAccountTransaction(
 
     JLOG(journal_.trace()) << "pubProposedAccountTransaction: " << iProposed;
 
-    if (!notify.empty() || !accountHistoryNotify.empty())
+    if (!notify.empty())
     {
         // Create two different Json objects, for different API versions
-        MultiApiJson jvObj = transJson(tx, result, false, ledger, std::nullopt);
+        MultiApiJson const jvObj = transJson(tx, result, false, ledger, std::nullopt);
 
         for (InfoSub::Ref isrListener : notify)
         {
@@ -3952,21 +3947,6 @@ NetworkOPsImp::pubProposedAccountTransaction(
                 isrListener->getApiVersion(),  //
                 [&](json::Value const& jv) { isrListener->send(jv, true); });
         }
-
-        XRPL_ASSERT(
-            jvObj.isMember(jss::account_history_tx_stream) == MultiApiJson::IsMemberResult::None,
-            "xrpl::NetworkOPs::pubProposedAccountTransaction : "
-            "account_history_tx_stream not set");
-        for (auto& info : accountHistoryNotify)
-        {
-            auto const& index = info.index;
-            if (index->forwardTxIndex == 0 && !index->haveHistorical)
-                jvObj.set(jss::account_history_tx_first, true);
-            jvObj.set(jss::account_history_tx_index, index->forwardTxIndex++);
-            jvObj.visit(
-                info.sink->getApiVersion(),  //
-                [&](json::Value const& jv) { info.sink->send(jv, true); });
-        }
     }
 }
 

src/xrpld/app/misc/detail/DeliverMax.cpp

@@ -1,139 +1,22 @@
 #include <xrpld/app/misc/DeliverMax.h>
 
-#include <xrpl/beast/core/LexicalCast.h>
-#include <xrpl/json/json_forwards.h>
-#include <xrpl/json/json_value.h>
-#include <xrpl/protocol/ErrorCodes.h>
 #include <xrpl/protocol/TxFormats.h>
 #include <xrpl/protocol/jss.h>
 
-#include <cstdint>
-
 namespace xrpl::rpc {
 
-namespace {
-
-// Whether TransactionType is `type` in a form STParsedJSON accepts: its name,
-// its number, or its number as a string.
-bool
-isTxnType(json::Value const& txJson, TxType type)
-{
-    if (!txJson.isObject())
-        return false;
-
-    auto const& value = txJson[jss::TransactionType];
-    if (value.isInt())
-        return value.asInt() == type;
-    if (value.isUInt())
-        return value.asUInt() == static_cast<json::UInt>(type);
-    if (!value.isString())
-        return false;
-
-    auto const& str = value.asString();
-    if (str.empty() || str[0] < '0' || str[0] > '9')
-    {
-        auto const* const format = TxFormats::getInstance().findByType(type);
-        return format != nullptr && format->getName() == str;
-    }
-
-    std::uint16_t number = 0;
-    return beast::lexicalCastChecked(number, str) && number == type;
-}
-
-bool
-isPayment(json::Value const& txJson)
-{
-    return isTxnType(txJson, ttPAYMENT);
-}
-
-// Calls f on each inner transaction object of a Batch, whichever field wraps it.
-template <class F>
-void
-forEachInnerTxn(json::Value& txJson, F&& f)
-{
-    if (!txJson.isMember(jss::RawTransactions) || !txJson[jss::RawTransactions].isArray())
-        return;
-
-    for (auto& raw : txJson[jss::RawTransactions])
-    {
-        if (!raw.isObject())
-            continue;
-
-        for (auto& wrapped : raw)
-        {
-            if (wrapped.isObject())
-                f(wrapped);
-        }
-    }
-}
-
-void
-copyAmountToDeliverMax(json::Value& txJson, unsigned int apiVersion)
-{
-    if (!txJson.isMember(jss::Amount))
-        return;
-
-    txJson[jss::DeliverMax] = txJson[jss::Amount];
-    if (apiVersion > 1)
-        txJson.removeMember(jss::Amount);
-}
-
-json::Value
-replaceDeliverMaxInPayment(json::Value& txJson)
-{
-    if (!txJson.isMember(jss::DeliverMax))
-        return {};
-
-    if (txJson.isMember(jss::Amount))
-    {
-        if (txJson[jss::DeliverMax] != txJson[jss::Amount])
-        {
-            return makeError(
-                RpcInvalidParams, "Cannot specify differing 'Amount' and 'DeliverMax'");
-        }
-    }
-    else
-    {
-        txJson[jss::Amount] = txJson[jss::DeliverMax];
-    }
-
-    txJson.removeMember(jss::DeliverMax);
-    return {};
-}
-
-}  // namespace
-
 void
 insertDeliverMax(json::Value& txJson, TxType txnType, unsigned int apiVersion)
 {
-    if (txnType == ttPAYMENT)
+    if (txJson.isMember(jss::Amount))
     {
-        copyAmountToDeliverMax(txJson, apiVersion);
+        if (txnType == ttPAYMENT)
+        {
+            txJson[jss::DeliverMax] = txJson[jss::Amount];
+            if (apiVersion > 1)
+                txJson.removeMember(jss::Amount);
+        }
     }
-    else if (txnType == ttBATCH)
-    {
-        forEachInnerTxn(txJson, [apiVersion](json::Value& inner) {
-            if (isPayment(inner))
-                copyAmountToDeliverMax(inner, apiVersion);
-        });
-    }
-}
-
-json::Value
-replaceDeliverMaxWithAmount(json::Value& txJson)
-{
-    if (isPayment(txJson))
-        return replaceDeliverMaxInPayment(txJson);
-
-    json::Value error;
-    if (!isTxnType(txJson, ttBATCH))
-        return error;
-
-    forEachInnerTxn(txJson, [&error](json::Value& inner) {
-        if (error.isNull() && isPayment(inner))
-            error = replaceDeliverMaxInPayment(inner);
-    });
-    return error;
 }
 
 }  // namespace xrpl::rpc

src/xrpld/rpc/detail/ServerHandler.cpp

@@ -628,7 +628,9 @@ ServerHandler::processRequest(
 
     bool batch = false;
     unsigned size = 1;
-    if (jsonOrig.isMember(jss::method) && jsonOrig[jss::method] == "batch")
+    // Spelled as a view so the name is compared in place. A bare literal would reach the
+    // Value-to-Value comparison instead, which builds a Value from it, allocating per request.
+    if (jsonOrig.isMember(jss::method) && jsonOrig[jss::method] == std::string_view{"batch"})
     {
         batch = true;
         if (!jsonOrig.isMember(jss::params) || !jsonOrig[jss::params].isArray())

src/xrpld/rpc/detail/TransactionSign.cpp

@@ -211,6 +211,25 @@ checkPayment(
     if (txJson[jss::TransactionType].asString() != jss::Payment)
         return json::Value();
 
+    // DeliverMax is an alias to Amount and we use Amount internally
+    if (txJson.isMember(jss::DeliverMax))
+    {
+        if (txJson.isMember(jss::Amount))
+        {
+            if (txJson[jss::DeliverMax] != txJson[jss::Amount])
+            {
+                return rpc::makeError(
+                    RpcInvalidParams, "Cannot specify differing 'Amount' and 'DeliverMax'");
+            }
+        }
+        else
+        {
+            txJson[jss::Amount] = txJson[jss::DeliverMax];
+        }
+
+        txJson.removeMember(jss::DeliverMax);
+    }
+
     if (!txJson.isMember(jss::Amount))
         return rpc::missingFieldError("tx_json.Amount");
 
@@ -497,10 +516,6 @@ transactionPreProcessImpl(
     if (rpc::containsError(txJsonResult))
         return std::move(txJsonResult);
 
-    // Fee autofill parses tx_json, so the alias is resolved first.
-    if (auto err = rpc::replaceDeliverMaxWithAmount(txJson); rpc::containsError(err))
-        return err;
-
     // This test covers the case where we're offline so the sequence number
     // cannot be determined locally.  If we're offline then the caller must
     // provide the sequence number.
@@ -1311,9 +1326,6 @@ transactionSubmitMultiSigned(
     if (rpc::containsError(txJsonResult))
         return std::move(txJsonResult);
 
-    if (auto err = rpc::replaceDeliverMaxWithAmount(txJson); rpc::containsError(err))
-        return err;
-
     SLE::const_pointer const sle = ledger->read(keylet::account(srcAddressID));
 
     if (!sle)

src/xrpld/rpc/handlers/transaction/Simulate.cpp

@@ -1,6 +1,5 @@
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/ledger/OpenLedger.h>
-#include <xrpld/app/misc/DeliverMax.h>
 #include <xrpld/app/misc/Transaction.h>
 #include <xrpld/app/misc/TxQ.h>
 #include <xrpld/rpc/Context.h>
@@ -304,10 +303,6 @@ simulateTxn(rpc::JsonContext& context, std::shared_ptr<Transaction> transaction)
     else
     {
         jvResult[jss::tx_json] = transaction->getJson(JsonOptions::Values::None);
-        rpc::insertDeliverMax(
-            jvResult[jss::tx_json],
-            transaction->getSTransaction()->getTxnType(),
-            context.apiVersion);
     }
 
     return jvResult;
@@ -343,10 +338,6 @@ doSimulate(rpc::JsonContext& context)
     if (txJson.isMember(jss::error))
         return txJson;
 
-    // Fee autofill parses tx_json, so the alias is resolved first.
-    if (auto err = rpc::replaceDeliverMaxWithAmount(txJson); err.isMember(jss::error))
-        return err;
-
     // autofill fields if they're not included (e.g. `Fee`, `Sequence`)
     if (auto error = autofillTx(txJson, context))
         return *error;

src/xrpld/rpc/handlers/transaction/Submit.cpp

@@ -1,5 +1,4 @@
 #include <xrpld/app/ledger/LedgerMaster.h>
-#include <xrpld/app/misc/DeliverMax.h>
 #include <xrpld/app/misc/Transaction.h>
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/Role.h>
@@ -143,10 +142,6 @@ doSubmit(rpc::JsonContext& context)
     try
     {
         jvResult[jss::tx_json] = transaction->getJson(JsonOptions::Values::None);
-        rpc::insertDeliverMax(
-            jvResult[jss::tx_json],
-            transaction->getSTransaction()->getTxnType(),
-            context.apiVersion);
         jvResult[jss::tx_blob] = strHex(transaction->getSTransaction()->getSerializer().peekData());
 
         if (temUNCERTAIN != transaction->getResult())