net/log: standardize peer+addr log formatting via `LogPeer` by l0rinc · Pull Request #34389 · bitcoin/bitcoin

l0rinc · 2026-01-23T12:09:26Z

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.

DrahtBot · 2026-01-23T12:09:34Z

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, naiyoma, sedited

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34743 (p2p: protect manual evictions by willcl-ark)
#34707 (net: keep finished private broadcast txs in memory, rebroadcast if evicted from mempool by andrewtoth)
#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)
#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.

src/net_processing.cpp

naiyoma

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 af07341

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

b12c137 log: show placeholders for missing peer fields
6281ebd log: fix minor formatting in debug logs
cd49c08 test: add pre-LogPeer net log assertion

changes LGTM

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

bitcoin/src/validation.cpp

Lines 1984 to 1987 in 0165132

    
           LogInfo("%s:  current best=%s  height=%d  log2_work=%f  date=%s\n", __func__, 
        
             tip->GetBlockHash().ToString(), m_chain.Height(), log(tip->nChainWork.getdouble())/log(2.0), 
        
             FormatISO8601DateTime(tip->GetBlockTime())); 
        
           CheckForkWarningConditions();

and in

bitcoin/src/net_processing.cpp

Line 4285 in 0165132

    
           LogDebug(BCLog::NET, "  getblocks stopping at %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString());

bitcoin/src/net_processing.cpp

Line 4299 in 0165132

    
           LogDebug(BCLog::NET, "  getblocks stopping at limit %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString());

naiyoma · 2026-01-30T15:31:54Z

src/addrman.cpp

        auto colliding_entry = mapInfo.find(vvTried[tried_bucket][tried_bucket_pos]);
        LogDebug(BCLog::ADDRMAN, "Collision with %s while attempting to move %s to tried table. Collisions=%d\n",
-                 colliding_entry != mapInfo.end() ? colliding_entry->second.ToStringAddrPort() : "",
+                 colliding_entry != mapInfo.end() ? colliding_entry->second.ToStringAddrPort() : "<unknown>",


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

Thanks, will apply the nits if we need another push

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.

Added "<unknown-addr>"

vasild

ACK 74bb2b3

vasild · 2026-02-13T11:47:58Z

src/net_processing.cpp

        index.nHeight,
-        peer.GetId(),
-        peer.LogIP(fLogIPs)
+        peer.LogPeer(fLogIPs)


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.

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.

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

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>

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

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>

l0rinc · 2026-02-15T21:57:53Z

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.

vasild

ACK 2233547

fanquake · 2026-02-20T11:06:24Z

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.

l0rinc · 2026-02-20T11:09:06Z

It's the same bugfix that #34293 adjusts (missing comma in logs that was previously there), applied systematically.

fanquake · 2026-02-20T11:18:46Z

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.

Sjors · 2026-02-24T13:39:20Z

#28521 was shipped with v29 and v30 so it's not a newly introduced regression that we want to keep out of v31. It could make sense to, similar to what @fanquake suggests, merge the full change after branch-off and back-port a minimal fix. Though I'm not sure if that's worth doing, unless it's really hurting someone's log parsing.

naiyoma · 2026-02-26T16:24:03Z

ACK 2233547

sedited

ACK 2233547

l0rinc mentioned this pull request Jan 23, 2026

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

Closed

luke-jr reviewed Jan 23, 2026

View reviewed changes

src/net_processing.cpp Outdated Show resolved Hide resolved

DrahtBot added the Needs rebase label Jan 27, 2026

l0rinc force-pushed the l0rinc/log-peer-formatting branch from d515f7c to fd8aaac Compare January 27, 2026 13:43

DrahtBot removed the Needs rebase label Jan 27, 2026

l0rinc force-pushed the l0rinc/log-peer-formatting branch from fd8aaac to af07341 Compare January 29, 2026 11:01

l0rinc mentioned this pull request Jan 29, 2026

refactor: rename remaining BIP14 subversion identifiers to user agent #34444

Closed

naiyoma reviewed Jan 30, 2026

View reviewed changes

DrahtBot added the Needs rebase label Feb 7, 2026

l0rinc force-pushed the l0rinc/log-peer-formatting branch from af07341 to 74bb2b3 Compare February 7, 2026 18:50

DrahtBot removed the Needs rebase label Feb 7, 2026

DrahtBot mentioned this pull request Feb 12, 2026

refactor: extract BlockDownloadManager from PeerManagerImpl #34565

Open

vasild approved these changes Feb 13, 2026

View reviewed changes

DrahtBot requested a review from naiyoma February 13, 2026 11:52

l0rinc and others added 4 commits February 15, 2026 22:54

log: show placeholders for missing peer fields

9cf82be

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

log: fix minor formatting in debug logs

736b17c

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>

test: add pre-LogPeer net log assertion

e55ea53

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

net: format peer+addr logs with LogPeer

2233547

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>

l0rinc force-pushed the l0rinc/log-peer-formatting branch from 74bb2b3 to 2233547 Compare February 15, 2026 21:57

vasild approved these changes Feb 16, 2026

View reviewed changes

l0rinc requested a review from luke-jr February 16, 2026 12:46

DrahtBot mentioned this pull request Feb 16, 2026

refactor: decompose Peer struct into focused sub-components #34588

Open

This was referenced Feb 17, 2026

validation: Prevent duplicate logging and looping in invalid block handling #34254

Open

refactor: Use NodeClock::time_point for m_addr_token_timestamp #34059

Open

achow101 added this to the 31.0 milestone Feb 20, 2026

fanquake removed this from the 31.0 milestone Feb 20, 2026

This was referenced Mar 2, 2026

net: keep finished private broadcast txs in memory, rebroadcast if evicted from mempool #34707

Open

p2p: protect manual evictions #34743

Open

sedited approved these changes Mar 8, 2026

View reviewed changes

DrahtBot mentioned this pull request Mar 11, 2026

rpc, net: remove startingheight field of getpeerinfo RPC and from node state #34796

Merged

sedited merged commit e96d9e6 into bitcoin:master Mar 11, 2026
26 checks passed

	LogInfo("%s: current best=%s height=%d log2_work=%f date=%s\n", __func__,
	tip->GetBlockHash().ToString(), m_chain.Height(), log(tip->nChainWork.getdouble())/log(2.0),
	FormatISO8601DateTime(tip->GetBlockTime()));
	CheckForkWarningConditions();

Conversation

l0rinc commented Jan 23, 2026

Problem

Fix

Reproducer

Uh oh!

DrahtBot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

Uh oh!

naiyoma left a comment

Choose a reason for hiding this comment

Uh oh!

naiyoma Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

l0rinc Jan 31, 2026

Choose a reason for hiding this comment

Uh oh!

vasild Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

l0rinc Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

vasild Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

l0rinc Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

l0rinc commented Feb 15, 2026

Uh oh!

vasild left a comment

Choose a reason for hiding this comment

Uh oh!

fanquake commented Feb 20, 2026

Uh oh!

l0rinc commented Feb 20, 2026

Uh oh!

fanquake commented Feb 20, 2026

Uh oh!

Sjors commented Feb 24, 2026

Uh oh!

naiyoma commented Feb 26, 2026

Uh oh!

sedited left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

DrahtBot commented Jan 23, 2026 •

edited

Loading

l0rinc Feb 15, 2026 •

edited

Loading

vasild Feb 13, 2026 •

edited

Loading