Merge branch 'develop' into copilot/remove-non-canonical-fields

Co-authored-by: mvadari <8029314+mvadari@users.noreply.github.com>

API-VERSION-3.md

@@ -6,6 +6,13 @@ For info about how [API versioning](https://xrpl.org/request-formatting.html#api
 
 ## Breaking Changes
 
+### Modifications to `tx` and `account_tx`
+
+In API version 2, the `tx_json` field in `tx` and `account_tx` responses includes server-added lower-case fields (`date`, `ledger_index`, and `ctid`) that are not part of the canonical signed transaction. In API version 3, these fields are removed from `tx_json` and are only present at the top-level result object.
+
+- **Before (API v2)**: The `tx_json` object in the response contained `date`, `ledger_index`, and `ctid` fields alongside the canonical PascalCase transaction fields.
+- **After (API v3)**: The `tx_json` object contains only the canonical signed transaction fields. The `date`, `ledger_index`, and `ctid` fields appear exclusively at the top-level result object.
+
 ### Modifications to `amm_info`
 
 The order of error checks has been changed to provide more specific error messages. ([#4924](https://github.com/XRPLF/rippled/pull/4924))

include/xrpl/protocol/STBase.h

@@ -23,9 +23,10 @@ struct JsonOptions
         none                        = 0b0000'0000,
         include_date                = 0b0000'0001,
         disable_API_prior_V2        = 0b0000'0010,
+        disable_API_prior_V3        = 0b0000'0100,
 
         // IMPORTANT `_all` must be union of all of the above; see also operator~
-        _all                        = 0b0000'0011
+        _all                        = 0b0000'0111
         // clang-format on
     };
 

src/test/rpc/AccountTx_test.cpp

@@ -122,20 +122,52 @@ class AccountTx_test : public beast::unit_test::suite
                     {
                         auto const& payment = j[jss::result][jss::transactions][1u];
 
-                        return (payment.isMember(jss::tx_json)) &&
-                            (payment[jss::tx_json][jss::TransactionType] == jss::Payment) &&
-                            (payment[jss::tx_json][jss::DeliverMax] == "10000000010") &&
-                            (!payment[jss::tx_json].isMember(jss::Amount)) &&
-                            (!payment[jss::tx_json].isMember(jss::hash)) &&
-                            (payment[jss::hash] ==
-                             "9F3085D85F472D1CC29627F260DF68EDE59D42D1D0C33E345"
-                             "ECF0D4CE981D0A8") &&
-                            (payment[jss::validated] == true) &&
-                            (payment[jss::ledger_index] == 3) &&
-                            (payment[jss::ledger_hash] ==
-                             "5476DCD816EA04CBBA57D47BBF1FC58A5217CC93A5ADD79CB"
-                             "580A5AFDD727E33") &&
-                            (payment[jss::close_time_iso] == "2000-01-01T00:00:10Z");
+                        if (apiVersion >= 3)
+                        {
+                            // In API v3, server-added lower-case fields must
+                            // not be in tx_json, but must be at result level
+                            return (payment.isMember(jss::tx_json)) &&
+                                (payment[jss::tx_json][jss::TransactionType] == jss::Payment) &&
+                                (payment[jss::tx_json][jss::DeliverMax] == "10000000010") &&
+                                (!payment[jss::tx_json].isMember(jss::Amount)) &&
+                                (!payment[jss::tx_json].isMember(jss::hash)) &&
+                                (!payment[jss::tx_json].isMember(jss::date)) &&
+                                (!payment[jss::tx_json].isMember(jss::ledger_index)) &&
+                                (!payment[jss::tx_json].isMember(jss::ctid)) &&
+                                // date and ctid must be at the transaction
+                                // object level (outside tx_json) in API v3
+                                (payment.isMember(jss::date)) && (payment.isMember(jss::ctid)) &&
+                                (payment[jss::hash] ==
+                                 "9F3085D85F472D1CC29627F260DF68EDE59D42D1D0C33E345"
+                                 "ECF0D4CE981D0A8") &&
+                                (payment[jss::validated] == true) &&
+                                (payment[jss::ledger_index] == 3) &&
+                                (payment[jss::ledger_hash] ==
+                                 "5476DCD816EA04CBBA57D47BBF1FC58A5217CC93A5ADD79CB"
+                                 "580A5AFDD727E33") &&
+                                (payment[jss::close_time_iso] == "2000-01-01T00:00:10Z");
+                        }
+                        else
+                        {
+                            // In API v2, date and ledger_index are still in
+                            // tx_json for backwards compatibility
+                            return (payment.isMember(jss::tx_json)) &&
+                                (payment[jss::tx_json][jss::TransactionType] == jss::Payment) &&
+                                (payment[jss::tx_json][jss::DeliverMax] == "10000000010") &&
+                                (!payment[jss::tx_json].isMember(jss::Amount)) &&
+                                (!payment[jss::tx_json].isMember(jss::hash)) &&
+                                (payment[jss::tx_json].isMember(jss::date)) &&
+                                (payment[jss::tx_json].isMember(jss::ledger_index)) &&
+                                (payment[jss::hash] ==
+                                 "9F3085D85F472D1CC29627F260DF68EDE59D42D1D0C33E345"
+                                 "ECF0D4CE981D0A8") &&
+                                (payment[jss::validated] == true) &&
+                                (payment[jss::ledger_index] == 3) &&
+                                (payment[jss::ledger_hash] ==
+                                 "5476DCD816EA04CBBA57D47BBF1FC58A5217CC93A5ADD79CB"
+                                 "580A5AFDD727E33") &&
+                                (payment[jss::close_time_iso] == "2000-01-01T00:00:10Z");
+                        }
                     }
                     else
                         return false;

src/test/rpc/Transaction_test.cpp

@@ -760,6 +760,25 @@ class Transaction_test : public beast::unit_test::suite
                 result[jss::result][jss::ledger_hash] ==
                 "B41882E20F0EC6228417D28B9AE0F33833645D35F6799DFB782AC97FC4BB51"
                 "D2");
+
+            auto const& tx_json = result[jss::result][jss::tx_json];
+            if (apiVersion >= 3)
+            {
+                // In API v3, server-added lower-case fields must not appear
+                // inside tx_json; they are at the result level.
+                BEAST_EXPECT(!tx_json.isMember(jss::date));
+                BEAST_EXPECT(!tx_json.isMember(jss::ledger_index));
+                BEAST_EXPECT(!tx_json.isMember(jss::ctid));
+                // date must be at result level in API v3
+                BEAST_EXPECT(result[jss::result].isMember(jss::date));
+            }
+            else
+            {
+                // In API v2, date and ledger_index are still included in
+                // tx_json for backwards compatibility.
+                BEAST_EXPECT(tx_json.isMember(jss::date));
+                BEAST_EXPECT(tx_json.isMember(jss::ledger_index));
+            }
         }
 
         for (auto memberIt = expected.begin(); memberIt != expected.end(); memberIt++)

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

