Fix: Remove non-canonical fields from tx_json in API v3#6448
Fix: Remove non-canonical fields from tx_json in API v3#6448
tx_json in API v3#6448Conversation
Co-authored-by: mvadari <8029314+mvadari@users.noreply.github.com>
tx_json in API v3
There was a problem hiding this comment.
Pull request overview
This PR updates XRPL’s versioned RPC JSON output so that, in API v3+, tx_json contains only canonical signed transaction fields, removing server-added lower-case fields that previously leaked into tx_json for tx and account_tx.
Changes:
- Add a new
JsonOptions::disable_API_prior_V3flag and use it in API v3+ to suppress server-addeddate/ledger_index/ctidinjection intotx_json. - Update
txandaccount_txhandlers to choose differentJsonOptionsfor API v2 vs v3+. - Add/adjust tests and update API v3 documentation to describe the breaking change.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| include/xrpl/protocol/STBase.h | Adds disable_API_prior_V3 JsonOptions bit and updates the mask. |
| src/xrpld/app/misc/detail/Transaction.cpp | Gates injection of ledger_index, date, and ctid behind !disable_API_prior_V3. |
| src/xrpld/rpc/handlers/Tx.cpp | Selects v2 vs v3+ JsonOptions when building tx_json. |
| src/xrpld/rpc/handlers/AccountTx.cpp | Selects v2 vs v3+ JsonOptions when building per-transaction tx_json. |
| src/test/rpc/Transaction_test.cpp | Adds assertions about presence/absence of server-added fields in tx_json across API versions. |
| src/test/rpc/AccountTx_test.cpp | Splits API v2/v3 expectations for server-added fields in tx_json. |
| API-VERSION-3.md | Documents the breaking change for v3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| 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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
API-VERSION-3.md
Outdated
| 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. |
There was a problem hiding this comment.
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).
| 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. |
There was a problem hiding this comment.
Fixed in cf2835e. The wording now says "In API version 2" (not "v1 and v2") since v1 uses tx (not tx_json).
| 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)); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Fixed in cf2835e. Added BEAST_EXPECT(result[jss::result].isMember(jss::date)) to assert date exists at the result level for API v3.
| 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"); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
@copilot please fix the pre-commit errors and respond to the review comments. You can use |
Co-authored-by: mvadari <8029314+mvadari@users.noreply.github.com>
Done in cf2835e. Fixed all pre-commit clang-format errors and addressed all review comments: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #6448 +/- ##
=======================================
Coverage 79.8% 79.8%
=======================================
Files 858 858
Lines 67757 67771 +14
Branches 7557 7556 -1
=======================================
+ Hits 54065 54080 +15
+ Misses 13692 13691 -1
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
In API v2,
txandaccount_txresponses include server-added lower-case fields (date,ledger_index,ctid) insidetx_jsonalongside canonical PascalCase transaction fields. In API v3, these fields must only appear at the top-level result object — a TODO inTransaction.cppalready noted this gap.High Level Overview of Change
For API v3,
tx_jsonnow contains only canonical signed transaction fields. Server-added metadata fields (date,ledger_index,ctid) are moved out oftx_jsonand appear exclusively at the top-level result object.Before (API v3, broken):
{ "result": { "ledger_index": 3885449, "tx_json": { "Account": "r...", "TransactionType": "Payment", "date": 804007520, "ledger_index": 3885449 } } }After (API v3, fixed):
{ "result": { "date": 804007520, "ledger_index": 3885449, "tx_json": { "Account": "r...", "TransactionType": "Payment" } } }API v2 behavior is unchanged.
Context of Change
Transaction::getJsonunconditionally addedledger_index,date, andctidto the returned JSON. A TODO comment explicitly called for adisable_API_prior_V3flag to suppress these fields. This PR implements that fix, ensuring the fields are moved to the result level rather than dropped entirely.Type of Change
API Impact
Changes
include/xrpl/protocol/STBase.h: Addeddisable_API_prior_V3 = 0b0000'0100toJsonOptions; updated_allmask accordingly.src/xrpld/app/misc/detail/Transaction.cpp: Wrappedledger_index,date, andctidinjection inif (!(options & JsonOptions::disable_API_prior_V3))— implements the existing TODO.src/xrpld/rpc/handlers/Tx.cpp: API v3+ usesdisable_API_prior_V2 | disable_API_prior_V3; API v2 retainsinclude_date | disable_API_prior_V2. For API v3+,dateis explicitly added at the result level fromresult.closeTime.src/xrpld/rpc/handlers/AccountTx.cpp: Same versioned options asTx.cpp. For API v3+,date(from ledger close time) andctid(computed from ledger seq, transaction index, and network ID) are explicitly added at the transaction-object level outsidetx_json. Added requiredCTID.handNetworkIDService.hincludes.src/test/rpc/Transaction_test.cpp: Assertsdate,ledger_index,ctidabsent fromtx_jsonin v3; assertsdatepresent at result level in v3; present intx_jsonin v2.src/test/rpc/AccountTx_test.cpp: Splitcase 2/case 3inhasTxswith assertions thatdateandctidare absent fromtx_jsonbut present at the transaction-object level in v3.API-VERSION-3.md: Documents breaking change with correct scope (API v2 only, not v1, since v1 usestxkey nottx_json).Original prompt
This section details on the original issue you should resolve
<issue_title>
txresponse should not include non-canonical fields intx_json</issue_title><issue_description>## Issue Description
#4775 was supposed to fix the format of transactions' JSON representation so that, among other things, server-added fields like
dateandledger_indexwere not included at the same level as canonical fields likeAccountandDeliverMaxthat are part of the signed transaction object.In API v2,
dateandledger_indexare still included in thetx_jsonnested object. Per the comments in PR #4775:However, even in API version 3 the fields are still there. (Also, I personally disagree with leaving them there in the first place—the point of the API versioning is to make breaking changes like this.)
Steps to Reproduce
Query a transaction using the
txmethod and"api_version": 3.Example
txWebSocket request [link]:{ "command": "tx", "transaction": "4483695FD3502B0536B4F1311E39F656442212A85EDDC72E6037253376CCDD33", "binary": false, "api_version": 3 }Expected Result
The
result.tx_jsonfield should contain only the canonical fields of the transaction, while the other lower-case fields (likehash,ledger_index,close_time_iso, etc.) are at theresultlevel.Actual Result
The
tx_jsonnested object contains adatefield (which should not exist) and aledger_indexfield (which is a duplicate of the one at the higher level).Example response:
{ "api_version": 2, "result": { "close_time_iso": "2025-06-23T15:25:20Z", "ctid": "C03B498900020002", "hash": "4483695FD3502B0536B4F1311E39F656442212A85EDDC72E6037253376CCDD33", "ledger_hash": "EBCF37756850E658E19B6DAEB3C3ECE3CE54B913D7F648A35AE5D185E43713BC", "ledger_index": 3885449, "meta": { "AffectedNodes": [ { "ModifiedNode": { "FinalFields": { "Account": "rDspBaumB5u6YWRdw5BWoBP3rcExFrXxoY", "Balance": "39999800", "Flags": 0, "OwnerCount": 1, "Sequence": 3885446 }, "LedgerEntryType": "AccountRoot", "LedgerIndex": "165D07DB16700C6AC3152DE1AB8AAD9F4275405BCFC48860A25C4BD1670DB086", "PreviousFields": { "Balance": "39999900", "Sequence": 3885445 }, "PreviousTxnID": "CE2A09315B7DFB3553C39A2C046EA400327DA0EC04E9CCB80D63B3DDF6FAED4C", "PreviousTxnLgrSeq": 3885446 } }, { "ModifiedNode": { "FinalFields": { "Flags": 32, "Issuer": "rDspBaumB5u6YWRdw5BWoBP3rcExFrXxoY", "MaximumAmount": "10000", "OutstandingAmount": "10000", "OwnerNode": "0", "Sequence": 3885444 }, "LedgerEntryType": "MPTokenIssuance", "LedgerIndex": "62D1A9121976428FBC2F363A90E7EE8B42C6767B6FDA39CEE9B7FF2416F77254", "PreviousFields": { "OutstandingAmount": "0" }, "PreviousTxnID": "CE2A09315B7DFB3553C39A2C046EA400327DA0EC04E9CCB80D63B3DDF6FAED4C", "PreviousTxnLgrSeq": 3885446 } }, { "ModifiedNode": { "FinalFields": { "Account": "rpjU8anTxTQX97GKHsuv8TyQ659j6pPak4", "Flags": 0, "MPTAmount": "10000", "MPTokenIssuanceID": "003B49848403524C52FC5B7E804DFE38271A5B1B3E46A93B", "OwnerNode": "0" }, "LedgerEntryType": "MPToken", "LedgerIndex": "DBDA63EDA5C34451B7C8F1E9A69F2EF28A6541C754F7E6CEC75354C8E894A106", "PreviousFields": {}, "PreviousTxnID": "0651B657A96690CDC4D2195ECA8E5BAEEBCEC463F4BFE862FD2BFEDF1C7EB2C0", "PreviousTxnLgrSeq": 3885447 } } ], "TransactionIndex": 2, "TransactionResult": "tesSUCCESS", "delivered_amount": { "mpt_issuance_id": "003B49848403524C52FC5B7E804DFE38271A5B1B3E46A93B", "value": "10000" } }, "tx_json": { "Account": "rDspBaumB5u6YWRdw5BWoBP3rcExFrXxoY", "DeliverMax": { "mpt_issuance_id": "003B49848403524C52FC5B7E804DFE38271A5B1B3E46A93B", "value": "10000" }, "Destination": "rpjU8anTxTQX97GKHsuv8TyQ659j6pPak4", "Fee": "100", "Sequence": 3885445, "Signing... </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes XRPLF/rippled#5507 <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/XRPLF/rippled/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.