cryptonote_protocol: accurate next_needed_height when there is an overlap
What the developer wrote
Authored by 0xFFFC0000
cryptonote_protocol: accurate next_needed_height when there is an overlap
What changed, and why it matters
This patch fixes a bookkeeping bug in how Monero decides which blockchain blocks to request next from peers. Previously, when two requested block ranges overlapped, the node could incorrectly think it already had a block it did not have, causing it to skip asking for that block. That could stall synchronization or leave a node stuck on an older chain. The fix correctly merges overlapping or adjacent ranges when calculating the next missing height.
Treat as a routine correctness fix. Include in release notes as a synchronization reliability improvement. No urgent security advisory is warranted based solely on the diff, but downstream nodes should update to avoid sync stalls.
Security signals we found
Protocol synchronization correctness bug
Potential denial-of-service via chain sync stall
No authentication/authorization boundary crossed
No memory safety or cryptographic flaw evident
Evidence from the diff
block_queue::get_next_needed_height() in src/cryptonote_protocol/block_queue.cpp previously tracked only last_needed_height and returned early if a span’s start did not exactly match it. When spans overlapped or were out of order, the function could return a height already covered by an earlier span, causing the node to omit a request for an uncovered block. The rewritten logic tracks covered_until, ignores spans wholly below the current chain height, returns the first gap when a span starts after covered_until, and extends coverage for overlapping/adjacent spans. This is a correctness fix in the P2P synchronization protocol.
Changed components
src/cryptonote_protocol/block_queue.cppblock_queue::get_next_needed_height()Monero P2P block synchronizationInspect captured patch +16 / −8
diff --git a/src/cryptonote_protocol/block_queue.cpp b/src/cryptonote_protocol/block_queue.cpp
index c5d37e1..52ad94d 100644
--- a/src/cryptonote_protocol/block_queue.cpp
+++ b/src/cryptonote_protocol/block_queue.cpp
@@ -153,18 +153,26 @@ uint64_t block_queue::get_next_needed_height(uint64_t blockchain_height) const
boost::unique_lock<boost::recursive_mutex> lock(mutex);
if (blocks.empty())
return blockchain_height;
- uint64_t last_needed_height = blockchain_height;
- bool first = true;
+
+ uint64_t covered_until = blockchain_height;
+
for (const auto &span: blocks)
{
- if (span.start_block_height + span.nblocks - 1 < blockchain_height)
+ // Ignore spans entirely below current chain height
+ const uint64_t span_end = span.start_block_height + span.nblocks - 1;
+ if (span_end < blockchain_height)
continue;
- if (span.start_block_height != last_needed_height || (first && span.blocks.empty()))
- return last_needed_height;
- last_needed_height = span.start_block_height + span.nblocks;
- first = false;
+
+ // If this span starts after what we already have/scheduled, we found the first gap
+ if (span.start_block_height > covered_until)
+ return covered_until;
+
+ // This span overlaps or is adjacent; extend coverage regardless of filled/scheduled
+ if (span.start_block_height <= covered_until)
+ covered_until = std::max(covered_until, span.start_block_height + span.nblocks);
}
- return last_needed_height;
+
+ return covered_until;
}
void block_queue::print() const
Why this scored 31/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.