@@ -141,28 +141,30 @@ Transaction::getJson(JsonOptions options, bool binary) const
             ret[jss::inLedger] = mLedgerIndex;
         }
 
-        // TODO: disable_API_prior_V3 to disable output of both `date` and
-        // `ledger_index` elements (taking precedence over include_date)
-        ret[jss::ledger_index] = mLedgerIndex;
-
-        if (options & JsonOptions::include_date)
+        if (!(options & JsonOptions::disable_API_prior_V3))
         {
-            auto ct = mApp.getLedgerMaster().getCloseTimeBySeq(mLedgerIndex);
-            if (ct)
-                ret[jss::date] = ct->time_since_epoch().count();
-        }
+            ret[jss::ledger_index] = mLedgerIndex;
 
-        // compute outgoing CTID
-        // override local network id if it's explicitly in the txn
-        std::optional netID = mNetworkID;
-        if (mTransaction->isFieldPresent(sfNetworkID))
-            netID = mTransaction->getFieldU32(sfNetworkID);
+            if (options & JsonOptions::include_date)
+            {
+                auto ct = mApp.getLedgerMaster().getCloseTimeBySeq(mLedgerIndex);
+                if (ct)
+                    ret[jss::date] = ct->time_since_epoch().count();
+            }
 
-        if (mTxnSeq && netID)
-        {
-            std::optional<std::string> const ctid = RPC::encodeCTID(mLedgerIndex, *mTxnSeq, *netID);
-            if (ctid)
-                ret[jss::ctid] = *ctid;
+            // compute outgoing CTID
+            // override local network id if it's explicitly in the txn
+            std::optional netID = mNetworkID;
+            if (mTransaction->isFieldPresent(sfNetworkID))
+                netID = mTransaction->getFieldU32(sfNetworkID);
+
+            if (mTxnSeq && netID)
+            {
+                std::optional<std::string> const ctid =
+                    RPC::encodeCTID(mLedgerIndex, *mTxnSeq, *netID);
+                if (ctid)
+                    ret[jss::ctid] = *ctid;
+            }
         }
     }
 

