From 042a97ce7fc672021cdb1dee62a550ef19c208fb Mon Sep 17 00:00:00 2001 From: glozow Date: Thu, 25 Apr 2024 11:43:04 +0100 Subject: [PATCH] [refactor] move tx inv/getdata handling to txdownload --- src/net_processing.cpp | 94 +++++---------------------------- src/node/txdownloadman.h | 24 +++++++++ src/node/txdownloadman_impl.cpp | 63 ++++++++++++++++++++++ src/node/txdownloadman_impl.h | 6 +++ 4 files changed, 105 insertions(+), 82 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1ba1e853e71..71162e957db 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -89,22 +89,6 @@ static constexpr auto PING_INTERVAL{2min}; static const unsigned int MAX_LOCATOR_SZ = 101; /** The maximum number of entries in an 'inv' protocol message */ static const unsigned int MAX_INV_SZ = 50000; -/** Maximum number of in-flight transaction requests from a peer. It is not a hard limit, but the threshold at which - * point the OVERLOADED_PEER_TX_DELAY kicks in. */ -static constexpr int32_t MAX_PEER_TX_REQUEST_IN_FLIGHT = 100; -/** Maximum number of transactions to consider for requesting, per peer. It provides a reasonable DoS limit to - * per-peer memory usage spent on announcements, while covering peers continuously sending INVs at the maximum - * rate (by our own policy, see INVENTORY_BROADCAST_PER_SECOND) for several minutes, while not receiving - * the actual transaction (from any peer) in response to requests for them. */ -static constexpr int32_t MAX_PEER_TX_ANNOUNCEMENTS = 5000; -/** How long to delay requesting transactions via txids, if we have wtxid-relaying peers */ -static constexpr auto TXID_RELAY_DELAY{2s}; -/** How long to delay requesting transactions from non-preferred peers */ -static constexpr auto NONPREF_PEER_TX_DELAY{2s}; -/** How long to delay requesting transactions from overloaded peers (see MAX_PEER_TX_REQUEST_IN_FLIGHT). */ -static constexpr auto OVERLOADED_PEER_TX_DELAY{2s}; -/** How long to wait before downloading a transaction from an additional peer */ -static constexpr auto GETDATA_TX_INTERVAL{60s}; /** Limit to avoid sending big packets. Not used in processing incoming GETDATA for compatibility */ static const unsigned int MAX_GETDATA_SZ = 1000; /** Number of blocks that can be requested at any given time from a single peer. */ @@ -156,7 +140,7 @@ static constexpr unsigned int INVENTORY_BROADCAST_TARGET = INVENTORY_BROADCAST_P /** Maximum number of inventory items to send per transmission. */ static constexpr unsigned int INVENTORY_BROADCAST_MAX = 1000; static_assert(INVENTORY_BROADCAST_MAX >= INVENTORY_BROADCAST_TARGET, "INVENTORY_BROADCAST_MAX too low"); -static_assert(INVENTORY_BROADCAST_MAX <= MAX_PEER_TX_ANNOUNCEMENTS, "INVENTORY_BROADCAST_MAX too high"); +static_assert(INVENTORY_BROADCAST_MAX <= node::MAX_PEER_TX_ANNOUNCEMENTS, "INVENTORY_BROADCAST_MAX too high"); /** Average delay between feefilter broadcasts in seconds. */ static constexpr auto AVG_FEEFILTER_BROADCAST_INTERVAL{10min}; /** Maximum feefilter broadcast delay after significant change. */ @@ -720,12 +704,6 @@ private: void SendBlockTransactions(CNode& pfrom, Peer& peer, const CBlock& block, const BlockTransactionsRequest& req); - /** Register with TxRequestTracker that an INV has been received from a - * peer. The announcement parameters are decided in PeerManager and then - * passed to TxRequestTracker. */ - void AddTxAnnouncement(const CNode& node, const GenTxid& gtxid, std::chrono::microseconds current_time) - EXCLUSIVE_LOCKS_REQUIRED(::cs_main, m_tx_download_mutex); - /** Send a message to a peer */ void PushMessage(CNode& node, CSerializedNetMsg&& msg) const { m_connman.PushMessage(&node, std::move(msg)); } template @@ -1571,36 +1549,6 @@ void PeerManagerImpl::PushNodeVersion(CNode& pnode, const Peer& peer) } } -void PeerManagerImpl::AddTxAnnouncement(const CNode& node, const GenTxid& gtxid, std::chrono::microseconds current_time) -{ - AssertLockHeld(::cs_main); // for State - AssertLockHeld(m_tx_download_mutex); // For m_txrequest - NodeId nodeid = node.GetId(); - - auto& m_txrequest = m_txdownloadman.GetTxRequestRef(); - if (!node.HasPermission(NetPermissionFlags::Relay) && m_txrequest.Count(nodeid) >= MAX_PEER_TX_ANNOUNCEMENTS) { - // Too many queued announcements from this peer - return; - } - const CNodeState* state = State(nodeid); - - // Decide the TxRequestTracker parameters for this announcement: - // - "preferred": if fPreferredDownload is set (= outbound, or NetPermissionFlags::NoBan permission) - // - "reqtime": current time plus delays for: - // - NONPREF_PEER_TX_DELAY for announcements from non-preferred connections - // - TXID_RELAY_DELAY for txid announcements while wtxid peers are available - // - OVERLOADED_PEER_TX_DELAY for announcements from peers which have at least - // MAX_PEER_TX_REQUEST_IN_FLIGHT requests in flight (and don't have NetPermissionFlags::Relay). - auto delay{0us}; - const bool preferred = state->fPreferredDownload; - if (!preferred) delay += NONPREF_PEER_TX_DELAY; - if (!gtxid.IsWtxid() && m_wtxid_relay_peers > 0) delay += TXID_RELAY_DELAY; - const bool overloaded = !node.HasPermission(NetPermissionFlags::Relay) && - m_txrequest.CountInFlight(nodeid) >= MAX_PEER_TX_REQUEST_IN_FLIGHT; - if (overloaded) delay += OVERLOADED_PEER_TX_DELAY; - m_txrequest.ReceivedInv(nodeid, gtxid, preferred, current_time + delay); -} - void PeerManagerImpl::UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) { LOCK(cs_main); @@ -4133,11 +4081,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, AddKnownTx(*peer, inv.hash); if (!m_chainman.IsInitialBlockDownload()) { - const bool fAlreadyHave = m_txdownloadman.AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true); + const bool fAlreadyHave{m_txdownloadman.AddTxAnnouncement(pfrom.GetId(), gtxid, current_time, /*p2p_inv=*/true)}; LogDebug(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); - if (!fAlreadyHave) { - AddTxAnnouncement(pfrom, gtxid, current_time); - } } } else { LogDebug(BCLog::NET, "Unknown inv type \"%s\" received from peer=%d\n", inv.ToString(), pfrom.GetId()); @@ -4546,7 +4491,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, AddKnownTx(*peer, parent_txid); // Exclude m_lazy_recent_rejects_reconsiderable: the missing parent may have been // previously rejected for being too low feerate. This orphan might CPFP it. - if (!m_txdownloadman.AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) AddTxAnnouncement(pfrom, gtxid, current_time); + if (!m_txdownloadman.AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) { + m_txdownloadman.AddTxAnnouncement(pfrom.GetId(), gtxid, current_time, /*p2p_inv=*/false); + } } if (m_orphanage.AddTx(ptx, pfrom.GetId())) { @@ -5186,7 +5133,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (msg_type == NetMsgType::NOTFOUND) { std::vector vInv; vRecv >> vInv; - if (vInv.size() <= MAX_PEER_TX_ANNOUNCEMENTS + MAX_BLOCKS_IN_TRANSIT_PER_PEER) { + if (vInv.size() <= node::MAX_PEER_TX_ANNOUNCEMENTS + MAX_BLOCKS_IN_TRANSIT_PER_PEER) { LOCK(m_tx_download_mutex); for (CInv &inv : vInv) { if (inv.IsGenTxMsg()) { @@ -6210,31 +6157,14 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // { LOCK(m_tx_download_mutex); - std::vector> expired; - auto requestable = m_txdownloadman.GetTxRequestRef().GetRequestable(pto->GetId(), current_time, &expired); - for (const auto& entry : expired) { - LogDebug(BCLog::NET, "timeout of inflight %s %s from peer=%d\n", entry.second.IsWtxid() ? "wtx" : "tx", - entry.second.GetHash().ToString(), entry.first); - } - for (const GenTxid& gtxid : requestable) { - // Exclude m_lazy_recent_rejects_reconsiderable: we may be requesting a missing parent - // that was previously rejected for being too low feerate. - if (!m_txdownloadman.AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) { - LogDebug(BCLog::NET, "Requesting %s %s peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx", - gtxid.GetHash().ToString(), pto->GetId()); - vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.GetHash()); - if (vGetData.size() >= MAX_GETDATA_SZ) { - MakeAndPushMessage(*pto, NetMsgType::GETDATA, vGetData); - vGetData.clear(); - } - m_txdownloadman.GetTxRequestRef().RequestedTx(pto->GetId(), gtxid.GetHash(), current_time + GETDATA_TX_INTERVAL); - } else { - // We have already seen this transaction, no need to download. This is just a belt-and-suspenders, as - // this should already be called whenever a transaction becomes AlreadyHaveTx(). - m_txdownloadman.GetTxRequestRef().ForgetTxHash(gtxid.GetHash()); + for (const GenTxid& gtxid : m_txdownloadman.GetRequestsToSend(pto->GetId(), current_time)) { + vGetData.emplace_back(gtxid.IsWtxid() ? MSG_WTX : (MSG_TX | GetFetchFlags(*peer)), gtxid.GetHash()); + if (vGetData.size() >= MAX_GETDATA_SZ) { + MakeAndPushMessage(*pto, NetMsgType::GETDATA, vGetData); + vGetData.clear(); } } - } // release m_tx_download_mutex + } if (!vGetData.empty()) MakeAndPushMessage(*pto, NetMsgType::GETDATA, vGetData); diff --git a/src/node/txdownloadman.h b/src/node/txdownloadman.h index 93c35b4d24a..ce0c9594281 100644 --- a/src/node/txdownloadman.h +++ b/src/node/txdownloadman.h @@ -19,6 +19,22 @@ class TxRequestTracker; namespace node { class TxDownloadManagerImpl; +/** Maximum number of in-flight transaction requests from a peer. It is not a hard limit, but the threshold at which + * point the OVERLOADED_PEER_TX_DELAY kicks in. */ +static constexpr int32_t MAX_PEER_TX_REQUEST_IN_FLIGHT = 100; +/** Maximum number of transactions to consider for requesting, per peer. It provides a reasonable DoS limit to + * per-peer memory usage spent on announcements, while covering peers continuously sending INVs at the maximum + * rate (by our own policy, see INVENTORY_BROADCAST_PER_SECOND) for several minutes, while not receiving + * the actual transaction (from any peer) in response to requests for them. */ +static constexpr int32_t MAX_PEER_TX_ANNOUNCEMENTS = 5000; +/** How long to delay requesting transactions via txids, if we have wtxid-relaying peers */ +static constexpr auto TXID_RELAY_DELAY{2s}; +/** How long to delay requesting transactions from non-preferred peers */ +static constexpr auto NONPREF_PEER_TX_DELAY{2s}; +/** How long to delay requesting transactions from overloaded peers (see MAX_PEER_TX_REQUEST_IN_FLIGHT). */ +static constexpr auto OVERLOADED_PEER_TX_DELAY{2s}; +/** How long to wait before downloading a transaction from an additional peer */ +static constexpr auto GETDATA_TX_INTERVAL{60s}; struct TxDownloadOptions { /** Read-only reference to mempool. */ const CTxMemPool& m_mempool; @@ -84,6 +100,14 @@ public: /** Deletes all txrequest announcements and orphans for a given peer. */ void DisconnectedPeer(NodeId nodeid); + + /** New inv has been received. May be added as a candidate to txrequest. + * @param[in] p2p_inv When true, only add this announcement if we don't already have the tx. + * Returns true if this was a dropped inv (p2p_inv=true and we already have the tx), false otherwise. */ + bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv); + + /** Get getdata requests to send. */ + std::vector GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time); }; } // namespace node #endif // BITCOIN_NODE_TXDOWNLOADMAN_H diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp index 6a00e1c70f2..d0125f69ec0 100644 --- a/src/node/txdownloadman_impl.cpp +++ b/src/node/txdownloadman_impl.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -58,6 +59,14 @@ void TxDownloadManager::DisconnectedPeer(NodeId nodeid) { m_impl->DisconnectedPeer(nodeid); } +bool TxDownloadManager::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv) +{ + return m_impl->AddTxAnnouncement(peer, gtxid, now, p2p_inv); +} +std::vector TxDownloadManager::GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time) +{ + return m_impl->GetRequestsToSend(nodeid, current_time); +} // TxDownloadManagerImpl void TxDownloadManagerImpl::ActiveTipChange() @@ -142,4 +151,58 @@ void TxDownloadManagerImpl::DisconnectedPeer(NodeId nodeid) } } + +bool TxDownloadManagerImpl::AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv) +{ + // If this is an inv received from a peer and we already have it, we can drop it. + if (p2p_inv && AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) return true; + + auto it = m_peer_info.find(peer); + if (it == m_peer_info.end()) return false; + const auto& info = it->second.m_connection_info; + if (!info.m_relay_permissions && m_txrequest.Count(peer) >= MAX_PEER_TX_ANNOUNCEMENTS) { + // Too many queued announcements for this peer + return false; + } + // Decide the TxRequestTracker parameters for this announcement: + // - "preferred": if fPreferredDownload is set (= outbound, or NetPermissionFlags::NoBan permission) + // - "reqtime": current time plus delays for: + // - NONPREF_PEER_TX_DELAY for announcements from non-preferred connections + // - TXID_RELAY_DELAY for txid announcements while wtxid peers are available + // - OVERLOADED_PEER_TX_DELAY for announcements from peers which have at least + // MAX_PEER_TX_REQUEST_IN_FLIGHT requests in flight (and don't have NetPermissionFlags::Relay). + auto delay{0us}; + if (!info.m_preferred) delay += NONPREF_PEER_TX_DELAY; + if (!gtxid.IsWtxid() && m_num_wtxid_peers > 0) delay += TXID_RELAY_DELAY; + const bool overloaded = !info.m_relay_permissions && m_txrequest.CountInFlight(peer) >= MAX_PEER_TX_REQUEST_IN_FLIGHT; + if (overloaded) delay += OVERLOADED_PEER_TX_DELAY; + + m_txrequest.ReceivedInv(peer, gtxid, info.m_preferred, now + delay); + + return false; +} + +std::vector TxDownloadManagerImpl::GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time) +{ + std::vector requests; + std::vector> expired; + auto requestable = m_txrequest.GetRequestable(nodeid, current_time, &expired); + for (const auto& entry : expired) { + LogDebug(BCLog::NET, "timeout of inflight %s %s from peer=%d\n", entry.second.IsWtxid() ? "wtx" : "tx", + entry.second.GetHash().ToString(), entry.first); + } + for (const GenTxid& gtxid : requestable) { + if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) { + LogDebug(BCLog::NET, "Requesting %s %s peer=%d\n", gtxid.IsWtxid() ? "wtx" : "tx", + gtxid.GetHash().ToString(), nodeid); + requests.emplace_back(gtxid); + m_txrequest.RequestedTx(nodeid, gtxid.GetHash(), current_time + GETDATA_TX_INTERVAL); + } else { + // We have already seen this transaction, no need to download. This is just a belt-and-suspenders, as + // this should already be called whenever a transaction becomes AlreadyHaveTx(). + m_txrequest.ForgetTxHash(gtxid.GetHash()); + } + } + return requests; +} } // namespace node diff --git a/src/node/txdownloadman_impl.h b/src/node/txdownloadman_impl.h index fc57f690240..49948908d41 100644 --- a/src/node/txdownloadman_impl.h +++ b/src/node/txdownloadman_impl.h @@ -150,6 +150,12 @@ public: void ConnectedPeer(NodeId nodeid, const TxDownloadConnectionInfo& info); void DisconnectedPeer(NodeId nodeid); + + /** New inv has been received. May be added as a candidate to txrequest. */ + bool AddTxAnnouncement(NodeId peer, const GenTxid& gtxid, std::chrono::microseconds now, bool p2p_inv); + + /** Get getdata requests to send. */ + std::vector GetRequestsToSend(NodeId nodeid, std::chrono::microseconds current_time); }; } // namespace node #endif // BITCOIN_NODE_TXDOWNLOADMAN_IMPL_H