diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 37d9e169e1..da4f99fb99 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -596,6 +596,45 @@ private: void ProcessValidTx(NodeId nodeid, const CTransactionRef& tx, const std::list& replaced_transactions) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main); + /** Handle the results of package validation: calls ProcessValidTx and ProcessInvalidTx for + * individual transactions, and caches rejection for the package as a group. + * @param[in] senders Must contain the nodeids of the peers that provided each transaction + * in package, in the same order. + * */ + void ProcessPackageResult(const Package& package, const PackageMempoolAcceptResult& package_result, const std::vector& senders) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main); + + /** A package to validate */ + struct PackageToValidate { + const Package m_txns; + const std::vector m_senders; + /** Construct a 1-parent-1-child package. */ + explicit PackageToValidate(const CTransactionRef& parent, + const CTransactionRef& child, + NodeId parent_sender, + NodeId child_sender) : + m_txns{parent, child}, + m_senders {parent_sender, child_sender} + {} + + std::string ToString() const { + Assume(m_txns.size() == 2); + return strprintf("parent %s (wtxid=%s, sender=%d) + child %s (wtxid=%s, sender=%d)", + m_txns.front()->GetHash().ToString(), + m_txns.front()->GetWitnessHash().ToString(), + m_senders.front(), + m_txns.back()->GetHash().ToString(), + m_txns.back()->GetWitnessHash().ToString(), + m_senders.back()); + } + }; + + /** Look for a child of this transaction in the orphanage to form a 1-parent-1-child package, + * skipping any combinations that have already been tried. Return the resulting package along with + * the senders of its respective transactions, or std::nullopt if no package is found. */ + std::optional Find1P1CPackage(const CTransactionRef& ptx, NodeId nodeid) + EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, cs_main); + /** * Reconsider orphan transactions after a parent has been accepted to the mempool. * @@ -3198,6 +3237,117 @@ void PeerManagerImpl::ProcessValidTx(NodeId nodeid, const CTransactionRef& tx, c } } +void PeerManagerImpl::ProcessPackageResult(const Package& package, const PackageMempoolAcceptResult& package_result, const std::vector& senders) +{ + AssertLockNotHeld(m_peer_mutex); + AssertLockHeld(g_msgproc_mutex); + AssertLockHeld(cs_main); + + if (package_result.m_state.IsInvalid()) { + m_recent_rejects_reconsiderable.insert(GetPackageHash(package)); + } + // We currently only expect to process 1-parent-1-child packages. Remove if this changes. + if (!Assume(package.size() == 2)) return; + + // No package results to look through for PCKG_POLICY or PCKG_MEMPOOL_ERROR + if (package_result.m_state.GetResult() == PackageValidationResult::PCKG_POLICY || + package_result.m_state.GetResult() == PackageValidationResult::PCKG_MEMPOOL_ERROR) return; + + // Iterate backwards to erase in-package descendants from the orphanage before they become + // relevant in AddChildrenToWorkSet. + auto package_iter = package.rbegin(); + auto senders_iter = senders.rbegin(); + while (package_iter != package.rend()) { + const auto& tx = *package_iter; + const NodeId nodeid = *senders_iter; + const auto it_result{package_result.m_tx_results.find(tx->GetWitnessHash())}; + if (Assume(it_result != package_result.m_tx_results.end())) { + const auto& tx_result = it_result->second; + switch (tx_result.m_result_type) { + case MempoolAcceptResult::ResultType::VALID: + { + Assume(tx_result.m_replaced_transactions.has_value()); + std::list empty_replacement_list; + ProcessValidTx(nodeid, tx, tx_result.m_replaced_transactions.value_or(empty_replacement_list)); + break; + } + case MempoolAcceptResult::ResultType::INVALID: + case MempoolAcceptResult::ResultType::DIFFERENT_WITNESS: + { + // Don't add to vExtraTxnForCompact, as these transactions should have already been + // added there when added to the orphanage or rejected for TX_RECONSIDERABLE. + // This should be updated if package submission is ever used for transactions + // that haven't already been validated before. + ProcessInvalidTx(nodeid, tx, tx_result.m_state, /*maybe_add_extra_compact_tx=*/false); + break; + } + case MempoolAcceptResult::ResultType::MEMPOOL_ENTRY: + { + // AlreadyHaveTx() should be catching transactions that are already in mempool. + Assume(false); + break; + } + } + } + package_iter++; + senders_iter++; + } +} + +std::optional PeerManagerImpl::Find1P1CPackage(const CTransactionRef& ptx, NodeId nodeid) +{ + AssertLockNotHeld(m_peer_mutex); + AssertLockHeld(g_msgproc_mutex); + AssertLockHeld(cs_main); + + const auto& parent_wtxid{ptx->GetWitnessHash()}; + + Assume(m_recent_rejects_reconsiderable.contains(parent_wtxid.ToUint256())); + + // Prefer children from this peer. This helps prevent censorship attempts in which an attacker + // sends lots of fake children for the parent, and we (unluckily) keep selecting the fake + // children instead of the real one provided by the honest peer. + const auto cpfp_candidates_same_peer{m_orphanage.GetChildrenFromSamePeer(ptx, nodeid)}; + + // These children should be sorted from newest to oldest. In the (probably uncommon) case + // of children that replace each other, this helps us accept the highest feerate (probably the + // most recent) one efficiently. + for (const auto& child : cpfp_candidates_same_peer) { + Package maybe_cpfp_package{ptx, child}; + if (!m_recent_rejects_reconsiderable.contains(GetPackageHash(maybe_cpfp_package))) { + return PeerManagerImpl::PackageToValidate{ptx, child, nodeid, nodeid}; + } + } + + // If no suitable candidate from the same peer is found, also try children that were provided by + // a different peer. This is useful because sometimes multiple peers announce both transactions + // to us, and we happen to download them from different peers (we wouldn't have known that these + // 2 transactions are related). We still want to find 1p1c packages then. + // + // If we start tracking all announcers of orphans, we can restrict this logic to parent + child + // pairs in which both were provided by the same peer, i.e. delete this step. + const auto cpfp_candidates_different_peer{m_orphanage.GetChildrenFromDifferentPeer(ptx, nodeid)}; + + // Find the first 1p1c that hasn't already been rejected. We randomize the order to not + // create a bias that attackers can use to delay package acceptance. + // + // Create a random permutation of the indices. + std::vector tx_indices(cpfp_candidates_different_peer.size()); + std::iota(tx_indices.begin(), tx_indices.end(), 0); + Shuffle(tx_indices.begin(), tx_indices.end(), m_rng); + + for (const auto index : tx_indices) { + // If we already tried a package and failed for any reason, the combined hash was + // cached in m_recent_rejects_reconsiderable. + const auto [child_tx, child_sender] = cpfp_candidates_different_peer.at(index); + Package maybe_cpfp_package{ptx, child_tx}; + if (!m_recent_rejects_reconsiderable.contains(GetPackageHash(maybe_cpfp_package))) { + return PeerManagerImpl::PackageToValidate{ptx, child_tx, nodeid, child_sender}; + } + } + return std::nullopt; +} + bool PeerManagerImpl::ProcessOrphanTx(Peer& peer) { AssertLockHeld(g_msgproc_mutex); @@ -4377,6 +4527,20 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, RelayTransaction(tx.GetHash(), tx.GetWitnessHash()); } } + + if (m_recent_rejects_reconsiderable.contains(wtxid)) { + // When a transaction is already in m_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. + LogPrint(BCLog::TXPACKAGES, "found tx %s (wtxid=%s) in reconsiderable rejects, looking for child in orphanage\n", + txid.ToString(), wtxid.ToString()); + if (auto package_to_validate{Find1P1CPackage(ptx, pfrom.GetId())}) { + const auto package_result{ProcessNewPackage(m_chainman.ActiveChainstate(), m_mempool, package_to_validate->m_txns, /*test_accept=*/false, /*client_maxfeerate=*/std::nullopt)}; + LogDebug(BCLog::TXPACKAGES, "package evaluation for %s: %s\n", package_to_validate->ToString(), + package_result.m_state.IsValid() ? "package accepted" : "package rejected"); + ProcessPackageResult(package_to_validate->m_txns, package_result, package_to_validate->m_senders); + } + } // 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. @@ -4418,10 +4582,23 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, } 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_recent_rejects and m_recent_rejects_reconsiderable. + // We can tolerate having up to 1 parent in m_recent_rejects_reconsiderable since we + // submit 1p1c packages. However, fail immediately if any are in m_recent_rejects. + std::optional rejected_parent_reconsiderable; for (const uint256& parent_txid : unique_parents) { if (m_recent_rejects.contains(parent_txid)) { fRejectedParents = true; break; + } else if (m_recent_rejects_reconsiderable.contains(parent_txid) && !m_mempool.exists(GenTxid::Txid(parent_txid))) { + // More than 1 parent in m_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) { @@ -4435,7 +4612,9 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, // protocol for getting all unconfirmed parents. const auto gtxid{GenTxid::Txid(parent_txid)}; AddKnownTx(*peer, parent_txid); - if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) AddTxAnnouncement(pfrom, gtxid, current_time); + // Exclude m_recent_rejects_reconsiderable: the missing parent may have been + // previously rejected for being too low feerate. This orphan might CPFP it. + if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) AddTxAnnouncement(pfrom, gtxid, current_time); } if (m_orphanage.AddTx(ptx, pfrom.GetId())) { @@ -4467,6 +4646,19 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, if (state.IsInvalid()) { ProcessInvalidTx(pfrom.GetId(), ptx, state, /*maybe_add_extra_compact_tx=*/true); } + // When a transaction fails for TX_RECONSIDERABLE, look for a matching child in the + // orphanage, as it is possible that they succeed as a package. + if (state.GetResult() == TxValidationResult::TX_RECONSIDERABLE) { + LogPrint(BCLog::TXPACKAGES, "tx %s (wtxid=%s) failed but reconsiderable, looking for child in orphanage\n", + txid.ToString(), wtxid.ToString()); + if (auto package_to_validate{Find1P1CPackage(ptx, pfrom.GetId())}) { + const auto package_result{ProcessNewPackage(m_chainman.ActiveChainstate(), m_mempool, package_to_validate->m_txns, /*test_accept=*/false, /*client_maxfeerate=*/std::nullopt)}; + LogDebug(BCLog::TXPACKAGES, "package evaluation for %s: %s\n", package_to_validate->ToString(), + package_result.m_state.IsValid() ? "package accepted" : "package rejected"); + ProcessPackageResult(package_to_validate->m_txns, package_result, package_to_validate->m_senders); + } + } + return; } @@ -6076,7 +6268,9 @@ bool PeerManagerImpl::SendMessages(CNode* pto) entry.second.GetHash().ToString(), entry.first); } for (const GenTxid& gtxid : requestable) { - if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/true)) { + // Exclude m_recent_rejects_reconsiderable: we may be requesting a missing parent + // that was previously rejected for being too low feerate. + if (!AlreadyHaveTx(gtxid, /*include_reconsiderable=*/false)) { LogPrint(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());