net/log: standardize peer+addr log formatting via LogPeer #34389

1. 
   l0rinc commented at 12:09 pm on January 23, 2026: contributor

   This is an alternative to #34293, but aims to address the remaining logging inconsistencies more broadly. It extends the example fixed there to every instance, restores the original separator behavior, applies it consistently via a single helper, and adds tests for logips (covering both current and new behavior).

   Problem

   After #28521 centralized peer address logging into CNode::LogIP(), the original comma separator before peeraddr= was lost, resulting in inconsistent formatting across net (and recent private broadcast) logs. Some lines also had double spaces, empty fields, or mismatched format specifiers.

   Fix

   Introduces CNode::LogPeer(bool) which always emits peer=<id> and, when -logips=1, appends , peeraddr=<addr>. This eliminates hand-rolled separators and makes peer identification predictable. Minor issues (double spaces, empty placeholders, format specifiers) are fixed along the way in separate commits.

   Reproducer

   Run with -debug=net -logips=1 and observe peer log lines now show peer=<id>, peeraddr=<addr> (comma-separated). The new assertion in feature_logging.py automates this check.

2. 
   DrahtBot commented at 12:09 pm on January 23, 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/34389.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	vasild
   Concept ACK	naiyoma

   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)
   • #34565 (refactor: extract BlockDownloadManager from PeerManagerImpl by w0xlt)
   • #34271 (net_processing: make m_tx_for_private_broadcast optional by vasild)
   • #34254 (validation: Prevent duplicate logging and looping in invalid block handling by mzumsande)
   • #34059 (refactor: Use NodeClock::time_point for m_addr_token_timestamp by maflcko)
   • #30343 (wallet, logging: Replace WalletLogPrintf() with LogInfo() by ryanofsky)
   • #30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)

   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.

