From 416fbc952b209817a37e76c09fff5d17be7a72d0 Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 30 Apr 2024 10:54:00 +0100 Subject: [PATCH] [refactor] move new orphan handling to ProcessInvalidTx --- src/net_processing.cpp | 159 +++++++++++++++++++++-------------------- 1 file changed, 81 insertions(+), 78 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 95fdf862f12..274653dae65 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3006,8 +3006,11 @@ void PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx AssertLockHeld(g_msgproc_mutex); AssertLockHeld(m_tx_download_mutex); + PeerRef peer{GetPeerRef(nodeid)}; + auto& m_orphanage = m_txdownloadman.GetOrphanageRef(); auto& m_txrequest = m_txdownloadman.GetTxRequestRef(); + const CTransaction& tx{*ptx}; LogDebug(BCLog::MEMPOOLREJ, "%s (wtxid=%s) from peer=%d was not accepted: %s\n", ptx->GetHash().ToString(), @@ -3016,6 +3019,84 @@ void PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx state.ToString()); if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) { + // Only process a new orphan if this is a first time failure, as otherwise it must be either + // already in orphanage or from 1p1c processing. + if (first_time_failure) { + bool fRejectedParents = false; // It may be the case that the orphans parents have all been rejected + + // Deduplicate parent txids, so that we don't have to loop over + // the same parent txid more than once down below. + std::vector unique_parents; + unique_parents.reserve(tx.vin.size()); + for (const CTxIn& txin : tx.vin) { + // We start with all parents, and then remove duplicates below. + unique_parents.push_back(txin.prevout.hash); + } + std::sort(unique_parents.begin(), unique_parents.end()); + unique_parents.erase(std::unique(unique_parents.begin(), unique_parents.end()), unique_parents.end()); + + // Distinguish between parents in m_lazy_recent_rejects and m_lazy_recent_rejects_reconsiderable. + // We can tolerate having up to 1 parent in m_lazy_recent_rejects_reconsiderable since we + // submit 1p1c packages. However, fail immediately if any are in m_lazy_recent_rejects. + std::optional rejected_parent_reconsiderable; + for (const uint256& parent_txid : unique_parents) { + if (RecentRejectsFilter().contains(parent_txid)) { + fRejectedParents = true; + break; + } else if (RecentRejectsReconsiderableFilter().contains(parent_txid) && !m_mempool.exists(GenTxid::Txid(parent_txid))) { + // More than 1 parent in m_lazy_recent_rejects_reconsiderable: 1p1c will not be + // sufficient to accept this package, so just give up here. + if (rejected_parent_reconsiderable.has_value()) { + fRejectedParents = true; + break; + } + rejected_parent_reconsiderable = parent_txid; + } + } + if (!fRejectedParents) { + const auto current_time{GetTime()}; + + for (const uint256& parent_txid : unique_parents) { + // Here, we only have the txid (and not wtxid) of the + // inputs, so we only request in txid mode, even for + // wtxidrelay peers. + // Eventually we should replace this with an improved + // protocol for getting all unconfirmed parents. + const auto gtxid{GenTxid::Txid(parent_txid)}; + if (peer) 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)) { + m_txdownloadman.AddTxAnnouncement(nodeid, gtxid, current_time, /*p2p_inv=*/false); + } + } + + if (m_orphanage.AddTx(ptx, nodeid)) { + AddToCompactExtraTransactions(ptx); + } + + // Once added to the orphan pool, a tx is considered AlreadyHave, and we shouldn't request it anymore. + m_txrequest.ForgetTxHash(tx.GetHash()); + m_txrequest.ForgetTxHash(tx.GetWitnessHash()); + + // DoS prevention: do not allow m_orphanage to grow unbounded (see CVE-2012-3789) + m_orphanage.LimitOrphans(m_opts.max_orphan_txs, m_rng); + } else { + LogDebug(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s (wtxid=%s)\n", + tx.GetHash().ToString(), + tx.GetWitnessHash().ToString()); + // We will continue to reject this tx since it has rejected + // parents so avoid re-requesting it from other peers. + // Here we add both the txid and the wtxid, as we know that + // regardless of what witness is provided, we will not accept + // this, so we don't need to allow for redownload of this txid + // from any of our non-wtxidrelay peers. + RecentRejectsFilter().insert(tx.GetHash().ToUint256()); + RecentRejectsFilter().insert(tx.GetWitnessHash().ToUint256()); + m_txrequest.ForgetTxHash(tx.GetHash()); + m_txrequest.ForgetTxHash(tx.GetWitnessHash()); + } + } return; } else if (state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) { // We can add the wtxid of this transaction to our reject filter. @@ -4375,7 +4456,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, LOCK2(cs_main, m_tx_download_mutex); auto& m_txrequest = m_txdownloadman.GetTxRequestRef(); - auto& m_orphanage = m_txdownloadman.GetOrphanageRef(); m_txrequest.ReceivedResponse(pfrom.GetId(), txid); if (tx.HasWitness()) m_txrequest.ReceivedResponse(pfrom.GetId(), wtxid); @@ -4445,83 +4525,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, ProcessValidTx(pfrom.GetId(), ptx, result.m_replaced_transactions); pfrom.m_last_tx_time = GetTime(); } - else if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) - { - bool fRejectedParents = false; // It may be the case that the orphans parents have all been rejected - - // Deduplicate parent txids, so that we don't have to loop over - // the same parent txid more than once down below. - std::vector unique_parents; - unique_parents.reserve(tx.vin.size()); - for (const CTxIn& txin : tx.vin) { - // We start with all parents, and then remove duplicates below. - unique_parents.push_back(txin.prevout.hash); - } - std::sort(unique_parents.begin(), unique_parents.end()); - unique_parents.erase(std::unique(unique_parents.begin(), unique_parents.end()), unique_parents.end()); - - // Distinguish between parents in m_lazy_recent_rejects and m_lazy_recent_rejects_reconsiderable. - // We can tolerate having up to 1 parent in m_lazy_recent_rejects_reconsiderable since we - // submit 1p1c packages. However, fail immediately if any are in m_lazy_recent_rejects. - std::optional rejected_parent_reconsiderable; - for (const uint256& parent_txid : unique_parents) { - if (RecentRejectsFilter().contains(parent_txid)) { - fRejectedParents = true; - break; - } else if (RecentRejectsReconsiderableFilter().contains(parent_txid) && !m_mempool.exists(GenTxid::Txid(parent_txid))) { - // More than 1 parent in m_lazy_recent_rejects_reconsiderable: 1p1c will not be - // sufficient to accept this package, so just give up here. - if (rejected_parent_reconsiderable.has_value()) { - fRejectedParents = true; - break; - } - rejected_parent_reconsiderable = parent_txid; - } - } - if (!fRejectedParents) { - const auto current_time{GetTime()}; - - for (const uint256& parent_txid : unique_parents) { - // Here, we only have the txid (and not wtxid) of the - // inputs, so we only request in txid mode, even for - // wtxidrelay peers. - // Eventually we should replace this with an improved - // protocol for getting all unconfirmed parents. - const auto gtxid{GenTxid::Txid(parent_txid)}; - 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)) { - m_txdownloadman.AddTxAnnouncement(pfrom.GetId(), gtxid, current_time, /*p2p_inv=*/false); - } - } - - if (m_orphanage.AddTx(ptx, pfrom.GetId())) { - AddToCompactExtraTransactions(ptx); - } - - // Once added to the orphan pool, a tx is considered AlreadyHave, and we shouldn't request it anymore. - m_txrequest.ForgetTxHash(tx.GetHash()); - m_txrequest.ForgetTxHash(tx.GetWitnessHash()); - - // DoS prevention: do not allow m_orphanage to grow unbounded (see CVE-2012-3789) - m_orphanage.LimitOrphans(m_opts.max_orphan_txs, m_rng); - } else { - LogDebug(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s (wtxid=%s)\n", - tx.GetHash().ToString(), - tx.GetWitnessHash().ToString()); - // We will continue to reject this tx since it has rejected - // parents so avoid re-requesting it from other peers. - // Here we add both the txid and the wtxid, as we know that - // regardless of what witness is provided, we will not accept - // this, so we don't need to allow for redownload of this txid - // from any of our non-wtxidrelay peers. - RecentRejectsFilter().insert(tx.GetHash().ToUint256()); - RecentRejectsFilter().insert(tx.GetWitnessHash().ToUint256()); - m_txrequest.ForgetTxHash(tx.GetHash()); - m_txrequest.ForgetTxHash(tx.GetWitnessHash()); - } - } if (state.IsInvalid()) { ProcessInvalidTx(pfrom.GetId(), ptx, state, /*first_time_failure=*/true); }