fix: Adds CTID to RPC tx and updates error (#4738)

This change fixes a number of issues involved with CTID:
* CTID is not present on all RPC tx transactions.
* rpcWRONG_NETWORK is missing in the ErrorCodes.cpp

src/libxrpl/protocol/ErrorCodes.cpp

@@ -96,6 +96,7 @@ constexpr static ErrorInfo unorderedErrorInfos[]{
     {rpcNOT_SYNCED,             "notSynced",            "Not synced to the network.", 503},
     {rpcNO_EVENTS,              "noEvents",             "Current transport does not support events.", 405},
     {rpcNO_NETWORK,             "noNetwork",            "Not synced to the network.", 503},
+    {rpcWRONG_NETWORK,          "wrongNetwork",         "Wrong network.", 503},
     {rpcNO_PERMISSION,          "noPermission",         "You don't have permission for this command.", 401},
     {rpcNO_PF_REQUEST,          "noPathRequest",        "No pathfinding request in progress.", 404},
     {rpcOBJECT_NOT_FOUND,       "objectNotFound",       "The requested object was not found.", 404},

src/test/rpc/Transaction_test.cpp

@@ -300,7 +300,7 @@ class Transaction_test : public beast::unit_test::suite
     void
     testRangeCTIDRequest(FeatureBitset features)
     {
-        testcase("ctid_range");
+        testcase("CTID Range Request");
 
         using namespace test::jtx;
         using std::to_string;
@@ -548,7 +548,7 @@ class Transaction_test : public beast::unit_test::suite
     void
     testCTIDValidation(FeatureBitset features)
     {
-        testcase("ctid_validation");
+        testcase("CTID Validation");
 
         using namespace test::jtx;
         using std::to_string;
@@ -570,20 +570,10 @@ class Transaction_test : public beast::unit_test::suite
         BEAST_EXPECT(!RPC::encodeCTID(0x1000'0000UL, 0xFFFFU, 0xFFFFU));
 
         // Test case 3: txn_index greater than 0xFFFF
-        // this test case is impossible in c++ due to the type, left in for
-        // completeness
-        auto const expected3 = std::optional<std::string>("CFFFFFFF0000FFFF");
-        BEAST_EXPECT(
-            RPC::encodeCTID(0x0FFF'FFFF, (uint16_t)0x10000, 0xFFFF) ==
-            expected3);
+        BEAST_EXPECT(!RPC::encodeCTID(0x0FFF'FFFF, 0x1'0000, 0xFFFF));
 
         // Test case 4: network_id greater than 0xFFFF
-        // this test case is impossible in c++ due to the type, left in for
-        // completeness
-        auto const expected4 = std::optional<std::string>("CFFFFFFFFFFF0000");
-        BEAST_EXPECT(
-            RPC::encodeCTID(0x0FFF'FFFFUL, 0xFFFFU, (uint16_t)0x1000'0U) ==
-            expected4);
+        BEAST_EXPECT(!RPC::encodeCTID(0x0FFF'FFFFUL, 0xFFFFU, 0x1'0000U));
 
         // Test case 5: Valid input values
         auto const expected51 =
@@ -647,14 +637,15 @@ class Transaction_test : public beast::unit_test::suite
     void
     testCTIDRPC(FeatureBitset features)
     {
-        testcase("ctid_rpc");
+        testcase("CTID RPC");
 
         using namespace test::jtx;
 
-        // test that the ctid AND the hash are in the response
+        // Use a Concise Transaction Identifier to request a transaction.
+        for (uint32_t netID : {11111, 65535, 65536})
         {
-            Env env{*this, makeNetworkConfig(11111)};
-            uint32_t netID = env.app().config().NETWORK_ID;
+            Env env{*this, makeNetworkConfig(netID)};
+            BEAST_EXPECT(netID == env.app().config().NETWORK_ID);
 
             auto const alice = Account("alice");
             auto const bob = Account("bob");
@@ -664,14 +655,22 @@ class Transaction_test : public beast::unit_test::suite
             env(pay(alice, bob, XRP(10)));
             env.close();
 
-            auto const ctid = *RPC::encodeCTID(startLegSeq, 0, netID);
+            auto const ctid = RPC::encodeCTID(startLegSeq, 0, netID);
+            if (netID > 0xFFFF)
+            {
+                // Concise transaction IDs do not support a network ID > 0xFFFF.
+                BEAST_EXPECT(ctid == std::nullopt);
+                continue;
+            }
+
             Json::Value jsonTx;
             jsonTx[jss::binary] = false;
-            jsonTx[jss::ctid] = ctid;
+            jsonTx[jss::ctid] = *ctid;
             jsonTx[jss::id] = 1;
-            auto jrr = env.rpc("json", "tx", to_string(jsonTx))[jss::result];
+            auto const jrr =
+                env.rpc("json", "tx", to_string(jsonTx))[jss::result];
             BEAST_EXPECT(jrr[jss::ctid] == ctid);
-            BEAST_EXPECT(jrr[jss::hash]);
+            BEAST_EXPECT(jrr.isMember(jss::hash));
         }
 
         // test querying with mixed case ctid
@@ -716,8 +715,44 @@ class Transaction_test : public beast::unit_test::suite
         }
 
         // test that if the network is 65535 the ctid is not in the response
+        // Using a hash to request the transaction, test the network ID
+        // boundary where the CTID is (not) in the response.
+        for (uint32_t netID : {2, 1024, 65535, 65536})
         {
-            Env env{*this, makeNetworkConfig(65535)};
+            Env env{*this, makeNetworkConfig(netID)};
+            BEAST_EXPECT(netID == env.app().config().NETWORK_ID);
+
+            auto const alice = Account("alice");
+            auto const bob = Account("bob");
+
+            env.fund(XRP(10000), alice, bob);
+            env(pay(alice, bob, XRP(10)));
+            env.close();
+
+            auto const ledgerSeq = env.current()->info().seq;
+
+            env(noop(alice), ter(tesSUCCESS));
+            env.close();
+
+            Json::Value params;
+            params[jss::id] = 1;
+            auto const hash = env.tx()->getJson(JsonOptions::none)[jss::hash];
+            params[jss::transaction] = hash;
+            auto const jrr =
+                env.rpc("json", "tx", to_string(params))[jss::result];
+            BEAST_EXPECT(jrr[jss::hash] == hash);
+
+            BEAST_EXPECT(jrr.isMember(jss::ctid) == (netID <= 0xFFFF));
+            if (jrr.isMember(jss::ctid))
+            {
+                auto const ctid = RPC::encodeCTID(ledgerSeq, 0, netID);
+                BEAST_EXPECT(jrr[jss::ctid] == *ctid);
+            }
+        }
+
+        // test the wrong network ID was submitted
+        {
+            Env env{*this, makeNetworkConfig(21337)};
             uint32_t netID = env.app().config().NETWORK_ID;
 
             auto const alice = Account("alice");
@@ -728,14 +763,19 @@ class Transaction_test : public beast::unit_test::suite
             env(pay(alice, bob, XRP(10)));
             env.close();
 
-            auto const ctid = *RPC::encodeCTID(startLegSeq, 0, netID);
+            auto const ctid = *RPC::encodeCTID(startLegSeq, 0, netID + 1);
             Json::Value jsonTx;
             jsonTx[jss::binary] = false;
             jsonTx[jss::ctid] = ctid;
             jsonTx[jss::id] = 1;
-            auto jrr = env.rpc("json", "tx", to_string(jsonTx))[jss::result];
-            BEAST_EXPECT(!jrr[jss::ctid]);
-            BEAST_EXPECT(jrr[jss::hash]);
+            auto const jrr =
+                env.rpc("json", "tx", to_string(jsonTx))[jss::result];
+            BEAST_EXPECT(jrr[jss::error] == "wrongNetwork");
+            BEAST_EXPECT(jrr[jss::error_code] == rpcWRONG_NETWORK);
+            BEAST_EXPECT(
+                jrr[jss::error_message] ==
+                "Wrong network. You should submit this request to a node "
+                "running on NetworkID: 21338");
         }
     }
 

src/xrpld/app/ledger/TransactionMaster.h

@@ -76,7 +76,11 @@ public:
 
     // return value: true = we had the transaction already
     bool
-    inLedger(uint256 const& hash, std::uint32_t ledger);
+    inLedger(
+        uint256 const& hash,
+        std::uint32_t ledger,
+        std::optional<uint32_t> tseq,
+        std::optional<uint32_t> netID);
 
     void
     canonicalize(std::shared_ptr<Transaction>* pTransaction);

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

@@ -39,14 +39,18 @@ TransactionMaster::TransactionMaster(Application& app)
 }
 
 bool
-TransactionMaster::inLedger(uint256 const& hash, std::uint32_t ledger)
+TransactionMaster::inLedger(
+    uint256 const& hash,
+    std::uint32_t ledger,
+    std::optional<uint32_t> tseq,
+    std::optional<uint32_t> netID)
 {
     auto txn = mCache.fetch(hash);
 
     if (!txn)
         return false;
 
-    txn->setStatus(COMMITTED, ledger);
+    txn->setStatus(COMMITTED, ledger, tseq, netID);
     return true;
 }
 

src/xrpld/app/misc/NetworkOPs.cpp

@@ -47,6 +47,7 @@
 #include <xrpld/overlay/predicates.h>
 #include <xrpld/perflog/PerfLog.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>
@@ -3080,6 +3081,20 @@ NetworkOPsImp::transJson(
             jvObj[jss::meta], transaction, meta->get());
     }
 
+    // add CTID where the needed data for it exists
+    if (auto const& lookup = ledger->txRead(transaction->getTransactionID());
+        lookup.second && lookup.second->isFieldPresent(sfTransactionIndex))
+    {
+        uint32_t const txnSeq = lookup.second->getFieldU32(sfTransactionIndex);
+        uint32_t netID = app_.config().NETWORK_ID;
+        if (transaction->isFieldPresent(sfNetworkID))
+            netID = transaction->getFieldU32(sfNetworkID);
+
+        if (std::optional<std::string> ctid =
+                RPC::encodeCTID(ledger->info().seq, txnSeq, netID);
+            ctid)
+            jvObj[jss::ctid] = *ctid;
+    }
     if (!ledger->open())
         jvObj[jss::ledger_hash] = to_string(ledger->info().hash);
 

src/xrpld/app/misc/Transaction.h

@@ -128,7 +128,11 @@ public:
     }
 
     void
-    setStatus(TransStatus status, std::uint32_t ledgerSeq);
+    setStatus(
+        TransStatus status,
+        std::uint32_t ledgerSeq,
+        std::optional<uint32_t> transactionSeq = std::nullopt,
+        std::optional<uint32_t> networkID = std::nullopt);
 
     void
     setStatus(TransStatus status)
@@ -388,6 +392,8 @@ private:
     uint256 mTransactionID;
 
     LedgerIndex mLedgerIndex = 0;
+    std::optional<uint32_t> mTxnSeq;
+    std::optional<uint32_t> mNetworkID;
     TransStatus mStatus = INVALID;
     TER mResult = temUNCERTAIN;
     bool mApplying = false;

src/xrpld/app/misc/detail/AccountTxPaging.cpp

@@ -41,12 +41,19 @@ convertBlobsToTxResult(
 
     auto tr = std::make_shared<Transaction>(txn, reason, app);
 
-    tr->setStatus(Transaction::sqlTransactionStatus(status));
-    tr->setLedger(ledger_index);
-
     auto metaset =
         std::make_shared<TxMeta>(tr->getID(), tr->getLedger(), rawMeta);
 
+    // if properly formed meta is available we can use it to generate ctid
+    if (metaset->getAsObject().isFieldPresent(sfTransactionIndex))
+        tr->setStatus(
+            Transaction::sqlTransactionStatus(status),
+            ledger_index,
+            metaset->getAsObject().getFieldU32(sfTransactionIndex),
+            app.config().NETWORK_ID);
+    else
+        tr->setStatus(Transaction::sqlTransactionStatus(status), ledger_index);
+
     to.emplace_back(std::move(tr), metaset);
 };
 

src/xrpld/app/misc/detail/Transaction.cpp

@@ -23,6 +23,7 @@
 #include <xrpld/app/misc/Transaction.h>
 #include <xrpld/app/rdb/backend/SQLiteDatabase.h>
 #include <xrpld/app/tx/apply.h>
+#include <xrpld/rpc/CTID.h>
 
 #include <xrpl/basics/safe_cast.h>
 #include <xrpl/protocol/ErrorCodes.h>
@@ -54,10 +55,18 @@ Transaction::Transaction(
 //
 
 void
-Transaction::setStatus(TransStatus ts, std::uint32_t lseq)
+Transaction::setStatus(
+    TransStatus ts,
+    std::uint32_t lseq,
+    std::optional<std::uint32_t> tseq,
+    std::optional<std::uint32_t> netID)
 {
     mStatus = ts;
     mLedgerIndex = lseq;
+    if (tseq)
+        mTxnSeq = tseq;
+    if (netID)
+        mNetworkID = netID;
 }
 
 TransStatus
@@ -174,6 +183,20 @@ Transaction::getJson(JsonOptions options, bool binary) const
             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;
+        }
     }
 
     return ret;

src/xrpld/app/rdb/backend/detail/Node.cpp

@@ -339,7 +339,11 @@ saveValidatedLedger(
                             seq, acceptedLedgerTx->getEscMeta()) +
                         ";");
 
-                app.getMasterTransaction().inLedger(transactionID, seq);
+                app.getMasterTransaction().inLedger(
+                    transactionID,
+                    seq,
+                    acceptedLedgerTx->getTxnSeq(),
+                    app.config().NETWORK_ID);
             }
 
             tr.commit();

