refactor: Extract per-message helpers from SendMessages() (move-only) #35522

pull pablomartin4btc wants to merge 17 commits into bitcoin:master from pablomartin4btc:refactor/extract-sendmessages-helpers changing 1 files +664 −427

pablomartin4btc commented at 11:48 PM on June 12, 2026: member

PeerManagerImpl::SendMessages() is a ~500-line function handling every p2p conditional send message type inline, which makes it hard to navigate and review. This was reduced to less than 90 lines.

I've identified this issue/ possible improvement while reviewing #34824.

This follows the pattern of similar PRs (#35502, and some commits fa5ab02 - from #35148 -, and fa55723 from #34059 ) and there was also a previous attempt in #9579 mentioned in some related PR's refactoring comment.

-<ins>Notes</ins>:

Each of the 17 commits represents a helper function extraction so it can be easily reviewed, it's only a pure code move, the only new lines are the declaration (with thread-safety annotations), the function signature, and the one-line call site, and they can be reviewed with the git options: --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space.

move-only: Extract ScheduleTxRelayTrickle() helper

Extracts the periodic relay scheduling logic from the tx-relay section
of SendMessages() into a self-contained helper. The helper determines
whether it is time to trickle inventory to a peer and clears the
pending inventory set if the peer has requested no transaction relay.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

a87a8e6b1f

move-only: Extract MaybeSendMempoolResponse() helper

Extracts the BIP35 mempool-response block from the tx-relay section of
SendMessages() into a self-contained helper. The m_send_mempool guard
moves inside as an early return, keeping the call site a single line.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

681f19a00a

move-only: Extract MaybeSendTxInventory() helper

Extracts the regular tx-inventory relay selection block from the
tx-relay section of SendMessages() into a self-contained helper.
The helper covers candidate collection, heap-based fee/topology
sorting, bloom/feerate filtering, and the m_last_inv_sequence update.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

e6d1d2bf18

move-only: Extract MaybeSendInitialGetheaders() helper

Extracts the initial headers-sync kick-off block from SendMessages()
into a self-contained helper. The helper handles the fSyncStarted
guard, the single-peer-or-close-to-tip condition, the getheaders
locator construction, and the timeout initialisation.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

d93acbabb6

move-only: Extract MaybeSendBlockAnnouncements() helper

Extracts the block-announcement section from SendMessages() into a
self-contained helper. The helper covers header-relay candidate
selection, compact-block sending (with cached-message optimisation),
multi-header relay, and the inv fallback path.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

8642e35a6c

move-only: Extract MaybeSendBlockInv() helper

Extracts the block-inv drain loop from SendMessages() into a
self-contained helper. The helper flushes m_blocks_for_inv_relay
into the caller-provided vInv vector so downstream tx-relay
logic can continue appending to the same batch.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

677e97e63a

move-only: Extract CheckBlockSyncTimeouts() helper

Extracts the three timeout/stall checks from SendMessages() into a
self-contained helper: block-download stall detection (with adaptive
timeout backoff), per-block in-flight timeout, and headers-sync
timeout. Returns true when a disconnect was triggered so the caller
can early-return, preserving identical semantics.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

c87093449d

move-only: Extract MaybeSendGetData() helper

Extracts the getdata section from SendMessages() into a self-contained
helper. The helper covers block requests (FindNextBlocksToDownload and
historical-block ranges), transaction download requests, and the final
getdata flush. vGetData is local to the helper; the m_tx_download_mutex
acquisition moves inside.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

6eb3f3e051

move-only: Extract SendCompactBlockOrHeaders() helper

Extracts the compact-block/headers send path from
MaybeSendBlockAnnouncements() into a self-contained helper. The helper
chooses between a compact-block (with cached-message optimisation),
a headers message, or sets fRevertToInv when neither format applies.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

6d022c3135

move-only: Extract SendBlockInvFallback() helper

Extracts the inv-fallback path from MaybeSendBlockAnnouncements() into
a self-contained helper. The helper inv-announces the chain tip when
neither a headers nor a compact-block relay was possible.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

c656841fcc

move-only: Extract CheckBlockDownloadStall() helper

Extracts the stall-detection block from CheckBlockSyncTimeouts() into a
self-contained helper. The helper detects when the block-download window
has stalled and disconnects the peer, applying an adaptive timeout backoff
to avoid disconnecting multiple peers when local bandwidth is insufficient.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

2790fecd56

move-only: Extract CheckBlockFlightTimeout() helper

Extracts the per-block in-flight timeout check from CheckBlockSyncTimeouts()
into a self-contained helper. The helper disconnects a peer when a requested
block has been in flight longer than the threshold, compensating for the
number of peers with validated downloads.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

608b92fdac

move-only: Extract CheckHeadersSyncTimeout() helper

Extracts the initial headers-sync timeout check from CheckBlockSyncTimeouts()
into a self-contained helper. The helper disconnects (or resets sync state for
NoBan peers) when initial headers sync has stalled, and resets the timeout
once the chain has caught up.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

0164ba0c2b

move-only: Extract QueueBlocksGetData() helper

Extracts the block getdata request queuing logic from MaybeSendGetData() into
a self-contained helper. The helper queues blocks via FindNextBlocksToDownload
and TryDownloadingHistoricalBlocks, and marks a stalling peer when the
in-flight queue runs empty.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

0518a402df

move-only: Extract QueueTxGetData() helper

Extracts the transaction getdata request queuing logic from MaybeSendGetData()
into a self-contained helper. The helper acquires m_tx_download_mutex, iterates
pending tx requests from m_txdownloadman, and flushes intermediate batches when
the getdata size limit is reached.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

062920c034

move-only: Extract ComputeSyncBlocksAndHeaders() helper

Extracts the sync-peer eligibility computation from the SendMessages() cs_main
block into a self-contained helper. The helper initialises m_best_header if
needed and returns whether blocks and headers should be synced from this peer
based on preferred-download status and current in-flight state.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

1baa4160a2

move-only: Extract MaybeSendTxMessages() helper

Extracts the tx-relay trickle and inventory dispatch block from the
SendMessages() cs_main block into a self-contained helper. The helper acquires
the per-peer tx_inventory_mutex, schedules the trickle interval, and
conditionally sends the mempool response and tx inventory.

This commit can be reviewed with the git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space

f69c587028

DrahtBot added the label Refactoring on Jun 12, 2026
DrahtBot commented at 11:48 PM on June 12, 2026: contributor



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



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35522.



Reviews

See the guideline for information on the review process. A summary of reviews will appear here.
pablomartin4btc commented at 11:52 PM on June 12, 2026: member

@w0xlt, my first impulse is to put it into draft till your both PRs #34824 and #35502 land and get merged before but perhaps you find this useful and easier to work on those changes on top of this one?

Contributors

pablomartin4btc

DrahtBot

Labels

Refactoring

Linked (view graph)

#9579 Net: Trivial-review: Make SendMessages easier to review #9608 Net: Divide ProcessMessage in smaller functions #34059 refactor: Use NodeClock::time_point for m_addr_token_timestamp #34824 net: refactor: replace Peer::TxRelay RecursiveMutex instances with Mutex #35148 refactor: Remove confusing DataStream::in_avail() alias #35502 refactor: extract per-message helpers from ProcessMessage (move-only)

refactor: Extract per-message helpers from SendMessages() (move-only) #35522

Code Coverage & Benchmarks

Reviews