refactor: [p2p] Make ProcessMessage private again, Use references when non-null #34181

1. 
   maflcko commented at 12:31 pm on December 30, 2025: member

   There is a single unit test, which calls the internal ProcessMessage function. This is problematic, because it makes future changes harder, since they will need to carry over this public internal interface each time.

   Also, there is a mixed use of pointers and references in p2p code, where just based on context, a pointer may sometimes assumed to be null, or non-null. This is confusing when reading the code, or making or reading future changes.

   Fix both issues in a series of commits, to:

   • refactor the single unit test to call higher-level functions
   • Make ProcessMessage private again
   • Use references instead of implicit non-null pointers, mostly in a scripted-diff
2. test: [refactor] Avoid calling private ProcessMessage() function

   Calling this low-level function from tests is confusing, and also makes
   it harder to change the peer manager implementation.
   
   So juse use the pre-existing test helpers to achieve the same.

   fafe14b8bc
3. refactor: Make ProcessMessage private again

   It is not used in tests anymore.

   ddddad93e6
4. refactor: Pass CNode& to ProcessMessages and SendMessages

   The node is never nullptr.
   
   This can be reviewed with the git option:
   --word-diff-regex=.

   fa80c68c09
5. refactor: Pass Peer& to ProcessMessage

   The peer is never nullptr.

   fa262229da
6. refactor: Separate peer/maybe_peer in ProcessMessages and SendMessages

   Introducing two names to refer to the peer makes it possible to have one
   refer to a non-null reference in a later commit.

   fa845deff0
7. scripted-diff: Use references when nullptr is not possible

   This allows to skip nullptr checks later in the code, both mentally and literally.
   
   This can be reviewed via the git option:
   --word-diff-regex=.
   
   -BEGIN VERIFY SCRIPT-
   
      sed --regexp-extended --in-place '
         /^bool PeerManagerImpl::SendMessages\(/,/^}$/   {
            s/auto& peer\{maybe_peer\}; .. alias cleaned up .*/Peer\& peer{*maybe_peer};/;
            s/peer->/peer./g;
            s/\*peer\>/peer/g;
   
            /CNode\* pto\{&node\}; .. alias removed .*/d;
            s/pto->/node./g;
            s/\*pto\>/node/g;
         }
      ' src/net_processing.cpp
   
      sed --regexp-extended --in-place '
         /^void PeerManagerImpl::ProcessMessage\(/,/^}$/   {
            /Peer\* peer\{&peer_alias_removed_in_later_commit};/d;
            s/peer_alias_removed_in_later_commit/peer/;
            s/peer->/peer./g;
            s/\*peer\>/peer/g;
         }
      ' src/net_processing.cpp
   
      sed --regexp-extended --in-place '
         /^bool PeerManagerImpl::ProcessMessages\(/,/^}$/   {
            s/auto& peer\{maybe_peer\}; .. alias cleaned up .*/Peer\& peer{*maybe_peer};/;
            s/peer->/peer./g;
            s/\*peer\>/peer/g;
   
            /CNode\* pfrom\{&node\}; .. alias removed .*/d;
            s/pfrom->/node./g;
            s/\*pfrom\>/node/g;
         }
      ' src/net_processing.cpp
   
   -END VERIFY SCRIPT-

   fad3625d68
8. DrahtBot renamed this:
   refactor: [p2p] Make ProcessMessage private again, Use references when non-null
   refactor: [p2p] Make ProcessMessage private again, Use references when non-null
   on Dec 30, 2025
9. DrahtBot added the label Refactoring on Dec 30, 2025
10. 
    DrahtBot commented at 12:31 pm on December 30, 2025: 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/34181.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #34059 (refactor: Use NodeClock::time_point for m_addr_token_timestamp by maflcko)
    • #32278 (doc: better document NetEventsInterface and the deletion of “CNode"s by vasild)
    • #29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
    • #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.

11. doc: Fix typo found by LLM in net_processing.cpp fa4dbf1b29
12. DrahtBot added the label CI failed on Dec 30, 2025
13. DrahtBot removed the label CI failed on Dec 30, 2025

Contributors
maflcko DrahtBot

Labels
Refactoring