src/xrpld/rpc/CTID.h

@@ -30,18 +30,24 @@ namespace ripple {
 
 namespace RPC {
 
+// CTID stands for Concise Transaction ID.
+//
+// The CTID comes from XLS-15d: Concise Transaction Identifier #34
+//
+//   https://github.com/XRPLF/XRPL-Standards/discussions/34
+//
+// The Concise Transaction ID provides a way to identify a transaction
+// that includes which network the transaction was submitted to.
+
 inline std::optional<std::string>
-encodeCTID(
-    uint32_t ledger_seq,
-    uint16_t txn_index,
-    uint16_t network_id) noexcept
+encodeCTID(uint32_t ledgerSeq, uint32_t txnIndex, uint32_t networkID) noexcept
 {
-    if (ledger_seq > 0x0FFF'FFFF)
+    if (ledgerSeq > 0x0FFF'FFFF || txnIndex > 0xFFFF || networkID > 0xFFFF)
         return {};
 
     uint64_t ctidValue =
-        ((0xC000'0000ULL + static_cast<uint64_t>(ledger_seq)) << 32) +
-        (static_cast<uint64_t>(txn_index) << 16) + network_id;
+        ((0xC000'0000ULL + static_cast<uint64_t>(ledgerSeq)) << 32) +
+        (static_cast<uint64_t>(txnIndex) << 16) + networkID;
 
     std::stringstream buffer;
     buffer << std::hex << std::uppercase << std::setfill('0') << std::setw(16)

src/xrpld/rpc/handlers/Tx.cpp

@@ -169,13 +169,17 @@ doTxHelp(RPC::Context& context, TxArgs args)
                 context.ledgerMaster.getCloseTimeBySeq(txn->getLedger());
 
         // compute outgoing CTID
-        uint32_t lgrSeq = ledger->info().seq;
-        uint32_t txnIdx = meta->getAsObject().getFieldU32(sfTransactionIndex);
-        uint32_t netID = context.app.config().NETWORK_ID;
+        if (meta->getAsObject().isFieldPresent(sfTransactionIndex))
+        {
+            uint32_t lgrSeq = ledger->info().seq;
+            uint32_t txnIdx =
+                meta->getAsObject().getFieldU32(sfTransactionIndex);
+            uint32_t netID = context.app.config().NETWORK_ID;
 
-        if (txnIdx <= 0xFFFFU && netID < 0xFFFFU && lgrSeq < 0x0FFF'FFFFUL)
-            result.ctid =
-                RPC::encodeCTID(lgrSeq, (uint16_t)txnIdx, (uint16_t)netID);
+            if (txnIdx <= 0xFFFFU && netID < 0xFFFFU && lgrSeq < 0x0FFF'FFFFUL)
+                result.ctid =
+                    RPC::encodeCTID(lgrSeq, (uint32_t)txnIdx, (uint32_t)netID);
+        }
     }
 
     return {result, rpcSUCCESS};