#35522 refactor: Extract per-message helpers from SendMessages() (move-only)
https://github.com/bitcoin/bitcoin/pull/35522 · · +661/-424 in 1 files, 17 commits · labels: Refactoring, Needs rebase
Goal
- Break up monolithic P2P message dispatching logic into discrete helpers
- Make navigation, locking verification, and review easier for P2P developers
Extracts 17 move-only helper functions out of the ~500-line `PeerManagerImpl::SendMessages()` method in `src/net_processing.cpp`. Each extracted helper handles a discrete conditional message send path or timeout check, reducing `SendMessages()` to under 90 lines.
Problem: `SendMessages()` is a monolithic function handling all conditional P2P message dispatching and timeout logic inline, making navigation, locking verification, and targeted review difficult for developers working on P2P message processing.
Category: P2P (#51 of 65)
P3 · cleanup
- P3 because it is a move-only refactoring that fixes no bugs and changes no protocol behavior
- Breaking up the monolithic routine improves maintainability and eases future reviews
Breaking up a 500-line monolithic function in net_processing provides clear maintainability value and simplifies future reviews, as noted by hodlinator. However, pure move-only refactorings that fix no bugs and change no protocol behavior rank as worthwhile cleanups (P3) rather than strategic priorities.
Membership: Touches src/net_processing.cpp to refactor SendMessages and related P2P message transmission routines.
Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 1
Reviewability: Stale: Needs rebase
- Needs rebase due to merge conflicts with master
The PR has merge conflicts with master and is flagged with the Needs rebase label.
Author status: active, last rebased in August 2026; currently needs another rebase.
Resolved concerns:
- w0xlt noted that MaybeSendTxMessages acquires the tx_relay mutex internally and does not require it held beforehand; the author corrected the comment.
Agreement: Strong
- Strong support for the refactoring with no objections
- Approach approval for consistency with existing patterns (w0xlt)
- Concept approval because named helpers simplify reviewing and discussing logic (hodlinator)
Concept and approach ACKs with stated rationales; no objections raised.
Both reviewers who participated supported the change: w0xlt gave an Approach ACK based on consistency with #35502, and hodlinator gave a Concept ACK noting that named helpers simplify reviewing and discussing the logic.
- w0xlt: 'Approach ACK, same pattern as #35502.'
- hodlinator: 'Concept ACK. Was recently reviewing some code here and felt it would have been nice to be able to refer to smaller parts of SendMessages() by name.'
Review verdicts (DrahtBot): 0
- Approach ACK: w0xlt
- Concept ACK: hodlinator
Files
0 lines under test/bench/ci.
- src/net_processing.cpp +661/-424
Card
This PR extracts 17 move-only helper functions out of the ~500-line SendMessages function in src/net_processing.cpp, shrinking the main dispatch function to under 90 lines. It addresses developer maintainability and review difficulty in the P2P message sending loop. Reviewers have supported the approach and concept, noting it makes parts of SendMessages easier to name and review. The PR is currently marked Needs rebase due to conflicts with master.