refactor: extract BlockDownloadManager from PeerManagerImpl

w0xlt commented at 6:54 pm on February 11, 2026: contributor

Motivation

net_processing.cpp is the largest file in the codebase (~6200 lines) and PeerManagerImpl mixes several largely independent subsystems into a single class: transaction relay, address gossip, headers sync, compactblocks, and block download. This makes the file difficult to review, test in isolation, and reason about lock ordering.

#30110 successfully extracted transaction download logic into TxDownloadManager. This PR applies the same approach to block download, continuing the incremental decomposition of PeerManagerImpl.

What this PR does

Extract all block download state and scheduling logic into a new BlockDownloadManager module:

Global state moved (from PeerManagerImpl): mapBlocksInFlight, mapBlockSource, nSyncStarted, m_block_stalling_timeout, m_last_tip_update, m_num_preferred_download_peers, m_peers_downloading_from.

Per-peer state moved (from CNodeState): pindexBestKnownBlock, hashLastUnknownBlock, pindexLastCommonBlock, pindexBestHeaderSent, fSyncStarted, vBlocksInFlight, m_downloading_since, m_stalling_since, fPreferredDownload.

Methods moved (from PeerManagerImpl): FindNextBlocksToDownload, TryDownloadingHistoricalBlocks, ProcessBlockAvailability, UpdateBlockAvailability, BlockRequested, RemoveBlockRequest, IsBlockRequested, IsBlockRequestedFromOutbound, TipMayBeStale.

The result:

net_processing.cpp shrinks from 6193 to 5751 lines (−442 net)
CNodeState loses 10 fields, retaining only compact block relay and chain sync timeout state
Block download logic becomes unit-testable in isolation (8 test cases added)

Design

Follows the TxDownloadManager pimpl pattern:

blockdownloadman.h — public interface
blockdownloadman_impl.h — implementation class with per-peer state
blockdownloadman_impl.cpp — implementation

Unlike TxDownloadManager, no new mutex is introduced. Block download is inherently tied to chain state through CBlockIndex* pointers, so cs_main remains the synchronizing lock. The extraction is for code organization and testability, not lock granularity.

Commits

Add BlockDownloadManager class with pimpl skeleton: all new module files, no call sites changed
Add unit tests: exercises the module in isolation
Migrate call sites: ~150 call sites in net_processing.cpp rewired through m_blockdownloadman, pure mechanical replacement

refactor: add BlockDownloadManager class with pimpl pattern

Create a new BlockDownloadManager module following the established
TxDownloadManager pimpl pattern:

- blockdownloadman.h: public interface with QueuedBlock, options/info
  structs, and full BlockDownloadManager API
- blockdownloadman_impl.h: BlockDownloadManagerImpl with
  PeerBlockDownloadState and per-peer map
- blockdownloadman_impl.cpp: forwarding wrappers and implementations
  for block request tracking, availability processing, download
  scheduling, stalling detection, and tip staleness checks

The module manages all block download state: in-flight tracking
(mapBlocksInFlight), block source attribution (mapBlockSource),
per-peer download state, sync coordination, and scheduling algorithms
(FindNextBlocksToDownload, TryDownloadingHistoricalBlocks).

Key design decisions:
- cs_main provides synchronization (no separate mutex), since block
  download is inherently tied to chain state via CBlockIndex pointers
- Two-phase peer registration: ConnectedPeer handles both initial
  registration (conservative defaults) and re-registration (real
  connection properties from VERSION handshake)
- TipMayBeStale preserves lazy initialization of m_last_tip_update
- CompareExchangeBlockStallingTimeout preserves CAS atomicity for
  the stalling timeout accessed from multiple threads

No call sites are migrated yet; that happens in a subsequent commit.

d9dde648fe

test: add unit tests for BlockDownloadManager

Exercise BlockDownloadManager in isolation:

- peer_lifecycle: registration, sync state (including idempotency
  and explicit unsetting), blocks in flight, and disconnect cleanup
  for all counters (preferred, sync, downloading)
- block_source_tracking: set/get/erase block sources
- stalling_and_tip_staleness: last tip update get/set, and
  TipMayBeStale lazy initialization and staleness detection
- block_requested_basic: request/remove lifecycle, outbound
  detection, duplicate rejection, in-flight counts
- remove_block_request_from_specific_peer: peer-targeted removal
  leaves other peers' requests intact
- find_block_in_flight: verify BlockInFlightInfo fields across
  no-flight, single-peer, and multi-peer scenarios with and
  without partialBlock
- tip_not_stale_with_blocks_in_flight: blocks in flight suppress
  staleness even with an old tip update
- connected_peer_reregistration: two-phase ConnectedPeer upgrade
  from preferred=false to preferred=true without double-counting
- compare_exchange_stalling_timeout: CAS success/failure semantics

041a331114

refactor: migrate block download state and methods into BlockDownloadManager

Move all block download state and logic from PeerManagerImpl and
CNodeState into BlockDownloadManager, completing the extraction.

Migrated from PeerManagerImpl:
- mapBlocksInFlight, mapBlockSource, nSyncStarted
- m_block_stalling_timeout, m_last_tip_update
- m_num_preferred_download_peers, m_peers_downloading_from
- IsBlockRequested, IsBlockRequestedFromOutbound
- RemoveBlockRequest, BlockRequested, TipMayBeStale
- ProcessBlockAvailability, UpdateBlockAvailability
- FindNextBlocksToDownload, TryDownloadingHistoricalBlocks

Migrated from CNodeState:
- pindexBestKnownBlock, hashLastUnknownBlock
- pindexLastCommonBlock, pindexBestHeaderSent
- fSyncStarted, vBlocksInFlight, m_downloading_since
- m_stalling_since, fPreferredDownload

~150 call sites updated to go through m_blockdownloadman.

6578aa80d9

DrahtBot added the label Refactoring on Feb 11, 2026

DrahtBot commented at 6:56 pm on February 11, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	sedited, 0xbrito

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34588 (refactor: decompose Peer struct into focused sub-components by w0xlt)
#34440 (Refactor CChain methods to use references, tests by optout21)
#34389 (net/log: standardize peer+addr log formatting via LogPeer by l0rinc)
#33854 (fix assumevalid is ignored during reindex by Eunovo)
#27052 (test: rpc: add last block announcement time to getpeerinfo result by LarryRuane)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

“the actual timeout is increased temporarily if peers are disconnected for hitting the timeout” -> “The actual timeout is increased temporarily if peers are disconnected for hitting the timeout” [Sentence begins with a lowercase “the” after a period, which should be capitalized to form a grammatically correct sentence.]

No other typographic or grammatical errors that make the English invalid or incomprehensible were found.

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

m_blockdownloadman.SetBlockSource(block_transactions.blockhash, pfrom.GetId(), false) in src/net_processing.cpp
m_blockdownloadman.SetBlockSource(pblock->GetHash(), pfrom.GetId(), false) in src/net_processing.cpp
m_blockdownloadman.SetBlockSource(hash, pfrom.GetId(), true) in src/net_processing.cpp

2026-02-11 18:56:08

sedited commented at 9:48 pm on February 11, 2026: contributor

Concept ACK

0xbrito commented at 6:49 am on February 12, 2026: none