Merge #19724: [net] Cleanup connection types- followups

eb1c5d090f4ef844dabc1e8bf3864c1afe1c791c [doc] Follow developer notes, add comment about missing default. (Amiti Uttarwar)
d5a57cef62ee9e9d30f7e3b80e178149ceeef67c [doc] Describe connection types in more depth. (Amiti Uttarwar)
4829b6fcc6489b445f80689af6c2a1a919f176b1 [refactor] Simplify connection type logic in ThreadOpenConnections (Amiti Uttarwar)
1e563aed785565af6b7aed7f7399c52205d8f19c [refactor] Simplify check for block-relay-only connection. (Amiti Uttarwar)
da3a0be61b025224231206cb4656e420453bfdeb [test] Add explicit tests that connection types get set correctly (Amiti Uttarwar)
1d74fc7df621b31d1b8adc9d7f53e7478d6e40b5 [trivial] Small style updates (Amiti Uttarwar)
ff6b9081add3a40d53b1cc1352b57eeb46e41d45 [doc] Explain address handling logic in process messages (Amiti Uttarwar)
dff16b184b1428a068d144e5e4dde7595b4729c0 [refactor] Restructure logic to check for addr relay. (Amiti Uttarwar)
a6ab1e81f964df131cfa0e11e07bedb3283b823f [net] Remove unnecessary default args on OpenNetworkConnection (Amiti Uttarwar)
8d6ff46f55f373e344278ab3f1ac27b1dba36623 scripted-diff: Rename `OUTBOUND` ConnectionType to `OUTBOUND_FULL_RELAY` (Amiti Uttarwar)

Pull request description:

  This PR addresses outstanding review comments from #19316. It further simplifies `net.cpp` complexity and adds documentation about design goals about different connection types.

ACKs for top commit:
  naumenkogs:
    ACK eb1c5d090f4ef844dabc1e8bf3864c1afe1c791c
  laanwj:
    Code review ACK eb1c5d090f4ef844dabc1e8bf3864c1afe1c791c

Tree-SHA512: 2fe14e428404c95661e5518c8c90db07ab5b9ebb1bac921b3bdf82b181f444fae379f8fc0a2b619e6b4693f01c55bd246fbd8c8eedadd96849a30de3161afee5

5 files changed, 18 insertions, 8 deletions

diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 93bfa8802..bf0659587 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -84,7 +84,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
 
     // Mock an outbound peer
     CAddress addr1(ip(0xa0b0c001), NODE_NONE);
-    CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", ConnectionType::OUTBOUND);
+    CNode dummyNode1(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), 0, INVALID_SOCKET, addr1, 0, 0, CAddress(), "", ConnectionType::OUTBOUND_FULL_RELAY);
     dummyNode1.SetSendVersion(PROTOCOL_VERSION);
 
     peerLogic->InitializeNode(&dummyNode1);
@@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
 static void AddRandomOutboundPeer(std::vector<CNode *> &vNodes, PeerLogicValidation &peerLogic, CConnmanTest* connman)
 {
     CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE);
-    vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK|NODE_WITNESS), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", ConnectionType::OUTBOUND));
+    vNodes.emplace_back(new CNode(id++, ServiceFlags(NODE_NETWORK | NODE_WITNESS), 0, INVALID_SOCKET, addr, 0, 0, CAddress(), "", ConnectionType::OUTBOUND_FULL_RELAY));
     CNode &node = *vNodes.back();
     node.SetSendVersion(PROTOCOL_VERSION);
 
diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp
index 1ff9d6b28..cd0c93b8d 100644
--- a/src/test/fuzz/net.cpp
+++ b/src/test/fuzz/net.cpp
@@ -46,7 +46,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
                fuzzed_data_provider.ConsumeIntegral<uint64_t>(),
                *address_bind,
                fuzzed_data_provider.ConsumeRandomLengthString(32),
