Merge branch 'develop' into copilot/fix-vetoed-type-error

# Conflicts:
#	src/test/rpc/Feature_test.cpp

API-CHANGELOG.md

@@ -75,6 +75,10 @@ This release contains bug fixes only and no API changes.
 
 [Version 2.5.0](https://github.com/XRPLF/rippled/releases/tag/2.5.0) was released on Jun 24, 2025.
 
+### Breaking changes in 2.5.0
+
+- `feature`: In admin-mode responses, the `vetoed` field is now always a boolean. Disabled obsolete amendments now have `"vetoed": true` and a new `"obsolete": true` field, instead of the previous `"vetoed": "Obsolete"` string value. This change improves type safety for API clients while avoiding changes to non-admin responses.
+
 ### Additions and bugfixes in 2.5.0
 
 - `tx`: Added `ctid` field to the response and improved error handling. ([#4738](https://github.com/XRPLF/rippled/pull/4738))

API-VERSION-3.md

@@ -6,13 +6,6 @@ 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,10 +23,9 @@ 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'0111
+        _all                        = 0b0000'0011
         // clang-format on
     };
 

include/xrpl/protocol/jss.h

@@ -449,6 +449,7 @@ JSS(node_write_retries);      // out: GetCounts
 JSS(node_writes_delayed);     // out::GetCounts
 JSS(nth);                     // out: RPC server_definitions
 JSS(obligations);             // out: GatewayBalances
+JSS(obsolete);                // out: AmendmentTableImpl
 JSS(offers);                  // out: NetworkOPs, AccountOffers, Subscribe
 JSS(offer_id);                // out: insertNFTokenOfferID
 JSS(offline);                 // in: TransactionSign

src/test/rpc/AccountTx_test.cpp

@@ -122,52 +122,20 @@ class AccountTx_test : public beast::unit_test::suite
                     {
                         auto const& payment = j[jss::result][jss::transactions][1u];
 
-                        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");
-                        }
+                        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");
                     }
                     else
                         return false;

src/test/rpc/Feature_test.cpp

@@ -148,12 +148,20 @@ class Feature_test : public beast::unit_test::suite
                 feature.isMember(jss::enabled) && !feature[jss::enabled].asBool(),
                 feature[jss::name].asString() + " enabled");
             BEAST_EXPECTS(
-                feature.isMember(jss::vetoed) && feature[jss::vetoed].isBool() == !expectObsolete &&
-                    (!feature[jss::vetoed].isBool() ||
-                     feature[jss::vetoed].asBool() == expectVeto) &&
-                    (feature[jss::vetoed].isBool() ||
-                     feature[jss::vetoed].asString() == "Obsolete"),
-                feature[jss::name].asString() + " vetoed");
+                feature.isMember(jss::vetoed) && feature[jss::vetoed].isBool(),
+                feature[jss::name].asString() + " vetoed is bool");
+            BEAST_EXPECTS(
+                feature[jss::vetoed].asBool() == (expectVeto || expectObsolete),
+                feature[jss::name].asString() + " vetoed value");
+            if (expectObsolete)
+                BEAST_EXPECTS(
+                    feature.isMember(jss::obsolete) && feature[jss::obsolete].isBool() &&
+                        feature[jss::obsolete].asBool() == true,
+                    feature[jss::name].asString() + " obsolete");
+            else
+                BEAST_EXPECTS(
+                    !feature.isMember(jss::obsolete),
+                    feature[jss::name].asString() + " no obsolete");
             BEAST_EXPECTS(
                 feature.isMember(jss::supported) && feature[jss::supported].asBool(),
                 feature[jss::name].asString() + " supported");
@@ -253,6 +261,7 @@ class Feature_test : public beast::unit_test::suite
                         (*it)[jss::supported].asBool() == expectSupported,
                     (*it)[jss::name].asString() + " supported");
                 BEAST_EXPECT(!(*it).isMember(jss::vetoed));
