rpc: further restrict and clean up get_transaction_pool

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

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

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(true);
-    const size_t count = m_blockchain.get_txpool_tx_count(include_sensitive_data);
     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 = meta.receive_time;
-      txi.receive_time = include_sensitive_data ? meta.receive_time : 0;
       txi.relayed = meta.relayed;
-      // In restricted mode we do not include this data:
+      txi.last_relayed_time = meta.dandelionpp_stem ? 0 : meta.last_relayed_time;
-      txi.last_relayed_time = (include_sensitive_data && !meta.dandelionpp_stem) ? meta.last_relayed_time : 0;
       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 = meta.receive_time;
-      txi.receive_time = include_sensitive ? meta.receive_time : 0;
       txi.relayed = meta.relayed;
-      // In restricted mode we do not include this data:
+      txi.last_relayed_time = meta.dandelionpp_stem ? 0 : meta.last_relayed_time;
-      txi.last_relayed_time = (include_sensitive && !meta.dandelionpp_stem) ? meta.last_relayed_time : 0;
       txi.do_not_relay = meta.do_not_relay;
       txi.double_spend_seen = meta.double_spend_seen;
       tx_infos.push_back(txi);

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

src/rpc/core_rpc_server.cpp

@@ -1358,14 +1358,10 @@ namespace cryptonote
   {
     RPC_TRACKER(get_transaction_pool);
 
-    const bool restricted = m_restricted && ctx;
+    size_t n_txes = m_core.get_pool_transactions_count(true);
-    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);
     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);
     }

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;

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",

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)
+  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 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)
   {
     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");

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

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"));