From 7ac7e14d66d1bfdb48f6b5f422dea6219dcbb583 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 20 Aug 2025 21:42:11 +0000 Subject: [PATCH] Fix missing nftoken_id in ledger RPC response --- .../xrpl/protocol/NFTSyntheticSerializer.h | 2 - .../protocol/NFTSyntheticSerializer.cpp | 6 +- src/test/app/NFToken_test.cpp | 187 +++++++++++++++++- src/xrpld/app/ledger/detail/LedgerToJson.cpp | 42 ++-- src/xrpld/app/misc/NetworkOPs.cpp | 7 +- src/xrpld/rpc/detail/SyntheticFields.cpp | 38 ++++ src/xrpld/rpc/detail/SyntheticFields.h | 39 ++++ src/xrpld/rpc/handlers/account/AccountTx.cpp | 7 +- .../rpc/handlers/transaction/Simulate.cpp | 7 +- src/xrpld/rpc/handlers/transaction/Tx.cpp | 7 +- 10 files changed, 281 insertions(+), 61 deletions(-) create mode 100644 src/xrpld/rpc/detail/SyntheticFields.cpp create mode 100644 src/xrpld/rpc/detail/SyntheticFields.h diff --git a/include/xrpl/protocol/NFTSyntheticSerializer.h b/include/xrpl/protocol/NFTSyntheticSerializer.h index bef05b9a8f..2b1888cae9 100644 --- a/include/xrpl/protocol/NFTSyntheticSerializer.h +++ b/include/xrpl/protocol/NFTSyntheticSerializer.h @@ -11,9 +11,7 @@ namespace xrpl::RPC { /** * Adds common synthetic fields to transaction-related JSON responses */ -/** @{ */ void insertNFTSyntheticInJson(json::Value&, std::shared_ptr const&, TxMeta const&); -/** @} */ } // namespace xrpl::RPC diff --git a/src/libxrpl/protocol/NFTSyntheticSerializer.cpp b/src/libxrpl/protocol/NFTSyntheticSerializer.cpp index 4f0a2d5071..2c0e77408a 100644 --- a/src/libxrpl/protocol/NFTSyntheticSerializer.cpp +++ b/src/libxrpl/protocol/NFTSyntheticSerializer.cpp @@ -13,12 +13,12 @@ namespace xrpl::RPC { void insertNFTSyntheticInJson( - json::Value& response, + json::Value& metadata, std::shared_ptr const& transaction, TxMeta const& transactionMeta) { - insertNFTokenID(response[jss::meta], transaction, transactionMeta); - insertNFTokenOfferID(response[jss::meta], transaction, transactionMeta); + insertNFTokenID(metadata, transaction, transactionMeta); + insertNFTokenOfferID(metadata, transaction, transactionMeta); } } // namespace xrpl::RPC diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index acd54ae26a..d056381eb2 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6119,17 +6119,88 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite env.tx()->getJson(JsonOptions::Values::None)[jss::hash].asString()}; env.close(); - json::Value const meta = env.rpc("tx", txHash)[jss::result][jss::meta]; - // Expect nftokens_id field - if (!BEAST_EXPECT(meta.isMember(jss::nftoken_id))) + // Test 1: Check tx RPC response + json::Value const txResult = env.rpc("tx", txHash)[jss::result]; + json::Value const& txMeta = txResult[jss::meta]; + + // Expect nftoken_id field + if (!BEAST_EXPECT(txMeta.isMember(jss::nftoken_id))) return; - // Check the value of NFT ID in the meta with the - // actual value + // Check the value of NFT ID matches uint256 nftID; - BEAST_EXPECT(nftID.parseHex(meta[jss::nftoken_id].asString())); + BEAST_EXPECT(nftID.parseHex(txMeta[jss::nftoken_id].asString())); BEAST_EXPECT(nftID == actualNftID); + + // Get ledger sequence from tx response + auto const ledgerSeq = txResult[jss::ledger_index].asUInt(); + + // Test 2: Check ledger RPC response with expanded transactions + json::Value ledgerParams; + ledgerParams[jss::ledger_index] = ledgerSeq; + ledgerParams[jss::transactions] = true; + ledgerParams[jss::expand] = true; + + auto const ledgerResult = env.rpc("json", "ledger", to_string(ledgerParams)); + auto const& tx = ledgerResult[jss::result][jss::ledger][jss::transactions][0u]; + + // Verify transaction hash matches + BEAST_EXPECT(tx[jss::hash].asString() == txHash); + + // Check synthetic fields in ledger response (this tests our + // LedgerToJson.cpp fix) + json::Value const* meta = nullptr; + if (tx.isMember(jss::meta)) + meta = &tx[jss::meta]; + else if (tx.isMember(jss::metaData)) + meta = &tx[jss::metaData]; + + if (BEAST_EXPECT(meta != nullptr)) + { + BEAST_EXPECT(meta->isMember(jss::nftoken_id)); + if (meta->isMember(jss::nftoken_id)) + { + uint256 ledgerNftId; + BEAST_EXPECT(ledgerNftId.parseHex((*meta)[jss::nftoken_id].asString())); + BEAST_EXPECT(ledgerNftId == actualNftID); + } + } + + // Test 3: Check account_tx RPC response + json::Value accountTxParams; + accountTxParams[jss::account] = alice.human(); + accountTxParams[jss::limit] = 1; + + auto const accountTxResult = env.rpc("json", "account_tx", to_string(accountTxParams)); + auto const& accountTx = accountTxResult[jss::result][jss::transactions][0u]; + + // Check if the latest transaction is ours (it should be, but + // account_tx can be ordering-dependent) + bool const isOurTransaction = (accountTx[jss::hash].asString() == txHash); + + // Only check synthetic fields if this is our transaction + if (isOurTransaction) + { + // Check synthetic fields in account_tx response + json::Value const* accountMeta = nullptr; + if (accountTx.isMember(jss::meta)) + accountMeta = &accountTx[jss::meta]; + else if (accountTx.isMember(jss::metaData)) + accountMeta = &accountTx[jss::metaData]; + + if (BEAST_EXPECT(accountMeta != nullptr)) + { + BEAST_EXPECT(accountMeta->isMember(jss::nftoken_id)); + if (accountMeta->isMember(jss::nftoken_id)) + { + uint256 accountNftId; + BEAST_EXPECT( + accountNftId.parseHex((*accountMeta)[jss::nftoken_id].asString())); + BEAST_EXPECT(accountNftId == actualNftID); + } + } + } }; // Verify `nftoken_ids` value equals to the NFTokenIDs that were @@ -6140,17 +6211,20 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite env.tx()->getJson(JsonOptions::Values::None)[jss::hash].asString()}; env.close(); - json::Value const meta = env.rpc("tx", txHash)[jss::result][jss::meta]; + + // Test 1: Check tx RPC response + json::Value const txResult = env.rpc("tx", txHash)[jss::result]; + json::Value const& txMeta = txResult[jss::meta]; // Expect nftokens_ids field and verify the values - if (!BEAST_EXPECT(meta.isMember(jss::nftoken_ids))) + if (!BEAST_EXPECT(txMeta.isMember(jss::nftoken_ids))) return; // Convert NFT IDs from json::Value to uint256 std::vector metaIDs; std::transform( - meta[jss::nftoken_ids].begin(), - meta[jss::nftoken_ids].end(), + txMeta[jss::nftoken_ids].begin(), + txMeta[jss::nftoken_ids].end(), std::back_inserter(metaIDs), [this](json::Value id) { uint256 nftID; @@ -6169,6 +6243,99 @@ class NFTokenBaseUtil_test : public beast::unit_test::Suite // actual values for (size_t i = 0; i < metaIDs.size(); ++i) BEAST_EXPECT(metaIDs[i] == actualNftIDs[i]); + + // Get ledger sequence from tx response + auto const ledgerSeq = txResult[jss::ledger_index].asUInt(); + + // Test 2: Check ledger RPC response with expanded transactions + json::Value ledgerParams; + ledgerParams[jss::ledger_index] = ledgerSeq; + ledgerParams[jss::transactions] = true; + ledgerParams[jss::expand] = true; + + auto const ledgerResult = env.rpc("json", "ledger", to_string(ledgerParams)); + auto const& tx = ledgerResult[jss::result][jss::ledger][jss::transactions][0u]; + + // Verify transaction hash matches + BEAST_EXPECT(tx[jss::hash].asString() == txHash); + + // Check synthetic fields in ledger response + json::Value const* meta = nullptr; + if (tx.isMember(jss::meta)) + meta = &tx[jss::meta]; + else if (tx.isMember(jss::metaData)) + meta = &tx[jss::metaData]; + + if (BEAST_EXPECT(meta != nullptr)) + { + BEAST_EXPECT(meta->isMember(jss::nftoken_ids)); + if (meta->isMember(jss::nftoken_ids)) + { + // Convert and verify NFT IDs in ledger response + std::vector ledgerMetaIDs; + std::transform( + (*meta)[jss::nftoken_ids].begin(), + (*meta)[jss::nftoken_ids].end(), + std::back_inserter(ledgerMetaIDs), + [this](json::Value id) { + uint256 nftID; + BEAST_EXPECT(nftID.parseHex(id.asString())); + return nftID; + }); + + std::sort(ledgerMetaIDs.begin(), ledgerMetaIDs.end()); + BEAST_EXPECT(ledgerMetaIDs.size() == actualNftIDs.size()); + for (size_t i = 0; i < ledgerMetaIDs.size(); ++i) + BEAST_EXPECT(ledgerMetaIDs[i] == actualNftIDs[i]); + } + } + + // Test 3: Check account_tx RPC response + json::Value accountTxParams; + accountTxParams[jss::account] = alice.human(); + accountTxParams[jss::limit] = 1; + + auto const accountTxResult = env.rpc("json", "account_tx", to_string(accountTxParams)); + auto const& accountTx = accountTxResult[jss::result][jss::transactions][0u]; + + // Check if the latest transaction is ours (it should be, but + // account_tx can be ordering-dependent) + bool const isOurTransaction = (accountTx[jss::hash].asString() == txHash); + + // Only check synthetic fields if this is our transaction + if (isOurTransaction) + { + // Check synthetic fields in account_tx response + json::Value const* accountMeta = nullptr; + if (accountTx.isMember(jss::meta)) + accountMeta = &accountTx[jss::meta]; + else if (accountTx.isMember(jss::metaData)) + accountMeta = &accountTx[jss::metaData]; + + if (BEAST_EXPECT(accountMeta != nullptr)) + { + BEAST_EXPECT(accountMeta->isMember(jss::nftoken_ids)); + if (accountMeta->isMember(jss::nftoken_ids)) + { + // Convert and verify NFT IDs in account_tx response + std::vector accountMetaIDs; + std::transform( + (*accountMeta)[jss::nftoken_ids].begin(), + (*accountMeta)[jss::nftoken_ids].end(), + std::back_inserter(accountMetaIDs), + [this](json::Value id) { + uint256 nftID; + BEAST_EXPECT(nftID.parseHex(id.asString())); + return nftID; + }); + + std::sort(accountMetaIDs.begin(), accountMetaIDs.end()); + BEAST_EXPECT(accountMetaIDs.size() == actualNftIDs.size()); + for (size_t i = 0; i < accountMetaIDs.size(); ++i) + BEAST_EXPECT(accountMetaIDs[i] == actualNftIDs[i]); + } + } + } }; // Verify `offer_id` value equals to the offerID that was diff --git a/src/xrpld/app/ledger/detail/LedgerToJson.cpp b/src/xrpld/app/ledger/detail/LedgerToJson.cpp index 7a581e2389..8e5c8009dc 100644 --- a/src/xrpld/app/ledger/detail/LedgerToJson.cpp +++ b/src/xrpld/app/ledger/detail/LedgerToJson.cpp @@ -4,8 +4,7 @@ #include #include #include -#include -#include +#include #include #include @@ -17,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -141,19 +141,12 @@ fillJsonTx( { txJson[jss::meta] = stMeta->getJson(JsonOptions::Values::None); - // If applicable, insert delivered amount - if (txnType == ttPAYMENT || txnType == ttCHECK_CASH) - { - RPC::insertDeliveredAmount( - txJson[jss::meta], - fill.ledger, - txn, - {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); - } - - // If applicable, insert mpt issuance id - RPC::insertMPTokenIssuanceID( - txJson[jss::meta], txn, {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); + // Insert all synthetic fields + RPC::insertAllSyntheticInJson( + txJson[jss::meta], + fill.ledger, + txn, + {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); } if (!fill.ledger.open()) @@ -177,19 +170,12 @@ fillJsonTx( { txJson[jss::metaData] = stMeta->getJson(JsonOptions::Values::None); - // If applicable, insert delivered amount - if (txnType == ttPAYMENT || txnType == ttCHECK_CASH) - { - RPC::insertDeliveredAmount( - txJson[jss::metaData], - fill.ledger, - txn, - {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); - } - - // If applicable, insert mpt issuance id - RPC::insertMPTokenIssuanceID( - txJson[jss::metaData], txn, {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); + // Insert all synthetic fields + RPC::insertAllSyntheticInJson( + txJson[jss::metaData], + fill.ledger, + txn, + {txn->getTransactionID(), fill.ledger.seq(), *stMeta}); } } diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 4b0091dff6..9d62ec2afb 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -30,9 +30,8 @@ #include #include #include -#include -#include #include +#include #include #include @@ -3281,9 +3280,7 @@ NetworkOPsImp::transJson( if (meta) { jvObj[jss::meta] = meta->get().getJson(JsonOptions::Values::None); - RPC::insertDeliveredAmount(jvObj[jss::meta], *ledger, transaction, meta->get()); - RPC::insertNFTSyntheticInJson(jvObj, transaction, meta->get()); - RPC::insertMPTokenIssuanceID(jvObj[jss::meta], transaction, meta->get()); + RPC::insertAllSyntheticInJson(jvObj[jss::meta], *ledger, transaction, meta->get()); } // add CTID where the needed data for it exists diff --git a/src/xrpld/rpc/detail/SyntheticFields.cpp b/src/xrpld/rpc/detail/SyntheticFields.cpp new file mode 100644 index 0000000000..37706e7e34 --- /dev/null +++ b/src/xrpld/rpc/detail/SyntheticFields.cpp @@ -0,0 +1,38 @@ +#include + +#include +#include + +#include +#include +#include + +namespace xrpl { +namespace RPC { + +void +insertAllSyntheticInJson( + json::Value& metadata, + ReadView const& ledger, + std::shared_ptr const& transaction, + TxMeta const& transactionMeta) +{ + insertDeliveredAmount(metadata, ledger, transaction, transactionMeta); + insertNFTSyntheticInJson(metadata, transaction, transactionMeta); + insertMPTokenIssuanceID(metadata, transaction, transactionMeta); +} + +void +insertAllSyntheticInJson( + json::Value& metadata, + JsonContext const& context, + std::shared_ptr const& transaction, + TxMeta const& transactionMeta) +{ + insertDeliveredAmount(metadata, context, transaction, transactionMeta); + insertNFTSyntheticInJson(metadata, transaction, transactionMeta); + insertMPTokenIssuanceID(metadata, transaction, transactionMeta); +} + +} // namespace RPC +} // namespace xrpl diff --git a/src/xrpld/rpc/detail/SyntheticFields.h b/src/xrpld/rpc/detail/SyntheticFields.h new file mode 100644 index 0000000000..bfdb9ecedd --- /dev/null +++ b/src/xrpld/rpc/detail/SyntheticFields.h @@ -0,0 +1,39 @@ +#pragma once + +#include +#include +#include + +#include + +namespace xrpl { + +class ReadView; + +namespace RPC { + +struct JsonContext; + +/** + * Adds all synthetic fields to transaction metadata JSON. + * This includes delivered amount, NFT synthetic fields, and MPToken issuance + * ID. + */ +/** @{ */ +void +insertAllSyntheticInJson( + json::Value& metadata, + ReadView const&, + std::shared_ptr const&, + TxMeta const&); + +void +insertAllSyntheticInJson( + json::Value& metadata, + JsonContext const&, + std::shared_ptr const&, + TxMeta const&); +/** @} */ + +} // namespace RPC +} // namespace xrpl diff --git a/src/xrpld/rpc/handlers/account/AccountTx.cpp b/src/xrpld/rpc/handlers/account/AccountTx.cpp index c43f560861..9d10b33bc3 100644 --- a/src/xrpld/rpc/handlers/account/AccountTx.cpp +++ b/src/xrpld/rpc/handlers/account/AccountTx.cpp @@ -4,12 +4,11 @@ #include #include #include -#include -#include #include #include #include #include +#include #include #include @@ -378,9 +377,7 @@ populateJsonResponse( if (txnMeta) { jvObj[jss::meta] = txnMeta->getJson(JsonOptions::Values::IncludeDate); - insertDeliveredAmount(jvObj[jss::meta], context, txn, *txnMeta); - RPC::insertNFTSyntheticInJson(jvObj, sttx, *txnMeta); - RPC::insertMPTokenIssuanceID(jvObj[jss::meta], sttx, *txnMeta); + RPC::insertAllSyntheticInJson(jvObj[jss::meta], context, sttx, *txnMeta); } else { diff --git a/src/xrpld/rpc/handlers/transaction/Simulate.cpp b/src/xrpld/rpc/handlers/transaction/Simulate.cpp index 0f163c7356..1eb3c67c3d 100644 --- a/src/xrpld/rpc/handlers/transaction/Simulate.cpp +++ b/src/xrpld/rpc/handlers/transaction/Simulate.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -290,12 +291,8 @@ simulateTxn(RPC::JsonContext& context, std::shared_ptr transaction) else { jvResult[jss::meta] = result.metadata->getJson(JsonOptions::Values::None); - RPC::insertDeliveredAmount( + RPC::insertAllSyntheticInJson( jvResult[jss::meta], view, transaction->getSTransaction(), *result.metadata); - RPC::insertNFTSyntheticInJson( - jvResult, transaction->getSTransaction(), *result.metadata); - RPC::insertMPTokenIssuanceID( - jvResult[jss::meta], transaction->getSTransaction(), *result.metadata); } } diff --git a/src/xrpld/rpc/handlers/transaction/Tx.cpp b/src/xrpld/rpc/handlers/transaction/Tx.cpp index c065bc268e..2a6428909c 100644 --- a/src/xrpld/rpc/handlers/transaction/Tx.cpp +++ b/src/xrpld/rpc/handlers/transaction/Tx.cpp @@ -5,8 +5,11 @@ #include #include #include +#include #include #include +#include +#include #include #include @@ -253,9 +256,7 @@ populateJsonResponse( if (meta) { response[jss::meta] = meta->getJson(JsonOptions::Values::None); - insertDeliveredAmount(response[jss::meta], context, result.txn, *meta); - RPC::insertNFTSyntheticInJson(response, sttx, *meta); - RPC::insertMPTokenIssuanceID(response[jss::meta], sttx, *meta); + RPC::insertAllSyntheticInJson(response[jss::meta], context, sttx, *meta); } } response[jss::validated] = result.validated;