From a3b55c94b9ffde07386aa887149ddca91b364967 Mon Sep 17 00:00:00 2001 From: glozow Date: Mon, 14 Aug 2023 14:05:34 +0100 Subject: [PATCH] [doc] move comment about AlreadyHaveTx DoS score to the right place This comment isn't in the right place, as detection of a tx in recent_rejects would cause the function to exit much earlier. Move the comment to the right place and tweak the first sentence for accuracy. --- src/net_processing.cpp | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ec8230fd688..c5a22f258a4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4135,6 +4135,21 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, RelayTransaction(tx.GetHash(), tx.GetWitnessHash()); } } + // If a tx is detected by m_recent_rejects it is ignored. Because we haven't + // submitted the tx to our mempool, we won't have computed a DoS + // score for it or determined exactly why we consider it invalid. + // + // This means we won't penalize any peer subsequently relaying a DoSy + // tx (even if we penalized the first peer who gave it to us) because + // we have to account for m_recent_rejects showing false positives. In + // other words, we shouldn't penalize a peer if we aren't *sure* they + // submitted a DoSy tx. + // + // Note that m_recent_rejects doesn't just record DoSy or invalid + // transactions, but any tx not accepted by the mempool, which may be + // due to node policy (vs. consensus). So we can't blanket penalize a + // peer simply for relaying a tx that our m_recent_rejects has caught, + // regardless of false positives. return; } @@ -4255,23 +4270,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } } - // If a tx has been detected by m_recent_rejects, we will have reached - // this point and the tx will have been ignored. Because we haven't - // submitted the tx to our mempool, we won't have computed a DoS - // score for it or determined exactly why we consider it invalid. - // - // This means we won't penalize any peer subsequently relaying a DoSy - // tx (even if we penalized the first peer who gave it to us) because - // we have to account for m_recent_rejects showing false positives. In - // other words, we shouldn't penalize a peer if we aren't *sure* they - // submitted a DoSy tx. - // - // Note that m_recent_rejects doesn't just record DoSy or invalid - // transactions, but any tx not accepted by the mempool, which may be - // due to node policy (vs. consensus). So we can't blanket penalize a - // peer simply for relaying a tx that our m_recent_rejects has caught, - // regardless of false positives. - if (state.IsInvalid()) { LogPrint(BCLog::MEMPOOLREJ, "%s (wtxid=%s) from peer=%d was not accepted: %s\n", tx.GetHash().ToString(),