Merge bitcoin/bitcoin#35032: net_processing: don't modify addrman for private broadcast connections

1ed1a12 net_processing: don't modify addrman for private broadcast connections (Vasil Dimov)

Pull request description:

  It is best if the internal addrman database is not modified with information coming from private broadcast connections because that information can potentially later be sent via other connections.

  instagibbs suggested this change, thank you!

ACKs for top commit:
  l0rinc:
    code review ACK 1ed1a12
  instagibbs:
    ACK 1ed1a12
  achow101:
    ACK 1ed1a12
  andrewtoth:
    ACK 1ed1a12
  danielabrozzoni:
    tACK 1ed1a12

Tree-SHA512: 13845778445e56e3015a569f3b4165a749011e9dd67dcab38e960a368a0f5d3822173af5e3cb950998326549a021d1c8df644ce6d761cd46dd7597106b9ceec9

Author: achow101

src/net_processing.cpp

@@ -3600,7 +3600,7 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
         }
         vRecv.ignore(8); // Ignore the addrMe service bits sent by the peer
         vRecv >> CNetAddr::V1(addrMe);
-        if (!pfrom.IsInboundConn())
+        if (!pfrom.IsInboundConn() && !pfrom.IsPrivateBroadcastConn())
         {
             // Overwrites potentially existing services. In contrast to this,
             // unvalidated services received via gossip relay in ADDR/ADDRV2

src/test/net_tests.cpp

@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <addrman.h>
 #include <chainparams.h>
 #include <clientversion.h>
 #include <common/args.h>
@@ -1559,4 +1560,34 @@ BOOST_AUTO_TEST_CASE(v2transport_test)
     }
 }
 
+BOOST_AUTO_TEST_CASE(private_broadcast_version_does_not_update_addrman_services)
+{
+    LOCK(NetEventsInterface::g_msgproc_mutex);
+
+    const CNetAddr source{LookupHost("2.3.4.5", /*fAllowLookup=*/false).value()};
+    const CAddress addr{Lookup("1.2.3.4", 8333, /*fAllowLookup=*/false).value(), NODE_NONE};
+    BOOST_REQUIRE(m_node.addrman->Add({addr}, source));
+    CNode node{/*id=*/0,
+               /*sock=*/nullptr,
+               /*addrIn=*/addr,
+               /*nKeyedNetGroupIn=*/0,
+               /*nLocalHostNonceIn=*/0,
+               /*addrBindIn=*/CService{},
+               /*addrNameIn=*/"",
+               /*conn_type_in=*/ConnectionType::PRIVATE_BROADCAST,
+               /*inbound_onion=*/false,
+               /*network_key=*/0};
+
+    auto& connman = static_cast<ConnmanTestMsg&>(*m_node.connman);
+    connman.Handshake(node,
+                      /*successfully_connected=*/false,
+                      /*remote_services=*/NODE_NETWORK,
+                      /*local_services=*/NODE_NONE,
+                      /*version=*/PROTOCOL_VERSION,
+                      /*relay_txs=*/true);
+
+    BOOST_CHECK_EQUAL(m_node.addrman->Select().first.nServices, NODE_NONE);
+    m_node.peerman->FinalizeNode(node);
+}
+
 BOOST_AUTO_TEST_SUITE_END()