Merge #21062: refactor: return MempoolAcceptResult from ATMP

53e716ea119658c28935fee24eb50090907c500e [refactor] improve style for touched code (gzhao408)
174cb5330af4b09f3a66974d3bae783ea43b190e [refactor] const ATMPArgs and non-const Workspace (gzhao408)
f82baf0762f60c2ca5ffc339b095f9271d7c2f33 [refactor] return MempoolAcceptResult (gzhao408)
9db10a55061e09021ff8ea1d6637d99f7959035f [refactor] clean up logic in testmempoolaccept (gzhao408)

Pull request description:

  This is the first 4 commits of #20833, and does refactoring only. It should be relatively simple to review, and offers a few nice things:
  - It makes accessing values that don't make sense (e.g. fee) when the tx is invalid an error.
  - Returning `MempoolAcceptResult` from ATMP makes the interface cleaner. The caller can get a const instead of passing in a mutable "out" param.
  - We don't have to be iterating through a bunch of lists for package validation, we can just return a `std::vector<MempoolAcceptResult>`.
  - We don't have to refactor all ATMP call sites again if/when we want to return more stuff from it.

ACKs for top commit:
  MarcoFalke:
    ACK 53e716ea119658c28935fee24eb50090907c500e 💿
  jnewbery:
    Code review ACK 53e716ea119658c28935fee24eb50090907c500e
  ariard:
    Code Review ACK 53e716e, I did tweak a bit the touched paths to see if we had good test coverage. Didn't find holes.

Tree-SHA512: fa6ec324a08ad9e6e55948615cda324cba176255708bf0a0a0f37cedb7a75311aa334ac6f223be7d8df3c7379502b1081102b9589f9a9afa1713ad3d9ab3c24f

1 files changed, 8 insertions, 8 deletions

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 6ee9846c52..4340eb120c 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -2178,10 +2178,10 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
         if (orphan_it == mapOrphanTransactions.end()) continue;
 
         const CTransactionRef porphanTx = orphan_it->second.tx;
-        TxValidationState state;
-        std::list<CTransactionRef> removed_txn;
+        const MempoolAcceptResult result = AcceptToMemoryPool(m_mempool, porphanTx, false /* bypass_limits */);
+        const TxValidationState& state = result.m_state;
 
-        if (AcceptToMemoryPool(m_mempool, state, porphanTx, &removed_txn, false /* bypass_limits */)) {
+        if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
             LogPrint(BCLog::MEMPOOL, "   accepted orphan tx %s\n", orphanHash.ToString());
             RelayTransaction(orphanHash, porphanTx->GetWitnessHash(), m_connman);
             for (unsigned int i = 0; i < porphanTx->vout.size(); i++) {
@@ -2193,7 +2193,7 @@ void PeerManagerImpl::ProcessOrphanTx(std::set<uint256>& orphan_work_set)
                 }
             }
             EraseOrphanTx(orphanHash);
-            for (const CTransactionRef& removedTx : removed_txn) {
+            for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) {
                 AddToCompactExtraTransactions(removedTx);
             }
             break;
@@ -3200,10 +3200,10 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
             return;
         }
 
-        TxValidationState state;
-        std::list<CTransactionRef> lRemovedTxn;
+        const MempoolAcceptResult result = AcceptToMemoryPool(m_mempool, ptx, false /* bypass_limits */);
+        const TxValidationState& state = result.m_state;
 
-        if (AcceptToMemoryPool(m_mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */)) {
+        if (result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
             m_mempool.check(&::ChainstateActive().CoinsTip());
             // As this version of the transaction was acceptable, we can forget about any
             // requests for it.
@@ -3226,7 +3226,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
                 tx.GetHash().ToString(),
                 m_mempool.size(), m_mempool.DynamicMemoryUsage() / 1000);
 
-            for (const CTransactionRef& removedTx : lRemovedTxn) {
+            for (const CTransactionRef& removedTx : result.m_replaced_transactions.value()) {
                 AddToCompactExtraTransactions(removedTx);
             }