+                BEAST_EXPECT(!(*it).isMember(jss::obsolete));
                 BEAST_EXPECT(!(*it).isMember(jss::majority));
                 BEAST_EXPECT(!(*it).isMember(jss::count));
                 BEAST_EXPECT(!(*it).isMember(jss::validations));
@@ -315,13 +324,23 @@ class Feature_test : public beast::unit_test::suite
                 BEAST_EXPECTS(
                     !(*it).isMember(jss::vetoed), (*it)[jss::name].asString() + " vetoed");
             else
+            {
                 BEAST_EXPECTS(
-                    (*it).isMember(jss::vetoed) && (*it)[jss::vetoed].isBool() == !expectObsolete &&
-                        (!(*it)[jss::vetoed].isBool() ||
-                         (*it)[jss::vetoed].asBool() == expectVeto) &&
-                        ((*it)[jss::vetoed].isBool() ||
-                         (*it)[jss::vetoed].asString() == "Obsolete"),
-                    (*it)[jss::name].asString() + " vetoed");
+                    (*it).isMember(jss::vetoed) && (*it)[jss::vetoed].isBool(),
+                    (*it)[jss::name].asString() + " vetoed is bool");
+                BEAST_EXPECTS(
+                    (*it)[jss::vetoed].asBool() == (expectVeto || expectObsolete),
+                    (*it)[jss::name].asString() + " vetoed value");
+                if (expectObsolete)
+                    BEAST_EXPECTS(
+                        (*it).isMember(jss::obsolete) && (*it)[jss::obsolete].isBool() &&
+                            (*it)[jss::obsolete].asBool() == true,
+                        (*it)[jss::name].asString() + " obsolete");
+                else
+                    BEAST_EXPECTS(
+                        !(*it).isMember(jss::obsolete),
+                        (*it)[jss::name].asString() + " no obsolete");
+            }
             BEAST_EXPECTS(
                 (*it).isMember(jss::supported) && (*it)[jss::supported].asBool() == expectSupported,
                 (*it)[jss::name].asString() + " supported");
@@ -389,12 +408,20 @@ class Feature_test : public beast::unit_test::suite
                 (expectVeto || expectObsolete) ^ feature.isMember(jss::majority),
                 feature[jss::name].asString() + " majority");
             BEAST_EXPECTS(
-                feature.isMember(jss::vetoed) && feature[jss::vetoed].isBool() == !expectObsolete &&
-                    (!feature[jss::vetoed].isBool() ||
-                     feature[jss::vetoed].asBool() == expectVeto) &&
-                    (feature[jss::vetoed].isBool() ||
-                     feature[jss::vetoed].asString() == "Obsolete"),
-                feature[jss::name].asString() + " vetoed");
+                feature.isMember(jss::vetoed) && feature[jss::vetoed].isBool(),
+                feature[jss::name].asString() + " vetoed is bool");
+            BEAST_EXPECTS(
+                feature[jss::vetoed].asBool() == (expectVeto || expectObsolete),
+                feature[jss::name].asString() + " vetoed value");
+            if (expectObsolete)
+                BEAST_EXPECTS(
+                    feature.isMember(jss::obsolete) && feature[jss::obsolete].isBool() &&
+                        feature[jss::obsolete].asBool() == true,
+                    feature[jss::name].asString() + " obsolete");
+            else
+                BEAST_EXPECTS(
+                    !feature.isMember(jss::obsolete),
+                    feature[jss::name].asString() + " no obsolete");
             BEAST_EXPECTS(feature.isMember(jss::count), feature[jss::name].asString() + " count");
             BEAST_EXPECTS(
                 feature.isMember(jss::threshold), feature[jss::name].asString() + " threshold");
@@ -485,8 +512,9 @@ class Feature_test : public beast::unit_test::suite
         auto feature = *(jrr.begin());
         BEAST_EXPECTS(feature[jss::name] == featureName, "name");
         BEAST_EXPECTS(
-            feature[jss::vetoed].isString() && feature[jss::vetoed].asString() == "Obsolete",
-            "vetoed");
+            feature[jss::vetoed].isBool() && feature[jss::vetoed].asBool() == true, "vetoed");
+        BEAST_EXPECTS(
+            feature[jss::obsolete].isBool() && feature[jss::obsolete].asBool() == true, "obsolete");
 
         jrr = env.rpc("feature", featureName, "reject")[jss::result];
         if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"))
