From 9d88853e0c85f765f7d982b15e8122ede50110ed Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 7 Jan 2022 15:45:23 +0000 Subject: [PATCH] AcceptPackage fixups No behavior changes, just clarifications. --- src/validation.cpp | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index b48eb6ac3c..8c142d5908 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -596,10 +596,10 @@ private: // Submit all transactions to the mempool and call ConsensusScriptChecks to add to the script // cache - should only be called after successful validation of all transactions in the package. - // The package may end up partially-submitted after size limitting; returns true if all + // The package may end up partially-submitted after size limiting; returns true if all // transactions are successfully added to the mempool, false otherwise. - bool FinalizePackage(const ATMPArgs& args, std::vector& workspaces, PackageValidationState& package_state, - std::map& results) + bool SubmitPackage(const ATMPArgs& args, std::vector& workspaces, PackageValidationState& package_state, + std::map& results) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); // Compare a package's feerate against minimum allowed. @@ -1038,12 +1038,17 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws) return true; } -bool MemPoolAccept::FinalizePackage(const ATMPArgs& args, std::vector& workspaces, - PackageValidationState& package_state, - std::map& results) +bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& workspaces, + PackageValidationState& package_state, + std::map& results) { AssertLockHeld(cs_main); AssertLockHeld(m_pool.cs); + // Sanity check: none of the transactions should be in the mempool, and none of the transactions + // should have a same-txid-different-witness equivalent in the mempool. + assert(std::all_of(workspaces.cbegin(), workspaces.cend(), [this](const auto& ws){ + return !m_pool.exists(GenTxid::Txid(ws.m_ptx->GetHash())); })); + bool all_submitted = true; // ConsensusScriptChecks adds to the script cache and is therefore consensus-critical; // CheckInputsFromMempoolAndCache asserts that transactions only spend coins available from the @@ -1058,10 +1063,10 @@ bool MemPoolAccept::FinalizePackage(const ATMPArgs& args, std::vector // Re-calculate mempool ancestors to call addUnchecked(). They may have changed since the // last calculation done in PreChecks, since package ancestors have already been submitted. - std::string err_string; + std::string unused_err_string; if(!m_pool.CalculateMemPoolAncestors(*ws.m_entry, ws.m_ancestors, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, - m_limit_descendant_size, err_string)) { + m_limit_descendant_size, unused_err_string)) { results.emplace(ws.m_ptx->GetWitnessHash(), MempoolAcceptResult::Failure(ws.m_state)); // Since PreChecks() and PackageMempoolChecks() both enforce limits, this should never fail. all_submitted = Assume(false); @@ -1188,7 +1193,7 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: if (args.m_test_accept) return PackageMempoolAcceptResult(package_state, std::move(results)); - if (!FinalizePackage(args, workspaces, package_state, results)) { + if (!SubmitPackage(args, workspaces, package_state, results)) { package_state.Invalid(PackageValidationResult::PCKG_TX, "submission failed"); return PackageMempoolAcceptResult(package_state, std::move(results)); } @@ -1214,11 +1219,13 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, return PackageMempoolAcceptResult(package_state, {}); } - const auto& child = package[package.size() - 1]; + // IsChildWithParents() guarantees the package is > 1 transactions. + assert(package.size() > 1); // The package must be 1 child with all of its unconfirmed parents. The package is expected to // be sorted, so the last transaction is the child. + const auto& child = package.back(); std::unordered_set unconfirmed_parent_txids; - std::transform(package.cbegin(), package.end() - 1, + std::transform(package.cbegin(), package.cend() - 1, std::inserter(unconfirmed_parent_txids, unconfirmed_parent_txids.end()), [](const auto& tx) { return tx->GetHash(); }); @@ -1348,12 +1355,12 @@ PackageMempoolAcceptResult ProcessNewPackage(CChainState& active_chainstate, CTx }(); // Uncache coins pertaining to transactions that were not submitted to the mempool. - // Ensure the coins cache is still within limits. if (test_accept || result.m_state.IsInvalid()) { for (const COutPoint& hashTx : coins_to_uncache) { active_chainstate.CoinsTip().Uncache(hashTx); } } + // Ensure the coins cache is still within limits. BlockValidationState state_dummy; active_chainstate.FlushStateToDisk(state_dummy, FlushStateMode::PERIODIC); return result;