Bugfix: net_processing: Restore missing comma between peer and peeraddr in “receive version message” and “New ___ peer connected” #34293

1. 
   luke-jr commented at 8:34 pm on January 14, 2026: member

   Versions prior to 29.x all included a comma here, and the general format of the message still includes commas between all other fields
2. 
   DrahtBot commented at 8:34 pm on January 14, 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/34293.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	l0rinc, vasild, 0xB10C

   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:

   • #34444 (refactor: rename remaining BIP14 subversion identifiers to user agent by l0rinc)
   • #34389 (net/log: standardize peer+addr log formatting via LogPeer by l0rinc)
   • #34181 (refactor: [p2p] Make ProcessMessage private again, Use references when non-null by maflcko)

   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. 
   polespinasa commented at 9:43 pm on January 14, 2026: member

   Trying to test this but in my logs I don’t see a peeraddr after peer. How can I test this?
4. in src/net_processing.cpp:3675 in 9e689f5cb5 outdated

   3672                   peer->m_starting_height, addrMe.ToStringAddrPort(), fRelay, pfrom.GetId(),
   3673-                  pfrom.LogIP(fLogIPs), (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));
   3674+                  fLogIPs ? "," : "", pfrom.LogIP(fLogIPs), (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));
   3675 
   3676         if (pfrom.IsPrivateBroadcastConn()) {
   3677             if (fRelay) {
   

   ---

   ---

   

   l0rinc commented at 11:01 pm on January 14, 2026:

   Seems the original contained the leading comma: https://github.com/bitcoin/bitcoin/pull/28521/changes#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3L3570 (cc: @Sjors)

   Edit: Can you please fix https://github.com/bitcoin/bitcoin/blob/b0b65336e76153f81c782e2a940ee6f925b32048/src/net_processing.cpp#L3803, too?

   ---

   

   luke-jr commented at 3:31 am on January 23, 2026:

   Good catch, done.
5. l0rinc approved
6. 
   l0rinc commented at 11:01 pm on January 14, 2026: contributor

   code review ACK 9e689f5cb5271c668610cf9cd21ba04cc8a755b4
7. Bugfix: net_processing: Restore missing comma between peer and peeraddr in "receive version message" and "New ___ peer connected"

   Versions prior to 29.x all included a comma here, and the general format of the message still includes commas between all other fields

   ffd09f8a0d
8. luke-jr force-pushed on Jan 23, 2026
9. 
   luke-jr commented at 3:31 am on January 23, 2026: member

   Trying to test this but in my logs I don’t see a peeraddr after peer. How can I test this?

   -logips option

10. luke-jr renamed this:
    Bugfix: net_processing: Restore missing comma between peer and peeraddr in "receive version message"
    Bugfix: net_processing: Restore missing comma between peer and peeraddr in "receive version message" and "New ___ peer connected"
    on Jan 23, 2026
11. 
    l0rinc commented at 12:12 pm on January 23, 2026: contributor

    ACK ffd09f8a0d024423d55c07223a588d35c0a97c0c

    This does fix an existing issue introduced in https://github.com/bitcoin/bitcoin/pull/28521/changes#diff-6875de769e90cec84d2e8a9c1b962cdbcda44d870d42e4215827e599e11e90e3L3570. I think we should fix all remaining LogPeer instances as well in a more systematic way, not just two of the instances, so I opened an alternative in #34389, added @luke-jr as a coauthor.

12. vasild approved
13. 
    vasild commented at 11:19 am on February 5, 2026: contributor

    ACK ffd09f8a0d024423d55c07223a588d35c0a97c0c

    Seems like this code would benefit from further improvements (in a followup?) to avoid repeated "peer=%s%d%Sss%%!*&*", fLogIPs ? "," : "", pfrom.LogIP(fLogIPs)

    Thanks!

14. 
    0xB10C commented at 12:41 pm on February 5, 2026: contributor

    ACK ffd09f8a0d024423d55c07223a588d35c0a97c0c

    I had noticed this while touching the New %s peer connected log in #34008 but it didn’t bother me enough to fix it.

15. 
    l0rinc commented at 12:51 pm on February 5, 2026: contributor

    Note that this only fixes 2 occurrences, but there’s an alternative where I’ve adjusted them in a more systematic way, see: https://github.com/bitcoin/bitcoin/pull/34389
16. DrahtBot added the label Needs rebase on Feb 7, 2026
17. 
    DrahtBot commented at 1:33 am on February 7, 2026: contributor

    🐙 This pull request conflicts with the target branch and needs rebase.
18. 
    fanquake commented at 11:02 am on February 20, 2026: member

    @luke-jr are you going to rebase this?
19. 
    l0rinc commented at 11:07 am on February 20, 2026: contributor

    #34389 contains these changes already and applies it consistently to all other cases and is rebased. I have added @luke-jr as a coauthor to it.

Contributors
luke-jr DrahtBot polespinasa l0rinc vasild 0xB10C fanquake

Labels
Needs rebase