From 8d6ff46f55f373e344278ab3f1ac27b1dba36623 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Tue, 11 Aug 2020 20:37:32 -0700 Subject: scripted-diff: Rename `OUTBOUND` ConnectionType to `OUTBOUND_FULL_RELAY` -BEGIN VERIFY SCRIPT- sed -i 's/OUTBOUND, /OUTBOUND_FULL_RELAY, /g' src/net.h sed -i 's/ConnectionType::OUTBOUND/ConnectionType::OUTBOUND_FULL_RELAY/g' src/test/net_tests.cpp src/test/fuzz/process_message.cpp src/test/fuzz/process_messages.cpp src/net.cpp src/test/denialofservice_tests.cpp src/net.h src/test/fuzz/net.cpp -END VERIFY SCRIPT- --- src/net.cpp | 6 +++--- src/net.h | 10 +++++----- src/test/denialofservice_tests.cpp | 4 ++-- src/test/fuzz/net.cpp | 2 +- src/test/fuzz/process_message.cpp | 2 +- src/test/fuzz/process_messages.cpp | 2 +- src/test/net_tests.cpp | 4 ++-- 7 files changed, 15 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index dc34743c5b..b013896ca3 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1847,7 +1847,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) case ConnectionType::INBOUND: case ConnectionType::MANUAL: break; - case ConnectionType::OUTBOUND: + case ConnectionType::OUTBOUND_FULL_RELAY: case ConnectionType::BLOCK_RELAY: case ConnectionType::ADDR_FETCH: case ConnectionType::FEELER: @@ -1953,12 +1953,12 @@ void CConnman::ThreadOpenConnections(const std::vector connect) if (fFeeler) { conn_type = ConnectionType::FEELER; } else if (nOutboundFullRelay < m_max_outbound_full_relay) { - conn_type = ConnectionType::OUTBOUND; + conn_type = ConnectionType::OUTBOUND_FULL_RELAY; } else if (nOutboundBlockRelay < m_max_outbound_block_relay) { conn_type = ConnectionType::BLOCK_RELAY; } else { // GetTryNewOutboundPeer() is true - conn_type = ConnectionType::OUTBOUND; + conn_type = ConnectionType::OUTBOUND_FULL_RELAY; } OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, conn_type); diff --git a/src/net.h b/src/net.h index 0b5ab2390e..40d5f5e47d 100644 --- a/src/net.h +++ b/src/net.h @@ -119,7 +119,7 @@ struct CSerializedNetMsg * connection. Aside from INBOUND, all types are initiated by us. */ enum class ConnectionType { INBOUND, /**< peer initiated connections */ - OUTBOUND, /**< full relay connections (blocks, addrs, txns) made automatically. Addresses selected from AddrMan. */ + OUTBOUND_FULL_RELAY, /**< full relay connections (blocks, addrs, txns) made automatically. Addresses selected from AddrMan. */ MANUAL, /**< connections to addresses added via addnode or the connect command line argument */ FEELER, /**< short lived connections used to test address validity */ BLOCK_RELAY, /**< only relay blocks to these automatic outbound connections. Addresses selected from AddrMan. */ @@ -209,7 +209,7 @@ public: bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; void SetNetworkActive(bool active); - void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, ConnectionType conn_type = ConnectionType::OUTBOUND); + void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, ConnectionType conn_type = ConnectionType::OUTBOUND_FULL_RELAY); bool CheckIncomingNonce(uint64_t nonce); bool ForNode(NodeId id, std::function func); @@ -824,7 +824,7 @@ public: bool IsOutboundOrBlockRelayConn() const { switch(m_conn_type) { - case ConnectionType::OUTBOUND: + case ConnectionType::OUTBOUND_FULL_RELAY: case ConnectionType::BLOCK_RELAY: return true; case ConnectionType::INBOUND: @@ -838,7 +838,7 @@ public: } bool IsFullOutboundConn() const { - return m_conn_type == ConnectionType::OUTBOUND; + return m_conn_type == ConnectionType::OUTBOUND_FULL_RELAY; } bool IsManualConn() const { @@ -867,7 +867,7 @@ public: case ConnectionType::MANUAL: case ConnectionType::FEELER: return false; - case ConnectionType::OUTBOUND: + case ConnectionType::OUTBOUND_FULL_RELAY: case ConnectionType::BLOCK_RELAY: case ConnectionType::ADDR_FETCH: return true; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 93bfa88024..028d3d482b 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 &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 1ff9d6b286..02a53f719c 100644 --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -46,7 +46,7 @@ void test_one_input(const std::vector& buffer) fuzzed_data_provider.ConsumeIntegral(), *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(0, 12)) { case 0: { diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index ec09acc6c6..52efe5ddfa 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& buffer) return; } CDataStream random_bytes_data_stream{fuzzed_data_provider.ConsumeRemainingBytes(), SER_NETWORK, PROTOCOL_VERSION}; - CNode& p2p_node = *MakeUnique(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(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 ef427442e9..33385c06cf 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& 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()); - 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(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 917ae571f5..1ac1d975d6 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -183,7 +183,7 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) CAddress addr = CAddress(CService(ipv4Addr, 7777), NODE_NETWORK); std::string pszDest; - std::unique_ptr pnode1 = MakeUnique(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, CAddress(), pszDest, ConnectionType::OUTBOUND); + std::unique_ptr pnode1 = MakeUnique(id++, NODE_NETWORK, height, hSocket, addr, 0, 0, CAddress(), pszDest, ConnectionType::OUTBOUND_FULL_RELAY); BOOST_CHECK(pnode1->IsInboundConn() == false); std::unique_ptr pnode2 = MakeUnique(id++, NODE_NETWORK, height, hSocket, addr, 1, 1, CAddress(), pszDest, ConnectionType::INBOUND); @@ -283,7 +283,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 pnode = MakeUnique(0, NODE_NETWORK, 0, INVALID_SOCKET, addr, 0, 0, CAddress{}, std::string{}, ConnectionType::OUTBOUND); + std::unique_ptr pnode = MakeUnique(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 -- cgit v1.2.3 From a6ab1e81f964df131cfa0e11e07bedb3283b823f Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Mon, 10 Aug 2020 12:21:07 -0700 Subject: [net] Remove unnecessary default args on OpenNetworkConnection --- src/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/net.h b/src/net.h index 40d5f5e47d..81c960cc9c 100644 --- a/src/net.h +++ b/src/net.h @@ -209,7 +209,7 @@ public: bool GetNetworkActive() const { return fNetworkActive; }; bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; }; void SetNetworkActive(bool active); - void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant *grantOutbound = nullptr, const char *strDest = nullptr, ConnectionType conn_type = ConnectionType::OUTBOUND_FULL_RELAY); + void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, const char* strDest, ConnectionType conn_type); bool CheckIncomingNonce(uint64_t nonce); bool ForNode(NodeId id, std::function func); -- cgit v1.2.3 From dff16b184b1428a068d144e5e4dde7595b4729c0 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Mon, 10 Aug 2020 14:48:54 -0700 Subject: [refactor] Restructure logic to check for addr relay. We previously identified if we relay addresses to the connection by checking for the existence of the m_addr_known data structure. With this commit, we answer this question based on the connection type. IsAddrRelayPeer() checked for the existence of the m_addr_known --- src/net.cpp | 3 +++ src/net.h | 8 ++++++-- src/net_processing.cpp | 15 +++++++-------- src/test/fuzz/net.cpp | 2 +- 4 files changed, 17 insertions(+), 11 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index b013896ca3..0a9a8f7fb4 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2784,6 +2784,9 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn hashContinue = uint256(); if (conn_type_in != ConnectionType::BLOCK_RELAY) { m_tx_relay = MakeUnique(); + } + + if (RelayAddrsWithConn()) { m_addr_known = MakeUnique(5000, 0.001); } diff --git a/src/net.h b/src/net.h index 81c960cc9c..a4830d09e9 100644 --- a/src/net.h +++ b/src/net.h @@ -861,6 +861,12 @@ public: return m_conn_type == ConnectionType::INBOUND; } + /* Whether we send addr messages over this connection */ + bool RelayAddrsWithConn() const + { + return m_conn_type != ConnectionType::BLOCK_RELAY; + } + bool ExpectServicesFromConn() const { switch(m_conn_type) { case ConnectionType::INBOUND: @@ -891,8 +897,6 @@ public: std::chrono::microseconds m_next_addr_send GUARDED_BY(cs_sendProcessing){0}; std::chrono::microseconds m_next_local_addr_send GUARDED_BY(cs_sendProcessing){0}; - bool IsAddrRelayPeer() const { return m_addr_known != nullptr; } - // List of block ids we still have announce. // There is no final sorting before sending, as they are always sent immediately // and in the order requested. diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0e049bd666..4da818c996 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1531,7 +1531,7 @@ static void RelayAddress(const CAddress& addr, bool fReachable, const CConnman& assert(nRelayNodes <= best.size()); auto sortfunc = [&best, &hasher, nRelayNodes](CNode* pnode) { - if (pnode->IsAddrRelayPeer()) { + if (pnode->RelayAddrsWithConn()) { uint64_t hashKey = CSipHasher(hasher).Write(pnode->GetId()).Finalize(); for (unsigned int i = 0; i < nRelayNodes; i++) { if (hashKey > best[i].first) { @@ -2458,9 +2458,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty UpdatePreferredDownload(pfrom, State(pfrom.GetId())); } - if (!pfrom.IsInboundConn() && pfrom.IsAddrRelayPeer()) - { - // Advertise our address + if (!pfrom.IsInboundConn() && !pfrom.IsBlockOnlyConn()) { if (fListen && !::ChainstateActive().IsInitialBlockDownload()) { CAddress addr = GetLocalAddress(&pfrom.addr, pfrom.GetLocalServices()); @@ -2479,6 +2477,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty // Get recent addresses m_connman.PushMessage(&pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR)); pfrom.fGetAddr = true; + m_connman.MarkAddressGood(pfrom.addr); } @@ -2584,7 +2583,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty std::vector vAddr; vRecv >> vAddr; - if (!pfrom.IsAddrRelayPeer()) { + if (!pfrom.RelayAddrsWithConn()) { return; } if (vAddr.size() > MAX_ADDR_TO_SEND) @@ -3522,7 +3521,7 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty LogPrint(BCLog::NET, "Ignoring \"getaddr\" from outbound connection. peer=%d\n", pfrom.GetId()); return; } - if (!pfrom.IsAddrRelayPeer()) { + if (!pfrom.RelayAddrsWithConn()) { LogPrint(BCLog::NET, "Ignoring \"getaddr\" from block-relay-only connection. peer=%d\n", pfrom.GetId()); return; } @@ -4122,7 +4121,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) int64_t nNow = GetTimeMicros(); auto current_time = GetTime(); - if (pto->IsAddrRelayPeer() && !::ChainstateActive().IsInitialBlockDownload() && pto->m_next_local_addr_send < current_time) { + if (pto->RelayAddrsWithConn() && !::ChainstateActive().IsInitialBlockDownload() && pto->m_next_local_addr_send < current_time) { AdvertiseLocal(pto); pto->m_next_local_addr_send = PoissonNextSend(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL); } @@ -4130,7 +4129,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // // Message: addr // - if (pto->IsAddrRelayPeer() && pto->m_next_addr_send < current_time) { + if (pto->RelayAddrsWithConn() && pto->m_next_addr_send < current_time) { pto->m_next_addr_send = PoissonNextSend(current_time, AVG_ADDRESS_BROADCAST_INTERVAL); std::vector vAddr; vAddr.reserve(pto->vAddrToSend.size()); diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp index 02a53f719c..cd0c93b8d0 100644 --- a/src/test/fuzz/net.cpp +++ b/src/test/fuzz/net.cpp @@ -147,7 +147,7 @@ void test_one_input(const std::vector& 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::PF_NONE, NetPermissionFlags::PF_BLOOMFILTER, NetPermissionFlags::PF_RELAY, NetPermissionFlags::PF_FORCERELAY, NetPermissionFlags::PF_NOBAN, NetPermissionFlags::PF_MEMPOOL, NetPermissionFlags::PF_ISIMPLICIT, NetPermissionFlags::PF_ALL}) : -- cgit v1.2.3 From ff6b9081add3a40d53b1cc1352b57eeb46e41d45 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Wed, 2 Sep 2020 14:03:43 -0700 Subject: [doc] Explain address handling logic in process messages Co-authored-by: Suhas Daftuar --- src/net_processing.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 4da818c996..dae77dabfb 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2459,6 +2459,22 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty } if (!pfrom.IsInboundConn() && !pfrom.IsBlockOnlyConn()) { + // For outbound peers, we try to relay our address (so that other + // nodes can try to find us more quickly, as we have no guarantee + // that an outbound peer is even aware of how to reach us) and do a + // one-time address fetch (to help populate/update our addrman). If + // we're starting up for the first time, our addrman may be pretty + // empty and no one will know who we are, so these mechanisms are + // important to help us connect to the network. + // + // We also update the addrman to record connection success for + // these peers (which include OUTBOUND_FULL_RELAY and FEELER + // connections) so that addrman will have an up-to-date notion of + // which peers are online and available. + // + // We skip these operations for BLOCK_RELAY peers to avoid + // potentially leaking information about our BLOCK_RELAY + // connections via the addrman or address relay. if (fListen && !::ChainstateActive().IsInitialBlockDownload()) { CAddress addr = GetLocalAddress(&pfrom.addr, pfrom.GetLocalServices()); @@ -2478,6 +2494,8 @@ void PeerLogicValidation::ProcessMessage(CNode& pfrom, const std::string& msg_ty m_connman.PushMessage(&pfrom, CNetMsgMaker(nSendVersion).Make(NetMsgType::GETADDR)); pfrom.fGetAddr = true; + // Moves address from New to Tried table in Addrman, resolves + // tried-table collisions, etc. m_connman.MarkAddressGood(pfrom.addr); } -- cgit v1.2.3 From 1d74fc7df621b31d1b8adc9d7f53e7478d6e40b5 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Tue, 11 Aug 2020 10:36:42 -0700 Subject: [trivial] Small style updates --- src/net.cpp | 2 +- src/net.h | 6 +++--- src/net_processing.cpp | 3 ++- src/test/denialofservice_tests.cpp | 4 ++-- 4 files changed, 8 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index 0a9a8f7fb4..b0597093e8 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1843,7 +1843,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) // but inbound and manual peers do not use our outbound slots. Inbound peers // also have the added issue that they could be attacker controlled and used // to prevent us from connecting to particular hosts if we used them here. - switch(pnode->m_conn_type){ + switch (pnode->m_conn_type) { case ConnectionType::INBOUND: case ConnectionType::MANUAL: break; diff --git a/src/net.h b/src/net.h index a4830d09e9..75e0657d3d 100644 --- a/src/net.h +++ b/src/net.h @@ -823,7 +823,7 @@ public: std::atomic_bool fPauseSend{false}; bool IsOutboundOrBlockRelayConn() const { - switch(m_conn_type) { + switch (m_conn_type) { case ConnectionType::OUTBOUND_FULL_RELAY: case ConnectionType::BLOCK_RELAY: return true; @@ -868,7 +868,7 @@ public: } bool ExpectServicesFromConn() const { - switch(m_conn_type) { + switch (m_conn_type) { case ConnectionType::INBOUND: case ConnectionType::MANUAL: case ConnectionType::FEELER: @@ -892,7 +892,7 @@ public: // flood relay std::vector vAddrToSend; - std::unique_ptr m_addr_known = nullptr; + std::unique_ptr m_addr_known{nullptr}; bool fGetAddr{false}; std::chrono::microseconds m_next_addr_send GUARDED_BY(cs_sendProcessing){0}; std::chrono::microseconds m_next_local_addr_send GUARDED_BY(cs_sendProcessing){0}; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index dae77dabfb..b419ee7e61 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -882,8 +882,9 @@ void PeerLogicValidation::InitializeNode(CNode *pnode) { LOCK(g_peer_mutex); g_peer_map.emplace_hint(g_peer_map.end(), nodeid, std::move(peer)); } - if(!pnode->IsInboundConn()) + if (!pnode->IsInboundConn()) { PushNodeVersion(*pnode, m_connman, GetTime()); + } } void PeerLogicValidation::ReattemptInitialBroadcast(CScheduler& scheduler) const diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 028d3d482b..bf0659587c 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_FULL_RELAY); + 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 &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_FULL_RELAY)); + 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); -- cgit v1.2.3 From da3a0be61b025224231206cb4656e420453bfdeb Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Tue, 11 Aug 2020 10:37:57 -0700 Subject: [test] Add explicit tests that connection types get set correctly --- src/test/net_tests.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'src') diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 1ac1d975d6..85ebc89673 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -184,9 +184,19 @@ BOOST_AUTO_TEST_CASE(cnode_simple_test) std::string pszDest; std::unique_ptr pnode1 = MakeUnique(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 pnode2 = MakeUnique(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); } -- cgit v1.2.3 From 1e563aed785565af6b7aed7f7399c52205d8f19c Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Wed, 12 Aug 2020 09:56:17 -0700 Subject: [refactor] Simplify check for block-relay-only connection. Previously we deduced it was a block-relay-only based on presence of the m_tx_relay structure. Now we have the ability to identify it directly via a connection type accessor function. --- src/net_processing.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b419ee7e61..ce4ac3cd75 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2018,7 +2018,7 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateMan } } - if (!pfrom.fDisconnect && pfrom.IsOutboundOrBlockRelayConn() && nodestate->pindexBestKnownBlock != nullptr && pfrom.m_tx_relay != nullptr) { + if (!pfrom.fDisconnect && pfrom.IsFullOutboundConn() && nodestate->pindexBestKnownBlock != nullptr) { // If this is an outbound full-relay peer, check to see if we should protect // it from the bad/lagging chain logic. // Note that block-relay-only peers are already implicitly protected, so we -- cgit v1.2.3 From 4829b6fcc6489b445f80689af6c2a1a919f176b1 Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Thu, 13 Aug 2020 21:54:38 -0700 Subject: [refactor] Simplify connection type logic in ThreadOpenConnections Consolidate the logic to determine connection type into one conditional to clarify how they are chosen. --- src/net.cpp | 61 ++++++++++++++++++++++++------------------------------------- 1 file changed, 24 insertions(+), 37 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index b0597093e8..262197117d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1856,28 +1856,32 @@ void CConnman::ThreadOpenConnections(const std::vector connect) } } - // Feeler Connections - // - // Design goals: - // * Increase the number of connectable addresses in the tried table. - // - // Method: - // * Choose a random address from new and attempt to connect to it if we can connect - // successfully it is added to tried. - // * Start attempting feeler connections only after node finishes making outbound - // connections. - // * Only make a feeler connection once every few minutes. - // + ConnectionType conn_type = ConnectionType::OUTBOUND_FULL_RELAY; + int64_t nTime = GetTimeMicros(); bool fFeeler = false; - if (nOutboundFullRelay >= m_max_outbound_full_relay && nOutboundBlockRelay >= m_max_outbound_block_relay && !GetTryNewOutboundPeer()) { - int64_t nTime = GetTimeMicros(); // The current time right now (in microseconds). - if (nTime > nNextFeeler) { - nNextFeeler = PoissonNextSend(nTime, FEELER_INTERVAL); - fFeeler = true; - } else { - continue; - } + // Determine what type of connection to open. Opening + // OUTBOUND_FULL_RELAY connections gets the highest priority until we + // meet our full-relay capacity. Then we open BLOCK_RELAY connection + // until we hit our block-relay-only peer limit. + // GetTryNewOutboundPeer() gets set when a stale tip is detected, so we + // try opening an additional OUTBOUND_FULL_RELAY connection. If none of + // these conditions are met, check the nNextFeeler timer to decide if + // we should open a FEELER. + + if (nOutboundFullRelay < m_max_outbound_full_relay) { + // OUTBOUND_FULL_RELAY + } else if (nOutboundBlockRelay < m_max_outbound_block_relay) { + conn_type = ConnectionType::BLOCK_RELAY; + } else if (GetTryNewOutboundPeer()) { + // OUTBOUND_FULL_RELAY + } else if (nTime > nNextFeeler) { + nNextFeeler = PoissonNextSend(nTime, FEELER_INTERVAL); + conn_type = ConnectionType::FEELER; + fFeeler = true; + } else { + // skip to next iteration of while loop + continue; } addrman.ResolveCollisions(); @@ -1944,23 +1948,6 @@ void CConnman::ThreadOpenConnections(const std::vector connect) LogPrint(BCLog::NET, "Making feeler connection to %s\n", addrConnect.ToString()); } - ConnectionType conn_type; - // Determine what type of connection to open. If fFeeler is not - // set, open OUTBOUND connections until we meet our full-relay - // capacity. Then open BLOCK_RELAY connections until we hit our - // block-relay peer limit. Otherwise, default to opening an - // OUTBOUND connection. - if (fFeeler) { - conn_type = ConnectionType::FEELER; - } else if (nOutboundFullRelay < m_max_outbound_full_relay) { - conn_type = ConnectionType::OUTBOUND_FULL_RELAY; - } else if (nOutboundBlockRelay < m_max_outbound_block_relay) { - conn_type = ConnectionType::BLOCK_RELAY; - } else { - // GetTryNewOutboundPeer() is true - conn_type = ConnectionType::OUTBOUND_FULL_RELAY; - } - OpenNetworkConnection(addrConnect, (int)setConnected.size() >= std::min(nMaxConnections - 1, 2), &grant, nullptr, conn_type); } } -- cgit v1.2.3 From d5a57cef62ee9e9d30f7e3b80e178149ceeef67c Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Thu, 13 Aug 2020 21:58:08 -0700 Subject: [doc] Describe connection types in more depth. --- src/net.h | 54 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/net.h b/src/net.h index 75e0657d3d..30abd3813d 100644 --- a/src/net.h +++ b/src/net.h @@ -118,12 +118,54 @@ struct CSerializedNetMsg * information we have available at the time of opening or accepting the * connection. Aside from INBOUND, all types are initiated by us. */ enum class ConnectionType { - INBOUND, /**< peer initiated connections */ - OUTBOUND_FULL_RELAY, /**< full relay connections (blocks, addrs, txns) made automatically. Addresses selected from AddrMan. */ - MANUAL, /**< connections to addresses added via addnode or the connect command line argument */ - FEELER, /**< short lived connections used to test address validity */ - BLOCK_RELAY, /**< only relay blocks to these automatic outbound connections. Addresses selected from AddrMan. */ - ADDR_FETCH, /**< short lived connections used to solicit addrs when starting the node without a populated AddrMan */ + /** + * Inbound connections are those initiated by a peer. This is the only + * property we know at the time of connection, until P2P messages are + * exchanged. + */ + INBOUND, + + /** + * These are the default connections that we use to connect with the + * network. There is no restriction on what is relayed- by default we relay + * blocks, addresses & transactions. We automatically attempt to open + * MAX_OUTBOUND_FULL_RELAY_CONNECTIONS using addresses from our AddrMan. + */ + OUTBOUND_FULL_RELAY, + + + /** + * We open manual connections to addresses that users explicitly inputted + * via the addnode RPC, or the -connect command line argument. Even if a + * manual connection is misbehaving, we do not automatically disconnect or + * add it to our discouragement filter. + */ + MANUAL, + + /** + * Feeler connections are short lived connections used to increase the + * number of connectable addresses in our AddrMan. Approximately every + * FEELER_INTERVAL, we attempt to connect to a random address from the new + * table. If successful, we add it to the tried table. + */ + FEELER, + + /** + * We use block-relay-only connections to help prevent against partition + * attacks. By not relaying transactions or addresses, these connections + * are harder to detect by a third party, thus helping obfuscate the + * network topology. We automatically attempt to open + * MAX_BLOCK_RELAY_ONLY_CONNECTIONS using addresses from our AddrMan. + */ + BLOCK_RELAY, + + /** + * AddrFetch connections are short lived connections used to solicit + * addresses from peers. These are initiated to addresses submitted via the + * -seednode command line argument, or under certain conditions when the + * AddrMan is empty. + */ + ADDR_FETCH, }; class NetEventsInterface; -- cgit v1.2.3 From eb1c5d090f4ef844dabc1e8bf3864c1afe1c791c Mon Sep 17 00:00:00 2001 From: Amiti Uttarwar Date: Thu, 20 Aug 2020 15:26:27 -0700 Subject: [doc] Follow developer notes, add comment about missing default. --- src/net.cpp | 2 +- src/net.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/net.cpp b/src/net.cpp index 262197117d..f483f9922f 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1852,7 +1852,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect) case ConnectionType::ADDR_FETCH: case ConnectionType::FEELER: setConnected.insert(pnode->addr.GetGroup(addrman.m_asmap)); - } + } // no default case, so the compiler can warn about missing cases } } diff --git a/src/net.h b/src/net.h index 30abd3813d..60c3dc6aef 100644 --- a/src/net.h +++ b/src/net.h @@ -874,7 +874,7 @@ public: case ConnectionType::ADDR_FETCH: case ConnectionType::FEELER: return false; - } + } // no default case, so the compiler can warn about missing cases assert(false); } @@ -919,7 +919,7 @@ public: case ConnectionType::BLOCK_RELAY: case ConnectionType::ADDR_FETCH: return true; - } + } // no default case, so the compiler can warn about missing cases assert(false); } -- cgit v1.2.3