-               fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH})};
+               fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH})};
     while (fuzzed_data_provider.ConsumeBool()) {
         switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 12)) {
         case 0: {
@@ -147,7 +147,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     const int ref_count = node.GetRefCount();
     assert(ref_count >= 0);
     (void)node.GetSendVersion();
-    (void)node.IsAddrRelayPeer();
+    (void)node.RelayAddrsWithConn();
 
     const NetPermissionFlags net_permission_flags = fuzzed_data_provider.ConsumeBool() ?
                                                         fuzzed_data_provider.PickValueInArray<NetPermissionFlags>({NetPermissionFlags::PF_NONE, NetPermissionFlags::PF_BLOOMFILTER, NetPermissionFlags::PF_RELAY, NetPermissionFlags::PF_FORCERELAY, NetPermissionFlags::PF_NOBAN, NetPermissionFlags::PF_MEMPOOL, NetPermissionFlags::PF_ISIMPLICIT, NetPermissionFlags::PF_ALL}) :
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp
index ec09acc6c..52efe5ddf 100644
--- a/src/test/fuzz/process_message.cpp
+++ b/src/test/fuzz/process_message.cpp
@@ -68,7 +68,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
         return;
     }
     CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(), SER_NETWORK, PROTOCOL_VERSION};
-    CNode& p2p_node = *MakeUnique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND).release();
+    CNode& p2p_node = *MakeUnique<CNode>(0, ServiceFlags(NODE_NETWORK | NODE_WITNESS | NODE_BLOOM), 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND_FULL_RELAY).release();
     p2p_node.fSuccessfullyConnected = true;
     p2p_node.nVersion = PROTOCOL_VERSION;
     p2p_node.SetSendVersion(PROTOCOL_VERSION);
diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp
index ef427442e..33385c06c 100644
--- a/src/test/fuzz/process_messages.cpp
+++ b/src/test/fuzz/process_messages.cpp
@@ -44,7 +44,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     const auto num_peers_to_add = fuzzed_data_provider.ConsumeIntegralInRange(1, 3);
     for (int i = 0; i < num_peers_to_add; ++i) {
         const ServiceFlags service_flags = ServiceFlags(fuzzed_data_provider.ConsumeIntegral<uint64_t>());
-        const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH});
+        const ConnectionType conn_type = fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH});
         peers.push_back(MakeUnique<CNode>(i, service_flags, 0, INVALID_SOCKET, CAddress{CService{in_addr{0x0100007f}, 7777}, NODE_NETWORK}, 0, 0, CAddress{}, std::string{}, conn_type).release());
         CNode& p2p_node = *peers.back();
 
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
index 917ae571f..85ebc8967 100644
--- a/src/test/net_tests.cpp
+++ b/src/test/net_tests.cpp
@@ -183,10 +183,20 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test)
     CAddress addr = CAddress(CService(ipv4Addr, 7777), NODE_NETWORK);
     std::string pszDest;
 
-    std::unique_ptr<CNode> pnode1 = MakeUnique<CNode>(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, CAddress(), pszDest, ConnectionType::OUTBOUND);
+    std::unique_ptr<CNode> pnode1 = MakeUnique<CNode>(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, CAddress(), pszDest, ConnectionType::OUTBOUND_FULL_RELAY);
+    BOOST_CHECK(pnode1->IsFullOutboundConn() == true);
+    BOOST_CHECK(pnode1->IsManualConn() == false);
+    BOOST_CHECK(pnode1->IsBlockOnlyConn() == false);
+    BOOST_CHECK(pnode1->IsFeelerConn() == false);
+    BOOST_CHECK(pnode1->IsAddrFetchConn() == false);
     BOOST_CHECK(pnode1->IsInboundConn() == false);
 
     std::unique_ptr<CNode> pnode2 = MakeUnique<CNode>(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, CAddress(), pszDest, ConnectionType::INBOUND);
+    BOOST_CHECK(pnode2->IsFullOutboundConn() == false);
+    BOOST_CHECK(pnode2->IsManualConn() == false);
+    BOOST_CHECK(pnode2->IsBlockOnlyConn() == false);
+    BOOST_CHECK(pnode2->IsFeelerConn() == false);
+    BOOST_CHECK(pnode2->IsAddrFetchConn() == false);
     BOOST_CHECK(pnode2->IsInboundConn() == true);
 }
 
@@ -283,7 +293,7 @@ BOOST_AUTO_TEST_CASE(ipv4_peer_with_ipv6_addrMe_test)
     in_addr ipv4AddrPeer;
     ipv4AddrPeer.s_addr = 0xa0b0c001;
     CAddress addr = CAddress(CService(ipv4AddrPeer, 7777), NODE_NETWORK);
-    std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, 0, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND);
+    std::unique_ptr<CNode> pnode = MakeUnique<CNode>(0, NODE_NETWORK, 0, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND_FULL_RELAY);
     pnode->fSuccessfullyConnected.store(true);
 
     // the peer claims to be reaching us via IPv6