Merge origin/develop and resolve conflicts

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

include/xrpl/protocol/NFTSyntheticSerializer.h

@@ -9,7 +9,7 @@
 namespace xrpl::RPC {
 
 /**
-   Adds common synthetic fields to transaction-related JSON responses
+   Adds common synthetic fields to transaction metadata JSON
 
    @{
  */

src/libxrpl/protocol/NFTSyntheticSerializer.cpp

@@ -13,12 +13,12 @@ namespace xrpl::RPC {
 
 void
 insertNFTSyntheticInJson(
-    json::Value& response,
+    json::Value& metadata,
     std::shared_ptr<STTx const> 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

src/test/app/NFToken_test.cpp

@@ -6104,17 +6104,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
@@ -6125,17 +6196,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<uint256> 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;
@@ -6154,6 +6228,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<uint256> 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<uint256> 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

src/xrpld/app/ledger/detail/LedgerToJson.cpp

@@ -4,8 +4,7 @@
 #include <xrpld/app/misc/DeliverMax.h>
 #include <xrpld/app/misc/TxQ.h>
 #include <xrpld/rpc/Context.h>
-#include <xrpld/rpc/DeliveredAmount.h>
-#include <xrpld/rpc/MPTokenIssuanceID.h>
+#include <xrpld/rpc/detail/SyntheticFields.h>
 
 #include <xrpl/basics/Log.h>
 #include <xrpl/basics/base_uint.h>
@@ -141,19 +140,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 +169,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});
         }
     }
 

src/xrpld/app/misc/NetworkOPs.cpp

@@ -31,9 +31,8 @@
 #include <xrpld/overlay/predicates.h>
 #include <xrpld/rpc/BookChanges.h>
 #include <xrpld/rpc/CTID.h>
-#include <xrpld/rpc/DeliveredAmount.h>
-#include <xrpld/rpc/MPTokenIssuanceID.h>
 #include <xrpld/rpc/ServerHandler.h>
+#include <xrpld/rpc/detail/SyntheticFields.h>
 
 #include <xrpl/basics/Log.h>
 #include <xrpl/basics/ToString.h>
@@ -3216,9 +3215,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

src/xrpld/rpc/detail/SyntheticFields.cpp

@@ -0,0 +1,37 @@
+#include <xrpld/rpc/DeliveredAmount.h>
+#include <xrpld/rpc/MPTokenIssuanceID.h>
+#include <xrpld/rpc/detail/SyntheticFields.h>
+
+#include <xrpl/json/json_value.h>
+#include <xrpl/protocol/NFTSyntheticSerializer.h>
+#include <xrpl/protocol/jss.h>
+
+namespace xrpl {
+namespace RPC {
+
+void
+insertAllSyntheticInJson(
+    Json::Value& metadata,
+    ReadView const& ledger,
+    std::shared_ptr<STTx const> 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<STTx const> const& transaction,
+    TxMeta const& transactionMeta)
+{
+    insertDeliveredAmount(metadata, context, transaction, transactionMeta);
+    insertNFTSyntheticInJson(metadata, transaction, transactionMeta);
+    insertMPTokenIssuanceID(metadata, transaction, transactionMeta);
+}
+
+}  // namespace RPC
+}  // namespace xrpl

src/xrpld/rpc/detail/SyntheticFields.h

@@ -0,0 +1,40 @@
+#pragma once
+
+#include <xrpl/json/json_forwards.h>
+#include <xrpl/protocol/STTx.h>
+#include <xrpl/protocol/TxMeta.h>
+
+#include <memory>
+
+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<STTx const> const&,
+    TxMeta const&);
+
+void
+insertAllSyntheticInJson(
+    Json::Value& metadata,
+    JsonContext const&,
+    std::shared_ptr<STTx const> const&,
+    TxMeta const&);
+/** @} */
+
+}  // namespace RPC
+}  // namespace xrpl

src/xrpld/rpc/handlers/account/AccountTx.cpp

@@ -4,12 +4,11 @@
 #include <xrpld/app/misc/Transaction.h>
 #include <xrpld/app/rdb/backend/SQLiteDatabase.h>
 #include <xrpld/rpc/Context.h>
-#include <xrpld/rpc/DeliveredAmount.h>
-#include <xrpld/rpc/MPTokenIssuanceID.h>
 #include <xrpld/rpc/Role.h>
 #include <xrpld/rpc/Status.h>
 #include <xrpld/rpc/detail/RPCHelpers.h>
 #include <xrpld/rpc/detail/RPCLedgerHelpers.h>
+#include <xrpld/rpc/detail/SyntheticFields.h>
 #include <xrpld/rpc/detail/Tuning.h>
 
 #include <xrpl/basics/Log.h>
@@ -334,9 +333,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
                     {

src/xrpld/rpc/handlers/transaction/Simulate.cpp

@@ -5,6 +5,7 @@
 #include <xrpld/rpc/Context.h>
 #include <xrpld/rpc/DeliveredAmount.h>
 #include <xrpld/rpc/MPTokenIssuanceID.h>
+#include <xrpld/rpc/detail/SyntheticFields.h>
 #include <xrpld/rpc/detail/TransactionSign.h>
 
 #include <xrpl/basics/Expected.h>
@@ -277,12 +278,8 @@ simulateTxn(RPC::JsonContext& context, std::shared_ptr<Transaction> 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);
         }
     }
 

src/xrpld/rpc/handlers/transaction/Tx.cpp

@@ -4,9 +4,8 @@
 #include <xrpld/app/misc/Transaction.h>
 #include <xrpld/rpc/CTID.h>
 #include <xrpld/rpc/Context.h>
-#include <xrpld/rpc/DeliveredAmount.h>
-#include <xrpld/rpc/MPTokenIssuanceID.h>
 #include <xrpld/rpc/Status.h>
+#include <xrpld/rpc/detail/SyntheticFields.h>
 
 #include <xrpl/basics/Blob.h>
 #include <xrpl/basics/RangeSet.h>
@@ -253,9 +252,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;