docs: Link #8558 and name every transaction stream in the API changelog

API-CHANGELOG.md

@@ -33,6 +33,10 @@ 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)
 

src/test/app/Batch_test.cpp

@@ -42,6 +42,7 @@
 #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/Feature.h>
 #include <xrpl/protocol/Indexes.h>
@@ -6015,6 +6016,149 @@ class Batch_test : public beast::unit_test::Suite
         }
     }
 
+    void
+    testDeliverMax(FeatureBitset features)
+    {
+        testcase("DeliverMax in inner Payments");
+
+        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 signBatch = [&](json::Value const& first) {
+            json::Value txJson = batch::outer(alice, seq, XRPAmount{}, tfAllOrNothing);
+            txJson.removeMember(jss::Fee);
+            txJson[jss::RawTransactions][0u][jss::RawTransaction] = first;
+            txJson[jss::RawTransactions][1u][jss::RawTransaction] = 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][jss::RawTransaction];
+                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);
+
+        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)
     {
@@ -6056,6 +6200,7 @@ class Batch_test : public beast::unit_test::Suite
         testUnsortedBatchSigners(features);
         testBatchSigCache(features);
         testWrappedInnerSubmission(features);
+        testDeliverMax(features);
     }
 
 public:

src/test/rpc/Simulate_test.cpp

@@ -24,6 +24,7 @@
 #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>
@@ -1336,6 +1337,65 @@ 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
@@ -1354,6 +1414,7 @@ public:
         testDeleteExpiredCredentials();
         testSuccessfulTransactionNetworkID();
         testSuccessfulTransactionAdditionalMetadata();
+        testDeliverMax();
     }
 };
 

src/test/rpc/Submit_test.cpp

@@ -8,6 +8,7 @@
 #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/Seed.h>
 #include <xrpl/protocol/jss.h>
 
@@ -86,10 +87,37 @@ 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/xrpld/app/misc/DeliverMax.h

@@ -10,7 +10,8 @@ namespace xrpl::rpc {
 
 /**
  * Copy `Amount` field to `DeliverMax` field in transaction output JSON.
- * This only applies to Payment transaction type, all others are ignored.
+ * This applies to a Payment and to each inner Payment in the
+ * `RawTransactions` of a Batch; all other transaction types are ignored.
  *
  * When apiVersion > 1 will also remove `Amount` field, forcing users
  * to access this value using new `DeliverMax` field only.
@@ -22,4 +23,17 @@ 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
+removeDeliverMax(json::Value& txJson);
+
 }  // namespace xrpl::rpc

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

@@ -1,22 +1,109 @@
 #include <xrpld/app/misc/DeliverMax.h>
 
+#include <xrpl/json/json_value.h>
+#include <xrpl/protocol/ErrorCodes.h>
 #include <xrpl/protocol/TxFormats.h>
 #include <xrpl/protocol/jss.h>
 
 namespace xrpl::rpc {
 
-void
-insertDeliverMax(json::Value& txJson, TxType txnType, unsigned int apiVersion)
+namespace {
+
+bool
+isTxnType(json::Value const& txJson, json::StaticString const& name)
 {
-    if (txJson.isMember(jss::Amount))
+    return txJson.isObject() && txJson[jss::TransactionType].isString() &&
+        txJson[jss::TransactionType].asString() == name;
+}
+
+bool
+isPayment(json::Value const& txJson)
+{
+    return isTxnType(txJson, jss::Payment);
+}
+
+// Calls f on each inner transaction object of a Batch.
+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 (txnType == ttPAYMENT)
-        {
-            txJson[jss::DeliverMax] = txJson[jss::Amount];
-            if (apiVersion > 1)
-                txJson.removeMember(jss::Amount);
-        }
+        if (raw.isMember(jss::RawTransaction) && raw[jss::RawTransaction].isObject())
+            f(raw[jss::RawTransaction]);
     }
 }
 
+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
+moveDeliverMaxToAmount(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)
+    {
+        copyAmountToDeliverMax(txJson, apiVersion);
+    }
+    else if (txnType == ttBATCH)
+    {
+        forEachInnerTxn(txJson, [apiVersion](json::Value& inner) {
+            if (isPayment(inner))
+                copyAmountToDeliverMax(inner, apiVersion);
+        });
+    }
+}
+
+json::Value
+removeDeliverMax(json::Value& txJson)
+{
+    if (isPayment(txJson))
+        return moveDeliverMaxToAmount(txJson);
+
+    json::Value error;
+    if (!isTxnType(txJson, jss::Batch))
+        return error;
+
+    forEachInnerTxn(txJson, [&error](json::Value& inner) {
+        if (error.isNull() && isPayment(inner))
+            error = moveDeliverMaxToAmount(inner);
+    });
+    return error;
+}
+
 }  // namespace xrpl::rpc

src/xrpld/rpc/detail/TransactionSign.cpp

@@ -211,25 +211,6 @@ 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");
 
@@ -516,6 +497,10 @@ transactionPreProcessImpl(
     if (rpc::containsError(txJsonResult))
         return std::move(txJsonResult);
 
+    // Fee autofill parses tx_json, so the alias is resolved first.
+    if (auto err = rpc::removeDeliverMax(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.
@@ -1326,6 +1311,9 @@ transactionSubmitMultiSigned(
     if (rpc::containsError(txJsonResult))
         return std::move(txJsonResult);
 
+    if (auto err = rpc::removeDeliverMax(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,5 +1,6 @@
 #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>
@@ -302,6 +303,10 @@ 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;
@@ -337,6 +342,10 @@ 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::removeDeliverMax(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,4 +1,5 @@
 #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>
@@ -142,6 +143,10 @@ 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())