From 2266eba43a973345351f2b0a8296523fb7de5576 Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 17 Sep 2024 19:26:57 -0400 Subject: [PATCH] [p2p] don't find 1p1cs for reconsiderable txns that are AlreadyHaveTx This is a slight behavior change: if a transaction is in both reconsiderable rejects and AlreadyHaveTx in another way, we don't try to return a 1p1c package. This is the correct thing to do, as we don't want to reconsider transactions that have multiple things wrong with them. For example, if a transaction is low feerate, and then later found to have a bad signature, we shouldn't try it again in a package. --- src/node/txdownloadman_impl.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/node/txdownloadman_impl.cpp b/src/node/txdownloadman_impl.cpp index 89125982dd6..ccc6cff8940 100644 --- a/src/node/txdownloadman_impl.cpp +++ b/src/node/txdownloadman_impl.cpp @@ -477,17 +477,7 @@ std::pair> TxDownloadManagerImpl::Receive // already; and an adversary can already relay us old transactions // (older than our recency filter) if trying to DoS us, without any need // for witness malleation. - if (AlreadyHaveTx(GenTxid::Wtxid(wtxid), /*include_reconsiderable=*/true)) { - - if (RecentRejectsReconsiderableFilter().contains(wtxid)) { - // When a transaction is already in m_lazy_recent_rejects_reconsiderable, we shouldn't submit - // it by itself again. However, look for a matching child in the orphanage, as it is - // possible that they succeed as a package. - LogDebug(BCLog::TXPACKAGES, "found tx %s (wtxid=%s) in reconsiderable rejects, looking for child in orphanage\n", - txid.ToString(), wtxid.ToString()); - return std::make_pair(false, Find1P1CPackage(ptx, nodeid)); - } - + if (AlreadyHaveTx(GenTxid::Wtxid(wtxid), /*include_reconsiderable=*/false)) { // If a tx is detected by m_lazy_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. @@ -504,8 +494,16 @@ std::pair> TxDownloadManagerImpl::Receive // peer simply for relaying a tx that our m_lazy_recent_rejects has caught, // regardless of false positives. return {false, std::nullopt}; + } else if (RecentRejectsReconsiderableFilter().contains(wtxid)) { + // When a transaction is already in m_lazy_recent_rejects_reconsiderable, we shouldn't submit + // it by itself again. However, look for a matching child in the orphanage, as it is + // possible that they succeed as a package. + LogDebug(BCLog::TXPACKAGES, "found tx %s (wtxid=%s) in reconsiderable rejects, looking for child in orphanage\n", + txid.ToString(), wtxid.ToString()); + return {false, Find1P1CPackage(ptx, nodeid)}; } + return {true, std::nullopt}; }