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

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

.github/scripts/strategy-matrix/generate.py

@@ -32,13 +32,10 @@ We will further set additional CMake arguments as follows:
 """
 
 
-def generate_strategy_matrix(all: bool, config: Config, distro: str = "") -> list:
+def generate_strategy_matrix(all: bool, config: Config) -> list:
     configurations = []
-    os_entries = config.os
-    if distro:
-        os_entries = [o for o in os_entries if o["distro_name"] == distro]
     for architecture, os, build_type, cmake_args in itertools.product(
-        config.architecture, os_entries, config.build_type, config.cmake_args
+        config.architecture, config.os, config.build_type, config.cmake_args
     ):
         # The default CMake target is 'all' for Linux and MacOS and 'install'
         # for Windows, but it can get overridden for certain configurations.
@@ -226,7 +223,7 @@ def generate_strategy_matrix(all: bool, config: Config, distro: str = "") -> lis
         if (n := os["compiler_version"]) != "":
             config_name += f"-{n}"
         config_name += (
-            f"-{architecture['platform'][architecture['platform'].find('/') + 1 :]}"
+            f"-{architecture['platform'][architecture['platform'].find('/')+1:]}"
         )
         config_name += f"-{build_type.lower()}"
         if "-Dcoverage=ON" in cmake_args:
@@ -316,32 +313,21 @@ if __name__ == "__main__":
         required=False,
         type=Path,
     )
-    parser.add_argument(
-        "-d",
-        "--distro",
-        help="Filter OS entries to only include those with this distro_name (e.g. 'debian', 'rhel', 'ubuntu').",
-        required=False,
-        type=str,
-        default="",
-    )
     args = parser.parse_args()
 
     matrix = []
     if args.config is None or args.config == "":
         matrix += generate_strategy_matrix(
-            args.all, read_config(THIS_DIR / "linux.json"), args.distro
+            args.all, read_config(THIS_DIR / "linux.json")
         )
         matrix += generate_strategy_matrix(
-            args.all, read_config(THIS_DIR / "macos.json"), args.distro
+            args.all, read_config(THIS_DIR / "macos.json")
         )
         matrix += generate_strategy_matrix(
-            args.all, read_config(THIS_DIR / "windows.json"), args.distro
+            args.all, read_config(THIS_DIR / "windows.json")
         )
     else:
-        matrix += generate_strategy_matrix(
-            args.all, read_config(args.config), args.distro
-        )
+        matrix += generate_strategy_matrix(args.all, read_config(args.config))
 
     # Generate the strategy matrix.
     print(f"matrix={json.dumps({'include': matrix})}")
-    # print(json.dumps(matrix, indent=2))

.github/workflows/on-pr.yml

@@ -128,23 +128,12 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        include:
-          - os: linux
-            distro: debian
-          - os: linux
-            distro: rhel
-          - os: linux
-            distro: ubuntu
-          - os: macos
-            distro: ""
-          - os: windows
-            distro: ""
+        os: [linux, macos, windows]
     with:
       # Enable ccache only for events targeting the XRPLF repository, since
       # other accounts will not have access to our remote cache storage.
       ccache_enabled: ${{ github.repository_owner == 'XRPLF' }}
       os: ${{ matrix.os }}
-      distro: ${{ matrix.distro }}
     secrets:
       CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
 

.github/workflows/on-trigger.yml

@@ -77,17 +77,7 @@ jobs:
     strategy:
       fail-fast: ${{ github.event_name == 'merge_group' }}
       matrix:
-        include:
-          - os: linux
-            distro: debian
-          - os: linux
-            distro: rhel
-          - os: linux
-            distro: ubuntu
-          - os: macos
-            distro: ""
-          - os: windows
-            distro: ""
+        os: [linux, macos, windows]
     with:
       # Enable ccache only for events targeting the XRPLF repository, since
       # other accounts will not have access to our remote cache storage.
@@ -96,7 +86,6 @@ jobs:
       # not identical to a regular compilation.
       ccache_enabled: ${{ github.repository_owner == 'XRPLF' && !startsWith(github.ref, 'refs/heads/release') }}
       os: ${{ matrix.os }}
-      distro: ${{ matrix.distro }}
       strategy_matrix: ${{ github.event_name == 'schedule' && 'all' || 'minimal' }}
     secrets:
       CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

.github/workflows/reusable-build-test.yml

@@ -26,12 +26,6 @@ on:
         type: string
         default: "minimal"
 
-      distro:
-        description: 'Filter to only include configs for this distro (e.g. "debian", "rhel", "ubuntu"). Leave empty for no filtering.'
-        required: false
-        type: string
-        default: ""
-
     secrets:
       CODECOV_TOKEN:
         description: "The Codecov token to use for uploading coverage reports."
@@ -44,11 +38,9 @@ jobs:
     with:
       os: ${{ inputs.os }}
       strategy_matrix: ${{ inputs.strategy_matrix }}
-      distro: ${{ inputs.distro }}
 
   # Build and test the binary for each configuration.
   build-test-config:
-    name: ${{ matrix.config_name }}
     needs:
       - generate-matrix
     uses: ./.github/workflows/reusable-build-test-config.yml

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

@@ -13,11 +13,6 @@ on:
         required: false
         type: string
         default: "minimal"
-      distro:
-        description: 'Filter to only include configs for this distro (e.g. "debian", "rhel", "ubuntu"). Leave empty for no filtering.'
-        required: false
-        type: string
-        default: ""
     outputs:
       matrix:
         description: "The generated strategy matrix."
@@ -47,5 +42,4 @@ jobs:
         env:
           GENERATE_CONFIG: ${{ inputs.os != '' && format('--config={0}.json', inputs.os) || '' }}
           GENERATE_OPTION: ${{ inputs.strategy_matrix == 'all' && '--all' || '' }}
-          GENERATE_DISTRO: ${{ inputs.distro != '' && format('--distro={0}', inputs.distro) || '' }}
-        run: ./generate.py ${GENERATE_OPTION} ${GENERATE_CONFIG} ${GENERATE_DISTRO} >> "${GITHUB_OUTPUT}"
+        run: ./generate.py ${GENERATE_OPTION} ${GENERATE_CONFIG} >> "${GITHUB_OUTPUT}"

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