From 83f814b1d1100baac9dca9c176f89b0ec2555dbc Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Thu, 24 Oct 2024 20:29:42 -0400 Subject: [PATCH] Remove m_all_conflicts from SubPackageState --- src/txmempool.h | 2 ++ src/validation.cpp | 26 ++++++++++++-------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/txmempool.h b/src/txmempool.h index 3c8aafbac5..fe8f89d43c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -804,6 +804,8 @@ public: 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); } + const CTxMemPool::setEntries& GetRemovals() const { return m_to_remove; } + util::Result CalculateMemPoolAncestors(TxHandle tx, const Limits& limits) { // Look up transaction in our cache first diff --git a/src/validation.cpp b/src/validation.cpp index 779f15b538..a76f56f1ef 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -635,7 +635,7 @@ private: CTxMemPool::setEntries m_ancestors; /* Handle to the tx in the changeset */ CTxMemPool::ChangeSet::TxHandle m_tx_handle; - /** Whether RBF-related data structures (m_conflicts, m_iters_conflicting, m_all_conflicting, + /** Whether RBF-related data structures (m_conflicts, m_iters_conflicting, * m_replaced_transactions) include a sibling in addition to txns with conflicting inputs. */ bool m_sibling_eviction{false}; @@ -739,8 +739,6 @@ private: /** Whether the transaction(s) would replace any mempool transactions and/or evict any siblings. * If so, RBF rules apply. */ bool m_rbf{false}; - /** All directly conflicting mempool transactions and their descendants. */ - CTxMemPool::setEntries m_all_conflicts; /** Mempool transactions that were replaced. */ std::list m_replaced_transactions; /* Changeset representing adding transactions and removing their conflicts. */ @@ -1089,13 +1087,15 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) strprintf("insufficient fee%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string); } + CTxMemPool::setEntries all_conflicts; + // Calculate all conflicting entries and enforce Rule #5. - if (const auto err_string{GetEntriesForConflicts(tx, m_pool, ws.m_iters_conflicting, m_subpackage.m_all_conflicts)}) { + if (const auto err_string{GetEntriesForConflicts(tx, m_pool, ws.m_iters_conflicting, all_conflicts)}) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, strprintf("too many potential replacements%s", ws.m_sibling_eviction ? " (including sibling eviction)" : ""), *err_string); } // Enforce Rule #2. - if (const auto err_string{HasNoNewUnconfirmed(tx, m_pool, m_subpackage.m_all_conflicts)}) { + if (const auto err_string{HasNoNewUnconfirmed(tx, m_pool, all_conflicts)}) { // Sibling eviction is only done for TRUC transactions, which cannot have multiple ancestors. Assume(!ws.m_sibling_eviction); return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, @@ -1104,7 +1104,7 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) // Check if it's economically rational to mine this transaction rather than the ones it // replaces and pays for its own relay fees. Enforce Rules #3 and #4. - for (CTxMemPool::txiter it : m_subpackage.m_all_conflicts) { + for (CTxMemPool::txiter it : all_conflicts) { m_subpackage.m_conflicting_fees += it->GetModifiedFee(); m_subpackage.m_conflicting_size += it->GetTxSize(); } @@ -1116,7 +1116,7 @@ bool MemPoolAccept::ReplacementChecks(Workspace& ws) } // Add all the to-be-removed transactions to the changeset. - for (auto it : m_subpackage.m_all_conflicts) { + for (auto it : all_conflicts) { m_subpackage.m_changeset->StageRemoval(it); } return true; @@ -1172,14 +1172,15 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector& txn // Don't consider replacements that would cause us to remove a large number of mempool entries. // This limit is not increased in a package RBF. Use the aggregate number of transactions. + CTxMemPool::setEntries all_conflicts; if (const auto err_string{GetEntriesForConflicts(*child_ws.m_ptx, m_pool, direct_conflict_iters, - m_subpackage.m_all_conflicts)}) { + all_conflicts)}) { return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: too many potential replacements", *err_string); } - for (CTxMemPool::txiter it : m_subpackage.m_all_conflicts) { + for (CTxMemPool::txiter it : all_conflicts) { m_subpackage.m_changeset->StageRemoval(it); m_subpackage.m_conflicting_fees += it->GetModifiedFee(); m_subpackage.m_conflicting_size += it->GetTxSize(); @@ -1287,9 +1288,9 @@ void MemPoolAccept::FinalizeSubpackage(const ATMPArgs& args) AssertLockHeld(cs_main); AssertLockHeld(m_pool.cs); - if (!m_subpackage.m_all_conflicts.empty()) Assume(args.m_allow_replacement); + if (!m_subpackage.m_changeset->GetRemovals().empty()) Assume(args.m_allow_replacement); // Remove conflicting transactions from the mempool - for (CTxMemPool::txiter it : m_subpackage.m_all_conflicts) + for (CTxMemPool::txiter it : m_subpackage.m_changeset->GetRemovals()) { std::string log_string = strprintf("replacing mempool tx %s (wtxid=%s, fees=%s, vsize=%s). ", it->GetTx().GetHash().ToString(), @@ -1329,9 +1330,6 @@ void MemPoolAccept::FinalizeSubpackage(const ATMPArgs& args) } m_subpackage.m_changeset->Apply(); m_subpackage.m_changeset.reset(); - // Don't attempt to process the same conflicts repeatedly during subpackage evaluation: - // they no longer exist on subsequent calls to Finalize() post-Apply() - m_subpackage.m_all_conflicts.clear(); } bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector& workspaces,