net: Don't log own ips during discover

sedited commented at 9:59 am on January 30, 2026: contributor

The -logips option seems to imply that ip addresses are only logged when it is set. This seems like an obvious case for not logging these by default. There might be prior discussion around this, but I was unable to find why these might be exempt. There are also a few issues (both open and closed) where printing these lines was useful.

The -logips option seems to imply that ip addresses are only logged when
it is set. This seems like an obvious case for not logging these by
default.

ca21fc8c28

DrahtBot added the label P2P on Jan 30, 2026

DrahtBot commented at 10:00 am on January 30, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	l0rinc, willcl-ark

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

in src/net.cpp:291 in ca21fc8c28

286@@ -287,7 +287,9 @@ bool AddLocal(const CService& addr_, int nScore)
287     if (!g_reachable_nets.Contains(addr))
288         return false;
289 
290-    LogInfo("AddLocal(%s,%i)\n", addr.ToStringAddrPort(), nScore);
291+    if (fLogIPs) {
292+        LogInfo("AddLocal(%s,%i)\n", addr.ToStringAddrPort(), nScore);

l0rinc commented at 10:17 am on January 30, 2026:

nit: while we’re here, let’s remove the trailing newlines nit2: we might as well format this similarly to the actual method call nit3: we can safely use %s for both nit4: we could still show nScore when fLogIPs is disabled

0        LogInfo("AddLocal(%s, %s)", fLogIPs ? addr.ToStringAddrPort() : "[redacted]", nScore);

We should likely do the same with RemoveLocal:

0void RemoveLocal(const CService& addr)
1{
2    LOCK(g_maplocalhost_mutex);
3    LogInfo("RemoveLocal(%s)", fLogIPs ? addr.ToStringAddrPort() : "[redacted]");
4    mapLocalHost.erase(addr);
5}

sedited commented at 12:40 pm on January 30, 2026:

I am not even sure why we log these in the first place. Could alternatively just relegate them to debug? Just redacting the IPs seems to make the log line useless.

in src/net.cpp:3351 in ca21fc8c28

3346@@ -3345,8 +3347,9 @@ void Discover()
3347         return;
3348 
3349     for (const CNetAddr &addr: GetLocalAddresses()) {
3350-        if (AddLocal(addr, LOCAL_IF))
3351+        if (AddLocal(addr, LOCAL_IF) && fLogIPs) {
3352             LogInfo("%s: %s\n", __func__, addr.ToStringAddr());

l0rinc commented at 10:53 am on January 30, 2026:

this will also avoid logging the whole line now - which is probably fine

willcl-ark commented at 10:58 am on February 3, 2026:

I think here we might log twice if fLogIPs is true? Once in AddLocal and once in the following line 3351? (not tested)

l0rinc changes_requested

l0rinc commented at 11:17 am on January 30, 2026: contributor

Concept ACK on taking PII logging more seriously. We can’t fully fix it, but I agree we should be able to reduce it.

The PR title should probably generalize to sanitizing LogInfo/LogWarning address logging - or show “Discover” at the end to note that it’s a method name.

Can we cover any of these cases as well (I have skipped all debug logs, we can do those separately)?

These were my local changes corresponding to the above:

  0diff --git a/src/httprpc.cpp b/src/httprpc.cpp
  1index 56a243085c..6e73469ada 100644
  2--- a/src/httprpc.cpp
  3+++ b/src/httprpc.cpp
  4@@ -120,7 +120,7 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
  5     jreq.context = context;
  6     jreq.peerAddr = req->GetPeer().ToStringAddrPort();
  7     if (!RPCAuthorized(authHeader.second, jreq.authUser)) {
  8-        LogWarning("ThreadRPCServer incorrect password attempt from %s", jreq.peerAddr);
  9+        LogWarning("ThreadRPCServer incorrect password attempt%s", fLogIPs ? strprintf(" from %s", jreq.peerAddr) : "");
 10 
 11         /* Deter brute-forcing
 12            If this results in a DoS the user really
 13diff --git a/src/i2p.cpp b/src/i2p.cpp
 14index 04093ea100..b99f6e6349 100644
 15--- a/src/i2p.cpp
 16+++ b/src/i2p.cpp
 17@@ -452,10 +452,10 @@ void Session::CreateIfNotCreatedAlready()
 18     m_session_id = session_id;
 19     m_control_sock = std::move(sock);
 20 
 21-    LogInfo("%s I2P SAM session %s created, my address=%s",
 22+    LogInfo("%s I2P SAM session %s created%s",
 23         Capitalize(session_type),
 24         m_session_id,
 25-        m_my_addr.ToStringAddrPort());
 26+        fLogIPs ? strprintf(", my address=%s", m_my_addr.ToStringAddrPort()) : "");
 27 }
 28 
 29 std::unique_ptr<Sock> Session::StreamAccept()
 30diff --git a/src/net.cpp b/src/net.cpp
 31index 16591461ef..635d4dff90 100644
 32--- a/src/net.cpp
 33+++ b/src/net.cpp
 34@@ -287,7 +287,7 @@ bool AddLocal(const CService& addr_, int nScore)
 35     if (!g_reachable_nets.Contains(addr))
 36         return false;
 37 
 38-    LogInfo("AddLocal(%s,%i)\n", addr.ToStringAddrPort(), nScore);
 39+    LogInfo("AddLocal(%s, %s)", fLogIPs ? addr.ToStringAddrPort() : "[redacted]", nScore);
 40 
 41     {
 42         LOCK(g_maplocalhost_mutex);
 43@@ -310,7 +310,7 @@ bool AddLocal(const CNetAddr &addr, int nScore)
 44 void RemoveLocal(const CService& addr)
 45 {
 46     LOCK(g_maplocalhost_mutex);
 47-    LogInfo("RemoveLocal(%s)\n", addr.ToStringAddrPort());
 48+    LogInfo("RemoveLocal(%s)", fLogIPs ? addr.ToStringAddrPort() : "[redacted]");
 49     mapLocalHost.erase(addr);
 50 }
 51 
 52@@ -385,7 +385,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect,
 53 
 54         // Look for an existing connection
 55         if (AlreadyConnectedToAddressPort(addrConnect)) {
 56-            LogInfo("Failed to open new connection to %s, already connected", addrConnect.ToStringAddrPort());
 57+            LogInfo("Failed to open new connection to %s, already connected", fLogIPs ? addrConnect.ToStringAddrPort() : "[redacted]");
 58             return nullptr;
 59         }
 60     }
 61@@ -416,7 +416,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect,
 62                 // It is possible that we already have a connection to the IP/port pszDest resolved to.
 63                 // In that case, drop the connection that was just created.
 64                 if (AlreadyConnectedToAddressPort(addrConnect)) {
 65-                    LogInfo("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort());
 66+                    LogInfo("Not opening a connection to %s, already connected to %s", pszDest, fLogIPs ? addrConnect.ToStringAddrPort() : "[redacted]");
 67                     return nullptr;
 68                 }
 69                 // Add the address to the resolved addresses vector so we can try to connect to it later on
 70@@ -1783,7 +1783,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
 71     }
 72 
 73     if (!sock->IsSelectable()) {
 74-        LogInfo("connection from %s dropped: non-selectable socket\n", addr.ToStringAddrPort());
 75+        LogInfo("connection from %s dropped: non-selectable socket", fLogIPs ? addr.ToStringAddrPort() : "[redacted]");
 76         return;
 77     }
 78 
 79@@ -3325,7 +3325,7 @@ bool CConnman::BindListenPort(const CService& addrBind, bilingual_str& strError,
 80         LogError("%s\n", strError.original);
 81         return false;
 82     }
 83-    LogInfo("Bound to %s\n", addrBind.ToStringAddrPort());
 84+    LogInfo("Bound to %s", fLogIPs ? addrBind.ToStringAddrPort() : "[redacted]");
 85 
 86     // Listen for incoming connections
 87     if (sock->Listen(SOMAXCONN) == SOCKET_ERROR)
 88@@ -3345,8 +3345,8 @@ void Discover()
 89         return;
 90 
 91     for (const CNetAddr &addr: GetLocalAddresses()) {
 92-        if (AddLocal(addr, LOCAL_IF))
 93-            LogInfo("%s: %s\n", __func__, addr.ToStringAddr());
 94+        if (AddLocal(addr, LOCAL_IF) && fLogIPs)
 95+            LogInfo("%s: %s", __func__, addr.ToStringAddr());
 96     }
 97 }
 98 
 99diff --git a/src/net_processing.cpp b/src/net_processing.cpp
100index 16b4735e5e..bd8ba085e6 100644
101--- a/src/net_processing.cpp
102+++ b/src/net_processing.cpp
103@@ -3620,7 +3620,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
104         // Disconnect if we connected to ourself
105         if (pfrom.IsInboundConn() && !m_connman.CheckIncomingNonce(nNonce))
106         {
107-            LogInfo("connected to self at %s, disconnecting\n", pfrom.addr.ToStringAddrPort());
108+            LogInfo("connected to self%s, disconnecting\n", fLogIPs ? strprintf(" at %s", pfrom.addr.ToStringAddrPort()) : "");
109             pfrom.fDisconnect = true;
110             return;
111         }
112diff --git a/src/netbase.cpp b/src/netbase.cpp
113index c1c03c5725..365e0a2cf9 100644
114--- a/src/netbase.cpp
115+++ b/src/netbase.cpp
116@@ -644,9 +644,10 @@ static bool ConnectToSocket(const Sock& sock, struct sockaddr* sockaddr, socklen
117 
118 std::unique_ptr<Sock> ConnectDirectly(const CService& dest, bool manual_connection)
119 {
120+    const std::string dest_str{fLogIPs ? dest.ToStringAddrPort() : "[redacted]"};
121     auto sock = CreateSock(dest.GetSAFamily(), SOCK_STREAM, IPPROTO_TCP);
122     if (!sock) {
123-        LogError("Cannot create a socket for connecting to %s\n", dest.ToStringAddrPort());
124+        LogError("Cannot create a socket for connecting%s", fLogIPs ? strprintf(" to %s", dest.ToStringAddrPort()) : "");
125         return {};
126     }
127 
128@@ -654,11 +655,11 @@ std::unique_ptr<Sock> ConnectDirectly(const CService& dest, bool manual_connecti
129     struct sockaddr_storage sockaddr;
130     socklen_t len = sizeof(sockaddr);
131     if (!dest.GetSockAddr((struct sockaddr*)&sockaddr, &len)) {
132-        LogInfo("Cannot get sockaddr for %s: unsupported network\n", dest.ToStringAddrPort());
133+        LogInfo("Cannot get sockaddr%s: unsupported network", fLogIPs ? strprintf(" for %s", dest.ToStringAddrPort()) : "");
134         return {};
135     }
136 
137-    if (!ConnectToSocket(*sock, (struct sockaddr*)&sockaddr, len, dest.ToStringAddrPort(), manual_connection)) {
138+    if (!ConnectToSocket(*sock, (struct sockaddr*)&sockaddr, len, dest_str, manual_connection)) {
139         return {};
140     }
141 
142diff --git a/test/functional/feature_anchors.py b/test/functional/feature_anchors.py
143index e82f28f4cc..1a2f0aa89c 100755
144--- a/test/functional/feature_anchors.py
145+++ b/test/functional/feature_anchors.py
146@@ -139,7 +139,7 @@ class AnchorsTest(BitcoinTestFramework):
147 
148         self.log.info("Restarting node attempts to reconnect to anchors")
149         with self.nodes[0].assert_debug_log([f"Trying to make an anchor connection to {ONION_ADDR}"]):
150-            self.start_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}"])
151+            self.start_node(0, extra_args=[f"-onion={onion_conf.addr[0]}:{onion_conf.addr[1]}", "-logips"])
152 
153 
154 if __name__ == "__main__":
155diff --git a/test/functional/p2p_addr_selfannouncement.py b/test/functional/p2p_addr_selfannouncement.py
156index 8ab75aaa06..88e73a2a15 100755
157--- a/test/functional/p2p_addr_selfannouncement.py
158+++ b/test/functional/p2p_addr_selfannouncement.py
159@@ -65,7 +65,7 @@ class SelfAnnouncementReceiver(P2PInterface):
160 class AddrSelfAnnouncementTest(BitcoinTestFramework):
161     def set_test_params(self):
162         self.num_nodes = 1
163-        self.extra_args = [[f"-externalip={IP_TO_ANNOUNCE}"]]
164+        self.extra_args = [[f"-externalip={IP_TO_ANNOUNCE}", "-logips"]]
165 
166     def run_test(self):
167         # populate addrman to have some addresses for a GETADDR response

sedited commented at 12:45 pm on January 30, 2026: contributor

Thanks for taking a look @l0rinc! I’m not sure if I want to litigate all of those yet. The example here just seems particularly egregious, because it leaks PII. Maybe we can figure out what to do with these here and then use that to inform us on the rest?

willcl-ark commented at 10:59 am on February 3, 2026: member

Concept ACK

I get not wanting to litigate all instances in this PR, so happy to ack this as-is as it’s a clear improvement on logging at startup all our PII, which is often the most-shared part of a debug log.

I would add my own suggestion of including RemoveLocal for inclusion in this change, mostly because you touch “opposing”" function AddLocal:

https://github.com/bitcoin/bitcoin/blob/be35408c5ad72885af1d0bab3f01dcee2435f74b/src/net.cpp#L310-L315

And this this protects against logging PII when Tor/I2P backends go up or down as part of normal operation.

net: Don’t log own ips during discover #34458

Reviews