Repository: bitcoin/bitcoin
File: src/net_processing.cpp (C++)

Code before:

    if (pfrom.nVersion == 0) {
        // Must have a version message before anything else
        LogDebug(BCLog::NET, "non-version message before version handshake. Message \"%s\" from peer=%d\n", SanitizeString(msg_type), pfrom.GetId());
        return;
    }

    if (msg_type == NetMsgType::VERACK) {
        if (pfrom.fSuccessfullyConnected) {
            LogDebug(BCLog::NET, "ignoring redundant verack message from peer=%d\n", pfrom.GetId());
            return;
        }

        // Log successful connections unconditionally for outbound, but not for inbound as those
        // can be triggered by an attacker at high rate.
        if (!pfrom.IsInboundConn() || LogAcceptCategory(BCLog::NET, BCLog::Level::Debug)) {
            const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
            LogInfo("New %s %s peer connected: version: %d, blocks=%d, peer=%d%s%s\n",
                      pfrom.ConnectionTypeAsString(),
                      TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
                      pfrom.nVersion.load(), peer->m_starting_height,
                      pfrom.GetId(), pfrom.LogIP(fLogIPs),
                      (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));
        }

        if (pfrom.GetCommonVersion() >= SHORT_IDS_BLOCKS_VERSION) {
            // Tell our peer we are willing to provide version 2 cmpctblocks.
            // However, we do not request new block announcements using
            // cmpctblock messages.
            // We send this to non-NODE NETWORK peers as well, because
            // they may wish to request compact blocks from us
            MakeAndPushMessage(pfrom, NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION);
        }

        if (m_txreconciliation) {
            if (!peer->m_wtxid_relay || !m_txreconciliation->IsPeerRegistered(pfrom.GetId())) {
                // We could have optimistically pre-registered/registered the peer. In that case,
                // we should forget about the reconciliation state here if this wasn't followed
                // by WTXIDRELAY (since WTXIDRELAY can't be announced later).
                m_txreconciliation->ForgetPeer(pfrom.GetId());
            }
        }

        if (auto tx_relay = peer->GetTxRelay()) {
            // `TxRelay::m_tx_inventory_to_send` must be empty before the
            // version handshake is completed as
            // `TxRelay::m_next_inv_send_time` is first initialised in
            // `SendMessages` after the verack is received. Any transactions
            // received during the version handshake would otherwise
            // immediately be advertised without random delay, potentially
            // leaking the time of arrival to a spy.

Review comment (bug):
Is there a reason why the message is split across the log statement and the `new_peer_info` lambda? That feels a bit clunky to me?

<details>
<summary>git diff on 8f553ac9df</summary>

```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index a823b2c2e2..54528ff3d0 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3659,23 +3659,23 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             return;
         }
 
-        auto new_peer_info = [&]() {
+        auto new_peer_msg = [&]() {
             const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
-            return strprintf("transport: %s, version: %d, blocks=%d peer=%d%s%s\n",
-                        TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
-                        pfrom.nVersion.load(), peer->m_starting_height,
-                        pfrom.GetId(), pfrom.LogIP(fLogIPs),
-                        (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));
+            return strprintf(
+                "New %s peer connected: transport: %s, version: %d, blocks=%d peer=%d%s%s\n",
+                pfrom.ConnectionTypeAsString(),
+                TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
+                pfrom.nVersion.load(), peer->m_starting_height,
+                pfrom.GetId(), pfrom.LogIP(fLogIPs),
+                (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));
         };
 
         // Log successful connections unconditionally for outbound, but not for inbound as those
         // can be triggered by an attacker at high rate.
         if (!pfrom.IsInboundConn()) {
-            LogInfo("New %s peer connected: %s",
-                    pfrom.ConnectionTypeAsString(), new_peer_info());
+            LogInfo("%s", new_peer_msg());
         } else {
-            LogDebug(BCLog::NET, "New %s peer connected: %s",
-                     pfrom.ConnectionTypeAsString(), new_peer_info());
+            LogDebug(BCLog::NET, "%s", new_peer_msg());
         }
 
         if (pfrom.GetCommonVersion() >= SHORT_IDS_BLOCKS_VERSION) {

```
</details>


Code after:

    if (pfrom.nVersion == 0) {
        // Must have a version message before anything else
        LogDebug(BCLog::NET, "non-version message before version handshake. Message \"%s\" from peer=%d\n", SanitizeString(msg_type), pfrom.GetId());
        return;
    }

    if (msg_type == NetMsgType::VERACK) {
        if (pfrom.fSuccessfullyConnected) {
            LogDebug(BCLog::NET, "ignoring redundant verack message from peer=%d\n", pfrom.GetId());
            return;
        }

        auto new_peer_msg = [&]() {
            const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
            return strprintf("New %s peer connected: transport: %s, version: %d, blocks=%d peer=%d%s%s\n",
                pfrom.ConnectionTypeAsString(),
                TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
                pfrom.nVersion.load(), peer->m_starting_height,
                pfrom.GetId(), pfrom.LogIP(fLogIPs),
                (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));
        };

        // Log successful connections unconditionally for outbound, but not for inbound as those
        // can be triggered by an attacker at high rate.
        if (pfrom.IsInboundConn()) {
            LogDebug(BCLog::NET, "%s", new_peer_msg());
        } else {
            LogInfo("%s", new_peer_msg());
        }

        if (pfrom.GetCommonVersion() >= SHORT_IDS_BLOCKS_VERSION) {
            // Tell our peer we are willing to provide version 2 cmpctblocks.
            // However, we do not request new block announcements using
            // cmpctblock messages.
            // We send this to non-NODE NETWORK peers as well, because
            // they may wish to request compact blocks from us
            MakeAndPushMessage(pfrom, NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION);
        }

        if (m_txreconciliation) {
            if (!peer->m_wtxid_relay || !m_txreconciliation->IsPeerRegistered(pfrom.GetId())) {
                // We could have optimistically pre-registered/registered the peer. In that case,
                // we should forget about the reconciliation state here if this wasn't followed
                // by WTXIDRELAY (since WTXIDRELAY can't be announced later).
                m_txreconciliation->ForgetPeer(pfrom.GetId());
            }
        }

        if (auto tx_relay = peer->GetTxRelay()) {
            // `TxRelay::m_tx_inventory_to_send` must be empty before the