wallet2: honor max_blocks bound during fast_refresh hash pulls
What changed, and why it matters
This change adjusts how a Monero wallet fetches block hashes during its fast refresh mode. Previously, fast_refresh could keep pulling hashes without respecting the caller's max_blocks limit, which could make a single refresh call run for a very long time and appear unresponsive. The patch caps the number of hash pulls per refresh call and lets the wallet resume later, improving responsiveness. It is a robustness/usability fix rather than a clear-cut security vulnerability, though unbounded network loops can sometimes be abused to delay or hang wallet operations.
Treat as a hardening/responsiveness improvement. No immediate security response required unless incident data shows the unbounded fast_refresh was exploited to hang wallets or daemons. Review callers of refresh() to ensure they handle partial progress and resume correctly.
Security signals we found
Unbounded loop in network-driven code path bounded by external parameter
RPC amplification / potential denial-of-service via long-running refresh
Wallet responsiveness / availability fix
No input validation bug or cryptographic flaw visible in diff
Evidence from the diff
wallet2::fast_refresh previously pulled block hashes in a while loop until reaching stop_height, ignoring the max_blocks bound passed to refresh(). The patch adds a max_pulls parameter and a MAX_HASH_PULLS_PER_REFRESH constant (3). When max_blocks is finite, fast_refresh returns false after exceeding the pull budget, and refresh() returns early, reporting blocks_fetched so the caller can resume. The function signature changes from void to bool, with all early returns now returning true except the budget-exhausted path. This prevents a single refresh invocation from performing an unbounded number of pull_hashes RPC calls.
Changed components
src/wallet/wallet2.cppsrc/wallet/wallet2.hwallet2::fast_refreshwallet2::refreshInspect captured patch +20 / −7
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index 3cb8c6f..eb7474a 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -143,6 +143,8 @@ using namespace cryptonote;
#define FIRST_REFRESH_GRANULARITY 1024
+#define MAX_HASH_PULLS_PER_REFRESH 3 // hash pulls per bounded refresh call, so single-threaded callers stay responsive
+
#define GAMMA_SHAPE 19.28
#define GAMMA_SCALE (1/1.61)
@@ -3937,9 +3939,10 @@ void wallet2::process_pool_state(const std::vector<std::tuple<cryptonote::transa
MTRACE("process_pool_state end");
}
//----------------------------------------------------------------------------------------------------
-void wallet2::fast_refresh(uint64_t stop_height, uint64_t &blocks_start_height, std::list<crypto::hash> &short_chain_history, bool force)
+bool wallet2::fast_refresh(uint64_t stop_height, uint64_t &blocks_start_height, std::list<crypto::hash> &short_chain_history, bool force, uint64_t max_pulls)
{
std::vector<crypto::hash> hashes;
+ uint64_t num_pulls = 0;
const uint64_t checkpoint_height = (stop_height < 1000) ? 0 : m_checkpoints.get_nearest_checkpoint_height(stop_height);
if ((stop_height > checkpoint_height && m_blockchain.size()-1 < checkpoint_height) && !force)
@@ -3957,13 +3960,15 @@ void wallet2::fast_refresh(uint64_t stop_height, uint64_t &blocks_start_height,
size_t current_index = m_blockchain.size();
while(m_run.load(std::memory_order_relaxed) && current_index < stop_height)
{
+ if (max_pulls > 0 && num_pulls++ >= max_pulls)
+ return false; // pull budget reached, caller may resume on a later call
pull_hashes(0, blocks_start_height, short_chain_history, hashes);
if (hashes.size() <= 3)
- return;
+ return true;
if (blocks_start_height < m_blockchain.offset())
{
MERROR("Blocks start before blockchain offset: " << blocks_start_height << " " << m_blockchain.offset());
- return;
+ return true;
}
current_index = blocks_start_height;
if (hashes.size() + current_index < stop_height) {
@@ -3992,13 +3997,14 @@ void wallet2::fast_refresh(uint64_t stop_height, uint64_t &blocks_start_height,
else if(bl_id != m_blockchain[current_index])
{
//split detected here !!!
- return;
+ return true;
}
++current_index;
if (current_index >= stop_height)
- return;
+ return true;
}
}
+ return true;
}
@@ -4101,7 +4107,14 @@ void wallet2::refresh(bool trusted_daemon, uint64_t start_height, uint64_t & blo
if (!start_height)
start_height = std::max(m_refresh_from_block_height, m_skip_to_height);;
// we can shortcut by only pulling hashes up to the start_height
- fast_refresh(start_height, blocks_start_height, short_chain_history);
+ const uint64_t pre_hashes_height = m_blockchain.size();
+ const uint64_t max_pulls = max_blocks == std::numeric_limits<uint64_t>::max() ? 0 : MAX_HASH_PULLS_PER_REFRESH;
+ if (!fast_refresh(start_height, blocks_start_height, short_chain_history, false, max_pulls))
+ {
+ // pull budget reached before start_height, report hashes added and resume on next call
+ blocks_fetched = m_blockchain.size() - pre_hashes_height;
+ return;
+ }
// regenerate the history now that we've got a full set of hashes
short_chain_history.clear();
get_short_chain_history(short_chain_history, (m_first_refresh_done || trusted_daemon) ? 1 : FIRST_REFRESH_GRANULARITY);
diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h
index e310d64..2f503be 100644
--- a/src/wallet/wallet2.h
+++ b/src/wallet/wallet2.h
@@ -1531,7 +1531,7 @@ private:
void clear_user_data();
void pull_blocks(bool first, bool try_incremental, uint64_t start_height, uint64_t& blocks_start_height, const std::list<crypto::hash> &short_chain_history, std::vector<cryptonote::block_complete_entry> &blocks, std::vector<cryptonote::COMMAND_RPC_GET_BLOCKS_FAST::block_output_indices> &o_indices, uint64_t ¤t_height, std::vector<std::tuple<cryptonote::transaction, crypto::hash, bool>>& process_pool_txs);
void pull_hashes(uint64_t start_height, uint64_t& blocks_start_height, const std::list<crypto::hash> &short_chain_history, std::vector<crypto::hash> &hashes);
- void fast_refresh(uint64_t stop_height, uint64_t &blocks_start_height, std::list<crypto::hash> &short_chain_history, bool force = false);
+ bool fast_refresh(uint64_t stop_height, uint64_t &blocks_start_height, std::list<crypto::hash> &short_chain_history, bool force = false, uint64_t max_pulls = 0);
void pull_and_parse_next_blocks(bool first, bool try_incremental, uint64_t start_height, uint64_t &blocks_start_height, std::list<crypto::hash> &short_chain_history, const std::vector<cryptonote::block_complete_entry> &prev_blocks, const std::vector<parsed_block> &prev_parsed_blocks, std::vector<cryptonote::block_complete_entry> &blocks, std::vector<parsed_block> &parsed_blocks, std::vector<std::tuple<cryptonote::transaction, crypto::hash, bool>>& process_pool_txs, bool &last, bool &error, std::exception_ptr &exception);
void process_parsed_blocks(const uint64_t start_height, const std::vector<cryptonote::block_complete_entry> &blocks, const std::vector<parsed_block> &parsed_blocks, uint64_t& blocks_added, std::map<std::pair<uint64_t, uint64_t>, size_t> *output_tracker_cache = NULL);
bool accept_pool_tx_for_processing(const crypto::hash &txid, const std::unordered_set<crypto::hash> &payments_tx_hashes);
Why this scored 26/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.