rpc: replace get_blocks.bin Levin pool budget

/get_blocks.bin is HTTP binary RPC serialized with portable storage, not a
P2P Levin message. The previously added pool limiter used
LEVIN_DEFAULT_MAX_PACKET_SIZE as a response budget, but that constant does
not apply to the RPC path.

src/cryptonote_core/cryptonote_core.cpp

@@ -1520,9 +1520,9 @@ namespace cryptonote
     return m_blockchain_storage.have_block(id, where);
   }
   //-----------------------------------------------------------------------------------------------
-  bool core::get_pool_transactions_info(const std::vector<crypto::hash>& txids, std::vector<std::pair<crypto::hash, tx_memory_pool::tx_details>>& txs, bool include_sensitive_txes, size_t limit_size) const
+  bool core::get_pool_transactions_info(const std::vector<crypto::hash>& txids, std::vector<std::pair<crypto::hash, tx_memory_pool::tx_details>>& txs, bool include_sensitive_txes) const
   {
-    return m_mempool.get_transactions_info(epee::to_span(txids), txs, include_sensitive_txes, limit_size);
+    return m_mempool.get_transactions_info(epee::to_span(txids), txs, include_sensitive_txes);
   }
   //-----------------------------------------------------------------------------------------------
   bool core::get_pool_transactions(std::vector<transaction>& txs, bool include_sensitive_data) const
@@ -1537,9 +1537,9 @@ namespace cryptonote
     return true;
   }
   //-----------------------------------------------------------------------------------------------
-  bool core::get_pool_info(time_t start_time, bool include_sensitive_txes, size_t max_tx_count, std::vector<std::pair<crypto::hash, tx_memory_pool::tx_details>>& added_txs, std::vector<crypto::hash>& remaining_added_txids, std::vector<crypto::hash>& removed_txs, bool& incremental, size_t limit_size) const
+  bool core::get_pool_info(time_t start_time, bool include_sensitive_txes, size_t max_tx_count, std::vector<std::pair<crypto::hash, tx_memory_pool::tx_details>>& added_txs, std::vector<crypto::hash>& remaining_added_txids, std::vector<crypto::hash>& removed_txs, bool& incremental) const
   {
-    return m_mempool.get_pool_info(start_time, include_sensitive_txes, max_tx_count, added_txs, remaining_added_txids, removed_txs, incremental, limit_size);
+    return m_mempool.get_pool_info(start_time, include_sensitive_txes, max_tx_count, added_txs, remaining_added_txids, removed_txs, incremental);
   }
   //-----------------------------------------------------------------------------------------------
   bool core::get_pool_transaction_stats(struct txpool_stats& stats, bool include_sensitive_data) const

src/cryptonote_core/cryptonote_core.h

@@ -491,7 +491,7 @@ namespace cryptonote
       *
       * @note see tx_memory_pool::get_pool_transactions_info
       */
-     bool get_pool_transactions_info(const std::vector<crypto::hash>& txids, std::vector<std::pair<crypto::hash, tx_memory_pool::tx_details>>& txs, bool include_sensitive_txes = false, size_t limit_size = 0) const;
+     bool get_pool_transactions_info(const std::vector<crypto::hash>& txids, std::vector<std::pair<crypto::hash, tx_memory_pool::tx_details>>& txs, bool include_sensitive_txes = false) const;
 
      /**
       * @copydoc tx_memory_pool::get_pool_info
@@ -500,7 +500,7 @@ namespace cryptonote
       *
       * @note see tx_memory_pool::get_pool_info
       */
