From 798cc8f5aac9bf2111ea88d4a4c3817d34e089e2 Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 15 May 2024 16:10:24 +0100 Subject: [PATCH] [refactor] move Find1P1CPackage into ProcessInvalidTx --- src/net_processing.cpp | 53 +++++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 274653dae65..1ffeb3a3ce1 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -565,8 +565,8 @@ private: bool MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer); struct PackageToValidate { - const Package m_txns; - const std::vector m_senders; + Package m_txns; + std::vector m_senders; /** Construct a 1-parent-1-child package. */ explicit PackageToValidate(const CTransactionRef& parent, const CTransactionRef& child, @@ -576,6 +576,16 @@ private: m_senders {parent_sender, child_sender} {} + // Move ctor + PackageToValidate(PackageToValidate&& other) : m_txns{std::move(other.m_txns)}, m_senders{std::move(other.m_senders)} {} + + // Move assignment + PackageToValidate& operator=(PackageToValidate&& other) { + this->m_txns = std::move(other.m_txns); + this->m_senders = std::move(other.m_senders); + return *this; + } + std::string ToString() const { Assume(m_txns.size() == 2); return strprintf("parent %s (wtxid=%s, sender=%d) + child %s (wtxid=%s, sender=%d)", @@ -589,11 +599,17 @@ private: }; /** Handle a transaction whose result was not MempoolAcceptResult::ResultType::VALID. - * @param[in] first_time_failure Whether this tx should be added to vExtraTxnForCompact. + * @param[in] first_time_failure Whether we should consider inserting into vExtraTxnForCompact, adding + * a new orphan to resolve, or looking for a package to submit. + * Set to true for transactions just received over p2p. * Set to false if the tx has already been rejected before, - * e.g. is an orphan, to avoid adding duplicate entries. - * Updates m_txrequest, m_lazy_recent_rejects, m_lazy_recent_rejects_reconsiderable, m_orphanage, and vExtraTxnForCompact. */ - void ProcessInvalidTx(NodeId nodeid, const CTransactionRef& tx, const TxValidationState& result, + * e.g. is already in the orphanage, to avoid adding duplicate entries. + * Updates m_txrequest, m_lazy_recent_rejects, m_lazy_recent_rejects_reconsiderable, m_orphanage, and vExtraTxnForCompact. + * + * @returns a PackageToValidate if this transaction has a reconsiderable failure and an eligible package was found, + * or std::nullopt otherwise. + */ + std::optional ProcessInvalidTx(NodeId nodeid, const CTransactionRef& tx, const TxValidationState& result, bool first_time_failure) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex, m_tx_download_mutex); @@ -2999,7 +3015,7 @@ void PeerManagerImpl::ProcessHeadersMessage(CNode& pfrom, Peer& peer, return; } -void PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx, const TxValidationState& state, +std::optional PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx, const TxValidationState& state, bool first_time_failure) { AssertLockNotHeld(m_peer_mutex); @@ -3017,6 +3033,8 @@ void PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx ptx->GetWitnessHash().ToString(), nodeid, state.ToString()); + // Populated if failure is reconsiderable and eligible package is found. + std::optional package_to_validate; 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 @@ -3097,7 +3115,7 @@ void PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx m_txrequest.ForgetTxHash(tx.GetWitnessHash()); } } - return; + return std::nullopt; } else if (state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) { // We can add the wtxid of this transaction to our reject filter. // Do not add txids of witness transactions or witness-stripped @@ -3117,6 +3135,14 @@ void PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx // because we should not download or submit this transaction by itself again, but may // submit it as part of a package later. RecentRejectsReconsiderableFilter().insert(ptx->GetWitnessHash().ToUint256()); + + if (first_time_failure) { + // 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. + LogDebug(BCLog::TXPACKAGES, "tx %s (wtxid=%s) failed but reconsiderable, looking for child in orphanage\n", + ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString()); + package_to_validate = Find1P1CPackage(ptx, nodeid); + } } else { RecentRejectsFilter().insert(ptx->GetWitnessHash().ToUint256()); } @@ -3147,6 +3173,8 @@ void PeerManagerImpl::ProcessInvalidTx(NodeId nodeid, const CTransactionRef& ptx if (Assume(state.GetResult() != TxValidationResult::TX_MISSING_INPUTS) && m_orphanage.EraseTx(ptx->GetWitnessHash()) > 0) { LogDebug(BCLog::TXPACKAGES, " removed orphan tx %s (wtxid=%s)\n", ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString()); } + + return package_to_validate; } void PeerManagerImpl::ProcessValidTx(NodeId nodeid, const CTransactionRef& tx, const std::list& replaced_transactions) @@ -4526,14 +4554,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, pfrom.m_last_tx_time = GetTime(); } if (state.IsInvalid()) { - ProcessInvalidTx(pfrom.GetId(), ptx, state, /*first_time_failure=*/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) { - LogDebug(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())}) { + if (auto package_to_validate{ProcessInvalidTx(pfrom.GetId(), ptx, state, /*first_time_failure=*/true)}) { 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");