diff --git a/src/test/rpc/Transaction_test.cpp b/src/test/rpc/Transaction_test.cpp index a1e156e7a..75aa7ccd4 100644 --- a/src/test/rpc/Transaction_test.cpp +++ b/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("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("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"); } // test the wrong network ID was submitted diff --git a/src/xrpld/app/ledger/TransactionMaster.h b/src/xrpld/app/ledger/TransactionMaster.h index 5a3ff3b2e..f6993dc0e 100644 --- a/src/xrpld/app/ledger/TransactionMaster.h +++ b/src/xrpld/app/ledger/TransactionMaster.h @@ -80,7 +80,7 @@ public: uint256 const& hash, std::uint32_t ledger, std::optional tseq, - std::optional netID); + std::optional netID); void canonicalize(std::shared_ptr* pTransaction); diff --git a/src/xrpld/app/ledger/detail/TransactionMaster.cpp b/src/xrpld/app/ledger/detail/TransactionMaster.cpp index 26a0d21e3..1f5ab7e5b 100644 --- a/src/xrpld/app/ledger/detail/TransactionMaster.cpp +++ b/src/xrpld/app/ledger/detail/TransactionMaster.cpp @@ -43,7 +43,7 @@ TransactionMaster::inLedger( uint256 const& hash, std::uint32_t ledger, std::optional tseq, - std::optional netID) + std::optional netID) { auto txn = mCache.fetch(hash); diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 238d38dde..ed9f9ad3a 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -3103,21 +3103,16 @@ NetworkOPsImp::transJson( if (auto const& lookup = ledger->txRead(transaction->getTransactionID()); lookup.second && lookup.second->isFieldPresent(sfTransactionIndex)) { - uint32_t txnSeq = lookup.second->getFieldU32(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 (txnSeq <= 0xFFFFU && netID < 0xFFFFU && - ledger->info().seq < 0xFFFFFFFUL) - { - if (std::optional ctid = - RPC::encodeCTID(ledger->info().seq, txnSeq, netID); - ctid) - jvObj[jss::ctid] = *ctid; - } + if (std::optional 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); diff --git a/src/xrpld/app/misc/Transaction.h b/src/xrpld/app/misc/Transaction.h index 4a338239a..82e5b55bf 100644 --- a/src/xrpld/app/misc/Transaction.h +++ b/src/xrpld/app/misc/Transaction.h @@ -132,7 +132,7 @@ public: TransStatus status, std::uint32_t ledgerSeq, std::optional transactionSeq = std::nullopt, - std::optional networkID = std::nullopt); + std::optional networkID = std::nullopt); void setStatus(TransStatus status) @@ -393,8 +393,7 @@ private: LedgerIndex mLedgerIndex = 0; std::optional mTxnSeq; - std::optional mNetworkID; - + std::optional mNetworkID; TransStatus mStatus = INVALID; TER mResult = temUNCERTAIN; bool mApplying = false; diff --git a/src/xrpld/app/misc/detail/AccountTxPaging.cpp b/src/xrpld/app/misc/detail/AccountTxPaging.cpp index ff59e4022..243d2d4d5 100644 --- a/src/xrpld/app/misc/detail/AccountTxPaging.cpp +++ b/src/xrpld/app/misc/detail/AccountTxPaging.cpp @@ -41,7 +41,8 @@ convertBlobsToTxResult( auto tr = std::make_shared(txn, reason, app); - auto metaset = std::make_shared(tr->getID(), ledger_index, rawMeta); + auto metaset = + std::make_shared(tr->getID(), tr->getLedger(), rawMeta); // if properly formed meta is available we can use it to generate ctid if (metaset->getAsObject().isFieldPresent(sfTransactionIndex)) diff --git a/src/xrpld/app/misc/detail/Transaction.cpp b/src/xrpld/app/misc/detail/Transaction.cpp index a0da831ad..cc38a77d7 100644 --- a/src/xrpld/app/misc/detail/Transaction.cpp +++ b/src/xrpld/app/misc/detail/Transaction.cpp @@ -59,7 +59,7 @@ Transaction::setStatus( TransStatus ts, std::uint32_t lseq, std::optional tseq, - std::optional netID) + std::optional netID) { mStatus = ts; mLedgerIndex = lseq; @@ -190,10 +190,9 @@ Transaction::getJson(JsonOptions options, bool binary) const if (mTransaction->isFieldPresent(sfNetworkID)) netID = mTransaction->getFieldU32(sfNetworkID); - if (mTxnSeq && netID && *mTxnSeq <= 0xFFFFU && *netID < 0xFFFFU && - mLedgerIndex < 0xFFFFFFFUL) + if (mTxnSeq && netID) { - std::optional ctid = + std::optional const ctid = RPC::encodeCTID(mLedgerIndex, *mTxnSeq, *netID); if (ctid) ret[jss::ctid] = *ctid; diff --git a/src/xrpld/rpc/CTID.h b/src/xrpld/rpc/CTID.h index 8407d5152..042b79b52 100644 --- a/src/xrpld/rpc/CTID.h +++ b/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 -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(ledger_seq)) << 32) + - (static_cast(txn_index) << 16) + network_id; + ((0xC000'0000ULL + static_cast(ledgerSeq)) << 32) + + (static_cast(txnIndex) << 16) + networkID; std::stringstream buffer; buffer << std::hex << std::uppercase << std::setfill('0') << std::setw(16) diff --git a/src/xrpld/rpc/handlers/Tx.cpp b/src/xrpld/rpc/handlers/Tx.cpp index 420f9cbe0..74d3bcf72 100644 --- a/src/xrpld/rpc/handlers/Tx.cpp +++ b/src/xrpld/rpc/handlers/Tx.cpp @@ -181,7 +181,7 @@ doTxHelp(RPC::Context& context, TxArgs args) if (txnIdx <= 0xFFFFU && netID < 0xFFFFU && lgrSeq < 0x0FFF'FFFFUL) result.ctid = - RPC::encodeCTID(lgrSeq, (uint16_t)txnIdx, (uint16_t)netID); + RPC::encodeCTID(lgrSeq, (uint32_t)txnIdx, (uint32_t)netID); } }