From 1b76e0473647d0df5c32ffe1bcdcbbcadc5d0dfa Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 4 Apr 2023 16:14:27 -0400 Subject: [PATCH] net: increase inbound capacity for block-relay-only connections ..and adjust the eviction logic. The new default max connection number is 200, the default maximum of tx-relaying inbounds is limited to 50% of all inbound connections. With 11 outbound connections, that is (200 - 11) * 0.5 = 94.5. As a result, the tx-related maximum traffic should not change drastically. When we receive an inbound connection and don't have space for another full-relay peer, we now attempt to evict specifically a full-relay inbound after receiving the version message of the new peer. Once this commit is widely deployed, the added inbound capacity will allow us to increase the number of outgoing block-relay-only connections. Co-authored-by: Amiti Uttarwar --- src/init.cpp | 6 ++++- src/net.cpp | 17 ++++++++++++++ src/net.h | 18 +++++++++++++-- src/net_processing.cpp | 27 +++++++++++++++++++++++ test/functional/interface_usdt_net.py | 4 ++-- test/functional/p2p_eviction.py | 8 ++++--- test/functional/p2p_opportunistic_1p1c.py | 2 +- 7 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index d2e2902040f..7f708412fd3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -578,7 +578,11 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddArg("-forcednsseed", strprintf("Always query for peer addresses via DNS lookup (default: %u)", DEFAULT_FORCEDNSSEED), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-listen", strprintf("Accept connections from outside (default: %u if no -proxy, -connect or -maxconnections=0)", DEFAULT_LISTEN), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-listenonion", strprintf("Automatically create Tor onion service (default: %d)", DEFAULT_LISTEN_ONION), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-maxconnections=", strprintf("Maintain at most automatic connections to peers (default: %u). This limit does not apply to connections manually added via -addnode or the addnode RPC, which have a separate limit of %u. It does not apply to short-lived private broadcast connections either, which have a separate limit of %u.", DEFAULT_MAX_PEER_CONNECTIONS, MAX_ADDNODE_CONNECTIONS, MAX_PRIVATE_BROADCAST_CONNECTIONS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-maxconnections=", strprintf("Maintain at most automatic connections to peers (default: %u). %u slots of these are reserved for outgoing connections, %u percent of the remaining ones can support transaction relay. " + "This limit does not apply to connections manually added via -addnode or the addnode RPC, which have a separate limit of %u. " + "It does not apply to short-lived private broadcast connections either, which have a separate limit of %u.", + DEFAULT_MAX_PEER_CONNECTIONS, MAX_OUTBOUND_FULL_RELAY_CONNECTIONS + MAX_BLOCK_RELAY_ONLY_CONNECTIONS + MAX_FEELER_CONNECTIONS, static_cast(100 * FULL_RELAY_INBOUND_PCT), MAX_ADDNODE_CONNECTIONS, MAX_PRIVATE_BROADCAST_CONNECTIONS), + ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxreceivebuffer=", strprintf("Maximum per-connection receive buffer, *1000 bytes (default: %u)", DEFAULT_MAXRECEIVEBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxsendbuffer=", strprintf("Maximum per-connection memory usage for the send buffer, *1000 bytes (default: %u)", DEFAULT_MAXSENDBUFFER), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-maxuploadtarget=", strprintf("Tries to keep outbound traffic under the given target per 24h. Limit does not apply to peers with 'download' permission or blocks created within past week. 0 = no limit (default: %s). Optional suffix units [k|K|m|M|g|G|t|T] (default: M). Lowercase is 1000 base while uppercase is 1024 base", DEFAULT_MAX_UPLOAD_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); diff --git a/src/net.cpp b/src/net.cpp index 74746ac13af..c9cd94b10c8 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2535,6 +2535,23 @@ int CConnman::GetExtraBlockRelayCount() const return std::max(block_relay_peers - m_max_outbound_block_relay, 0); } +bool CConnman::EvictTxPeerIfFull(std::optional protect_peer) +{ + int tx_inbound_peers{0}; + { + LOCK(m_nodes_mutex); + for (const CNode* pnode : m_nodes) { + if (!pnode->fDisconnect && pnode->IsInboundConn() && pnode->m_relays_txs) { + ++tx_inbound_peers; + } + } + } + if (tx_inbound_peers > m_max_inbound_full_relay) { + return AttemptToEvictConnection(/*evict_tx_relay_peer_only=*/true, protect_peer); + } + return true; +} + std::unordered_set CConnman::GetReachableEmptyNetworks() const { std::unordered_set networks{}; diff --git a/src/net.h b/src/net.h index 22f44df42ab..fa5768cb279 100644 --- a/src/net.h +++ b/src/net.h @@ -78,7 +78,9 @@ static constexpr size_t MAX_PRIVATE_BROADCAST_CONNECTIONS{64}; /** -listen default */ static const bool DEFAULT_LISTEN = true; /** The maximum number of peer connections to maintain. */ -static const unsigned int DEFAULT_MAX_PEER_CONNECTIONS = 125; +static const unsigned int DEFAULT_MAX_PEER_CONNECTIONS{200}; +/** Percentage of inbound connection slots that tx-relaying peers can use */ +static const int FULL_RELAY_INBOUND_PCT{50}; /** The default for -maxuploadtarget. 0 = Unlimited */ static const std::string DEFAULT_MAX_UPLOAD_TARGET{"0M"}; /** Default for blocks only*/ @@ -1086,7 +1088,7 @@ public: struct Options { ServiceFlags m_local_services = NODE_NONE; - int m_max_automatic_connections = 0; + int m_max_automatic_connections = DEFAULT_MAX_PEER_CONNECTIONS; CClientUIInterface* uiInterface = nullptr; NetEventsInterface* m_msgproc = nullptr; BanMan* m_banman = nullptr; @@ -1122,6 +1124,7 @@ public: m_max_outbound_block_relay = std::min(MAX_BLOCK_RELAY_ONLY_CONNECTIONS, m_max_automatic_connections - m_max_outbound_full_relay); m_max_automatic_outbound = m_max_outbound_full_relay + m_max_outbound_block_relay + m_max_feeler; m_max_inbound = std::max(0, m_max_automatic_connections - m_max_automatic_outbound); + m_max_inbound_full_relay = std::max(0, static_cast(FULL_RELAY_INBOUND_PCT / 100.0 * m_max_inbound)); m_use_addrman_outgoing = connOptions.m_use_addrman_outgoing; m_client_interface = connOptions.uiInterface; m_banman = connOptions.m_banman; @@ -1343,6 +1346,16 @@ public: int GetExtraFullOutboundCount() const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); // Count the number of block-relay-only peers we have over our limit. int GetExtraBlockRelayCount() const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); + /** + * If we are at capacity for inbound tx-relay peers, attempt to evict one. + * @param[in] protect_peer NodeId of a peer we want to protect + * @return bool Returns true if successful (either there is + * no need for eviction, or a peer was evicted). + * Returns false, if we are full but couldn't find + * a peer to evict (all eligible peers are protected) + * so that the caller can deal with this. + */ + bool EvictTxPeerIfFull(std::optional protect_peer = std::nullopt) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex); bool AddNode(const AddedNodeParams& add) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); bool RemoveAddedNode(std::string_view node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex); @@ -1720,6 +1733,7 @@ private: int m_max_feeler{MAX_FEELER_CONNECTIONS}; int m_max_automatic_outbound; int m_max_inbound; + int m_max_inbound_full_relay; bool m_use_addrman_outgoing; CClientUIInterface* m_client_interface; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c01f93c21ae..8905b15893b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -602,6 +602,20 @@ private: */ bool MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer); + /** If an inbound peer wants tx relay and we are at capacity for those, attempt to + * evict a tx-relaying inbound peer - possibly node itself, unless it is protected. + * Only if no peer can be evicted, disconnect node. + * + * @param[in] node The node that wants to relay txs to us. + * @param[in] msg_type The message that triggered this check, for logging. + * @param[in] protect_peer Peer that is exempt from being evicted. + * @return True if the node was disconnected because no eviction candidate + * was found. If false is returned, a non-protected node may still have + * been marked for disconnection via regular eviction. + */ + bool MaybeDisconnectForTxRelayCapacity(CNode& node, const std::string& msg_type, + std::optional protect_peer = std::nullopt); + /** Handle a transaction whose result was not MempoolAcceptResult::ResultType::VALID. * @param[in] first_time_failure Whether we should consider inserting into vExtraTxnForCompact, adding * a new orphan to resolve, or looking for a package to submit. @@ -3772,6 +3786,9 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string // MakeAndPushFeature(pfrom, NetMsgFeature::FOO, uint32_t{1}); } + // If we have too many tx-relaying inbound peers, attempt to evict an existing one. + // Only if this fails, disconnect this peer. + if (MaybeDisconnectForTxRelayCapacity(pfrom, msg_type, /*protect_peer=*/pfrom.GetId())) return; MakeAndPushMessage(pfrom, NetMsgType::VERACK); // Potentially mark this peer as a preferred download peer. @@ -5138,6 +5155,16 @@ bool PeerManagerImpl::MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer) return true; } +bool PeerManagerImpl::MaybeDisconnectForTxRelayCapacity(CNode& node, const std::string& msg_type, std::optional protect_peer) +{ + if (!node.IsInboundConn() || !node.m_relays_txs) return false; + if (m_connman.EvictTxPeerIfFull(protect_peer)) return false; + + LogDebug(BCLog::NET, "failed to find a tx-relaying eviction candidate - connection dropped after %s message, peer=%d\n", msg_type, node.GetId()); + node.fDisconnect = true; + return true; +} + bool PeerManagerImpl::ProcessMessages(CNode& node, std::atomic& interruptMsgProc) { AssertLockNotHeld(m_tx_download_mutex); diff --git a/test/functional/interface_usdt_net.py b/test/functional/interface_usdt_net.py index de481133aae..6ec3d1268d1 100755 --- a/test/functional/interface_usdt_net.py +++ b/test/functional/interface_usdt_net.py @@ -35,8 +35,8 @@ MAX_MSG_DATA_LENGTH = 150 # from net_address.h NETWORK_TYPE_UNROUTABLE = 0 # Use in -maxconnections. Results in a maximum of 21 inbound connections -MAX_CONNECTIONS = 32 -MAX_INBOUND_CONNECTIONS = MAX_CONNECTIONS - 10 - 1 # 10 outbound and 1 feeler +MAX_CONNECTIONS = 53 +MAX_INBOUND_CONNECTIONS = 21 # 10 outbound and 1 feeler, (MAX_CONNECTIONS - 10 - 1) / 2 slots for tx-relaying inbounds net_tracepoints_program = """ #include diff --git a/test/functional/p2p_eviction.py b/test/functional/p2p_eviction.py index c96f2a2c439..f297b8f39f7 100755 --- a/test/functional/p2p_eviction.py +++ b/test/functional/p2p_eviction.py @@ -45,10 +45,12 @@ class SlowP2PInterface(P2PInterface): class P2PEvict(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - # The choice of maxconnections=32 results in a maximum of 21 inbound connections - # (32 - 10 outbound - 1 feeler). 20 inbound peers are protected from eviction: + # The choice of maxconnections=53 results in a maximum of 21 tx-relaying inbound connections + # (53 - 10 outbound - 1 feeler) * 0.5 = 21. The other inbound slots are reserved for block-relay-only + # peers that don't play a role in this test. + # 20 inbound peers are protected from eviction: # 4 by netgroup, 4 that sent us blocks, 4 that sent us transactions and 8 via lowest ping time - self.extra_args = [['-maxconnections=32']] + self.extra_args = [['-maxconnections=53']] def run_test(self): protected_peers = set() # peers that we expect to be protected from eviction diff --git a/test/functional/p2p_opportunistic_1p1c.py b/test/functional/p2p_opportunistic_1p1c.py index 77616de13e5..6841d7c3a14 100755 --- a/test/functional/p2p_opportunistic_1p1c.py +++ b/test/functional/p2p_opportunistic_1p1c.py @@ -76,7 +76,7 @@ class PackageRelayTest(BitcoinTestFramework): self.setup_clean_chain = True self.num_nodes = 1 self.extra_args = [[ - "-maxmempool=5", + "-maxmempool=5","-maxconnections=150" ]] def create_tx_below_mempoolminfee(self, wallet, utxo_to_spend=None):