← Watch feed
rpc: further restrict and clean up get_transaction_pool
Public commit record
50/100 · Thin
What the developer wrote
Authored by selsta
rpc: further restrict and clean up get_transaction_pool
✓ Specific, descriptive subject✓ Names a concrete action or component! No meaningful explanatory body
What changed, and why it matters
This commit changes how Monero's public RPC endpoint 'get_transaction_pool' reports the contents of the memory pool (pending transactions). Previously, the endpoint could be called in 'restricted' mode and would hide some sensitive timing fields but still return transaction data. Now the endpoint always returns full timing metadata, but the entire method is blocked when the RPC is running in restricted mode. In short, the patch trades more complete data for a smaller attack surface: only trusted callers can query the transaction pool at all.
Recommended action
Operators running restricted RPC nodes should verify that get_transaction_pool is now unavailable to untrusted clients and that any monitoring tools using it are authenticated. Developers should review whether the unconditional exposure of receive_time/last_relayed_time to all internal callers (including the HTTP RPC path) is acceptable, or whether additional filtering should be reintroduced at the HTTP RPC layer rather than inside the mempool.
Security signals we found
01
Removal of sensitive-data flag from internal mempool query APIs
02
Unconditional exposure of receive_time and last_relayed_time to callers of get_transactions_and_spent_keys_info / get_pool_for_rpc
03
Addition of get_transaction_pool to restricted-mode blocklist in ZMQ RPC
04
core_rpc_server no longer distinguishes restricted vs unrestricted origins for this endpoint
05
Test updates show the API now always returns the full (all-category) set instead of a broadcast-only subset
Evidence from the diff
The patch removes the include_sensitive/include_sensitive_data parameter from tx_memory_pool::get_transactions_and_spent_keys_info, tx_memory_pool::get_pool_for_rpc, and their core wrappers. These helpers now unconditionally use relay_category::all and always emit receive_time and last_relayed_time (with the existing dandelionpp_stem masking for last_relayed_time). On the RPC side, core_rpc_server.cpp no longer computes allow_sensitive based on request origin/restricted mode; it always passes true for the pool count and calls the parameterless info getter. Most importantly, zmq_restricted_methods.cpp adds ‘get_transaction_pool’ to the blocked-in-restricted-mode list, and tests are updated accordingly. The change therefore centralizes the access control at the RPC gate rather than filtering data inside the pool query.
Changed components
src/cryptonote_core/tx_pool.cppsrc/cryptonote_core/tx_pool.hsrc/cryptonote_core/cryptonote_core.cppsrc/cryptonote_core/cryptonote_core.hsrc/rpc/core_rpc_server.cppsrc/rpc/daemon_handler.cppsrc/rpc/zmq_restricted_methods.cppInspect captured patch +28 / −73
diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp
index 4db58ea..8681471 100644
--- a/src/cryptonote_core/cryptonote_core.cpp
+++ b/src/cryptonote_core/cryptonote_core.cpp
@@ -1528,14 +1528,14 @@ namespace cryptonote
return m_mempool.have_tx(id, relay_category::legacy);
}
//-----------------------------------------------------------------------------------------------
- bool core::get_pool_transactions_and_spent_keys_info(std::vector<tx_info>& tx_infos, std::vector<spent_key_image_info>& key_image_infos, bool include_sensitive_data) const
+ bool core::get_pool_transactions_and_spent_keys_info(std::vector<tx_info>& tx_infos, std::vector<spent_key_image_info>& key_image_infos) const
{
- return m_mempool.get_transactions_and_spent_keys_info(tx_infos, key_image_infos, include_sensitive_data);
+ return m_mempool.get_transactions_and_spent_keys_info(tx_infos, key_image_infos);
}
//-----------------------------------------------------------------------------------------------
- bool core::get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos, bool include_sensitive) const
+ bool core::get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos) const
{
- return m_mempool.get_pool_for_rpc(tx_infos, key_image_infos, include_sensitive);
+ return m_mempool.get_pool_for_rpc(tx_infos, key_image_infos);
}
//-----------------------------------------------------------------------------------------------
bool core::get_short_chain_history(std::list<crypto::hash>& ids, uint64_t& current_height) const
diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h
index 66fa9d7..5d49467 100644
--- a/src/cryptonote_core/cryptonote_core.h
+++ b/src/cryptonote_core/cryptonote_core.h
@@ -505,19 +505,15 @@ namespace cryptonote
/**
* @copydoc tx_memory_pool::get_pool_transactions_and_spent_keys_info
- * @param include_sensitive_txes include private transactions
- *
* @note see tx_memory_pool::get_pool_transactions_and_spent_keys_info
*/
- bool get_pool_transactions_and_spent_keys_info(std::vector<tx_info>& tx_infos, std::vector<spent_key_image_info>& key_image_infos, bool include_sensitive_txes = false) const;
+ bool get_pool_transactions_and_spent_keys_info(std::vector<tx_info>& tx_infos, std::vector<spent_key_image_info>& key_image_infos) const;
/**
* @copydoc tx_memory_pool::get_pool_for_rpc
- * @param include_sensitive include node-private fields (timing)
- *
* @note see tx_memory_pool::get_pool_for_rpc
*/
- bool get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos, bool include_sensitive) const;
+ bool get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos) const;
/**
* @copydoc tx_memory_pool::get_transactions_count
diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp
index 7790fcb..7c5b956 100644
--- a/src/cryptonote_core/tx_pool.cpp
+++ b/src/cryptonote_core/tx_pool.cpp
@@ -1228,15 +1228,14 @@ namespace cryptonote
}
//------------------------------------------------------------------
//TODO: investigate whether boolean return is appropriate
- bool tx_memory_pool::get_transactions_and_spent_keys_info(std::vector<tx_info>& tx_infos, std::vector<spent_key_image_info>& key_image_infos, bool include_sensitive_data) const
+ bool tx_memory_pool::get_transactions_and_spent_keys_info(std::vector<tx_info>& tx_infos, std::vector<spent_key_image_info>& key_image_infos) const
{
CRITICAL_REGION_LOCAL(m_transactions_lock);
CRITICAL_REGION_LOCAL1(m_blockchain);
- const relay_category category = include_sensitive_data ? relay_category::all : relay_category::broadcasted;
- const size_t count = m_blockchain.get_txpool_tx_count(include_sensitive_data);
+ const size_t count = m_blockchain.get_txpool_tx_count(true);
tx_infos.reserve(count);
key_image_infos.reserve(count);
- m_blockchain.for_all_txpool_txes([&tx_infos, key_image_infos, include_sensitive_data](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata_ref *bd){
+ m_blockchain.for_all_txpool_txes([&tx_infos, key_image_infos](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata_ref *bd){
tx_info txi;
txi.id_hash = epee::string_tools::pod_to_hex(txid);
txi.tx_blob = blobdata(bd->data(), bd->size());
@@ -1257,16 +1256,14 @@ namespace cryptonote
txi.max_used_block_id_hash = epee::string_tools::pod_to_hex(meta.max_used_block_id);
txi.last_failed_height = meta.last_failed_height;
txi.last_failed_id_hash = epee::string_tools::pod_to_hex(meta.last_failed_id);
- // In restricted mode we do not include this data:
- txi.receive_time = include_sensitive_data ? meta.receive_time : 0;
+ txi.receive_time = meta.receive_time;
txi.relayed = meta.relayed;
- // In restricted mode we do not include this data:
- txi.last_relayed_time = (include_sensitive_data && !meta.dandelionpp_stem) ? meta.last_relayed_time : 0;
+ txi.last_relayed_time = meta.dandelionpp_stem ? 0 : meta.last_relayed_time;
txi.do_not_relay = meta.do_not_relay;
txi.double_spend_seen = meta.double_spend_seen;
tx_infos.push_back(std::move(txi));
return true;
- }, true, category);
+ }, true, relay_category::all);
for (const key_images_container::value_type& kee : m_spent_key_images) {
const crypto::key_image& k_image = kee.first;
@@ -1275,7 +1272,7 @@ namespace cryptonote
ki.id_hash = epee::string_tools::pod_to_hex(k_image);
for (const crypto::hash& tx_id_hash : kei_image_set)
{
- if (m_blockchain.txpool_tx_matches_category(tx_id_hash, category))
+ if (m_blockchain.txpool_tx_matches_category(tx_id_hash, relay_category::all))
ki.txs_hashes.push_back(epee::string_tools::pod_to_hex(tx_id_hash));
}
@@ -1286,13 +1283,13 @@ namespace cryptonote
return true;
}
//---------------------------------------------------------------------------------
- bool tx_memory_pool::get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos, bool include_sensitive) const
+ bool tx_memory_pool::get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos) const
{
CRITICAL_REGION_LOCAL(m_transactions_lock);
CRITICAL_REGION_LOCAL1(m_blockchain);
tx_infos.reserve(m_blockchain.get_txpool_tx_count());
key_image_infos.reserve(m_blockchain.get_txpool_tx_count());
- m_blockchain.for_all_txpool_txes([&tx_infos, key_image_infos, include_sensitive](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata_ref *bd){
+ m_blockchain.for_all_txpool_txes([&tx_infos, key_image_infos](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata_ref *bd){
cryptonote::rpc::tx_in_pool txi;
txi.tx_hash = txid;
if (!(meta.pruned ? parse_and_validate_tx_base_from_blob(*bd, txi.tx) : parse_and_validate_tx_from_blob(*bd, txi.tx)))
@@ -1310,11 +1307,9 @@ namespace cryptonote
txi.max_used_block_hash = meta.max_used_block_id;
txi.last_failed_block_height = meta.last_failed_height;
txi.last_failed_block_hash = meta.last_failed_id;
- // In restricted mode we do not include this data:
- txi.receive_time = include_sensitive ? meta.receive_time : 0;
+ txi.receive_time = meta.receive_time;
txi.relayed = meta.relayed;
- // In restricted mode we do not include this data:
- txi.last_relayed_time = (include_sensitive && !meta.dandelionpp_stem) ? meta.last_relayed_time : 0;
+ txi.last_relayed_time = meta.dandelionpp_stem ? 0 : meta.last_relayed_time;
txi.do_not_relay = meta.do_not_relay;
txi.double_spend_seen = meta.double_spend_seen;
tx_infos.push_back(txi);
diff --git a/src/cryptonote_core/tx_pool.h b/src/cryptonote_core/tx_pool.h
index e3f0758..45feae6 100644
--- a/src/cryptonote_core/tx_pool.h
+++ b/src/cryptonote_core/tx_pool.h
@@ -315,12 +315,9 @@ namespace cryptonote
*
* @param tx_infos return-by-reference the transactions' information
* @param key_image_infos return-by-reference the spent key images' information
- * @param include_sensitive_data return stempool, anonymity-pool, and unrelayed
- * txes and fields that are sensitive to the node privacy
- *
* @return true
*/
- bool get_transactions_and_spent_keys_info(std::vector<tx_info>& tx_infos, std::vector<spent_key_image_info>& key_image_infos, bool include_sensitive_data = false) const;
+ bool get_transactions_and_spent_keys_info(std::vector<tx_info>& tx_infos, std::vector<spent_key_image_info>& key_image_infos) const;
/**
* @brief get information about all transactions and key images in the pool
@@ -329,11 +326,9 @@ namespace cryptonote
*
* @param tx_infos [out] the transactions' information
* @param key_image_infos [out] the spent key images' information
- * @param include_sensitive include fields that are sensitive to node privacy
- *
* @return true
*/
- bool get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos, bool include_sensitive) const;
+ bool get_pool_for_rpc(std::vector<cryptonote::rpc::tx_in_pool>& tx_infos, cryptonote::rpc::key_images_with_tx_hashes& key_image_infos) const;
/**
* @brief check for presence of key images in the pool
diff --git a/src/rpc/core_rpc_server.cpp b/src/rpc/core_rpc_server.cpp
index 104be85..2715a56 100644
--- a/src/rpc/core_rpc_server.cpp
+++ b/src/rpc/core_rpc_server.cpp
@@ -1358,14 +1358,10 @@ namespace cryptonote
{
RPC_TRACKER(get_transaction_pool);
- const bool restricted = m_restricted && ctx;
- const bool request_has_rpc_origin = ctx != NULL;
- const bool allow_sensitive = !request_has_rpc_origin || !restricted;
-
- size_t n_txes = m_core.get_pool_transactions_count(allow_sensitive);
+ size_t n_txes = m_core.get_pool_transactions_count(true);
if (n_txes > 0)
{
- m_core.get_pool_transactions_and_spent_keys_info(res.transactions, res.spent_key_images, allow_sensitive);
+ m_core.get_pool_transactions_and_spent_keys_info(res.transactions, res.spent_key_images);
for (tx_info& txi : res.transactions)
txi.tx_blob = epee::string_tools::buff_to_hex_nodelimer(txi.tx_blob);
}
diff --git a/src/rpc/daemon_handler.cpp b/src/rpc/daemon_handler.cpp
index 65fcecb..f70bebe 100644
--- a/src/rpc/daemon_handler.cpp
+++ b/src/rpc/daemon_handler.cpp
@@ -756,7 +756,7 @@ namespace rpc
void DaemonHandler::handle(const GetTransactionPool::Request& req, GetTransactionPool::Response& res)
{
- bool r = m_core.get_pool_for_rpc(res.transactions, res.key_images, !m_restricted);
+ bool r = m_core.get_pool_for_rpc(res.transactions, res.key_images);
if (!r) res.status = Message::STATUS_FAILED;
else res.status = Message::STATUS_OK;
diff --git a/src/rpc/zmq_restricted_methods.cpp b/src/rpc/zmq_restricted_methods.cpp
index 8769dbf..bec9f48 100644
--- a/src/rpc/zmq_restricted_methods.cpp
+++ b/src/rpc/zmq_restricted_methods.cpp
@@ -39,9 +39,10 @@ namespace rpc
{
namespace
{
- constexpr std::array<std::string_view, 9> blocked_in_restricted_mode{{
+ constexpr std::array<std::string_view, 10> blocked_in_restricted_mode{{
"flush_txpool",
"get_peer_list",
+ "get_transaction_pool",
"mining_status",
"relay_tx",
"save_bc",
diff --git a/tests/core_tests/tx_pool.cpp b/tests/core_tests/tx_pool.cpp
index ca2a024..ca2a718 100644
--- a/tests/core_tests/tx_pool.cpp
+++ b/tests/core_tests/tx_pool.cpp
@@ -44,20 +44,12 @@
txpool_base::txpool_base()
: test_chain_unit_base()
- , m_broadcasted_tx_count(0)
, m_all_tx_count(0)
{
- REGISTER_CALLBACK_METHOD(txpool_spend_key_public, increase_broadcasted_tx_count);
REGISTER_CALLBACK_METHOD(txpool_spend_key_public, increase_all_tx_count);
REGISTER_CALLBACK_METHOD(txpool_spend_key_public, check_txpool_spent_keys);
}
-bool txpool_base::increase_broadcasted_tx_count(cryptonote::core& /*c*/, size_t /*ev_index*/, const std::vector<test_event_entry>& /*events*/)
-{
- ++m_broadcasted_tx_count;
- return true;
-}
-
bool txpool_base::increase_all_tx_count(cryptonote::core& /*c*/, size_t /*ev_index*/, const std::vector<test_event_entry>& /*events*/)
{
++m_all_tx_count;
@@ -68,23 +60,7 @@ bool txpool_base::check_txpool_spent_keys(cryptonote::core& c, size_t /*ev_index
{
std::vector<cryptonote::tx_info> infos{};
std::vector<cryptonote::spent_key_image_info> key_images{};
- if (!c.get_pool_transactions_and_spent_keys_info(infos, key_images) || infos.size() != m_broadcasted_tx_count || key_images.size() != m_broadcasted_tx_count)
- {
- MERROR("Failed broadcasted spent keys retrieval - Expected Broadcasted Count: " << m_broadcasted_tx_count << " Actual Info Count: " << infos.size() << " Actual Key Image Count: " << key_images.size());
- return false;
- }
-
- infos.clear();
- key_images.clear();
- if (!c.get_pool_transactions_and_spent_keys_info(infos, key_images, false) || infos.size() != m_broadcasted_tx_count || key_images.size() != m_broadcasted_tx_count)
- {
- MERROR("Failed broadcasted spent keys retrieval - Expected Broadcasted Count: " << m_broadcasted_tx_count << " Actual Info Count: " << infos.size() << " Actual Key Image Count: " << key_images.size());
- return false;
- }
-
- infos.clear();
- key_images.clear();
- if (!c.get_pool_transactions_and_spent_keys_info(infos, key_images, true) || infos.size() != m_all_tx_count || key_images.size() != m_all_tx_count)
+ if (!c.get_pool_transactions_and_spent_keys_info(infos, key_images) || infos.size() != m_all_tx_count || key_images.size() != m_all_tx_count)
{
MERROR("Failed all spent keys retrieval - Expected All Count: " << m_all_tx_count << " Actual Info Count: " << infos.size() << " Actual Key Image Count: " << key_images.size());
return false;
@@ -99,7 +75,6 @@ bool txpool_spend_key_public::generate(std::vector<test_event_entry>& events) co
DO_CALLBACK(events, "check_txpool_spent_keys");
MAKE_TX(events, tx_0, miner_account, bob_account, send_amount, blk_0r);
- DO_CALLBACK(events, "increase_broadcasted_tx_count");
DO_CALLBACK(events, "increase_all_tx_count");
DO_CALLBACK(events, "check_txpool_spent_keys");
@@ -245,7 +220,7 @@ bool txpool_double_spend_base::check_changed(cryptonote::core& c, const size_t e
{
std::vector<cryptonote::tx_info> infos{};
std::vector<cryptonote::spent_key_image_info> key_images{};
- if (!c.get_pool_transactions_and_spent_keys_info(infos, key_images, true) || infos.size() != m_all_hashes.size())
+ if (!c.get_pool_transactions_and_spent_keys_info(infos, key_images) || infos.size() != m_all_hashes.size())
{
MERROR("Unable to retrieve all txpool metadata");
return false;
@@ -455,7 +430,7 @@ bool txpool_double_spend_base::check_changed(cryptonote::core& c, const size_t e
{
std::vector<cryptonote::rpc::tx_in_pool> infos{};
cryptonote::rpc::key_images_with_tx_hashes key_images{};
- if (!c.get_pool_for_rpc(infos, key_images, true) || infos.size() != m_broadcasted_hashes.size() || key_images.size() != m_broadcasted_hashes.size())
+ if (!c.get_pool_for_rpc(infos, key_images) || infos.size() != m_broadcasted_hashes.size() || key_images.size() != m_broadcasted_hashes.size())
{
MERROR("Expected broadcasted rpc data to return " << m_broadcasted_hashes.size() << " but got " << infos.size() << " infos and " << key_images.size() << "key images");
return false;
@@ -553,7 +528,6 @@ bool txpool_double_spend_local::generate(std::vector<test_event_entry>& events)
SET_EVENT_VISITOR_SETT(events, 0);
DO_CALLBACK(events, "timestamp_change_pause");
events.push_back(tx_0);
- DO_CALLBACK(events, "increase_broadcasted_tx_count");
DO_CALLBACK(events, "check_txpool_spent_keys");
DO_CALLBACK(events, "mark_timestamp_change");
DO_CALLBACK(events, "check_new_broadcasted");
@@ -624,7 +598,6 @@ bool txpool_stem_loop::generate(std::vector<test_event_entry>& events) const
DO_CALLBACK(events, "check_new_hidden");
DO_CALLBACK(events, "timestamp_change_pause");
events.push_back(tx_0);
- DO_CALLBACK(events, "increase_broadcasted_tx_count");
DO_CALLBACK(events, "check_txpool_spent_keys");
DO_CALLBACK(events, "mark_timestamp_change");
DO_CALLBACK(events, "check_new_broadcasted");
diff --git a/tests/core_tests/tx_pool.h b/tests/core_tests/tx_pool.h
index 315c1d6..9a87df3 100644
--- a/tests/core_tests/tx_pool.h
+++ b/tests/core_tests/tx_pool.h
@@ -44,13 +44,11 @@ enum class relay_test
class txpool_base : public test_chain_unit_base
{
- size_t m_broadcasted_tx_count;
size_t m_all_tx_count;
public:
txpool_base();
- bool increase_broadcasted_tx_count(cryptonote::core& c, size_t /*ev_index*/, const std::vector<test_event_entry>& events);
bool increase_all_tx_count(cryptonote::core& c, size_t /*ev_index*/, const std::vector<test_event_entry>& events);
bool check_txpool_spent_keys(cryptonote::core& c, size_t /*ev_index*/, const std::vector<test_event_entry>& events);
};
diff --git a/tests/unit_tests/zmq_rpc.cpp b/tests/unit_tests/zmq_rpc.cpp
index 882a0e6..14b771e 100644
--- a/tests/unit_tests/zmq_rpc.cpp
+++ b/tests/unit_tests/zmq_rpc.cpp
@@ -88,6 +88,7 @@ TEST(ZmqRestrictedMethods, BasicCoverage)
{
EXPECT_TRUE(cryptonote::rpc::is_blocked_in_restricted_mode("flush_txpool"));
EXPECT_TRUE(cryptonote::rpc::is_blocked_in_restricted_mode("get_peer_list"));
+ EXPECT_TRUE(cryptonote::rpc::is_blocked_in_restricted_mode("get_transaction_pool"));
EXPECT_TRUE(cryptonote::rpc::is_blocked_in_restricted_mode("mining_status"));
EXPECT_TRUE(cryptonote::rpc::is_blocked_in_restricted_mode("relay_tx"));
EXPECT_TRUE(cryptonote::rpc::is_blocked_in_restricted_mode("save_bc"));
Risk score
Our methodology →Why this scored 51/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.