-     bool get_pool_info(time_t start_time, bool include_sensitive_txes, size_t max_tx_count, std::vector<std::pair<crypto::hash, tx_memory_pool::tx_details>>& added_txs, std::vector<crypto::hash>& remaining_added_txids, std::vector<crypto::hash>& removed_txs, bool& incremental, size_t limit_size = 0) const;
+     bool get_pool_info(time_t start_time, bool include_sensitive_txes, size_t max_tx_count, std::vector<std::pair<crypto::hash, tx_memory_pool::tx_details>>& added_txs, std::vector<crypto::hash>& remaining_added_txids, std::vector<crypto::hash>& removed_txs, bool& incremental) const;
 
     /**
       * @copydoc tx_memory_pool::get_transactions

src/cryptonote_core/tx_pool.cpp

@@ -665,7 +665,7 @@ namespace cryptonote
     return true;
   }
   //------------------------------------------------------------------
-  bool tx_memory_pool::get_transactions_info(const epee::span<const crypto::hash> txids, std::vector<std::pair<crypto::hash, tx_details>>& txs, bool include_sensitive, size_t cumul_txblob_size_limit, size_t max_tx_count) const
+  bool tx_memory_pool::get_transactions_info(const epee::span<const crypto::hash> txids, std::vector<std::pair<crypto::hash, tx_details>>& txs, bool include_sensitive, size_t max_tx_count) const
   {
     CRITICAL_REGION_LOCAL(m_transactions_lock);
     CRITICAL_REGION_LOCAL1(m_blockchain);
@@ -676,7 +676,6 @@ namespace cryptonote
       max_tx_count ? std::min(max_tx_count, txids.size()) : txids.size();
 
     txs.reserve(std::min<size_t>(max_allowed, 1000000)); // reserve limited to min(1 million, max_allowed)
-    size_t cumul_txblob_size = 0;
 
     for (size_t i = 0; i < txids.size() && txs.size() < max_allowed; ++i)
       {
@@ -686,11 +685,6 @@ namespace cryptonote
         if (!success)
           continue;
 
-        if (cumul_txblob_size_limit &&
-            cumul_txblob_size + details.blob_size >= cumul_txblob_size_limit)
-          continue;
-
-        cumul_txblob_size += details.blob_size;
         txs.emplace_back(it, std::move(details));
       }
 
@@ -972,7 +966,7 @@ namespace cryptonote
     }, false, category);
   }
   //------------------------------------------------------------------
-  bool tx_memory_pool::get_pool_info(time_t start_time, bool include_sensitive, size_t max_tx_count, std::vector<std::pair<crypto::hash, tx_details>>& added_txs, std::vector<crypto::hash>& remaining_added_txids, std::vector<crypto::hash>& removed_txs, bool& incremental, size_t cumul_limit_size) const
+  bool tx_memory_pool::get_pool_info(time_t start_time, bool include_sensitive, size_t max_tx_count, std::vector<std::pair<crypto::hash, tx_details>>& added_txs, std::vector<crypto::hash>& remaining_added_txids, std::vector<crypto::hash>& removed_txs, bool& incremental) const
   {
     CRITICAL_REGION_LOCAL(m_transactions_lock);
     CRITICAL_REGION_LOCAL1(m_blockchain);
@@ -1007,8 +1001,7 @@ namespace cryptonote
       LOG_PRINT_L2("Giving back the whole pool");
 
     // If incremental, handle removed TXIDs first since it's important that txs are removed
-    // from synchronizers' pools, and we need to estimate how much space we have left to
+    // from synchronizers' pools.
-    // request full-bodied txs
     if (incremental)
     {
       std::multimap<time_t, removed_tx_info>::const_iterator rit = m_removed_txs_by_time.lower_bound(start_time);
@@ -1039,20 +1032,12 @@ namespace cryptonote
         txids.push_back(pit.first);
     }
 
-    // Estimate max cumulative size left for full tx blobs
+    // Perform TX info fetch, limited to max_tx_count.
-    const size_t removed_txids_clawback{32 * removed_txs.size()};
+    if (!txids.empty())
-    const size_t remaining_txids_clawback{32 * txids.size()};
-    const size_t added_tx_txid_clawback(32 * txids.size());
-    const size_t total_clawback{removed_txids_clawback + remaining_txids_clawback + added_tx_txid_clawback};
-    const size_t cumulative_txblob_size_limit{cumul_limit_size > total_clawback ? cumul_limit_size - total_clawback : 0};
-
-    // Perform TX info fetch, limited to max_tx_count and cumulative_txblob_size_limit
-    if (cumulative_txblob_size_limit && !txids.empty() && max_tx_count)
     {
       if (!get_transactions_info(epee::to_span(txids),
                                  added_txs,
                                  include_sensitive,
-                                 cumulative_txblob_size_limit,
                                  max_tx_count))
         return false;
     }

src/cryptonote_core/tx_pool.h

@@ -503,7 +503,7 @@ namespace cryptonote
     /**
      * @brief get information about multiple transactions
      */
