From 8cecfc8d1bc1d645a37ddf07b10cb627ff215aaf Mon Sep 17 00:00:00 2001 From: Navid Rahimi Date: Mon, 2 Mar 2026 07:17:56 +0000 Subject: [PATCH] src: fix rpc limit Co-authored-by: jeffro256 --- src/cryptonote_core/cryptonote_core.cpp | 8 +- src/cryptonote_core/cryptonote_core.h | 4 +- src/cryptonote_core/tx_pool.cpp | 110 ++++++++++++++------- src/cryptonote_core/tx_pool.h | 6 +- src/rpc/core_rpc_server.cpp | 121 ++++++++++++++---------- 5 files changed, 155 insertions(+), 94 deletions(-) diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp index 473afa450..f78a958d7 100644 --- a/src/cryptonote_core/cryptonote_core.cpp +++ b/src/cryptonote_core/cryptonote_core.cpp @@ -1546,9 +1546,9 @@ namespace cryptonote return m_blockchain_storage.have_block(id, where); } //----------------------------------------------------------------------------------------------- - bool core::get_pool_transactions_info(const std::vector& txids, std::vector>& txs, bool include_sensitive_txes) const + bool core::get_pool_transactions_info(const std::vector& txids, std::vector>& txs, bool include_sensitive_txes, size_t limit_size) const { - return m_mempool.get_transactions_info(txids, txs, include_sensitive_txes); + return m_mempool.get_transactions_info(epee::to_span(txids), txs, include_sensitive_txes, limit_size); } //----------------------------------------------------------------------------------------------- bool core::get_pool_transactions(std::vector& txs, bool include_sensitive_data) const @@ -1563,9 +1563,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>& added_txs, std::vector& remaining_added_txids, std::vector& removed_txs, bool& incremental) const + bool core::get_pool_info(time_t start_time, bool include_sensitive_txes, size_t max_tx_count, std::vector>& added_txs, std::vector& remaining_added_txids, std::vector& removed_txs, bool& incremental, size_t limit_size) const { - return m_mempool.get_pool_info(start_time, include_sensitive_txes, max_tx_count, added_txs, remaining_added_txids, removed_txs, incremental); + return m_mempool.get_pool_info(start_time, include_sensitive_txes, max_tx_count, added_txs, remaining_added_txids, removed_txs, incremental, limit_size); } //----------------------------------------------------------------------------------------------- bool core::get_pool_transaction_stats(struct txpool_stats& stats, bool include_sensitive_data) const diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h index e9b2571b5..773e154bf 100644 --- a/src/cryptonote_core/cryptonote_core.h +++ b/src/cryptonote_core/cryptonote_core.h @@ -511,7 +511,7 @@ namespace cryptonote * * @note see tx_memory_pool::get_pool_transactions_info */ - bool get_pool_transactions_info(const std::vector& txids, std::vector>& txs, bool include_sensitive_txes = false) const; + bool get_pool_transactions_info(const std::vector& txids, std::vector>& txs, bool include_sensitive_txes = false, size_t limit_size = 0) const; /** * @copydoc tx_memory_pool::get_pool_info @@ -520,7 +520,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>& added_txs, std::vector& remaining_added_txids, std::vector& removed_txs, bool& incremental) const; + bool get_pool_info(time_t start_time, bool include_sensitive_txes, size_t max_tx_count, std::vector>& added_txs, std::vector& remaining_added_txids, std::vector& removed_txs, bool& incremental, size_t limit_size = 0) const; /** * @copydoc tx_memory_pool::get_transactions diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index d6824049c..d72c92ffd 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -665,22 +665,35 @@ namespace cryptonote return true; } //------------------------------------------------------------------ - bool tx_memory_pool::get_transactions_info(const std::vector& txids, std::vector>& txs, bool include_sensitive) const + bool tx_memory_pool::get_transactions_info(const epee::span txids, std::vector>& txs, bool include_sensitive, size_t cumul_txblob_size_limit, size_t max_tx_count) const { CRITICAL_REGION_LOCAL(m_transactions_lock); CRITICAL_REGION_LOCAL1(m_blockchain); txs.clear(); - for (const auto &it: txids) - { - tx_details details; - bool success = get_transaction_info(it, details, include_sensitive, true/*include_blob*/); - if (success) + const size_t max_allowed = + max_tx_count ? std::min(max_tx_count, txids.size()) : txids.size(); + + txs.reserve(std::min(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) { - txs.push_back(std::make_pair(it, std::move(details))); + const crypto::hash &it{txids[i]}; + tx_details details; + const bool success = get_transaction_info(it, details, include_sensitive, true /*include_blob*/); + 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)); } - } + return true; } //--------------------------------------------------------------------------------- @@ -952,7 +965,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>& added_txs, std::vector& remaining_added_txids, std::vector& removed_txs, bool& incremental) const + bool tx_memory_pool::get_pool_info(time_t start_time, bool include_sensitive, size_t max_tx_count, std::vector>& added_txs, std::vector& remaining_added_txids, std::vector& removed_txs, bool& incremental, size_t cumul_limit_size) const { CRITICAL_REGION_LOCAL(m_transactions_lock); CRITICAL_REGION_LOCAL1(m_blockchain); @@ -983,47 +996,76 @@ namespace cryptonote remaining_added_txids.clear(); removed_txs.clear(); - std::vector txids; if (!incremental) - { LOG_PRINT_L2("Giving back the whole pool"); - // Give back the whole pool in 'added_txs'; because calling 'get_transaction_info' right inside the - // anonymous method somehow results in an LMDB error with transactions we have to build a list of - // ids first and get the full info afterwards - get_transaction_hashes(txids, include_sensitive); - if (txids.size() > max_tx_count) + + // If incremental, handle removed TXIDs first since it's important that txs are removed + // from synchronizers' pools, and we need need to estimate how much space we have left to + // request full-bodied txs + if (incremental) + { + std::multimap::const_iterator rit = m_removed_txs_by_time.lower_bound(start_time); + while (rit != m_removed_txs_by_time.end()) { - remaining_added_txids = std::vector(txids.begin() + max_tx_count, txids.end()); - txids.erase(txids.begin() + max_tx_count, txids.end()); + if (include_sensitive || !rit->second.sensitive) + { + removed_txs.push_back(rit->second.txid); + } + ++rit; } - get_transactions_info(txids, added_txs, include_sensitive); - return true; } - // Give back incrementally, based on time of entry into the map + // Build a list of TXIDs that we want information for eventually. When `!incremental`, we might + // collect TXIDs that have already left the pool which will cause a followup RPC call that + // fetches non-existent txs, but that's *okay* since a non-incremental caller probably doesn't + // care about being incredibly efficient. This problem is also unavoidable anyways if we need to + // ever return anything inside `remaining_added_txids`, since those txs might have left the mempool + // by the time the followup RPC call reaches the daemon + std::vector txids; + txids.reserve(m_added_txs_by_id.size()); for (const auto &pit : m_added_txs_by_id) { - if (pit.second >= start_time) + const bool relevant_txid{!incremental || pit.second >= start_time}; + if (relevant_txid) txids.push_back(pit.first); } - get_transactions_info(txids, added_txs, include_sensitive); - if (added_txs.size() > max_tx_count) + + // Estimate max cumulative size left for full tx blobs + const size_t removed_txids_clawback{32 * removed_txs.size()}; + 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) { - remaining_added_txids.reserve(added_txs.size() - max_tx_count); - for (size_t i = max_tx_count; i < added_txs.size(); ++i) - remaining_added_txids.push_back(added_txs[i].first); - added_txs.erase(added_txs.begin() + max_tx_count, added_txs.end()); + if (!get_transactions_info(epee::to_span(txids), + added_txs, + include_sensitive, + cumulative_txblob_size_limit, + max_tx_count)) + return false; } - std::multimap::const_iterator rit = m_removed_txs_by_time.lower_bound(start_time); - while (rit != m_removed_txs_by_time.end()) + // Populate `remaining_added_txids` with all TXIDs in `txids` and not in `added_txs` + if (added_txs.size() < txids.size()) { - if (include_sensitive || !rit->second.sensitive) - { - removed_txs.push_back(rit->second.txid); + // 1. Create a set of added IDs for fast lookup + std::unordered_set added_set; + for (const auto& tx : added_txs) { + added_set.insert(tx.first); + } + + // 2. Iterate the original list. If not in set, it is "remaining". + remaining_added_txids.reserve(txids.size() - added_txs.size()); + for (const auto& id : txids) { + if (added_set.find(id) == added_set.end()) { + remaining_added_txids.push_back(id); + } } - ++rit; } + return true; } //------------------------------------------------------------------ diff --git a/src/cryptonote_core/tx_pool.h b/src/cryptonote_core/tx_pool.h index 8528e63c8..fbdf7b26d 100644 --- a/src/cryptonote_core/tx_pool.h +++ b/src/cryptonote_core/tx_pool.h @@ -492,14 +492,14 @@ namespace cryptonote }; /** - * @brief get infornation about a single transaction + * @brief get information about a single transaction */ bool get_transaction_info(const crypto::hash &txid, tx_details &td, bool include_sensitive_data, bool include_blob = false) const; /** * @brief get information about multiple transactions */ - bool get_transactions_info(const std::vector& txids, std::vector>& txs, bool include_sensitive_data = false) const; + bool get_transactions_info(const epee::span txids, std::vector>& txs, bool include_sensitive_data = false, size_t cumul_txblob_size_limit = 0, size_t max_tx_count = 0) const; /** * @brief get transactions not in the passed set @@ -511,7 +511,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>& added_txs, std::vector& remaining_added_txids, std::vector& removed_txs, bool& incremental) const; + bool get_pool_info(time_t start_time, bool include_sensitive, size_t max_tx_count, std::vector>& added_txs, std::vector& remaining_added_txids, std::vector& removed_txs, bool& incremental, size_t cumul_limit_size = 0) const; private: diff --git a/src/rpc/core_rpc_server.cpp b/src/rpc/core_rpc_server.cpp index bf4cdb4cf..68fb54a60 100644 --- a/src/rpc/core_rpc_server.cpp +++ b/src/rpc/core_rpc_server.cpp @@ -673,53 +673,7 @@ namespace cryptonote res.pool_info_extent = COMMAND_RPC_GET_BLOCKS_FAST::NONE; - if (get_pool) - { - 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::max(); - - bool incremental; - std::vector> 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 (success) - { - res.added_pool_txs.clear(); - if (m_rpc_payment) - { - CHECK_PAYMENT_SAME_TS(req, res, added_pool_txs.size() * COST_PER_TX + (res.remaining_added_pool_txids.size() + res.removed_pool_txids.size()) * COST_PER_POOL_HASH); - } - for (const auto &added_pool_tx: added_pool_txs) - { - COMMAND_RPC_GET_BLOCKS_FAST::pool_tx_info info; - info.tx_hash = added_pool_tx.first; - std::stringstream oss; - binary_archive ar(oss); - bool r = req.prune - ? const_cast(added_pool_tx.second.tx).serialize_base(ar) - : ::serialization::serialize(ar, const_cast(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)); - } - } - if (success) - { - res.pool_info_extent = incremental ? COMMAND_RPC_GET_BLOCKS_FAST::INCREMENTAL : COMMAND_RPC_GET_BLOCKS_FAST::FULL; - } - else - { - res.status = "Failed to get pool info"; - return true; - } - } - + size_t cumul_block_data_size = 0; if (get_blocks) { // quick check for noop @@ -764,7 +718,7 @@ namespace cryptonote CHECK_PAYMENT_SAME_TS(req, res, bs.size() * COST_PER_BLOCK); - size_t size = 0, ntxes = 0; + size_t ntxes = 0; res.blocks.reserve(bs.size()); res.output_indices.reserve(bs.size()); for(auto& bd: bs) @@ -772,7 +726,7 @@ namespace cryptonote res.blocks.resize(res.blocks.size()+1); res.blocks.back().pruned = req.prune; res.blocks.back().block = bd.first.first; - size += bd.first.first.size(); + cumul_block_data_size += bd.first.first.size(); res.output_indices.push_back(COMMAND_RPC_GET_BLOCKS_FAST::block_output_indices()); ntxes += bd.second.size(); res.output_indices.back().indices.reserve(1 + bd.second.size()); @@ -784,7 +738,7 @@ namespace cryptonote res.blocks.back().txs.push_back({std::move(i->second), crypto::null_hash}); i->second.clear(); i->second.shrink_to_fit(); - size += res.blocks.back().txs.back().blob.size(); + cumul_block_data_size += res.blocks.back().txs.back().blob.size(); } const size_t n_txes_to_lookup = bd.second.size() + (req.no_miner_tx ? 0 : 1); @@ -806,7 +760,72 @@ namespace cryptonote res.output_indices.back().indices.push_back({std::move(indices[i])}); } } - MDEBUG("on_get_blocks: " << bs.size() << " blocks, " << ntxes << " txes, size " << size); + MDEBUG("on_get_blocks: " << bs.size() << " blocks, " << ntxes << " txes, size " << cumul_block_data_size); + } + + if (get_pool) + { + 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::max(); + + const size_t limit = LEVIN_DEFAULT_MAX_PACKET_SIZE * 0.9; + + // If the blocks alone already consume the safe packet budget, skip pool info + if (cumul_block_data_size >= limit) + { + LOG_ERROR("on_get_blocks: omitting pool info, response already " + << cumul_block_data_size << " bytes (limit " << limit << ")"); + // res.pool_info_extent stays as NONE (set earlier), which tells wallets + // that there is no pool info in this response. + } + else + { + const size_t pool_limit = limit - cumul_block_data_size; + + bool incremental; + std::vector> 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, pool_limit); + + if (!success) + { + res.status = "Failed to get pool info"; + return true; + } + + res.added_pool_txs.clear(); + if (m_rpc_payment) + { + CHECK_PAYMENT_SAME_TS(req, res, + added_pool_txs.size() * COST_PER_TX + + (res.remaining_added_pool_txids.size() + res.removed_pool_txids.size()) * COST_PER_POOL_HASH); + } + for (const auto &added_pool_tx: added_pool_txs) + { + COMMAND_RPC_GET_BLOCKS_FAST::pool_tx_info info; + info.tx_hash = added_pool_tx.first; + std::stringstream oss; + binary_archive ar(oss); + bool r = req.prune + ? const_cast(added_pool_tx.second.tx).serialize_base(ar) + : ::serialization::serialize(ar, const_cast(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.status = CORE_RPC_STATUS_OK;