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

@@ -22,9 +22,10 @@ struct JsonOptions
         None = 0b0000'0000,
         IncludeDate = 0b0000'0001,
         DisableApiPriorV2 = 0b0000'0010,
+        DisableApiPriorV3 = 0b0000'0100,
 
         // IMPORTANT `All` must be union of all of the above; see also operator~
-        All = IncludeDate | DisableApiPriorV2  // 0b0000'0011
+        All = IncludeDate | DisableApiPriorV2 | DisableApiPriorV3  // 0b0000'0111
     };
 
     constexpr JsonOptions(underlying_t v) noexcept : value(v)

src/test/rpc/AccountTx_test.cpp

@@ -164,20 +164,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
                     {

src/test/rpc/Transaction_test.cpp

@@ -798,6 +798,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

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

src/xrpld/rpc/handlers/account/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>
@@ -17,6 +18,7 @@
 #include <xrpl/basics/chrono.h>
 #include <xrpl/basics/strHex.h>
 #include <xrpl/beast/utility/instrumentation.h>
+#include <xrpl/core/NetworkIDService.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/ledger/ReadView.h>
 #include <xrpl/protocol/AccountID.h>
@@ -309,12 +311,18 @@ populateJsonResponse(
                     auto const jsonTx = (context.apiVersion > 1 ? jss::tx_json : jss::tx);
                     if (context.apiVersion > 1)
                     {
-                        jvObj[jsonTx] = txn->getJson(
+                        static constexpr auto kOptionsJsonV2 =
                             static_cast<JsonOptions::underlying_t>(
                                 JsonOptions::Values::IncludeDate) |
-                                static_cast<JsonOptions::underlying_t>(
-                                    JsonOptions::Values::DisableApiPriorV2),
-                            false);
+                            static_cast<JsonOptions::underlying_t>(
+                                JsonOptions::Values::DisableApiPriorV2);
+                        static constexpr auto kOptionsJsonV3 =
+                            static_cast<JsonOptions::underlying_t>(
+                                JsonOptions::Values::DisableApiPriorV2) |
+                            static_cast<JsonOptions::underlying_t>(
+                                JsonOptions::Values::DisableApiPriorV3);
+                        auto const opts = context.apiVersion >= 3 ? kOptionsJsonV3 : kOptionsJsonV2;
+                        jvObj[jsonTx] = txn->getJson(opts, false);
                         jvObj[jss::hash] = to_string(txn->getID());
                         jvObj[jss::ledger_index] = txn->getLedger();
                         jvObj[jss::ledger_hash] =
@@ -322,7 +330,20 @@ populateJsonResponse(
 
                         if (auto closeTime =
                                 context.ledgerMaster.getCloseTimeBySeq(txn->getLedger()))
+                        {
                             jvObj[jss::close_time_iso] = toStringIso(*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
                     {

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

@@ -205,9 +205,13 @@ populateJsonResponse(
         auto const& sttx = result.txn->getSTransaction();
         if (context.apiVersion > 1)
         {
-            static constexpr auto kOptionsJson =
+            static constexpr auto kOptionsJsonV2 =
                 static_cast<JsonOptions::underlying_t>(JsonOptions::Values::IncludeDate) |
                 static_cast<JsonOptions::underlying_t>(JsonOptions::Values::DisableApiPriorV2);
+            static constexpr auto kOptionsJsonV3 =
+                static_cast<JsonOptions::underlying_t>(JsonOptions::Values::DisableApiPriorV2) |
+                static_cast<JsonOptions::underlying_t>(JsonOptions::Values::DisableApiPriorV3);
+            auto const kOptionsJson = context.apiVersion >= 3 ? kOptionsJsonV3 : kOptionsJsonV2;
             if (args.binary)
             {
                 response[jss::tx_blob] = result.txn->getJson(kOptionsJson, true);
@@ -229,7 +233,11 @@ populateJsonResponse(
             {
                 response[jss::ledger_index] = result.txn->getLedger();
                 if (result.closeTime)
+                {
                     response[jss::close_time_iso] = toStringIso(*result.closeTime);
+                    if (context.apiVersion >= 3)
+                        response[jss::date] = result.closeTime->time_since_epoch().count();
+                }
             }
         }
         else