From c6131bf407c1ada78a0e5509a702bc7da0bfd57d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 7 May 2020 10:22:47 -0700 Subject: Abstract logic to determine whether to answer tx GETDATA --- src/net_processing.cpp | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 1df1fab59d..0193e3f1e3 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1608,6 +1608,26 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c } } +//! Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). +CTransactionRef static FindTxForGetData(const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds longlived_mempool_time) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +{ + // Look up transaction in relay pool + auto mi = mapRelay.find(txid); + if (mi != mapRelay.end()) return mi->second; + + auto txinfo = mempool.info(txid); + if (txinfo.tx) { + // To protect privacy, do not answer getdata using the mempool when + // that TX couldn't have been INVed in reply to a MEMPOOL request, + // or when it's too recent to have expired from mapRelay. + if ((mempool_req.count() && txinfo.m_time <= mempool_req) || txinfo.m_time <= longlived_mempool_time) { + return txinfo.tx; + } + } + + return {}; +} + void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnman* connman, CTxMemPool& mempool, const std::atomic& interruptMsgProc) LOCKS_EXCLUDED(cs_main) { AssertLockNotHeld(cs_main); @@ -1643,31 +1663,10 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm continue; } - // Send stream from relay memory - bool push = false; - auto mi = mapRelay.find(inv.hash); - int nSendFlags = (inv.type == MSG_TX ? SERIALIZE_TRANSACTION_NO_WITNESS : 0); - if (mi != mapRelay.end()) { - connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *mi->second)); - push = true; - } else { - auto txinfo = mempool.info(inv.hash); - // To protect privacy, do not answer getdata using the mempool when - // that TX couldn't have been INVed in reply to a MEMPOOL request, - // or when it's too recent to have expired from mapRelay. - if (txinfo.tx && ( - (mempool_req.count() && txinfo.m_time <= mempool_req) - || (txinfo.m_time <= longlived_mempool_time))) - { - connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *txinfo.tx)); - push = true; - } - } - - if (push) { - // We interpret fulfilling a GETDATA for a transaction as a - // successful initial broadcast and remove it from our - // unbroadcast set. + CTransactionRef tx = FindTxForGetData(inv.hash, mempool_req, longlived_mempool_time); + if (tx) { + int nSendFlags = (inv.type == MSG_TX ? SERIALIZE_TRANSACTION_NO_WITNESS : 0); + connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *tx)); mempool.RemoveUnbroadcastTx(inv.hash); } else { vNotFound.push_back(inv); -- cgit v1.2.3 From f2f32a3dee9a965c8198f9ddd3aaebc627c273e4 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 12 May 2020 12:40:54 -0700 Subject: Push down use of cs_main into FindTxForGetData --- src/net_processing.cpp | 59 ++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 31 deletions(-) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0193e3f1e3..418d1474d1 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1609,11 +1609,14 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c } //! Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). -CTransactionRef static FindTxForGetData(const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds longlived_mempool_time) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +CTransactionRef static FindTxForGetData(const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds longlived_mempool_time) LOCKS_EXCLUDED(cs_main) { - // Look up transaction in relay pool - auto mi = mapRelay.find(txid); - if (mi != mapRelay.end()) return mi->second; + { + LOCK(cs_main); + // Look up transaction in relay pool + auto mi = mapRelay.find(txid); + if (mi != mapRelay.end()) return mi->second; + } auto txinfo = mempool.info(txid); if (txinfo.tx) { @@ -1642,37 +1645,31 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm const std::chrono::seconds mempool_req = pfrom->m_tx_relay != nullptr ? pfrom->m_tx_relay->m_last_mempool_req.load() : std::chrono::seconds::min(); - { - LOCK(cs_main); - - // 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 - // them. - while (it != pfrom->vRecvGetData.end() && (it->type == MSG_TX || it->type == MSG_WITNESS_TX)) { - if (interruptMsgProc) - return; - // The send buffer provides backpressure. If there's no space in - // the buffer, pause processing until the next call. - if (pfrom->fPauseSend) - break; + // 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 + // them. + while (it != pfrom->vRecvGetData.end() && (it->type == MSG_TX || it->type == MSG_WITNESS_TX)) { + if (interruptMsgProc) return; + // The send buffer provides backpressure. If there's no space in + // the buffer, pause processing until the next call. + if (pfrom->fPauseSend) break; - const CInv &inv = *it++; + const CInv &inv = *it++; - if (pfrom->m_tx_relay == nullptr) { - // Ignore GETDATA requests for transactions from blocks-only peers. - continue; - } + if (pfrom->m_tx_relay == nullptr) { + // Ignore GETDATA requests for transactions from blocks-only peers. + continue; + } - CTransactionRef tx = FindTxForGetData(inv.hash, mempool_req, longlived_mempool_time); - if (tx) { - int nSendFlags = (inv.type == MSG_TX ? SERIALIZE_TRANSACTION_NO_WITNESS : 0); - connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *tx)); - mempool.RemoveUnbroadcastTx(inv.hash); - } else { - vNotFound.push_back(inv); - } + CTransactionRef tx = FindTxForGetData(inv.hash, mempool_req, longlived_mempool_time); + if (tx) { + int nSendFlags = (inv.type == MSG_TX ? SERIALIZE_TRANSACTION_NO_WITNESS : 0); + connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *tx)); + mempool.RemoveUnbroadcastTx(inv.hash); + } else { + vNotFound.push_back(inv); } - } // release cs_main + } // Only process one BLOCK item per call, since they're uncommon and can be // expensive to process. -- cgit v1.2.3 From 2896c412fadbc03916a33028f4f50fd87ac48edb Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 7 May 2020 10:29:00 -0700 Subject: Do not answer GETDATA for to-be-announced tx --- src/net_processing.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 418d1474d1..0050904932 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1609,8 +1609,16 @@ void static ProcessGetBlockData(CNode* pfrom, const CChainParams& chainparams, c } //! Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). -CTransactionRef static FindTxForGetData(const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds longlived_mempool_time) LOCKS_EXCLUDED(cs_main) +CTransactionRef static FindTxForGetData(CNode* peer, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds longlived_mempool_time) LOCKS_EXCLUDED(cs_main) { + // Check if the requested transaction is so recent that we're just + // about to announce it to the peer; if so, they certainly shouldn't + // know we already have it. + { + LOCK(peer->m_tx_relay->cs_tx_inventory); + if (peer->m_tx_relay->setInventoryTxToSend.count(txid)) return {}; + } + { LOCK(cs_main); // Look up transaction in relay pool @@ -1661,7 +1669,7 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm continue; } - CTransactionRef tx = FindTxForGetData(inv.hash, mempool_req, longlived_mempool_time); + CTransactionRef tx = FindTxForGetData(pfrom, inv.hash, mempool_req, longlived_mempool_time); if (tx) { int nSendFlags = (inv.type == MSG_TX ? SERIALIZE_TRANSACTION_NO_WITNESS : 0); connman->PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *tx)); -- cgit v1.2.3