diff --git a/src/txmempool.cpp b/src/txmempool.cpp index c931b361d4..dc8ec8ed5a 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -435,6 +435,8 @@ void CTxMemPool::AddTransactionsUpdated(unsigned int n) void CTxMemPool::Apply(ChangeSet* changeset) { AssertLockHeld(cs); + m_txgraph->CommitStaging(); + RemoveStaged(changeset->m_to_remove, false, MemPoolRemovalReason::REPLACED); for (size_t i=0; im_entry_vec.size(); ++i) { @@ -891,7 +893,10 @@ void CTxMemPool::PrioritiseTransaction(const Txid& hash, const CAmount& nFeeDelt delta = SaturatingAdd(delta, nFeeDelta); txiter it = mapTx.find(hash); if (it != mapTx.end()) { + // PrioritiseTransaction calls stack on previous ones. Set the new + // transaction fee to be current modified fee + feedelta. mapTx.modify(it, [&nFeeDelta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(nFeeDelta); }); + m_txgraph->SetTransactionFee(*it, it->GetModifiedFee()); // Now update all ancestors' modified fees with descendants auto ancestors{AssumeCalculateMemPoolAncestors(__func__, *it, Limits::NoLimits(), /*fSearchForParents=*/false)}; for (txiter ancestorIt : ancestors) { @@ -1387,15 +1392,28 @@ CTxMemPool::ChangeSet::TxHandle CTxMemPool::ChangeSet::StageAddition(const CTran { LOCK(m_pool->cs); Assume(m_to_add.find(tx->GetHash()) == m_to_add.end()); - auto newit = m_to_add.emplace(TxGraph::Ref(), tx, fee, time, entry_height, entry_sequence, spends_coinbase, sigops_cost, lp).first; + CAmount delta{0}; m_pool->ApplyDelta(tx->GetHash(), delta); - if (delta) m_to_add.modify(newit, [&delta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(delta); }); + + TxGraph::Ref ref(m_pool->m_txgraph->AddTransaction(FeePerWeight(fee, GetSigOpsAdjustedWeight(GetTransactionWeight(*tx), sigops_cost, ::nBytesPerSigOp)))); + auto newit = m_to_add.emplace(std::move(ref), tx, fee, time, entry_height, entry_sequence, spends_coinbase, sigops_cost, lp).first; + if (delta) { + m_to_add.modify(newit, [&delta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(delta); }); + m_pool->m_txgraph->SetTransactionFee(*newit, newit->GetModifiedFee()); + } m_entry_vec.push_back(newit); return newit; } +void CTxMemPool::ChangeSet::StageRemoval(CTxMemPool::txiter it) +{ + LOCK(m_pool->cs); + m_pool->m_txgraph->RemoveTransaction(*it); + m_to_remove.insert(it); +} + void CTxMemPool::ChangeSet::Apply() { LOCK(m_pool->cs); diff --git a/src/txmempool.h b/src/txmempool.h index bc6eb36c84..acf5598404 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -371,6 +371,7 @@ public: * the mempool is consistent with the new chain tip and fully populated. */ mutable RecursiveMutex cs; + std::unique_ptr m_txgraph GUARDED_BY(cs); indexed_transaction_set mapTx GUARDED_BY(cs); using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator; @@ -382,7 +383,6 @@ public: uint64_t CalculateDescendantMaximum(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); private: - std::unique_ptr m_txgraph GUARDED_BY(cs); typedef std::map cacheMap; @@ -822,8 +822,14 @@ public: */ class ChangeSet { public: - explicit ChangeSet(CTxMemPool* pool) : m_pool(pool) {} - ~ChangeSet() EXCLUSIVE_LOCKS_REQUIRED(m_pool->cs) { m_pool->m_have_changeset = false; } + explicit ChangeSet(CTxMemPool* pool) : m_pool(pool) { m_pool->m_txgraph->StartStaging(); } + ~ChangeSet() EXCLUSIVE_LOCKS_REQUIRED(m_pool->cs) { + AssertLockHeld(m_pool->cs); + if (m_pool->m_txgraph->HaveStaging()) { + m_pool->m_txgraph->AbortStaging(); + } + m_pool->m_have_changeset = false; + } ChangeSet(const ChangeSet&) = delete; ChangeSet& operator=(const ChangeSet&) = delete; @@ -831,7 +837,8 @@ public: using TxHandle = CTxMemPool::txiter; TxHandle StageAddition(const CTransactionRef& tx, const CAmount fee, int64_t time, unsigned int entry_height, uint64_t entry_sequence, bool spends_coinbase, int64_t sigops_cost, LockPoints lp); - void StageRemoval(CTxMemPool::txiter it) { m_to_remove.insert(it); } + + void StageRemoval(CTxMemPool::txiter it); const CTxMemPool::setEntries& GetRemovals() const { return m_to_remove; } diff --git a/src/validation.cpp b/src/validation.cpp index 7f97f99b47..25831db6fb 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -591,14 +591,26 @@ public: void CleanupTemporaryCoins() EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); // Single transaction acceptance - MempoolAcceptResult AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + MempoolAcceptResult AcceptSingleTransactionAndCleanup(const CTransactionRef& ptx, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { + LOCK(m_pool.cs); + MempoolAcceptResult result = AcceptSingleTransactionInternal(ptx, args); + ClearSubPackageState(); + return result; + } + MempoolAcceptResult AcceptSingleTransactionInternal(const CTransactionRef& ptx, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); /** * Multiple transaction acceptance. Transactions may or may not be interdependent, but must not * conflict with each other, and the transactions cannot already be in the mempool. Parents must * come before children if any dependencies exist. */ - PackageMempoolAcceptResult AcceptMultipleTransactions(const std::vector& txns, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + PackageMempoolAcceptResult AcceptMultipleTransactionsAndCleanup(const std::vector& txns, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { + LOCK(m_pool.cs); + PackageMempoolAcceptResult result = AcceptMultipleTransactionsInternal(txns, args); + ClearSubPackageState(); + return result; + } + PackageMempoolAcceptResult AcceptMultipleTransactionsInternal(const std::vector& txns, ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs); /** * Submission of a subpackage. @@ -1423,10 +1435,10 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& return all_submitted; } -MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef& ptx, ATMPArgs& args) +MempoolAcceptResult MemPoolAccept::AcceptSingleTransactionInternal(const CTransactionRef& ptx, ATMPArgs& args) { AssertLockHeld(cs_main); - LOCK(m_pool.cs); // mempool "read lock" (held through m_pool.m_opts.signals->TransactionAddedToMempool()) + AssertLockHeld(m_pool.cs); Workspace ws(ptx); const std::vector single_wtxid{ws.m_ptx->GetWitnessHash()}; @@ -1515,9 +1527,10 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef effective_feerate, single_wtxid); } -PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::vector& txns, ATMPArgs& args) +PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactionsInternal(const std::vector& txns, ATMPArgs& args) { AssertLockHeld(cs_main); + AssertLockHeld(m_pool.cs); // These context-free package limits can be done before taking the mempool lock. PackageValidationState package_state; @@ -1529,8 +1542,6 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: [](const auto& tx) { return Workspace(tx); }); std::map results; - LOCK(m_pool.cs); - // Do all PreChecks first and fail fast to avoid running expensive script checks when unnecessary. for (Workspace& ws : workspaces) { if (!PreChecks(args, ws)) { @@ -1681,11 +1692,11 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptSubPackage(const std::vector 1) { - return AcceptMultipleTransactions(subpackage, args); + return AcceptMultipleTransactionsInternal(subpackage, args); } const auto& tx = subpackage.front(); ATMPArgs single_args = ATMPArgs::SingleInPackageAccept(args); - const auto single_res = AcceptSingleTransaction(tx, single_args); + const auto single_res = AcceptSingleTransactionInternal(tx, single_args); PackageValidationState package_state_wrapped; if (single_res.m_result_type != MempoolAcceptResult::ResultType::VALID) { package_state_wrapped.Invalid(PackageValidationResult::PCKG_TX, "transaction failed"); @@ -1863,8 +1874,10 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra CTxMemPool& pool{*active_chainstate.GetMempool()}; std::vector coins_to_uncache; + auto args = MemPoolAccept::ATMPArgs::SingleAccept(chainparams, accept_time, bypass_limits, coins_to_uncache, test_accept); - MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransaction(tx, args); + MempoolAcceptResult result = MemPoolAccept(pool, active_chainstate).AcceptSingleTransactionAndCleanup(tx, args); + if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) { // Remove coins that were not present in the coins cache before calling // AcceptSingleTransaction(); this is to prevent memory DoS in case we receive a large @@ -1897,7 +1910,7 @@ PackageMempoolAcceptResult ProcessNewPackage(Chainstate& active_chainstate, CTxM AssertLockHeld(cs_main); if (test_accept) { auto args = MemPoolAccept::ATMPArgs::PackageTestAccept(chainparams, GetTime(), coins_to_uncache); - return MemPoolAccept(pool, active_chainstate).AcceptMultipleTransactions(package, args); + return MemPoolAccept(pool, active_chainstate).AcceptMultipleTransactionsAndCleanup(package, args); } else { auto args = MemPoolAccept::ATMPArgs::PackageChildWithParents(chainparams, GetTime(), coins_to_uncache, client_maxfeerate); return MemPoolAccept(pool, active_chainstate).AcceptPackage(package, args);