3. in src/net_processing.cpp:3671 in d515f7c8d5 outdated

   3666@@ -3668,17 +3667,17 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
   3667         }
   3668 
   3669         const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
   3670-        LogDebug(BCLog::NET, "receive version message: %s: version %d, blocks=%d, us=%s, txrelay=%d, peer=%d%s%s\n",
   3671-                  cleanSubVer, pfrom.nVersion,
   3672-                  peer->m_starting_height, addrMe.ToStringAddrPort(), fRelay, pfrom.GetId(),
   3673-                  pfrom.LogIP(fLogIPs), (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));
   3674+        LogDebug(BCLog::NET, "receive version message: %s: version %d, blocks=%d, us=%s, txrelay=%d, %s%s\n",
   3675+                  cleanSubVer.empty() ? "<no subver>" : cleanSubVer, pfrom.nVersion,
   

   ---

   ---

   

   luke-jr commented at 1:14 pm on January 23, 2026:

   Probably should stop calling this a “subver” at least in new user-facing docs.

   ---

   

   l0rinc commented at 1:35 pm on January 23, 2026:

   Can you suggest what to call it here instead?

   ---

   

   luke-jr commented at 4:35 am on January 29, 2026:

   useragent?

   ---

   

   sipa commented at 4:37 am on January 29, 2026:

   For context, see BIP-14.

   ---

   

   l0rinc commented at 11:03 am on January 29, 2026:

   Thank you for the context, adjusted it, and will push a separate PR to rename remaining subver references to user agent.

   ---

   

   l0rinc commented at 1:25 pm on January 29, 2026:

   Adjusted the remaining instances in #34444, thanks for the hint Edit: was closed since
4. DrahtBot added the label Needs rebase on Jan 27, 2026
5. l0rinc force-pushed on Jan 27, 2026
6. DrahtBot removed the label Needs rebase on Jan 27, 2026
7. l0rinc force-pushed on Jan 29, 2026
8. 
   naiyoma commented at 3:25 pm on January 30, 2026: contributor

   Concept ACK

   Renaming LogIp to LogPeer, expanding it to also handle peer Id, and restoring the comma separator between peer= and peeraddr=. This refactors improves readability and consistency in the logs.

   briefly tested af073419f75ad79b6628333ba3c43578ea2f250a

   2026-01-30T12:25:06Z [net] receive version message: /Satoshi:30.0.0/: version 70016, blocks=934343, us=[::]:0, txrelay=1, peer=7, peeraddr=rf66bdjae6n2gziqe4346hrlfe2xxkzysjq6l72p5imbyig7hxwzdkid.onion:8333

   reviewed

   • b12c137db7b2895a0cba6d5b4a67208bb5581ce3 log: show placeholders for missing peer fields
   • 6281ebd5da67546232777c1d8c30064799d1d578 log: fix minor formatting in debug logs
   • cd49c08fb2c5212a4d2b6e931942bfadb54e2855 test: add pre-LogPeer net log assertion

   changes LGTM

   noticed these double spaces and i think they can be removed in 6281ebd5da67546232777c1d8c30064799d1d578, but I’m not sure if they’re intentional.

   https://github.com/bitcoin/bitcoin/blob/01651324f4e540f6b96cff31e89752c3f9417293/src/validation.cpp#L1984-L1987

   and in https://github.com/bitcoin/bitcoin/blob/01651324f4e540f6b96cff31e89752c3f9417293/src/net_processing.cpp#L4285

   https://github.com/bitcoin/bitcoin/blob/01651324f4e540f6b96cff31e89752c3f9417293/src/net_processing.cpp#L4299

9. in src/addrman.cpp:667 in af073419f7 outdated

   663@@ -664,7 +664,7 @@ bool AddrManImpl::Good_(const CService& addr, bool test_before_evict, NodeSecond
   664         // Output the entry we'd be colliding with, for debugging purposes
   665         auto colliding_entry = mapInfo.find(vvTried[tried_bucket][tried_bucket_pos]);
   666         LogDebug(BCLog::ADDRMAN, "Collision with %s while attempting to move %s to tried table. Collisions=%d\n",
   667-                 colliding_entry != mapInfo.end() ? colliding_entry->second.ToStringAddrPort() : "",
   668+                 colliding_entry != mapInfo.end() ? colliding_entry->second.ToStringAddrPort() : "<unknown>",
   

   ---

   ---

   

   naiyoma commented at 3:31 pm on January 30, 2026:

   nit: feel free to ignore IMO <unknown-entry> or <unknown-addr> is clearer

   ---

   

   l0rinc commented at 12:08 pm on January 31, 2026:

   Thanks, will apply the nits if we need another push

   ---

   

   vasild commented at 11:23 am on February 13, 2026:

   That would designate some addrman corruption, I guess. Just a thought, out of the scope of this PR since it is about logging - maybe add an assert or Assume that colliding_entry != mapInfo.end().

   The current change is fine, because, “unknown” it better than an empty string.

   ---

   

   l0rinc commented at 9:57 pm on February 15, 2026:

   Added “<unknown-addr>”
10. DrahtBot added the label Needs rebase on Feb 7, 2026
11. l0rinc force-pushed on Feb 7, 2026
12. DrahtBot removed the label Needs rebase on Feb 7, 2026
13. in src/net_processing.cpp:3520 in 74bb2b3567

    3518         via_compact_block ? "cmpctblock " : "",
    3519         index.GetBlockHash().ToString(),
    3520         index.nHeight,
    3521-        peer.GetId(),
    3522-        peer.LogIP(fLogIPs)
    3523+        peer.LogPeer(fLogIPs)
    

    ---

    ---

    

    vasild commented at 11:47 am on February 13, 2026:

    Since all callers of LogPeer() pass the same global variable fLogIPs, I wouldn’t mind if callers are spared of having to do that and the LogPeer() method looks up that global variable internally.

    ---

    

    l0rinc commented at 9:57 pm on February 15, 2026:

    I wouldn’t mind if callers are spared of having to do that

    Excellent, removed all of them, the call sites are so much cleaner. Added you as coauthor.

14. vasild approved
15. 
    vasild commented at 11:52 am on February 13, 2026: contributor

    ACK 74bb2b35670ba1c2baf793b740861a6d16b73c16
16. DrahtBot requested review from naiyoma on Feb 13, 2026
17. log: show placeholders for missing peer fields

    Avoid logging an empty field when peer user agent or collision entry is missing.
    
    Co-authored-by: naiyoma <lankas.aurelia@gmail.com>

    9cf82bed32
18. log: fix minor formatting in debug logs

    Tidy a few debug log strings to avoid double spaces, concatenated status words, and mismatched format specifiers.
    
    Co-authored-by: naiyoma <lankas.aurelia@gmail.com>

    736b17c0f0
19. test: add pre-`LogPeer` net log assertion

    Assert net log output contains `peer=... peeraddr=...` before the comma change.

    e55ea534f7
20. net: format peer+addr logs with `LogPeer`

    Use `LogPeer` for peer id plus optional `peeraddr`, separated with a comma.
    
    Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
    Co-authored-by: Vasil Dimov <vd@freebsd.org>

    22335474d7
21. l0rinc force-pushed on Feb 15, 2026
22. 
    l0rinc commented at 9:57 pm on February 15, 2026: contributor

    Thanks for the reviews!

    noticed these double spaces and i think they can be removed

    Indeed, applied them and added you as coathor.

    Rebased, and since I was modifying all those lines, I have also removed all trailing newlines to leave the campground a bit cleaner that we found it.

23. vasild approved
24. 
    vasild commented at 11:26 am on February 16, 2026: contributor

    ACK 22335474d768f99067856173ff2764b6db753f67
25. l0rinc requested review from luke-jr on Feb 16, 2026
26. achow101 added this to the milestone 31.0 on Feb 20, 2026
27. 
    fanquake commented at 11:06 am on February 20, 2026: member

    This was added to 31.x, but I’m not sure why. It doesn’t seem to be a bugfix, or a new feature, or any more pressing than any other PRs? So I’ve dropped it back off for now.
28. fanquake removed this from the milestone 31.0 on Feb 20, 2026
29. 
    l0rinc commented at 11:09 am on February 20, 2026: contributor

    It’s the same bugfix that #34293 adjusts (missing comma in logs that was previously there), applied systematically.
30. 
    fanquake commented at 11:18 am on February 20, 2026: member

    I’m not sure that a missing comma in a log line, is a bug. If it was the case, then I think it’d be better to merge (and possibly backport) the single commit that fixes the issue, rather than larger, general refactor, right before feature freeze.

Contributors
l0rinc DrahtBot luke-jr sipa naiyoma vasild fanquake

Review Requested
luke-jr naiyoma