-    bool get_transactions_info(const epee::span<const crypto::hash> txids, std::vector<std::pair<crypto::hash, tx_details>>& txs, bool include_sensitive_data = false, size_t cumul_txblob_size_limit = 0, size_t max_tx_count = 0) const;
+    bool get_transactions_info(const epee::span<const crypto::hash> txids, std::vector<std::pair<crypto::hash, tx_details>>& txs, bool include_sensitive_data = false, size_t max_tx_count = 0) const;
 
     /**
      * @brief get transactions not in the passed set
@@ -515,7 +515,7 @@ namespace cryptonote
      *
      * @return true on success, false on error
      */
-    bool get_pool_info(time_t start_time, bool include_sensitive, size_t max_tx_count, std::vector<std::pair<crypto::hash, tx_details>>& added_txs, std::vector<crypto::hash>& remaining_added_txids, std::vector<crypto::hash>& removed_txs, bool& incremental, size_t cumul_limit_size = 0) const;
+    bool get_pool_info(time_t start_time, bool include_sensitive, size_t max_tx_count, std::vector<std::pair<crypto::hash, tx_details>>& added_txs, std::vector<crypto::hash>& remaining_added_txids, std::vector<crypto::hash>& removed_txs, bool& incremental) const;
 
   private:
 

src/rpc/core_rpc_server.cpp

@@ -75,6 +75,8 @@ using namespace epee;
 #define RESTRICTED_SPENT_KEY_IMAGES_COUNT 5000
 #define RESTRICTED_BLOCK_COUNT 1000
 
+static constexpr size_t GET_BLOCKS_BIN_MAX_ADDED_POOL_TX_BODIES = 20000;
+
 #define RPC_TRACKER(rpc) \
   PERF_TIMER(rpc); \
   RPCTracker tracker(#rpc, PERF_TIMER_NAME(rpc))
@@ -519,10 +521,8 @@ namespace cryptonote
     return true;
   }
   //------------------------------------------------------------------------------------------------------------------------------