src/xrpld/rpc/handlers/AccountTx.cpp

@@ -3,6 +3,7 @@
 #include <xrpld/app/misc/DeliverMax.h>
 #include <xrpld/app/misc/Transaction.h>
 #include <xrpld/app/rdb/backend/SQLiteDatabase.h>
+#include <xrpld/rpc/CTID.h>
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/DeliveredAmount.h>
 #include <xrpld/rpc/MPTokenIssuanceID.h>
@@ -11,6 +12,7 @@
 #include <xrpld/rpc/detail/RPCLedgerHelpers.h>
 #include <xrpld/rpc/detail/Tuning.h>
 
+#include <xrpl/core/NetworkIDService.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/protocol/ErrorCodes.h>
@@ -286,8 +288,10 @@ populateJsonResponse(
                     auto const json_tx = (context.apiVersion > 1 ? jss::tx_json : jss::tx);
                     if (context.apiVersion > 1)
                     {
-                        jvObj[json_tx] = txn->getJson(
-                            JsonOptions::include_date | JsonOptions::disable_API_prior_V2, false);
+                        auto const opts = context.apiVersion >= 3
+                            ? JsonOptions::disable_API_prior_V2 | JsonOptions::disable_API_prior_V3
+                            : JsonOptions::include_date | JsonOptions::disable_API_prior_V2;
+                        jvObj[json_tx] = txn->getJson(opts, false);
                         jvObj[jss::hash] = to_string(txn->getID());
                         jvObj[jss::ledger_index] = txn->getLedger();
                         jvObj[jss::ledger_hash] =
@@ -295,7 +299,20 @@ populateJsonResponse(
 
                         if (auto closeTime =
                                 context.ledgerMaster.getCloseTimeBySeq(txn->getLedger()))
+                        {
                             jvObj[jss::close_time_iso] = to_string_iso(*closeTime);
+                            if (context.apiVersion >= 3)
+                                jvObj[jss::date] = closeTime->time_since_epoch().count();
+                        }
+
+                        if (context.apiVersion >= 3 && txnMeta)
+                        {
+                            uint32_t const lgrSeq = txn->getLedger();
+                            uint32_t const txnIdx = txnMeta->getIndex();
+                            uint32_t const netID = context.app.getNetworkIDService().getNetworkID();
+                            if (auto const ctid = RPC::encodeCTID(lgrSeq, txnIdx, netID))
+                                jvObj[jss::ctid] = *ctid;
+                        }
                     }
                     else
                         jvObj[json_tx] = txn->getJson(JsonOptions::include_date);

src/xrpld/rpc/handlers/Tx.cpp

@@ -189,8 +189,14 @@ populateJsonResponse(
         auto const& sttx = result.txn->getSTransaction();
         if (context.apiVersion > 1)
         {
-            constexpr auto optionsJson =
+            // In API v2, include_date and disable_API_prior_V2 are used to
+            // include date/ledger_index/ctid in tx_json. In API v3+, those
+            // fields are excluded from tx_json and are only at result level.
+            constexpr auto optionsV2 =
                 JsonOptions::include_date | JsonOptions::disable_API_prior_V2;
+            constexpr auto optionsV3 =
+                JsonOptions::disable_API_prior_V2 | JsonOptions::disable_API_prior_V3;
+            auto const optionsJson = context.apiVersion >= 3 ? optionsV3 : optionsV2;
             if (args.binary)
                 response[jss::tx_blob] = result.txn->getJson(optionsJson, true);
             else
@@ -210,7 +216,11 @@ populateJsonResponse(
             {
                 response[jss::ledger_index] = result.txn->getLedger();
                 if (result.closeTime)
+                {
                     response[jss::close_time_iso] = to_string_iso(*result.closeTime);
+                    if (context.apiVersion >= 3)
+                        response[jss::date] = result.closeTime->time_since_epoch().count();
+                }
             }
         }
         else