From 284a1d33f1dcbc3b3404ea40a948ff6600239613 Mon Sep 17 00:00:00 2001
From: Suhas Daftuar <sdaftuar@chaincode.com>
Date: Wed, 16 Oct 2024 16:36:36 -0400
Subject: [PATCH] Move prioritisation into changeset

---
 src/test/rbf_tests.cpp |  5 ++++-
 src/txmempool.cpp      | 15 +++++----------
 src/validation.cpp     |  7 +++----
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp
index eeb1061c8f1aa..49c707511e85f 100644
--- a/src/test/rbf_tests.cpp
+++ b/src/test/rbf_tests.cpp
@@ -362,7 +362,7 @@ BOOST_FIXTURE_TEST_CASE(improves_feerate, TestChain100Setup)
     const CAmount normal_fee{CENT/10};
 
     // low feerate parent with normal feerate child
-    const auto tx1 = make_tx(/*inputs=*/ {m_coinbase_txns[0]}, /*output_values=*/ {10 * COIN});
+    const auto tx1 = make_tx(/*inputs=*/ {m_coinbase_txns[0], m_coinbase_txns[1]}, /*output_values=*/ {10 * COIN});
     AddToMempool(pool, entry.Fee(low_fee).FromTx(tx1));
     const auto tx2 = make_tx(/*inputs=*/ {tx1}, /*output_values=*/ {995 * CENT});
     AddToMempool(pool, entry.Fee(normal_fee).FromTx(tx2));
@@ -374,6 +374,9 @@ BOOST_FIXTURE_TEST_CASE(improves_feerate, TestChain100Setup)
     const auto tx2_fee = entry2->GetModifiedFee();
     const auto tx2_size = entry2->GetTxSize();
 
+    // conflicting transactions
+    const auto tx1_conflict = make_tx(/*inputs=*/ {m_coinbase_txns[0], m_coinbase_txns[2]}, /*output_values=*/ {10 * COIN});
+
     // Now test ImprovesFeerateDiagram with various levels of "package rbf" feerates
 
     // It doesn't improve itself
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 6d2920ece51ee..13ff480492082 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -478,16 +478,6 @@ void CTxMemPool::addNewTransaction(CTxMemPool::txiter newit, CTxMemPool::setEntr
 {
     const CTxMemPoolEntry& entry = *newit;
 
-    // Update transaction for any feeDelta created by PrioritiseTransaction
-    // TODO: move this into the changeset instead.
-    CAmount delta{0};
-    ApplyDelta(newit->GetTx().GetHash(), delta);
-    // The following call to UpdateModifiedFee assumes no previous fee modifications
-    Assume(newit->GetFee() == newit->GetModifiedFee());
-    if (delta) {
-        mapTx.modify(newit, [&delta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(delta); });
-    }
-
     // Update cachedInnerUsage to include contained transaction's usage.
     // (When we update the entry for in-mempool parents, memory usage will be
     // further updated.)
@@ -1411,8 +1401,13 @@ util::Result<std::pair<std::vector<FeeFrac>, std::vector<FeeFrac>>> CTxMemPool::
 
 CTxMemPool::ChangeSet::TxHandle CTxMemPool::ChangeSet::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)
 {
+    LOCK(m_pool->cs);
     Assume(m_to_add.find(tx->GetHash()) == m_to_add.end());
     auto newit = m_to_add.emplace(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); });
+
     m_entry_vec.push_back(newit);
     return newit;
 }
diff --git a/src/validation.cpp b/src/validation.cpp
index 8adc76d72cc96..24ae4f5db1117 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -889,10 +889,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
 
     int64_t nSigOpsCost = GetTransactionSigOpCost(tx, m_view, STANDARD_SCRIPT_VERIFY_FLAGS);
 
-    // ws.m_modified_fees includes any fee deltas from PrioritiseTransaction
-    ws.m_modified_fees = ws.m_base_fees;
-    m_pool.ApplyDelta(hash, ws.m_modified_fees);
-
     // Keep track of transactions that spend a coinbase, which we re-scan
     // during reorgs to ensure COINBASE_MATURITY is still met.
     bool fSpendsCoinbase = false;
@@ -912,6 +908,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
     }
     ws.m_tx_handle = m_subpackage.m_changeset->StageAddition(ptx, ws.m_base_fees, nAcceptTime, m_active_chainstate.m_chain.Height(), entry_sequence, fSpendsCoinbase, nSigOpsCost, lock_points.value());
 
+    // ws.m_modified_fees includes any fee deltas from PrioritiseTransaction
+    ws.m_modified_fees = ws.m_tx_handle->GetModifiedFee();
+
     ws.m_vsize = ws.m_tx_handle->GetTxSize();
 
     // Enforces 0-fee for dust transactions, no incentive to be mined alone