-  bool core_rpc_server::handle_get_blocks(const COMMAND_RPC_GET_BLOCKS_FAST::request& req, COMMAND_RPC_GET_BLOCKS_FAST::response& res, const connection_context *ctx, std::size_t &cumul_block_data_size)
+  bool core_rpc_server::handle_get_blocks(const COMMAND_RPC_GET_BLOCKS_FAST::request& req, COMMAND_RPC_GET_BLOCKS_FAST::response& res, const connection_context *ctx)
   {
-    cumul_block_data_size = 0;
-
     // quick check for noop
     if (req.start_height > 0 || !req.block_ids.empty())
     {
@@ -552,6 +552,7 @@ namespace cryptonote
       return false;
     }
 
+    size_t cumul_block_data_size = 0;
     size_t ntxes = 0;
     res.blocks.reserve(bs.size());
     res.output_indices.reserve(bs.size());
@@ -647,10 +648,9 @@ namespace cryptonote
 
     res.pool_info_extent = COMMAND_RPC_GET_BLOCKS_FAST::NONE;
 
-    size_t cumul_block_data_size = 0;
     if (get_blocks)
     {
-      if (!handle_get_blocks(req, res, ctx, cumul_block_data_size))
+      if (!handle_get_blocks(req, res, ctx))
       {
         res.status = "Failed";
         return true;
@@ -662,58 +662,45 @@ namespace cryptonote
       const bool restricted = m_restricted && ctx;
       const bool request_has_rpc_origin = ctx != NULL;
       const bool allow_sensitive = !request_has_rpc_origin || !restricted;
-      const size_t max_tx_count = restricted ? RESTRICTED_TRANSACTIONS_COUNT : std::numeric_limits<size_t>::max();
+      const size_t max_tx_count = restricted
+        ? std::min<size_t>(RESTRICTED_TRANSACTIONS_COUNT, GET_BLOCKS_BIN_MAX_ADDED_POOL_TX_BODIES)
+        : GET_BLOCKS_BIN_MAX_ADDED_POOL_TX_BODIES;
 
-      const size_t limit = LEVIN_DEFAULT_MAX_PACKET_SIZE * 0.9;
+      bool incremental;
+      std::vector<std::pair<crypto::hash, tx_memory_pool::tx_details>> added_pool_txs;
+      bool success = m_core.get_pool_info((time_t)req.pool_info_since, allow_sensitive, max_tx_count,
+                                          added_pool_txs, res.remaining_added_pool_txids, res.removed_pool_txids,
+                                          incremental);
 
-      // If the blocks alone already consume the safe packet budget, skip pool info
+      if (!success)
-      if (cumul_block_data_size >= limit)
       {
-        LOG_ERROR("on_get_blocks: omitting pool info, response already "
+        res.status = "Failed to get pool info";
-               << cumul_block_data_size << " bytes (limit " << limit << ")");
+        return true;
-        // res.pool_info_extent stays as NONE (set earlier), which tells wallets
-        // that there is no pool info in this response.
       }
-      else
+
+      res.added_pool_txs.clear();
+      for (const auto &added_pool_tx: added_pool_txs)
       {
-        const size_t pool_limit = limit - cumul_block_data_size;
+        COMMAND_RPC_GET_BLOCKS_FAST::pool_tx_info info;
-
+        info.tx_hash = added_pool_tx.first;
-        bool incremental;
+        std::stringstream oss;
-        std::vector<std::pair<crypto::hash, tx_memory_pool::tx_details>> added_pool_txs;
+        binary_archive<true> ar(oss);
-        bool success = m_core.get_pool_info((time_t)req.pool_info_since, allow_sensitive, max_tx_count,
+        bool r = req.prune
-                                            added_pool_txs, res.remaining_added_pool_txids, res.removed_pool_txids,
+          ? const_cast<cryptonote::transaction&>(added_pool_tx.second.tx).serialize_base(ar)
-                                            incremental, pool_limit);
+          : ::serialization::serialize(ar, const_cast<cryptonote::transaction&>(added_pool_tx.second.tx));
-
+        if (!r)
-        if (!success)
         {
-          res.status = "Failed to get pool info";
+          res.status = "Failed to serialize transaction";
           return true;
         }
-
+        info.tx_blob = oss.str();
-        res.added_pool_txs.clear();
+        info.double_spend_seen = added_pool_tx.second.double_spend_seen;
-        for (const auto &added_pool_tx: added_pool_txs)
+        res.added_pool_txs.push_back(std::move(info));
-        {
-          COMMAND_RPC_GET_BLOCKS_FAST::pool_tx_info info;
-          info.tx_hash = added_pool_tx.first;
-          std::stringstream oss;
-          binary_archive<true> ar(oss);
-          bool r = req.prune
-            ? const_cast<cryptonote::transaction&>(added_pool_tx.second.tx).serialize_base(ar)
-            : ::serialization::serialize(ar, const_cast<cryptonote::transaction&>(added_pool_tx.second.tx));
-          if (!r)
-          {
-            res.status = "Failed to serialize transaction";
-            return true;
-          }
-          info.tx_blob = oss.str();
-          info.double_spend_seen = added_pool_tx.second.double_spend_seen;
-          res.added_pool_txs.push_back(std::move(info));
-        }
-
-        res.pool_info_extent = incremental
-          ? COMMAND_RPC_GET_BLOCKS_FAST::INCREMENTAL
-          : COMMAND_RPC_GET_BLOCKS_FAST::FULL;
       }
+
+      res.pool_info_extent = incremental
+        ? COMMAND_RPC_GET_BLOCKS_FAST::INCREMENTAL
+        : COMMAND_RPC_GET_BLOCKS_FAST::FULL;
     }
 
     res.status = CORE_RPC_STATUS_OK;

src/rpc/core_rpc_server.h

@@ -271,7 +271,7 @@ private:
     template <typename COMMAND_TYPE>
     bool use_bootstrap_daemon_if_necessary(const invoke_http_mode &mode, const std::string &command_name, const typename COMMAND_TYPE::request& req, typename COMMAND_TYPE::response& res, bool &r);
     bool get_block_template(const account_public_address &address, const crypto::hash *prev_block, const cryptonote::blobdata &extra_nonce, size_t &reserved_offset, cryptonote::difficulty_type &difficulty, uint64_t &height, uint64_t &expected_reward, uint64_t& cumulative_weight, block &b, uint64_t &seed_height, crypto::hash &seed_hash, crypto::hash &next_seed_hash, epee::json_rpc::error &error_resp);
-    bool handle_get_blocks(const COMMAND_RPC_GET_BLOCKS_FAST::request& req, COMMAND_RPC_GET_BLOCKS_FAST::response& res, const connection_context *ctx, std::size_t &cumul_block_data_size_out);
+    bool handle_get_blocks(const COMMAND_RPC_GET_BLOCKS_FAST::request& req, COMMAND_RPC_GET_BLOCKS_FAST::response& res, const connection_context *ctx);
 
     core& m_core;
     nodetool::node_server<cryptonote::t_cryptonote_protocol_handler<cryptonote::core> >& m_p2p;