From 91b65adff2aaf16f42c5ccca6e16b951e0e84f9a Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Thu, 5 Sep 2024 21:40:52 -0400 Subject: [PATCH 1/6] refactor: add OrphanTxBase for external use Enables external entities to obtain orphan information --- src/txorphanage.cpp | 2 +- src/txorphanage.h | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 35a215c88a4..35cf26a7be1 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -33,7 +33,7 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) return false; } - auto ret = m_orphans.emplace(wtxid, OrphanTx{tx, peer, Now() + ORPHAN_TX_EXPIRE_TIME, m_orphan_list.size()}); + auto ret = m_orphans.emplace(wtxid, OrphanTx{{tx, peer, Now() + ORPHAN_TX_EXPIRE_TIME}, m_orphan_list.size()}); assert(ret.second); m_orphan_list.push_back(ret.first); for (const CTxIn& txin : tx->vin) { diff --git a/src/txorphanage.h b/src/txorphanage.h index 2c53d1d40f6..5123bfe8678 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -72,11 +72,15 @@ public: return m_orphans.size(); } -protected: - struct OrphanTx { + /** Allows providing orphan information externally */ + struct OrphanTxBase { CTransactionRef tx; NodeId fromPeer; NodeSeconds nTimeExpire; + }; + +protected: + struct OrphanTx : public OrphanTxBase { size_t list_pos; }; From 532491faf1aa90053af52cbedce403b9eccf0bc3 Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:28:04 -0400 Subject: [PATCH 2/6] net: add GetOrphanTransactions() to PeerManager Updates PeerManager (and Impl) to provide orphans with metadata --- src/net_processing.cpp | 7 +++++++ src/net_processing.h | 3 +++ src/txorphanage.cpp | 10 ++++++++++ src/txorphanage.h | 2 ++ 4 files changed, 22 insertions(+) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b7d0f5360de..be16884011e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -515,6 +515,7 @@ public: std::optional FetchBlock(NodeId peer_id, const CBlockIndex& block_index) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); + std::vector GetOrphanTransactions() override EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex); PeerManagerInfo GetInfo() const override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void SendPings() override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); void RelayTransaction(const uint256& txid, const uint256& wtxid) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); @@ -1917,6 +1918,12 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c return true; } +std::vector PeerManagerImpl::GetOrphanTransactions() +{ + LOCK(m_tx_download_mutex); + return m_orphanage.GetOrphanTransactions(); +} + PeerManagerInfo PeerManagerImpl::GetInfo() const { return PeerManagerInfo{ diff --git a/src/net_processing.h b/src/net_processing.h index ccacd15e422..0d2dc59c5ae 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -7,6 +7,7 @@ #define BITCOIN_NET_PROCESSING_H #include +#include #include #include @@ -99,6 +100,8 @@ public: /** Get statistics from node state */ virtual bool GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) const = 0; + virtual std::vector GetOrphanTransactions() = 0; + /** Get peer manager info. */ virtual PeerManagerInfo GetInfo() const = 0; diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index 35cf26a7be1..ba4ba6c3b62 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -277,3 +277,13 @@ std::vector> TxOrphanage::GetChildrenFromDiff } return children_found; } + +std::vector TxOrphanage::GetOrphanTransactions() const +{ + std::vector ret; + ret.reserve(m_orphans.size()); + for (auto const& o : m_orphans) { + ret.push_back({o.second.tx, o.second.fromPeer, o.second.nTimeExpire}); + } + return ret; +} diff --git a/src/txorphanage.h b/src/txorphanage.h index 5123bfe8678..5501d109228 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -79,6 +79,8 @@ public: NodeSeconds nTimeExpire; }; + std::vector GetOrphanTransactions() const; + protected: struct OrphanTx : public OrphanTxBase { size_t list_pos; From f511ff3654d999951a64098c8a9f2f8e29771dad Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:28:43 -0400 Subject: [PATCH 3/6] refactor: move verbosity parsing to rpc/util Provides a common way for rpcs to obtain verbosity from an rpc parameter --- src/rpc/blockchain.cpp | 9 +-------- src/rpc/rawtransaction.cpp | 10 +--------- src/rpc/util.cpp | 12 ++++++++++++ src/rpc/util.h | 9 +++++++++ 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index c4fc06b34e7..360f24ec553 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -766,14 +766,7 @@ static RPCHelpMan getblock() { uint256 hash(ParseHashV(request.params[0], "blockhash")); - int verbosity = 1; - if (!request.params[1].isNull()) { - if (request.params[1].isBool()) { - verbosity = request.params[1].get_bool() ? 1 : 0; - } else { - verbosity = request.params[1].getInt(); - } - } + int verbosity{ParseVerbosity(request.params[1], /*default_verbosity=*/1)}; const CBlockIndex* pblockindex; const CBlockIndex* tip; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index bb072370ce1..65e6e40b0dc 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -338,15 +338,7 @@ static RPCHelpMan getrawtransaction() throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "The genesis block coinbase is not considered an ordinary transaction and cannot be retrieved"); } - // Accept either a bool (true) or a num (>=0) to indicate verbosity. - int verbosity{0}; - if (!request.params[1].isNull()) { - if (request.params[1].isBool()) { - verbosity = request.params[1].get_bool(); - } else { - verbosity = request.params[1].getInt(); - } - } + int verbosity{ParseVerbosity(request.params[1], /*default_verbosity=*/0)}; if (!request.params[2].isNull()) { LOCK(cs_main); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 678bac7a185..7757c258fd5 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -81,6 +81,18 @@ void RPCTypeCheckObj(const UniValue& o, } } +int ParseVerbosity(const UniValue& arg, int default_verbosity) +{ + if (!arg.isNull()) { + if (arg.isBool()) { + return arg.get_bool(); // true = 1 + } else { + return arg.getInt(); + } + } + return default_verbosity; +} + CAmount AmountFromValue(const UniValue& value, int decimals) { if (!value.isNum() && !value.isStr()) diff --git a/src/rpc/util.h b/src/rpc/util.h index 23024376e09..b8e67597688 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -100,6 +100,15 @@ uint256 ParseHashO(const UniValue& o, std::string_view strKey); std::vector ParseHexV(const UniValue& v, std::string_view name); std::vector ParseHexO(const UniValue& o, std::string_view strKey); +/** + * Parses verbosity from provided UniValue. + * + * @param[in] arg The verbosity argument as a bool (true) or int (0, 1, 2,...) + * @param[in] default_verbosity The value to return if verbosity argument is null + * @returns An integer describing the verbosity level (e.g. 0, 1, 2, etc.) + */ +int ParseVerbosity(const UniValue& arg, int default_verbosity); + /** * Validate and return a CAmount from a UniValue number or string. * From 34a9c10e8cdb3e9cd40fc3a420df8f73e0208a48 Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:29:32 -0400 Subject: [PATCH 4/6] rpc: add getorphantxs Adds an rpc to obtain data about the transactions currently in the orphanage. Hidden and marked as experimental --- src/rpc/client.cpp | 2 + src/rpc/mempool.cpp | 102 ++++++++++++++++++++++++++++++++++++++++++ src/test/fuzz/rpc.cpp | 1 + 3 files changed, 105 insertions(+) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 0112a261ce7..601e4fa7bf5 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -254,6 +254,8 @@ static const CRPCConvertParam vRPCConvertParams[] = { "keypoolrefill", 0, "newsize" }, { "getrawmempool", 0, "verbose" }, { "getrawmempool", 1, "mempool_sequence" }, + { "getorphantxs", 0, "verbosity" }, + { "getorphantxs", 0, "verbose" }, { "estimatesmartfee", 0, "conf_target" }, { "estimaterawfee", 0, "conf_target" }, { "estimaterawfee", 1, "threshold" }, diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index d61898260bf..27a00c5d917 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -8,8 +8,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -24,6 +26,7 @@ #include #include #include +#include #include @@ -812,6 +815,104 @@ static RPCHelpMan savemempool() }; } +static std::vector OrphanDescription() +{ + return { + RPCResult{RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, + RPCResult{RPCResult::Type::STR_HEX, "wtxid", "The transaction witness hash in hex"}, + RPCResult{RPCResult::Type::NUM, "bytes", "The serialized transaction size in bytes"}, + RPCResult{RPCResult::Type::NUM, "vsize", "The virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted."}, + RPCResult{RPCResult::Type::NUM, "weight", "The transaction weight as defined in BIP 141."}, + RPCResult{RPCResult::Type::NUM_TIME, "expiration", "The orphan expiration time expressed in " + UNIX_EPOCH_TIME}, + RPCResult{RPCResult::Type::ARR, "from", "", + { + RPCResult{RPCResult::Type::NUM, "peer_id", "Peer ID"}, + }}, + }; +} + +static UniValue OrphanToJSON(const TxOrphanage::OrphanTxBase& orphan) +{ + UniValue o(UniValue::VOBJ); + o.pushKV("txid", orphan.tx->GetHash().ToString()); + o.pushKV("wtxid", orphan.tx->GetWitnessHash().ToString()); + o.pushKV("bytes", orphan.tx->GetTotalSize()); + o.pushKV("vsize", GetVirtualTransactionSize(*orphan.tx)); + o.pushKV("weight", GetTransactionWeight(*orphan.tx)); + o.pushKV("expiration", int64_t{TicksSinceEpoch(orphan.nTimeExpire)}); + UniValue from(UniValue::VARR); + from.push_back(orphan.fromPeer); // only one fromPeer for now + o.pushKV("from", from); + return o; +} + +static RPCHelpMan getorphantxs() +{ + return RPCHelpMan{"getorphantxs", + "\nShows transactions in the tx orphanage.\n" + "\nEXPERIMENTAL warning: this call may be changed in future releases.\n", + { + {"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{0}, "0 for an array of txids (may contain duplicates), 1 for an array of objects with tx details, and 2 for details from (1) and tx hex", + RPCArgOptions{.skip_type_check = true}}, + }, + { + RPCResult{"for verbose = 0", + RPCResult::Type::ARR, "", "", + { + {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"}, + }}, + RPCResult{"for verbose = 1", + RPCResult::Type::ARR, "", "", + { + {RPCResult::Type::OBJ, "", "", OrphanDescription()}, + }}, + RPCResult{"for verbose = 2", + RPCResult::Type::ARR, "", "", + { + {RPCResult::Type::OBJ, "", "", + Cat>( + OrphanDescription(), + {{RPCResult::Type::STR_HEX, "hex", "The serialized, hex-encoded transaction data"}} + ) + }, + }}, + }, + RPCExamples{ + HelpExampleCli("getorphantxs", "2") + + HelpExampleRpc("getorphantxs", "2") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + const NodeContext& node = EnsureAnyNodeContext(request.context); + PeerManager& peerman = EnsurePeerman(node); + std::vector orphanage = peerman.GetOrphanTransactions(); + + int verbosity{ParseVerbosity(request.params[0], /*default_verbosity=*/0)}; + + UniValue ret(UniValue::VARR); + + if (verbosity <= 0) { + for (auto const& orphan : orphanage) { + ret.push_back(orphan.tx->GetHash().ToString()); + } + } else if (verbosity == 1) { + for (auto const& orphan : orphanage) { + ret.push_back(OrphanToJSON(orphan)); + } + } else { + // >= 2 + for (auto const& orphan : orphanage) { + UniValue o{OrphanToJSON(orphan)}; + o.pushKV("hex", EncodeHexTx(*orphan.tx)); + ret.push_back(o); + } + } + + return ret; + }, + }; +} + static RPCHelpMan submitpackage() { return RPCHelpMan{"submitpackage", @@ -1027,6 +1128,7 @@ void RegisterMempoolRPCCommands(CRPCTable& t) {"blockchain", &getrawmempool}, {"blockchain", &importmempool}, {"blockchain", &savemempool}, + {"hidden", &getorphantxs}, {"rawtransactions", &submitpackage}, }; for (const auto& c : commands) { diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 9122617e462..4db37ab7b7a 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -143,6 +143,7 @@ const std::vector RPC_COMMANDS_SAFE_FOR_FUZZING{ "getnetworkhashps", "getnetworkinfo", "getnodeaddresses", + "getorphantxs", "getpeerinfo", "getprioritisedtransactions", "getrawaddrman", From 93f48fceb7dd332ef980ce890ff7750b995d6077 Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:29:55 -0400 Subject: [PATCH 5/6] test: add tx_in_orphanage() Allows tests to check if a transaction is contained within the orphanage --- test/functional/test_framework/mempool_util.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/functional/test_framework/mempool_util.py b/test/functional/test_framework/mempool_util.py index fe47123e138..a6a7940c608 100644 --- a/test/functional/test_framework/mempool_util.py +++ b/test/functional/test_framework/mempool_util.py @@ -8,6 +8,7 @@ from decimal import Decimal from .blocktools import ( COINBASE_MATURITY, ) +from .messages import CTransaction from .util import ( assert_equal, assert_greater_than, @@ -83,3 +84,8 @@ def fill_mempool(test_framework, node, *, tx_sync_fun=None): test_framework.log.debug("Check that mempoolminfee is larger than minrelaytxfee") assert_equal(node.getmempoolinfo()['minrelaytxfee'], Decimal('0.00001000')) assert_greater_than(node.getmempoolinfo()['mempoolminfee'], Decimal('0.00001000')) + +def tx_in_orphanage(node, tx: CTransaction) -> bool: + """Returns true if the transaction is in the orphanage.""" + found = [o for o in node.getorphantxs(verbosity=1) if o["txid"] == tx.rehash() and o["wtxid"] == tx.getwtxid()] + return len(found) == 1 From 98c1536852d1de9a978b11046e7414e79ed40b46 Mon Sep 17 00:00:00 2001 From: tdb3 <106488469+tdb3@users.noreply.github.com> Date: Wed, 18 Sep 2024 12:30:15 -0400 Subject: [PATCH 6/6] test: add getorphantxs tests Adds functional tests for getorphantxs --- test/functional/rpc_getorphantxs.py | 130 ++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 131 insertions(+) create mode 100755 test/functional/rpc_getorphantxs.py diff --git a/test/functional/rpc_getorphantxs.py b/test/functional/rpc_getorphantxs.py new file mode 100755 index 00000000000..8d32ce16384 --- /dev/null +++ b/test/functional/rpc_getorphantxs.py @@ -0,0 +1,130 @@ +#!/usr/bin/env python3 +# Copyright (c) 2014-2024 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test the getorphantxs RPC.""" + +from test_framework.mempool_util import tx_in_orphanage +from test_framework.messages import msg_tx +from test_framework.p2p import P2PInterface +from test_framework.util import assert_equal +from test_framework.test_framework import BitcoinTestFramework +from test_framework.wallet import MiniWallet + + +class GetOrphanTxsTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + + def run_test(self): + self.wallet = MiniWallet(self.nodes[0]) + self.test_orphan_activity() + self.test_orphan_details() + + def test_orphan_activity(self): + self.log.info("Check that orphaned transactions are returned with getorphantxs") + node = self.nodes[0] + + self.log.info("Create two 1P1C packages, but only broadcast the children") + tx_parent_1 = self.wallet.create_self_transfer() + tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"]) + tx_parent_2 = self.wallet.create_self_transfer() + tx_child_2 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_2["new_utxo"]) + peer = node.add_p2p_connection(P2PInterface()) + peer.send_and_ping(msg_tx(tx_child_1["tx"])) + peer.send_and_ping(msg_tx(tx_child_2["tx"])) + + self.log.info("Check that neither parent is in the mempool") + assert_equal(node.getmempoolinfo()["size"], 0) + + self.log.info("Check that both children are in the orphanage") + + orphanage = node.getorphantxs(verbosity=0) + self.log.info("Check the size of the orphanage") + assert_equal(len(orphanage), 2) + self.log.info("Check that negative verbosity is treated as 0") + assert_equal(orphanage, node.getorphantxs(verbosity=-1)) + assert tx_in_orphanage(node, tx_child_1["tx"]) + assert tx_in_orphanage(node, tx_child_2["tx"]) + + self.log.info("Broadcast parent 1") + peer.send_and_ping(msg_tx(tx_parent_1["tx"])) + self.log.info("Check that parent 1 and child 1 are in the mempool") + raw_mempool = node.getrawmempool() + assert_equal(len(raw_mempool), 2) + assert tx_parent_1["txid"] in raw_mempool + assert tx_child_1["txid"] in raw_mempool + + self.log.info("Check that orphanage only contains child 2") + orphanage = node.getorphantxs() + assert_equal(len(orphanage), 1) + assert tx_in_orphanage(node, tx_child_2["tx"]) + + peer.send_and_ping(msg_tx(tx_parent_2["tx"])) + self.log.info("Check that all parents and children are now in the mempool") + raw_mempool = node.getrawmempool() + assert_equal(len(raw_mempool), 4) + assert tx_parent_1["txid"] in raw_mempool + assert tx_child_1["txid"] in raw_mempool + assert tx_parent_2["txid"] in raw_mempool + assert tx_child_2["txid"] in raw_mempool + self.log.info("Check that the orphanage is empty") + assert_equal(len(node.getorphantxs()), 0) + + self.log.info("Confirm the transactions (clears mempool)") + self.generate(node, 1) + assert_equal(node.getmempoolinfo()["size"], 0) + + def test_orphan_details(self): + self.log.info("Check the transaction details returned from getorphantxs") + node = self.nodes[0] + + self.log.info("Create two orphans, from different peers") + tx_parent_1 = self.wallet.create_self_transfer() + tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"]) + tx_parent_2 = self.wallet.create_self_transfer() + tx_child_2 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_2["new_utxo"]) + peer_1 = node.add_p2p_connection(P2PInterface()) + peer_2 = node.add_p2p_connection(P2PInterface()) + peer_1.send_and_ping(msg_tx(tx_child_1["tx"])) + peer_2.send_and_ping(msg_tx(tx_child_2["tx"])) + + orphanage = node.getorphantxs(verbosity=2) + assert tx_in_orphanage(node, tx_child_1["tx"]) + assert tx_in_orphanage(node, tx_child_2["tx"]) + + self.log.info("Check that orphan 1 and 2 were from different peers") + assert orphanage[0]["from"][0] != orphanage[1]["from"][0] + + self.log.info("Unorphan child 2") + peer_2.send_and_ping(msg_tx(tx_parent_2["tx"])) + assert not tx_in_orphanage(node, tx_child_2["tx"]) + + self.log.info("Checking orphan details") + orphanage = node.getorphantxs(verbosity=1) + assert_equal(len(node.getorphantxs()), 1) + orphan_1 = orphanage[0] + self.orphan_details_match(orphan_1, tx_child_1, verbosity=1) + + self.log.info("Checking orphan details (verbosity 2)") + orphanage = node.getorphantxs(verbosity=2) + orphan_1 = orphanage[0] + self.orphan_details_match(orphan_1, tx_child_1, verbosity=2) + + def orphan_details_match(self, orphan, tx, verbosity): + self.log.info("Check txid/wtxid of orphan") + assert_equal(orphan["txid"], tx["txid"]) + assert_equal(orphan["wtxid"], tx["wtxid"]) + + self.log.info("Check the sizes of orphan") + assert_equal(orphan["bytes"], len(tx["tx"].serialize())) + assert_equal(orphan["vsize"], tx["tx"].get_vsize()) + assert_equal(orphan["weight"], tx["tx"].get_weight()) + + if verbosity == 2: + self.log.info("Check the transaction hex of orphan") + assert_equal(orphan["hex"], tx["hex"]) + + +if __name__ == '__main__': + GetOrphanTxsTest(__file__).main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 301f62d7b04..14a1e42924d 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -160,6 +160,7 @@ BASE_SCRIPTS = [ 'wallet_importmulti.py --legacy-wallet', 'mempool_limit.py', 'rpc_txoutproof.py', + 'rpc_getorphantxs.py', 'wallet_listreceivedby.py --legacy-wallet', 'wallet_listreceivedby.py --descriptors', 'wallet_abandonconflict.py --legacy-wallet',