[refactor] back-fill results in AcceptPackage

Instead of populating the last PackageMempoolAcceptResult with stuff
from results_final and individual_results_nonfinal, fill results_final
and create a PackageMempoolAcceptResult using that one.

A future commit will add LimitMempoolSize() which may change the status
of each of these transactions from "already in mempool" or "submitted to
mempool" to "no longer in mempool". We will change those transactions'
results here.

A future commit also gets rid of the last AcceptSubPackage outside of
the loop. It makes more sense to use results_final as the place where
all results end up.

Author: glozow

src/validation.cpp

@@ -1443,12 +1443,12 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
     m_view.SetBackend(m_dummy);
 
     LOCK(m_pool.cs);
-    // Stores final results that won't change
+    // Stores results from which we will create the returned PackageMempoolAcceptResult.
+    // A result may be changed if a mempool transaction is evicted later due to LimitMempoolSize().
     std::map<uint256, MempoolAcceptResult> results_final;
-    // Results from individual validation. "Nonfinal" because if a transaction fails by itself but
-    // succeeds later (i.e. when evaluated with a fee-bumping child), the result changes (though not
-    // reflected in this map). If a transaction fails more than once, we want to return the first
-    // result, when it was considered on its own. So changes will only be from invalid -> valid.
+    // Results from individual validation which will be returned if no other result is available for
+    // this transaction. "Nonfinal" because if a transaction fails by itself but succeeds later
+    // (i.e. when evaluated with a fee-bumping child), the result in this map may be discarded.
     std::map<uint256, MempoolAcceptResult> individual_results_nonfinal;
     bool quit_early{false};
     std::vector<CTransactionRef> txns_package_eval;
@@ -1514,32 +1514,28 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package,
         }
     }
 
-    // Quit early because package validation won't change the result or the entire package has
-    // already been submitted.
-    if (quit_early || txns_package_eval.empty()) {
-        for (const auto& [wtxid, mempoolaccept_res] : individual_results_nonfinal) {
-            Assume(results_final.emplace(wtxid, mempoolaccept_res).second);
-            Assume(mempoolaccept_res.m_result_type == MempoolAcceptResult::ResultType::INVALID);
-        }
-        return PackageMempoolAcceptResult(package_state_quit_early, std::move(results_final));
-    }
-    // Validate the (deduplicated) transactions as a package. Note that submission_result has its
-    // own PackageValidationState; package_state_quit_early is unused past this point.
-    auto submission_result = AcceptSubPackage(txns_package_eval, args);
-    // Include already-in-mempool transaction results in the final result.
-    for (const auto& [wtxid, mempoolaccept_res] : results_final) {
-        Assume(submission_result.m_tx_results.emplace(wtxid, mempoolaccept_res).second);
-        Assume(mempoolaccept_res.m_result_type != MempoolAcceptResult::ResultType::INVALID);
-    }
-    if (submission_result.m_state.GetResult() == PackageValidationResult::PCKG_TX) {
-        // Package validation failed because one or more transactions failed. Provide a result for
-        // each transaction; if a transaction doesn't have an entry in submission_result,
-        // include the previous individual failure reason.
-        submission_result.m_tx_results.insert(individual_results_nonfinal.cbegin(),
-                                              individual_results_nonfinal.cend());
-        Assume(submission_result.m_tx_results.size() == package.size());
-    }
-    return submission_result;
+    auto multi_submission_result = quit_early || txns_package_eval.empty() ? PackageMempoolAcceptResult(package_state_quit_early, {}) :
+        AcceptSubPackage(txns_package_eval, args);
+    PackageValidationState& package_state_final = multi_submission_result.m_state;
+
+    for (const auto& tx : package) {
+        const auto& wtxid = tx->GetWitnessHash();
+        if (multi_submission_result.m_tx_results.count(wtxid) > 0) {
+            // We shouldn't have re-submitted if the tx result was already in results_final.
+            Assume(results_final.count(wtxid) == 0);
+            results_final.emplace(wtxid, multi_submission_result.m_tx_results.at(wtxid));
+        } else if (const auto it{results_final.find(wtxid)}; it != results_final.end()) {
+            // Already-in-mempool transaction.
+            Assume(it->second.m_result_type != MempoolAcceptResult::ResultType::INVALID);
+            Assume(individual_results_nonfinal.count(wtxid) == 0);
+        } else if (const auto it{individual_results_nonfinal.find(wtxid)}; it != individual_results_nonfinal.end()) {
+            Assume(it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID);
+            // Interesting result from previous processing.
+            results_final.emplace(wtxid, it->second);
+        }
+    }
+    Assume(results_final.size() == package.size());
+    return PackageMempoolAcceptResult(package_state_final, std::move(results_final));
 }
 
 } // anon namespace