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
This commit is contained in:
Denis Angell
2025-04-10 14:38:52 +02:00
committed by tequ
parent 7f6b1ad6ec
commit d849fd08f5
9 changed files with 95 additions and 55 deletions

View File

@@ -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");
}
// test the wrong network ID was submitted

View File

@@ -80,7 +80,7 @@ public:
uint256 const& hash,
std::uint32_t ledger,
std::optional<uint32_t> tseq,
std::optional<uint16_t> netID);
std::optional<uint32_t> netID);
void
canonicalize(std::shared_ptr<Transaction>* pTransaction);

View File

@@ -43,7 +43,7 @@ TransactionMaster::inLedger(
uint256 const& hash,
std::uint32_t ledger,
std::optional<uint32_t> tseq,
std::optional<uint16_t> netID)
std::optional<uint32_t> netID)
{
auto txn = mCache.fetch(hash);

View File

@@ -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<std::string> ctid =
RPC::encodeCTID(ledger->info().seq, txnSeq, netID);
ctid)
jvObj[jss::ctid] = *ctid;
}
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);

View File

@@ -132,7 +132,7 @@ public:
TransStatus status,
std::uint32_t ledgerSeq,
std::optional<uint32_t> transactionSeq = std::nullopt,
std::optional<uint16_t> networkID = std::nullopt);
std::optional<uint32_t> networkID = std::nullopt);
void
setStatus(TransStatus status)
@@ -393,8 +393,7 @@ private:
LedgerIndex mLedgerIndex = 0;
std::optional<uint32_t> mTxnSeq;
std::optional<uint16_t> mNetworkID;
std::optional<uint32_t> mNetworkID;
TransStatus mStatus = INVALID;
TER mResult = temUNCERTAIN;
bool mApplying = false;

View File

@@ -41,7 +41,8 @@ convertBlobsToTxResult(
auto tr = std::make_shared<Transaction>(txn, reason, app);
auto metaset = std::make_shared<TxMeta>(tr->getID(), ledger_index, rawMeta);
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))

View File

@@ -59,7 +59,7 @@ Transaction::setStatus(
TransStatus ts,
std::uint32_t lseq,
std::optional<std::uint32_t> tseq,
std::optional<std::uint16_t> netID)
std::optional<std::uint32_t> 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<std::string> ctid =
std::optional<std::string> const ctid =
RPC::encodeCTID(mLedgerIndex, *mTxnSeq, *netID);
if (ctid)
ret[jss::ctid] = *ctid;

View File

@@ -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)

View File

@@ -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);
}
}