Skip to content

Fix: Remove non-canonical fields from tx_json in API v3 #6448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 5 commits into
base: develop
Choose a base branch
from
+127 −39
Open

Fix: Remove non-canonical fields from tx_json in API v3 #6448

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a31ee1
Initial plan
Copilot Feb 27, 2026
9b0e87a
Fix: remove non-canonical fields from tx_json in API v3
Copilot Feb 27, 2026
cf2835e
Fix date/ctid missing from result level in API v3, fix pre-commit errors
Copilot Feb 27, 2026
cbabee1
Merge branch 'develop' into copilot/remove-non-canonical-fields
mvadari Feb 27, 2026
249fb12
Merge branch 'develop' into copilot/remove-non-canonical-fields
mvadari Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions API-VERSION-3.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 versions 1 and 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 v1 and v2)**: The `tx_json` object in the response contained `date`, `ledger_index`, and `ctid` fields alongside the canonical PascalCase transaction fields.
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section says “In API versions 1 and 2, the tx_json field …”, but API v1 responses use tx (not tx_json) in these handlers. Consider rewording to avoid implying tx_json exists in v1 (e.g., refer to the transaction object in tx/account_tx, or limit the statement to API v2).

Suggested change
In API versions 1 and 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 v1 and v2)**: The `tx_json` object in the response contained `date`, `ledger_index`, and `ctid` fields alongside the canonical PascalCase transaction fields.
In API versions 1 and 2, the transaction object in `tx` and `account_tx` responses (`tx` in v1, `tx_json` in v2) 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 v1 and v2)**: The transaction object in the response (`tx` in v1, `tx_json` in v2) contained `date`, `ledger_index`, and `ctid` fields alongside the canonical PascalCase transaction fields.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cf2835e. The wording now says "In API version 2" (not "v1 and v2") since v1 uses tx (not tx_json).

- **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))
Expand Down
3 changes: 2 additions & 1 deletion include/xrpl/protocol/STBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand Down
57 changes: 43 additions & 14 deletions src/test/rpc/AccountTx_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,20 +122,49 @@ 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
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)) &&
(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");
}
Comment on lines 125 to 149
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For API v3 this test asserts date/ledger_index/ctid are absent from tx_json, but it doesn’t assert where date/ctid went. If the goal is to move these server-added fields to the transaction object level (outside tx_json), consider adding assertions that payment includes date (and ctid when available) for API v3 so the test fails if those fields are accidentally dropped.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cf2835e. Added assertions (payment.isMember(jss::date)) && (payment.isMember(jss::ctid)) to verify these fields exist at the transaction object level (outside tx_json) for API v3.

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;
Expand Down
17 changes: 17 additions & 0 deletions src/test/rpc/Transaction_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,23 @@ 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));
}
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));
}
Comment on lines 764 to 781
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new v3 assertions only check that date/ledger_index/ctid are absent from tx_json, but the intended behavior (and API v3 doc update) is that these server-added fields still exist at the result level. Since the handler changes currently remove date from tx_json without adding it elsewhere, this test would not catch a regression where date is accidentally dropped from the response. Consider asserting the expected result-level fields for API v3 (at least date, and ctid when applicable) in addition to the tx_json checks.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cf2835e. Added BEAST_EXPECT(result[jss::result].isMember(jss::date)) to assert date exists at the result level for API v3.

}

for (auto memberIt = expected.begin(); memberIt != expected.end(); memberIt++)
Expand Down
44 changes: 23 additions & 21 deletions src/xrpld/app/misc/detail/Transaction.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
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);

if (mTxnSeq && netID)
if (!(options & JsonOptions::disable_API_prior_V3))
{
std::optional<std::string> const ctid = RPC::encodeCTID(mLedgerIndex, *mTxnSeq, *netID);
if (ctid)
ret[jss::ctid] = *ctid;
ret[jss::ledger_index] = mLedgerIndex;

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);

if (mTxnSeq && netID)
{
std::optional<std::string> const ctid =
RPC::encodeCTID(mLedgerIndex, *mTxnSeq, *netID);
if (ctid)
ret[jss::ctid] = *ctid;
}
}
}

Expand Down
8 changes: 6 additions & 2 deletions src/xrpld/rpc/handlers/AccountTx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,12 @@ 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);
Comment on lines 291 to 294
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In API v3, opts includes disable_API_prior_V3, which prevents Transaction::getJson from injecting date/ledger_index/ctid into tx_json. This handler only adds ledger_index at the transaction-object level, so API v3 account_tx entries will lose date (and also ctid) entirely instead of moving them out of tx_json as described. Consider adding jvObj[jss::date] (from ledger close time) and jvObj[jss::ctid] (computed from ledger seq + transaction index + network ID, similar to tx) when apiVersion >= 3.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cf2835e. For API v3, added jvObj[jss::date] from the ledger close time and jvObj[jss::ctid] computed from ledger sequence, transaction index (txnMeta->getIndex()), and the server's network ID. Also added the required CTID.h and NetworkIDService.h includes.

jvObj[jss::hash] = to_string(txn->getID());
jvObj[jss::ledger_index] = txn->getLedger();
jvObj[jss::ledger_hash] =
Expand Down
8 changes: 7 additions & 1 deletion src/xrpld/rpc/handlers/Tx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment on lines +192 to +199
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For API v3 you’re passing optionsV3 without include_date, and Transaction::getJson is now suppressing date/ledger_index/ctid injection when disable_API_prior_V3 is set. Since Tx.cpp never adds date at the response/result level, this change makes the numeric date field disappear entirely from tx responses in API v3 (despite the intent/docs saying it should be at the result level). Consider explicitly adding result.date/response[jss::date] for v3+ (using the same close-time source as before) so the field is moved out of tx_json rather than removed.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in cf2835e. Added response[jss::date] = result.closeTime->time_since_epoch().count() at the result level in Tx.cpp for API v3+ when result.closeTime is available.

if (args.binary)
response[jss::tx_blob] = result.txn->getJson(optionsJson, true);
else
Expand Down
Loading