@@ -497,8 +525,9 @@ class Feature_test : public beast::unit_test::suite
         feature = *(jrr.begin());
         BEAST_EXPECTS(feature[jss::name] == featureName, "name");
         BEAST_EXPECTS(
-            feature[jss::vetoed].isString() && feature[jss::vetoed].asString() == "Obsolete",
-            "vetoed");
+            feature[jss::vetoed].isBool() && feature[jss::vetoed].asBool() == true, "vetoed");
+        BEAST_EXPECTS(
+            feature[jss::obsolete].isBool() && feature[jss::obsolete].asBool() == true, "obsolete");
 
         jrr = env.rpc("feature", featureName, "accept")[jss::result];
         if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"))
@@ -509,8 +538,9 @@ class Feature_test : public beast::unit_test::suite
         feature = *(jrr.begin());
         BEAST_EXPECTS(feature[jss::name] == featureName, "name");
         BEAST_EXPECTS(
-            feature[jss::vetoed].isString() && feature[jss::vetoed].asString() == "Obsolete",
-            "vetoed");
+            feature[jss::vetoed].isBool() && feature[jss::vetoed].asBool() == true, "vetoed");
+        BEAST_EXPECTS(
+            feature[jss::obsolete].isBool() && feature[jss::obsolete].asBool() == true, "obsolete");
 
         // anything other than accept or reject is an error
         jrr = env.rpc("feature", featureName, "maybe");

src/test/rpc/Transaction_test.cpp

@@ -760,25 +760,6 @@ 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/AmendmentTable.cpp

@@ -931,7 +931,10 @@ AmendmentTableImpl::injectJson(
     if (!fs.enabled && isAdmin)
     {
         if (fs.vote == AmendmentVote::obsolete)
-            v[jss::vetoed] = "Obsolete";
+        {
+            v[jss::vetoed] = true;
+            v[jss::obsolete] = true;
+        }
         else
             v[jss::vetoed] = fs.vote == AmendmentVote::down;
     }

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

@@ -141,30 +141,28 @@ Transaction::getJson(JsonOptions options, bool binary) const
             ret[jss::inLedger] = mLedgerIndex;
         }
 
-        if (!(options & JsonOptions::disable_API_prior_V3))
+        // 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)
         {
-            ret[jss::ledger_index] = mLedgerIndex;
+            auto ct = mApp.getLedgerMaster().getCloseTimeBySeq(mLedgerIndex);
+            if (ct)
+                ret[jss::date] = ct->time_since_epoch().count();
+        }
 
-            if (options & JsonOptions::include_date)
-            {
-                auto ct = mApp.getLedgerMaster().getCloseTimeBySeq(mLedgerIndex);
-                if (ct)
-                    ret[jss::date] = ct->time_since_epoch().count();
-            }
+        // 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);
 
-            // 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;
-            }
+        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,7 +3,6 @@
 #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>
@@ -12,7 +11,6 @@
 #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>
@@ -288,10 +286,8 @@ populateJsonResponse(
                     auto const json_tx = (context.apiVersion > 1 ? jss::tx_json : jss::tx);
                     if (context.apiVersion > 1)
                     {
-                        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[json_tx] = txn->getJson(
+                            JsonOptions::include_date | JsonOptions::disable_API_prior_V2, false);
                         jvObj[jss::hash] = to_string(txn->getID());
                         jvObj[jss::ledger_index] = txn->getLedger();
                         jvObj[jss::ledger_hash] =
@@ -299,20 +295,7 @@ 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,14 +189,8 @@ populateJsonResponse(
         auto const& sttx = result.txn->getSTransaction();
         if (context.apiVersion > 1)
         {
-            // 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 =
+            constexpr auto optionsJson =
                 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
@@ -216,11 +210,7 @@ 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