diff --git a/doc/release-notes-6365.md b/doc/release-notes-6365.md new file mode 100644 index 0000000000000..3b478f11b7929 --- /dev/null +++ b/doc/release-notes-6365.md @@ -0,0 +1,8 @@ +P2P and network changes +------ + +- Nodes with multiple reachable networks will actively try to have at least one + outbound connection to each network. This improves individual resistance to + eclipse attacks and network level resistance to partition attacks. Users no + longer need to perform active measures to ensure being connected to multiple + enabled networks. diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 1a4cc323e2925..1a3f5381181b9 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -134,6 +134,7 @@ BITCOIN_TESTS =\ test/minisketch_tests.cpp \ test/miner_tests.cpp \ test/multisig_tests.cpp \ + test/net_peer_connection_tests.cpp \ test/net_peer_eviction_tests.cpp \ test/net_tests.cpp \ test/netbase_tests.cpp \ diff --git a/src/Makefile.test_fuzz.include b/src/Makefile.test_fuzz.include index 06734546e9d4c..6f31b838ef27f 100644 --- a/src/Makefile.test_fuzz.include +++ b/src/Makefile.test_fuzz.include @@ -11,7 +11,8 @@ TEST_FUZZ_H = \ test/fuzz/fuzz.h \ test/fuzz/FuzzedDataProvider.h \ test/fuzz/util.h \ - test/util/mining.h + test/util/mining.h \ + test/fuzz/util/net.h libtest_fuzz_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(MINIUPNPC_CPPFLAGS) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS) libtest_fuzz_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) @@ -19,6 +20,7 @@ libtest_fuzz_a_SOURCES = \ test/fuzz/fuzz.cpp \ test/util/mining.cpp \ test/fuzz/util.cpp \ + test/fuzz/util/net.cpp \ $(TEST_FUZZ_H) LIBTEST_FUZZ += $(LIBBITCOIN_SERVER) diff --git a/src/net.cpp b/src/net.cpp index 351c114d60869..d72a9aeef5354 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -107,6 +107,9 @@ static constexpr std::chrono::seconds MAX_UPLOAD_TIMEFRAME{60 * 60 * 24}; // A random time period (0 to 1 seconds) is added to feeler connections to prevent synchronization. static constexpr auto FEELER_SLEEP_WINDOW{1s}; +/** Frequency to attempt extra connections to reachable networks we're not connected to yet **/ +static constexpr auto EXTRA_NETWORK_PEER_INTERVAL{5min}; + /** Used to pass flags to the Bind() function */ enum BindFlags { BF_NONE = 0, @@ -520,21 +523,25 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo const uint16_t default_port{pszDest != nullptr ? Params().GetDefaultPort(pszDest) : Params().GetDefaultPort()}; if (pszDest) { - const std::vector resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)}; + std::vector resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)}; if (!resolved.empty()) { - const CService& rnd{resolved[GetRand(resolved.size())]}; - addrConnect = CAddress{MaybeFlipIPv6toCJDNS(rnd), NODE_NONE}; - if (!addrConnect.IsValid()) { - LogPrint(BCLog::NET, "Resolver returned invalid address %s for %s\n", addrConnect.ToStringAddrPort(), pszDest); - return nullptr; - } - // It is possible that we already have a connection to the IP/port pszDest resolved to. - // In that case, drop the connection that was just created. - LOCK(m_nodes_mutex); - CNode* pnode = FindNode(static_cast(addrConnect)); - if (pnode) { - LogPrintf("Failed to open new connection, already connected\n"); - return nullptr; + Shuffle(resolved.begin(), resolved.end(), FastRandomContext()); + // If the connection is made by name, it can be the case that the name resolves to more than one address. + // We don't want to connect any more of them if we are already connected to one + for (const auto& r : resolved) { + addrConnect = CAddress{MaybeFlipIPv6toCJDNS(r), NODE_NONE}; + if (!addrConnect.IsValid()) { + LogPrint(BCLog::NET, "Resolver returned invalid address %s for %s\n", addrConnect.ToStringAddrPort(), pszDest); + return nullptr; + } + // It is possible that we already have a connection to the IP/port pszDest resolved to. + // In that case, drop the connection that was just created. + LOCK(m_nodes_mutex); + CNode* pnode = FindNode(static_cast(addrConnect)); + if (pnode) { + LogPrintf("Not opening a connection to %s, already connected to %s\n", pszDest, addrConnect.ToStringAddrPort()); + return nullptr; + } } } } @@ -2252,6 +2259,9 @@ void CConnman::DisconnectNodes() // close socket and cleanup pnode->CloseSocketDisconnect(this); + // update connection count by network + if (pnode->IsManualOrFullOutboundConn()) --m_network_conn_counts[pnode->addr.GetNetwork()]; + // hold in disconnected pool until all refs are released pnode->Release(); m_nodes_disconnected.push_back(pnode); @@ -3178,6 +3188,28 @@ std::unordered_set CConnman::GetReachableEmptyNetworks() const return networks; } +bool CConnman::MultipleManualOrFullOutboundConns(Network net) const +{ + AssertLockHeld(m_nodes_mutex); + return m_network_conn_counts[net] > 1; +} + +bool CConnman::MaybePickPreferredNetwork(std::optional& network) +{ + std::array nets{NET_IPV4, NET_IPV6, NET_ONION, NET_I2P, NET_CJDNS}; + Shuffle(nets.begin(), nets.end(), FastRandomContext()); + + LOCK(m_nodes_mutex); + for (const auto net : nets) { + if (IsReachable(net) && m_network_conn_counts[net] == 0 && addrman.Size(net) != 0) { + network = net; + return true; + } + } + + return false; +} + void CConnman::ThreadOpenConnections(const std::vector connect, CDeterministicMNManager& dmnman) { AssertLockNotHeld(m_unused_i2p_sessions_mutex); @@ -3213,6 +3245,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe // Minimum time before next feeler connection (in microseconds). auto next_feeler = GetExponentialRand(start, FEELER_INTERVAL); auto next_extra_block_relay = GetExponentialRand(start, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL); + auto next_extra_network_peer{GetExponentialRand(start, EXTRA_NETWORK_PEER_INTERVAL)}; const bool dnsseed = gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED); bool add_fixed_seeds = gArgs.GetBoolArg("-fixedseeds", DEFAULT_FIXEDSEEDS); const bool use_seednodes{gArgs.IsArgSet("-seednode")}; @@ -3341,6 +3374,7 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe auto now = GetTime(); bool anchor = false; bool fFeeler = false; + std::optional preferred_net; bool onion_only = false; // Determine what type of connection to open. Opening @@ -3391,6 +3425,17 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe next_feeler = GetExponentialRand(now, FEELER_INTERVAL); conn_type = ConnectionType::FEELER; fFeeler = true; + } else if (nOutboundFullRelay == m_max_outbound_full_relay && + m_max_outbound_full_relay == MAX_OUTBOUND_FULL_RELAY_CONNECTIONS && + now > next_extra_network_peer && + MaybePickPreferredNetwork(preferred_net)) { + // Full outbound connection management: Attempt to get at least one + // outbound peer from each reachable network by making extra connections + // and then protecting "only" peers from a network during outbound eviction. + // This is not attempted if the user changed -maxconnections to a value + // so low that less than MAX_OUTBOUND_FULL_RELAY_CONNECTIONS are made, + // to prevent interactions with otherwise protected outbound peers. + next_extra_network_peer = GetExponentialRand(now, EXTRA_NETWORK_PEER_INTERVAL); } else if (nOutboundOnionRelay < m_max_outbound_onion && IsReachable(Network::NET_ONION)) { onion_only = true; } else { @@ -3448,7 +3493,10 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe } } else { // Not a feeler - std::tie(addr, addr_last_try) = addrman.Select(); + // If preferred_net has a value set, pick an extra outbound + // peer from that network. The eviction logic in net_processing + // ensures that a peer from another network will be evicted. + std::tie(addr, addr_last_try) = addrman.Select(false, preferred_net); } auto dmn = mnList.GetMNByService(addr); @@ -3499,6 +3547,17 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe continue; } + // Do not make automatic outbound connections to addnode peers, to + // not use our limited outbound slots for them and to ensure + // addnode connections benefit from their intended protections. + if (AddedNodesContain(addr)) { + LogPrint(BCLog::NET, "Not making automatic %s%s connection to %s peer selected for manual (addnode) connection%s\n", + preferred_net.has_value() ? "network-specific " : "", + ConnectionTypeAsString(conn_type), GetNetworkName(addr.GetNetwork()), + fLogIPs ? strprintf(": %s", addr.ToStringAddrPort()) : ""); + continue; + } + addrConnect = addr; break; } @@ -3515,6 +3574,9 @@ void CConnman::ThreadOpenConnections(const std::vector connect, CDe LogPrint(BCLog::NET, "Making feeler connection\n"); } } + + if (preferred_net != std::nullopt) LogPrint(BCLog::NET, "Making network specific connection to %s on %s.\n", addrConnect.ToStringAddrPort(), GetNetworkName(preferred_net.value())); + // Record addrman failure attempts when node has at least 2 persistent outbound connections to peers with // different netgroups in ipv4/ipv6 networks + all peers in Tor/I2P/CJDNS networks. // Don't record addrman failure attempts when node is offline. This can be identified since all local @@ -3540,7 +3602,7 @@ std::vector CConnman::GetCurrentBlockRelayOnlyConns() const return ret; } -std::vector CConnman::GetAddedNodeInfo() const +std::vector CConnman::GetAddedNodeInfo(bool include_connected) const { std::vector ret; @@ -3575,6 +3637,9 @@ std::vector CConnman::GetAddedNodeInfo() const // strAddNode is an IP:port auto it = mapConnected.find(service); if (it != mapConnected.end()) { + if (!include_connected) { + continue; + } addedNode.resolvedAddress = service; addedNode.fConnected = true; addedNode.fInbound = it->second; @@ -3583,6 +3648,9 @@ std::vector CConnman::GetAddedNodeInfo() const // strAddNode is a name auto it = mapConnectedByName.find(addr.m_added_node); if (it != mapConnectedByName.end()) { + if (!include_connected) { + continue; + } addedNode.resolvedAddress = it->second.second; addedNode.fConnected = true; addedNode.fInbound = it->second.first; @@ -3601,21 +3669,19 @@ void CConnman::ThreadOpenAddedConnections() while (true) { CSemaphoreGrant grant(*semAddnode); - std::vector vInfo = GetAddedNodeInfo(); + std::vector vInfo = GetAddedNodeInfo(/*include_connected=*/false); bool tried = false; for (const AddedNodeInfo& info : vInfo) { - if (!info.fConnected) { - if (!grant) { - // If we've used up our semaphore and need a new one, let's not wait here since while we are waiting - // the addednodeinfo state might change. - break; - } - tried = true; - CAddress addr(CService(), NODE_NONE); - OpenNetworkConnection(addr, false, std::move(grant), info.m_params.m_added_node.c_str(), ConnectionType::MANUAL, info.m_params.m_use_v2transport); - if (!interruptNet.sleep_for(std::chrono::milliseconds(500))) return; - grant = CSemaphoreGrant(*semAddnode, /*fTry=*/true); + if (!grant) { + // If we've used up our semaphore and need a new one, let's not wait here since while we are waiting + // the addednodeinfo state might change. + break; } + tried = true; + CAddress addr(CService(), NODE_NONE); + OpenNetworkConnection(addr, false, std::move(grant), info.m_params.m_added_node.c_str(), ConnectionType::MANUAL, info.m_params.m_use_v2transport); + if (!interruptNet.sleep_for(std::chrono::milliseconds(500))) return; + grant = CSemaphoreGrant(*semAddnode, /*fTry=*/true); } // See if any reconnections are desired. PerformReconnections(); @@ -3877,6 +3943,9 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai { LOCK(m_nodes_mutex); m_nodes.push_back(pnode); + + // update connection count by network + if (pnode->IsManualOrFullOutboundConn()) ++m_network_conn_counts[pnode->addr.GetNetwork()]; } { if (m_edge_trig_events) { @@ -4499,9 +4568,12 @@ std::vector CConnman::GetAddresses(CNode& requestor, size_t max_addres bool CConnman::AddNode(const AddedNodeParams& add) { + const CService resolved(LookupNumeric(add.m_added_node, Params().GetDefaultPort(add.m_added_node))); + const bool resolved_is_valid{resolved.IsValid()}; + LOCK(m_added_nodes_mutex); for (const auto& it : m_added_node_params) { - if (add.m_added_node == it.m_added_node) return false; + if (add.m_added_node == it.m_added_node || (resolved_is_valid && resolved == LookupNumeric(it.m_added_node, Params().GetDefaultPort(it.m_added_node)))) return false; } m_added_node_params.push_back(add); @@ -4520,6 +4592,17 @@ bool CConnman::RemoveAddedNode(const std::string& strNode) return false; } +bool CConnman::AddedNodesContain(const CAddress& addr) const +{ + AssertLockNotHeld(m_added_nodes_mutex); + const std::string addr_str{addr.ToStringAddr()}; + const std::string addr_port_str{addr.ToStringAddrPort()}; + LOCK(m_added_nodes_mutex); + return (m_added_node_params.size() < 24 // bound the query to a reasonable limit + && std::any_of(m_added_node_params.cbegin(), m_added_node_params.cend(), + [&](const auto& p) { return p.m_added_node == addr_str || p.m_added_node == addr_port_str; })); +} + bool CConnman::AddPendingMasternode(const uint256& proTxHash) { LOCK(cs_vPendingMasternodes); diff --git a/src/net.h b/src/net.h index 1691f6f8d508f..bbeaf207263a5 100644 --- a/src/net.h +++ b/src/net.h @@ -889,6 +889,22 @@ class CNode return m_conn_type == ConnectionType::MANUAL; } + bool IsManualOrFullOutboundConn() const + { + switch (m_conn_type) { + case ConnectionType::INBOUND: + case ConnectionType::FEELER: + case ConnectionType::BLOCK_RELAY: + case ConnectionType::ADDR_FETCH: + return false; + case ConnectionType::OUTBOUND_FULL_RELAY: + case ConnectionType::MANUAL: + return true; + } // no default case, so the compiler can warn about missing cases + + assert(false); + } + bool IsBlockOnlyConn() const { return m_conn_type == ConnectionType::BLOCK_RELAY; } @@ -929,10 +945,8 @@ class CNode /** Whether this peer provides all services that we want. Used for eviction decisions */ std::atomic_bool m_has_all_wanted_services{false}; - /** Whether we should relay transactions to this peer (their version - * message did not include fRelay=false and this is not a block-relay-only - * connection). This only changes from false to true. It will never change - * back to false. Used only in inbound eviction logic. */ + /** Whether we should relay transactions to this peer. This only changes + * from false to true. It will never change back to false. */ std::atomic_bool m_relays_txs{false}; /** Whether this peer has loaded a bloom filter. Used only in inbound @@ -1283,6 +1297,9 @@ friend class CNode; EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex, !mutexMsgProc); bool CheckIncomingNonce(uint64_t nonce); + // alias for thread safety annotations only, not defined + RecursiveMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex); + struct CFullyConnectedOnly { bool operator() (const CNode* pnode) const { return NodeFullyConnected(pnode); @@ -1464,7 +1481,8 @@ friend class CNode; bool AddNode(const AddedNodeParams& add) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); bool RemoveAddedNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); - std::vector GetAddedNodeInfo() const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); + bool AddedNodesContain(const CAddress& addr) const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); + std::vector GetAddedNodeInfo(bool include_connected) const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); /** * Attempts to open a connection. Currently only used from tests. @@ -1536,6 +1554,8 @@ friend class CNode; /** Return true if we should disconnect the peer for failing an inactivity check. */ bool ShouldRunInactivityChecks(const CNode& node, std::chrono::seconds now) const; + bool MultipleManualOrFullOutboundConns(Network net) const EXCLUSIVE_LOCKS_REQUIRED(m_nodes_mutex); + /** * RAII helper to atomically create a copy of `m_nodes` and add a reference * to each of the nodes. The nodes are released when this object is destroyed. @@ -1736,6 +1756,18 @@ friend class CNode; */ std::vector GetCurrentBlockRelayOnlyConns() const; + /** + * Search for a "preferred" network, a reachable network to which we + * currently don't have any OUTBOUND_FULL_RELAY or MANUAL connections. + * There needs to be at least one address in AddrMan for a preferred + * network to be picked. + * + * @param[out] network Preferred network, if found. + * + * @return bool Whether a preferred network was found. + */ + bool MaybePickPreferredNetwork(std::optional& network); + // Whether the node should be passed out in ForEach* callbacks static bool NodeFullyConnected(const CNode* pnode); @@ -1778,6 +1810,9 @@ friend class CNode; std::atomic nLastNodeId{0}; unsigned int nPrevNodeCount{0}; + // Stores number of full-tx connections (outbound and manual) per network + std::array m_network_conn_counts GUARDED_BY(m_nodes_mutex) = {}; + std::vector vPendingMasternodes; mutable RecursiveMutex cs_vPendingMasternodes; std::map, std::set> masternodeQuorumNodes GUARDED_BY(cs_vPendingMasternodes); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b82a79e807b75..547f516b787d2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -304,10 +304,25 @@ struct Peer { std::chrono::microseconds m_next_inv_send_time GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0}; }; - // in bitcoin: m_tx_relay == nullptr if we're not relaying transactions with this peer - // in dash: m_tx_relay should never be nullptr, we don't relay transactions if - // `IsBlockOnlyConn() == true` is instead - std::unique_ptr m_tx_relay{std::make_unique()}; + /** + * (Bitcoin) Initializes a TxRelay struct for this peer. Can be called at most once for a peer. + * (Dash) Enables the flag that allows GetTxRelay() to return m_tx_relay */ + TxRelay* SetTxRelay() + { + Assume(!m_can_tx_relay); + m_can_tx_relay = true; + return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get()); + }; + + TxRelay* GetInvRelay() + { + return WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get()); + } + + TxRelay* GetTxRelay() + { + return m_can_tx_relay ? WITH_LOCK(m_tx_relay_mutex, return m_tx_relay.get()) : nullptr; + }; /** A vector of addresses to send to the peer, limited to MAX_ADDR_TO_SEND. */ std::vector m_addrs_to_send GUARDED_BY(NetEventsInterface::g_msgproc_mutex); @@ -336,8 +351,8 @@ struct Peer { * This field must correlate with whether m_addr_known has been * initialized.*/ std::atomic_bool m_addr_relay_enabled{false}; - /** Whether a Peer can only be relayed blocks */ - const bool m_block_relay_only{false}; + /** Whether a peer can relay transactions */ + bool m_can_tx_relay{false}; /** Whether a getaddr request to this peer is outstanding. */ bool m_getaddr_sent GUARDED_BY(NetEventsInterface::g_msgproc_mutex){false}; /** Guards address sending timers. */ @@ -375,12 +390,20 @@ struct Peer { /** Time of the last getheaders message to this peer */ std::atomic m_last_getheaders_timestamp GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0s}; - explicit Peer(NodeId id, ServiceFlags our_services, bool block_relay_only) + explicit Peer(NodeId id, ServiceFlags our_services) : m_id(id) , m_our_services{our_services} - , m_tx_relay(std::make_unique()) - , m_block_relay_only{block_relay_only} {} + +private: + Mutex m_tx_relay_mutex; + + /** Transaction relay data. + * (Bitcoin) Transaction relay data. May be a nullptr. + * (Dash) Always initialized but selectively available through GetTxRelay() + * (non-transaction relay should use GetInvRelay(), which will provide + * unconditional access) */ + std::unique_ptr m_tx_relay GUARDED_BY(m_tx_relay_mutex){std::make_unique()}; }; using PeerRef = std::shared_ptr; @@ -721,9 +744,11 @@ class PeerManagerImpl final : public PeerManager /** Next time to check for stale tip */ std::chrono::seconds m_stale_tip_check_time GUARDED_BY(cs_main){0s}; - /** Whether this node is running in blocks only mode */ + /** Whether this node is running in -blocksonly mode */ const bool m_ignore_incoming_txs; + bool RejectIncomingTxs(const CNode& peer) const; + /** Whether we've completed initial sync yet, for determining when to turn * on extra block-relay-only peers. */ bool m_initial_sync_finished GUARDED_BY(cs_main){false}; @@ -1033,11 +1058,11 @@ void PeerManagerImpl::PushAddress(Peer& peer, const CAddress& addr, FastRandomCo static void AddKnownInv(Peer& peer, const uint256& hash) { - // Dash always initializes m_tx_relay - assert(peer.m_tx_relay != nullptr); + auto inv_relay = peer.GetInvRelay(); + assert(inv_relay); - LOCK(peer.m_tx_relay->m_tx_inventory_mutex); - peer.m_tx_relay->m_tx_inventory_known_filter.insert(hash); + LOCK(inv_relay->m_tx_inventory_mutex); + inv_relay->m_tx_inventory_known_filter.insert(hash); } /** Whether this peer can serve us blocks. */ @@ -1071,8 +1096,8 @@ static uint16_t GetHeadersLimit(const CNode& pfrom, bool compressed) static void PushInv(Peer& peer, const CInv& inv) { - // Dash always initializes m_tx_relay - assert(peer.m_tx_relay != nullptr); + auto inv_relay = peer.GetInvRelay(); + assert(inv_relay); ASSERT_IF_DEBUG(inv.type != MSG_BLOCK); if (inv.type == MSG_BLOCK) { @@ -1080,17 +1105,17 @@ static void PushInv(Peer& peer, const CInv& inv) return; } - LOCK(peer.m_tx_relay->m_tx_inventory_mutex); - if (peer.m_tx_relay->m_tx_inventory_known_filter.contains(inv.hash)) { + LOCK(inv_relay->m_tx_inventory_mutex); + if (inv_relay->m_tx_inventory_known_filter.contains(inv.hash)) { LogPrint(BCLog::NET, "%s -- skipping known inv: %s peer=%d\n", __func__, inv.ToString(), peer.m_id); return; } LogPrint(BCLog::NET, "%s -- adding new inv: %s peer=%d\n", __func__, inv.ToString(), peer.m_id); if (inv.type == MSG_TX || inv.type == MSG_DSTX) { - peer.m_tx_relay->m_tx_inventory_to_send.insert(inv.hash); + inv_relay->m_tx_inventory_to_send.insert(inv.hash); return; } - peer.m_tx_relay->vInventoryOtherToSend.push_back(inv); + inv_relay->vInventoryOtherToSend.push_back(inv); } std::chrono::microseconds PeerManagerImpl::NextInvToInbounds(std::chrono::microseconds now, @@ -1175,7 +1200,7 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid) { AssertLockHeld(cs_main); - // Never request high-bandwidth mode from peers if we're blocks-only. Our + // When in -blocksonly mode, never request high-bandwidth mode from peers. Our // mempool will not contain the transactions necessary to reconstruct the // compact block. if (m_ignore_incoming_txs) return; @@ -1552,7 +1577,7 @@ void PeerManagerImpl::InitializeNode(CNode& node, ServiceFlags our_services) { LOCK(cs_main); m_node_states.emplace_hint(m_node_states.end(), std::piecewise_construct, std::forward_as_tuple(nodeid), std::forward_as_tuple(node.IsInboundConn())); } - PeerRef peer = std::make_shared(nodeid, our_services, /* block_relay_only = */ node.IsBlockOnlyConn()); + PeerRef peer = std::make_shared(nodeid, our_services); { LOCK(m_peer_mutex); m_peer_map.emplace_hint(m_peer_map.end(), nodeid, peer); @@ -1685,8 +1710,8 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats& stats) c ping_wait = GetTime() - peer->m_ping_start.load(); } - if (!peer->m_block_relay_only) { - stats.m_relay_txs = WITH_LOCK(peer->m_tx_relay->m_bloom_filter_mutex, return peer->m_tx_relay->m_relay_txs); + if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + stats.m_relay_txs = WITH_LOCK(tx_relay->m_bloom_filter_mutex, return tx_relay->m_relay_txs); } else { stats.m_relay_txs = false; } @@ -2211,10 +2236,11 @@ bool PeerManagerImpl::IsInvInFilter(NodeId nodeid, const uint256& hash) const PeerRef peer = GetPeerRef(nodeid); if (peer == nullptr) return false; - if (peer->m_block_relay_only) - return false; - LOCK(peer->m_tx_relay->m_tx_inventory_mutex); - return peer->m_tx_relay->m_tx_inventory_known_filter.contains(hash); + if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + LOCK(tx_relay->m_tx_inventory_mutex); + return tx_relay->m_tx_inventory_known_filter.contains(hash); + } + return false; } void PeerManagerImpl::PushInventory(NodeId nodeid, const CInv& inv) @@ -2243,21 +2269,23 @@ void PeerManagerImpl::RelayInvFiltered(CInv &inv, const CTransaction& relatedTx, { // TODO: Migrate to iteration through m_peer_map m_connman.ForEachNode([&](CNode* pnode) { - if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || pnode->IsBlockOnlyConn()) { + PeerRef peer = GetPeerRef(pnode->GetId()); + if (peer == nullptr) return; + + auto tx_relay = peer->GetTxRelay(); + if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || tx_relay == nullptr) { return; } - PeerRef peer = GetPeerRef(pnode->GetId()); - if (peer == nullptr) return; { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - if (!peer->m_tx_relay->m_relay_txs) { + LOCK(tx_relay->m_bloom_filter_mutex); + if (!tx_relay->m_relay_txs) { return; } - if (peer->m_tx_relay->m_bloom_filter && !peer->m_tx_relay->m_bloom_filter->IsRelevantAndUpdate(relatedTx)) { + if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->IsRelevantAndUpdate(relatedTx)) { return; } - } + } // LOCK(tx_relay->m_bloom_filter_mutex) PushInv(*peer, inv); }); } @@ -2266,21 +2294,23 @@ void PeerManagerImpl::RelayInvFiltered(CInv &inv, const uint256& relatedTxHash, { // TODO: Migrate to iteration through m_peer_map m_connman.ForEachNode([&](CNode* pnode) { - if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || pnode->IsBlockOnlyConn()) { + PeerRef peer = GetPeerRef(pnode->GetId()); + if (peer == nullptr) return; + + auto tx_relay = peer->GetTxRelay(); + if (pnode->nVersion < minProtoVersion || !pnode->CanRelay() || tx_relay == nullptr) { return; } - PeerRef peer = GetPeerRef(pnode->GetId()); - if (peer == nullptr) return; { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - if (!peer->m_tx_relay->m_relay_txs) { + LOCK(tx_relay->m_bloom_filter_mutex); + if (!tx_relay->m_relay_txs) { return; } - if (peer->m_tx_relay->m_bloom_filter && !peer->m_tx_relay->m_bloom_filter->contains(relatedTxHash)) { + if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->contains(relatedTxHash)) { return; } - } + } // LOCK(tx_relay->m_bloom_filter_mutex) PushInv(*peer, inv); }); } @@ -2290,7 +2320,8 @@ void PeerManagerImpl::RelayTransaction(const uint256& txid) LOCK(m_peer_mutex); for(auto& it : m_peer_map) { Peer& peer = *it.second; - if (!peer.m_tx_relay) continue; + auto tx_relay = peer.GetTxRelay(); + if (!tx_relay) continue; const CInv inv{m_cj_ctx->dstxman->GetDSTX(txid) ? MSG_DSTX : MSG_TX, txid}; PushInv(peer, inv); @@ -2424,11 +2455,11 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv& } else if (inv.IsMsgFilteredBlk()) { bool sendMerkleBlock = false; CMerkleBlock merkleBlock; - if (!pfrom.IsBlockOnlyConn()) { - LOCK(peer.m_tx_relay->m_bloom_filter_mutex); - if (peer.m_tx_relay->m_bloom_filter) { + if (auto tx_relay = peer.GetTxRelay(); tx_relay != nullptr) { + LOCK(tx_relay->m_bloom_filter_mutex); + if (tx_relay->m_bloom_filter) { sendMerkleBlock = true; - merkleBlock = CMerkleBlock(*pblock, *peer.m_tx_relay->m_bloom_filter); + merkleBlock = CMerkleBlock(*pblock, *tx_relay->m_bloom_filter); } } if (sendMerkleBlock) { @@ -2520,14 +2551,16 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic { AssertLockNotHeld(cs_main); + auto tx_relay = peer.GetTxRelay(); + std::deque::iterator it = peer.m_getdata_requests.begin(); std::vector vNotFound; const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); const std::chrono::seconds now = GetTime(); // Get last mempool request time - const std::chrono::seconds mempool_req = !pfrom.IsBlockOnlyConn() ? peer.m_tx_relay->m_last_mempool_req.load() - : std::chrono::seconds::min(); + const std::chrono::seconds mempool_req = tx_relay != nullptr ? tx_relay->m_last_mempool_req.load() + : std::chrono::seconds::min(); // Process as many TX items from the front of the getdata queue as // possible, since they're common and it's efficient to batch process @@ -2547,7 +2580,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic } ++it; - if (peer.m_block_relay_only && NetMessageViolatesBlocksOnly(inv.GetCommand())) { + if (tx_relay == nullptr && NetMessageViolatesBlocksOnly(inv.GetCommand())) { // Note that if we receive a getdata for non-block messages // from a block-relay-only outbound peer that violate the policy, // we skip such getdata messages from this peer @@ -2588,7 +2621,7 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic for (const uint256& parent_txid : parent_ids_to_add) { // Relaying a transaction with a recent but unconfirmed parent. - if (WITH_LOCK(peer.m_tx_relay->m_tx_inventory_mutex, return !peer.m_tx_relay->m_tx_inventory_known_filter.contains(parent_txid))) { + if (WITH_LOCK(tx_relay->m_tx_inventory_mutex, return !tx_relay->m_tx_inventory_known_filter.contains(parent_txid))) { LOCK(cs_main); State(pfrom.GetId())->m_recently_announced_invs.insert(parent_txid); } @@ -3486,10 +3519,18 @@ void PeerManagerImpl::ProcessMessage( } peer->m_starting_height = starting_height; - if (!pfrom.IsBlockOnlyConn()) { + // We only initialize the m_tx_relay data structure if: + // - this isn't an outbound block-relay-only connection; and + // - this isn't an outbound feeler connection, and + // - fRelay=true or we're offering NODE_BLOOM to this peer + // (NODE_BLOOM means that the peer may turn on tx relay later) + if (!pfrom.IsBlockOnlyConn() && + !pfrom.IsFeelerConn() && + (fRelay || (peer->m_our_services & NODE_BLOOM))) { + auto* const tx_relay = peer->SetTxRelay(); { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - peer->m_tx_relay->m_relay_txs = fRelay; // set to true after we get the first filter* message + LOCK(tx_relay->m_bloom_filter_mutex); + tx_relay->m_relay_txs = fRelay; // set to true after we get the first filter* message } if (fRelay) pfrom.m_relays_txs = true; } @@ -3590,8 +3631,6 @@ void PeerManagerImpl::ProcessMessage( // At this point, the outgoing message serialization version can't change. const CNetMsgMaker msgMaker(pfrom.GetCommonVersion()); - bool fBlocksOnly = pfrom.IsBlockRelayOnly(); - if (msg_type == NetMsgType::VERACK) { if (pfrom.fSuccessfullyConnected) { LogPrint(BCLog::NET, "ignoring redundant verack message from peer=%d\n", pfrom.GetId()); @@ -3627,7 +3666,7 @@ void PeerManagerImpl::ProcessMessage( m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION)); } - if (!fBlocksOnly) { + if (!RejectIncomingTxs(pfrom)) { // Tell our peer that he should send us CoinJoin queue messages m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDDSQUEUE, true)); // Tell our peer that he should send us intra-quorum messages @@ -3706,7 +3745,7 @@ void PeerManagerImpl::ProcessMessage( } // Stop processing non-block data early in blocks only mode and for block-relay-only peers - if (fBlocksOnly && NetMessageViolatesBlocksOnly(msg_type)) { + if (RejectIncomingTxs(pfrom) && NetMessageViolatesBlocksOnly(msg_type)) { LogPrint(BCLog::NET, "%s sent in violation of protocol peer=%d\n", msg_type, pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -3833,6 +3872,8 @@ void PeerManagerImpl::ProcessMessage( return; } + const bool reject_tx_invs{RejectIncomingTxs(pfrom)}; + LOCK(cs_main); const auto current_time = GetTime(); @@ -3862,7 +3903,7 @@ void PeerManagerImpl::ProcessMessage( best_block = &inv.hash; } } else { - if (fBlocksOnly && NetMessageViolatesBlocksOnly(inv.GetCommand())) { + if (reject_tx_invs && NetMessageViolatesBlocksOnly(inv.GetCommand())) { LogPrint(BCLog::NET, "%s (%s) inv sent in violation of protocol, disconnecting peer=%d\n", inv.GetCommand(), inv.hash.ToString(), pfrom.GetId()); pfrom.fDisconnect = true; return; @@ -3878,7 +3919,7 @@ void PeerManagerImpl::ProcessMessage( AddKnownInv(*peer, inv.hash); if (!fAlreadyHave) { - if (fBlocksOnly && inv.type == MSG_ISDLOCK) { + if (reject_tx_invs && inv.type == MSG_ISDLOCK) { if (pfrom.GetCommonVersion() <= ADDRV2_PROTO_VERSION) { // It's ok to receive these invs, we just ignore them // and do not request corresponding objects. @@ -4750,9 +4791,9 @@ void PeerManagerImpl::ProcessMessage( return; } - if (!pfrom.IsBlockOnlyConn()) { - LOCK(peer->m_tx_relay->m_tx_inventory_mutex); - peer->m_tx_relay->m_send_mempool = true; + if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + LOCK(tx_relay->m_tx_inventory_mutex); + tx_relay->m_send_mempool = true; } return; } @@ -4843,13 +4884,11 @@ void PeerManagerImpl::ProcessMessage( { // There is no excuse for sending a too-large filter Misbehaving(pfrom.GetId(), 100, "too-large bloom filter"); - } - else if (!pfrom.IsBlockOnlyConn()) - { + } else if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - peer->m_tx_relay->m_bloom_filter.reset(new CBloomFilter(filter)); - peer->m_tx_relay->m_relay_txs = true; + LOCK(tx_relay->m_bloom_filter_mutex); + tx_relay->m_bloom_filter.reset(new CBloomFilter(filter)); + tx_relay->m_relay_txs = true; } pfrom.m_bloom_filter_loaded = true; pfrom.m_relays_txs = true; @@ -4871,10 +4910,10 @@ void PeerManagerImpl::ProcessMessage( bool bad = false; if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) { bad = true; - } else if (!pfrom.IsBlockOnlyConn()) { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - if (peer->m_tx_relay->m_bloom_filter) { - peer->m_tx_relay->m_bloom_filter->insert(vData); + } else if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + LOCK(tx_relay->m_bloom_filter_mutex); + if (tx_relay->m_bloom_filter) { + tx_relay->m_bloom_filter->insert(vData); } else { bad = true; } @@ -4891,14 +4930,13 @@ void PeerManagerImpl::ProcessMessage( pfrom.fDisconnect = true; return; } - if (pfrom.IsBlockOnlyConn()) { - return; - } + auto tx_relay = peer->GetTxRelay(); + if (!tx_relay) return; { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - peer->m_tx_relay->m_bloom_filter = nullptr; - peer->m_tx_relay->m_relay_txs = true; + LOCK(tx_relay->m_bloom_filter_mutex); + tx_relay->m_bloom_filter = nullptr; + tx_relay->m_relay_txs = true; } pfrom.m_bloom_filter_loaded = false; pfrom.m_relays_txs = true; @@ -5273,13 +5311,16 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) // Pick the outbound-full-relay peer that least recently announced // us a new block, with ties broken by choosing the more recent // connection (higher node id) + // Protect peers from eviction if we don't have another connection + // to their network, counting both outbound-full-relay and manual peers. NodeId worst_peer = -1; int64_t oldest_block_announcement = std::numeric_limits::max(); // We want to prevent recently connected to Onion peers from being disconnected here, protect them as long as // there are more non_onion nodes than onion nodes so far size_t onion_count = 0; - m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) { + + m_connman.ForEachNode([&](CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_connman.GetNodesMutex()) { AssertLockHeld(::cs_main); if (pnode->addr.IsTor() && ++onion_count <= m_connman.GetMaxOutboundOnionNodeCount()) return; // Don't disconnect masternodes just because they were slow in block announcement @@ -5291,6 +5332,9 @@ void PeerManagerImpl::EvictExtraOutboundPeers(std::chrono::seconds now) if (state == nullptr) return; // shouldn't be possible, but just in case // Don't evict our protected peers if (state->m_chain_sync.m_protect) return; + // If this is the only connection on a particular network that is + // OUTBOUND_FULL_RELAY or MANUAL, protect it. + if (!m_connman.MultipleManualOrFullOutboundConns(pnode->addr.GetNetwork())) return; if (state->m_last_block_announcement < oldest_block_announcement || (state->m_last_block_announcement == oldest_block_announcement && pnode->GetId() > worst_peer)) { worst_peer = pnode->GetId(); oldest_block_announcement = state->m_last_block_announcement; @@ -5478,6 +5522,15 @@ class CompareInvMempoolOrder return mp->CompareDepthAndScore(*b, *a); } }; +} // namespace + +bool PeerManagerImpl::RejectIncomingTxs(const CNode& peer) const +{ + // block-relay-only peers may never send txs to us + if (peer.IsBlockOnlyConn()) return true; + // In -blocksonly mode, peers need the 'relay' permission to send txs to us + if (m_ignore_incoming_txs && !peer.HasPermission(NetPermissionFlags::Relay)) return true; + return false; } bool PeerManagerImpl::SetupAddressRelay(const CNode& node, Peer& peer) @@ -5759,9 +5812,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto) LOCK(peer->m_block_inv_mutex); size_t reserve = INVENTORY_BROADCAST_MAX_PER_1MB_BLOCK * MaxBlockSize() / 1000000; - if (!pto->IsBlockOnlyConn()) { - LOCK(peer->m_tx_relay->m_tx_inventory_mutex); - reserve = std::min(peer->m_tx_relay->m_tx_inventory_to_send.size(), reserve); + if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + LOCK(tx_relay->m_tx_inventory_mutex); + reserve = std::min(tx_relay->m_tx_inventory_to_send.size(), reserve); } reserve = std::max(reserve, peer->m_blocks_for_inv_relay.size()); reserve = std::min(reserve, MAX_INV_SZ); @@ -5778,9 +5831,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) peer->m_blocks_for_inv_relay.clear(); } - auto queueAndMaybePushInv = [this, pto, peer, &vInv, &msgMaker](const CInv& invIn) EXCLUSIVE_LOCKS_REQUIRED(peer->m_tx_relay->m_tx_inventory_mutex) { - AssertLockHeld(peer->m_tx_relay->m_tx_inventory_mutex); - peer->m_tx_relay->m_tx_inventory_known_filter.insert(invIn.hash); + auto queueAndMaybePushInv = [this, pto, peer, &vInv, &msgMaker](const CInv& invIn) { LogPrint(BCLog::NET, "SendMessages -- queued inv: %s index=%d peer=%d\n", invIn.ToString(), vInv.size(), pto->GetId()); // Responses to MEMPOOL requests bypass the m_recently_announced_invs filter. vInv.push_back(invIn); @@ -5791,19 +5842,19 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } }; - if (!pto->IsBlockOnlyConn()) { - LOCK(peer->m_tx_relay->m_tx_inventory_mutex); + if (auto tx_relay = peer->GetTxRelay(); tx_relay != nullptr) { + LOCK(tx_relay->m_tx_inventory_mutex); // Check whether periodic sends should happen // Note: If this node is running in a Masternode mode, it makes no sense to delay outgoing txes // because we never produce any txes ourselves i.e. no privacy is lost in this case. bool fSendTrickle = pto->HasPermission(NetPermissionFlags::NoBan) || is_masternode; - if (peer->m_tx_relay->m_next_inv_send_time < current_time) { + if (tx_relay->m_next_inv_send_time < current_time) { fSendTrickle = true; if (pto->IsInboundConn()) { - peer->m_tx_relay->m_next_inv_send_time = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL); + tx_relay->m_next_inv_send_time = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL); } else { // Use half the delay for Masternode outbound peers, as there is less privacy concern for them. - peer->m_tx_relay->m_next_inv_send_time = pto->GetVerifiedProRegTxHash().IsNull() ? + tx_relay->m_next_inv_send_time = pto->GetVerifiedProRegTxHash().IsNull() ? GetExponentialRand(current_time, OUTBOUND_INVENTORY_BROADCAST_INTERVAL) : GetExponentialRand(current_time, OUTBOUND_INVENTORY_BROADCAST_INTERVAL / 2); } @@ -5811,49 +5862,53 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // Time to send but the peer has requested we not relay transactions. if (fSendTrickle) { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); - if (!peer->m_tx_relay->m_relay_txs) peer->m_tx_relay->m_tx_inventory_to_send.clear(); + LOCK(tx_relay->m_bloom_filter_mutex); + if (!tx_relay->m_relay_txs) tx_relay->m_tx_inventory_to_send.clear(); } // Respond to BIP35 mempool requests - if (fSendTrickle && peer->m_tx_relay->m_send_mempool) { + if (fSendTrickle && tx_relay->m_send_mempool) { auto vtxinfo = m_mempool.infoAll(); - peer->m_tx_relay->m_send_mempool = false; + tx_relay->m_send_mempool = false; - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); + LOCK(tx_relay->m_bloom_filter_mutex); // Send invs for txes and corresponding IS-locks for (const auto& txinfo : vtxinfo) { const uint256& hash = txinfo.tx->GetHash(); - peer->m_tx_relay->m_tx_inventory_to_send.erase(hash); - if (peer->m_tx_relay->m_bloom_filter && !peer->m_tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; + tx_relay->m_tx_inventory_to_send.erase(hash); + if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; int nInvType = m_cj_ctx->dstxman->GetDSTX(hash) ? MSG_DSTX : MSG_TX; + tx_relay->m_tx_inventory_known_filter.insert(hash); queueAndMaybePushInv(CInv(nInvType, hash)); const auto islock = m_llmq_ctx->isman->GetInstantSendLockByTxid(hash); if (islock == nullptr) continue; if (pto->nVersion < ISDLOCK_PROTO_VERSION) continue; - queueAndMaybePushInv(CInv(MSG_ISDLOCK, ::SerializeHash(*islock))); + uint256 isLockHash{::SerializeHash(*islock)}; + tx_relay->m_tx_inventory_known_filter.insert(isLockHash); + queueAndMaybePushInv(CInv(MSG_ISDLOCK, isLockHash)); } // Send an inv for the best ChainLock we have const auto& clsig = m_llmq_ctx->clhandler->GetBestChainLock(); if (!clsig.IsNull()) { - uint256 chainlockHash = ::SerializeHash(clsig); + uint256 chainlockHash{::SerializeHash(clsig)}; + tx_relay->m_tx_inventory_known_filter.insert(chainlockHash); queueAndMaybePushInv(CInv(MSG_CLSIG, chainlockHash)); } - peer->m_tx_relay->m_last_mempool_req = std::chrono::duration_cast(current_time); + tx_relay->m_last_mempool_req = std::chrono::duration_cast(current_time); } // Determine transactions to relay if (fSendTrickle) { - LOCK(peer->m_tx_relay->m_bloom_filter_mutex); + LOCK(tx_relay->m_bloom_filter_mutex); // Produce a vector with all candidates for sending std::vector::iterator> vInvTx; - vInvTx.reserve(peer->m_tx_relay->m_tx_inventory_to_send.size()); - for (std::set::iterator it = peer->m_tx_relay->m_tx_inventory_to_send.begin(); it != peer->m_tx_relay->m_tx_inventory_to_send.end(); it++) { + vInvTx.reserve(tx_relay->m_tx_inventory_to_send.size()); + for (std::set::iterator it = tx_relay->m_tx_inventory_to_send.begin(); it != tx_relay->m_tx_inventory_to_send.end(); it++) { vInvTx.push_back(it); } // Topologically and fee-rate sort the inventory we send for privacy and priority reasons. @@ -5863,7 +5918,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // No reason to drain out at many times the network's capacity, // especially since we have many peers and some will draw much shorter delays. unsigned int nRelayedTransactions = 0; - size_t broadcast_max{INVENTORY_BROADCAST_MAX_PER_1MB_BLOCK * MaxBlockSize() / 1000000 + (peer->m_tx_relay->m_tx_inventory_to_send.size()/1000)*5}; + size_t broadcast_max{INVENTORY_BROADCAST_MAX_PER_1MB_BLOCK * MaxBlockSize() / 1000000 + (tx_relay->m_tx_inventory_to_send.size()/1000)*5}; broadcast_max = std::min(1000, broadcast_max); while (!vInvTx.empty() && nRelayedTransactions < broadcast_max) { @@ -5873,9 +5928,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto) vInvTx.pop_back(); uint256 hash = *it; // Remove it from the to-be-sent set - peer->m_tx_relay->m_tx_inventory_to_send.erase(it); + tx_relay->m_tx_inventory_to_send.erase(it); // Check if not in the filter already - if (peer->m_tx_relay->m_tx_inventory_known_filter.contains(hash)) { + if (tx_relay->m_tx_inventory_known_filter.contains(hash)) { continue; } // Not in the mempool anymore? don't bother sending it. @@ -5883,7 +5938,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) if (!txinfo.tx) { continue; } - if (peer->m_tx_relay->m_bloom_filter && !peer->m_tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; + if (tx_relay->m_bloom_filter && !tx_relay->m_bloom_filter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send State(pto->GetId())->m_recently_announced_invs.insert(hash); nRelayedTransactions++; @@ -5901,29 +5956,33 @@ bool PeerManagerImpl::SendMessages(CNode* pto) } } int nInvType = m_cj_ctx->dstxman->GetDSTX(hash) ? MSG_DSTX : MSG_TX; + tx_relay->m_tx_inventory_known_filter.insert(hash); queueAndMaybePushInv(CInv(nInvType, hash)); } } } { + auto inv_relay = peer->GetInvRelay(); + // Send non-tx/non-block inventory items - LOCK2(peer->m_tx_relay->m_tx_inventory_mutex, peer->m_tx_relay->m_bloom_filter_mutex); + LOCK2(inv_relay->m_tx_inventory_mutex, inv_relay->m_bloom_filter_mutex); - bool fSendIS = peer->m_tx_relay->m_relay_txs && !pto->IsBlockRelayOnly(); + bool fSendIS = inv_relay->m_relay_txs && !pto->IsBlockRelayOnly(); - for (const auto& inv : peer->m_tx_relay->vInventoryOtherToSend) { - if (!peer->m_tx_relay->m_relay_txs && NetMessageViolatesBlocksOnly(inv.GetCommand())) { + for (const auto& inv : inv_relay->vInventoryOtherToSend) { + if (!inv_relay->m_relay_txs && NetMessageViolatesBlocksOnly(inv.GetCommand())) { continue; } - if (peer->m_tx_relay->m_tx_inventory_known_filter.contains(inv.hash)) { + if (inv_relay->m_tx_inventory_known_filter.contains(inv.hash)) { continue; } if (!fSendIS && inv.type == MSG_ISDLOCK) { continue; } + inv_relay->m_tx_inventory_known_filter.insert(inv.hash); queueAndMaybePushInv(inv); } - peer->m_tx_relay->vInventoryOtherToSend.clear(); + inv_relay->vInventoryOtherToSend.clear(); } if (!vInv.empty()) m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 444b356b4d8a0..6466aec6fa2c0 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -598,7 +598,7 @@ static std::string IPv6ToString(Span a, uint32_t scope_id) return r; } -static std::string OnionToString(Span addr) +std::string OnionToString(Span addr) { uint8_t checksum[torv3::CHECKSUM_LEN]; torv3::Checksum(addr, checksum); diff --git a/src/netaddress.h b/src/netaddress.h index 5f540b480cf5f..102de00225a8e 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -113,6 +113,8 @@ static constexpr size_t ADDR_INTERNAL_SIZE = 10; /// SAM 3.1 and earlier do not support specifying ports and force the port to 0. static constexpr uint16_t I2P_SAM31_PORT{0}; +std::string OnionToString(Span addr); + /** * Network address. */ diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 491c0af3912fb..ecb1441ee6d7b 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -498,7 +498,7 @@ static RPCHelpMan getaddednodeinfo() const NodeContext& node = EnsureAnyNodeContext(request.context); const CConnman& connman = EnsureConnman(node);; - std::vector vInfo = connman.GetAddedNodeInfo(); + std::vector vInfo = connman.GetAddedNodeInfo(/*include_connected=*/true); if (!request.params[0].isNull()) { bool found = false; diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 9cebbf5b8c717..def090aa63d87 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -111,9 +111,19 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction) peerman.FinalizeNode(dummyNode1); } -static void AddRandomOutboundPeer(NodeId& id, std::vector& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType) +static void AddRandomOutboundPeer(NodeId& id, std::vector& vNodes, PeerManager& peerLogic, ConnmanTestMsg& connman, ConnectionType connType, bool onion_peer = false) { - CAddress addr(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); + CAddress addr; + + if (onion_peer) { + auto tor_addr{g_insecure_rand_ctx.randbytes(ADDR_TORV3_SIZE)}; + BOOST_REQUIRE(addr.SetSpecial(OnionToString(tor_addr))); + } + + while (!addr.IsRoutable()) { + addr = CAddress(ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE); + } + vNodes.emplace_back(new CNode{id++, /*sock=*/nullptr, addr, @@ -205,6 +215,30 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) BOOST_CHECK(vNodes[max_outbound_full_relay-1]->fDisconnect == true); BOOST_CHECK(vNodes.back()->fDisconnect == false); + vNodes[max_outbound_full_relay - 1]->fDisconnect = false; + + // Add an onion peer, that will be protected because it is the only one for + // its network, so another peer gets disconnected instead. + SetMockTime(time_init); + AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::OUTBOUND_FULL_RELAY, /*onion_peer=*/true); + SetMockTime(time_later); + peerLogic->CheckForStaleTipAndEvictPeers(); + + for (int i = 0; i < max_outbound_full_relay - 2; ++i) { + BOOST_CHECK(vNodes[i]->fDisconnect == false); + } + BOOST_CHECK(vNodes[max_outbound_full_relay - 2]->fDisconnect == false); + BOOST_CHECK(vNodes[max_outbound_full_relay - 1]->fDisconnect == true); + BOOST_CHECK(vNodes[max_outbound_full_relay]->fDisconnect == false); + + // Add a second onion peer which won't be protected + SetMockTime(time_init); + AddRandomOutboundPeer(id, vNodes, *peerLogic, *connman, ConnectionType::OUTBOUND_FULL_RELAY, /*onion_peer=*/true); + SetMockTime(time_later); + peerLogic->CheckForStaleTipAndEvictPeers(); + + BOOST_CHECK(vNodes.back()->fDisconnect == true); + for (const CNode *node : vNodes) { peerLogic->FinalizeNode(*node); } diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 1f10effec3bfe..0de69455c7a97 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/fuzz/banman.cpp b/src/test/fuzz/banman.cpp index 8c613d93ab96f..7c8ebfff0602c 100644 --- a/src/test/fuzz/banman.cpp +++ b/src/test/fuzz/banman.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 2154c9e6b7b4f..6fa704323f47d 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -123,7 +124,7 @@ FUZZ_TARGET_INIT(connman, initialize_connman) connman.SetTryNewOutboundPeer(fuzzed_data_provider.ConsumeBool()); }); } - (void)connman.GetAddedNodeInfo(); + (void)connman.GetAddedNodeInfo(fuzzed_data_provider.ConsumeBool()); (void)connman.GetExtraFullOutboundCount(); (void)connman.GetLocalServices(); assert(connman.GetMaxOutboundTarget() == max_outbound_limit); diff --git a/src/test/fuzz/netaddress.cpp b/src/test/fuzz/netaddress.cpp index 7348507b15804..61279c32ec825 100644 --- a/src/test/fuzz/netaddress.cpp +++ b/src/test/fuzz/netaddress.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #include diff --git a/src/test/fuzz/netbase_dns_lookup.cpp b/src/test/fuzz/netbase_dns_lookup.cpp index 331d0bd811ab2..9403472196fe4 100644 --- a/src/test/fuzz/netbase_dns_lookup.cpp +++ b/src/test/fuzz/netbase_dns_lookup.cpp @@ -6,7 +6,7 @@ #include #include #include -#include +#include #include #include diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index fb2f9c05fe9be..848040bb6f7f9 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -389,28 +390,6 @@ bool ContainsSpentInput(const CTransaction& tx, const CCoinsViewCache& inputs) n return false; } -CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept -{ - const Network network = fuzzed_data_provider.PickValueInArray({Network::NET_IPV4, Network::NET_IPV6, Network::NET_INTERNAL, Network::NET_ONION}); - CNetAddr net_addr; - if (network == Network::NET_IPV4) { - in_addr v4_addr = {}; - v4_addr.s_addr = fuzzed_data_provider.ConsumeIntegral(); - net_addr = CNetAddr{v4_addr}; - } else if (network == Network::NET_IPV6) { - if (fuzzed_data_provider.remaining_bytes() >= 16) { - in6_addr v6_addr = {}; - memcpy(v6_addr.s6_addr, fuzzed_data_provider.ConsumeBytes(16).data(), 16); - net_addr = CNetAddr{v6_addr, fuzzed_data_provider.ConsumeIntegral()}; - } - } else if (network == Network::NET_INTERNAL) { - net_addr.SetInternal(fuzzed_data_provider.ConsumeBytesAsString(32)); - } else if (network == Network::NET_ONION) { - net_addr.SetSpecial(fuzzed_data_provider.ConsumeBytesAsString(32)); - } - return net_addr; -} - FILE* FuzzedFileProvider::open() { SetFuzzedErrNo(m_fuzzed_data_provider); diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 521164bad49e6..00c004c8df9f9 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -296,8 +297,6 @@ template return random_bytes; } -CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept; - inline CSubNet ConsumeSubNet(FuzzedDataProvider& fuzzed_data_provider) noexcept { return {ConsumeNetAddr(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral()}; diff --git a/src/test/fuzz/util/net.cpp b/src/test/fuzz/util/net.cpp new file mode 100644 index 0000000000000..a1494f8d3bdcd --- /dev/null +++ b/src/test/fuzz/util/net.cpp @@ -0,0 +1,36 @@ +// Copyright (c) 2009-2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include + +#include +#include + +CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept +{ + const Network network = fuzzed_data_provider.PickValueInArray({Network::NET_IPV4, Network::NET_IPV6, Network::NET_INTERNAL, Network::NET_ONION}); + CNetAddr net_addr; + if (network == Network::NET_IPV4) { + in_addr v4_addr = {}; + v4_addr.s_addr = fuzzed_data_provider.ConsumeIntegral(); + net_addr = CNetAddr{v4_addr}; + } else if (network == Network::NET_IPV6) { + if (fuzzed_data_provider.remaining_bytes() >= 16) { + in6_addr v6_addr = {}; + memcpy(v6_addr.s6_addr, fuzzed_data_provider.ConsumeBytes(16).data(), 16); + net_addr = CNetAddr{v6_addr, fuzzed_data_provider.ConsumeIntegral()}; + } + } else if (network == Network::NET_INTERNAL) { + net_addr.SetInternal(fuzzed_data_provider.ConsumeBytesAsString(32)); + } else if (network == Network::NET_ONION) { + auto pub_key{fuzzed_data_provider.ConsumeBytes(ADDR_TORV3_SIZE)}; + pub_key.resize(ADDR_TORV3_SIZE); + const bool ok{net_addr.SetSpecial(OnionToString(pub_key))}; + assert(ok); + } + return net_addr; +} diff --git a/src/test/fuzz/util/net.h b/src/test/fuzz/util/net.h new file mode 100644 index 0000000000000..d81adab650894 --- /dev/null +++ b/src/test/fuzz/util/net.h @@ -0,0 +1,14 @@ +// Copyright (c) 2009-2021 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_TEST_FUZZ_UTIL_NET_H +#define BITCOIN_TEST_FUZZ_UTIL_NET_H + +#include + +class FuzzedDataProvider; + +CNetAddr ConsumeNetAddr(FuzzedDataProvider& fuzzed_data_provider) noexcept; + +#endif // BITCOIN_TEST_FUZZ_UTIL_NET_H diff --git a/src/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp new file mode 100644 index 0000000000000..87a190e88c66e --- /dev/null +++ b/src/test/net_peer_connection_tests.cpp @@ -0,0 +1,156 @@ +// Copyright (c) 2023-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include + +struct LogIPsTestingSetup : public TestingSetup { + LogIPsTestingSetup() + : TestingSetup{CBaseChainParams::MAIN, /*extra_args=*/{"-logips"}} {} +}; + +BOOST_FIXTURE_TEST_SUITE(net_peer_connection_tests, LogIPsTestingSetup) + +static CService ip(uint32_t i) +{ + struct in_addr s; + s.s_addr = i; + return CService{CNetAddr{s}, Params().GetDefaultPort()}; +} + +/** Create a peer and connect to it. If the optional `address` (IP/CJDNS only) isn't passed, a random address is created. */ +static void AddPeer(NodeId& id, std::vector& nodes, PeerManager& peerman, ConnmanTestMsg& connman, ConnectionType conn_type, bool onion_peer = false, std::optional address = std::nullopt) +{ + CAddress addr{}; + + if (address.has_value()) { + addr = CAddress{MaybeFlipIPv6toCJDNS(LookupNumeric(address.value(), Params().GetDefaultPort())), NODE_NONE}; + } else if (onion_peer) { + auto tor_addr{g_insecure_rand_ctx.randbytes(ADDR_TORV3_SIZE)}; + BOOST_REQUIRE(addr.SetSpecial(OnionToString(tor_addr))); + } + + while (!addr.IsLocal() && !addr.IsRoutable()) { + addr = CAddress{ip(g_insecure_rand_ctx.randbits(32)), NODE_NONE}; + } + + BOOST_REQUIRE(addr.IsValid()); + + const bool inbound_onion{onion_peer && conn_type == ConnectionType::INBOUND}; + + nodes.emplace_back(new CNode{++id, + /*sock=*/nullptr, + addr, + /*nKeyedNetGroupIn=*/0, + /*nLocalHostNonceIn=*/0, + CAddress{}, + /*addrNameIn=*/"", + conn_type, + /*inbound_onion=*/inbound_onion}); + CNode& node = *nodes.back(); + node.SetCommonVersion(PROTOCOL_VERSION); + + peerman.InitializeNode(node, ServiceFlags(NODE_NETWORK)); + node.fSuccessfullyConnected = true; + + connman.AddTestNode(node); +} + +BOOST_AUTO_TEST_CASE(test_addnode_getaddednodeinfo_and_connection_detection) +{ + const auto& chainparams = Params(); + auto connman = std::make_unique(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); + auto peerman = PeerManager::make(chainparams, *connman, *m_node.addrman, nullptr, + *m_node.chainman, *m_node.mempool, *m_node.mn_metaman, *m_node.mn_sync, + *m_node.govman, *m_node.sporkman, /* mn_activeman = */ nullptr, m_node.dmnman, + m_node.cj_ctx, m_node.llmq_ctx, /* ignore_incoming_txs = */ false); + NodeId id{0}; + std::vector nodes; + + // Connect a localhost peer. + { + ASSERT_DEBUG_LOG("Added connection to 127.0.0.1:9999 peer=1"); + AddPeer(id, nodes, *peerman, *connman, ConnectionType::MANUAL, /*onion_peer=*/false, /*address=*/"127.0.0.1"); + BOOST_REQUIRE(nodes.back() != nullptr); + } + + // Call ConnectNode(), which is also called by RPC addnode onetry, for a localhost + // address that resolves to multiple IPs, including that of the connected peer. + // The connection attempt should consistently fail due to the check in ConnectNode(). + for (int i = 0; i < 10; ++i) { + ASSERT_DEBUG_LOG("Not opening a connection to localhost, already connected to 127.0.0.1:9999"); + BOOST_CHECK(!connman->ConnectNodePublic(*peerman, "localhost", ConnectionType::MANUAL)); + } + + // Add 3 more peer connections. + AddPeer(id, nodes, *peerman, *connman, ConnectionType::OUTBOUND_FULL_RELAY); + AddPeer(id, nodes, *peerman, *connman, ConnectionType::BLOCK_RELAY, /*onion_peer=*/true); + AddPeer(id, nodes, *peerman, *connman, ConnectionType::INBOUND); + + BOOST_TEST_MESSAGE("Call AddNode() for all the peers"); + for (auto node : connman->TestNodes()) { + BOOST_CHECK(connman->AddNode({/*m_added_node=*/node->addr.ToStringAddrPort(), /*m_use_v2transport=*/true})); + BOOST_TEST_MESSAGE(strprintf("peer id=%s addr=%s", node->GetId(), node->addr.ToStringAddrPort())); + } + + BOOST_TEST_MESSAGE("\nCall AddNode() with 2 addrs resolving to existing localhost addnode entry; neither should be added"); + BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"127.0.0.1", /*m_use_v2transport=*/true})); + BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"127.1", /*m_use_v2transport=*/true})); + + BOOST_TEST_MESSAGE("\nExpect GetAddedNodeInfo to return expected number of peers with `include_connected` true/false"); + BOOST_CHECK_EQUAL(connman->GetAddedNodeInfo(/*include_connected=*/true).size(), nodes.size()); + BOOST_CHECK(connman->GetAddedNodeInfo(/*include_connected=*/false).empty()); + + // Test AddedNodesContain() + for (auto node : connman->TestNodes()) { + BOOST_CHECK(connman->AddedNodesContain(node->addr)); + } + AddPeer(id, nodes, *peerman, *connman, ConnectionType::OUTBOUND_FULL_RELAY); + BOOST_CHECK(!connman->AddedNodesContain(nodes.back()->addr)); + + BOOST_TEST_MESSAGE("\nPrint GetAddedNodeInfo contents:"); + for (const auto& info : connman->GetAddedNodeInfo(/*include_connected=*/true)) { + BOOST_TEST_MESSAGE(strprintf("\nadded node: %s", info.m_params.m_added_node)); + BOOST_TEST_MESSAGE(strprintf("connected: %s", info.fConnected)); + if (info.fConnected) { + BOOST_TEST_MESSAGE(strprintf("IP address: %s", info.resolvedAddress.ToStringAddrPort())); + BOOST_TEST_MESSAGE(strprintf("direction: %s", info.fInbound ? "inbound" : "outbound")); + } + } + + BOOST_TEST_MESSAGE("\nCheck that all connected peers are correctly detected as connected"); + for (auto node : connman->TestNodes()) { + BOOST_CHECK(connman->AlreadyConnectedPublic(node->addr)); + } + + // Clean up + for (auto node : connman->TestNodes()) { + peerman->FinalizeNode(*node); + } + connman->ClearTestNodes(); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/net.cpp b/src/test/util/net.cpp index ee29613fa04ed..de8eac2523365 100644 --- a/src/test/util/net.cpp +++ b/src/test/util/net.cpp @@ -4,11 +4,15 @@ #include -#include -#include #include #include +#include #include +#include +#include +#include +#include +#include #include #include @@ -98,6 +102,17 @@ bool ConnmanTestMsg::ReceiveMsgFrom(CNode& node, CSerializedNetMsg&& ser_msg) co return complete; } +CNode* ConnmanTestMsg::ConnectNodePublic(PeerManager& peerman, const char* pszDest, ConnectionType conn_type) +{ + CNode* node = ConnectNode(CAddress{}, pszDest, /*fCountFailure=*/false, conn_type, /*use_v2transport=*/true); + if (!node) return nullptr; + node->SetCommonVersion(PROTOCOL_VERSION); + peerman.InitializeNode(*node, ServiceFlags(NODE_NETWORK)); + node->fSuccessfullyConnected = true; + AddTestNode(*node); + return node; +} + std::vector GetRandomNodeEvictionCandidates(int n_candidates, FastRandomContext& random_context) { std::vector candidates; diff --git a/src/test/util/net.h b/src/test/util/net.h index 933309b6983da..4c2be00e49dba 100644 --- a/src/test/util/net.h +++ b/src/test/util/net.h @@ -6,16 +6,30 @@ #define BITCOIN_TEST_UTIL_NET_H #include -#include -#include #include +#include +#include +#include +#include +#include +#include #include +#include #include #include +#include +#include #include #include #include +#include +#include + +class FastRandomContext; + +template +class Span; struct ConnmanTestMsg : public CConnman { using CConnman::CConnman; @@ -25,11 +39,20 @@ struct ConnmanTestMsg : public CConnman { m_peer_connect_timeout = timeout; } + std::vector TestNodes() + { + LOCK(m_nodes_mutex); + return m_nodes; + } + void AddTestNode(CNode& node) { LOCK(m_nodes_mutex); m_nodes.push_back(&node); + + if (node.IsManualOrFullOutboundConn()) ++m_network_conn_counts[node.addr.GetNetwork()]; } + void ClearTestNodes() { LOCK(m_nodes_mutex); @@ -53,6 +76,11 @@ struct ConnmanTestMsg : public CConnman { bool ReceiveMsgFrom(CNode& node, CSerializedNetMsg&& ser_msg) const; void FlushSendBuffer(CNode& node) const; + + bool AlreadyConnectedPublic(const CAddress& addr) { return AlreadyConnectedToAddress(addr); }; + + CNode* ConnectNodePublic(PeerManager& peerman, const char* pszDest, ConnectionType conn_type) + EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); }; constexpr ServiceFlags ALL_SERVICE_FLAGS[]{ diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py index 828079c5b9a47..eaa83aaf75b79 100755 --- a/test/functional/p2p_blocksonly.py +++ b/test/functional/p2p_blocksonly.py @@ -90,6 +90,11 @@ def blocks_relay_conn_tests(self): assert_equal(self.nodes[0].getpeerinfo()[0]['relaytxes'], False) _, txid, tx_hex = self.check_p2p_tx_violation() + self.log.info("Tests with node in normal mode with block-relay-only connection, sending an inv") + conn = self.nodes[0].add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, connection_type="block-relay-only") + assert_equal(self.nodes[0].getpeerinfo()[0]['relaytxes'], False) + self.check_p2p_inv_violation(conn) + self.log.info("Check that txs from RPC are not sent to blockrelay connection") conn = self.nodes[0].add_outbound_p2p_connection(P2PTxInvStore(), p2p_idx=1, connection_type="block-relay-only") @@ -101,6 +106,13 @@ def blocks_relay_conn_tests(self): conn.sync_send_with_ping() assert(int(txid, 16) not in conn.get_invs()) + def check_p2p_inv_violation(self, peer): + self.log.info("Check that tx-invs from P2P are rejected and result in disconnect") + with self.nodes[0].assert_debug_log(["inv sent in violation of protocol, disconnecting peer"]): + peer.send_message(msg_inv([CInv(t=MSG_TX, h=0x12345)])) + peer.wait_for_disconnect() + self.nodes[0].disconnect_p2ps() + def check_p2p_tx_violation(self): self.log.info('Check that txs from P2P are rejected and result in disconnect') spendtx = self.miniwallet.create_self_transfer(from_node=self.nodes[0]) @@ -109,9 +121,7 @@ def check_p2p_tx_violation(self): self.nodes[0].p2ps[0].send_message(msg_tx(spendtx['tx'])) self.nodes[0].p2ps[0].wait_for_disconnect() assert_equal(self.nodes[0].getmempoolinfo()['size'], 0) - - # Remove the disconnected peer - del self.nodes[0].p2ps[0] + self.nodes[0].disconnect_p2ps() return spendtx['tx'], spendtx['txid'], spendtx['hex'] diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 5cc1dc617b05d..47787b633a85f 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -239,8 +239,11 @@ def test_getaddednodeinfo(self): # add a node (node2) to node0 ip_port = "127.0.0.1:{}".format(p2p_port(2)) self.nodes[0].addnode(node=ip_port, command='add') + # try to add an equivalent ip + ip_port2 = "127.1:{}".format(p2p_port(2)) + assert_raises_rpc_error(-23, "Node already added", self.nodes[0].addnode, node=ip_port2, command='add') # check that the node has indeed been added - added_nodes = self.nodes[0].getaddednodeinfo(ip_port) + added_nodes = self.nodes[0].getaddednodeinfo() assert_equal(len(added_nodes), 1) assert_equal(added_nodes[0]['addednode'], ip_port) # check that node cannot be added again