What the developer wrote
Authored by tobtoht
Merge pull request #11006
967aa9f cryptonote_core: fully parse incoming block batches (selsta)
e59781a cryptonote_protocol: avoid overlapping reserved spans (selsta)
ACKs: jpk68, plowsof
What changed, and why it matters
This Monero update fixes two related bugs in how the node downloads and processes batches of blocks from peers. First, it ensures every block in an incoming batch is fully parsed even when some blocks are already known, so the node does not accidentally skip validation steps. Second, it prevents the node from asking different peers for the same block ranges twice at the same time, which could waste bandwidth and potentially confuse the sync logic. The commit does not describe these changes as security fixes, but they touch on consensus-critical code paths.
Treat as a routine but important correctness patch for Monero's P2P synchronization layer. Nodes should upgrade to avoid sync inefficiencies and edge cases in block validation. Security teams should monitor for follow-up disclosures or CVE assignment, since the commit does not explicitly rule out security relevance.
Security signals we found
Change in consensus-critical block ingestion path (prepare_handle_incoming_blocks)
Incomplete parsing of incoming block batch under prior blocks_exist short-circuit
Overlapping block span reservation between peers in block queue
Unit test added to enforce non-overlapping span reservation behavior
No explicit security framing or CVE reference in commit message
Evidence from the diff
The patch contains two logical fixes. In blockchain.cpp, prepare_handle_incoming_blocks() now continues parsing all blocks in a batch even after blocks_exist becomes true, and only sets the flag once. Previously the loop short-circuited parsing once a duplicate was found, which could leave later blocks unvalidated. In block_queue.cpp, reserve_span() now checks !requested_internal((*i).first) before extending a span, preventing it from reserving a range that overlaps an already-requested span. A unit test verifies that a larger recalculated sync size fills only the gap left by a removed span and does not overlap the remaining reserved span.
Changed components
src/cryptonote_core/blockchain.cppsrc/cryptonote_protocol/block_queue.cpptests/unit_tests/block_queue.cppInspect captured patch +25 / −4
### src/cryptonote_core/blockchain.cpp
@@ -4975,22 +4975,22 @@ bool Blockchain::prepare_handle_incoming_blocks(const std::vector<block_complete
return true;
}
}
- if (have_block(block_hash))
+ if (!blocks_exist && have_block(block_hash))
blocks_exist = true;
std::advance(it, 1);
}
}
- for (unsigned i = 0; i < extra && !blocks_exist; i++, blockidx++)
+ for (unsigned i = 0; i < extra; i++, blockidx++)
{
block &block = blocks[blockidx];
crypto::hash block_hash;
if (!parse_and_validate_block_from_blob(it->block, block, block_hash))
return false;
- if (have_block(block_hash))
+ if (!blocks_exist && have_block(block_hash))
blocks_exist = true;
std::advance(it, 1);
### src/cryptonote_protocol/block_queue.cpp
@@ -297,7 +297,8 @@ std::pair<uint64_t, uint64_t> block_queue::reserve_span(uint64_t first_block_hei
uint64_t span_length = 0;
std::vector<crypto::hash> hashes;
bool first_is_pruned = sync_pruned_blocks && !tools::has_unpruned_block(span_start_height + span_length, blockchain_height, local_pruning_seed);
- while (i != block_hashes.end() && span_length < max_blocks && (sync_pruned_blocks || tools::has_unpruned_block(span_start_height + span_length, blockchain_height, pruning_seed)))
+ while (i != block_hashes.end() && span_length < max_blocks && !requested_internal((*i).first) &&
+ (sync_pruned_blocks || tools::has_unpruned_block(span_start_height + span_length, blockchain_height, pruning_seed)))
{
// if we want to sync pruned blocks, stop at the first block for which we need full data
if (sync_pruned_blocks && first_is_pruned == tools::has_unpruned_block(span_start_height + span_length, blockchain_height, local_pruning_seed))
### tests/unit_tests/block_queue.cpp
@@ -87,3 +87,23 @@ TEST(block_queue, flush_uuid)
bq.add_blocks(0, 200, uuid1(), na);
ASSERT_EQ(bq.get_max_block_height(), 399);
}
+
+TEST(block_queue, reserve_does_not_overlap_later_span)
+{
+ cryptonote::block_queue bq;
+ epee::net_utils::network_address na;
+ std::vector<std::pair<crypto::hash, uint64_t>> hashes;
+ hashes.reserve(100);
+ for (size_t i = 0; i < 100; ++i)
+ hashes.emplace_back(crypto::rand<crypto::hash>(), 0);
+
+ ASSERT_EQ(bq.reserve_span(1, 100, 28, uuid1(), na, false, 0, 0, 101, hashes), std::make_pair(uint64_t{1}, uint64_t{28}));
+ ASSERT_EQ(bq.reserve_span(1, 100, 28, uuid1(), na, false, 0, 0, 101, hashes), std::make_pair(uint64_t{29}, uint64_t{28}));
+ ASSERT_EQ(bq.reserve_span(1, 100, 28, uuid1(), na, false, 0, 0, 101, hashes), std::make_pair(uint64_t{57}, uint64_t{28}));
+
+ ASSERT_TRUE(bq.remove_span(29));
+
+ // A recalculated, larger sync size must fill only the gap, without
+ // overlapping the span which is already reserved at height 57.
+ ASSERT_EQ(bq.reserve_span(1, 100, 36, uuid2(), na, false, 0, 0, 101, hashes), std::make_pair(uint64_t{29}, uint64_t{28}));
+}Why this scored 57/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.